Merge pull request #128 from topcoder-platform/PM-5447

PM-5447: Validate phase shortening after schedule recalculation

Author: jmgasper

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<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.
  *
@@ -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"));
   }
 

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'