Show reopen button for registration phase, if submission or TG submission phase is open (PS-445) #1303
Conversation
…sion phase is open
Show reopen button for registration phase, if submission or TG submission phase is open
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| submission, | ||
| })) | ||
|
|
||
| entries.sort((a: SubmissionScoreEntry, b: SubmissionScoreEntry) => { |
There was a problem hiding this comment.
[performance]
The sorting logic in sortSubmissionsByReviewScoreDesc could be optimized by using a single comparison function. Consider combining the checks for scoreA and scoreB into one comparison to reduce the number of conditional branches.
| const selectedTab = props.selectedTab | ||
| const providedReviews = props.reviews | ||
| const providedSubmitterReviews = props.submitterReviews | ||
| const normalizedSelectedTab = useMemo( |
There was a problem hiding this comment.
[💡 performance]
The use of useMemo for normalizedSelectedTab is appropriate, but ensure that the performance benefit justifies the added complexity. If selectedTab changes infrequently, the memoization might not provide significant gains.
| () => normalizeTabLabel(selectedTab), | ||
| [selectedTab], | ||
| ) | ||
| const shouldSortReviewTabByScore = useMemo( |
There was a problem hiding this comment.
[💡 performance]
The useMemo hook for shouldSortReviewTabByScore is used correctly, but consider if the logic is complex enough to warrant memoization. If the computation is trivial, memoization might add unnecessary complexity.
| [resolvedSubmitterReviews], | ||
| ) | ||
| const reviewerRowsForReviewTab = useMemo( | ||
| () => (shouldSortReviewTabByScore |
There was a problem hiding this comment.
[performance]
Ensure that sortSubmissionsByReviewScoreDesc is efficient for large datasets. Sorting can be costly, and if filteredReviews or filteredSubmitterReviews are large, this could impact performance.
| className?: string | ||
| } | ||
|
|
||
| const normalizePhaseName = (name?: string): string => (name ? name.trim() |
There was a problem hiding this comment.
[💡 maintainability]
Consider adding a return type annotation to normalizePhaseName for better type safety and clarity.
| challengePhases.forEach(phase => { | ||
| if (!phase?.isOpen || !phase.predecessor) { | ||
| const addPhaseIdentifiers = (phase?: BackendPhase): void => { | ||
| if (!phase) { |
There was a problem hiding this comment.
[performance]
The addPhaseIdentifiers function is defined inside useMemo, which could lead to unnecessary re-creations of the function on every render. Consider moving it outside to improve performance.
| }) | ||
|
|
||
| const hasSubmissionVariantOpen = challengePhases.some(phase => ( | ||
| phase?.isOpen && SUBMISSION_PHASE_NAMES.has(normalizePhaseName(phase.name)) |
There was a problem hiding this comment.
[💡 performance]
The normalizePhaseName function is called multiple times in the loop. Consider storing the result in a variable to avoid redundant computations.
Sort reviews on past-challenges by review score in descending order
Show reopen button for registration phase, if submission or TG submission phase is open
https://topcoder.atlassian.net/browse/PS-445