From cce75e7f553cefd407d386dd5e411dde411c716e Mon Sep 17 00:00:00 2001 From: jmgasper Date: Thu, 25 Jun 2026 09:48:16 +1000 Subject: [PATCH] PM-5447: Validate phase shortening after schedule recalculation What was broken Active development challenges with shortened phase durations could fail when the schedule was moved from a scheduled launch time to immediately. Dependent phases shifted earlier with the rest of the schedule, but the update could be rejected as if a non-Design phase duration had been shortened. Root cause (if identifiable) Phase shortening validation ran against raw incoming phase end dates before predecessor-derived scheduled start dates were recalculated. For dependent phases, the validator compared the new end date against the old start date and misclassified a whole-schedule shift as a duration reduction. What was changed Moved schedule-shortening validation to run after phase dates are recalculated, using the effective scheduled start and end dates that will be persisted. Removed the now-unused raw request date resolution helpers. Any added/updated tests Added a phase helper regression test covering active non-Design dependent phases moving earlier with unchanged duration. --- src/common/phase-helper.js | 106 ++++++++++++++++----------------- test/unit/phase-helper.test.js | 73 +++++++++++++++++++++++ 2 files changed, 126 insertions(+), 53 deletions(-) diff --git a/src/common/phase-helper.js b/src/common/phase-helper.js index 639706c..8d12b6a 100644 --- a/src/common/phase-helper.js +++ b/src/common/phase-helper.js @@ -46,51 +46,6 @@ function isDesignTrack(track) { return normalizeTrackToken(track) === DESIGN_TRACK; } -/** - * Resolve the requested scheduled start date from a phase update payload. - * - * @param {Object} phase existing challenge phase - * @param {Object|null|undefined} newPhase phase update payload - * @returns {Date|String|undefined} requested scheduled start date, falling back to current start - */ -function resolveRequestedScheduledStartDate(phase, newPhase) { - if (_.isNil(newPhase) || _.isNil(_.get(newPhase, "scheduledStartDate"))) { - return _.get(phase, "scheduledStartDate"); - } - - return _.get(newPhase, "scheduledStartDate"); -} - -/** - * Resolve the requested scheduled end date from a phase update payload. - * - * @param {Object} phase existing challenge phase - * @param {Object|null|undefined} newPhase phase update payload - * @returns {Date|String|undefined} requested scheduled end date when the payload changes it - */ -function resolveRequestedScheduledEndDate(phase, newPhase) { - if (_.isNil(newPhase)) { - return undefined; - } - - if (!_.isNil(_.get(newPhase, "scheduledEndDate"))) { - return _.get(newPhase, "scheduledEndDate"); - } - - const requestedDuration = _.get(newPhase, "duration"); - const requestedScheduledStartDate = resolveRequestedScheduledStartDate(phase, newPhase); - if (_.isNil(requestedDuration) || _.isNil(requestedScheduledStartDate)) { - return undefined; - } - - const scheduledStart = moment(requestedScheduledStartDate); - if (!scheduledStart.isValid()) { - return undefined; - } - - return scheduledStart.add(requestedDuration, "seconds").toDate().toISOString(); -} - /** * Check whether a requested schedule reduces the phase duration. * @@ -187,6 +142,58 @@ function validateActivePhaseScheduledEndDateChange( } } +/** + * Validate recalculated schedules against persisted phase schedules. + * + * @param {Array} originalPhases persisted challenge phases before the update + * @param {Array} updatedPhases recalculated challenge phases that will be persisted + * @param {Object} options validation options forwarded to the phase schedule validator + * @returns {undefined} validates only + * @throws {BadRequestError} when a recalculated schedule violates shortening rules + */ +function validateRecalculatedPhaseSchedules(originalPhases, updatedPhases, options = {}) { + const originalById = new Map(); + const originalByPhaseId = new Map(); + + _.each(originalPhases, (phase) => { + if (_.isNil(phase)) { + return; + } + + if (!_.isNil(phase.id)) { + originalById.set(phase.id, phase); + } + if (!_.isNil(phase.phaseId)) { + originalByPhaseId.set(phase.phaseId, phase); + } + }); + + _.each(updatedPhases, (updatedPhase) => { + if (_.isNil(updatedPhase)) { + return; + } + + const originalPhase = ( + !_.isNil(updatedPhase.id) + ? originalById.get(updatedPhase.id) + : undefined + ) || originalByPhaseId.get(updatedPhase.phaseId); + + if (_.isNil(originalPhase)) { + return; + } + + validateActivePhaseScheduledEndDateChange( + originalPhase, + updatedPhase.scheduledEndDate, + { + ...options, + requestedScheduledStartDate: updatedPhase.scheduledStartDate, + } + ); + }); +} + /** * Apply an explicit scheduled end date to a phase and update its duration. * @@ -384,14 +391,6 @@ class ChallengePhaseHelper { const phaseFromTemplate = timelineTemplateMap.get(phase.phaseId); const phaseDefinition = phaseDefinitionMap.get(phase.phaseId); const newPhase = findPhaseUpdate(newPhases, phase); - validateActivePhaseScheduledEndDateChange( - phase, - resolveRequestedScheduledEndDate(phase, newPhase), - { - ...options, - requestedScheduledStartDate: resolveRequestedScheduledStartDate(phase, newPhase), - } - ); const templatePredecessor = _.get(phaseFromTemplate, "predecessor"); // Prefer template predecessor only when that phase exists on the challenge, otherwise keep the stored link. const resolvedPredecessor = _.isNil(phaseFromTemplate) @@ -485,6 +484,7 @@ class ChallengePhaseHelper { } recalculateScheduledEndDate(phase); } + validateRecalculatedPhaseSchedules(challengePhasesOrdered, updatedPhases, options); return _.map(updatedPhases, (phase) => _.omit(phase, "requestedScheduledEndDate")); } diff --git a/test/unit/phase-helper.test.js b/test/unit/phase-helper.test.js index 84d9c23..635c94e 100644 --- a/test/unit/phase-helper.test.js +++ b/test/unit/phase-helper.test.js @@ -436,6 +436,79 @@ describe('phase helper unit tests', () => { updatedPhases[0].duration.should.equal(duration) }) + it('allows active non-Design dependent phases to move earlier when duration is unchanged', async () => { + const registrationPhaseId = 'development-registration-phase' + const reviewPhaseId = 'development-review-phase' + const duration = 24 * 60 * 60 + const currentRegistrationStartDate = '2099-05-26T05:14:00.000Z' + const currentRegistrationEndDate = '2099-05-27T05:14:00.000Z' + const currentReviewEndDate = '2099-05-28T05:14:00.000Z' + const requestedRegistrationStartDate = '2099-05-25T05:14:00.000Z' + const requestedRegistrationEndDate = '2099-05-26T05:14:00.000Z' + const requestedReviewEndDate = '2099-05-27T05:14:00.000Z' + + stubPhaseLookups( + [ + { id: registrationPhaseId, name: 'Registration', description: 'Registration phase' }, + { id: reviewPhaseId, name: 'Review', description: 'Review phase' } + ], + [ + { phaseId: registrationPhaseId, defaultDuration: duration }, + { + phaseId: reviewPhaseId, + predecessor: registrationPhaseId, + defaultDuration: duration + } + ] + ) + + const updatedPhases = await phaseHelper.populatePhasesForChallengeUpdate( + [ + { + duration, + isOpen: true, + name: 'Registration', + phaseId: registrationPhaseId, + scheduledStartDate: currentRegistrationStartDate, + scheduledEndDate: currentRegistrationEndDate + }, + { + duration, + name: 'Review', + phaseId: reviewPhaseId, + predecessor: registrationPhaseId, + scheduledStartDate: currentRegistrationEndDate, + scheduledEndDate: currentReviewEndDate + } + ], + [ + { + duration, + phaseId: registrationPhaseId, + scheduledStartDate: requestedRegistrationStartDate, + scheduledEndDate: requestedRegistrationEndDate + }, + { + duration, + phaseId: reviewPhaseId, + scheduledEndDate: requestedReviewEndDate + } + ], + 'timeline-template-id', + false, + { + allowActivePhaseShortening: false, + preventPhaseShortening: true + } + ) + + updatedPhases[0].scheduledStartDate.should.equal(requestedRegistrationStartDate) + updatedPhases[0].scheduledEndDate.should.equal(requestedRegistrationEndDate) + updatedPhases[1].scheduledStartDate.should.equal(requestedRegistrationEndDate) + updatedPhases[1].scheduledEndDate.should.equal(requestedReviewEndDate) + updatedPhases[1].duration.should.equal(duration) + }) + it('rejects active non-Design phase updates that shorten duration after moving start earlier', async () => { const registrationPhaseId = 'development-registration-phase' const currentRegistrationStartDate = '2099-05-26T05:14:00.000Z'