From 1cafcb8824540568435649fd48b0c3cd3c68931b Mon Sep 17 00:00:00 2001 From: jmgasper Date: Tue, 25 Aug 2026 14:00:17 +1000 Subject: [PATCH] PM-5961: only allow deleting submissions while their submission phase is open What was broken In Design challenges the submitter could still delete a submission after the phase that created it had ended. In particular a checkpoint submission stayed deletable for the whole Submission phase, so it could be removed after it had already been screened and scored in Checkpoint Screening / Checkpoint Review. Root cause SubmissionsTable computed the delete permission from a single "submissionPhaseStartDate" prop, which was the start date of *any* open Submission or Checkpoint Submission phase, and compared it against subObject.submissionDate - a field that member submissions do not carry (they expose "created"). The comparison therefore resolved to "now is after the open phase start", i.e. the delete button was enabled for every submission whenever any submission phase was open, regardless of which phase actually produced the submission. What was changed - Added belongsToActiveSubmissionPhase() to utils/challenge-detail/submission-limit, which reuses the existing getActiveSubmissionType() phase precedence to decide whether a submission was created by the currently open submission phase. - SubmissionsTable now derives allowDelete from the submission's own type against the open phase, so checkpoint submissions stop being deletable when the Checkpoint Submission phase closes, and nothing is deletable once every submission phase is closed. - Removed the now-unused submissionPhaseStartDate prop from the submission management container and components. Any added/updated tests - __tests__/shared/utils/challenge-detail/submission-limit.test.js: new belongsToActiveSubmissionPhase suite covering checkpoint deletion during the checkpoint phase, checkpoint deletion blocked once the Submission phase opens, everything blocked during screening/review, and missing submission/phase input. - Updated the SubmissionsTable shallow snapshot for the now-boolean allowDelete prop. Co-Authored-By: Claude Opus 5 (1M context) --- .../__snapshots__/SubmissionsTable.jsx.snap | 1 + .../challenge-detail/submission-limit.test.js | 58 +++++++++++++++++++ .../SubmissionManagement/index.jsx | 3 - .../SubmissionsTable/index.jsx | 11 +--- .../containers/SubmissionManagement/index.jsx | 8 --- .../challenge-detail/submission-limit.js | 28 +++++++++ 6 files changed, 90 insertions(+), 19 deletions(-) diff --git a/__tests__/shared/components/SubmissionManagement/__snapshots__/SubmissionsTable.jsx.snap b/__tests__/shared/components/SubmissionManagement/__snapshots__/SubmissionsTable.jsx.snap index d8f5971b3..46edffe3d 100644 --- a/__tests__/shared/components/SubmissionManagement/__snapshots__/SubmissionsTable.jsx.snap +++ b/__tests__/shared/components/SubmissionManagement/__snapshots__/SubmissionsTable.jsx.snap @@ -25,6 +25,7 @@ exports[`Matches shallow shapshot 1`] = ` { }); }); +describe('belongsToActiveSubmissionPhase', () => { + test('allows deleting a checkpoint submission while the checkpoint phase is open', () => { + const phases = [ + { isOpen: true, name: 'Checkpoint Submission' }, + { isOpen: false, name: 'Submission' }, + ]; + + expect(belongsToActiveSubmissionPhase( + { id: 'checkpoint-submission', type: 'CHECKPOINT_SUBMISSION' }, + phases, + )).toBe(true); + expect(belongsToActiveSubmissionPhase( + { id: 'legacy-checkpoint', submissionType: 'checkpoint' }, + phases, + )).toBe(true); + }); + + test('blocks deleting a checkpoint submission once the submission phase opens', () => { + const phases = [ + { isOpen: false, name: 'Checkpoint Submission' }, + { isOpen: true, name: 'Submission' }, + ]; + + expect(belongsToActiveSubmissionPhase( + { id: 'checkpoint-submission', type: 'CHECKPOINT_SUBMISSION' }, + phases, + )).toBe(false); + expect(belongsToActiveSubmissionPhase( + { id: 'contest-submission', type: 'CONTEST_SUBMISSION' }, + phases, + )).toBe(true); + }); + + test('blocks deleting anything once every submission phase is closed', () => { + const phases = [ + { isOpen: false, name: 'Checkpoint Submission' }, + { isOpen: false, name: 'Submission' }, + { isOpen: true, name: 'Checkpoint Review' }, + { isOpen: true, name: 'Review' }, + ]; + + expect(belongsToActiveSubmissionPhase( + { id: 'checkpoint-submission', type: 'CHECKPOINT_SUBMISSION' }, + phases, + )).toBe(false); + expect(belongsToActiveSubmissionPhase( + { id: 'contest-submission', type: 'CONTEST_SUBMISSION' }, + phases, + )).toBe(false); + }); + + test('resolves to false for missing submissions or phases', () => { + expect(belongsToActiveSubmissionPhase(null, [{ isOpen: true, name: 'Submission' }])).toBe(false); + expect(belongsToActiveSubmissionPhase({ type: 'CONTEST_SUBMISSION' }, undefined)).toBe(false); + }); +}); + describe('getSubmissionLimitReachedMessage', () => { test('uses the requested singular limit message', () => { expect(getSubmissionLimitReachedMessage(1)).toBe( diff --git a/src/shared/components/SubmissionManagement/SubmissionManagement/index.jsx b/src/shared/components/SubmissionManagement/SubmissionManagement/index.jsx index 2eaa362d7..faf6902c3 100644 --- a/src/shared/components/SubmissionManagement/SubmissionManagement/index.jsx +++ b/src/shared/components/SubmissionManagement/SubmissionManagement/index.jsx @@ -40,7 +40,6 @@ export default function SubmissionManagement(props) { onShowDetails, challengeUrl, onlineReviewUrl, - submissionPhaseStartDate, onDownloadArtifacts, getSubmissionArtifacts, getSubmissionScores, @@ -191,7 +190,6 @@ export default function SubmissionManagement(props) { showDetails={showDetails} track={trackName} status={challenge.status} - submissionPhaseStartDate={submissionPhaseStartDate} {...componentConfig} /> ) @@ -251,5 +249,4 @@ SubmissionManagement.propTypes = { submissionLimitCheckPending: PT.bool, loadingSubmissions: PT.bool, challengeUrl: PT.string, - submissionPhaseStartDate: PT.string.isRequired, }; diff --git a/src/shared/components/SubmissionManagement/SubmissionsTable/index.jsx b/src/shared/components/SubmissionManagement/SubmissionsTable/index.jsx index a9ca5652f..74ccbef21 100644 --- a/src/shared/components/SubmissionManagement/SubmissionsTable/index.jsx +++ b/src/shared/components/SubmissionManagement/SubmissionsTable/index.jsx @@ -18,8 +18,8 @@ import _ from 'lodash'; import React, { useCallback, useState } from 'react'; import PT from 'prop-types'; import shortid from 'shortid'; -import moment from 'moment'; import { COMPETITION_TRACKS } from 'utils/tc'; +import { belongsToActiveSubmissionPhase } from 'utils/challenge-detail/submission-limit'; import ScreeningDetails from '../ScreeningDetails'; import DownloadArtifactsModal from '../DownloadArtifactsModal'; import Submission from '../Submission'; @@ -45,7 +45,6 @@ export default function SubmissionsTable(props) { onDownload, onShowDetails, status, - submissionPhaseStartDate, onDownloadArtifacts, getSubmissionArtifacts, getSubmissionScores, @@ -72,9 +71,6 @@ export default function SubmissionsTable(props) { )); } else { submissionObjects.forEach((subObject) => { - // submissionPhaseStartDate will be the start date of - // the current submission/checkpoint or empty string if any other phase - const TERMINAL_STATUSES = [ 'COMPLETED', 'FAILURE', @@ -93,8 +89,8 @@ export default function SubmissionsTable(props) { ? true : workflowRunsForSubmission.every(run => TERMINAL_STATUSES.includes(run.status)); - const allowDelete = submissionPhaseStartDate - && moment(subObject.submissionDate).isAfter(submissionPhaseStartDate); + // Submissions can only be removed while the phase that created them is still open. + const allowDelete = belongsToActiveSubmissionPhase(subObject, challenge.phases); const submission = ( @@ -233,6 +229,5 @@ SubmissionsTable.propTypes = { onDownloadArtifacts: PT.func, getSubmissionArtifacts: PT.func, status: PT.string.isRequired, - submissionPhaseStartDate: PT.string.isRequired, getSubmissionScores: PT.func, }; diff --git a/src/shared/containers/SubmissionManagement/index.jsx b/src/shared/containers/SubmissionManagement/index.jsx index 13f19e5b6..e9d0306ac 100644 --- a/src/shared/containers/SubmissionManagement/index.jsx +++ b/src/shared/containers/SubmissionManagement/index.jsx @@ -483,7 +483,6 @@ export class SubmissionManagementPageContainer extends React.Component { challengesUrl, deleting, loadingSubmissionsForChallengeId, - submissionPhaseStartDate, isLoadingChallenge, onCancelSubmissionDelete, onShowDetails, @@ -562,7 +561,6 @@ export class SubmissionManagementPageContainer extends React.Component { submissionLimitCheckPending={submissionLimitCheckPending} showDetails={showDetails} submissionWorkflowRuns={submissionWorkflowRuns} - submissionPhaseStartDate={submissionPhaseStartDate} {...smConfig} /> )} @@ -672,7 +670,6 @@ SubmissionManagementPageContainer.propTypes = { toBeDeletedId: PT.string, deletionSucceed: PT.bool, onSubmissionDeleteConfirmed: PT.func.isRequired, - submissionPhaseStartDate: PT.string.isRequired, }; function mapStateToProps(state, props) { @@ -682,9 +679,6 @@ function mapStateToProps(state, props) { mySubmissions = challengeId === mySubmissions.challengeId ? mySubmissions.v2 : null; - const allPhases = state.challenge.details.phases || []; - const submissionPhase = allPhases.find(phase => ['Submission', 'Checkpoint Submission'].includes(phase.name) && phase.isOpen) || {}; - return { challengeId: String(challengeId), challenge: state.challenge.details, @@ -698,8 +692,6 @@ function mapStateToProps(state, props) { state.challenge.loadingSubmissionsForChallengeId || '', mySubmissions, - submissionPhaseStartDate: submissionPhase.actualStartDate || submissionPhase.scheduledStartDate || '', - showDetails: state.page.submissionManagement.showDetails, submissionWorkflowRuns: state.page.submissionManagement.submissionWorkflowRuns, diff --git a/src/shared/utils/challenge-detail/submission-limit.js b/src/shared/utils/challenge-detail/submission-limit.js index 061a9332b..7a008d5fa 100644 --- a/src/shared/utils/challenge-detail/submission-limit.js +++ b/src/shared/utils/challenge-detail/submission-limit.js @@ -2,6 +2,7 @@ const SUBMISSION_LIMIT_METADATA_NAME = 'submissionLimit'; const CHECKPOINT_SUBMISSION_TYPE = 'CHECKPOINT_SUBMISSION'; const CONTEST_SUBMISSION_TYPE = 'CONTEST_SUBMISSION'; const FINAL_FIX_SUBMISSION_TYPE = 'STUDIO_FINAL_FIX_SUBMISSION'; +const SUBMISSION_PHASE_NAMES = ['Checkpoint Submission', 'Submission']; /** * Converts a metadata value to a positive integer submission limit. @@ -164,6 +165,33 @@ function normalizeSubmissionType(value) { return normalizedValue; } +/** + * Checks whether a submission was created by the currently open submission phase. + * + * Submitters may only remove a submission while the phase that produced it is still open. A + * checkpoint submission therefore stops being deletable as soon as the Checkpoint Submission + * phase ends, even though the Submission phase opens right after it, and nothing is deletable + * once every submission phase is closed (screening, review, appeals, ...). + * + * @param {Object} submission Member submission, using the V6 `type` or legacy `submissionType`. + * @param {Array} phases Challenge phases. + * @return {Boolean} Whether the submission belongs to the currently open submission phase. + * @throws Does not throw; missing submissions or phases resolve to false. + */ +export function belongsToActiveSubmissionPhase(submission, phases) { + const challengePhases = Array.isArray(phases) ? phases : []; + const hasOpenSubmissionPhase = challengePhases.some(phase => ( + phase && phase.isOpen && SUBMISSION_PHASE_NAMES.includes(phase.name) + )); + + if (!submission || !hasOpenSubmissionPhase) { + return false; + } + + return normalizeSubmissionType(submission.type || submission.submissionType) + === getActiveSubmissionType(challengePhases); +} + /** * Checks whether concept submission limits apply to a V6 submission type. *