fixed some cases for broken status history populating

fixed and improved some related unit tests

Author: maxceem

src/models/milestone.js

@@ -9,35 +9,44 @@ import { STATUS_HISTORY_REFERENCES } from '../constants';
  * NOTE that this function mutates milestone
  *
  * @param {Array|Object} milestone one milestone or list of milestones
+ * @param {Object}       options   options which has been used to call main method
  *
  * @returns {Promise} promise
  */
-const populateWithStatusHistory = async (milestone) => {
+const populateWithStatusHistory = async (milestone, options) => {
+  // depend on this option `milestone` is a sequlize ORM object or plain JS object
+  const isRaw = !!_.get(options, 'raw');
+  const getMilestoneId = m => (
+    isRaw ? m.id : m.dataValues.id
+  );
+  const formatMilestone = statusHistory => (
+    isRaw ? { statusHistory } : { dataValues: { statusHistory } }
+  );
   if (Array.isArray(milestone)) {
     const allStatusHistory = await models.StatusHistory.findAll({
       where: {
-        referenceId: { $in: milestone.map(m => m.dataValues.id) },
+        referenceId: { $in: milestone.map(getMilestoneId) },
         reference: 'milestone',
       },
       order: [['createdAt', 'desc']],
       raw: true,
     });
 
     return milestone.map((m, index) => {
-      const statusHistory = allStatusHistory.filter(s => s.referenceId === m.dataValues.id);
-      return _.merge(milestone[index], { dataValues: { statusHistory } });
+      const statusHistory = _.filter(allStatusHistory, { referenceId: getMilestoneId(m) });
+      return _.merge(milestone[index], formatMilestone(statusHistory));
     });
   }
 
   const statusHistory = await models.StatusHistory.findAll({
     where: {
-      referenceId: milestone.dataValues.id,
+      referenceId: getMilestoneId(milestone),
       reference: 'milestone',
     },
     order: [['createdAt', 'desc']],
     raw: true,
   });
-  return _.merge(milestone, { dataValues: { statusHistory } });
+  return _.merge(milestone, formatMilestone(statusHistory));
 };
 
 /**
@@ -131,7 +140,7 @@ module.exports = (sequelize, DataTypes) => {
         updatedBy: milestone.updatedBy,
       }, {
         transaction: options.transaction,
-      }).then(() => populateWithStatusHistory(milestone)),
+      }).then(() => populateWithStatusHistory(milestone, options)),
 
       afterBulkCreate: (milestones, options) => {
         const listStatusHistory = milestones.map(({ dataValues }) => ({
@@ -145,7 +154,7 @@ module.exports = (sequelize, DataTypes) => {
 
         return models.StatusHistory.bulkCreate(listStatusHistory, {
           transaction: options.transaction,
-        }).then(() => populateWithStatusHistory(milestones));
+        }).then(() => populateWithStatusHistory(milestones, options));
       },
 
       afterUpdate: (milestone, options) => {
@@ -161,12 +170,12 @@ module.exports = (sequelize, DataTypes) => {
             transaction: options.transaction,
           }).then(() => populateWithStatusHistory(milestone));
         }
-        return populateWithStatusHistory(milestone);
+        return populateWithStatusHistory(milestone, options);
       },
 
-      afterFind: (milestone) => {
+      afterFind: (milestone, options) => {
         if (!milestone) return Promise.resolve();
-        return populateWithStatusHistory(milestone);
+        return populateWithStatusHistory(milestone, options);
       },
     },
   });

src/routes/milestones/list.spec.js

@@ -82,7 +82,7 @@ const milestones = [
   },
 ];
 
-describe('LIST timelines', () => {
+describe('LIST milestones', () => {
   before(function beforeHook(done) {
     this.timeout(10000);
     testUtil.clearDb()
@@ -165,13 +165,12 @@ describe('LIST timelines', () => {
                   updatedBy: 2,
                 },
               ]))
-              .then(() =>
-                // Create timelines and milestones
-                models.Timeline.bulkCreate(timelines)
-                  .then(() => models.Milestone.bulkCreate(milestones)))
-              .then((mappedMilestones) => {
+              // Create timelines and milestones
+              .then(() => models.Timeline.bulkCreate(timelines))
+              .then(() => models.Milestone.bulkCreate(milestones))
+              .then((createdMilestones) => {
                 // Index to ES
-                timelines[0].milestones = mappedMilestones.map(({ dataValues }) => dataValues);
+                timelines[0].milestones = _.map(createdMilestones, cm => _.omit(cm.toJSON(), 'deletedAt', 'deletedBy'));
                 timelines[0].projectId = 1;
                 return server.services.es.index({
                   index: ES_TIMELINE_INDEX,

src/routes/milestones/update.js

@@ -171,8 +171,8 @@ module.exports = [
             }
             const statusHistory = await models.StatusHistory.findAll({
               where: { referenceId: milestone.id },
-              order: [['createdAt', 'desc']],
-              attributes: ['status'],
+              order: [['createdAt', 'desc'], ['id', 'desc']],
+              attributes: ['status', 'id'],
               limit: 2,
               raw: true,
             });

src/routes/milestones/update.spec.js

@@ -1178,8 +1178,8 @@ describe('UPDATE Milestone', () => {
           duration: 2,
           startDate: '2018-05-13T00:00:00.000Z',
           endDate: '2018-05-14T00:00:00.000Z',
-          completionDate: '2018-05-15T00:00:00.000Z',
-          status: 'paused',
+          completionDate: '2018-05-16T00:00:00.000Z',
+          status: 'active',
           type: 'type1',
           details: {
             detail1: {
@@ -1198,28 +1198,10 @@ describe('UPDATE Milestone', () => {
           createdAt: '2018-05-11T00:00:00.000Z',
           updatedAt: '2018-05-11T00:00:00.000Z',
         },
-      ]).then(() => models.StatusHistory.bulkCreate([
-        {
-          reference: 'milestone',
-          referenceId: 7,
-          status: 'active',
-          comment: 'comment',
-          createdBy: 1,
-          createdAt: '2018-05-15T00:00:00Z',
-          updatedBy: 1,
-          updatedAt: '2018-05-15T00:00:00Z',
-        },
-        {
-          reference: 'milestone',
-          referenceId: 7,
-          status: 'paused',
-          comment: 'comment',
-          createdBy: 1,
-          createdAt: '2018-05-16T00:00:00Z',
-          updatedBy: 1,
-          updatedAt: '2018-05-16T00:00:00Z',
-        },
-      ]).then(() => {
+      ]).then(() => models.Milestone.findById(7)
+        // pause milestone before resume
+        .then(milestone => milestone.update(_.assign({}, milestone.toJSON(), { status: 'paused' }))),
+      ).then(() => {
         request(server)
         .patch('/v4/timelines/1/milestones/7')
         .set({
@@ -1245,11 +1227,11 @@ describe('UPDATE Milestone', () => {
               }).then((statusHistories) => {
                 statusHistories.length.should.be.eql(1);
                 done();
-              });
-            });
+              }).catch(done);
+            }).catch(done);
           }
         });
-      }));
+      });
     });
 
     describe('Bus api', () => {

src/routes/timelines/list.spec.js

@@ -182,25 +182,31 @@ describe('LIST timelines', () => {
                ]))
               .then(() =>
                 // Create timelines
-                 Promise.all([
-                   models.Timeline.bulkCreate(timelines, { returning: true }),
-                   models.Milestone.bulkCreate(milestones),
-                 ]))
-              .then(([createdTimelines, mappedMilestones]) =>
+                models.Timeline.bulkCreate(timelines, { returning: true })
+                  .then(createdTimelines => (
+                    // create milestones after timelines
+                    models.Milestone.bulkCreate(milestones))
+                      .then(createdMilestones => [createdTimelines, createdMilestones]),
+                  ),
+              ).then(([createdTimelines, createdMilestones]) =>
                 // Index to ES
-                 Promise.all(_.map(createdTimelines, (createdTimeline) => {
-                   const timelineJson = _.omit(createdTimeline.toJSON(), 'deletedAt', 'deletedBy');
-                   timelineJson.projectId = createdTimeline.id !== 3 ? 1 : 2;
-                   if (timelineJson.id === 1) {
-                     timelineJson.milestones = mappedMilestones;
-                   }
-                   return server.services.es.index({
-                     index: ES_TIMELINE_INDEX,
-                     type: ES_TIMELINE_TYPE,
-                     id: timelineJson.id,
-                     body: timelineJson,
-                   });
-                 }))
+                Promise.all(_.map(createdTimelines, (createdTimeline) => {
+                  const timelineJson = _.omit(createdTimeline.toJSON(), 'deletedAt', 'deletedBy');
+                  timelineJson.projectId = createdTimeline.id !== 3 ? 1 : 2;
+                  if (timelineJson.id === 1) {
+                    timelineJson.milestones = _.map(
+                      createdMilestones,
+                      cm => _.omit(cm.toJSON(), 'deletedAt', 'deletedBy'),
+                    );
+                  }
+
+                  return server.services.es.index({
+                    index: ES_TIMELINE_INDEX,
+                    type: ES_TIMELINE_TYPE,
+                    id: timelineJson.id,
+                    body: timelineJson,
+                  });
+                }))
                   .then(() => {
                     // sleep for some time, let elasticsearch indices be settled
                     sleep.sleep(5);