Conversation
What was broken Marathon Match submissions without review summations were omitted from the submissions tab, so newer uploads could exist in the challenge data but never render in the list. Root cause The client rebuilt Marathon Match submission rows only from review summations. Any raw submission record that had not produced a summation yet was dropped from the derived data. What was changed Updated the Marathon Match submission builder to merge raw challenge submissions with review summations, preserve member metadata, and keep unscored attempts visible in submission history. Wired challenge detail state to pass raw submissions into that builder. Any added/updated tests Added utility coverage for raw Marathon Match submissions without summations and for newer raw submissions that should remain visible alongside scored attempts.
What was broken - Marathon Match provisional scores were showing under Final Score in My Submissions before review was complete. - The public Submissions tab could show stale provisional scores of 0 even when the scored attempt was 100. Root cause - The MM normalization path preferred raw provisionalScore values over initialScore, even when provisionalScore was stale. - My Submissions rendered finalScore directly instead of treating the review phase as the point when final scores become visible. What was changed - Prefer initialScore when normalizing MM provisional scores from raw submissions and normalized attempts. - Added a display helper in My Submissions so provisional scores come from the initial score and final scores stay hidden until review completes. - Tightened the existing header test fixture so the full Jest suite no longer confuses the registration deadline text with the Register action. Any added/updated tests - Added a regression test for stale MM provisional scores in mm-review-summations. - Added My Submissions display tests covering pre-review and post-review score rendering. - Updated the challenge-detail header action test fixture used by the full Jest suite.
What was broken Task challenge detail pages could still render Register, Unregister, and Submit a solution actions for users who should not see them. Root cause (if identifiable) The challenge detail header only treated challenges with a Task type name as tasks, while tasks created from the newer work app are identified through task metadata on the payload instead. What was changed Updated the challenge detail header to reuse task detection that also respects task metadata on the challenge payload, so task pages no longer render the register, unregister, or submit actions. Resolved the conflicted header/test blocks so the header keeps the current action rendering path and the task guard applies consistently. Any added/updated tests Updated the challenge detail header tests to cover both classic task challenges and work-app task payloads, and to confirm non-task challenges still show the actions.
What was broken Marathon Match challenge detail pages kept rendering final scores and final ranks as N/A until the Review phase closed, even when review summations had already produced final results during an active review/system-test phase. My Submissions had the same problem for final scores. Root cause The UI gated all final Marathon Match results on review-phase completion instead of on actual result availability. When system tests were configured to run in Review, final scores could already exist in the submission payload before the phase closed. What was changed Added a small challenge-detail utility that detects when final Marathon Match results are already present in the loaded submissions. Updated Marathon Match submission rows, history rows, and the submission details modal to show final ranks and scores as soon as final results exist, and to default MM sorting to final rank when those results are visible. Updated My Submissions score display to surface final scores when they already exist, while preserving the existing provisional-score behavior when no final result is available. Any added/updated tests Added utility coverage for review-phase completion and early final-result visibility. Added My Submissions score-display tests for active review and completed review cases. Included the existing MM review summation test in the focused Jest run to confirm the upstream data shaping still passes.
PM-4662: fix MM provisional score display
PM-4648: hide task actions in challenge header
What was broken The PM-4648 header fix already handled classic, work-app, and pure V5 task payloads, but the merged regression coverage only exercised the classic and work-app cases. Root cause (if identifiable) The follow-up pure V5 task coverage from the closed PM-4648-2 branch never landed after the main behavior fix merged. What was changed Updated the challenge detail header test fixture to match the current prop shapes used by the component. Split the task action assertions so classic type-only tasks, work-app task metadata, and pure V5 task metadata are each covered explicitly. Any added/updated tests Updated __tests__/shared/components/challenge-detail/Header/index.jsx to cover classic task, work-app task, pure V5 task, and non-task challenge header actions.
PM-4608: show MM final scores during active review
PM-4648: add missing task header regression coverage
PM-4720 Update to v6
PM-4720 Fix page payload for copilots api
There was a problem hiding this comment.
Pull request overview
This release PR updates Marathon Match (MM) submissions/final-results handling and adjusts a few challenge-detail behaviors and services for production.
Changes:
- Enhance MM submissions building by merging review summations with raw submission records (so attempts without summations can still render).
- Introduce shared MM “final results visibility” logic and wire it into challenge-detail submissions + my-submissions views.
- Misc. production updates: Wipro registration guard exception for Topgear Task, copilot opportunities API switch to v6, task challenge header action hiding, Trivy action bump.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/shared/utils/mm-review-summations.js | Adds raw-submission merging, new helpers for submission normalization, and a new update path for entries sourced from raw submissions. |
| src/shared/utils/challenge-detail/mm-final-results.js | New utility module to decide when final MM scores/ranks should be visible. |
| src/shared/services/copilotOpportunities.js | Switches opportunities endpoint base from V5 to V6 and sanitizes page parameter. |
| src/shared/containers/challenge-detail/index.jsx | Updates Wipro registration guard signature/behavior and rebuilds MM submissions using raw submissions; normalizes initial score usage. |
| src/shared/components/challenge-detail/Submissions/index.jsx | Uses new “should show final MM results” logic and propagates renamed prop to row/modal components. |
| src/shared/components/challenge-detail/Submissions/SubmissionRow/index.jsx | Renames isReviewPhaseComplete to showFinalResults and updates final score/rank display logic. |
| src/shared/components/challenge-detail/Submissions/SubmissionRow/SubmissionHistoryRow/index.jsx | Same prop rename; final-score display gated by showFinalResults. |
| src/shared/components/challenge-detail/Submissions/SubmissionInformationModal/index.jsx | Same prop rename; hides final score unless showFinalResults. |
| src/shared/components/challenge-detail/MySubmissions/SubmissionsList/index.jsx | Adds getDisplayedScores to prefer initialScore and hide final score until appropriate. |
| src/shared/components/challenge-detail/Header/index.jsx | Hides registration/submission actions for task challenges and guards phase timer rendering. |
| tests/shared/utils/mm-review-summations.test.js | Adds coverage for merging raw submissions with summations and score preference rules. |
| tests/shared/utils/challenge-detail/my-submission-scores.test.js | Adds tests for getDisplayedScores behavior. |
| tests/shared/utils/challenge-detail/mm-final-results.test.js | Adds tests for review-phase completion and early final-result visibility. |
| tests/shared/containers/challenge-detail/index.jsx | Updates tests for the new Wipro guard signature + Topgear Task exception. |
| tests/shared/components/challenge-detail/MySubmissions/SubmissionsList/index.jsx | Adds a second test suite for getDisplayedScores (duplicates coverage). |
| tests/shared/components/challenge-detail/Header/index.jsx | Adds shallow-render tests for task challenge header action visibility. |
| .github/workflows/trivy.yaml | Bumps Trivy GitHub Action version. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const initialScore = toNumericScore(normalizedAttempt.initialScore); | ||
| if (!_.isNil(initialScore)) { | ||
| normalizedAttempt.initialScore = initialScore; | ||
| normalizedAttempt.provisionalScore = initialScore; | ||
| } |
There was a problem hiding this comment.
The new initial-score normalization uses toNumericScore, which currently does Number(value) without guarding against "" or "-". Number('') becomes 0, which would incorrectly override provisionalScore to 0 for empty initial scores. Consider aligning toNumericScore with normalizeScoreValue (treating nil/""/"-" as null) before applying it to initialScore.
| import { getDisplayedScores } from '../../../../../../src/shared/components/challenge-detail/MySubmissions/SubmissionsList'; | ||
|
|
||
| describe('getDisplayedScores', () => { | ||
| test('shows final scores when a system review already produced one before review completes', () => { | ||
| expect(getDisplayedScores( | ||
| { | ||
| finalScore: 100, | ||
| initialScore: 100, | ||
| provisionalScore: 0, | ||
| }, | ||
| { | ||
| phases: [ | ||
| { | ||
| isOpen: true, | ||
| name: 'Registration', | ||
| scheduledStartDate: '2030-01-01T00:00:00.000Z', | ||
| }, | ||
| ], | ||
| }, | ||
| )).toEqual({ | ||
| finalScore: 100, | ||
| provisionalScore: 100, | ||
| }); | ||
| }); | ||
|
|
||
| test('shows final scores once the review phase is complete', () => { | ||
| expect(getDisplayedScores( | ||
| { | ||
| finalScore: 100, | ||
| initialScore: 95, | ||
| provisionalScore: 0, | ||
| }, | ||
| { | ||
| phases: [ | ||
| { | ||
| isOpen: false, | ||
| name: 'Review', | ||
| scheduledStartDate: '2000-01-01T00:00:00.000Z', | ||
| }, | ||
| ], | ||
| }, | ||
| )).toEqual({ | ||
| finalScore: 100, | ||
| provisionalScore: 95, | ||
| }); | ||
| }); |
There was a problem hiding this comment.
This test file duplicates the getDisplayedScores coverage already added in __tests__/shared/utils/challenge-detail/my-submission-scores.test.js (and the utils test is more complete). To avoid redundant test maintenance and slower runs, consider removing this duplicate suite or consolidating the unique assertions into a single location.
| import { getDisplayedScores } from '../../../../../../src/shared/components/challenge-detail/MySubmissions/SubmissionsList'; | |
| describe('getDisplayedScores', () => { | |
| test('shows final scores when a system review already produced one before review completes', () => { | |
| expect(getDisplayedScores( | |
| { | |
| finalScore: 100, | |
| initialScore: 100, | |
| provisionalScore: 0, | |
| }, | |
| { | |
| phases: [ | |
| { | |
| isOpen: true, | |
| name: 'Registration', | |
| scheduledStartDate: '2030-01-01T00:00:00.000Z', | |
| }, | |
| ], | |
| }, | |
| )).toEqual({ | |
| finalScore: 100, | |
| provisionalScore: 100, | |
| }); | |
| }); | |
| test('shows final scores once the review phase is complete', () => { | |
| expect(getDisplayedScores( | |
| { | |
| finalScore: 100, | |
| initialScore: 95, | |
| provisionalScore: 0, | |
| }, | |
| { | |
| phases: [ | |
| { | |
| isOpen: false, | |
| name: 'Review', | |
| scheduledStartDate: '2000-01-01T00:00:00.000Z', | |
| }, | |
| ], | |
| }, | |
| )).toEqual({ | |
| finalScore: 100, | |
| provisionalScore: 95, | |
| }); | |
| }); | |
| describe('getDisplayedScores', () => { | |
| test.todo( | |
| 'coverage for getDisplayedScores lives in __tests__/shared/utils/challenge-detail/my-submission-scores.test.js', | |
| ); |
| const dedupeKey = _.toString(_.get(summation, 'id', '')).trim() | ||
| || `${_.toString(_.get(summation, 'submissionId', '')).trim()}::${_.toString(_.get(summation, 'legacySubmissionId', '')).trim()}::${_.toString(_.get(summation, 'submitterId', '')).trim()}::${_.toString(_.get(summation, 'aggregateScore', '')).trim()}::${_.toString(_.get(summation, 'reviewedDate', '')).trim()}::${index}`; |
There was a problem hiding this comment.
dedupeReviewSummations's fallback dedupeKey appends ::${index}. When summation.id is missing, this makes the key unique per array position, so true duplicates (same submissionId/score/date/etc.) will never be removed. Consider only using index when the composed fallback key would otherwise be empty, or omit index entirely from the fallback key so identical records can be deduped.
| const dedupeKey = _.toString(_.get(summation, 'id', '')).trim() | |
| || `${_.toString(_.get(summation, 'submissionId', '')).trim()}::${_.toString(_.get(summation, 'legacySubmissionId', '')).trim()}::${_.toString(_.get(summation, 'submitterId', '')).trim()}::${_.toString(_.get(summation, 'aggregateScore', '')).trim()}::${_.toString(_.get(summation, 'reviewedDate', '')).trim()}::${index}`; | |
| const idDedupeKey = _.toString(_.get(summation, 'id', '')).trim(); | |
| const fallbackDedupeParts = [ | |
| _.toString(_.get(summation, 'submissionId', '')).trim(), | |
| _.toString(_.get(summation, 'legacySubmissionId', '')).trim(), | |
| _.toString(_.get(summation, 'submitterId', '')).trim(), | |
| _.toString(_.get(summation, 'aggregateScore', '')).trim(), | |
| _.toString(_.get(summation, 'reviewedDate', '')).trim(), | |
| ]; | |
| const fallbackDedupeKey = fallbackDedupeParts.join('::'); | |
| const dedupeKey = idDedupeKey | |
| || (_.some(fallbackDedupeParts, part => part !== '') | |
| ? fallbackDedupeKey | |
| : `unknown-review-summation-${index}`); |
| if (!handle || !_.isString(handle) || !handle.trim()) { | ||
| return 'unknown'; | ||
| } | ||
|
|
||
| return handle; |
There was a problem hiding this comment.
getSubmissionHandle returns a constant 'unknown' when no handle fields are present. Because membersByHandle is keyed only by handle, multiple submissions with missing handles would be merged into a single member bucket (and could also pick up the wrong memberId/rating). Consider falling back to a more unique key (e.g., memberId when available, or an unknown-${memberId}/unknown-${index} key) rather than a shared constant.
| if (!handle || !_.isString(handle) || !handle.trim()) { | |
| return 'unknown'; | |
| } | |
| return handle; | |
| if (handle && _.isString(handle) && handle.trim()) { | |
| return handle; | |
| } | |
| const memberId = getSubmissionMemberId(submission); | |
| if (memberId) { | |
| return `unknown-${memberId}`; | |
| } | |
| const submissionId = _.get(submission, 'id', _.get(submission, 'submissionId')); | |
| if (!_.isNil(submissionId) && _.toString(submissionId).trim()) { | |
| return `unknown-${_.toString(submissionId)}`; | |
| } | |
| return _.uniqueId('unknown-'); |
https://topcoder.atlassian.net/browse/PM-4609
https://topcoder.atlassian.net/browse/PM-4648
https://topcoder.atlassian.net/browse/PM-4662
https://topcoder.atlassian.net/browse/PM-4648
https://topcoder.atlassian.net/browse/PM-4720