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 d60f714617..f6a6fead83 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 127c7a3846..b42a687db1 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 328dcf0cf6..366fbc3859 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 27a15a6f8e..0b41b52468 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 65fd838b01..e5c20311cc 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 0000000000..27f66b3262 --- /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 24308df33e..0ab16fa82b 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 {