From 7d3f20131a47ddf672b68d2ccb8f08ce24a22f03 Mon Sep 17 00:00:00 2001 From: Justin Gasper Date: Sat, 4 Apr 2026 05:55:33 +1100 Subject: [PATCH 1/2] PM-4608: show MM final scores during review What was broken Marathon Match final scores stayed hidden in Community App while the review phase was still open, even after Review API had stored the system-test final scores. Root cause The MM submission views treated review completion as a prerequisite for rendering final scores, so open-review challenges forced the final-score fields to `N/A` or `-` despite having `finalScore` data. What was changed Removed the review-complete gate from the MM submissions table and submission history rows. Updated My Submissions and the submission details modal to surface `finalScore` as soon as it exists, while still preferring `initialScore` over stale provisional-score values. Any added/updated tests Updated the My Submissions score regression test to verify final scores remain visible before the review phase closes. --- .../MySubmissions/SubmissionsList/index.jsx | 6 ++--- .../MySubmissions/SubmissionsList/index.jsx | 14 ++++------- .../SubmissionInformationModal/index.jsx | 24 +++++++++++++++---- .../SubmissionHistoryRow/index.jsx | 6 ----- .../Submissions/SubmissionRow/index.jsx | 3 --- 5 files changed, 27 insertions(+), 26 deletions(-) diff --git a/__tests__/shared/components/challenge-detail/MySubmissions/SubmissionsList/index.jsx b/__tests__/shared/components/challenge-detail/MySubmissions/SubmissionsList/index.jsx index d60f71461..f6a6fead8 100644 --- a/__tests__/shared/components/challenge-detail/MySubmissions/SubmissionsList/index.jsx +++ b/__tests__/shared/components/challenge-detail/MySubmissions/SubmissionsList/index.jsx @@ -1,7 +1,7 @@ import { getDisplayedScores } from '../../../../../../src/shared/components/challenge-detail/MySubmissions/SubmissionsList'; describe('getDisplayedScores', () => { - test('uses the initial score as the provisional score before review completes', () => { + test('uses the initial score as the provisional score while keeping final scores visible', () => { expect(getDisplayedScores( { finalScore: 100, @@ -18,12 +18,12 @@ describe('getDisplayedScores', () => { ], }, )).toEqual({ - finalScore: null, + finalScore: 100, provisionalScore: 100, }); }); - test('shows final scores once the review phase is complete', () => { + test('shows final scores after the review phase is complete', () => { expect(getDisplayedScores( { finalScore: 100, diff --git a/src/shared/components/challenge-detail/MySubmissions/SubmissionsList/index.jsx b/src/shared/components/challenge-detail/MySubmissions/SubmissionsList/index.jsx index 127c7a384..b42a687db 100644 --- a/src/shared/components/challenge-detail/MySubmissions/SubmissionsList/index.jsx +++ b/src/shared/components/challenge-detail/MySubmissions/SubmissionsList/index.jsx @@ -77,29 +77,23 @@ const getSubmissionCreatedTime = (submission) => { /** * Returns the scores that should be displayed for a marathon match submission row. * Initial score is the authoritative provisional score for MM submissions, while - * final scores should remain hidden until the review phase has completed. + * final scores should surface as soon as Review API provides them. * * @param {Object} submission submission attempt shown in My Submissions. - * @param {Object} challenge challenge that owns the submission. * @returns {{ finalScore: number|null, provisionalScore: number|null }} display-ready scores. */ -export function getDisplayedScores(submission = {}, challenge = {}) { +export function getDisplayedScores(submission = {}) { const toNumericScore = (value) => { const numeric = Number(value); return Number.isFinite(numeric) ? numeric : null; }; - const isReviewPhaseComplete = _.some( - challenge.phases || [], - phase => phase.name === 'Review' && !phase.isOpen && moment(phase.scheduledStartDate).isBefore(), - ); - const initialScore = toNumericScore(_.get(submission, 'initialScore')); const provisionalScore = toNumericScore(_.get(submission, 'provisionalScore')); const finalScore = toNumericScore(_.get(submission, 'finalScore')); return { - finalScore: isReviewPhaseComplete ? finalScore : null, + finalScore, provisionalScore: !_.isNil(initialScore) ? initialScore : provisionalScore, }; } @@ -465,7 +459,7 @@ class SubmissionsListView extends React.Component { { sortedSubmissions.map((mySubmission) => { - let { finalScore, provisionalScore } = getDisplayedScores(mySubmission, challenge); + let { finalScore, provisionalScore } = getDisplayedScores(mySubmission); if (_.isNumber(finalScore)) { if (finalScore > 0) { finalScore = finalScore.toFixed(2); diff --git a/src/shared/components/challenge-detail/Submissions/SubmissionInformationModal/index.jsx b/src/shared/components/challenge-detail/Submissions/SubmissionInformationModal/index.jsx index 328dcf0cf..366fbc385 100644 --- a/src/shared/components/challenge-detail/Submissions/SubmissionInformationModal/index.jsx +++ b/src/shared/components/challenge-detail/Submissions/SubmissionInformationModal/index.jsx @@ -61,11 +61,26 @@ class SubmissionInformationModal extends React.Component { render() { const { toggleTestcase, onClose, isLoadingSubmissionInformation, - submissionInformation, isReviewPhaseComplete, + submissionInformation, } = this.props; const submissionBasicInfo = isLoadingSubmissionInformation ? null : this.getSubmissionBasicInfo(); const testcases = isLoadingSubmissionInformation ? [] : this.getTestcases(); + const toNumericScore = (value) => { + const numeric = Number(value); + return Number.isFinite(numeric) ? numeric : null; + }; + const displayedScores = submissionBasicInfo + ? { + finalScore: toNumericScore(_.get(submissionBasicInfo, 'finalScore')), + provisionalScore: (() => { + const initialScore = toNumericScore(_.get(submissionBasicInfo, 'initialScore')); + return !_.isNil(initialScore) + ? initialScore + : toNumericScore(_.get(submissionBasicInfo, 'provisionalScore')); + })(), + } + : { finalScore: null, provisionalScore: null }; return ( onClose(false)}> @@ -90,12 +105,14 @@ class SubmissionInformationModal extends React.Component {
- {(!submissionBasicInfo.finalScore && submissionBasicInfo.finalScore !== 0) || !isReviewPhaseComplete ? '-' : submissionBasicInfo.finalScore} + {displayedScores.finalScore === null ? '-' : displayedScores.finalScore}
- {(!submissionBasicInfo.provisionalScore && submissionBasicInfo.provisionalScore !== 0) ? '-' : submissionBasicInfo.provisionalScore} + {displayedScores.provisionalScore === null + ? '-' + : displayedScores.provisionalScore}
{moment(submissionBasicInfo.submissionTime) @@ -180,7 +197,6 @@ SubmissionInformationModal.propTypes = { openTestcase: PT.shape({}).isRequired, clearTestcaseOpen: PT.func.isRequired, submission: PT.shape().isRequired, - isReviewPhaseComplete: PT.bool.isRequired, }; export default SubmissionInformationModal; diff --git a/src/shared/components/challenge-detail/Submissions/SubmissionRow/SubmissionHistoryRow/index.jsx b/src/shared/components/challenge-detail/Submissions/SubmissionRow/SubmissionHistoryRow/index.jsx index 27a15a6f8..0b41b5246 100644 --- a/src/shared/components/challenge-detail/Submissions/SubmissionRow/SubmissionHistoryRow/index.jsx +++ b/src/shared/components/challenge-detail/Submissions/SubmissionRow/SubmissionHistoryRow/index.jsx @@ -27,7 +27,6 @@ export default function SubmissionHistoryRow({ provisionalScore, submissionTime, createdAt, - isReviewPhaseComplete, status, challengeStatus, auth, @@ -63,9 +62,6 @@ export default function SubmissionHistoryRow({ } }; const getFinalScore = () => { - if (!isReviewPhaseComplete) { - return 'N/A'; - } if (finalScoreValue === null) { return 'N/A'; } @@ -139,7 +135,6 @@ export default function SubmissionHistoryRow({ SubmissionHistoryRow.defaultProps = { finalScore: null, provisionalScore: null, - isReviewPhaseComplete: false, isLoggedIn: false, createdAt: null, submissionTime: null, @@ -169,7 +164,6 @@ SubmissionHistoryRow.propTypes = { PT.oneOf([null]), ]), challengeStatus: PT.string.isRequired, - isReviewPhaseComplete: PT.bool, auth: PT.shape().isRequired, numWinners: PT.number.isRequired, submissionId: PT.string.isRequired, diff --git a/src/shared/components/challenge-detail/Submissions/SubmissionRow/index.jsx b/src/shared/components/challenge-detail/Submissions/SubmissionRow/index.jsx index 65fd838b0..e5c20311c 100644 --- a/src/shared/components/challenge-detail/Submissions/SubmissionRow/index.jsx +++ b/src/shared/components/challenge-detail/Submissions/SubmissionRow/index.jsx @@ -66,9 +66,6 @@ export default function SubmissionRow({ }; const getFinalReviewResult = () => { - if (!isReviewPhaseComplete) { - return 'N/A'; - } if (_.isNil(finalScore)) { return 'N/A'; } From e7e10a31fb1608ff4f885ec256fc56f37df3313d Mon Sep 17 00:00:00 2001 From: jmgasper Date: Tue, 21 Apr 2026 10:39:43 +1000 Subject: [PATCH 2/2] Better handling of challenges with >500 submissions --- __tests__/shared/services/reviewSummations.js | 107 ++++++++++++++++++ src/shared/services/reviewSummations.js | 3 +- 2 files changed, 108 insertions(+), 2 deletions(-) create mode 100644 __tests__/shared/services/reviewSummations.js diff --git a/__tests__/shared/services/reviewSummations.js b/__tests__/shared/services/reviewSummations.js new file mode 100644 index 000000000..27f66b326 --- /dev/null +++ b/__tests__/shared/services/reviewSummations.js @@ -0,0 +1,107 @@ +/* eslint-env jest */ +import { config } from 'topcoder-react-utils'; +import getReviewSummations from '../../../src/shared/services/reviewSummations'; + +const baseUrl = `${config.API.V6}${config.URL.REVIEW_SUMMATIONS_API_URL}`; + +describe('reviewSummations service', () => { + const originalFetch = global.fetch; + const originalHeaders = global.Headers; + + beforeAll(() => { + if (!global.Headers) { + global.Headers = class HeadersMock { + constructor() { + this.values = {}; + } + + set(key, value) { + this.values[key] = value; + } + + get(key) { + return this.values[key]; + } + }; + } + }); + + afterAll(() => { + global.fetch = originalFetch; + global.Headers = originalHeaders; + }); + + beforeEach(() => { + global.fetch = jest.fn(); + }); + + it('loads every page when review summations metadata reports more pages', async () => { + global.fetch + .mockResolvedValueOnce({ + ok: true, + json: () => Promise.resolve({ + data: [{ id: 'summation-page-1' }], + meta: { + page: 1, + perPage: 100, + totalPages: 2, + totalItems: 101, + }, + }), + }) + .mockResolvedValueOnce({ + ok: true, + json: () => Promise.resolve({ + data: [{ id: 'summation-page-2' }], + meta: { + page: 2, + perPage: 100, + totalPages: 2, + totalItems: 101, + }, + }), + }); + + const result = await getReviewSummations('token-v3', 'challenge-id'); + + expect(global.fetch).toHaveBeenCalledTimes(2); + expect(global.fetch).toHaveBeenNthCalledWith( + 1, + `${baseUrl}?challengeId=challenge-id&perPage=500&page=1`, + expect.objectContaining({ method: 'GET' }), + ); + expect(global.fetch).toHaveBeenNthCalledWith( + 2, + `${baseUrl}?challengeId=challenge-id&perPage=500&page=2`, + expect.objectContaining({ method: 'GET' }), + ); + expect(result.data).toEqual([ + { id: 'summation-page-1' }, + { id: 'summation-page-2' }, + ]); + expect(result.meta).toEqual(expect.objectContaining({ + page: 2, + perPage: 500, + totalItems: 2, + totalPages: 2, + })); + }); + + it('uses the short-page heuristic only when metadata does not include total pages', async () => { + global.fetch.mockResolvedValueOnce({ + ok: true, + json: () => Promise.resolve({ + data: [{ id: 'summation-only-page' }], + meta: { + page: 1, + perPage: 100, + }, + }), + }); + + const result = await getReviewSummations(null, 'challenge-id'); + + expect(global.fetch).toHaveBeenCalledTimes(1); + expect(result.data).toEqual([{ id: 'summation-only-page' }]); + }); +}); diff --git a/src/shared/services/reviewSummations.js b/src/shared/services/reviewSummations.js index 24308df33..0ab16fa82 100644 --- a/src/shared/services/reviewSummations.js +++ b/src/shared/services/reviewSummations.js @@ -30,8 +30,7 @@ async function fetchReviewSummationsPage({ const totalPages = _.get(payload, 'meta.totalPages') || _.get(payload, 'meta.total_pages'); const reachedEnd = !data.length - || (totalPages && page >= totalPages) - || data.length < DEFAULT_PER_PAGE; + || (totalPages ? page >= totalPages : data.length < DEFAULT_PER_PAGE); if (reachedEnd) { return {