From 5da59421e36e5d8746e91fcd21fb614bce1fbd4c Mon Sep 17 00:00:00 2001 From: jmgasper Date: Wed, 17 Jun 2026 11:42:30 +1000 Subject: [PATCH 1/6] PM-5361: Preserve phase schedule updates by id What was broken Launched challenges with schedule updates could apply the wrong incoming phase payload when multiple challenge phase rows shared a phase definition id. That could make edited end dates and minute durations appear to revert after save for some timelines. Root cause (if identifiable) populatePhasesForChallengeUpdate matched incoming phase edits only by phaseId, which identifies the phase definition rather than the persisted challenge phase row. What was changed Added a phase update lookup that prefers the persisted challenge phase id and falls back to the existing phaseId behavior for compatibility. Kept the existing scheduledEndDate duration recalculation behavior intact. Fixed a missing Chai binding in challenge-helper tests so the unit command can load that file. Any added/updated tests Added phase-helper regression coverage for launched open phases that share a phase definition id and must preserve distinct scheduledEndDate and duration updates. --- src/common/phase-helper.js | 25 ++++++++++- test/unit/challenge-helper.test.js | 3 +- test/unit/phase-helper.test.js | 67 ++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 2 deletions(-) diff --git a/src/common/phase-helper.js b/src/common/phase-helper.js index 0274604..e69a653 100644 --- a/src/common/phase-helper.js +++ b/src/common/phase-helper.js @@ -65,6 +65,29 @@ function recalculateScheduledEndDate(phase) { .toISOString(); } +/** + * Find the incoming update payload for a persisted challenge phase. + * This helper does not raise exceptions. + * + * @param {Array} newPhases phase updates from the challenge update request + * @param {Object} phase persisted challenge phase being updated + * @returns {Object|undefined} the matching phase update, preferring challenge phase row id + */ +function findPhaseUpdate(newPhases, phase) { + if (!Array.isArray(newPhases)) { + return undefined; + } + + if (!_.isNil(phase.id)) { + const phaseUpdate = _.find(newPhases, (p) => p.id === phase.id); + if (!_.isNil(phaseUpdate)) { + return phaseUpdate; + } + } + + return _.find(newPhases, (p) => p.phaseId === phase.phaseId); +} + class ChallengePhaseHelper { phaseDefinitionMap = {}; timelineTemplateMap = {}; @@ -182,7 +205,7 @@ class ChallengePhaseHelper { const updatedPhases = _.map(challengePhasesOrdered, (phase) => { const phaseFromTemplate = timelineTemplateMap.get(phase.phaseId); const phaseDefinition = phaseDefinitionMap.get(phase.phaseId); - const newPhase = _.find(newPhases, (p) => p.phaseId === phase.phaseId); + const newPhase = findPhaseUpdate(newPhases, phase); 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) diff --git a/test/unit/challenge-helper.test.js b/test/unit/challenge-helper.test.js index e43e72f..e9de367 100644 --- a/test/unit/challenge-helper.test.js +++ b/test/unit/challenge-helper.test.js @@ -1,6 +1,7 @@ require("../../app-bootstrap"); -const { expect } = require("chai"); +const chai = require("chai"); +const { expect } = chai; const { ChallengeStatusEnum } = require("@prisma/client"); const challengeHelper = require("../../src/common/challenge-helper"); diff --git a/test/unit/phase-helper.test.js b/test/unit/phase-helper.test.js index dc3e84c..2800d60 100644 --- a/test/unit/phase-helper.test.js +++ b/test/unit/phase-helper.test.js @@ -99,6 +99,73 @@ describe('phase helper unit tests', () => { updatedPhases[1].duration.should.equal(4 * 24 * 60 * 60) }) + it('matches launched phase updates by challenge phase id before phase definition id', async () => { + const sharedPhaseId = 'shared-registration-phase' + const staleDuration = 120 * 60 * 60 + const firstPhaseStartDate = '2026-06-15T09:29:45.575Z' + const secondPhaseStartDate = '2026-06-15T09:29:45.576Z' + const firstPhaseEndDate = '2026-06-20T10:14:45.575Z' + const secondPhaseEndDate = '2026-06-20T11:44:45.576Z' + + stubPhaseLookups( + [{ id: sharedPhaseId, name: 'Registration', description: 'Registration phase' }], + [{ phaseId: sharedPhaseId, defaultDuration: staleDuration }] + ) + + const updatedPhases = await phaseHelper.populatePhasesForChallengeUpdate( + [ + { + id: 'first-challenge-phase', + duration: staleDuration, + name: 'Registration', + phaseId: sharedPhaseId, + isOpen: true, + scheduledStartDate: firstPhaseStartDate, + scheduledEndDate: '2026-06-20T09:29:45.575Z', + actualStartDate: firstPhaseStartDate, + actualEndDate: null + }, + { + id: 'second-challenge-phase', + duration: staleDuration, + name: 'Registration', + phaseId: sharedPhaseId, + isOpen: true, + scheduledStartDate: secondPhaseStartDate, + scheduledEndDate: '2026-06-20T09:29:45.576Z', + actualStartDate: secondPhaseStartDate, + actualEndDate: null + } + ], + [ + { + id: 'first-challenge-phase', + duration: staleDuration, + phaseId: sharedPhaseId, + scheduledEndDate: firstPhaseEndDate, + scheduledStartDate: firstPhaseStartDate + }, + { + id: 'second-challenge-phase', + duration: staleDuration, + phaseId: sharedPhaseId, + scheduledEndDate: secondPhaseEndDate, + scheduledStartDate: secondPhaseStartDate + } + ], + 'timeline-template-id', + false + ) + + const firstUpdatedPhase = updatedPhases.find((phase) => phase.id === 'first-challenge-phase') + const secondUpdatedPhase = updatedPhases.find((phase) => phase.id === 'second-challenge-phase') + + firstUpdatedPhase.scheduledEndDate.should.equal(firstPhaseEndDate) + firstUpdatedPhase.duration.should.equal(120 * 60 * 60 + 45 * 60) + secondUpdatedPhase.scheduledEndDate.should.equal(secondPhaseEndDate) + secondUpdatedPhase.duration.should.equal(122 * 60 * 60 + 15 * 60) + }) + it('uses scheduled end dates from update payload for MM phases', async () => { const registrationPhaseId = 'mm-registration-phase' const submissionPhaseId = 'mm-submission-phase' From ebdf8a9b327e94d65007162dff29da3a7e90cd47 Mon Sep 17 00:00:00 2001 From: jmgasper Date: Wed, 17 Jun 2026 12:15:38 +1000 Subject: [PATCH 2/6] PM-5378: Allow Design active phase shortening in API What was broken Active phase end dates could only move later. The API rejected shortening even for Design challenges, and direct phase updates only recalculated successor schedules when an end date was extended. Root cause The phase validation did not account for challenge track, and successor recalculation used an extended-only check. What was changed Added track-aware active phase end-date validation that allows Design track shortening while blocking end dates before the current date/time. Wired it into challenge updates and direct phase updates, and recalculated dependent phases whenever the active end date changes. Any added/updated tests Added phase-helper coverage for Design shortening, non-Design rejection, and past end-date rejection. Added a ChallengePhaseService regression test for Design shortening and successor schedule recalculation. --- src/common/phase-helper.js | 145 ++++++++++++++++++++++- src/services/ChallengePhaseService.js | 30 ++--- src/services/ChallengeService.js | 2 + test/unit/ChallengePhaseService.test.js | 135 +++++++++++++++++++++ test/unit/phase-helper.test.js | 150 ++++++++++++++++++++++++ 5 files changed, 443 insertions(+), 19 deletions(-) diff --git a/src/common/phase-helper.js b/src/common/phase-helper.js index 0274604..86fe8c9 100644 --- a/src/common/phase-helper.js +++ b/src/common/phase-helper.js @@ -9,6 +9,120 @@ const timelineTemplateService = require("../services/TimelineTemplateService"); const prisma = require("../common/prisma").getClient(); const SUBMISSION_PHASE_PRIORITY = ["Topgear Submission", "Topcoder Submission", "Submission"]; +const DESIGN_TRACK = "DESIGN"; + +/** + * Resolve a track object or token to the canonical track value used in challenge metadata. + * + * @param {Object|String|null|undefined} track challenge track relation, display value, or token + * @returns {String} normalized uppercase track token + */ +function normalizeTrackToken(track) { + if (_.isNil(track)) { + return ""; + } + + if (_.isString(track)) { + return _.toUpper(_.trim(track)); + } + + return _.toUpper( + _.trim( + _.get(track, "track") || + _.get(track, "name") || + _.get(track, "abbreviation") || + "" + ) + ); +} + +/** + * Check whether a challenge track represents Design. + * + * @param {Object|String|null|undefined} track challenge track relation, display value, or token + * @returns {Boolean} true when the track is Design + */ +function isDesignTrack(track) { + return normalizeTrackToken(track) === DESIGN_TRACK; +} + +/** + * 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"); + if (_.isNil(requestedDuration) || _.isNil(phase.scheduledStartDate)) { + return undefined; + } + + const scheduledStart = moment(phase.scheduledStartDate); + if (!scheduledStart.isValid()) { + return undefined; + } + + return scheduledStart.add(requestedDuration, "seconds").toDate().toISOString(); +} + +/** + * Validate an active phase scheduled end date change against PM-5378 rules. + * + * @param {Object} phase existing challenge phase + * @param {Date|String|null|undefined} requestedScheduledEndDate requested scheduled end date + * @param {Object} options validation options + * @param {Boolean} options.allowActivePhaseShortening whether active phase shortening is allowed + * @returns {undefined} validates only + * @throws {BadRequestError} when active phase shortening is disallowed or would end in the past + */ +function validateActivePhaseScheduledEndDateChange( + phase, + requestedScheduledEndDate, + options = {} +) { + if (_.isNil(phase) || phase.isOpen !== true || _.isNil(requestedScheduledEndDate)) { + return; + } + + const requestedEnd = moment(requestedScheduledEndDate); + if (!requestedEnd.isValid()) { + return; + } + + const currentEnd = moment(phase.scheduledEndDate); + const hasCurrentEnd = currentEnd.isValid(); + const hasChangedEndDate = !hasCurrentEnd || requestedEnd.valueOf() !== currentEnd.valueOf(); + + if (!hasChangedEndDate) { + return; + } + + if (requestedEnd.isBefore(moment())) { + throw new errors.BadRequestError( + "Active phase scheduledEndDate cannot be set before the current date/time." + ); + } + + if ( + hasCurrentEnd && + requestedEnd.isBefore(currentEnd) && + options.allowActivePhaseShortening !== true + ) { + throw new errors.BadRequestError( + "Active phases can only be shortened for Design track challenges." + ); + } +} /** * Apply an explicit scheduled end date to a phase and update its duration. @@ -142,7 +256,8 @@ class ChallengePhaseHelper { challengePhases, newPhases, timelineTemplateId, - isBeingActivated + isBeingActivated, + options = {} ) { const { timelineTemplateMap, timelineTempate } = await this.getTemplateAndTemplateMap( timelineTemplateId @@ -183,6 +298,11 @@ class ChallengePhaseHelper { const phaseFromTemplate = timelineTemplateMap.get(phase.phaseId); const phaseDefinition = phaseDefinitionMap.get(phase.phaseId); const newPhase = _.find(newPhases, (p) => p.phaseId === phase.phaseId); + validateActivePhaseScheduledEndDateChange( + phase, + resolveRequestedScheduledEndDate(phase, newPhase), + options + ); 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) @@ -340,6 +460,29 @@ class ChallengePhaseHelper { } return this.timelineTemplateMap[timelineTemplateId]; } + + /** + * Check whether a challenge track represents Design. + * + * @param {Object|String|null|undefined} track challenge track relation, display value, or token + * @returns {Boolean} true when the track is Design + */ + isDesignTrack(track) { + return isDesignTrack(track); + } + + /** + * Validate an active phase scheduled end date change against PM-5378 rules. + * + * @param {Object} phase existing challenge phase + * @param {Date|String|null|undefined} requestedScheduledEndDate requested scheduled end date + * @param {Object} options validation options + * @returns {undefined} validates only + * @throws {BadRequestError} when active phase shortening is disallowed or would end in the past + */ + validateActivePhaseScheduledEndDateChange(phase, requestedScheduledEndDate, options = {}) { + validateActivePhaseScheduledEndDateChange(phase, requestedScheduledEndDate, options); + } } module.exports = new ChallengePhaseHelper(); diff --git a/src/services/ChallengePhaseService.js b/src/services/ChallengePhaseService.js index cc991d9..9ffa80e 100644 --- a/src/services/ChallengePhaseService.js +++ b/src/services/ChallengePhaseService.js @@ -11,6 +11,7 @@ const logger = require("../common/logger"); const errors = require("../common/errors"); const constants = require("../../app-constants"); const { getReviewClient } = require("../common/review-prisma"); +const phaseHelper = require("../common/phase-helper"); const { indexChallengeAndPostToKafka, ensureAIPhaseCanBeClosed, @@ -50,18 +51,6 @@ function datesAreSame(dateA, dateB) { return new Date(dateA).getTime() === new Date(dateB).getTime(); } -function dateIsAfter(dateA, dateB) { - if (_.isNil(dateA) || _.isNil(dateB)) { - return false; - } - const timeA = new Date(dateA).getTime(); - const timeB = new Date(dateB).getTime(); - if (Number.isNaN(timeA) || Number.isNaN(timeB)) { - return false; - } - return timeA > timeB; -} - function buildPhaseIdentifiers(phase) { const identifiers = []; if (phase && phase.id) { @@ -479,13 +468,13 @@ async function hasPendingEscalationRequestsForChallenge(challengeId) { /** * Load a challenge for challenge-scoped phase operations. * @param {String} challengeId the challenge id - * @returns {Object} the challenge with the given id and type metadata + * @returns {Object} the challenge with the given id and type/track metadata * @throws {NotFoundError} when the challenge does not exist */ async function getChallengeForPhaseAccess(challengeId) { const challenge = await prisma.challenge.findUnique({ where: { id: challengeId }, - include: { type: true }, + include: { track: true, type: true }, }); if (!challenge) { throw new errors.NotFoundError(`Challenge with id: ${challengeId} doesn't exist`); @@ -928,6 +917,11 @@ async function partiallyUpdateChallengePhase(currentUser, challengeId, id, data) } } } + phaseHelper.validateActivePhaseScheduledEndDateChange( + challengePhase, + data.scheduledEndDate, + { allowActivePhaseShortening: phaseHelper.isDesignTrack(challenge.track) } + ); const dataToUpdate = _.omit(data, "constraints"); const shouldRefreshPhaseNames = Object.prototype.hasOwnProperty.call(data, "isOpen") || @@ -939,10 +933,10 @@ async function partiallyUpdateChallengePhase(currentUser, challengeId, id, data) id: challengePhase.id, }, }); - let scheduleExtended = false; + let scheduleEndChanged = false; if (shouldAttemptSuccessorRecalc) { - scheduleExtended = dateIsAfter(updatedPhase.scheduledEndDate, originalScheduledEndDate); - if (scheduleExtended) { + scheduleEndChanged = !datesAreSame(updatedPhase.scheduledEndDate, originalScheduledEndDate); + if (scheduleEndChanged) { await recalculateDependentPhaseDates(tx, challengeId, updatedPhase, currentUserId); } } @@ -952,7 +946,7 @@ async function partiallyUpdateChallengePhase(currentUser, challengeId, id, data) !_.isNil(updatedPhase.actualEndDate) ) { const shiftBaselineScheduledEndDate = - scheduleExtended && !_.isNil(updatedPhase.scheduledEndDate) + scheduleEndChanged && !_.isNil(updatedPhase.scheduledEndDate) ? updatedPhase.scheduledEndDate : originalScheduledEndDate; const scheduledEndTime = new Date(shiftBaselineScheduledEndDate).getTime(); diff --git a/src/services/ChallengeService.js b/src/services/ChallengeService.js index eff0b60..d7c72c8 100644 --- a/src/services/ChallengeService.js +++ b/src/services/ChallengeService.js @@ -3848,6 +3848,7 @@ async function updateChallenge(currentUser, challengeId, data, options = {}) { let isChallengeBeingActivated = isStatusChangingToActive; let isChallengeBeingCancelled = false; + const allowActivePhaseShortening = phaseHelper.isDesignTrack(challenge.track); const isStatusChangingToCancelled = isCancelledChallengeStatus(data.status) && !isCancelledChallengeStatus(challenge.status); if (data.status) { @@ -4045,6 +4046,7 @@ async function updateChallenge(currentUser, challengeId, data, options = {}) { data.phases, challenge.timelineTemplateId, isChallengeBeingActivated, + { allowActivePhaseShortening }, ); } phasesUpdated = true; diff --git a/test/unit/ChallengePhaseService.test.js b/test/unit/ChallengePhaseService.test.js index 0030472..455c848 100644 --- a/test/unit/ChallengePhaseService.test.js +++ b/test/unit/ChallengePhaseService.test.js @@ -1401,6 +1401,141 @@ describe('challenge phase service unit tests', () => { } }) + it('partially update challenge phase - allows Design active phase shortening and recalculates successor schedules', async function () { + this.timeout(50000) + const originalChallenge = await prisma.challenge.findUnique({ + where: { id: data.challenge.id }, + select: { trackId: true } + }) + let designTrack + let reviewPhase + let appealsPhase + const reviewChallengePhaseId = uuid() + const appealsChallengePhaseId = uuid() + const now = Date.now() + const reviewStartDate = new Date(now - 60 * 60 * 1000) + const reviewEndDate = new Date(now + 4 * 24 * 60 * 60 * 1000) + const shortenedReviewEndDate = new Date(now + 2 * 24 * 60 * 60 * 1000) + const reviewDuration = Math.round( + (reviewEndDate.getTime() - reviewStartDate.getTime()) / 1000 + ) + const appealsDuration = 43200 + + try { + designTrack = await prisma.challengeTrack.create({ + data: { + id: uuid(), + name: `Design ${shortId()}`, + description: 'Design track for active phase shortening tests', + isActive: true, + abbreviation: `D${shortId()}`, + track: 'DESIGN', + createdBy: 'admin', + updatedBy: 'admin' + } + }) + await prisma.challenge.update({ + where: { id: data.challenge.id }, + data: { trackId: designTrack.id } + }) + + reviewPhase = await prisma.phase.create({ + data: { + id: uuid(), + name: 'Review', + description: 'desc', + isOpen: false, + duration: 86400, + createdBy: 'admin', + updatedBy: 'admin' + } + }) + appealsPhase = await prisma.phase.create({ + data: { + id: uuid(), + name: 'Appeals', + description: 'desc', + isOpen: false, + duration: appealsDuration, + createdBy: 'admin', + updatedBy: 'admin' + } + }) + + await prisma.challengePhase.createMany({ + data: [ + { + id: reviewChallengePhaseId, + challengeId: data.challenge.id, + phaseId: reviewPhase.id, + name: 'Review', + duration: reviewDuration, + isOpen: true, + actualStartDate: reviewStartDate, + scheduledStartDate: reviewStartDate, + scheduledEndDate: reviewEndDate, + createdBy: 'admin', + updatedBy: 'admin' + }, + { + id: appealsChallengePhaseId, + challengeId: data.challenge.id, + phaseId: appealsPhase.id, + predecessor: reviewPhase.id, + name: 'Appeals', + duration: appealsDuration, + scheduledStartDate: reviewEndDate, + scheduledEndDate: new Date(reviewEndDate.getTime() + appealsDuration * 1000), + createdBy: 'admin', + updatedBy: 'admin' + } + ] + }) + + const updatedReview = await service.partiallyUpdateChallengePhase( + authUser, + data.challenge.id, + reviewChallengePhaseId, + { scheduledEndDate: shortenedReviewEndDate } + ) + + should.equal( + new Date(updatedReview.scheduledEndDate).toISOString(), + shortenedReviewEndDate.toISOString() + ) + + const updatedAppeals = await prisma.challengePhase.findUnique({ + where: { id: appealsChallengePhaseId } + }) + should.equal( + new Date(updatedAppeals.scheduledStartDate).toISOString(), + shortenedReviewEndDate.toISOString() + ) + should.equal( + new Date(updatedAppeals.scheduledEndDate).toISOString(), + new Date(shortenedReviewEndDate.getTime() + appealsDuration * 1000).toISOString() + ) + } finally { + await prisma.challengePhase.deleteMany({ + where: { id: { in: [reviewChallengePhaseId, appealsChallengePhaseId] } } + }) + if (reviewPhase || appealsPhase) { + await prisma.phase.deleteMany({ + where: { id: { in: _.compact([reviewPhase?.id, appealsPhase?.id]) } } + }) + } + if (originalChallenge) { + await prisma.challenge.update({ + where: { id: data.challenge.id }, + data: { trackId: originalChallenge.trackId } + }) + } + if (designTrack) { + await prisma.challengeTrack.delete({ where: { id: designTrack.id } }) + } + } + }) + it('partially update challenge phase - cannot close Appeals Response when appeals lack responses', async function () { this.timeout(50000) const appealsPhaseId = uuid() diff --git a/test/unit/phase-helper.test.js b/test/unit/phase-helper.test.js index dc3e84c..a4717c7 100644 --- a/test/unit/phase-helper.test.js +++ b/test/unit/phase-helper.test.js @@ -163,4 +163,154 @@ describe('phase helper unit tests', () => { updatedPhases[1].scheduledEndDate.should.equal(submissionEndDate) updatedPhases[1].duration.should.equal(3 * 24 * 60 * 60) }) + + it('allows active Design phases to be shortened to a future end date', async () => { + const registrationPhaseId = 'design-registration-phase' + const submissionPhaseId = 'design-submission-phase' + const staleDuration = 5 * 24 * 60 * 60 + const registrationStartDate = '2099-05-26T05:14:00.000Z' + const currentRegistrationEndDate = '2099-05-31T05:14:00.000Z' + const shortenedRegistrationEndDate = '2099-05-29T05:14:00.000Z' + + stubPhaseLookups( + [ + { id: registrationPhaseId, name: 'Registration', description: 'Registration phase' }, + { id: submissionPhaseId, name: 'Submission', description: 'Submission phase' } + ], + [ + { phaseId: registrationPhaseId, defaultDuration: staleDuration }, + { + phaseId: submissionPhaseId, + predecessor: registrationPhaseId, + defaultDuration: staleDuration + } + ] + ) + + const updatedPhases = await phaseHelper.populatePhasesForChallengeUpdate( + [ + { + duration: staleDuration, + isOpen: true, + name: 'Registration', + phaseId: registrationPhaseId, + scheduledStartDate: registrationStartDate, + scheduledEndDate: currentRegistrationEndDate + }, + { + duration: staleDuration, + name: 'Submission', + phaseId: submissionPhaseId, + predecessor: registrationPhaseId, + scheduledStartDate: currentRegistrationEndDate, + scheduledEndDate: '2099-06-05T05:14:00.000Z' + } + ], + [ + { + phaseId: registrationPhaseId, + scheduledEndDate: shortenedRegistrationEndDate + } + ], + 'timeline-template-id', + false, + { allowActivePhaseShortening: true } + ) + + updatedPhases[0].scheduledEndDate.should.equal(shortenedRegistrationEndDate) + updatedPhases[0].duration.should.equal(3 * 24 * 60 * 60) + updatedPhases[1].scheduledStartDate.should.equal(shortenedRegistrationEndDate) + }) + + it('rejects active phase shortening for non-Design tracks', async () => { + const registrationPhaseId = 'development-registration-phase' + const staleDuration = 5 * 24 * 60 * 60 + + stubPhaseLookups( + [ + { id: registrationPhaseId, name: 'Registration', description: 'Registration phase' } + ], + [ + { phaseId: registrationPhaseId, defaultDuration: staleDuration } + ] + ) + + try { + await phaseHelper.populatePhasesForChallengeUpdate( + [ + { + duration: staleDuration, + isOpen: true, + name: 'Registration', + phaseId: registrationPhaseId, + scheduledStartDate: '2099-05-26T05:14:00.000Z', + scheduledEndDate: '2099-05-31T05:14:00.000Z' + } + ], + [ + { + phaseId: registrationPhaseId, + scheduledEndDate: '2099-05-29T05:14:00.000Z' + } + ], + 'timeline-template-id', + false, + { allowActivePhaseShortening: false } + ) + } catch (e) { + e.message.should.equal('Active phases can only be shortened for Design track challenges.') + return + } + + throw new Error('should not reach here') + }) + + it('rejects active phase end dates before the current date/time', async () => { + const registrationPhaseId = 'past-registration-phase' + const now = Date.now() + const registrationStartDate = new Date(now - 2 * 60 * 60 * 1000).toISOString() + const pastRegistrationEndDate = new Date(now - 60 * 60 * 1000).toISOString() + const currentRegistrationEndDate = new Date(now + 24 * 60 * 60 * 1000).toISOString() + const staleDuration = 24 * 60 * 60 + + stubPhaseLookups( + [ + { id: registrationPhaseId, name: 'Registration', description: 'Registration phase' } + ], + [ + { phaseId: registrationPhaseId, defaultDuration: staleDuration } + ] + ) + + try { + await phaseHelper.populatePhasesForChallengeUpdate( + [ + { + duration: staleDuration, + isOpen: true, + name: 'Registration', + phaseId: registrationPhaseId, + scheduledStartDate: registrationStartDate, + scheduledEndDate: currentRegistrationEndDate + } + ], + [ + { + phaseId: registrationPhaseId, + scheduledEndDate: pastRegistrationEndDate + } + ], + 'timeline-template-id', + false, + { allowActivePhaseShortening: true } + ) + } catch (e) { + e.message.should.equal( + 'Active phase scheduledEndDate cannot be set before the current date/time.' + ) + return + } + + throw new Error('should not reach here') + }) }) From 9ba006d084eebfc73915d5d9501995d487b9dc88 Mon Sep 17 00:00:00 2001 From: jmgasper Date: Thu, 18 Jun 2026 11:04:09 +1000 Subject: [PATCH 3/6] PM-5378: Allow Design shortening across phases What was broken The previous fix only allowed Design-track shortening for the currently open phase. Active non-Design schedule validation could also reject unchanged saved end dates when a stale duration from the Work app implied shortening, and phase update matching did not use challenge phase row ids. Root cause The PM-5378 guard was keyed to phase.isOpen instead of the active challenge schedule as a whole. The update helper also had a row-id-aware matcher but still used phase definition ids in the main update path. What was changed Design challenges can now shorten any incomplete phase, provided the requested end date is not before the current date/time. Active non-Design challenges now guard every incomplete phase against shortening, while pre-launch non-Design reductions remain allowed. Full challenge updates and direct phase PATCHes both pass the active-challenge guard options, and challenge phase updates now prefer row id matching before phase definition id matching. Any added/updated tests Bootstrapped the phase helper test and added regressions for Design future-phase shortening, active non-Design future-phase rejection, pre-launch non-Design shortening, and stale duration payloads that keep the persisted end date. Focused phase-helper tests and pnpm lint passed. Full pnpm test is blocked without DATABASE_URL, and pnpm build is unavailable because the package has no build script. --- src/common/phase-helper.js | 37 ++-- src/services/ChallengePhaseService.js | 15 +- src/services/ChallengeService.js | 4 +- test/unit/phase-helper.test.js | 253 ++++++++++++++++++++++++-- 4 files changed, 274 insertions(+), 35 deletions(-) diff --git a/src/common/phase-helper.js b/src/common/phase-helper.js index 3945d17..50d0a5f 100644 --- a/src/common/phase-helper.js +++ b/src/common/phase-helper.js @@ -76,21 +76,26 @@ function resolveRequestedScheduledEndDate(phase, newPhase) { } /** - * Validate an active phase scheduled end date change against PM-5378 rules. + * Validate a phase scheduled end date change against PM-5378 rules. * * @param {Object} phase existing challenge phase * @param {Date|String|null|undefined} requestedScheduledEndDate requested scheduled end date * @param {Object} options validation options - * @param {Boolean} options.allowActivePhaseShortening whether active phase shortening is allowed + * @param {Boolean} options.allowActivePhaseShortening whether Design track phase shortening is allowed + * @param {Boolean} options.preventPhaseShortening whether shortening is guarded for all incomplete phases * @returns {undefined} validates only - * @throws {BadRequestError} when active phase shortening is disallowed or would end in the past + * @throws {BadRequestError} when phase shortening is disallowed or would end in the past */ function validateActivePhaseScheduledEndDateChange( phase, requestedScheduledEndDate, options = {} ) { - if (_.isNil(phase) || phase.isOpen !== true || _.isNil(requestedScheduledEndDate)) { + if (!_.isNil(phase?.actualEndDate)) { + return; + } + + if (_.isNil(phase) || _.isNil(requestedScheduledEndDate)) { return; } @@ -107,19 +112,25 @@ function validateActivePhaseScheduledEndDateChange( return; } - if (requestedEnd.isBefore(moment())) { + const shouldValidatePhaseEnd = + phase.isOpen === true || + options.allowActivePhaseShortening === true || + options.preventPhaseShortening === true; + const isShortened = hasCurrentEnd && requestedEnd.isBefore(currentEnd); + + if (shouldValidatePhaseEnd && requestedEnd.isBefore(moment())) { throw new errors.BadRequestError( - "Active phase scheduledEndDate cannot be set before the current date/time." + "Phase scheduledEndDate cannot be set before the current date/time." ); } if ( - hasCurrentEnd && - requestedEnd.isBefore(currentEnd) && - options.allowActivePhaseShortening !== true + isShortened && + options.allowActivePhaseShortening !== true && + (phase.isOpen === true || options.preventPhaseShortening === true) ) { throw new errors.BadRequestError( - "Active phases can only be shortened for Design track challenges." + "Challenge phase schedules can only be shortened for Design track challenges." ); } } @@ -320,7 +331,7 @@ class ChallengePhaseHelper { const updatedPhases = _.map(challengePhasesOrdered, (phase) => { const phaseFromTemplate = timelineTemplateMap.get(phase.phaseId); const phaseDefinition = phaseDefinitionMap.get(phase.phaseId); - const newPhase = _.find(newPhases, (p) => p.phaseId === phase.phaseId); + const newPhase = findPhaseUpdate(newPhases, phase); validateActivePhaseScheduledEndDateChange( phase, resolveRequestedScheduledEndDate(phase, newPhase), @@ -495,13 +506,13 @@ class ChallengePhaseHelper { } /** - * Validate an active phase scheduled end date change against PM-5378 rules. + * Validate a phase scheduled end date change against PM-5378 rules. * * @param {Object} phase existing challenge phase * @param {Date|String|null|undefined} requestedScheduledEndDate requested scheduled end date * @param {Object} options validation options * @returns {undefined} validates only - * @throws {BadRequestError} when active phase shortening is disallowed or would end in the past + * @throws {BadRequestError} when phase shortening is disallowed or would end in the past */ validateActivePhaseScheduledEndDateChange(phase, requestedScheduledEndDate, options = {}) { validateActivePhaseScheduledEndDateChange(phase, requestedScheduledEndDate, options); diff --git a/src/services/ChallengePhaseService.js b/src/services/ChallengePhaseService.js index 9ffa80e..250f3bf 100644 --- a/src/services/ChallengePhaseService.js +++ b/src/services/ChallengePhaseService.js @@ -17,7 +17,7 @@ const { ensureAIPhaseCanBeClosed, } = require("./ChallengeService"); -const { getClient } = require("../common/prisma"); +const { getClient, ChallengeStatusEnum } = require("../common/prisma"); const prisma = getClient(); const PENDING_REVIEW_STATUSES = Object.freeze(["PENDING", "IN_PROGRESS", "DRAFT", "SUBMITTED"]); const REVIEW_PHASE_NAMES = Object.freeze([ @@ -648,6 +648,7 @@ getChallengePhase.schema = { * @param {Object} data the partial phase update * @returns {Object} the updated challengePhase * @throws {ForbiddenError} when the current user cannot modify the challenge + * @throws {BadRequestError} when phase schedule shortening violates track or timing rules */ async function partiallyUpdateChallengePhase(currentUser, challengeId, id, data) { const challenge = await getChallengeForPhaseAccess(challengeId); @@ -917,11 +918,13 @@ async function partiallyUpdateChallengePhase(currentUser, challengeId, id, data) } } } - phaseHelper.validateActivePhaseScheduledEndDateChange( - challengePhase, - data.scheduledEndDate, - { allowActivePhaseShortening: phaseHelper.isDesignTrack(challenge.track) } - ); + const allowActivePhaseShortening = phaseHelper.isDesignTrack(challenge.track); + const preventPhaseShortening = + challenge.status === ChallengeStatusEnum.ACTIVE && !allowActivePhaseShortening; + phaseHelper.validateActivePhaseScheduledEndDateChange(challengePhase, data.scheduledEndDate, { + allowActivePhaseShortening, + preventPhaseShortening, + }); const dataToUpdate = _.omit(data, "constraints"); const shouldRefreshPhaseNames = Object.prototype.hasOwnProperty.call(data, "isOpen") || diff --git a/src/services/ChallengeService.js b/src/services/ChallengeService.js index d7c72c8..2bb4b00 100644 --- a/src/services/ChallengeService.js +++ b/src/services/ChallengeService.js @@ -3849,6 +3849,8 @@ async function updateChallenge(currentUser, challengeId, data, options = {}) { let isChallengeBeingActivated = isStatusChangingToActive; let isChallengeBeingCancelled = false; const allowActivePhaseShortening = phaseHelper.isDesignTrack(challenge.track); + const preventPhaseShortening = + challenge.status === ChallengeStatusEnum.ACTIVE && !allowActivePhaseShortening; const isStatusChangingToCancelled = isCancelledChallengeStatus(data.status) && !isCancelledChallengeStatus(challenge.status); if (data.status) { @@ -4046,7 +4048,7 @@ async function updateChallenge(currentUser, challengeId, data, options = {}) { data.phases, challenge.timelineTemplateId, isChallengeBeingActivated, - { allowActivePhaseShortening }, + { allowActivePhaseShortening, preventPhaseShortening }, ); } phasesUpdated = true; diff --git a/test/unit/phase-helper.test.js b/test/unit/phase-helper.test.js index 4244eb4..e59cd92 100644 --- a/test/unit/phase-helper.test.js +++ b/test/unit/phase-helper.test.js @@ -1,5 +1,6 @@ const chai = require('chai') +require('../../app-bootstrap') const phaseHelper = require('../../src/common/phase-helper') chai.should() @@ -289,19 +290,116 @@ describe('phase helper unit tests', () => { updatedPhases[1].scheduledStartDate.should.equal(shortenedRegistrationEndDate) }) - it('rejects active phase shortening for non-Design tracks', async () => { - const registrationPhaseId = 'development-registration-phase' - const staleDuration = 5 * 24 * 60 * 60 + it('allows future Design phases to be shortened to a future end date', async () => { + const registrationPhaseId = 'design-registration-phase' + const reviewPhaseId = 'design-review-phase' + const registrationDuration = 2 * 24 * 60 * 60 + const reviewDuration = 5 * 24 * 60 * 60 + const registrationStartDate = '2099-05-26T05:14:00.000Z' + const registrationEndDate = '2099-05-28T05:14:00.000Z' + const currentReviewEndDate = '2099-06-02T05:14:00.000Z' + const shortenedReviewEndDate = '2099-05-30T05:14:00.000Z' stubPhaseLookups( [ - { id: registrationPhaseId, name: 'Registration', description: 'Registration phase' } + { id: registrationPhaseId, name: 'Registration', description: 'Registration phase' }, + { id: reviewPhaseId, name: 'Review', description: 'Review phase' } ], [ - { phaseId: registrationPhaseId, defaultDuration: staleDuration } + { phaseId: registrationPhaseId, defaultDuration: registrationDuration }, + { + phaseId: reviewPhaseId, + predecessor: registrationPhaseId, + defaultDuration: reviewDuration + } ] ) + const updatedPhases = await phaseHelper.populatePhasesForChallengeUpdate( + [ + { + duration: registrationDuration, + isOpen: true, + name: 'Registration', + phaseId: registrationPhaseId, + scheduledStartDate: registrationStartDate, + scheduledEndDate: registrationEndDate + }, + { + duration: reviewDuration, + name: 'Review', + phaseId: reviewPhaseId, + predecessor: registrationPhaseId, + scheduledStartDate: registrationEndDate, + scheduledEndDate: currentReviewEndDate + } + ], + [ + { + phaseId: reviewPhaseId, + scheduledEndDate: shortenedReviewEndDate + } + ], + 'timeline-template-id', + false, + { allowActivePhaseShortening: true } + ) + + updatedPhases[1].scheduledStartDate.should.equal(registrationEndDate) + updatedPhases[1].scheduledEndDate.should.equal(shortenedReviewEndDate) + updatedPhases[1].duration.should.equal(2 * 24 * 60 * 60) + }) + + it('keeps a persisted end date when a stale duration would imply active non-Design shortening', async () => { + const registrationPhaseId = 'development-registration-phase' + const registrationStartDate = '2099-05-26T05:14:00.000Z' + const currentRegistrationEndDate = '2099-05-27T05:14:00.000Z' + const staleShortDuration = 23 * 60 * 60 + + stubPhaseLookups( + [{ id: registrationPhaseId, name: 'Registration', description: 'Registration phase' }], + [{ phaseId: registrationPhaseId, defaultDuration: 6 * 24 * 60 * 60 }] + ) + + const updatedPhases = await phaseHelper.populatePhasesForChallengeUpdate( + [ + { + duration: 24 * 60 * 60, + isOpen: true, + name: 'Registration', + phaseId: registrationPhaseId, + scheduledStartDate: registrationStartDate, + scheduledEndDate: currentRegistrationEndDate + } + ], + [ + { + duration: staleShortDuration, + phaseId: registrationPhaseId, + scheduledEndDate: currentRegistrationEndDate + } + ], + 'timeline-template-id', + false, + { + allowActivePhaseShortening: false, + preventPhaseShortening: true + } + ) + + updatedPhases[0].scheduledEndDate.should.equal(currentRegistrationEndDate) + updatedPhases[0].duration.should.equal(24 * 60 * 60) + }) + + it('rejects active phase shortening for non-Design tracks', async () => { + const registrationPhaseId = 'development-registration-phase' + const staleDuration = 5 * 24 * 60 * 60 + + stubPhaseLookups( + [{ id: registrationPhaseId, name: 'Registration', description: 'Registration phase' }], + [{ phaseId: registrationPhaseId, defaultDuration: staleDuration }] + ) + try { await phaseHelper.populatePhasesForChallengeUpdate( [ @@ -325,13 +423,144 @@ describe('phase helper unit tests', () => { { allowActivePhaseShortening: false } ) } catch (e) { - e.message.should.equal('Active phases can only be shortened for Design track challenges.') + e.message.should.equal( + 'Challenge phase schedules can only be shortened for Design track challenges.' + ) + return + } + + throw new Error('should not reach here') + }) + + it('rejects future phase shortening for active non-Design challenges', async () => { + const registrationPhaseId = 'development-registration-phase' + const reviewPhaseId = 'development-review-phase' + const registrationDuration = 2 * 24 * 60 * 60 + const reviewDuration = 5 * 24 * 60 * 60 + const registrationStartDate = '2099-05-26T05:14:00.000Z' + const registrationEndDate = '2099-05-28T05:14:00.000Z' + const currentReviewEndDate = '2099-06-02T05:14:00.000Z' + const shortenedReviewEndDate = '2099-05-30T05:14:00.000Z' + + stubPhaseLookups( + [ + { id: registrationPhaseId, name: 'Registration', description: 'Registration phase' }, + { id: reviewPhaseId, name: 'Review', description: 'Review phase' } + ], + [ + { phaseId: registrationPhaseId, defaultDuration: registrationDuration }, + { + phaseId: reviewPhaseId, + predecessor: registrationPhaseId, + defaultDuration: reviewDuration + } + ] + ) + + try { + await phaseHelper.populatePhasesForChallengeUpdate( + [ + { + duration: registrationDuration, + isOpen: true, + name: 'Registration', + phaseId: registrationPhaseId, + scheduledStartDate: registrationStartDate, + scheduledEndDate: registrationEndDate + }, + { + duration: reviewDuration, + name: 'Review', + phaseId: reviewPhaseId, + predecessor: registrationPhaseId, + scheduledStartDate: registrationEndDate, + scheduledEndDate: currentReviewEndDate + } + ], + [ + { + phaseId: reviewPhaseId, + scheduledEndDate: shortenedReviewEndDate + } + ], + 'timeline-template-id', + false, + { + allowActivePhaseShortening: false, + preventPhaseShortening: true + } + ) + } catch (e) { + e.message.should.equal( + 'Challenge phase schedules can only be shortened for Design track challenges.' + ) return } throw new Error('should not reach here') }) + it('allows future non-Design phases to be shortened before launch', async () => { + const registrationPhaseId = 'development-registration-phase' + const reviewPhaseId = 'development-review-phase' + const registrationDuration = 2 * 24 * 60 * 60 + const reviewDuration = 5 * 24 * 60 * 60 + const registrationStartDate = '2099-05-26T05:14:00.000Z' + const registrationEndDate = '2099-05-28T05:14:00.000Z' + const currentReviewEndDate = '2099-06-02T05:14:00.000Z' + const shortenedReviewEndDate = '2099-05-30T05:14:00.000Z' + + stubPhaseLookups( + [ + { id: registrationPhaseId, name: 'Registration', description: 'Registration phase' }, + { id: reviewPhaseId, name: 'Review', description: 'Review phase' } + ], + [ + { phaseId: registrationPhaseId, defaultDuration: registrationDuration }, + { + phaseId: reviewPhaseId, + predecessor: registrationPhaseId, + defaultDuration: reviewDuration + } + ] + ) + + const updatedPhases = await phaseHelper.populatePhasesForChallengeUpdate( + [ + { + duration: registrationDuration, + name: 'Registration', + phaseId: registrationPhaseId, + scheduledStartDate: registrationStartDate, + scheduledEndDate: registrationEndDate + }, + { + duration: reviewDuration, + name: 'Review', + phaseId: reviewPhaseId, + predecessor: registrationPhaseId, + scheduledStartDate: registrationEndDate, + scheduledEndDate: currentReviewEndDate + } + ], + [ + { + phaseId: reviewPhaseId, + scheduledEndDate: shortenedReviewEndDate + } + ], + 'timeline-template-id', + false, + { + allowActivePhaseShortening: false, + preventPhaseShortening: false + } + ) + + updatedPhases[1].scheduledEndDate.should.equal(shortenedReviewEndDate) + updatedPhases[1].duration.should.equal(2 * 24 * 60 * 60) + }) + it('rejects active phase end dates before the current date/time', async () => { const registrationPhaseId = 'past-registration-phase' const now = Date.now() @@ -341,12 +570,8 @@ describe('phase helper unit tests', () => { const staleDuration = 24 * 60 * 60 stubPhaseLookups( - [ - { id: registrationPhaseId, name: 'Registration', description: 'Registration phase' } - ], - [ - { phaseId: registrationPhaseId, defaultDuration: staleDuration } - ] + [{ id: registrationPhaseId, name: 'Registration', description: 'Registration phase' }], + [{ phaseId: registrationPhaseId, defaultDuration: staleDuration }] ) try { @@ -372,9 +597,7 @@ describe('phase helper unit tests', () => { { allowActivePhaseShortening: true } ) } catch (e) { - e.message.should.equal( - 'Active phase scheduledEndDate cannot be set before the current date/time.' - ) + e.message.should.equal('Phase scheduledEndDate cannot be set before the current date/time.') return } From 9f92126e7dd39c93d3c8fe53fb115362ed7fd4e0 Mon Sep 17 00:00:00 2001 From: jmgasper Date: Mon, 22 Jun 2026 16:15:43 +1000 Subject: [PATCH 4/6] PM-5378: Preserve explicit phase end dates What was broken Direct phase updates could include both an explicit scheduledEndDate and a stale duration. The stale duration won, recalculating the end date and causing active challenge edits to be rejected or saved with the wrong duration. Root cause ChallengePhaseService.partiallyUpdateChallengePhase treated any duration in the payload as authoritative before considering a user-selected scheduledEndDate. What was changed When scheduledEndDate is provided, derive duration from the phase start and that explicit end date. Only derive scheduledEndDate from duration when no explicit end date is present. Kept the phase-helper regression fixture dates in the future so current-time validation remains deterministic. Any added/updated tests Added ChallengePhaseService coverage for explicit scheduledEndDate taking precedence over stale duration. Updated the PM-5378 phase-helper regression fixture dates. --- src/services/ChallengePhaseService.js | 15 ++++++++++++++- test/unit/ChallengePhaseService.test.js | 22 ++++++++++++++++++++++ test/unit/phase-helper.test.js | 12 ++++++------ 3 files changed, 42 insertions(+), 7 deletions(-) diff --git a/src/services/ChallengePhaseService.js b/src/services/ChallengePhaseService.js index 250f3bf..20fc50a 100644 --- a/src/services/ChallengePhaseService.js +++ b/src/services/ChallengePhaseService.js @@ -902,7 +902,20 @@ async function partiallyUpdateChallengePhase(currentUser, challengeId, id, data) // Update ChallengePhase const currentUserId = String(currentUser.userId); data.updatedBy = currentUserId; - if (!_.isNil(data.duration)) { + if (!_.isNil(data.scheduledEndDate)) { + const startInput = !_.isNil(data.scheduledStartDate) + ? data.scheduledStartDate + : !_.isNil(challengePhase.scheduledStartDate) + ? challengePhase.scheduledStartDate + : null; + if (startInput) { + const startDate = new Date(startInput); + const endDate = new Date(data.scheduledEndDate); + if (!Number.isNaN(startDate.getTime()) && !Number.isNaN(endDate.getTime())) { + data.duration = Math.floor((endDate.getTime() - startDate.getTime()) / 1000); + } + } + } else if (!_.isNil(data.duration)) { const startInput = !_.isNil(data.scheduledStartDate) ? data.scheduledStartDate : !_.isNil(challengePhase.scheduledStartDate) diff --git a/test/unit/ChallengePhaseService.test.js b/test/unit/ChallengePhaseService.test.js index 455c848..e04353f 100644 --- a/test/unit/ChallengePhaseService.test.js +++ b/test/unit/ChallengePhaseService.test.js @@ -391,6 +391,28 @@ describe('challenge phase service unit tests', () => { ) }) + it('partially update challenge phase - explicit scheduledEndDate wins over stale duration', async function () { + this.timeout(50000) + const scheduledStartDate = '2025-01-01T00:00:00.000Z' + const scheduledEndDate = new Date( + new Date(scheduledStartDate).getTime() + 7200 * 1000 + ).toISOString() + const challengePhase = await service.partiallyUpdateChallengePhase( + authUser, + data.challenge.id, + data.challengePhase1Id, + { + duration: 3600, + scheduledEndDate, + scheduledStartDate + } + ) + + should.equal(new Date(challengePhase.scheduledStartDate).toISOString(), scheduledStartDate) + should.equal(new Date(challengePhase.scheduledEndDate).toISOString(), scheduledEndDate) + should.equal(challengePhase.duration, 7200) + }) + it('partially update challenge phase - closing sets actual end date', async () => { await prisma.challengePhase.update({ where: { id: data.challengePhase1Id }, diff --git a/test/unit/phase-helper.test.js b/test/unit/phase-helper.test.js index e59cd92..12d4c87 100644 --- a/test/unit/phase-helper.test.js +++ b/test/unit/phase-helper.test.js @@ -103,10 +103,10 @@ describe('phase helper unit tests', () => { it('matches launched phase updates by challenge phase id before phase definition id', async () => { const sharedPhaseId = 'shared-registration-phase' const staleDuration = 120 * 60 * 60 - const firstPhaseStartDate = '2026-06-15T09:29:45.575Z' - const secondPhaseStartDate = '2026-06-15T09:29:45.576Z' - const firstPhaseEndDate = '2026-06-20T10:14:45.575Z' - const secondPhaseEndDate = '2026-06-20T11:44:45.576Z' + const firstPhaseStartDate = '2099-06-15T09:29:45.575Z' + const secondPhaseStartDate = '2099-06-15T09:29:45.576Z' + const firstPhaseEndDate = '2099-06-20T10:14:45.575Z' + const secondPhaseEndDate = '2099-06-20T11:44:45.576Z' stubPhaseLookups( [{ id: sharedPhaseId, name: 'Registration', description: 'Registration phase' }], @@ -122,7 +122,7 @@ describe('phase helper unit tests', () => { phaseId: sharedPhaseId, isOpen: true, scheduledStartDate: firstPhaseStartDate, - scheduledEndDate: '2026-06-20T09:29:45.575Z', + scheduledEndDate: '2099-06-20T09:29:45.575Z', actualStartDate: firstPhaseStartDate, actualEndDate: null }, @@ -133,7 +133,7 @@ describe('phase helper unit tests', () => { phaseId: sharedPhaseId, isOpen: true, scheduledStartDate: secondPhaseStartDate, - scheduledEndDate: '2026-06-20T09:29:45.576Z', + scheduledEndDate: '2099-06-20T09:29:45.576Z', actualStartDate: secondPhaseStartDate, actualEndDate: null } From c316bc90e98824ea3d00eaeee33ade6f776987bd Mon Sep 17 00:00:00 2001 From: jmgasper Date: Tue, 23 Jun 2026 18:10:48 +1000 Subject: [PATCH 5/6] PM-5378: Allow unchanged-duration active schedule shifts What was broken Active Development challenge edits could still be rejected with the Design-only shortening error when a user moved a phase start earlier, or switched the challenge start to Immediately, while keeping the phase duration unchanged. Root cause The active phase schedule guard compared the requested scheduled end date only against the persisted end date. When the start date also moved earlier, an unchanged-duration phase had an earlier end date and was incorrectly classified as a shortened phase. What was changed The phase helper now resolves the requested scheduled start date before validation and treats a non-Design active update as shortening only when the requested phase duration is actually reduced. The direct phase PATCH path now passes the same requested start-date context to the shared validation. Any added/updated tests Added phase-helper regression coverage for allowing active non-Design phase start shifts when duration is unchanged, and for still rejecting active non-Design updates that reduce duration after moving the start earlier. --- src/common/phase-helper.js | 63 ++++++++++++++++-- src/services/ChallengePhaseService.js | 4 ++ test/unit/phase-helper.test.js | 94 +++++++++++++++++++++++++++ 3 files changed, 157 insertions(+), 4 deletions(-) diff --git a/src/common/phase-helper.js b/src/common/phase-helper.js index 50d0a5f..639706c 100644 --- a/src/common/phase-helper.js +++ b/src/common/phase-helper.js @@ -46,6 +46,21 @@ 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. * @@ -63,11 +78,12 @@ function resolveRequestedScheduledEndDate(phase, newPhase) { } const requestedDuration = _.get(newPhase, "duration"); - if (_.isNil(requestedDuration) || _.isNil(phase.scheduledStartDate)) { + const requestedScheduledStartDate = resolveRequestedScheduledStartDate(phase, newPhase); + if (_.isNil(requestedDuration) || _.isNil(requestedScheduledStartDate)) { return undefined; } - const scheduledStart = moment(phase.scheduledStartDate); + const scheduledStart = moment(requestedScheduledStartDate); if (!scheduledStart.isValid()) { return undefined; } @@ -75,6 +91,34 @@ function resolveRequestedScheduledEndDate(phase, newPhase) { return scheduledStart.add(requestedDuration, "seconds").toDate().toISOString(); } +/** + * Check whether a requested schedule reduces the phase duration. + * + * @param {Object} phase existing challenge phase + * @param {Date|String|null|undefined} requestedScheduledStartDate requested phase start date + * @param {Date|String} requestedScheduledEndDate requested phase end date + * @returns {Boolean} true when requested duration is shorter than persisted duration + */ +function isPhaseDurationShortened(phase, requestedScheduledStartDate, requestedScheduledEndDate) { + const currentStart = moment(phase.scheduledStartDate); + const currentEnd = moment(phase.scheduledEndDate); + const requestedStart = moment( + _.defaultTo(requestedScheduledStartDate, phase.scheduledStartDate) + ); + const requestedEnd = moment(requestedScheduledEndDate); + + if ( + !currentStart.isValid() || + !currentEnd.isValid() || + !requestedStart.isValid() || + !requestedEnd.isValid() + ) { + return requestedEnd.isBefore(currentEnd); + } + + return requestedEnd.diff(requestedStart, "seconds") < currentEnd.diff(currentStart, "seconds"); +} + /** * Validate a phase scheduled end date change against PM-5378 rules. * @@ -83,6 +127,7 @@ function resolveRequestedScheduledEndDate(phase, newPhase) { * @param {Object} options validation options * @param {Boolean} options.allowActivePhaseShortening whether Design track phase shortening is allowed * @param {Boolean} options.preventPhaseShortening whether shortening is guarded for all incomplete phases + * @param {Date|String|null|undefined} options.requestedScheduledStartDate requested scheduled start date * @returns {undefined} validates only * @throws {BadRequestError} when phase shortening is disallowed or would end in the past */ @@ -116,7 +161,14 @@ function validateActivePhaseScheduledEndDateChange( phase.isOpen === true || options.allowActivePhaseShortening === true || options.preventPhaseShortening === true; - const isShortened = hasCurrentEnd && requestedEnd.isBefore(currentEnd); + const isShortened = + hasCurrentEnd && + requestedEnd.isBefore(currentEnd) && + isPhaseDurationShortened( + phase, + options.requestedScheduledStartDate, + requestedScheduledEndDate + ); if (shouldValidatePhaseEnd && requestedEnd.isBefore(moment())) { throw new errors.BadRequestError( @@ -335,7 +387,10 @@ class ChallengePhaseHelper { validateActivePhaseScheduledEndDateChange( phase, resolveRequestedScheduledEndDate(phase, newPhase), - options + { + ...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. diff --git a/src/services/ChallengePhaseService.js b/src/services/ChallengePhaseService.js index 20fc50a..07cd85e 100644 --- a/src/services/ChallengePhaseService.js +++ b/src/services/ChallengePhaseService.js @@ -934,9 +934,13 @@ async function partiallyUpdateChallengePhase(currentUser, challengeId, id, data) const allowActivePhaseShortening = phaseHelper.isDesignTrack(challenge.track); const preventPhaseShortening = challenge.status === ChallengeStatusEnum.ACTIVE && !allowActivePhaseShortening; + const requestedScheduledStartDate = !_.isNil(data.scheduledStartDate) + ? data.scheduledStartDate + : challengePhase.scheduledStartDate; phaseHelper.validateActivePhaseScheduledEndDateChange(challengePhase, data.scheduledEndDate, { allowActivePhaseShortening, preventPhaseShortening, + requestedScheduledStartDate, }); const dataToUpdate = _.omit(data, "constraints"); const shouldRefreshPhaseNames = diff --git a/test/unit/phase-helper.test.js b/test/unit/phase-helper.test.js index 12d4c87..84d9c23 100644 --- a/test/unit/phase-helper.test.js +++ b/test/unit/phase-helper.test.js @@ -391,6 +391,100 @@ describe('phase helper unit tests', () => { updatedPhases[0].duration.should.equal(24 * 60 * 60) }) + it('allows active non-Design phase start to move earlier when duration is unchanged', async () => { + const registrationPhaseId = 'development-registration-phase' + const currentRegistrationStartDate = '2099-05-26T05:14:00.000Z' + const currentRegistrationEndDate = '2099-05-31T05:14:00.000Z' + const requestedRegistrationStartDate = '2099-05-25T05:14:00.000Z' + const requestedRegistrationEndDate = '2099-05-30T05:14:00.000Z' + const duration = 5 * 24 * 60 * 60 + + stubPhaseLookups( + [{ id: registrationPhaseId, name: 'Registration', description: 'Registration phase' }], + [{ phaseId: registrationPhaseId, defaultDuration: duration }] + ) + + const updatedPhases = await phaseHelper.populatePhasesForChallengeUpdate( + [ + { + duration, + isOpen: true, + name: 'Registration', + phaseId: registrationPhaseId, + scheduledStartDate: currentRegistrationStartDate, + scheduledEndDate: currentRegistrationEndDate + } + ], + [ + { + duration, + phaseId: registrationPhaseId, + scheduledStartDate: requestedRegistrationStartDate, + scheduledEndDate: requestedRegistrationEndDate + } + ], + 'timeline-template-id', + false, + { + allowActivePhaseShortening: false, + preventPhaseShortening: true + } + ) + + updatedPhases[0].scheduledStartDate.should.equal(requestedRegistrationStartDate) + updatedPhases[0].scheduledEndDate.should.equal(requestedRegistrationEndDate) + updatedPhases[0].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' + const currentRegistrationEndDate = '2099-05-31T05:14:00.000Z' + const requestedRegistrationStartDate = '2099-05-25T05:14:00.000Z' + const requestedRegistrationEndDate = '2099-05-29T05:14:00.000Z' + const duration = 5 * 24 * 60 * 60 + + stubPhaseLookups( + [{ id: registrationPhaseId, name: 'Registration', description: 'Registration phase' }], + [{ phaseId: registrationPhaseId, defaultDuration: duration }] + ) + + try { + await phaseHelper.populatePhasesForChallengeUpdate( + [ + { + duration, + isOpen: true, + name: 'Registration', + phaseId: registrationPhaseId, + scheduledStartDate: currentRegistrationStartDate, + scheduledEndDate: currentRegistrationEndDate + } + ], + [ + { + phaseId: registrationPhaseId, + scheduledStartDate: requestedRegistrationStartDate, + scheduledEndDate: requestedRegistrationEndDate + } + ], + 'timeline-template-id', + false, + { + allowActivePhaseShortening: false, + preventPhaseShortening: true + } + ) + } catch (e) { + e.message.should.equal( + 'Challenge phase schedules can only be shortened for Design track challenges.' + ) + return + } + + throw new Error('should not reach here') + }) + it('rejects active phase shortening for non-Design tracks', async () => { const registrationPhaseId = 'development-registration-phase' const staleDuration = 5 * 24 * 60 * 60 From e5c2030cd2134e2f82a44bcc264ad4573d753c5e Mon Sep 17 00:00:00 2001 From: jmgasper Date: Wed, 24 Jun 2026 08:24:21 +1000 Subject: [PATCH 6/6] Allow M2M tokens to access challenges with groups associated with them --- app-routes.js | 124 +++++++++++++++++++++++++---- test/unit/ChallengeRoutes.test.js | 32 +++++++- test/unit/helper-whitelist.test.js | 4 + 3 files changed, 141 insertions(+), 19 deletions(-) diff --git a/app-routes.js b/app-routes.js index dac9b7f..5abc141 100644 --- a/app-routes.js +++ b/app-routes.js @@ -13,6 +13,85 @@ const logger = require("./src/common/logger"); const routes = require("./src/routes"); const authenticator = require("tc-core-library-js").middleware.jwtAuthenticator; +/** + * Returns whether a normalized auth user has interactive roles. + * + * @param {Object} authUser the decoded auth user from the authenticator + * @returns {Boolean} true when the caller has one or more user roles + */ +function hasInteractiveRoles(authUser) { + const roles = _.get(authUser, "roles"); + return _.isArray(roles) ? roles.length > 0 : !!roles; +} + +/** + * Read the scope claim from an auth user. + * + * The shared authenticator normally copies Auth0's `scope` string into + * `scopes`, but this also handles callers already carrying either shape. + * + * @param {Object} authUser the decoded auth user from the authenticator + * @returns {Array|String|undefined} scopes from the token + */ +function getAuthUserScopes(authUser) { + if (!authUser) { + return undefined; + } + return ( + authUser.scopes || + _.find(authUser, (value, key) => { + return key.indexOf("scope") !== -1; + }) + ); +} + +/** + * Determine whether an auth user represents an M2M caller. + * + * Some valid client-credentials tokens are decoded with scopes but without the + * `isMachine` flag. Treat no-role scoped callers as M2M so route-level scope + * checks, not user group membership, decide access. + * + * @param {Object} authUser the decoded auth user from the authenticator + * @returns {Boolean} true when the caller should be handled as M2M + */ +function isM2MAuthUser(authUser) { + return !!( + authUser && + (_.get(authUser, "isMachine", false) || + (getAuthUserScopes(authUser) && !hasInteractiveRoles(authUser))) + ); +} + +/** + * Check whether an M2M caller has any scope required by the route definition. + * + * @param {Object} def route definition from src/routes.js + * @param {Object} authUser the decoded auth user from the authenticator + * @returns {Boolean} true when one required route scope is present + */ +function hasRequiredM2MScopes(def, authUser) { + const scopes = getAuthUserScopes(authUser); + return !!(def.scopes && scopes && helper.checkIfExists(def.scopes, scopes)); +} + +/** + * Normalize a valid M2M caller to the shape expected by service authorization. + * + * @param {Object} def route definition from src/routes.js + * @param {Object} authUser the decoded auth user from the authenticator + * @returns {Boolean} true when the caller has required M2M scopes + */ +function normalizeM2MAuthUser(def, authUser) { + if (!isM2MAuthUser(authUser) || !hasRequiredM2MScopes(def, authUser)) { + return false; + } + const scopes = getAuthUserScopes(authUser); + authUser.isMachine = true; + authUser.scopes = _.isString(scopes) ? scopes.split(" ") : scopes; + return true; +} + const sanitizeForLog = (value) => { const seen = new WeakSet(); try { @@ -43,7 +122,7 @@ const getSignature = (req) => req.signature || req._reqLogId || "no-signature"; * Configure all routes for express app * @param app the express app */ -module.exports = (app) => { +function configureRoutes(app) { // Load all routes _.each(routes, (verbs, path) => { _.each(verbs, (def, verb) => { @@ -82,17 +161,19 @@ module.exports = (app) => { }); actions.push((req, res, next) => { - if (req.authUser.isMachine) { + if (isM2MAuthUser(req.authUser)) { // M2M - if (!req.authUser.scopes || !helper.checkIfExists(def.scopes, req.authUser.scopes)) { + if (!normalizeM2MAuthUser(def, req.authUser)) { logger.warn( `[${getSignature(req)}] Machine token scope mismatch. required=${safeInspect( def.scopes - )} provided=${safeInspect(req.authUser.scopes)}` + )} provided=${safeInspect(getAuthUserScopes(req.authUser))}` ); - next(new errors.ForbiddenError(`You are not allowed to perform this action, because the scopes are incorrect. \ + next( + new errors.ForbiddenError(`You are not allowed to perform this action, because the scopes are incorrect. \ Required scopes: ${JSON.stringify(def.scopes)} \ - Provided scopes: ${JSON.stringify(req.authUser.scopes)}`)); + Provided scopes: ${JSON.stringify(getAuthUserScopes(req.authUser))}`) + ); } else { req.authUser.handle = config.M2M_AUDIT_HANDLE; req.authUser.userId = config.M2M_AUDIT_USERID; @@ -120,9 +201,11 @@ module.exports = (app) => { def.access )} provided=${safeInspect(req.authUser.roles)}` ); - next(new errors.ForbiddenError(`You are not allowed to perform this action, because the roles are incorrect. \ + next( + new errors.ForbiddenError(`You are not allowed to perform this action, because the roles are incorrect. \ Required roles: ${JSON.stringify(def.access)} \ - Provided roles: ${JSON.stringify(req.authUser.roles)}`)); + Provided roles: ${JSON.stringify(req.authUser.roles)}`) + ); } else { // user token is used in create/update challenge to ensure user can create/update challenge under specific project req.userToken = req.headers.authorization.split(" ")[1]; @@ -135,8 +218,12 @@ module.exports = (app) => { } } else { logger.warn(`[${getSignature(req)}] Authenticated user missing roles`); - next(new errors.ForbiddenError("You are not authorized to perform this action, \ - because no roles were provided")); + next( + new errors.ForbiddenError( + "You are not authorized to perform this action, \ + because no roles were provided" + ) + ); } } }); @@ -178,12 +265,8 @@ module.exports = (app) => { if (!req.authUser) { logger.info(`[${getSignature(req)}] Public route: no authUser context`); next(); - } else if (req.authUser.isMachine) { - if ( - !def.scopes || - !req.authUser.scopes || - !helper.checkIfExists(def.scopes, req.authUser.scopes) - ) { + } else if (isM2MAuthUser(req.authUser)) { + if (!normalizeM2MAuthUser(def, req.authUser)) { logger.info( `[${getSignature(req)}] Public route: preserving machine token whitelist bypass despite scope mismatch` ); @@ -242,4 +325,13 @@ module.exports = (app) => { }); } }); +} + +module.exports = configureRoutes; +module.exports.__testables = { + getAuthUserScopes, + hasInteractiveRoles, + hasRequiredM2MScopes, + isM2MAuthUser, + normalizeM2MAuthUser, }; diff --git a/test/unit/ChallengeRoutes.test.js b/test/unit/ChallengeRoutes.test.js index 81dedb7..53ca5a6 100644 --- a/test/unit/ChallengeRoutes.test.js +++ b/test/unit/ChallengeRoutes.test.js @@ -1,12 +1,12 @@ const { expect } = require("chai"); const constants = require("../../app-constants"); +const appRoutes = require("../../app-routes"); const routes = require("../../src/routes"); describe("Challenge route access", () => { - const talentManagerRoles = [ - constants.UserRoles.TalentManager, - ]; + const { __testables } = appRoutes; + const talentManagerRoles = [constants.UserRoles.TalentManager]; const challengeEditRoutes = [ ["/challenges", "post"], @@ -32,4 +32,30 @@ describe("Challenge route access", () => { }); }); }); + + it("normalizes valid no-role scoped tokens as M2M callers", () => { + const authUser = { + scope: "read:challenges", + }; + + expect(__testables.isM2MAuthUser(authUser)).to.equal(true); + expect( + __testables.normalizeM2MAuthUser(routes["/challenges/:challengeId"].get, authUser), + ).to.equal(true); + expect(authUser.isMachine).to.equal(true); + expect(authUser.scopes).to.deep.equal(["read:challenges"]); + }); + + it("does not normalize role-bearing users with scopes as M2M callers", () => { + const authUser = { + roles: [constants.UserRoles.User], + scope: "read:challenges", + }; + + expect(__testables.isM2MAuthUser(authUser)).to.equal(false); + expect( + __testables.normalizeM2MAuthUser(routes["/challenges/:challengeId"].get, authUser), + ).to.equal(false); + expect(authUser.isMachine).to.equal(undefined); + }); }); diff --git a/test/unit/helper-whitelist.test.js b/test/unit/helper-whitelist.test.js index b70365a..57aa818 100644 --- a/test/unit/helper-whitelist.test.js +++ b/test/unit/helper-whitelist.test.js @@ -22,4 +22,8 @@ describe("challenge whitelist helper", () => { }), ).to.equal(true); }); + + it("does not apply challenge group checks to M2M callers", async () => { + await helper.ensureAccessibleByGroupsAccess({ isMachine: true }, { groups: ["private-group"] }); + }); });