Skip to content

PM-5447: Validate phase shortening after schedule recalculation #128

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jmgasper merged 1 commit into from
Jun 25, 2026
Merged

PM-5447: Validate phase shortening after schedule recalculation #128

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 53 additions & 53 deletions src/common/phase-helper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -187,6 +142,58 @@ function validateActivePhaseScheduledEndDateChange(
}
}

/**
* Validate recalculated schedules against persisted phase schedules.
*
* @param {Array<Object>} originalPhases persisted challenge phases before the update
* @param {Array<Object>} 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.
*
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -485,6 +484,7 @@ class ChallengePhaseHelper {
}
recalculateScheduledEndDate(phase);
}
validateRecalculatedPhaseSchedules(challengePhasesOrdered, updatedPhases, options);
return _.map(updatedPhases, (phase) => _.omit(phase, "requestedScheduledEndDate"));
}

Expand Down
73 changes: 73 additions & 0 deletions test/unit/phase-helper.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
Loading