Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
src/apps/review/src/lib/components/ChallengeDetailsContent/TabContentSubmissions.tsx
Show resolved
Hide resolved
src/apps/review/src/lib/components/ChallengeDetailsContent/TabContentSubmissions.tsx
Show resolved
Hide resolved
|
|
||
| return ( | ||
| <div> | ||
| <a href={profileUrl} style={{ color: resolvedColor }} target='_blank' rel='noreferrer'> |
There was a problem hiding this comment.
[❗❗ security]
The target='_blank' attribute is used without rel='noopener noreferrer', which can pose a security risk by allowing the new page to access the window.opener property. Ensure rel='noopener noreferrer' is included to mitigate this risk.
| () => challengeSubmissions.map(convertBackendSubmissionToSubmissionInfo), | ||
| [challengeSubmissions], | ||
| () => challengeSubmissions.map(s => convertBackendSubmissionToSubmissionInfo(s, registrants)), | ||
| [challengeSubmissions, registrants], |
There was a problem hiding this comment.
[performance]
Adding registrants to the dependency array of useMemo is correct to ensure that submissionInfos is recalculated when registrants changes. However, ensure that registrants is stable and doesn't change unnecessarily, as this could lead to unnecessary recalculations.
| */ | ||
| export function convertBackendSubmissionToSubmissionInfo( | ||
| data: BackendSubmission, | ||
| registrants?: BackendResource[] | undefined, |
There was a problem hiding this comment.
[correctness]
The registrants parameter is marked as optional, but the function logic assumes it can be an array. Consider handling the case where registrants is undefined more explicitly to avoid potential runtime errors.
| const registrantMap = new Map<string, BackendResource>() | ||
| if (Array.isArray(registrants)) { | ||
| registrants.forEach(r => { | ||
| if (r?.memberId !== undefined && r?.memberId !== null) { |
There was a problem hiding this comment.
[💡 readability]
The check r?.memberId !== undefined && r?.memberId !== null can be simplified to r?.memberId != null to handle both undefined and null in a more concise way.
| submittedDate, | ||
| submittedDateString, | ||
| type: data.type, | ||
| userInfo: registrantMap.get(data.memberId), |
There was a problem hiding this comment.
[correctness]
The use of registrantMap.get(data.memberId) assumes that data.memberId is always a valid key in the map. Consider adding a fallback or error handling for cases where data.memberId might not exist in registrantMap.
| <a href={profileUrl} style={{ color: resolvedColor }} target='_blank' rel='noreferrer'> | ||
| {handle} | ||
| </a> | ||
| {userInfo?.memberEmail ? ( |
There was a problem hiding this comment.
[💡 maintainability]
The condition userInfo?.memberEmail is checked twice, once in the ternary condition and again inside the block. Consider simplifying by removing the redundant check.
2 participants
Related JIRA Ticket:
https://topcoder.atlassian.net/browse/PM-2587
What's in this PR?
Render submitter info in submission tab for copilot & PM