From f554bbe5ead6eb7b2c78255abfdc68897d073c4d Mon Sep 17 00:00:00 2001 From: Muhamad Fikri Alhawarizmi Date: Tue, 25 Jun 2019 16:56:32 +0700 Subject: [PATCH] improve milestone hooks and change `referenceId` in milestone to BIGINT --- src/models/milestone.js | 63 +++++++++++++++++++++------- src/models/statusHistory.js | 2 +- src/routes/milestones/create.spec.js | 12 ++++-- src/routes/milestones/delete.spec.js | 3 ++ src/routes/milestones/get.spec.js | 28 ++----------- src/routes/milestones/list.spec.js | 44 +++++++------------ src/routes/milestones/update.js | 2 +- src/routes/milestones/update.spec.js | 35 ++++------------ src/routes/timelines/create.spec.js | 3 +- src/routes/timelines/delete.spec.js | 2 + src/routes/timelines/get.spec.js | 27 +----------- src/routes/timelines/list.spec.js | 36 ++++------------ src/routes/timelines/update.spec.js | 26 +----------- 13 files changed, 100 insertions(+), 183 deletions(-) diff --git a/src/models/milestone.js b/src/models/milestone.js index 746a437e..54072443 100644 --- a/src/models/milestone.js +++ b/src/models/milestone.js @@ -5,22 +5,42 @@ import models from '../models'; /** * Populate and map milestone model with statusHistory + * NOTE that this function mutates milestone * @param {Object} milestone the milestone * @returns {Promise} promise */ const mapWithStatusHistory = async (milestone) => { - try { - const statusHistory = await models.StatusHistory.findAll({ - where: { - referenceId: milestone.id.toString(), - reference: 'milestone', - }, - order: [['createdAt', 'desc']], - raw: true, - }); - return _.merge(milestone, { dataValues: { statusHistory } }); - } catch (err) { - return _.merge(milestone, { dataValues: { statusHistory: [] } }); + if (Array.isArray(milestone)) { + try { + const allStatusHistory = await models.StatusHistory.findAll({ + where: { + referenceId: { $in: milestone.map(m => m.dataValues.id) }, + 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 } }); + }); + } catch (err) { + return milestone.map((m, index) => _.merge(milestone[index], { dataValues: { statusHistory: [] } })); + } + } else { + try { + const statusHistory = await models.StatusHistory.findAll({ + where: { + referenceId: milestone.dataValues.id, + reference: 'milestone', + }, + order: [['createdAt', 'desc']], + raw: true, + }); + return _.merge(milestone, { dataValues: { statusHistory } }); + } catch (err) { + return _.merge(milestone, { dataValues: { statusHistory: [] } }); + } } }; @@ -117,6 +137,21 @@ module.exports = (sequelize, DataTypes) => { transaction: options.transaction, }).then(() => mapWithStatusHistory(milestone)), + afterBulkCreate: (milestones, options) => { + const listStatusHistory = milestones.map(({ dataValues }) => ({ + reference: 'milestone', + referenceId: dataValues.id, + status: dataValues.status, + comment: null, + createdBy: dataValues.createdBy, + updatedBy: dataValues.updatedBy, + })); + + return models.StatusHistory.bulkCreate(listStatusHistory, { + transaction: options.transaction, + }).then(() => mapWithStatusHistory(milestones)); + }, + afterUpdate: (milestone, options) => { if (milestone.changed().includes('status')) { return models.StatusHistory.create({ @@ -135,10 +170,6 @@ module.exports = (sequelize, DataTypes) => { afterFind: (milestone) => { if (!milestone) return Promise.resolve(); - - if (Array.isArray(milestone)) { - return Promise.all(milestone.map(mapWithStatusHistory)); - } return mapWithStatusHistory(milestone); }, }, diff --git a/src/models/statusHistory.js b/src/models/statusHistory.js index e9970b8b..b73d3650 100644 --- a/src/models/statusHistory.js +++ b/src/models/statusHistory.js @@ -7,7 +7,7 @@ module.exports = function defineStatusHistory(sequelize, DataTypes) { const StatusHistory = sequelize.define('StatusHistory', { id: { type: DataTypes.BIGINT, primaryKey: true, autoIncrement: true }, reference: { type: DataTypes.STRING, allowNull: false }, - referenceId: { type: DataTypes.STRING, allowNull: false }, + referenceId: { type: DataTypes.BIGINT, allowNull: false }, status: { type: DataTypes.STRING, allowNull: false, diff --git a/src/routes/milestones/create.spec.js b/src/routes/milestones/create.spec.js index b5eed6a3..ec04afda 100644 --- a/src/routes/milestones/create.spec.js +++ b/src/routes/milestones/create.spec.js @@ -142,6 +142,7 @@ describe('CREATE milestone', () => { // Create milestones models.Milestone.bulkCreate([ { + id: 11, timelineId: 1, name: 'milestone 1', duration: 2, @@ -164,6 +165,7 @@ describe('CREATE milestone', () => { updatedBy: 2, }, { + id: 12, timelineId: 1, name: 'milestone 2', duration: 3, @@ -179,6 +181,7 @@ describe('CREATE milestone', () => { updatedBy: 3, }, { + id: 13, timelineId: 1, name: 'milestone 3', duration: 4, @@ -526,9 +529,10 @@ describe('CREATE milestone', () => { // validate statusHistory should.exist(resJson.statusHistory); resJson.statusHistory.should.be.an('array'); + resJson.statusHistory.length.should.be.eql(1); resJson.statusHistory.forEach((statusHistory) => { statusHistory.reference.should.be.eql('milestone'); - statusHistory.referenceId.should.be.eql(`${resJson.id}`); + statusHistory.referenceId.should.be.eql(resJson.id); }); // eslint-disable-next-line no-unused-expressions @@ -538,11 +542,11 @@ describe('CREATE milestone', () => { models.Milestone.findAll({ where: { timelineId: 1 } }) .then((milestones) => { _.each(milestones, (milestone) => { - if (milestone.id === 1) { + if (milestone.id === 11) { milestone.order.should.be.eql(1); - } else if (milestone.id === 2) { + } else if (milestone.id === 12) { milestone.order.should.be.eql(2 + 1); - } else if (milestone.id === 3) { + } else if (milestone.id === 13) { milestone.order.should.be.eql(3 + 1); } }); diff --git a/src/routes/milestones/delete.spec.js b/src/routes/milestones/delete.spec.js index c756b7b0..5b237d1d 100644 --- a/src/routes/milestones/delete.spec.js +++ b/src/routes/milestones/delete.spec.js @@ -160,6 +160,7 @@ describe('DELETE milestone', () => { // Create milestones models.Milestone.bulkCreate([ { + id: 1, timelineId: 1, name: 'milestone 1', duration: 2, @@ -182,6 +183,7 @@ describe('DELETE milestone', () => { updatedBy: 2, }, { + id: 2, timelineId: 1, name: 'milestone 2', duration: 3, @@ -197,6 +199,7 @@ describe('DELETE milestone', () => { updatedBy: 3, }, { + id: 3, timelineId: 1, name: 'milestone 3', duration: 4, diff --git a/src/routes/milestones/get.spec.js b/src/routes/milestones/get.spec.js index b9a8db58..5ae3adf9 100644 --- a/src/routes/milestones/get.spec.js +++ b/src/routes/milestones/get.spec.js @@ -190,30 +190,7 @@ describe('GET milestone', () => { deletedAt: '2018-05-04T00:00:00.000Z', }, ]) - .then(() => - models.StatusHistory.bulkCreate([ - { - reference: 'milestone', - referenceId: '1', - status: 'active', - comment: 'comment', - createdBy: 1, - createdAt: '2018-05-15T00:00:00Z', - updatedBy: 1, - updatedAt: '2018-05-15T00:00:00Z', - }, - { - reference: 'milestone', - referenceId: '1', - status: 'active', - comment: 'comment', - createdBy: 1, - createdAt: '2018-05-15T00:00:00Z', - updatedBy: 1, - updatedAt: '2018-05-15T00:00:00Z', - }, - ]) - .then(() => done())); + .then(() => done()); }); }); }); @@ -330,9 +307,10 @@ describe('GET milestone', () => { // validate statusHistory should.exist(resJson.statusHistory); resJson.statusHistory.should.be.an('array'); + resJson.statusHistory.length.should.be.eql(1); resJson.statusHistory.forEach((statusHistory) => { statusHistory.reference.should.be.eql('milestone'); - statusHistory.referenceId.should.be.eql(`${resJson.id}`); + statusHistory.referenceId.should.be.eql(resJson.id); }); done(); diff --git a/src/routes/milestones/list.spec.js b/src/routes/milestones/list.spec.js index a4336ef4..48d631f3 100644 --- a/src/routes/milestones/list.spec.js +++ b/src/routes/milestones/list.spec.js @@ -51,6 +51,7 @@ const milestones = [ detail2: [1, 2, 3], }, order: 1, + hidden: false, plannedText: 'plannedText 1', activeText: 'activeText 1', completedText: 'completedText 1', @@ -59,16 +60,6 @@ const milestones = [ updatedBy: 2, createdAt: '2018-05-11T00:00:00.000Z', updatedAt: '2018-05-11T00:00:00.000Z', - statusHistory: [{ - reference: 'milestone', - referenceId: '1', - status: 'active', - comment: 'comment', - createdBy: 1, - createdAt: '2018-05-15T00:00:00Z', - updatedBy: 1, - updatedAt: '2018-05-15T00:00:00Z', - }], }, { id: 2, @@ -79,6 +70,7 @@ const milestones = [ status: 'open', type: 'type2', order: 2, + hidden: false, plannedText: 'plannedText 2', activeText: 'activeText 2', completedText: 'completedText 2', @@ -87,16 +79,6 @@ const milestones = [ updatedBy: 3, createdAt: '2018-05-11T00:00:00.000Z', updatedAt: '2018-05-11T00:00:00.000Z', - statusHistory: [{ - reference: 'milestone', - referenceId: '2', - status: 'active', - comment: 'comment', - createdBy: 1, - createdAt: '2018-05-15T00:00:00Z', - updatedBy: 1, - updatedAt: '2018-05-15T00:00:00Z', - }], }, ]; @@ -186,13 +168,10 @@ describe('LIST timelines', () => { .then(() => // Create timelines and milestones models.Timeline.bulkCreate(timelines) - .then(() => { - const mappedMilstones = milestones.map(milestone => _.omit(milestone, ['statusHistory'])); - return models.Milestone.bulkCreate(mappedMilstones); - })) - .then(() => { + .then(() => models.Milestone.bulkCreate(milestones))) + .then((mappedMilestones) => { // Index to ES - timelines[0].milestones = milestones; + timelines[0].milestones = mappedMilestones.map(({ dataValues }) => dataValues); timelines[0].projectId = 1; return server.services.es.index({ index: ES_TIMELINE_INDEX, @@ -268,12 +247,15 @@ describe('LIST timelines', () => { resJson.forEach((milestone, index) => { milestone.statusHistory.should.be.an('array'); + milestone.statusHistory.length.should.be.eql(1); milestone.statusHistory.forEach((statusHistory) => { statusHistory.reference.should.be.eql('milestone'); - statusHistory.referenceId.should.be.eql(`${milestone.id}`); + statusHistory.referenceId.should.be.eql(milestone.id); }); - milestone.should.be.eql(milestones[index]); + const m = _.omit(milestone, ['statusHistory']); + + m.should.be.eql(milestones[index]); }); done(); @@ -349,8 +331,10 @@ describe('LIST timelines', () => { const resJson = res.body.result.content; resJson.should.have.length(2); - resJson[0].should.be.eql(milestones[1]); - resJson[1].should.be.eql(milestones[0]); + const m1 = _.omit(resJson[0], ['statusHistory']); + const m2 = _.omit(resJson[1], ['statusHistory']); + m1.should.be.eql(milestones[1]); + m2.should.be.eql(milestones[0]); done(); }); diff --git a/src/routes/milestones/update.js b/src/routes/milestones/update.js index 59a8a3bf..f3cd83cb 100644 --- a/src/routes/milestones/update.js +++ b/src/routes/milestones/update.js @@ -170,7 +170,7 @@ module.exports = [ return Promise.reject(apiErr); } const statusHistory = await models.StatusHistory.findAll({ - where: { referenceId: milestone.id.toString() }, + where: { referenceId: milestone.id }, order: [['createdAt', 'desc']], attributes: ['status'], limit: 2, diff --git a/src/routes/milestones/update.spec.js b/src/routes/milestones/update.spec.js index 50c61591..5db038df 100644 --- a/src/routes/milestones/update.spec.js +++ b/src/routes/milestones/update.spec.js @@ -252,29 +252,7 @@ describe('UPDATE Milestone', () => { updatedAt: '2018-05-11T00:00:00.000Z', }, ]))) - .then(() => models.StatusHistory.bulkCreate([ - { - reference: 'milestone', - referenceId: '1', - status: 'active', - comment: 'comment', - createdBy: 1, - createdAt: '2018-05-15T00:00:00Z', - updatedBy: 1, - updatedAt: '2018-05-15T00:00:00Z', - }, - { - reference: 'milestone', - referenceId: '2', - status: 'active', - comment: 'comment', - createdBy: 1, - createdAt: '2018-05-15T00:00:00Z', - updatedBy: 1, - updatedAt: '2018-05-15T00:00:00Z', - }, - ])) - .then(() => done()); + .then(() => done()); }); }); }); @@ -549,9 +527,10 @@ describe('UPDATE Milestone', () => { // validate statusHistory should.exist(resJson.statusHistory); resJson.statusHistory.should.be.an('array'); + resJson.statusHistory.length.should.be.eql(2); resJson.statusHistory.forEach((statusHistory) => { statusHistory.reference.should.be.eql('milestone'); - statusHistory.referenceId.should.be.eql(`${resJson.id}`); + statusHistory.referenceId.should.be.eql(resJson.id); }); // eslint-disable-next-line no-unused-expressions @@ -1160,7 +1139,7 @@ describe('UPDATE Milestone', () => { return models.StatusHistory.findAll({ where: { reference: 'milestone', - referenceId: milestone.id.toString(), + referenceId: milestone.id, status: milestone.status, comment: 'milestone paused', }, @@ -1222,7 +1201,7 @@ describe('UPDATE Milestone', () => { ]).then(() => models.StatusHistory.bulkCreate([ { reference: 'milestone', - referenceId: '7', + referenceId: 7, status: 'active', comment: 'comment', createdBy: 1, @@ -1232,7 +1211,7 @@ describe('UPDATE Milestone', () => { }, { reference: 'milestone', - referenceId: '7', + referenceId: 7, status: 'paused', comment: 'comment', createdBy: 1, @@ -1258,7 +1237,7 @@ describe('UPDATE Milestone', () => { return models.StatusHistory.findAll({ where: { reference: 'milestone', - referenceId: milestone.id.toString(), + referenceId: milestone.id, status: 'active', comment: 'new comment', }, diff --git a/src/routes/timelines/create.spec.js b/src/routes/timelines/create.spec.js index 9ee655c2..c6ab1bc8 100644 --- a/src/routes/timelines/create.spec.js +++ b/src/routes/timelines/create.spec.js @@ -533,9 +533,10 @@ describe('CREATE timeline', () => { // validate statusHistory should.exist(milestone.statusHistory); milestone.statusHistory.should.be.an('array'); + milestone.statusHistory.length.should.be.eql(1); milestone.statusHistory.forEach((statusHistory) => { statusHistory.reference.should.be.eql('milestone'); - statusHistory.referenceId.should.be.eql(`${milestone.id}`); + statusHistory.referenceId.should.be.eql(milestone.id); }); }); diff --git a/src/routes/timelines/delete.spec.js b/src/routes/timelines/delete.spec.js index 68c505b2..f609397e 100644 --- a/src/routes/timelines/delete.spec.js +++ b/src/routes/timelines/delete.spec.js @@ -159,6 +159,7 @@ describe('DELETE timeline', () => { // Create milestones models.Milestone.bulkCreate([ { + id: 1, timelineId: 1, name: 'milestone 1', duration: 2, @@ -181,6 +182,7 @@ describe('DELETE timeline', () => { updatedBy: 2, }, { + id: 2, timelineId: 1, name: 'milestone 2', duration: 2, diff --git a/src/routes/timelines/get.spec.js b/src/routes/timelines/get.spec.js index efb7953f..da22d117 100644 --- a/src/routes/timelines/get.spec.js +++ b/src/routes/timelines/get.spec.js @@ -58,29 +58,6 @@ const milestones = [ }, ]; -const statusHistories = [ - { - reference: 'milestone', - referenceId: '1', - status: 'active', - comment: 'comment', - createdBy: 1, - createdAt: '2018-05-15T00:00:00Z', - updatedBy: 1, - updatedAt: '2018-05-15T00:00:00Z', - }, - { - reference: 'milestone', - referenceId: '2', - status: 'active', - comment: 'comment', - createdBy: 1, - createdAt: '2018-05-15T00:00:00Z', - updatedBy: 1, - updatedAt: '2018-05-15T00:00:00Z', - }, -]; - describe('GET timeline', () => { before((done) => { testUtil.clearDb() @@ -200,7 +177,6 @@ describe('GET timeline', () => { }, ])) .then(() => models.Milestone.bulkCreate(milestones)) - .then(() => models.StatusHistory.bulkCreate(statusHistories)) .then(() => done()); }); }); @@ -290,9 +266,10 @@ describe('GET timeline', () => { // validate statusHistory should.exist(milestone.statusHistory); milestone.statusHistory.should.be.an('array'); + milestone.statusHistory.length.should.be.eql(1); milestone.statusHistory.forEach((statusHistory) => { statusHistory.reference.should.be.eql('milestone'); - statusHistory.referenceId.should.be.eql(`${milestone.id}`); + statusHistory.referenceId.should.be.eql(milestone.id); }); }); diff --git a/src/routes/timelines/list.spec.js b/src/routes/timelines/list.spec.js index 5931933a..4f68c843 100644 --- a/src/routes/timelines/list.spec.js +++ b/src/routes/timelines/list.spec.js @@ -75,18 +75,6 @@ const milestones = [ updatedBy: 2, createdAt: '2018-05-11T00:00:00.000Z', updatedAt: '2018-05-11T00:00:00.000Z', - statusHistory: [ - { - reference: 'milestone', - referenceId: '1', - status: 'active', - comment: 'comment', - createdBy: 1, - createdAt: '2018-05-15T00:00:00Z', - updatedBy: 1, - updatedAt: '2018-05-15T00:00:00Z', - }, - ], }, { id: 2, @@ -105,18 +93,6 @@ const milestones = [ updatedBy: 3, createdAt: '2018-05-11T00:00:00.000Z', updatedAt: '2018-05-11T00:00:00.000Z', - statusHistory: [ - { - reference: 'milestone', - referenceId: '2', - status: 'active', - comment: 'comment', - createdBy: 1, - createdAt: '2018-05-15T00:00:00Z', - updatedBy: 1, - updatedAt: '2018-05-15T00:00:00Z', - }, - ], }, ]; @@ -206,14 +182,17 @@ describe('LIST timelines', () => { ])) .then(() => // Create timelines - models.Timeline.bulkCreate(timelines, { returning: true })) - .then(createdTimelines => + Promise.all([ + models.Timeline.bulkCreate(timelines, { returning: true }), + models.Milestone.bulkCreate(milestones), + ])) + .then(([createdTimelines, mappedMilestones]) => // 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 = milestones; + timelineJson.milestones = mappedMilestones; } return server.services.es.index({ index: ES_TIMELINE_INDEX, @@ -304,9 +283,10 @@ describe('LIST timelines', () => { // validate statusHistory should.exist(milestone.statusHistory); milestone.statusHistory.should.be.an('array'); + milestone.statusHistory.length.should.be.eql(1); milestone.statusHistory.forEach((statusHistory) => { statusHistory.reference.should.be.eql('milestone'); - statusHistory.referenceId.should.be.eql(`${milestone.id}`); + statusHistory.referenceId.should.be.eql(milestone.id); }); }); diff --git a/src/routes/timelines/update.spec.js b/src/routes/timelines/update.spec.js index fdafa66c..d0726cf1 100644 --- a/src/routes/timelines/update.spec.js +++ b/src/routes/timelines/update.spec.js @@ -61,28 +61,6 @@ const milestones = [ updatedAt: '2018-05-11T00:00:00.000Z', }, ]; -const statusHistories = [ - { - reference: 'milestone', - referenceId: '1', - status: 'active', - comment: 'comment', - createdBy: 1, - createdAt: '2018-05-15T00:00:00Z', - updatedBy: 1, - updatedAt: '2018-05-15T00:00:00Z', - }, - { - reference: 'milestone', - referenceId: '2', - status: 'active', - comment: 'comment', - createdBy: 1, - createdAt: '2018-05-15T00:00:00Z', - updatedBy: 1, - updatedAt: '2018-05-15T00:00:00Z', - }, -]; describe('UPDATE timeline', () => { beforeEach((done) => { @@ -203,7 +181,6 @@ describe('UPDATE timeline', () => { }, ])) .then(() => models.Milestone.bulkCreate(milestones)) - .then(() => models.StatusHistory.bulkCreate(statusHistories)) .then(() => done()); }); }); @@ -521,9 +498,10 @@ describe('UPDATE timeline', () => { // validate statusHistory should.exist(milestone.statusHistory); milestone.statusHistory.should.be.an('array'); + milestone.statusHistory.length.should.be.eql(1); milestone.statusHistory.forEach((statusHistory) => { statusHistory.reference.should.be.eql('milestone'); - statusHistory.referenceId.should.be.eql(`${milestone.id}`); + statusHistory.referenceId.should.be.eql(milestone.id); }); });