Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 64d0c41

Browse files
elkhawajahelkhawajah
authoredAug 2, 2021
Merge pull request #662 from eisbilir/feature/new-milestone-concept
allow update phase members with create-update phase
2 parents 1a2e995 + 9237cb8 commit 64d0c41

File tree

13 files changed

+537
-83
lines changed

13 files changed

+537
-83
lines changed
 

‎docs/Project API.postman_collection.json

Lines changed: 282 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"info": {
3-
"_postman_id": "3eba12ae-a066-4d5a-bdd5-3121377e476b",
3+
"_postman_id": "4c51e04b-42d3-4c9f-bf08-af02f51f7756",
44
"name": "Project API",
55
"schema": "https://schema.getpostman.com/json/collection/v2.1.0/collection.json"
66
},
@@ -4577,6 +4577,208 @@
45774577
{
45784578
"name": "Project Phase",
45794579
"item": [
4580+
{
4581+
"name": "Before Start",
4582+
"item": [
4583+
{
4584+
"name": "Create project type",
4585+
"event": [
4586+
{
4587+
"listen": "test",
4588+
"script": {
4589+
"exec": [
4590+
"pm.test(\"Status code is 201\", function () {",
4591+
" pm.response.to.have.status(201);",
4592+
" if(pm.response.status === \"Created\") {",
4593+
" const response = pm.response.json()",
4594+
" pm.environment.set(\"projectTypeId\", response.key);",
4595+
" }",
4596+
"});"
4597+
],
4598+
"type": "text/javascript"
4599+
}
4600+
}
4601+
],
4602+
"request": {
4603+
"method": "POST",
4604+
"header": [
4605+
{
4606+
"key": "Content-Type",
4607+
"value": "application/json"
4608+
},
4609+
{
4610+
"key": "Authorization",
4611+
"value": "Bearer {{jwt-token}}"
4612+
}
4613+
],
4614+
"body": {
4615+
"mode": "raw",
4616+
"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 }"
4617+
},
4618+
"url": {
4619+
"raw": "{{api-url}}/projects/metadata/projectTypes",
4620+
"host": [
4621+
"{{api-url}}"
4622+
],
4623+
"path": [
4624+
"projects",
4625+
"metadata",
4626+
"projectTypes"
4627+
]
4628+
}
4629+
},
4630+
"response": []
4631+
},
4632+
{
4633+
"name": "Create project",
4634+
"event": [
4635+
{
4636+
"listen": "test",
4637+
"script": {
4638+
"exec": [
4639+
"pm.test(\"Status code is 201\", function () {",
4640+
" pm.response.to.have.status(201);",
4641+
" if(pm.response.status === \"Created\") {",
4642+
" const response = pm.response.json()",
4643+
" pm.environment.set(\"projectId\", response.id);",
4644+
" }",
4645+
"});"
4646+
],
4647+
"type": "text/javascript"
4648+
}
4649+
}
4650+
],
4651+
"request": {
4652+
"method": "POST",
4653+
"header": [
4654+
{
4655+
"key": "Authorization",
4656+
"value": "Bearer {{jwt-token}}"
4657+
},
4658+
{
4659+
"key": "Content-Type",
4660+
"value": "application/json"
4661+
}
4662+
],
4663+
"body": {
4664+
"mode": "raw",
4665+
"raw": "{\n\t\"name\": \"test project\",\n\t\"description\": \"Hello I am a test project\",\n\t\"type\": \"{{projectTypeId}}\"\n}"
4666+
},
4667+
"url": {
4668+
"raw": "{{api-url}}/projects",
4669+
"host": [
4670+
"{{api-url}}"
4671+
],
4672+
"path": [
4673+
"projects"
4674+
]
4675+
},
4676+
"description": "Valid request body. Project should be created successfully."
4677+
},
4678+
"response": []
4679+
},
4680+
{
4681+
"name": "Create project member - 1",
4682+
"event": [
4683+
{
4684+
"listen": "test",
4685+
"script": {
4686+
"exec": [
4687+
"pm.test(\"Status code is 201\", function () {",
4688+
" pm.response.to.have.status(201);",
4689+
" if(pm.response.status === \"Created\") {",
4690+
" const response = pm.response.json()",
4691+
" pm.environment.set(\"phaseMemberId-1\", response.userId);",
4692+
" }",
4693+
"});"
4694+
],
4695+
"type": "text/javascript"
4696+
}
4697+
}
4698+
],
4699+
"request": {
4700+
"method": "POST",
4701+
"header": [
4702+
{
4703+
"key": "Authorization",
4704+
"value": "Bearer {{jwt-token}}"
4705+
},
4706+
{
4707+
"key": "Content-Type",
4708+
"value": "application/json"
4709+
}
4710+
],
4711+
"body": {
4712+
"mode": "raw",
4713+
"raw": "{\n \"userId\": \"40158994\",\n \"role\": \"copilot\"\n}"
4714+
},
4715+
"url": {
4716+
"raw": "{{api-url}}/projects/{{projectId}}/members",
4717+
"host": [
4718+
"{{api-url}}"
4719+
],
4720+
"path": [
4721+
"projects",
4722+
"{{projectId}}",
4723+
"members"
4724+
]
4725+
},
4726+
"description": "If the request payload is valid, than project member should be created."
4727+
},
4728+
"response": []
4729+
},
4730+
{
4731+
"name": "Create project member - 2",
4732+
"event": [
4733+
{
4734+
"listen": "test",
4735+
"script": {
4736+
"exec": [
4737+
"pm.test(\"Status code is 201\", function () {",
4738+
" pm.response.to.have.status(201);",
4739+
" if(pm.response.status === \"Created\") {",
4740+
" const response = pm.response.json()",
4741+
" pm.environment.set(\"phaseMemberId-2\", response.userId);",
4742+
" }",
4743+
"});"
4744+
],
4745+
"type": "text/javascript"
4746+
}
4747+
}
4748+
],
4749+
"request": {
4750+
"method": "POST",
4751+
"header": [
4752+
{
4753+
"key": "Authorization",
4754+
"value": "Bearer {{jwt-token}}"
4755+
},
4756+
{
4757+
"key": "Content-Type",
4758+
"value": "application/json"
4759+
}
4760+
],
4761+
"body": {
4762+
"mode": "raw",
4763+
"raw": "{\n \"userId\": \"40153800\",\n \"role\": \"copilot\"\n}"
4764+
},
4765+
"url": {
4766+
"raw": "{{api-url}}/projects/{{projectId}}/members",
4767+
"host": [
4768+
"{{api-url}}"
4769+
],
4770+
"path": [
4771+
"projects",
4772+
"{{projectId}}",
4773+
"members"
4774+
]
4775+
},
4776+
"description": "If the request payload is valid, than project member should be created."
4777+
},
4778+
"response": []
4779+
}
4780+
]
4781+
},
45804782
{
45814783
"name": "Create Phase",
45824784
"event": [
@@ -4715,6 +4917,52 @@
47154917
},
47164918
"response": []
47174919
},
4920+
{
4921+
"name": "Create Phase with members",
4922+
"event": [
4923+
{
4924+
"listen": "test",
4925+
"script": {
4926+
"exec": [
4927+
"pm.test(\"Status code is 201\", function () {",
4928+
" pm.response.to.have.status(201);",
4929+
" pm.environment.set(\"phaseId\", pm.response.json().id);",
4930+
"});"
4931+
],
4932+
"type": "text/javascript"
4933+
}
4934+
}
4935+
],
4936+
"request": {
4937+
"method": "POST",
4938+
"header": [
4939+
{
4940+
"key": "Authorization",
4941+
"value": "Bearer {{jwt-token}}"
4942+
},
4943+
{
4944+
"key": "Content-Type",
4945+
"value": "application/json"
4946+
}
4947+
],
4948+
"body": {
4949+
"mode": "raw",
4950+
"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}"
4951+
},
4952+
"url": {
4953+
"raw": "{{api-url}}/projects/{{projectId}}/phases",
4954+
"host": [
4955+
"{{api-url}}"
4956+
],
4957+
"path": [
4958+
"projects",
4959+
"{{projectId}}",
4960+
"phases"
4961+
]
4962+
}
4963+
},
4964+
"response": []
4965+
},
47184966
{
47194967
"name": "List Phase",
47204968
"request": {
@@ -5008,6 +5256,39 @@
50085256
},
50095257
"response": []
50105258
},
5259+
{
5260+
"name": "Update Phase with members",
5261+
"request": {
5262+
"method": "PATCH",
5263+
"header": [
5264+
{
5265+
"key": "Authorization",
5266+
"value": "Bearer {{jwt-token}}"
5267+
},
5268+
{
5269+
"key": "Content-Type",
5270+
"value": "application/json"
5271+
}
5272+
],
5273+
"body": {
5274+
"mode": "raw",
5275+
"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}"
5276+
},
5277+
"url": {
5278+
"raw": "{{api-url}}/projects/{{projectId}}/phases/{{phaseId}}",
5279+
"host": [
5280+
"{{api-url}}"
5281+
],
5282+
"path": [
5283+
"projects",
5284+
"{{projectId}}",
5285+
"phases",
5286+
"{{phaseId}}"
5287+
]
5288+
}
5289+
},
5290+
"response": []
5291+
},
50115292
{
50125293
"name": "Delete Phase",
50135294
"request": {

‎src/permissions/index.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,8 @@ module.exports = () => {
9898
Authorizer.setPolicy('project.updatePhaseProduct', copilotAndAbove);
9999
Authorizer.setPolicy('project.deletePhaseProduct', copilotAndAbove);
100100

101-
Authorizer.setPolicy('phaseMember.update', projectAdmin);
102-
Authorizer.setPolicy('phaseMember.delete', projectAdmin);
101+
Authorizer.setPolicy('phaseMember.update', copilotAndAbove);
102+
Authorizer.setPolicy('phaseMember.delete', copilotAndAbove);
103103
Authorizer.setPolicy('phaseMember.view', generalPermission(PERMISSION.READ_PROJECT_MEMBER));
104104

105105
Authorizer.setPolicy('milestoneTemplate.clone', projectAdmin);

‎src/routes/phaseMembers/delete.spec.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ describe('Delete phase member', () => {
130130
.expect(403, done);
131131
});
132132

133-
it('should return 200 for connect admin', (done) => {
133+
it('should return 204 for connect admin', (done) => {
134134
request(server)
135135
.delete(`/v5/projects/${id}/phases/${phaseId}/members/${copilotUser.userId}`)
136136
.set({

‎src/routes/phaseMembers/update.js

Lines changed: 4 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { middleware as tcMiddleware } from 'tc-core-library-js';
55
import models from '../../models';
66
import util from '../../util';
77
import { EVENT, RESOURCES, ROUTES } from '../../constants';
8+
import updateService from './updateService';
89

910
/**
1011
* API to update a project phase members.
@@ -28,10 +29,7 @@ module.exports = [
2829
async (req, res, next) => {
2930
const projectId = _.parseInt(req.params.projectId);
3031
const phaseId = _.parseInt(req.params.phaseId);
31-
const createdBy = _.parseInt(req.authUser.userId);
32-
const updatedBy = _.parseInt(req.authUser.userId);
3332
const newPhaseMembers = req.body.userIds;
34-
const transaction = await models.sequelize.transaction();
3533
try {
3634
// chekc if project and phase exist
3735
const phase = await models.ProjectPhase.findOne({
@@ -48,31 +46,12 @@ module.exports = [
4846
err.status = 404;
4947
throw (err);
5048
}
51-
const projectMembers = _.map(await models.ProjectMember.getActiveProjectMembers(projectId), 'userId');
52-
const notProjectMembers = _.difference(newPhaseMembers, projectMembers);
53-
if (notProjectMembers.length > 0) {
54-
const err = new Error(`Members with id: ${notProjectMembers} are not members of project ${projectId}`);
55-
err.status = 404;
56-
throw (err);
57-
}
5849
const phaseMembers = await models.ProjectPhaseMember.getPhaseMembers(phaseId);
59-
const existentPhaseMembers = _.map(phaseMembers, 'userId');
60-
let updatedPhaseMembers = _.cloneDeep(phaseMembers);
61-
const updatedPhase = _.cloneDeep(phase);
62-
const membersToAdd = _.difference(newPhaseMembers, existentPhaseMembers);
63-
const membersToRemove = _.differenceBy(existentPhaseMembers, newPhaseMembers);
64-
if (membersToRemove.length > 0) {
65-
await models.ProjectPhaseMember.destroy({ where: { phaseId, userId: membersToRemove }, transaction });
66-
updatedPhaseMembers = _.filter(updatedPhaseMembers, row => !_.includes(membersToRemove, row.userId));
67-
}
68-
if (membersToAdd.length > 0) {
69-
const createData = _.map(membersToAdd, userId => ({ phaseId, userId, createdBy, updatedBy }));
70-
const result = await models.ProjectPhaseMember.bulkCreate(createData, { transaction });
71-
updatedPhaseMembers.push(..._.map(result, item => item.toJSON()));
72-
}
50+
const updatedPhaseMembers = await updateService(req.authUser, projectId, phaseId, newPhaseMembers);
7351
req.log.debug('updated phase members', JSON.stringify(newPhaseMembers, null, 2));
52+
const updatedPhase = _.cloneDeep(phase);
7453
// emit event
75-
if (membersToRemove.length > 0 || membersToAdd.length > 0) {
54+
if (_.intersectionBy(phaseMembers, updatedPhaseMembers, 'id').length !== updatedPhaseMembers.length) {
7655
util.sendResourceToKafkaBus(
7756
req,
7857
EVENT.ROUTING_KEY.PROJECT_PHASE_UPDATED,
@@ -81,10 +60,8 @@ module.exports = [
8160
_.assign(phase, { members: phaseMembers }),
8261
ROUTES.PHASES.UPDATE);
8362
}
84-
await transaction.commit();
8563
res.json(updatedPhaseMembers);
8664
} catch (err) {
87-
await transaction.rollback();
8865
next(err);
8966
}
9067
},

‎src/routes/phaseMembers/updateService.js

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
import _ from 'lodash';
2+
import models from '../../models';
3+
4+
/**
5+
* Update phase members
6+
* @param {Object} currentUser the user who perform this operation
7+
* @param {String} projectId the project id
8+
* @param {String} phaseId the phase id
9+
* @param {Array<Number>} newPhaseMembers the array of userIds
10+
* @param {Object} _transaction the sequelize transaction (optional)
11+
* @returns {Array<Number>} the array of updated phase member objects
12+
*/
13+
async function update(currentUser, projectId, phaseId, newPhaseMembers, _transaction) {
14+
const createdBy = _.parseInt(currentUser.userId);
15+
const updatedBy = _.parseInt(currentUser.userId);
16+
const newMembers = _.uniq(newPhaseMembers);
17+
let transaction;
18+
if (_.isUndefined(_transaction)) {
19+
transaction = await models.sequelize.transaction();
20+
} else {
21+
transaction = _transaction;
22+
}
23+
try {
24+
const projectMembers = _.map(await models.ProjectMember.getActiveProjectMembers(projectId), 'userId');
25+
const notProjectMembers = _.difference(newMembers, projectMembers);
26+
if (notProjectMembers.length > 0) {
27+
const err = new Error(`Members with id: ${notProjectMembers} are not members of project ${projectId}`);
28+
err.status = 400;
29+
throw (err);
30+
}
31+
let phaseMembers = await models.ProjectPhaseMember.getPhaseMembers(phaseId);
32+
const existentPhaseMembers = _.map(phaseMembers, 'userId');
33+
const membersToAdd = _.difference(newMembers, existentPhaseMembers);
34+
const membersToRemove = _.differenceBy(existentPhaseMembers, newMembers);
35+
if (membersToRemove.length > 0) {
36+
await models.ProjectPhaseMember.destroy({ where: { phaseId, userId: membersToRemove }, transaction });
37+
phaseMembers = _.filter(phaseMembers, row => !_.includes(membersToRemove, row.userId));
38+
}
39+
if (membersToAdd.length > 0) {
40+
const createData = _.map(membersToAdd, userId => ({ phaseId, userId, createdBy, updatedBy }));
41+
const result = await models.ProjectPhaseMember.bulkCreate(createData, { transaction });
42+
phaseMembers.push(..._.map(result, item => item.toJSON()));
43+
}
44+
if (_.isUndefined(_transaction)) {
45+
await transaction.commit();
46+
}
47+
return phaseMembers;
48+
} catch (err) {
49+
if (_.isUndefined(_transaction)) {
50+
await transaction.rollback();
51+
}
52+
throw err;
53+
}
54+
}
55+
56+
export default update;

‎src/routes/phases/create.js

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import models from '../../models';
66
import util from '../../util';
77
import { EVENT, RESOURCES } from '../../constants';
88

9+
import updatePhaseMemberService from '../phaseMembers/updateService';
10+
911
const permissions = require('tc-core-library-js').middleware.permissions;
1012

1113

@@ -24,6 +26,7 @@ const addProjectPhaseValidations = {
2426
details: Joi.any().optional(),
2527
order: Joi.number().integer().optional(),
2628
productTemplateId: Joi.number().integer().positive().optional(),
29+
members: Joi.array().items(Joi.number().integer()).optional(),
2730
}).required(),
2831
};
2932

@@ -44,7 +47,7 @@ module.exports = [
4447
});
4548

4649
let newProjectPhase = null;
47-
models.sequelize.transaction(() => {
50+
models.sequelize.transaction((transaction) => {
4851
req.log.debug('Create Phase - Starting transaction');
4952
return models.Project.findOne({
5053
where: { id: projectId, deletedAt: { $eq: null } },
@@ -61,7 +64,7 @@ module.exports = [
6164
throw err;
6265
}
6366
return models.ProjectPhase
64-
.create(data)
67+
.create(_.omit(data, 'members'), { transaction })
6568
.then((_newProjectPhase) => {
6669
newProjectPhase = _.cloneDeep(_newProjectPhase);
6770
req.log.debug('new project phase created (id# %d, name: %s)',
@@ -85,7 +88,6 @@ module.exports = [
8588
err.status = 400;
8689
throw err;
8790
}
88-
8991
// Create the phase product
9092
return models.PhaseProduct.create({
9193
name: productTemplate.name,
@@ -95,13 +97,22 @@ module.exports = [
9597
phaseId: newProjectPhase.id,
9698
createdBy: req.authUser.userId,
9799
updatedBy: req.authUser.userId,
98-
})
100+
}, { transaction })
99101
.then((phaseProduct) => {
100102
newProjectPhase.products = [
101103
_.omit(phaseProduct.toJSON(), ['deletedAt', 'deletedBy']),
102104
];
103105
});
104106
});
107+
})
108+
// create phase members if `members` is defined
109+
.then(() => {
110+
if (_.isNil(data.members) || _.isEmpty(data.members)) {
111+
return Promise.resolve();
112+
}
113+
114+
return updatePhaseMemberService(req.authUser, projectId, newProjectPhase.id, data.members, transaction)
115+
.then(members => _.assign(newProjectPhase, { members }));
105116
});
106117
})
107118
.then(() => {
@@ -110,10 +121,13 @@ module.exports = [
110121
EVENT.ROUTING_KEY.PROJECT_PHASE_ADDED,
111122
RESOURCES.PHASE,
112123
newProjectPhase);
113-
114-
res.status(201).json(newProjectPhase);
124+
return util.populatePhasesWithMemberDetails(newProjectPhase, req)
125+
.then(phase => res.status(201).json(phase));
115126
})
116127
.catch((err) => {
128+
if (err.message) {
129+
_.assign(err, { details: err.message });
130+
}
117131
util.handleError('Error creating project phase', err, req, next);
118132
});
119133
},

‎src/routes/phases/create.spec.js

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,6 +369,44 @@ describe('Project Phases', () => {
369369
});
370370
});
371371

372+
it('should return 201 with member details if payload has members property', (done) => {
373+
const bodyWithMembers = _.cloneDeep(body);
374+
_.assign(bodyWithMembers, { members: [copilotUser.userId] });
375+
request(server)
376+
.post(`/v5/projects/${projectId}/phases/`)
377+
.set({
378+
Authorization: `Bearer ${testUtil.jwts.admin}`,
379+
})
380+
.send(bodyWithMembers)
381+
.expect('Content-Type', /json/)
382+
.expect(201)
383+
.end((err, res) => {
384+
if (err) {
385+
done(err);
386+
} else {
387+
const resJson = res.body;
388+
validatePhase(resJson, bodyWithMembers);
389+
resJson.members.should.have.length(1);
390+
resJson.members[0].userId.should.eql(copilotUser.userId);
391+
done();
392+
}
393+
});
394+
});
395+
396+
it('should return 400 if members property includes userId who is not a member of project', (done) => {
397+
const bodyWithMembers = _.cloneDeep(body);
398+
_.assign(bodyWithMembers, { members: [999] });
399+
request(server)
400+
.post(`/v5/projects/${projectId}/phases/`)
401+
.set({
402+
Authorization: `Bearer ${testUtil.jwts.admin}`,
403+
})
404+
.send(bodyWithMembers)
405+
.expect('Content-Type', /json/)
406+
.expect(400)
407+
.end(done);
408+
});
409+
372410
describe('Bus api', () => {
373411
let createEventSpy;
374412
const sandbox = sinon.sandbox.create();

‎src/routes/phases/delete.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ module.exports = [
1616
const projectId = _.parseInt(req.params.projectId);
1717
const phaseId = _.parseInt(req.params.phaseId);
1818

19-
models.sequelize.transaction(() =>
19+
models.sequelize.transaction(transaction =>
2020
// soft delete the record
2121
models.ProjectPhase.findOne({
2222
where: {
@@ -32,9 +32,9 @@ module.exports = [
3232
err.status = 404;
3333
return Promise.reject(err);
3434
}
35-
return existing.update({ deletedBy: req.authUser.userId });
35+
return existing.update({ deletedBy: req.authUser.userId }, { transaction });
3636
})
37-
.then(entity => entity.destroy()))
37+
.then(entity => entity.destroy({ transaction })))
3838
.then((deleted) => {
3939
req.log.debug('deleted project phase', JSON.stringify(deleted, null, 2));
4040

‎src/routes/phases/get.js

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,7 @@ import util from '../../util';
55
import models from '../../models';
66

77
const permissions = tcMiddleware.permissions;
8-
const populateMemberDetails = async (phase, req) => {
9-
const members = _.map(phase.members, member => _.pick(member, 'userId'));
10-
try {
11-
const detailedMembers = await util.getObjectsWithMemberDetails(members, ['userId', 'handle', 'photoURL'], req);
12-
return _.assign(phase, { members: detailedMembers });
13-
} catch (err) {
14-
return _.assign(phase, { members });
15-
}
16-
};
8+
179
module.exports = [
1810
permissions('project.view'),
1911
(req, res, next) => {
@@ -60,14 +52,14 @@ module.exports = [
6052
err.status = 404;
6153
throw err;
6254
}
63-
return populateMemberDetails(phase.toJSON(), req)
55+
return util.populatePhasesWithMemberDetails(phase.toJSON(), req)
6456
.then(result => res.json(result));
6557
})
6658
.catch(err => next(err));
6759
}
6860
req.log.debug('phase found in ES');
6961
// eslint-disable-next-line no-underscore-dangle
70-
return populateMemberDetails(data[0].inner_hits.phases.hits.hits[0]._source, req)
62+
return util.populatePhasesWithMemberDetails(data[0].inner_hits.phases.hits.hits[0]._source, req)
7163
.then(phase => res.json(phase));
7264
})
7365
.catch(next);

‎src/routes/phases/list.js

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,19 +15,6 @@ const PHASE_ATTRIBUTES = _.keys(models.ProjectPhase.rawAttributes);
1515

1616
const permissions = tcMiddleware.permissions;
1717

18-
const populateMemberDetails = async (phases, req) => {
19-
let members = _.reduce(phases, (acc, phase) =>
20-
_.concat(acc, _.map(phase.members, member => _.pick(member, 'userId'))), []);
21-
members = _.uniqBy(members, 'userId');
22-
try {
23-
const detailedMembers = await util.getObjectsWithMemberDetails(members, ['userId', 'handle', 'photoURL'], req);
24-
return _.map(phases, phase =>
25-
_.assign(phase, { members: _.intersectionBy(detailedMembers, phase.members, 'userId') }));
26-
} catch (err) {
27-
return _.map(phases, phase =>
28-
_.assign(phase, { members: _.map(phase.members, member => _.pick(member, 'userId')) }));
29-
}
30-
};
3118
module.exports = [
3219
permissions('project.view'),
3320
(req, res, next) => {
@@ -71,7 +58,10 @@ module.exports = [
7158
}
7259

7360
phases = _.map(phases, phase => _.pick(phase, fields));
74-
return populateMemberDetails(phases, req)
61+
if (_.indexOf(fields, 'members') < 0) {
62+
return res.json(phases);
63+
}
64+
return util.populatePhasesWithMemberDetails(phases, req)
7565
.then(result => res.json(result));
7666
})
7767
.catch((err) => {
@@ -122,7 +112,10 @@ module.exports = [
122112
}
123113
phases = _.map(phases, phase => _.pick(phase, fields));
124114
// Write to response
125-
return populateMemberDetails(phases, req)
115+
if (_.indexOf(fields, 'members') < 0) {
116+
return res.json(phases);
117+
}
118+
return util.populatePhasesWithMemberDetails(phases, req)
126119
.then(result => res.json(result));
127120
});
128121
}

‎src/routes/phases/update.js

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import models from '../../models';
77
import util from '../../util';
88
import { EVENT, RESOURCES, ROUTES } from '../../constants';
99

10+
import updatePhaseMemberService from '../phaseMembers/updateService';
1011

1112
const permissions = tcMiddleware.permissions;
1213

@@ -24,17 +25,9 @@ const updateProjectPhaseValidation = {
2425
progress: Joi.number().min(0).optional(),
2526
details: Joi.any().optional(),
2627
order: Joi.number().integer().optional(),
28+
members: Joi.array().items(Joi.number().integer()).optional(),
2729
}).required(),
2830
};
29-
const populateMemberDetails = async (phase, req) => {
30-
const members = _.map(phase.members, member => _.pick(member, 'userId'));
31-
try {
32-
const detailedMembers = await util.getObjectsWithMemberDetails(members, ['userId', 'handle', 'photoURL'], req);
33-
return _.assign(phase, { members: detailedMembers });
34-
} catch (err) {
35-
return _.assign(phase, { members });
36-
}
37-
};
3831

3932
module.exports = [
4033
// validate request payload
@@ -52,7 +45,7 @@ module.exports = [
5245
let previousValue;
5346
let updated;
5447

55-
models.sequelize.transaction(() => models.ProjectPhase.findOne({
48+
models.sequelize.transaction(transaction => models.ProjectPhase.findOne({
5649
where: {
5750
id: phaseId,
5851
projectId,
@@ -88,35 +81,45 @@ module.exports = [
8881
err.status = 400;
8982
reject(err);
9083
} else {
91-
_.extend(existing, updatedProps);
92-
existing.save().then(accept).catch(reject);
84+
_.extend(existing, _.omit(updatedProps, 'members'));
85+
existing.save({ transaction }).then(accept).catch(reject);
9386
}
9487
}
9588
}))
9689
.then((updatedPhase) => {
97-
updated = updatedPhase;
90+
updated = updatedPhase.get({ plain: true });
91+
})
92+
.then(() => {
93+
if (_.isNil(updatedProps.members)) {
94+
return Promise.resolve();
95+
}
96+
97+
return updatePhaseMemberService(req.authUser, projectId, phaseId, updatedProps.members, transaction)
98+
.then(members => _.assign(updated, { members }));
9899
}),
99100
)
100101
.then(() => {
101102
req.log.debug('updated project phase', JSON.stringify(updated, null, 2));
102103

103-
const updatedValue = updated.get({ plain: true });
104-
105104
// emit event
106105
util.sendResourceToKafkaBus(
107106
req,
108107
EVENT.ROUTING_KEY.PROJECT_PHASE_UPDATED,
109108
RESOURCES.PHASE,
110-
updatedValue,
109+
updated,
111110
previousValue,
112111
ROUTES.PHASES.UPDATE);
112+
if (updated.members) {
113+
return util.populatePhasesWithMemberDetails(updated, req)
114+
.then(result => res.json(result));
115+
}
113116
return models.ProjectPhase.findOne({
114117
where: { id: phaseId, projectId },
115118
include: [{
116119
model: models.ProjectPhaseMember,
117120
as: 'members',
118121
}],
119-
}).then(phaseWithMembers => populateMemberDetails(phaseWithMembers.toJSON(), req)
122+
}).then(phaseWithMembers => util.populatePhasesWithMemberDetails(phaseWithMembers.toJSON(), req)
120123
.then(result => res.json(result)));
121124
})
122125
.catch(err => next(err));

‎src/routes/phases/update.spec.js

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,74 @@ describe('Project Phases', () => {
284284
});
285285
});
286286

287+
it('should return 200 with member details after updating members', (done) => {
288+
const bodyWithMembers = _.cloneDeep(updateBody);
289+
_.assign(bodyWithMembers, { members: [copilotUser.userId] });
290+
request(server)
291+
.patch(`/v5/projects/${projectId}/phases/${phaseId}`)
292+
.set({
293+
Authorization: `Bearer ${testUtil.jwts.admin}`,
294+
})
295+
.send(bodyWithMembers)
296+
.expect('Content-Type', /json/)
297+
.expect(200)
298+
.end((err, res) => {
299+
if (err) {
300+
done(err);
301+
} else {
302+
const resJson = res.body;
303+
validatePhase(resJson, bodyWithMembers);
304+
resJson.members.should.have.length(1);
305+
resJson.members[0].userId.should.eql(copilotUser.userId);
306+
done();
307+
}
308+
});
309+
});
310+
311+
it('should return 200 with existent member details vith valid payload without members', (done) => {
312+
models.ProjectPhaseMember.create({
313+
id: 1,
314+
userId: copilotUser.userId,
315+
phaseId,
316+
createdBy: 1,
317+
updatedBy: 1,
318+
}).then(() => {
319+
request(server)
320+
.patch(`/v5/projects/${projectId}/phases/${phaseId}`)
321+
.set({
322+
Authorization: `Bearer ${testUtil.jwts.admin}`,
323+
})
324+
.send(updateBody)
325+
.expect('Content-Type', /json/)
326+
.expect(200)
327+
.end((err, res) => {
328+
if (err) {
329+
done(err);
330+
} else {
331+
const resJson = res.body;
332+
validatePhase(resJson, updateBody);
333+
resJson.members.should.have.length(1);
334+
resJson.members[0].userId.should.eql(copilotUser.userId);
335+
done();
336+
}
337+
});
338+
});
339+
});
340+
341+
it('should return 400 if members property includes userId who is not a member of project', (done) => {
342+
const bodyWithMembers = _.cloneDeep(updateBody);
343+
_.assign(bodyWithMembers, { members: [999] });
344+
request(server)
345+
.patch(`/v5/projects/${projectId}/phases/${phaseId}`)
346+
.set({
347+
Authorization: `Bearer ${testUtil.jwts.admin}`,
348+
})
349+
.send(bodyWithMembers)
350+
.expect('Content-Type', /json/)
351+
.expect(400)
352+
.end(done);
353+
});
354+
287355
it('should return 403 if requested by manager which is not a member', (done) => {
288356
request(server)
289357
.patch(`/v5/projects/${projectId}/phases/${phaseId}`)

‎src/util.js

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -791,6 +791,38 @@ const projectServiceUtils = {
791791
});
792792
},
793793

794+
/**
795+
* Add member details to Project Phase objects
796+
*
797+
* @param {Array|Object} phases Array of phase object or single phase object
798+
* @param {Object} req The request object
799+
*
800+
* @return {Array|Object} Phase(s) with member details
801+
*/
802+
populatePhasesWithMemberDetails: async (phases, req) => {
803+
if (_.isArray(phases)) {
804+
let members = _.reduce(phases, (acc, phase) =>
805+
_.concat(acc, _.map(phase.members, member => _.pick(member, 'userId'))), []);
806+
members = _.uniqBy(members, 'userId');
807+
try {
808+
const detailedMembers = await util.getObjectsWithMemberDetails(members, ['userId', 'handle', 'photoURL'], req);
809+
return _.map(phases, phase =>
810+
_.assign(phase, { members: _.intersectionBy(detailedMembers, phase.members, 'userId') }));
811+
} catch (err) {
812+
return _.map(phases, phase =>
813+
_.assign(phase, { members: _.map(phase.members, member => _.pick(member, 'userId')) }));
814+
}
815+
} else {
816+
const members = _.map(phases.members, member => _.pick(member, 'userId'));
817+
try {
818+
const detailedMembers = await util.getObjectsWithMemberDetails(members, ['userId', 'handle', 'photoURL'], req);
819+
return _.assign(phases, { members: detailedMembers });
820+
} catch (err) {
821+
return _.assign(phases, { members });
822+
}
823+
}
824+
},
825+
794826
/**
795827
* Retrieve member details from userIds
796828
*/

0 commit comments

Comments
 (0)
Please sign in to comment.