From 3fe176e8b146590924cffc1cd86520d500426546 Mon Sep 17 00:00:00 2001 From: eisbilir Date: Fri, 30 Jul 2021 00:38:49 +0300 Subject: [PATCH 1/2] allow update phase members with create-update phase --- docs/Project API.postman_collection.json | 283 ++++++++++++++++++++++- src/permissions/index.js | 4 +- src/routes/phaseMembers/delete.spec.js | 2 +- src/routes/phaseMembers/update.js | 31 +-- src/routes/phaseMembers/updateService.js | 46 ++++ src/routes/phases/create.js | 21 +- src/routes/phases/create.spec.js | 38 +++ src/routes/phases/get.js | 14 +- src/routes/phases/list.js | 23 +- src/routes/phases/update.js | 33 +-- src/routes/phases/update.spec.js | 68 ++++++ src/util.js | 32 +++ 12 files changed, 520 insertions(+), 75 deletions(-) create mode 100644 src/routes/phaseMembers/updateService.js diff --git a/docs/Project API.postman_collection.json b/docs/Project API.postman_collection.json index 0146caa6..1b7583f4 100644 --- a/docs/Project API.postman_collection.json +++ b/docs/Project API.postman_collection.json @@ -1,6 +1,6 @@ { "info": { - "_postman_id": "3eba12ae-a066-4d5a-bdd5-3121377e476b", + "_postman_id": "4c51e04b-42d3-4c9f-bf08-af02f51f7756", "name": "Project API", "schema": "https://schema.getpostman.com/json/collection/v2.1.0/collection.json" }, @@ -4577,6 +4577,208 @@ { "name": "Project Phase", "item": [ + { + "name": "Before Start", + "item": [ + { + "name": "Create project type", + "event": [ + { + "listen": "test", + "script": { + "exec": [ + "pm.test(\"Status code is 201\", function () {", + " pm.response.to.have.status(201);", + " if(pm.response.status === \"Created\") {", + " const response = pm.response.json()", + " pm.environment.set(\"projectTypeId\", response.key);", + " }", + "});" + ], + "type": "text/javascript" + } + } + ], + "request": { + "method": "POST", + "header": [ + { + "key": "Content-Type", + "value": "application/json" + }, + { + "key": "Authorization", + "value": "Bearer {{jwt-token}}" + } + ], + "body": { + "mode": "raw", + "raw": "{\r\n \"key\": \"new key\",\r\n \"displayName\": \"new displayName\",\r\n \"icon\": \"http://example.com/icon4.ico\",\r\n \t\"question\": \"question 4\",\r\n \t\"info\": \"info 4\",\r\n \t\"aliases\": [\"key-41\", \"key_42\"],\r\n \t\"metadata\": {}\r\n }" + }, + "url": { + "raw": "{{api-url}}/projects/metadata/projectTypes", + "host": [ + "{{api-url}}" + ], + "path": [ + "projects", + "metadata", + "projectTypes" + ] + } + }, + "response": [] + }, + { + "name": "Create project", + "event": [ + { + "listen": "test", + "script": { + "exec": [ + "pm.test(\"Status code is 201\", function () {", + " pm.response.to.have.status(201);", + " if(pm.response.status === \"Created\") {", + " const response = pm.response.json()", + " pm.environment.set(\"projectId\", response.id);", + " }", + "});" + ], + "type": "text/javascript" + } + } + ], + "request": { + "method": "POST", + "header": [ + { + "key": "Authorization", + "value": "Bearer {{jwt-token}}" + }, + { + "key": "Content-Type", + "value": "application/json" + } + ], + "body": { + "mode": "raw", + "raw": "{\n\t\"name\": \"test project\",\n\t\"description\": \"Hello I am a test project\",\n\t\"type\": \"{{projectTypeId}}\"\n}" + }, + "url": { + "raw": "{{api-url}}/projects", + "host": [ + "{{api-url}}" + ], + "path": [ + "projects" + ] + }, + "description": "Valid request body. Project should be created successfully." + }, + "response": [] + }, + { + "name": "Create project member - 1", + "event": [ + { + "listen": "test", + "script": { + "exec": [ + "pm.test(\"Status code is 201\", function () {", + " pm.response.to.have.status(201);", + " if(pm.response.status === \"Created\") {", + " const response = pm.response.json()", + " pm.environment.set(\"phaseMemberId-1\", response.userId);", + " }", + "});" + ], + "type": "text/javascript" + } + } + ], + "request": { + "method": "POST", + "header": [ + { + "key": "Authorization", + "value": "Bearer {{jwt-token}}" + }, + { + "key": "Content-Type", + "value": "application/json" + } + ], + "body": { + "mode": "raw", + "raw": "{\n \"userId\": \"40158994\",\n \"role\": \"copilot\"\n}" + }, + "url": { + "raw": "{{api-url}}/projects/{{projectId}}/members", + "host": [ + "{{api-url}}" + ], + "path": [ + "projects", + "{{projectId}}", + "members" + ] + }, + "description": "If the request payload is valid, than project member should be created." + }, + "response": [] + }, + { + "name": "Create project member - 2", + "event": [ + { + "listen": "test", + "script": { + "exec": [ + "pm.test(\"Status code is 201\", function () {", + " pm.response.to.have.status(201);", + " if(pm.response.status === \"Created\") {", + " const response = pm.response.json()", + " pm.environment.set(\"phaseMemberId-2\", response.userId);", + " }", + "});" + ], + "type": "text/javascript" + } + } + ], + "request": { + "method": "POST", + "header": [ + { + "key": "Authorization", + "value": "Bearer {{jwt-token}}" + }, + { + "key": "Content-Type", + "value": "application/json" + } + ], + "body": { + "mode": "raw", + "raw": "{\n \"userId\": \"40153800\",\n \"role\": \"copilot\"\n}" + }, + "url": { + "raw": "{{api-url}}/projects/{{projectId}}/members", + "host": [ + "{{api-url}}" + ], + "path": [ + "projects", + "{{projectId}}", + "members" + ] + }, + "description": "If the request payload is valid, than project member should be created." + }, + "response": [] + } + ] + }, { "name": "Create Phase", "event": [ @@ -4715,6 +4917,52 @@ }, "response": [] }, + { + "name": "Create Phase with members", + "event": [ + { + "listen": "test", + "script": { + "exec": [ + "pm.test(\"Status code is 201\", function () {", + " pm.response.to.have.status(201);", + " pm.environment.set(\"phaseId\", pm.response.json().id);", + "});" + ], + "type": "text/javascript" + } + } + ], + "request": { + "method": "POST", + "header": [ + { + "key": "Authorization", + "value": "Bearer {{jwt-token}}" + }, + { + "key": "Content-Type", + "value": "application/json" + } + ], + "body": { + "mode": "raw", + "raw": "{\n\t\"name\": \"test project phase\",\n\t\"status\": \"active\",\n\t\"startDate\": \"2018-05-15T00:00:00\",\n\t\"endDate\": \"2018-05-16T00:00:00\",\n\t\"budget\": 20,\n\t\"details\": {\n\t\t\"aDetails\": \"a details\"\n\t},\n \"members\": [{{phaseMemberId-1}},{{phaseMemberId-2}}]\n}" + }, + "url": { + "raw": "{{api-url}}/projects/{{projectId}}/phases", + "host": [ + "{{api-url}}" + ], + "path": [ + "projects", + "{{projectId}}", + "phases" + ] + } + }, + "response": [] + }, { "name": "List Phase", "request": { @@ -5008,6 +5256,39 @@ }, "response": [] }, + { + "name": "Update Phase with members", + "request": { + "method": "PATCH", + "header": [ + { + "key": "Authorization", + "value": "Bearer {{jwt-token}}" + }, + { + "key": "Content-Type", + "value": "application/json" + } + ], + "body": { + "mode": "raw", + "raw": "{\n\t\"name\": \"test project phase xxx\",\n\t\"status\": \"inactive\",\n\t\"startDate\": \"2018-05-14T00:00:00\",\n\t\"endDate\": \"2018-05-15T00:00:00\",\n\t\"budget\": 30,\n\t\"progress\": 15,\n\t\"details\": {\n\t\t\"message\": \"phase details\"\n\t},\n \"members\": [{{phaseMemberId-1}},{{phaseMemberId-2}}]\n}" + }, + "url": { + "raw": "{{api-url}}/projects/{{projectId}}/phases/{{phaseId}}", + "host": [ + "{{api-url}}" + ], + "path": [ + "projects", + "{{projectId}}", + "phases", + "{{phaseId}}" + ] + } + }, + "response": [] + }, { "name": "Delete Phase", "request": { diff --git a/src/permissions/index.js b/src/permissions/index.js index 0abd4c7b..8dc1f2ff 100644 --- a/src/permissions/index.js +++ b/src/permissions/index.js @@ -98,8 +98,8 @@ module.exports = () => { Authorizer.setPolicy('project.updatePhaseProduct', copilotAndAbove); Authorizer.setPolicy('project.deletePhaseProduct', copilotAndAbove); - Authorizer.setPolicy('phaseMember.update', projectAdmin); - Authorizer.setPolicy('phaseMember.delete', projectAdmin); + Authorizer.setPolicy('phaseMember.update', copilotAndAbove); + Authorizer.setPolicy('phaseMember.delete', copilotAndAbove); Authorizer.setPolicy('phaseMember.view', generalPermission(PERMISSION.READ_PROJECT_MEMBER)); Authorizer.setPolicy('milestoneTemplate.clone', projectAdmin); diff --git a/src/routes/phaseMembers/delete.spec.js b/src/routes/phaseMembers/delete.spec.js index 46a9ed00..72401df8 100644 --- a/src/routes/phaseMembers/delete.spec.js +++ b/src/routes/phaseMembers/delete.spec.js @@ -130,7 +130,7 @@ describe('Delete phase member', () => { .expect(403, done); }); - it('should return 200 for connect admin', (done) => { + it('should return 204 for connect admin', (done) => { request(server) .delete(`/v5/projects/${id}/phases/${phaseId}/members/${copilotUser.userId}`) .set({ diff --git a/src/routes/phaseMembers/update.js b/src/routes/phaseMembers/update.js index b05aef57..64c55e54 100644 --- a/src/routes/phaseMembers/update.js +++ b/src/routes/phaseMembers/update.js @@ -5,6 +5,7 @@ import { middleware as tcMiddleware } from 'tc-core-library-js'; import models from '../../models'; import util from '../../util'; import { EVENT, RESOURCES, ROUTES } from '../../constants'; +import updateService from './updateService'; /** * API to update a project phase members. @@ -28,10 +29,7 @@ module.exports = [ async (req, res, next) => { const projectId = _.parseInt(req.params.projectId); const phaseId = _.parseInt(req.params.phaseId); - const createdBy = _.parseInt(req.authUser.userId); - const updatedBy = _.parseInt(req.authUser.userId); const newPhaseMembers = req.body.userIds; - const transaction = await models.sequelize.transaction(); try { // chekc if project and phase exist const phase = await models.ProjectPhase.findOne({ @@ -48,31 +46,12 @@ module.exports = [ err.status = 404; throw (err); } - const projectMembers = _.map(await models.ProjectMember.getActiveProjectMembers(projectId), 'userId'); - const notProjectMembers = _.difference(newPhaseMembers, projectMembers); - if (notProjectMembers.length > 0) { - const err = new Error(`Members with id: ${notProjectMembers} are not members of project ${projectId}`); - err.status = 404; - throw (err); - } const phaseMembers = await models.ProjectPhaseMember.getPhaseMembers(phaseId); - const existentPhaseMembers = _.map(phaseMembers, 'userId'); - let updatedPhaseMembers = _.cloneDeep(phaseMembers); - const updatedPhase = _.cloneDeep(phase); - const membersToAdd = _.difference(newPhaseMembers, existentPhaseMembers); - const membersToRemove = _.differenceBy(existentPhaseMembers, newPhaseMembers); - if (membersToRemove.length > 0) { - await models.ProjectPhaseMember.destroy({ where: { phaseId, userId: membersToRemove }, transaction }); - updatedPhaseMembers = _.filter(updatedPhaseMembers, row => !_.includes(membersToRemove, row.userId)); - } - if (membersToAdd.length > 0) { - const createData = _.map(membersToAdd, userId => ({ phaseId, userId, createdBy, updatedBy })); - const result = await models.ProjectPhaseMember.bulkCreate(createData, { transaction }); - updatedPhaseMembers.push(..._.map(result, item => item.toJSON())); - } + const updatedPhaseMembers = await updateService(req.authUser, projectId, phaseId, newPhaseMembers); req.log.debug('updated phase members', JSON.stringify(newPhaseMembers, null, 2)); + const updatedPhase = _.cloneDeep(phase); // emit event - if (membersToRemove.length > 0 || membersToAdd.length > 0) { + if (_.intersectionBy(phaseMembers, updatedPhaseMembers, 'id').length !== updatedPhaseMembers.length) { util.sendResourceToKafkaBus( req, EVENT.ROUTING_KEY.PROJECT_PHASE_UPDATED, @@ -81,10 +60,8 @@ module.exports = [ _.assign(phase, { members: phaseMembers }), ROUTES.PHASES.UPDATE); } - await transaction.commit(); res.json(updatedPhaseMembers); } catch (err) { - await transaction.rollback(); next(err); } }, diff --git a/src/routes/phaseMembers/updateService.js b/src/routes/phaseMembers/updateService.js new file mode 100644 index 00000000..8a474ae0 --- /dev/null +++ b/src/routes/phaseMembers/updateService.js @@ -0,0 +1,46 @@ +import _ from 'lodash'; +import models from '../../models'; + +/** + * Update phase members + * @param {Object} currentUser the user who perform this operation + * @param {String} projectId the project id + * @param {String} phaseId the phase id + * @param {Array} newPhaseMembers the array of userIds + * @returns {Array} the array of updated phase member objects + */ +async function update(currentUser, projectId, phaseId, newPhaseMembers) { + const createdBy = _.parseInt(currentUser.userId); + const updatedBy = _.parseInt(currentUser.userId); + const newMembers = _.uniq(newPhaseMembers); + const transaction = await models.sequelize.transaction(); + try { + const projectMembers = _.map(await models.ProjectMember.getActiveProjectMembers(projectId), 'userId'); + const notProjectMembers = _.difference(newMembers, projectMembers); + if (notProjectMembers.length > 0) { + const err = new Error(`Members with id: ${notProjectMembers} are not members of project ${projectId}`); + err.status = 400; + throw (err); + } + let phaseMembers = await models.ProjectPhaseMember.getPhaseMembers(phaseId); + const existentPhaseMembers = _.map(phaseMembers, 'userId'); + const membersToAdd = _.difference(newMembers, existentPhaseMembers); + const membersToRemove = _.differenceBy(existentPhaseMembers, newMembers); + if (membersToRemove.length > 0) { + await models.ProjectPhaseMember.destroy({ where: { phaseId, userId: membersToRemove }, transaction }); + phaseMembers = _.filter(phaseMembers, row => !_.includes(membersToRemove, row.userId)); + } + if (membersToAdd.length > 0) { + const createData = _.map(membersToAdd, userId => ({ phaseId, userId, createdBy, updatedBy })); + const result = await models.ProjectPhaseMember.bulkCreate(createData, { transaction }); + phaseMembers.push(..._.map(result, item => item.toJSON())); + } + await transaction.commit(); + return phaseMembers; + } catch (err) { + await transaction.rollback(); + throw err; + } +} + +export default update; diff --git a/src/routes/phases/create.js b/src/routes/phases/create.js index a6ff8bef..7a0636c6 100644 --- a/src/routes/phases/create.js +++ b/src/routes/phases/create.js @@ -6,6 +6,8 @@ import models from '../../models'; import util from '../../util'; import { EVENT, RESOURCES } from '../../constants'; +import updatePhaseMemberService from '../phaseMembers/updateService'; + const permissions = require('tc-core-library-js').middleware.permissions; @@ -24,6 +26,7 @@ const addProjectPhaseValidations = { details: Joi.any().optional(), order: Joi.number().integer().optional(), productTemplateId: Joi.number().integer().positive().optional(), + members: Joi.array().items(Joi.number().integer()).optional(), }).required(), }; @@ -61,7 +64,7 @@ module.exports = [ throw err; } return models.ProjectPhase - .create(data) + .create(_.omit(data, 'members')) .then((_newProjectPhase) => { newProjectPhase = _.cloneDeep(_newProjectPhase); req.log.debug('new project phase created (id# %d, name: %s)', @@ -102,6 +105,15 @@ module.exports = [ ]; }); }); + }) + // create phase members if `members` is defined + .then(() => { + if (_.isNil(data.members) || _.isEmpty(data.members)) { + return Promise.resolve(); + } + + return updatePhaseMemberService(req.authUser, projectId, newProjectPhase.id, data.members) + .then(members => _.assign(newProjectPhase, { members })); }); }) .then(() => { @@ -110,10 +122,13 @@ module.exports = [ EVENT.ROUTING_KEY.PROJECT_PHASE_ADDED, RESOURCES.PHASE, newProjectPhase); - - res.status(201).json(newProjectPhase); + return util.populatePhasesWithMemberDetails(newProjectPhase, req) + .then(phase => res.status(201).json(phase)); }) .catch((err) => { + if (err.message) { + _.assign(err, { details: err.message }); + } util.handleError('Error creating project phase', err, req, next); }); }, diff --git a/src/routes/phases/create.spec.js b/src/routes/phases/create.spec.js index 68200ff1..40d9650b 100644 --- a/src/routes/phases/create.spec.js +++ b/src/routes/phases/create.spec.js @@ -369,6 +369,44 @@ describe('Project Phases', () => { }); }); + it('should return 201 with member details if payload has members property', (done) => { + const bodyWithMembers = _.cloneDeep(body); + _.assign(bodyWithMembers, { members: [copilotUser.userId] }); + request(server) + .post(`/v5/projects/${projectId}/phases/`) + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .send(bodyWithMembers) + .expect('Content-Type', /json/) + .expect(201) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body; + validatePhase(resJson, bodyWithMembers); + resJson.members.should.have.length(1); + resJson.members[0].userId.should.eql(copilotUser.userId); + done(); + } + }); + }); + + it('should return 400 if members property includes userId who is not a member of project', (done) => { + const bodyWithMembers = _.cloneDeep(body); + _.assign(bodyWithMembers, { members: [999] }); + request(server) + .post(`/v5/projects/${projectId}/phases/`) + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .send(bodyWithMembers) + .expect('Content-Type', /json/) + .expect(400) + .end(done); + }); + describe('Bus api', () => { let createEventSpy; const sandbox = sinon.sandbox.create(); diff --git a/src/routes/phases/get.js b/src/routes/phases/get.js index deb439b8..5d329363 100644 --- a/src/routes/phases/get.js +++ b/src/routes/phases/get.js @@ -5,15 +5,7 @@ import util from '../../util'; import models from '../../models'; const permissions = tcMiddleware.permissions; -const populateMemberDetails = async (phase, req) => { - const members = _.map(phase.members, member => _.pick(member, 'userId')); - try { - const detailedMembers = await util.getObjectsWithMemberDetails(members, ['userId', 'handle', 'photoURL'], req); - return _.assign(phase, { members: detailedMembers }); - } catch (err) { - return _.assign(phase, { members }); - } -}; + module.exports = [ permissions('project.view'), (req, res, next) => { @@ -60,14 +52,14 @@ module.exports = [ err.status = 404; throw err; } - return populateMemberDetails(phase.toJSON(), req) + return util.populatePhasesWithMemberDetails(phase.toJSON(), req) .then(result => res.json(result)); }) .catch(err => next(err)); } req.log.debug('phase found in ES'); // eslint-disable-next-line no-underscore-dangle - return populateMemberDetails(data[0].inner_hits.phases.hits.hits[0]._source, req) + return util.populatePhasesWithMemberDetails(data[0].inner_hits.phases.hits.hits[0]._source, req) .then(phase => res.json(phase)); }) .catch(next); diff --git a/src/routes/phases/list.js b/src/routes/phases/list.js index 7b76922e..cb69e8ee 100644 --- a/src/routes/phases/list.js +++ b/src/routes/phases/list.js @@ -15,19 +15,6 @@ const PHASE_ATTRIBUTES = _.keys(models.ProjectPhase.rawAttributes); const permissions = tcMiddleware.permissions; -const populateMemberDetails = async (phases, req) => { - let members = _.reduce(phases, (acc, phase) => - _.concat(acc, _.map(phase.members, member => _.pick(member, 'userId'))), []); - members = _.uniqBy(members, 'userId'); - try { - const detailedMembers = await util.getObjectsWithMemberDetails(members, ['userId', 'handle', 'photoURL'], req); - return _.map(phases, phase => - _.assign(phase, { members: _.intersectionBy(detailedMembers, phase.members, 'userId') })); - } catch (err) { - return _.map(phases, phase => - _.assign(phase, { members: _.map(phase.members, member => _.pick(member, 'userId')) })); - } -}; module.exports = [ permissions('project.view'), (req, res, next) => { @@ -71,7 +58,10 @@ module.exports = [ } phases = _.map(phases, phase => _.pick(phase, fields)); - return populateMemberDetails(phases, req) + if (_.indexOf(fields, 'members') < 0) { + return res.json(phases); + } + return util.populatePhasesWithMemberDetails(phases, req) .then(result => res.json(result)); }) .catch((err) => { @@ -122,7 +112,10 @@ module.exports = [ } phases = _.map(phases, phase => _.pick(phase, fields)); // Write to response - return populateMemberDetails(phases, req) + if (_.indexOf(fields, 'members') < 0) { + return res.json(phases); + } + return util.populatePhasesWithMemberDetails(phases, req) .then(result => res.json(result)); }); } diff --git a/src/routes/phases/update.js b/src/routes/phases/update.js index 698f2701..4707ba9a 100644 --- a/src/routes/phases/update.js +++ b/src/routes/phases/update.js @@ -7,6 +7,7 @@ import models from '../../models'; import util from '../../util'; import { EVENT, RESOURCES, ROUTES } from '../../constants'; +import updatePhaseMemberService from '../phaseMembers/updateService'; const permissions = tcMiddleware.permissions; @@ -24,17 +25,9 @@ const updateProjectPhaseValidation = { progress: Joi.number().min(0).optional(), details: Joi.any().optional(), order: Joi.number().integer().optional(), + members: Joi.array().items(Joi.number().integer()).optional(), }).required(), }; -const populateMemberDetails = async (phase, req) => { - const members = _.map(phase.members, member => _.pick(member, 'userId')); - try { - const detailedMembers = await util.getObjectsWithMemberDetails(members, ['userId', 'handle', 'photoURL'], req); - return _.assign(phase, { members: detailedMembers }); - } catch (err) { - return _.assign(phase, { members }); - } -}; module.exports = [ // validate request payload @@ -88,35 +81,45 @@ module.exports = [ err.status = 400; reject(err); } else { - _.extend(existing, updatedProps); + _.extend(existing, _.omit(updatedProps, 'members')); existing.save().then(accept).catch(reject); } } })) .then((updatedPhase) => { - updated = updatedPhase; + updated = updatedPhase.get({ plain: true }); + }) + .then(() => { + if (_.isNil(updatedProps.members)) { + return Promise.resolve(); + } + + return updatePhaseMemberService(req.authUser, projectId, phaseId, updatedProps.members) + .then(members => _.assign(updated, { members })); }), ) .then(() => { req.log.debug('updated project phase', JSON.stringify(updated, null, 2)); - const updatedValue = updated.get({ plain: true }); - // emit event util.sendResourceToKafkaBus( req, EVENT.ROUTING_KEY.PROJECT_PHASE_UPDATED, RESOURCES.PHASE, - updatedValue, + updated, previousValue, ROUTES.PHASES.UPDATE); + if (updated.members) { + return util.populatePhasesWithMemberDetails(updated, req) + .then(result => res.json(result)); + } return models.ProjectPhase.findOne({ where: { id: phaseId, projectId }, include: [{ model: models.ProjectPhaseMember, as: 'members', }], - }).then(phaseWithMembers => populateMemberDetails(phaseWithMembers.toJSON(), req) + }).then(phaseWithMembers => util.populatePhasesWithMemberDetails(phaseWithMembers.toJSON(), req) .then(result => res.json(result))); }) .catch(err => next(err)); diff --git a/src/routes/phases/update.spec.js b/src/routes/phases/update.spec.js index cd5720ac..18554c86 100644 --- a/src/routes/phases/update.spec.js +++ b/src/routes/phases/update.spec.js @@ -284,6 +284,74 @@ describe('Project Phases', () => { }); }); + it('should return 200 with member details after updating members', (done) => { + const bodyWithMembers = _.cloneDeep(updateBody); + _.assign(bodyWithMembers, { members: [copilotUser.userId] }); + request(server) + .patch(`/v5/projects/${projectId}/phases/${phaseId}`) + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .send(bodyWithMembers) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body; + validatePhase(resJson, bodyWithMembers); + resJson.members.should.have.length(1); + resJson.members[0].userId.should.eql(copilotUser.userId); + done(); + } + }); + }); + + it('should return 200 with existent member details vith valid payload without members', (done) => { + models.ProjectPhaseMember.create({ + id: 1, + userId: copilotUser.userId, + phaseId, + createdBy: 1, + updatedBy: 1, + }).then(() => { + request(server) + .patch(`/v5/projects/${projectId}/phases/${phaseId}`) + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .send(updateBody) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body; + validatePhase(resJson, updateBody); + resJson.members.should.have.length(1); + resJson.members[0].userId.should.eql(copilotUser.userId); + done(); + } + }); + }); + }); + + it('should return 400 if members property includes userId who is not a member of project', (done) => { + const bodyWithMembers = _.cloneDeep(updateBody); + _.assign(bodyWithMembers, { members: [999] }); + request(server) + .patch(`/v5/projects/${projectId}/phases/${phaseId}`) + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .send(bodyWithMembers) + .expect('Content-Type', /json/) + .expect(400) + .end(done); + }); + it('should return 403 if requested by manager which is not a member', (done) => { request(server) .patch(`/v5/projects/${projectId}/phases/${phaseId}`) diff --git a/src/util.js b/src/util.js index a5ab739a..c3432ff4 100644 --- a/src/util.js +++ b/src/util.js @@ -791,6 +791,38 @@ const projectServiceUtils = { }); }, + /** + * Add member details to Project Phase objects + * + * @param {Array|Object} phases Array of phase object or single phase object + * @param {Object} req The request object + * + * @return {Array|Object} Phase(s) with member details + */ + populatePhasesWithMemberDetails: async (phases, req) => { + if (_.isArray(phases)) { + let members = _.reduce(phases, (acc, phase) => + _.concat(acc, _.map(phase.members, member => _.pick(member, 'userId'))), []); + members = _.uniqBy(members, 'userId'); + try { + const detailedMembers = await util.getObjectsWithMemberDetails(members, ['userId', 'handle', 'photoURL'], req); + return _.map(phases, phase => + _.assign(phase, { members: _.intersectionBy(detailedMembers, phase.members, 'userId') })); + } catch (err) { + return _.map(phases, phase => + _.assign(phase, { members: _.map(phase.members, member => _.pick(member, 'userId')) })); + } + } else { + const members = _.map(phases.members, member => _.pick(member, 'userId')); + try { + const detailedMembers = await util.getObjectsWithMemberDetails(members, ['userId', 'handle', 'photoURL'], req); + return _.assign(phases, { members: detailedMembers }); + } catch (err) { + return _.assign(phases, { members }); + } + } + }, + /** * Retrieve member details from userIds */ From 9237cb80c81d7e816a70e3a3d9aae14e930dc23e Mon Sep 17 00:00:00 2001 From: eisbilir Date: Fri, 30 Jul 2021 16:28:47 +0300 Subject: [PATCH 2/2] use transaction --- src/routes/phaseMembers/updateService.js | 18 ++++++++++++++---- src/routes/phases/create.js | 9 ++++----- src/routes/phases/delete.js | 6 +++--- src/routes/phases/update.js | 6 +++--- 4 files changed, 24 insertions(+), 15 deletions(-) diff --git a/src/routes/phaseMembers/updateService.js b/src/routes/phaseMembers/updateService.js index 8a474ae0..b0781c34 100644 --- a/src/routes/phaseMembers/updateService.js +++ b/src/routes/phaseMembers/updateService.js @@ -7,13 +7,19 @@ import models from '../../models'; * @param {String} projectId the project id * @param {String} phaseId the phase id * @param {Array} newPhaseMembers the array of userIds + * @param {Object} _transaction the sequelize transaction (optional) * @returns {Array} the array of updated phase member objects */ -async function update(currentUser, projectId, phaseId, newPhaseMembers) { +async function update(currentUser, projectId, phaseId, newPhaseMembers, _transaction) { const createdBy = _.parseInt(currentUser.userId); const updatedBy = _.parseInt(currentUser.userId); const newMembers = _.uniq(newPhaseMembers); - const transaction = await models.sequelize.transaction(); + let transaction; + if (_.isUndefined(_transaction)) { + transaction = await models.sequelize.transaction(); + } else { + transaction = _transaction; + } try { const projectMembers = _.map(await models.ProjectMember.getActiveProjectMembers(projectId), 'userId'); const notProjectMembers = _.difference(newMembers, projectMembers); @@ -35,10 +41,14 @@ async function update(currentUser, projectId, phaseId, newPhaseMembers) { const result = await models.ProjectPhaseMember.bulkCreate(createData, { transaction }); phaseMembers.push(..._.map(result, item => item.toJSON())); } - await transaction.commit(); + if (_.isUndefined(_transaction)) { + await transaction.commit(); + } return phaseMembers; } catch (err) { - await transaction.rollback(); + if (_.isUndefined(_transaction)) { + await transaction.rollback(); + } throw err; } } diff --git a/src/routes/phases/create.js b/src/routes/phases/create.js index 7a0636c6..1463d898 100644 --- a/src/routes/phases/create.js +++ b/src/routes/phases/create.js @@ -47,7 +47,7 @@ module.exports = [ }); let newProjectPhase = null; - models.sequelize.transaction(() => { + models.sequelize.transaction((transaction) => { req.log.debug('Create Phase - Starting transaction'); return models.Project.findOne({ where: { id: projectId, deletedAt: { $eq: null } }, @@ -64,7 +64,7 @@ module.exports = [ throw err; } return models.ProjectPhase - .create(_.omit(data, 'members')) + .create(_.omit(data, 'members'), { transaction }) .then((_newProjectPhase) => { newProjectPhase = _.cloneDeep(_newProjectPhase); req.log.debug('new project phase created (id# %d, name: %s)', @@ -88,7 +88,6 @@ module.exports = [ err.status = 400; throw err; } - // Create the phase product return models.PhaseProduct.create({ name: productTemplate.name, @@ -98,7 +97,7 @@ module.exports = [ phaseId: newProjectPhase.id, createdBy: req.authUser.userId, updatedBy: req.authUser.userId, - }) + }, { transaction }) .then((phaseProduct) => { newProjectPhase.products = [ _.omit(phaseProduct.toJSON(), ['deletedAt', 'deletedBy']), @@ -112,7 +111,7 @@ module.exports = [ return Promise.resolve(); } - return updatePhaseMemberService(req.authUser, projectId, newProjectPhase.id, data.members) + return updatePhaseMemberService(req.authUser, projectId, newProjectPhase.id, data.members, transaction) .then(members => _.assign(newProjectPhase, { members })); }); }) diff --git a/src/routes/phases/delete.js b/src/routes/phases/delete.js index 3dcaa017..eccc2c02 100644 --- a/src/routes/phases/delete.js +++ b/src/routes/phases/delete.js @@ -16,7 +16,7 @@ module.exports = [ const projectId = _.parseInt(req.params.projectId); const phaseId = _.parseInt(req.params.phaseId); - models.sequelize.transaction(() => + models.sequelize.transaction(transaction => // soft delete the record models.ProjectPhase.findOne({ where: { @@ -32,9 +32,9 @@ module.exports = [ err.status = 404; return Promise.reject(err); } - return existing.update({ deletedBy: req.authUser.userId }); + return existing.update({ deletedBy: req.authUser.userId }, { transaction }); }) - .then(entity => entity.destroy())) + .then(entity => entity.destroy({ transaction }))) .then((deleted) => { req.log.debug('deleted project phase', JSON.stringify(deleted, null, 2)); diff --git a/src/routes/phases/update.js b/src/routes/phases/update.js index 4707ba9a..3008f583 100644 --- a/src/routes/phases/update.js +++ b/src/routes/phases/update.js @@ -45,7 +45,7 @@ module.exports = [ let previousValue; let updated; - models.sequelize.transaction(() => models.ProjectPhase.findOne({ + models.sequelize.transaction(transaction => models.ProjectPhase.findOne({ where: { id: phaseId, projectId, @@ -82,7 +82,7 @@ module.exports = [ reject(err); } else { _.extend(existing, _.omit(updatedProps, 'members')); - existing.save().then(accept).catch(reject); + existing.save({ transaction }).then(accept).catch(reject); } } })) @@ -94,7 +94,7 @@ module.exports = [ return Promise.resolve(); } - return updatePhaseMemberService(req.authUser, projectId, phaseId, updatedProps.members) + return updatePhaseMemberService(req.authUser, projectId, phaseId, updatedProps.members, transaction) .then(members => _.assign(updated, { members })); }), )