PM-6007 - fix phase ordering when creating challenge

src/common/phase-helper.ts

@@ -272,6 +272,57 @@ function findPhaseUpdate(newPhases, phase) {
   return _.find(newPhases, (p) => p.phaseId === phase.phaseId);
 }
 
+/**
+ * Order timeline template phases so every phase follows its predecessor.
+ *
+ * TimelineTemplatePhase rows carry no ordering column, so the database returns them
+ * in unspecified physical order. Scheduling resolves predecessor dates in a single
+ * pass, which is only correct when a predecessor is processed before its dependents.
+ *
+ * @param {Array<Object>} phases template phases, each with phaseId and optional predecessor
+ * @returns {Array<Object>} phases in dependency order; phases unreachable from a root
+ *   (dangling predecessor or cycle) are appended last, keeping their relative order
+ */
+function orderPhasesByPredecessorChain(phases) {
+  if (!Array.isArray(phases)) {
+    return [];
+  }
+
+  const knownPhaseIds = new Set(_.map(phases, "phaseId"));
+  const childrenOf = new Map();
+  _.each(phases, (phase) => {
+    if (_.isNil(phase.predecessor) || !knownPhaseIds.has(phase.predecessor)) {
+      return;
+    }
+    const siblings = childrenOf.get(phase.predecessor) || [];
+    siblings.push(phase);
+    childrenOf.set(phase.predecessor, siblings);
+  });
+
+  const ordered = [];
+  const visited = new Set();
+  const queue = _.filter(phases, (phase) => _.isNil(phase.predecessor));
+  while (queue.length > 0) {
+    const phase = queue.shift();
+    if (visited.has(phase)) {
+      continue;
+    }
+    visited.add(phase);
+    ordered.push(phase);
+    queue.push(...(childrenOf.get(phase.phaseId) || []));
+  }
+
+  // Phases with a dangling predecessor, or caught in a cycle, are never reachable from
+  // a root. Keep them so the caller still emits them, unscheduled, as it did before.
+  _.each(phases, (phase) => {
+    if (!visited.has(phase)) {
+      ordered.push(phase);
+    }
+  });
+
+  return ordered;
+}
+
 class ChallengePhaseHelper {
   phaseDefinitionMap: any = {};
   timelineTemplateMap: any = {};
@@ -281,10 +332,13 @@ class ChallengePhaseHelper {
       throw new errors.BadRequestError(`Invalid timeline template ID: ${timelineTemplateId}`);
     }
     const { timelineTempate } = await this.getTemplateAndTemplateMap(timelineTemplateId);
-    console.log("Selected timeline template", JSON.stringify(timelineTempate));
     const { phaseDefinitionMap } = await this.getPhaseDefinitionsAndMap();
+    // The template rows have no stored order, so walk the predecessor chain instead of
+    // trusting the order the database happened to return them in.
+    const orderedTemplate = orderPhasesByPredecessorChain(timelineTempate);
+    console.log("Selected timeline template", JSON.stringify(orderedTemplate));
     let fixedStartDate = undefined;
-    const finalPhases = _.map(timelineTempate, (phaseFromTemplate) => {
+    const finalPhases = _.map(orderedTemplate, (phaseFromTemplate) => {
       const phaseDefinition = phaseDefinitionMap.get(phaseFromTemplate.phaseId);
       const phaseFromInput = _.find(phases, (p) => p.phaseId === phaseFromTemplate.phaseId);
       const phase = {
@@ -324,23 +378,29 @@ class ChallengePhaseHelper {
       return phase;
     });
     for (const phase of finalPhases) {
-      if (_.isUndefined(phase.predecessor)) {
+      if (_.isNil(phase.predecessor)) {
         continue;
       }
       const precedecessorPhase = _.find(finalPhases, {
         phaseId: phase.predecessor,
       });
-      if (!_.isNil(precedecessorPhase)) {
-        if (phase.name === "Iterative Review") {
-          phase.scheduledStartDate = precedecessorPhase.scheduledStartDate;
-        } else {
-          phase.scheduledStartDate = precedecessorPhase.scheduledEndDate;
-        }
-        phase.scheduledEndDate = moment(phase.scheduledStartDate)
-          .add(phase.duration, "seconds")
-          .toDate()
-          .toISOString();
+      if (_.isNil(precedecessorPhase)) {
+        continue;
+      }
+      const inheritedStartDate =
+        phase.name === "Iterative Review"
+          ? precedecessorPhase.scheduledStartDate
+          : precedecessorPhase.scheduledEndDate;
+      // An unresolved predecessor would make moment() fall back to the current time,
+      // scheduling this phase before the challenge even starts. Leave it unscheduled.
+      if (_.isNil(inheritedStartDate)) {
+        continue;
       }
+      phase.scheduledStartDate = inheritedStartDate;
+      phase.scheduledEndDate = moment(phase.scheduledStartDate)
+        .add(phase.duration, "seconds")
+        .toDate()
+        .toISOString();
     }
     return finalPhases;
   }

test/unit/phase-helper.test.js

@@ -817,4 +817,88 @@ describe('phase helper unit tests', () => {
 
     throw new Error('should not reach here')
   })
+  it('schedules created phases along the predecessor chain regardless of template row order', async () => {
+    const registrationPhaseId = 'registration-phase'
+    const submissionPhaseId = 'submission-phase'
+    const appealsPhaseId = 'appeals-phase'
+    const oneDay = 24 * 60 * 60
+    const startDate = '2026-09-01T00:00:00.000Z'
+
+    // The DB returns TimelineTemplatePhase rows in unspecified order, so hand them over
+    // reversed: Appeals arrives before the predecessors it depends on.
+    stubPhaseLookups(
+      [
+        { id: registrationPhaseId, name: 'Registration', description: 'Registration phase' },
+        { id: submissionPhaseId, name: 'Submission', description: 'Submission phase' },
+        { id: appealsPhaseId, name: 'Appeals', description: 'Appeals phase' }
+      ],
+      [
+        { phaseId: appealsPhaseId, predecessor: submissionPhaseId, defaultDuration: oneDay },
+        { phaseId: submissionPhaseId, predecessor: registrationPhaseId, defaultDuration: oneDay },
+        { phaseId: registrationPhaseId, defaultDuration: oneDay }
+      ]
+    )
+
+    const createdPhases = await phaseHelper.populatePhasesForChallengeCreation(
+      [],
+      startDate,
+      'timeline-template-id'
+    )
+
+    const byName = new Map(createdPhases.map((phase) => [phase.name, phase]))
+
+    byName.get('Registration').scheduledStartDate.should.equal(startDate)
+    byName.get('Registration').scheduledEndDate.should.equal('2026-09-02T00:00:00.000Z')
+    byName.get('Submission').scheduledStartDate.should.equal('2026-09-02T00:00:00.000Z')
+    byName.get('Submission').scheduledEndDate.should.equal('2026-09-03T00:00:00.000Z')
+    byName.get('Appeals').scheduledStartDate.should.equal('2026-09-03T00:00:00.000Z')
+    byName.get('Appeals').scheduledEndDate.should.equal('2026-09-04T00:00:00.000Z')
+
+    // Challenges are read back ordered by scheduledEndDate, so Appeals must land last.
+    const namesByEndDate = createdPhases
+      .slice()
+      .sort((a, b) => a.scheduledEndDate.localeCompare(b.scheduledEndDate))
+      .map((phase) => phase.name)
+
+    namesByEndDate.should.deep.equal(['Registration', 'Submission', 'Appeals'])
+  })
+
+  it('leaves created phases unscheduled when the predecessor chain has no resolvable root', async () => {
+    const registrationPhaseId = 'registration-phase'
+    const reviewPhaseId = 'review-phase'
+    const appealsPhaseId = 'appeals-phase'
+    const oneDay = 24 * 60 * 60
+    const startDate = '2026-09-01T00:00:00.000Z'
+
+    // Review points at a phase absent from the template, so neither Review nor the
+    // Appeals phase hanging off it can be scheduled.
+    stubPhaseLookups(
+      [
+        { id: registrationPhaseId, name: 'Registration', description: 'Registration phase' },
+        { id: reviewPhaseId, name: 'Review', description: 'Review phase' },
+        { id: appealsPhaseId, name: 'Appeals', description: 'Appeals phase' }
+      ],
+      [
+        { phaseId: appealsPhaseId, predecessor: reviewPhaseId, defaultDuration: oneDay },
+        { phaseId: reviewPhaseId, predecessor: 'phase-not-in-template', defaultDuration: oneDay },
+        { phaseId: registrationPhaseId, defaultDuration: oneDay }
+      ]
+    )
+
+    const createdPhases = await phaseHelper.populatePhasesForChallengeCreation(
+      [],
+      startDate,
+      'timeline-template-id'
+    )
+
+    createdPhases.should.have.lengthOf(3)
+    const byName = new Map(createdPhases.map((phase) => [phase.name, phase]))
+
+    byName.get('Registration').scheduledStartDate.should.equal(startDate)
+    // Previously these fell back to moment(undefined), scheduling them at "now".
+    for (const name of ['Review', 'Appeals']) {
+      chai.expect(byName.get(name).scheduledStartDate).to.be.undefined
+      chai.expect(byName.get(name).scheduledEndDate).to.be.undefined
+    }
+  })
 })