Fix an issue where we were making bad requests to load scorecards for… #1705
Conversation
… the reviewers section
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| const normalizeTrackForScorecards = (challenge, metadata) => { | ||
| const normalize = (value) => { | ||
| if (!value) return null | ||
| const normalized = value.toString().trim().toUpperCase().replace(/\s+/g, '_') |
There was a problem hiding this comment.
[correctness]
The use of replace(/\s+/g, '_') could lead to unexpected behavior if the input contains multiple spaces. Consider using replace(/\s+/g, '_') to ensure consistent normalization.
|
|
||
| if (!challenge) return null | ||
|
|
||
| if (challenge.trackId && metadata && Array.isArray(metadata.challengeTracks)) { |
There was a problem hiding this comment.
[correctness]
The function normalizeTrackForScorecards returns null if challenge is falsy, but this might not be the desired behavior if metadata is provided. Consider handling the case where metadata is valid even if challenge is not.
|
|
||
| loadScorecards () { | ||
| const { challenge, loadScorecards } = this.props | ||
| const { challenge, loadScorecards, metadata } = this.props |
There was a problem hiding this comment.
[❗❗ correctness]
The loadScorecards function now depends on metadata, but it doesn't check if metadata is defined before using it. Consider adding a check to ensure metadata is not undefined to avoid potential runtime errors.
| workflows: PropTypes.array, | ||
| resourceRoles: PropTypes.array | ||
| resourceRoles: PropTypes.array, | ||
| challengeTracks: PropTypes.array |
There was a problem hiding this comment.
[maintainability]
The addition of challengeTracks to PropTypes suggests that it is used in the component, but there is no validation for its presence when used in normalizeTrackForScorecards. Consider adding validation to ensure challengeTracks is provided when expected.
… the reviewers section