From 52ee6f6c55584ac5197cc1c3d41ce79304058c26 Mon Sep 17 00:00:00 2001 From: morehappiness Date: Sat, 28 Jul 2018 23:18:04 +0800 Subject: [PATCH] fix Issue #128 --- src/models/phaseProduct.js | 2 +- src/models/project.js | 2 + src/models/projectAttachment.js | 3 +- src/models/projectHistory.js | 2 +- src/models/projectMember.js | 1 + src/models/projectPhase.js | 2 +- src/routes/attachments/delete.js | 7 +- src/routes/attachments/delete.spec.js | 28 +++- src/routes/milestoneTemplates/delete.js | 37 ++-- src/routes/milestoneTemplates/delete.spec.js | 31 +++- src/routes/milestones/delete.spec.js | 167 +++++++++++-------- src/routes/phaseProducts/delete.js | 14 +- src/routes/phaseProducts/delete.spec.js | 31 +++- src/routes/phases/delete.js | 11 +- src/routes/phases/delete.spec.js | 29 +++- src/routes/productCategories/delete.js | 33 +--- src/routes/productCategories/delete.spec.js | 31 +++- src/routes/productTemplates/delete.js | 34 +--- src/routes/productTemplates/delete.spec.js | 31 +++- src/routes/projectMembers/delete.js | 5 +- src/routes/projectMembers/delete.spec.js | 42 +++-- src/routes/projectTemplates/delete.js | 34 +--- src/routes/projectTemplates/delete.spec.js | 31 +++- src/routes/projectTypes/delete.js | 33 +--- src/routes/projectTypes/delete.spec.js | 31 +++- src/routes/projects/delete.js | 48 +++--- src/routes/projects/delete.spec.js | 156 +++++++++-------- src/routes/timelines/delete.spec.js | 59 +++++-- 28 files changed, 574 insertions(+), 361 deletions(-) diff --git a/src/models/phaseProduct.js b/src/models/phaseProduct.js index 4ec1ea90..04ec131e 100644 --- a/src/models/phaseProduct.js +++ b/src/models/phaseProduct.js @@ -22,7 +22,7 @@ module.exports = function definePhaseProduct(sequelize, DataTypes) { updatedBy: { type: DataTypes.INTEGER, allowNull: false }, }, { tableName: 'phase_products', - paranoid: false, + paranoid: true, timestamps: true, updatedAt: 'updatedAt', createdAt: 'createdAt', diff --git a/src/models/project.js b/src/models/project.js index f103a1fa..6bb6b66f 100644 --- a/src/models/project.js +++ b/src/models/project.js @@ -38,11 +38,13 @@ module.exports = function defineProject(sequelize, DataTypes) { deletedAt: { type: DataTypes.DATE, allowNull: true }, createdAt: { type: DataTypes.DATE, defaultValue: DataTypes.NOW }, updatedAt: { type: DataTypes.DATE, defaultValue: DataTypes.NOW }, + deletedBy: DataTypes.BIGINT, createdBy: { type: DataTypes.INTEGER, allowNull: false }, updatedBy: { type: DataTypes.INTEGER, allowNull: false }, version: { type: DataTypes.STRING(3), allowNull: false, defaultValue: 'v3' }, }, { tableName: 'projects', + paranoid: true, timestamps: true, updatedAt: 'updatedAt', createdAt: 'createdAt', diff --git a/src/models/projectAttachment.js b/src/models/projectAttachment.js index 8fab5508..9e917b25 100644 --- a/src/models/projectAttachment.js +++ b/src/models/projectAttachment.js @@ -12,11 +12,12 @@ module.exports = function defineProjectAttachment(sequelize, DataTypes) { deletedAt: { type: DataTypes.DATE, allowNull: true }, createdAt: { type: DataTypes.DATE, defaultValue: DataTypes.NOW }, updatedAt: { type: DataTypes.DATE, defaultValue: DataTypes.NOW }, + deletedBy: DataTypes.BIGINT, createdBy: { type: DataTypes.INTEGER, allowNull: false }, updatedBy: { type: DataTypes.INTEGER, allowNull: false }, }, { tableName: 'project_attachments', - paranoid: false, + paranoid: true, timestamps: true, updatedAt: 'updatedAt', createdAt: 'createdAt', diff --git a/src/models/projectHistory.js b/src/models/projectHistory.js index 23bdbe93..af169583 100644 --- a/src/models/projectHistory.js +++ b/src/models/projectHistory.js @@ -11,7 +11,7 @@ module.exports = function defineProjectHistory(sequelize, DataTypes) { updatedBy: { type: DataTypes.INTEGER, allowNull: false }, }, { tableName: 'project_history', - paranoid: false, + paranoid: true, timestamps: true, updatedAt: 'updatedAt', createdAt: 'createdAt', diff --git a/src/models/projectMember.js b/src/models/projectMember.js index 2ab4c94b..1003c017 100644 --- a/src/models/projectMember.js +++ b/src/models/projectMember.js @@ -17,6 +17,7 @@ module.exports = function defineProjectMember(sequelize, DataTypes) { deletedAt: { type: DataTypes.DATE, allowNull: true }, createdAt: { type: DataTypes.DATE, defaultValue: DataTypes.NOW }, updatedAt: { type: DataTypes.DATE, defaultValue: DataTypes.NOW }, + deletedBy: DataTypes.BIGINT, createdBy: { type: DataTypes.INTEGER, allowNull: false }, updatedBy: { type: DataTypes.INTEGER, allowNull: false }, }, { diff --git a/src/models/projectPhase.js b/src/models/projectPhase.js index bcfe827a..75b8e3dd 100644 --- a/src/models/projectPhase.js +++ b/src/models/projectPhase.js @@ -23,7 +23,7 @@ module.exports = function defineProjectPhase(sequelize, DataTypes) { updatedBy: { type: DataTypes.INTEGER, allowNull: false }, }, { tableName: 'project_phases', - paranoid: false, + paranoid: true, timestamps: true, updatedAt: 'updatedAt', createdAt: 'createdAt', diff --git a/src/routes/attachments/delete.js b/src/routes/attachments/delete.js index 3401c06b..f5292b88 100644 --- a/src/routes/attachments/delete.js +++ b/src/routes/attachments/delete.js @@ -37,8 +37,9 @@ module.exports = [ return Promise.reject(err); } attachment = _attachment; - return _attachment.destroy(); - }) + return _attachment.update({ deletedBy: req.authUser.userId }) + .then(() => _attachment.destroy()); + })) .then((_attachment) => { if (process.env.NODE_ENV !== 'development') { return fileService.deleteFile(req, _attachment.filePath); @@ -56,6 +57,6 @@ module.exports = [ req.app.emit(EVENT.ROUTING_KEY.PROJECT_ATTACHMENT_REMOVED, { req, pattachment }); res.status(204).json({}); }) - .catch(err => next(err))); + .catch(err => next(err)); }, ]; diff --git a/src/routes/attachments/delete.spec.js b/src/routes/attachments/delete.spec.js index 87727788..76f223d1 100644 --- a/src/routes/attachments/delete.spec.js +++ b/src/routes/attachments/delete.spec.js @@ -7,6 +7,7 @@ import models from '../../models'; import util from '../../util'; import server from '../../app'; import testUtil from '../../tests/util'; +import chai from 'chai'; describe('Project Attachments delete', () => { @@ -113,8 +114,31 @@ describe('Project Attachments delete', () => { if (err) { done(err); } else { - deleteSpy.should.have.been.calledOnce; - done(); + setTimeout(() => + models.ProjectAttachment.findOne({ + where: { + projectId: project1.id, + id: attachment.id, + }, + paranoid: false, + }) + .then((res) => { + if (!res) { + throw new Error('Should found the entity'); + } else { + deleteSpy.should.have.been.calledOnce; + + chai.assert.isNotNull(res.deletedAt); + chai.assert.isNotNull(res.deletedBy); + + request(server) + .get(`/v4/projects/${project1.id}/attachments/${attachment.id}`) + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect(404, done); + } + }), 500); } }); }); diff --git a/src/routes/milestoneTemplates/delete.js b/src/routes/milestoneTemplates/delete.js index bacb3e36..43d3fd57 100644 --- a/src/routes/milestoneTemplates/delete.js +++ b/src/routes/milestoneTemplates/delete.js @@ -25,33 +25,24 @@ module.exports = [ productTemplateId: req.params.productTemplateId, }; - return models.sequelize.transaction(tx => - // Update the deletedBy - models.ProductMilestoneTemplate.update({ deletedBy: req.authUser.userId }, { + return models.sequelize.transaction(() => + // soft delete the record + models.ProductMilestoneTemplate.findOne({ where, - returning: true, - raw: true, - transaction: tx, + }).then((existing) => { + if (!existing) { + // handle 404 + const err = new Error( + `Milestone template not found for milestone template id ${req.params.milestoneTemplateId}`); + err.status = 404; + return Promise.reject(err); + } + return existing.update({ deletedBy: req.authUser.userId }); }) - .then((updatedResults) => { - // Not found - if (updatedResults[0] === 0) { - const apiErr = new Error( - `Milestone template not found for milestone template id ${req.params.milestoneTemplateId}`); - apiErr.status = 404; - return Promise.reject(apiErr); - } - - // Soft delete - return models.ProductMilestoneTemplate.destroy({ - where, - transaction: tx, - }); - }) + .then(entity => entity.destroy())) .then(() => { res.status(204).end(); }) - .catch(next), - ); + .catch(next); }, ]; diff --git a/src/routes/milestoneTemplates/delete.spec.js b/src/routes/milestoneTemplates/delete.spec.js index c5c5ab0a..67d99cd0 100644 --- a/src/routes/milestoneTemplates/delete.spec.js +++ b/src/routes/milestoneTemplates/delete.spec.js @@ -6,7 +6,34 @@ import request from 'supertest'; import models from '../../models'; import server from '../../app'; import testUtil from '../../tests/util'; +import chai from 'chai'; +const expectAfterDelete = (productTemplateId, id, err, next) => { + if (err) throw err; + setTimeout(() => + models.ProductMilestoneTemplate.findOne({ + where: { + id, + productTemplateId, + }, + paranoid: false, + }) + .then((res) => { + if (!res) { + throw new Error('Should found the entity'); + } else { + chai.assert.isNotNull(res.deletedAt); + chai.assert.isNotNull(res.deletedBy); + + request(server) + .get(`/v4/productTemplates/${productTemplateId}/milestones/${id}`) + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect(404, next); + } + }), 500); +}; const productTemplates = [ { name: 'name 1', @@ -180,7 +207,7 @@ describe('DELETE milestone template', () => { Authorization: `Bearer ${testUtil.jwts.admin}`, }) .expect(204) - .end(done); + .end(err => expectAfterDelete(1, 1, err, done)); }); it('should return 204, for connect admin, if template was successfully removed', (done) => { @@ -190,7 +217,7 @@ describe('DELETE milestone template', () => { Authorization: `Bearer ${testUtil.jwts.connectAdmin}`, }) .expect(204) - .end(done); + .end(err => expectAfterDelete(1, 1, err, done)); }); }); }); diff --git a/src/routes/milestones/delete.spec.js b/src/routes/milestones/delete.spec.js index 21502333..477c2bb3 100644 --- a/src/routes/milestones/delete.spec.js +++ b/src/routes/milestones/delete.spec.js @@ -7,8 +7,35 @@ import models from '../../models'; import server from '../../app'; import testUtil from '../../tests/util'; import { EVENT } from '../../constants'; +import chai from 'chai'; +const expectAfterDelete = (timelineId, id, err, next) => { + if (err) throw err; + models.Milestone.findOne({ + where: { + timelineId, + id, + }, + paranoid: false, + }) + .then((res) => { + if (!res) { + throw new Error('Should found the entity'); + } else { + chai.assert.isNotNull(res.deletedAt); + chai.assert.isNotNull(res.deletedBy); + + request(server) + .get(`/v4/timelines/${timelineId}/milestones/${id}`) + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect(404, next); + } + }); +}; + describe('DELETE milestone', () => { beforeEach((done) => { testUtil.clearDb() @@ -57,72 +84,72 @@ describe('DELETE milestone', () => { }, ]).then(() => // Create phase - models.ProjectPhase.bulkCreate([ - { - projectId: 1, - name: 'test project phase 1', - status: 'active', - startDate: '2018-05-15T00:00:00Z', - endDate: '2018-05-15T12:00:00Z', - budget: 20.0, - progress: 1.23456, - details: { - message: 'This can be any json 2', - }, - createdBy: 1, - updatedBy: 1, - }, - { - projectId: 2, - name: 'test project phase 2', - status: 'active', - startDate: '2018-05-16T00:00:00Z', - endDate: '2018-05-16T12:00:00Z', - budget: 21.0, - progress: 1.234567, - details: { - message: 'This can be any json 2', - }, - createdBy: 2, - updatedBy: 2, - deletedAt: '2018-05-15T00:00:00Z', - }, - ])) + models.ProjectPhase.bulkCreate([ + { + projectId: 1, + name: 'test project phase 1', + status: 'active', + startDate: '2018-05-15T00:00:00Z', + endDate: '2018-05-15T12:00:00Z', + budget: 20.0, + progress: 1.23456, + details: { + message: 'This can be any json 2', + }, + createdBy: 1, + updatedBy: 1, + }, + { + projectId: 2, + name: 'test project phase 2', + status: 'active', + startDate: '2018-05-16T00:00:00Z', + endDate: '2018-05-16T12:00:00Z', + budget: 21.0, + progress: 1.234567, + details: { + message: 'This can be any json 2', + }, + createdBy: 2, + updatedBy: 2, + deletedAt: '2018-05-15T00:00:00Z', + }, + ])) .then(() => // Create timelines - models.Timeline.bulkCreate([ - { - name: 'name 1', - description: 'description 1', - startDate: '2018-05-11T00:00:00.000Z', - endDate: '2018-05-12T00:00:00.000Z', - reference: 'project', - referenceId: 1, - createdBy: 1, - updatedBy: 1, - }, - { - name: 'name 2', - description: 'description 2', - startDate: '2018-05-12T00:00:00.000Z', - endDate: '2018-05-13T00:00:00.000Z', - reference: 'phase', - referenceId: 1, - createdBy: 1, - updatedBy: 1, - }, - { - name: 'name 3', - description: 'description 3', - startDate: '2018-05-13T00:00:00.000Z', - endDate: '2018-05-14T00:00:00.000Z', - reference: 'phase', - referenceId: 1, - createdBy: 1, - updatedBy: 1, - deletedAt: '2018-05-14T00:00:00.000Z', - }, - ])) + models.Timeline.bulkCreate([ + { + name: 'name 1', + description: 'description 1', + startDate: '2018-05-11T00:00:00.000Z', + endDate: '2018-05-12T00:00:00.000Z', + reference: 'project', + referenceId: 1, + createdBy: 1, + updatedBy: 1, + }, + { + name: 'name 2', + description: 'description 2', + startDate: '2018-05-12T00:00:00.000Z', + endDate: '2018-05-13T00:00:00.000Z', + reference: 'phase', + referenceId: 1, + createdBy: 1, + updatedBy: 1, + }, + { + name: 'name 3', + description: 'description 3', + startDate: '2018-05-13T00:00:00.000Z', + endDate: '2018-05-14T00:00:00.000Z', + reference: 'phase', + referenceId: 1, + createdBy: 1, + updatedBy: 1, + deletedAt: '2018-05-14T00:00:00.000Z', + }, + ])) .then(() => { // Create milestones models.Milestone.bulkCreate([ @@ -275,11 +302,11 @@ describe('DELETE milestone', () => { Authorization: `Bearer ${testUtil.jwts.admin}`, }) .expect(204) - .end(() => { + .end(err => expectAfterDelete(1, 1, err, () => { // eslint-disable-next-line no-unused-expressions server.services.pubsub.publish.calledWith(EVENT.ROUTING_KEY.MILESTONE_REMOVED).should.be.true; done(); - }); + })); }); it('should return 204, for connect admin, if timeline was successfully removed', (done) => { @@ -289,7 +316,7 @@ describe('DELETE milestone', () => { Authorization: `Bearer ${testUtil.jwts.connectAdmin}`, }) .expect(204) - .end(done); + .end(err => expectAfterDelete(1, 1, err, done)); }); it('should return 204, for connect manager, if timeline was successfully removed', (done) => { @@ -299,7 +326,7 @@ describe('DELETE milestone', () => { Authorization: `Bearer ${testUtil.jwts.manager}`, }) .expect(204) - .end(done); + .end(err => expectAfterDelete(1, 1, err, done)); }); it('should return 204, for copilot, if timeline was successfully removed', (done) => { @@ -309,7 +336,7 @@ describe('DELETE milestone', () => { Authorization: `Bearer ${testUtil.jwts.copilot}`, }) .expect(204) - .end(done); + .end(err => expectAfterDelete(1, 1, err, done)); }); it('should return 204, for member, if timeline was successfully removed', (done) => { @@ -319,7 +346,7 @@ describe('DELETE milestone', () => { Authorization: `Bearer ${testUtil.jwts.member}`, }) .expect(204) - .end(done); + .end(err => expectAfterDelete(1, 1, err, done)); }); }); }); diff --git a/src/routes/phaseProducts/delete.js b/src/routes/phaseProducts/delete.js index 2bc50b7f..b8efbb39 100644 --- a/src/routes/phaseProducts/delete.js +++ b/src/routes/phaseProducts/delete.js @@ -25,18 +25,17 @@ module.exports = [ phaseId, deletedAt: { $eq: null }, }, - }).then(existing => new Promise((accept, reject) => { + }).then((existing) => { if (!existing) { // handle 404 const err = new Error('No active phase product found for project id ' + `${projectId}, phase id ${phaseId} and product id ${productId}`); err.status = 404; - reject(err); - } else { - _.extend(existing, { deletedBy: req.authUser.userId, deletedAt: Date.now() }); - existing.save().then(accept).catch(reject); + return Promise.reject(err); } - }))) + return existing.update({ deletedBy: req.authUser.userId }); + }) + .then(entity => entity.destroy())) .then((deleted) => { req.log.debug('deleted phase product', JSON.stringify(deleted, null, 2)); @@ -49,6 +48,7 @@ module.exports = [ req.app.emit(EVENT.ROUTING_KEY.PROJECT_PHASE_PRODUCT_REMOVED, { req, deleted }); res.status(204).json({}); - }).catch(err => next(err)); + }) + .catch(err => next(err)); }, ]; diff --git a/src/routes/phaseProducts/delete.spec.js b/src/routes/phaseProducts/delete.spec.js index 9bf234b4..7901475c 100644 --- a/src/routes/phaseProducts/delete.spec.js +++ b/src/routes/phaseProducts/delete.spec.js @@ -4,7 +4,35 @@ import request from 'supertest'; import server from '../../app'; import models from '../../models'; import testUtil from '../../tests/util'; +import chai from 'chai'; +const expectAfterDelete = (projectId, phaseId, id, err, next) => { + if (err) throw err; + setTimeout(() => + models.PhaseProduct.findOne({ + where: { + id, + projectId, + phaseId, + }, + paranoid: false, + }) + .then((res) => { + if (!res) { + throw new Error('Should found the entity'); + } else { + chai.assert.isNotNull(res.deletedAt); + chai.assert.isNotNull(res.deletedBy); + + request(server) + .get(`/v4/projects/${projectId}/phases/${phaseId}/products/${id}`) + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect(404, next); + } + }), 500); +}; const body = { name: 'test phase product', type: 'product1', @@ -156,7 +184,8 @@ describe('Phase Products', () => { .set({ Authorization: `Bearer ${testUtil.jwts.copilot}`, }) - .expect(204, done); + .expect(204) + .end(err => expectAfterDelete(projectId, phaseId, productId, err, done)); }); }); }); diff --git a/src/routes/phases/delete.js b/src/routes/phases/delete.js index 5ba2461d..afa440e0 100644 --- a/src/routes/phases/delete.js +++ b/src/routes/phases/delete.js @@ -23,18 +23,17 @@ module.exports = [ projectId, deletedAt: { $eq: null }, }, - }).then(existing => new Promise((accept, reject) => { + }).then((existing) => { if (!existing) { // handle 404 const err = new Error('no active project phase found for project id ' + `${projectId} and phase id ${phaseId}`); err.status = 404; - reject(err); - } else { - _.extend(existing, { deletedBy: req.authUser.userId, deletedAt: Date.now() }); - existing.save().then(accept).catch(reject); + return Promise.reject(err); } - }))) + return existing.update({ deletedBy: req.authUser.userId }); + }) + .then(entity => entity.destroy())) .then((deleted) => { req.log.debug('deleted project phase', JSON.stringify(deleted, null, 2)); diff --git a/src/routes/phases/delete.spec.js b/src/routes/phases/delete.spec.js index 43a56b13..ca93208b 100644 --- a/src/routes/phases/delete.spec.js +++ b/src/routes/phases/delete.spec.js @@ -4,7 +4,34 @@ import request from 'supertest'; import server from '../../app'; import models from '../../models'; import testUtil from '../../tests/util'; +import chai from 'chai'; +const expectAfterDelete = (projectId, id, err, next) => { + if (err) throw err; + setTimeout(() => + models.ProjectPhase.findOne({ + where: { + id, + projectId, + }, + paranoid: false, + }) + .then((res) => { + if (!res) { + throw new Error('Should found the entity'); + } else { + chai.assert.isNotNull(res.deletedAt); + chai.assert.isNotNull(res.deletedBy); + + request(server) + .get(`/v4/projects/${projectId}/phases/${id}`) + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect(404, next); + } + }), 500); +}; const body = { name: 'test project phase', status: 'active', @@ -130,7 +157,7 @@ describe('Project Phases', () => { .set({ Authorization: `Bearer ${testUtil.jwts.copilot}`, }) - .expect(204, done); + .end(err => expectAfterDelete(projectId, phaseId, err, done)); }); }); }); diff --git a/src/routes/productCategories/delete.js b/src/routes/productCategories/delete.js index b12170fb..89f575bd 100644 --- a/src/routes/productCategories/delete.js +++ b/src/routes/productCategories/delete.js @@ -17,38 +17,21 @@ const schema = { module.exports = [ validate(schema), permissions('productCategory.delete'), - (req, res, next) => { - const where = { - deletedAt: { $eq: null }, - key: req.params.key, - }; - - return models.sequelize.transaction(tx => - // Update the deletedBy - models.ProductCategory.update({ deletedBy: req.authUser.userId }, { - where, - returning: true, - raw: true, - transaction: tx, - }) - .then((updatedResults) => { - // Not found - if (updatedResults[0] === 0) { + (req, res, next) => + models.sequelize.transaction(() => + models.ProductCategory.findById(req.params.key) + .then((entity) => { + if (!entity) { const apiErr = new Error(`Product category not found for key ${req.params.key}`); apiErr.status = 404; return Promise.reject(apiErr); } - - // Soft delete - return models.ProductCategory.destroy({ - where, - transaction: tx, - }); + // Update the deletedBy, then delete + return entity.update({ deletedBy: req.authUser.userId }); }) + .then(entity => entity.destroy())) .then(() => { res.status(204).end(); }) .catch(next), - ); - }, ]; diff --git a/src/routes/productCategories/delete.spec.js b/src/routes/productCategories/delete.spec.js index 4fe89a64..dc33a92f 100644 --- a/src/routes/productCategories/delete.spec.js +++ b/src/routes/productCategories/delete.spec.js @@ -2,12 +2,39 @@ * Tests for delete.js */ import request from 'supertest'; +import chai from 'chai'; import models from '../../models'; import server from '../../app'; import testUtil from '../../tests/util'; +const expectAfterDelete = (key, err, next) => { + if (err) throw err; + setTimeout(() => + models.ProductCategory.findOne({ + where: { + key, + }, + paranoid: false, + }) + .then((res) => { + if (!res) { + throw new Error('Should found the entity'); + } else { + chai.assert.isNotNull(res.deletedAt); + chai.assert.isNotNull(res.deletedBy); + + request(server) + .get(`/v4/productCategories/${key}`) + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect(404, next); + } + }), 500); +}; + describe('DELETE product category', () => { const key = 'key1'; @@ -87,7 +114,7 @@ describe('DELETE product category', () => { Authorization: `Bearer ${testUtil.jwts.admin}`, }) .expect(204) - .end(done); + .end(err => expectAfterDelete(key, err, done)); }); it('should return 204, for connect admin, if the product category was successfully removed', (done) => { @@ -97,7 +124,7 @@ describe('DELETE product category', () => { Authorization: `Bearer ${testUtil.jwts.connectAdmin}`, }) .expect(204) - .end(done); + .end(err => expectAfterDelete(key, err, done)); }); }); }); diff --git a/src/routes/productTemplates/delete.js b/src/routes/productTemplates/delete.js index 81c65b6b..03ce1d3b 100644 --- a/src/routes/productTemplates/delete.js +++ b/src/routes/productTemplates/delete.js @@ -17,39 +17,21 @@ const schema = { module.exports = [ validate(schema), permissions('productTemplate.delete'), - (req, res, next) => { - const where = { - deletedAt: { $eq: null }, - id: req.params.templateId, - }; - - return models.sequelize.transaction(tx => - // Update the deletedBy - models.ProductTemplate.update({ deletedBy: req.authUser.userId }, { - where, - returning: true, - raw: true, - transaction: tx, - }) - .then((updatedResults) => { - // Not found - if (updatedResults[0] === 0) { + (req, res, next) => + models.sequelize.transaction(() => + models.ProductTemplate.findById(req.params.templateId) + .then((entity) => { + if (!entity) { const apiErr = new Error(`Product template not found for template id ${req.params.templateId}`); apiErr.status = 404; return Promise.reject(apiErr); } - - // Soft delete - return models.ProductTemplate.destroy({ - where, - transaction: tx, - raw: true, - }); + // Update the deletedBy, then delete + return entity.update({ deletedBy: req.authUser.userId }); }) + .then(entity => entity.destroy())) .then(() => { res.status(204).end(); }) .catch(next), - ); - }, ]; diff --git a/src/routes/productTemplates/delete.spec.js b/src/routes/productTemplates/delete.spec.js index e76e9c67..0f39eb15 100644 --- a/src/routes/productTemplates/delete.spec.js +++ b/src/routes/productTemplates/delete.spec.js @@ -2,11 +2,38 @@ * Tests for delete.js */ import request from 'supertest'; +import chai from 'chai'; import models from '../../models'; import server from '../../app'; import testUtil from '../../tests/util'; +const expectAfterDelete = (id, err, next) => { + if (err) throw err; + setTimeout(() => + models.ProductTemplate.findOne({ + where: { + id, + }, + paranoid: false, + }) + .then((res) => { + if (!res) { + throw new Error('Should found the entity'); + } else { + chai.assert.isNotNull(res.deletedAt); + chai.assert.isNotNull(res.deletedBy); + + request(server) + .get(`/v4/productTemplates/${id}`) + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect(404, next); + } + }), 500); +}; + describe('DELETE product template', () => { let templateId; @@ -107,7 +134,7 @@ describe('DELETE product template', () => { Authorization: `Bearer ${testUtil.jwts.admin}`, }) .expect(204) - .end(done); + .end(err => expectAfterDelete(templateId, err, done)); }); it('should return 204, for connect admin, if template was successfully removed', (done) => { @@ -117,7 +144,7 @@ describe('DELETE product template', () => { Authorization: `Bearer ${testUtil.jwts.connectAdmin}`, }) .expect(204) - .end(done); + .end(err => expectAfterDelete(templateId, err, done)); }); }); }); diff --git a/src/routes/projectMembers/delete.js b/src/routes/projectMembers/delete.js index d0f83ff3..43e1f037 100644 --- a/src/routes/projectMembers/delete.js +++ b/src/routes/projectMembers/delete.js @@ -24,12 +24,13 @@ module.exports = [ }) .then((member) => { if (!member) { - const err = new Error('Record not found'); + const err = new Error(`Project member not found for member id ${req.params.id}`); err.status = 404; return Promise.reject(err); } - return member.destroy({ logging: console.log }); // eslint-disable-line no-console + return member.update({ deletedBy: req.authUser.userId }); // eslint-disable-line no-console }) + .then(member => member.destroy({ logging: console.log })) .then(member => member.save()) // if primary co-pilot is removed promote the next co-pilot to primary #43 .then(member => new Promise((accept, reject) => { diff --git a/src/routes/projectMembers/delete.spec.js b/src/routes/projectMembers/delete.spec.js index e600742d..3667bcff 100644 --- a/src/routes/projectMembers/delete.spec.js +++ b/src/routes/projectMembers/delete.spec.js @@ -11,6 +11,24 @@ import testUtil from '../../tests/util'; const should = chai.should(); +const expectAfterDelete = (projectId, id, err, next) => { + if (err) throw err; + setTimeout(() => + models.ProjectMember.findOne({ + where: { + id, + projectId, + }, + paranoid: false, + }) + .then((res) => { + if (!res) { + throw new Error('Should found the entity'); + } else { + next(); + } + }), 500); +}; describe('Project members delete', () => { let project1; let member1; @@ -109,9 +127,7 @@ describe('Project members delete', () => { }) .expect(204) .end((err) => { - if (err) { - done(err); - } else { + expectAfterDelete(project1.id, member1.id, err, () => { const removedMember = { projectId: project1.id, userId: 40051332, @@ -121,7 +137,7 @@ describe('Project members delete', () => { server.services.pubsub.publish.calledWith('project.member.removed', sinon.match(removedMember)).should.be.true; done(); - } + }); // models.ProjectMember // .count({where: { projectId: project1.id, deletedAt: { $eq: null } }}) @@ -170,9 +186,7 @@ describe('Project members delete', () => { }) .expect(204) .end((err) => { - if (err) { - done(err); - } else { + expectAfterDelete(project1.id, member1.id, err, () => { const removedMember = { projectId: project1.id, userId: 40051332, @@ -201,7 +215,7 @@ describe('Project members delete', () => { plain.userId.should.equal(40051331); done(); }); - } + }); }); }); }); @@ -230,9 +244,7 @@ describe('Project members delete', () => { }) .expect(204) .end((err) => { - if (err) { - done(err); - } else { + expectAfterDelete(project1.id, member2.id, err, () => { const removedMember = { projectId: project1.id, userId: 40051334, @@ -243,7 +255,7 @@ describe('Project members delete', () => { sinon.match(removedMember)).should.be.true; postSpy.should.have.been.calledOnce; done(); - } + }); }); }); @@ -279,9 +291,7 @@ describe('Project members delete', () => { }) .expect(204) .end((err) => { - if (err) { - done(err); - } else { + expectAfterDelete(project1.id, member2.id, err, () => { const removedMember = { projectId: project1.id, userId: 40051334, @@ -292,7 +302,7 @@ describe('Project members delete', () => { sinon.match(removedMember)).should.be.true; postSpy.should.not.have.been.calledOnce; done(); - } + }); }); }); }); diff --git a/src/routes/projectTemplates/delete.js b/src/routes/projectTemplates/delete.js index 4db9a855..b3f353b6 100644 --- a/src/routes/projectTemplates/delete.js +++ b/src/routes/projectTemplates/delete.js @@ -17,39 +17,21 @@ const schema = { module.exports = [ validate(schema), permissions('projectTemplate.delete'), - (req, res, next) => { - const where = { - deletedAt: { $eq: null }, - id: req.params.templateId, - }; - - return models.sequelize.transaction(tx => - // Update the deletedBy - models.ProjectTemplate.update({ deletedBy: req.authUser.userId }, { - where, - returning: true, - raw: true, - transaction: tx, - }) - .then((updatedResults) => { - // Not found - if (updatedResults[0] === 0) { + (req, res, next) => + models.sequelize.transaction(() => + models.ProjectTemplate.findById(req.params.templateId) + .then((entity) => { + if (!entity) { const apiErr = new Error(`Project template not found for template id ${req.params.templateId}`); apiErr.status = 404; return Promise.reject(apiErr); } - - // Soft delete - return models.ProjectTemplate.destroy({ - where, - transaction: tx, - raw: true, - }); + // Update the deletedBy, then delete + return entity.update({ deletedBy: req.authUser.userId }); }) + .then(entity => entity.destroy())) .then(() => { res.status(204).end(); }) .catch(next), - ); - }, ]; diff --git a/src/routes/projectTemplates/delete.spec.js b/src/routes/projectTemplates/delete.spec.js index f82d61ba..a475ee5d 100644 --- a/src/routes/projectTemplates/delete.spec.js +++ b/src/routes/projectTemplates/delete.spec.js @@ -2,11 +2,37 @@ * Tests for delete.js */ import request from 'supertest'; +import chai from 'chai'; import models from '../../models'; import server from '../../app'; import testUtil from '../../tests/util'; +const expectAfterDelete = (id, err, next) => { + if (err) throw err; + setTimeout(() => + models.ProjectTemplate.findOne({ + where: { + id, + }, + paranoid: false, + }) + .then((res) => { + if (!res) { + throw new Error('Should found the entity'); + } else { + chai.assert.isNotNull(res.deletedAt); + chai.assert.isNotNull(res.deletedBy); + + request(server) + .get(`/v4/projectTemplates/${id}`) + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect(404, next); + } + }), 500); +}; describe('DELETE project template', () => { let templateId; @@ -113,8 +139,7 @@ describe('DELETE project template', () => { .set({ Authorization: `Bearer ${testUtil.jwts.admin}`, }) - .expect(204) - .end(done); + .end(err => expectAfterDelete(templateId, err, done)); }); it('should return 204, for connect admin, if template was successfully removed', (done) => { @@ -124,7 +149,7 @@ describe('DELETE project template', () => { Authorization: `Bearer ${testUtil.jwts.connectAdmin}`, }) .expect(204) - .end(done); + .end(err => expectAfterDelete(templateId, err, done)); }); }); }); diff --git a/src/routes/projectTypes/delete.js b/src/routes/projectTypes/delete.js index 7592641c..2f5e2f07 100644 --- a/src/routes/projectTypes/delete.js +++ b/src/routes/projectTypes/delete.js @@ -17,38 +17,21 @@ const schema = { module.exports = [ validate(schema), permissions('projectType.delete'), - (req, res, next) => { - const where = { - deletedAt: { $eq: null }, - key: req.params.key, - }; - - return models.sequelize.transaction(tx => - // Update the deletedBy - models.ProjectType.update({ deletedBy: req.authUser.userId }, { - where, - returning: true, - raw: true, - transaction: tx, - }) - .then((updatedResults) => { - // Not found - if (updatedResults[0] === 0) { + (req, res, next) => + models.sequelize.transaction(() => + models.ProjectType.findById(req.params.key) + .then((entity) => { + if (!entity) { const apiErr = new Error(`Project type not found for key ${req.params.key}`); apiErr.status = 404; return Promise.reject(apiErr); } - - // Soft delete - return models.ProjectType.destroy({ - where, - transaction: tx, - }); + // Update the deletedBy, then delete + return entity.update({ deletedBy: req.authUser.userId }); }) + .then(entity => entity.destroy())) .then(() => { res.status(204).end(); }) .catch(next), - ); - }, ]; diff --git a/src/routes/projectTypes/delete.spec.js b/src/routes/projectTypes/delete.spec.js index 053bfb1c..5a063187 100644 --- a/src/routes/projectTypes/delete.spec.js +++ b/src/routes/projectTypes/delete.spec.js @@ -2,11 +2,36 @@ * Tests for delete.js */ import request from 'supertest'; - +import chai from 'chai'; import models from '../../models'; import server from '../../app'; import testUtil from '../../tests/util'; +const expectAfterDelete = (key, err, next) => { + if (err) throw err; + setTimeout(() => + models.ProjectType.findOne({ + where: { + key, + }, + paranoid: false, + }) + .then((res) => { + if (!res) { + throw new Error('Should found the entity'); + } else { + chai.assert.isNotNull(res.deletedAt); + chai.assert.isNotNull(res.deletedBy); + + request(server) + .get(`/v4/projectTypes/${key}`) + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect(404, next); + } + }), 500); +}; describe('DELETE project type', () => { const key = 'key1'; @@ -87,7 +112,7 @@ describe('DELETE project type', () => { Authorization: `Bearer ${testUtil.jwts.admin}`, }) .expect(204) - .end(done); + .end(err => expectAfterDelete(key, err, done)); }); it('should return 204, for connect admin, if type was successfully removed', (done) => { @@ -97,7 +122,7 @@ describe('DELETE project type', () => { Authorization: `Bearer ${testUtil.jwts.connectAdmin}`, }) .expect(204) - .end(done); + .end(err => expectAfterDelete(key, err, done)); }); }); }); diff --git a/src/routes/projects/delete.js b/src/routes/projects/delete.js index 6b23d486..915d91d9 100644 --- a/src/routes/projects/delete.js +++ b/src/routes/projects/delete.js @@ -16,30 +16,28 @@ module.exports = [ (req, res, next) => { const projectId = _.parseInt(req.params.projectId); - models.sequelize.transaction(t => - // soft delete the record - models.Project.destroy({ - where: { id: projectId }, - cascade: true, - transaction: t, - }), - ) - .then((count) => { - if (count === 0) { - const err = new Error('Project not found'); - err.status = 404; - next(err); - } else { - req.app.services.pubsub.publish( - EVENT.ROUTING_KEY.PROJECT_DELETED, - { id: projectId }, - { correlationId: req.id }, - ); - // emit event - req.app.emit(EVENT.ROUTING_KEY.PROJECT_DELETED, { req, id: projectId }); - res.status(204).json({}); - } - }) - .catch(err => next(err)); + models.sequelize.transaction(() => + models.Project.findById(req.params.projectId) + .then((entity) => { + if (!entity) { + const apiErr = new Error(`Project template not found for template id ${projectId}`); + apiErr.status = 404; + return Promise.reject(apiErr); + } + // Update the deletedBy, then delete + return entity.update({ deletedBy: req.authUser.userId }); + }) + .then(project => project.destroy({ cascade: true }))) + .then(() => { + req.app.services.pubsub.publish( + EVENT.ROUTING_KEY.PROJECT_DELETED, + { id: projectId }, + { correlationId: req.id }, + ); + // emit event + req.app.emit(EVENT.ROUTING_KEY.PROJECT_DELETED, { req, id: projectId }); + res.status(204).json({}); + }) + .catch(err => next(err)); }, ]; diff --git a/src/routes/projects/delete.spec.js b/src/routes/projects/delete.spec.js index 8fee3330..4d1e76e6 100644 --- a/src/routes/projects/delete.spec.js +++ b/src/routes/projects/delete.spec.js @@ -4,70 +4,97 @@ import request from 'supertest'; import models from '../../models'; import server from '../../app'; import testUtil from '../../tests/util'; +import chai from 'chai'; +const expectAfterDelete = (id, err, next) => { + if (err) throw err; + setTimeout(() => + models.Project.findOne({ + where: { + id, + }, + paranoid: false, + }) + .then((res) => { + if (!res) { + throw new Error('Should found the entity'); + } else { + server.services.pubsub.publish.calledWith('project.deleted').should.be.true; + chai.assert.isNotNull(res.deletedAt); + chai.assert.isNotNull(res.deletedBy); + + request(server) + .get(`/v4/projects/${id}`) + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect(404, next); + } + }), 500); +}; describe('Project delete test', () => { let project1; beforeEach((done) => { testUtil.clearDb() - .then(() => { - models.Project.create({ - type: 'generic', - directProjectId: 1, - billingAccountId: 1, - name: 'test1', - description: 'test project1', - status: 'draft', - details: {}, - createdBy: 1, - updatedBy: 1, - }).then((p) => { - project1 = p; - // create members - const promises = [ - // owner - models.ProjectMember.create({ - userId: 40051331, - projectId: project1.id, - role: 'customer', - isPrimary: true, - createdBy: 1, - updatedBy: 1, - }), - // manager - models.ProjectMember.create({ - userId: 40051334, - projectId: project1.id, - role: 'manager', - isPrimary: true, - createdBy: 1, - updatedBy: 1, - }), - // copilot - models.ProjectMember.create({ - userId: 40051332, - projectId: project1.id, - role: 'copilot', - isPrimary: true, - createdBy: 1, - updatedBy: 1, - }), - // team member - models.ProjectMember.create({ - userId: 40051335, - projectId: project1.id, - role: 'customer', - isPrimary: false, - createdBy: 1, - updatedBy: 1, - }), - ]; - Promise.all(promises) + .then(() => { + models.Project.create({ + type: 'generic', + directProjectId: 1, + billingAccountId: 1, + name: 'test1', + description: 'test project1', + status: 'draft', + details: {}, + createdBy: 1, + updatedBy: 1, + }).then((p) => { + project1 = p; + // create members + const promises = [ + // owner + models.ProjectMember.create({ + userId: 40051331, + projectId: project1.id, + role: 'customer', + isPrimary: true, + createdBy: 1, + updatedBy: 1, + }), + // manager + models.ProjectMember.create({ + userId: 40051334, + projectId: project1.id, + role: 'manager', + isPrimary: true, + createdBy: 1, + updatedBy: 1, + }), + // copilot + models.ProjectMember.create({ + userId: 40051332, + projectId: project1.id, + role: 'copilot', + isPrimary: true, + createdBy: 1, + updatedBy: 1, + }), + // team member + models.ProjectMember.create({ + userId: 40051335, + projectId: project1.id, + role: 'customer', + isPrimary: false, + createdBy: 1, + updatedBy: 1, + }), + ]; + Promise.all(promises) .then(() => { done(); }); - }); }); + }); }); after((done) => { @@ -92,12 +119,7 @@ describe('Project delete test', () => { }) .expect(204) .end((err) => { - if (err) { - done(err); - } else { - server.services.pubsub.publish.calledWith('project.deleted').should.be.true; - done(); - } + expectAfterDelete(project1.id, err, done); }); }); @@ -109,12 +131,7 @@ describe('Project delete test', () => { }) .expect(204) .end((err) => { - if (err) { - done(err); - } else { - server.services.pubsub.publish.calledWith('project.deleted').should.be.true; - done(); - } + expectAfterDelete(project1.id, err, done); }); }); @@ -126,12 +143,7 @@ describe('Project delete test', () => { }) .expect(204) .end((err) => { - if (err) { - done(err); - } else { - server.services.pubsub.publish.calledWith('project.deleted').should.be.true; - done(); - } + expectAfterDelete(project1.id, err, done); }); }); }); diff --git a/src/routes/timelines/delete.spec.js b/src/routes/timelines/delete.spec.js index 76a0fb55..44ad6f07 100644 --- a/src/routes/timelines/delete.spec.js +++ b/src/routes/timelines/delete.spec.js @@ -11,6 +11,32 @@ import { EVENT } from '../../constants'; const should = chai.should(); // eslint-disable-line no-unused-vars +const expectAfterDelete = (id, err, next) => { + if (err) throw err; + setTimeout(() => + models.Timeline.findOne({ + where: { + id, + }, + paranoid: false, + }) + .then((res) => { + if (!res) { + throw new Error('Should found the entity'); + } else { + chai.assert.isNotNull(res.deletedAt); + chai.assert.isNotNull(res.deletedBy); + + request(server) + .get(`/v4/timelines/${id}`) + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect(404, next); + } + }), 500); +}; + describe('DELETE timeline', () => { beforeEach((done) => { testUtil.clearDb() @@ -180,6 +206,7 @@ describe('DELETE timeline', () => { after(testUtil.clearDb); + describe('DELETE /timelines/{timelineId}', () => { it('should return 403 if user is not authenticated', (done) => { request(server) @@ -246,19 +273,21 @@ describe('DELETE timeline', () => { Authorization: `Bearer ${testUtil.jwts.admin}`, }) .expect(204) - .end(() => { - // eslint-disable-next-line no-unused-expressions - server.services.pubsub.publish.calledWith(EVENT.ROUTING_KEY.TIMELINE_REMOVED).should.be.true; + .end((err) => { + expectAfterDelete(1, err, () => { + // eslint-disable-next-line no-unused-expressions + server.services.pubsub.publish.calledWith(EVENT.ROUTING_KEY.TIMELINE_REMOVED).should.be.true; - // Milestones are cascade deleted - setTimeout(() => { - models.Milestone.findAll({ where: { timelineId: 1 } }) - .then((afterResults) => { - afterResults.should.have.length(0); + // Milestones are cascade deleted + setTimeout(() => { + models.Milestone.findAll({ where: { timelineId: 1 } }) + .then((afterResults) => { + afterResults.should.have.length(0); - done(); - }); - }, 3000); + done(); + }); + }, 3000); + }); }); }); }); @@ -270,7 +299,7 @@ describe('DELETE timeline', () => { Authorization: `Bearer ${testUtil.jwts.connectAdmin}`, }) .expect(204) - .end(done); + .end(err => expectAfterDelete(1, err, done)); }); it('should return 204, for connect manager, if timeline was successfully removed', (done) => { @@ -280,7 +309,7 @@ describe('DELETE timeline', () => { Authorization: `Bearer ${testUtil.jwts.manager}`, }) .expect(204) - .end(done); + .end(err => expectAfterDelete(1, err, done)); }); it('should return 204, for copilot, if timeline was successfully removed', (done) => { @@ -290,7 +319,7 @@ describe('DELETE timeline', () => { Authorization: `Bearer ${testUtil.jwts.copilot}`, }) .expect(204) - .end(done); + .end(err => expectAfterDelete(1, err, done)); }); it('should return 204, for member, if timeline was successfully removed', (done) => { @@ -300,7 +329,7 @@ describe('DELETE timeline', () => { Authorization: `Bearer ${testUtil.jwts.member}`, }) .expect(204) - .end(done); + .end(err => expectAfterDelete(1, err, done)); }); }); });