From 7ac613571ae727a03bb7b664918c6217c5865b65 Mon Sep 17 00:00:00 2001 From: Maksym Mykhailenko Date: Wed, 13 Feb 2019 09:33:06 +0800 Subject: [PATCH 01/18] trigger redeployment --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index c9efc4e7..2b4a6450 100644 --- a/README.md +++ b/README.md @@ -5,6 +5,7 @@ Microservice to manage CRUD operations for all things Projects. ### Note : Steps mentioned below are best to our capability as guide for local deployment, however, we expect from contributor, being a developer, to resolve run-time issues (e.g. OS and node version issues etc), if any. ### Local Development + * We use docker-compose for running dependencies locally. Instructions for Docker compose setup - https://docs.docker.com/compose/install/ * Nodejs 8.9.4 - consider using [nvm](https://github.com/creationix/nvm) or equivalent to manage your node version * Install [libpg](https://www.npmjs.com/package/pg-native) From 1a679eb50ab1bbd6829f5cdf982b7ebe6050c151 Mon Sep 17 00:00:00 2001 From: Samir Date: Tue, 19 Feb 2019 01:20:44 +0100 Subject: [PATCH 02/18] account manager changes --- src/constants.js | 3 +- src/routes/projectMembers/create.js | 44 ++++++++++++++++++++++++----- src/routes/projectMembers/update.js | 2 +- 3 files changed, 40 insertions(+), 9 deletions(-) diff --git a/src/constants.js b/src/constants.js index 032e1e8d..8e01042c 100644 --- a/src/constants.js +++ b/src/constants.js @@ -18,6 +18,7 @@ export const PROJECT_MEMBER_ROLE = { OBSERVER: 'observer', CUSTOMER: 'customer', COPILOT: 'copilot', + ACCOUNT_MANAGER: 'account_manager', }; export const PROJECT_MEMBER_MANAGER_ROLES = [PROJECT_MEMBER_ROLE.MANAGER, PROJECT_MEMBER_ROLE.OBSERVER]; @@ -31,7 +32,7 @@ export const USER_ROLE = { export const ADMIN_ROLES = [USER_ROLE.CONNECT_ADMIN, USER_ROLE.TOPCODER_ADMIN]; -export const MANAGER_ROLES = [...ADMIN_ROLES, USER_ROLE.MANAGER]; +export const MANAGER_ROLES = [...ADMIN_ROLES, USER_ROLE.MANAGER, PROJECT_MEMBER_ROLE.ACCOUNT_MANAGER]; export const EVENT = { ROUTING_KEY: { diff --git a/src/routes/projectMembers/create.js b/src/routes/projectMembers/create.js index 96c34a85..43f754c7 100644 --- a/src/routes/projectMembers/create.js +++ b/src/routes/projectMembers/create.js @@ -1,9 +1,9 @@ - - import _ from 'lodash'; +import Joi from 'joi'; +import validate from 'express-validation'; import { middleware as tcMiddleware } from 'tc-core-library-js'; import util from '../../util'; -import { USER_ROLE, PROJECT_MEMBER_ROLE, MANAGER_ROLES, INVITE_STATUS } from '../../constants'; +import { INVITE_STATUS, MANAGER_ROLES, PROJECT_MEMBER_ROLE, USER_ROLE } from '../../constants'; import models from '../../models'; /** @@ -13,12 +13,38 @@ import models from '../../models'; */ const permissions = tcMiddleware.permissions; +const createProjectMemberValidations = { + body: { + param: Joi.object() + .keys({ + role: Joi.any() + .valid(PROJECT_MEMBER_ROLE.MANAGER, PROJECT_MEMBER_ROLE.ACCOUNT_MANAGER, PROJECT_MEMBER_ROLE.COPILOT), + }), + }, +}; + module.exports = [ // handles request validations + validate(createProjectMemberValidations), permissions('project.addMember'), (req, res, next) => { let targetRole; - if (util.hasRoles(req, [USER_ROLE.MANAGER])) { + if (_.get(req, 'body.param.role')) { + targetRole = _.get(req, 'body.param.role'); + + if ([PROJECT_MEMBER_ROLE.MANAGER, PROJECT_MEMBER_ROLE.ACCOUNT_MANAGER].includes(targetRole) && + !util.hasRoles(req, [USER_ROLE.MANAGER])) { + const err = new Error(`Only manager is able to join as ${targetRole}`); + err.status = 401; + return next(err); + } + + if (targetRole === PROJECT_MEMBER_ROLE.COPILOT && !util.hasRoles(req, [USER_ROLE.COPILOT])) { + const err = new Error(`Only copilot is able to join as ${targetRole}`); + err.status = 401; + return next(err); + } + } else if (util.hasRoles(req, [USER_ROLE.MANAGER])) { targetRole = PROJECT_MEMBER_ROLE.MANAGER; } else if (util.hasRoles(req, [USER_ROLE.COPILOT])) { targetRole = PROJECT_MEMBER_ROLE.COPILOT; @@ -60,13 +86,17 @@ module.exports = [ .then((_invite) => { invite = _invite; if (!invite) { - return res.status(201).json(util.wrapResponse(req.id, newMember, 1, 201)); + return res.status(201) + .json(util.wrapResponse(req.id, newMember, 1, 201)); } return invite.update({ status: INVITE_STATUS.ACCEPTED, - }).then(() => res.status(201).json(util.wrapResponse(req.id, newMember, 1, 201))); + }) + .then(() => res.status(201) + .json(util.wrapResponse(req.id, newMember, 1, 201))); }); }); - }).catch(err => next(err)); + }) + .catch(err => next(err)); }, ]; diff --git a/src/routes/projectMembers/update.js b/src/routes/projectMembers/update.js index 97efadb1..7001133f 100644 --- a/src/routes/projectMembers/update.js +++ b/src/routes/projectMembers/update.js @@ -17,7 +17,7 @@ const updateProjectMemberValdiations = { param: Joi.object().keys({ isPrimary: Joi.boolean(), role: Joi.any().valid(PROJECT_MEMBER_ROLE.CUSTOMER, PROJECT_MEMBER_ROLE.MANAGER, - PROJECT_MEMBER_ROLE.COPILOT, PROJECT_MEMBER_ROLE.OBSERVER).required(), + PROJECT_MEMBER_ROLE.ACCOUNT_MANAGER, PROJECT_MEMBER_ROLE.COPILOT, PROJECT_MEMBER_ROLE.OBSERVER).required(), }), }, }; From 85ab047beb628d9ad6bf0c4cc6a3d017b242c489 Mon Sep 17 00:00:00 2001 From: Maksym Mykhailenko Date: Tue, 19 Feb 2019 17:48:00 +0800 Subject: [PATCH 03/18] winning submission for challenge 30083089 - Topcoder Connect - Filter project table by columns, fix for issue #245 --- migrations/elasticsearch_sync.js | 3 +- postman.json | 116 +++++++++++++++++++- src/routes/projects/list.js | 142 ++++++++++++++++++++---- src/routes/projects/list.spec.js | 182 ++++++++++++++++++++++++++++++- src/util.js | 2 +- swagger.yaml | 6 +- 6 files changed, 425 insertions(+), 26 deletions(-) mode change 100755 => 100644 swagger.yaml diff --git a/migrations/elasticsearch_sync.js b/migrations/elasticsearch_sync.js index 349be0b6..b116082d 100644 --- a/migrations/elasticsearch_sync.js +++ b/migrations/elasticsearch_sync.js @@ -209,7 +209,8 @@ function getRequestBody(indexName) { }, }, id: { - type: 'long', + type: 'string', + index: 'not_analyzed', }, members: { type: 'nested', diff --git a/postman.json b/postman.json index 999689af..86172fb6 100644 --- a/postman.json +++ b/postman.json @@ -1485,6 +1485,120 @@ }, "response": [] }, + { + "name": "List projects with filters - name, code, customer, manager", + "request": { + "url": { + "raw": "{{api-url}}/v4/projects?filter=id%3D1*%26name%3Dtes*%26code=test*%26customer%3DDiya*%26manager=first*", + "host": [ + "{{api-url}}" + ], + "path": [ + "v4", + "projects" + ], + "query": [ + { + "key": "filter", + "value": "id%3D1*%26name%3Dtes*%26code=test*%26customer%3DDiya*%26manager=first*", + "equals": true, + "description": "" + } + ], + "variable": [] + }, + "method": "GET", + "header": [ + { + "key": "Authorization", + "value": "Bearer {{jwt-token}}", + "description": "" + } + ], + "body": { + "mode": "raw", + "raw": "" + }, + "description": "List all the project with filters applied. The filter string should be url encoded. Default limit and offset is applicable" + }, + "response": [] + }, + { + "name": "List projects with filters - id", + "request": { + "url": { + "raw": "{{api-url}}/v4/projects?filter=id%3D2", + "host": [ + "{{api-url}}" + ], + "path": [ + "v4", + "projects" + ], + "query": [ + { + "key": "filter", + "value": "id%3D2", + "equals": true, + "description": "" + } + ], + "variable": [] + }, + "method": "GET", + "header": [ + { + "key": "Authorization", + "value": "Bearer {{jwt-token}}", + "description": "" + } + ], + "body": { + "mode": "raw", + "raw": "" + }, + "description": "List all the project with filters applied. The filter string should be url encoded. Default limit and offset is applicable" + }, + "response": [] + }, + { + "name": "List projects with filters - code", + "request": { + "url": { + "raw": "{{api-url}}/v4/projects?filter=code%3Dtest*", + "host": [ + "{{api-url}}" + ], + "path": [ + "v4", + "projects" + ], + "query": [ + { + "key": "filter", + "value": "code%3Dtest*", + "equals": true, + "description": "" + } + ], + "variable": [] + }, + "method": "GET", + "header": [ + { + "key": "Authorization", + "value": "Bearer {{jwt-token}}", + "description": "" + } + ], + "body": { + "mode": "raw", + "raw": "" + }, + "description": "List all the project with filters applied. The filter string should be url encoded. Default limit and offset is applicable" + }, + "response": [] + }, { "name": "List projects with sort applied", "request": { @@ -5395,4 +5509,4 @@ ] } ] -} \ No newline at end of file +} diff --git a/src/routes/projects/list.js b/src/routes/projects/list.js index 1e490f57..691d7d81 100755 --- a/src/routes/projects/list.js +++ b/src/routes/projects/list.js @@ -102,6 +102,87 @@ const buildEsFullTextQuery = (keyword, matchType, singleFieldName) => { }; }; +/** + * Build ES query search request body based on value, keyword, matchType and fieldName + * + * @param {String} value the value to build request body for + * @param {String} keyword the keyword to query + * @param {String} matchType wildcard match or exact match + * @param {Array} fieldName the fieldName + * @return {Object} search request body that can be passed to .search api call + */ +const buildEsQueryWithFilter = (value, keyword, matchType, fieldName) => { + let should = []; + if (value !== 'details' && value !== 'customer' && value !== 'manager') { + should = _.concat(should, { + query_string: { + query: keyword, + analyze_wildcard: (matchType === MATCH_TYPE_WILDCARD), + fields: fieldName, + }, + }); + } + + if (value === 'details') { + should = _.concat(should, { + nested: { + path: 'details', + query: { + nested: { + path: 'details.utm', + query: { + query_string: { + query: keyword, + analyze_wildcard: (matchType === MATCH_TYPE_WILDCARD), + fields: fieldName, + }, + }, + }, + }, + }, + }); + } + + if (value === 'customer' || value === 'manager') { + should = _.concat(should, { + nested: { + path: 'members', + query: { + bool: { + must: [ + { match: { 'members.role': value } }, + { + query_string: { + query: keyword, + analyze_wildcard: (matchType === MATCH_TYPE_WILDCARD), + fields: fieldName, + }, + }, + ], + }, + }, + }, + }); + } + + return should; +}; + +/** + * Prepare search request body based on wildcard query + * + * @param {String} value the value to build request body for + * @param {String} keyword the keyword to query + * @param {Array} fieldName the fieldName + * @return {Object} search request body that can be passed to .search api call + */ +const setFilter = (value, keyword, fieldName) => { + if (keyword.indexOf('*') > -1) { + return buildEsQueryWithFilter(value, keyword, MATCH_TYPE_WILDCARD, fieldName); + } + return buildEsQueryWithFilter(value, keyword, MATCH_TYPE_EXACT_PHRASE, fieldName); +}; + /** * Parse the ES search criteria and prepare search request body * @@ -152,6 +233,7 @@ const parseElasticSearchCriteria = (criteria, fields, order) => { } // prepare the elasticsearch filter criteria const boolQuery = []; + let mustQuery = []; let fullTextQuery; if (_.has(criteria, 'filters.id.$in')) { boolQuery.push({ @@ -159,6 +241,34 @@ const parseElasticSearchCriteria = (criteria, fields, order) => { values: criteria.filters.id.$in, }, }); + } else if (_.has(criteria, 'filters.id') && criteria.filters.id.indexOf('*') > -1) { + mustQuery = _.concat(mustQuery, buildEsQueryWithFilter('id', criteria.filters.id, MATCH_TYPE_WILDCARD, ['id'])); + } else if (_.has(criteria, 'filters.id')) { + boolQuery.push({ + term: { + id: criteria.filters.id, + }, + }); + } + + if (_.has(criteria, 'filters.name')) { + mustQuery = _.concat(mustQuery, setFilter('name', criteria.filters.name, ['name'])); + } + + if (_.has(criteria, 'filters.code')) { + mustQuery = _.concat(mustQuery, setFilter('details', criteria.filters.code, ['details.utm.code'])); + } + + if (_.has(criteria, 'filters.customer')) { + mustQuery = _.concat(mustQuery, setFilter('customer', + criteria.filters.customer, + ['members.firstName', 'members.lastName'])); + } + + if (_.has(criteria, 'filters.manager')) { + mustQuery = _.concat(mustQuery, setFilter('manager', + criteria.filters.manager, + ['members.firstName', 'members.lastName'])); } if (_.has(criteria, 'filters.status.$in')) { @@ -177,21 +287,6 @@ const parseElasticSearchCriteria = (criteria, fields, order) => { }); } - if (_.has(criteria, 'filters.type.$in')) { - // type is an array - boolQuery.push({ - terms: { - type: criteria.filters.type.$in, - }, - }); - } else if (_.has(criteria, 'filters.type')) { - // type is simple string - boolQuery.push({ - term: { - type: criteria.filters.type, - }, - }); - } if (_.has(criteria, 'filters.keyword')) { // keyword is a full text search // escape special fields from keyword search @@ -222,7 +317,7 @@ const parseElasticSearchCriteria = (criteria, fields, order) => { if (!keyword) { // Not a specific field search nor an exact phrase search, do a wildcard match - keyword = escapeEsKeyword(criteria.filters.keyword); + keyword = criteria.filters.keyword; matchType = MATCH_TYPE_WILDCARD; } @@ -234,6 +329,12 @@ const parseElasticSearchCriteria = (criteria, fields, order) => { filter: boolQuery, }; } + + if (mustQuery.length > 0) { + body.query.bool = _.merge(body.query.bool, { + must: mustQuery, + }); + } if (fullTextQuery) { body.query = _.merge(body.query, fullTextQuery); if (body.query.bool) { @@ -241,10 +342,9 @@ const parseElasticSearchCriteria = (criteria, fields, order) => { } } - if (fullTextQuery || boolQuery.length > 0) { + if (fullTextQuery || boolQuery.length > 0 || mustQuery.length > 0) { searchCriteria.body = body; } - return searchCriteria; }; @@ -267,8 +367,7 @@ const retrieveProjects = (req, criteria, sort, ffields) => { fields.projects.push('id'); } - const searchCriteria = parseElasticSearchCriteria(criteria, fields, order); - + const searchCriteria = parseElasticSearchCriteria(criteria, fields, order) || {}; return new Promise((accept, reject) => { const es = util.getElasticSearchClient(); es.search(searchCriteria).then((docs) => { @@ -300,7 +399,8 @@ module.exports = [ 'name', 'name asc', 'name desc', 'type', 'type asc', 'type desc', ]; - if (!util.isValidFilter(filters, ['id', 'status', 'type', 'memberOnly', 'keyword']) || + if (!util.isValidFilter(filters, + ['id', 'status', 'memberOnly', 'keyword', 'name', 'code', 'customer', 'manager']) || (sort && _.indexOf(sortableProps, sort) < 0)) { return util.handleError('Invalid filters or sort', null, req, next); } diff --git a/src/routes/projects/list.spec.js b/src/routes/projects/list.spec.js index ef1bb7c6..8eee10df 100644 --- a/src/routes/projects/list.spec.js +++ b/src/routes/projects/list.spec.js @@ -37,6 +37,8 @@ const data = [ userId: 40051331, projectId: 1, role: 'customer', + firstName: 'Firstname', + lastName: 'Lastname', handle: 'test_tourist_handle', isPrimary: true, createdBy: 1, @@ -101,6 +103,19 @@ const data = [ updatedBy: 1, lastActivityAt: 3, lastActivityUserId: '1', + members: [{ + id: 5, + userId: 40051334, + projectId: 2, + role: 'manager', + firstName: 'first', + lastName: 'last', + handle: 'manager_handle', + isPrimary: true, + createdBy: 1, + updatedBy: 1, + }, + ], }, ]; @@ -193,7 +208,18 @@ describe('LIST Project', () => { lastActivityUserId: '1', }).then((p) => { project3 = p; - return Promise.resolve(); + // create members + return models.ProjectMember.create({ + userId: 40051334, + projectId: project3.id, + role: 'manager', + firstName: 'first', + lastName: 'last', + handle: 'manager_handle', + isPrimary: true, + createdBy: 1, + updatedBy: 1, + }); }); return Promise.all([p1, p2, p3]).then(() => { @@ -487,6 +513,160 @@ describe('LIST Project', () => { }); }); + it('should return project that match when filtering by id', (done) => { + request(server) + .get('/v4/projects/?filter=id%3D1*') + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body.result.content; + should.exist(resJson); + resJson.should.have.lengthOf(1); + resJson[0].id.should.equal(1); + resJson[0].name.should.equal('test1'); + done(); + } + }); + }); + + it('should return project that match when filtering by name', (done) => { + request(server) + .get('/v4/projects/?filter=name%3Dtest1') + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body.result.content; + should.exist(resJson); + resJson.should.have.lengthOf(1); + resJson[0].name.should.equal('test1'); + done(); + } + }); + }); + + it('should return project that match when filtering by name\'s substring', (done) => { + request(server) + .get('/v4/projects/?filter=name%3D*st1') + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body.result.content; + should.exist(resJson); + resJson.should.have.lengthOf(1); + resJson[0].name.should.equal('test1'); + done(); + } + }); + }); + + it('should return all projects that match when filtering by details code', (done) => { + request(server) + .get('/v4/projects/?filter=code%3Dcode1') + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body.result.content; + should.exist(resJson); + resJson.should.have.lengthOf(1); + resJson[0].name.should.equal('test1'); + resJson[0].details.utm.code.should.equal('code1'); + done(); + } + }); + }); + + it('should return all projects that match when filtering by details code\'s substring', (done) => { + request(server) + .get('/v4/projects/?filter=code%3D*de1') + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body.result.content; + should.exist(resJson); + resJson.should.have.lengthOf(1); + resJson[0].name.should.equal('test1'); + resJson[0].details.utm.code.should.equal('code1'); + done(); + } + }); + }); + + it('should return all projects that match when filtering by customer', (done) => { + request(server) + .get('/v4/projects/?filter=customer%3Dfirst*') + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body.result.content; + should.exist(resJson); + resJson.should.have.lengthOf(1); + resJson[0].name.should.equal('test1'); + resJson[0].members.should.have.deep.property('[0].role', 'customer'); + resJson[0].members[0].userId.should.equal(40051331); + done(); + } + }); + }); + + it('should return all projects that match when filtering by manager', (done) => { + request(server) + .get('/v4/projects/?filter=manager%3D*ast') + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body.result.content; + should.exist(resJson); + resJson.should.have.lengthOf(1); + resJson[0].name.should.equal('test3'); + resJson[0].members.should.have.deep.property('[0].role', 'manager'); + resJson[0].members[0].userId.should.equal(40051334); + done(); + } + }); + }); + it('should return list of projects ordered ascending by lastActivityAt when sort column is "lastActivityAt"', (done) => { request(server) .get('/v4/projects/?sort=lastActivityAt') diff --git a/src/util.js b/src/util.js index c1852c79..5fe2b511 100644 --- a/src/util.js +++ b/src/util.js @@ -180,7 +180,7 @@ _.assignIn(util, { } return val; }); - if (queryFilter.id) { + if (queryFilter.id && queryFilter.id.$in) { queryFilter.id.$in = _.map(queryFilter.id.$in, _.parseInt); } return queryFilter; diff --git a/swagger.yaml b/swagger.yaml old mode 100755 new mode 100644 index 2d7e7c8b..eb8d063f --- a/swagger.yaml +++ b/swagger.yaml @@ -55,6 +55,10 @@ paths: - type - memberOnly - keyword + - name + - code + - customer + - manager - name: sort required: false description: | @@ -4129,4 +4133,4 @@ definitions: metadata: $ref: "#/definitions/ResponseMetadata" content: - $ref: "#/definitions/ProjectMemberInvite" + $ref: "#/definitions/ProjectMemberInvite" \ No newline at end of file From 281af4a9ca524d59e82ade4c039eb3ddfa1be69f Mon Sep 17 00:00:00 2001 From: Maksym Mykhailenko Date: Tue, 19 Feb 2019 19:11:55 +0800 Subject: [PATCH 04/18] remove filtering by id substring (only exact match support for now) --- migrations/elasticsearch_sync.js | 3 +- postman.json | 94 ++++++++++++++------------------ src/routes/projects/list.js | 19 ++++++- src/routes/projects/list.spec.js | 4 +- 4 files changed, 60 insertions(+), 60 deletions(-) diff --git a/migrations/elasticsearch_sync.js b/migrations/elasticsearch_sync.js index b116082d..349be0b6 100644 --- a/migrations/elasticsearch_sync.js +++ b/migrations/elasticsearch_sync.js @@ -209,8 +209,7 @@ function getRequestBody(indexName) { }, }, id: { - type: 'string', - index: 'not_analyzed', + type: 'long', }, members: { type: 'nested', diff --git a/postman.json b/postman.json index 86172fb6..96995389 100644 --- a/postman.json +++ b/postman.json @@ -1,6 +1,6 @@ { "info": { - "_postman_id": "97085cd7-b298-4f1c-9629-24af14ff5f13", + "_postman_id": "4fc2b7cf-404a-4fd7-b6d2-4828a3994859", "name": "tc-project-service", "schema": "https://schema.getpostman.com/json/collection/v2.1.0/collection.json" }, @@ -1452,7 +1452,7 @@ "response": [] }, { - "name": "List projects with filters applied", + "name": "List projects with filters - type (exact)", "request": { "method": "GET", "header": [ @@ -1466,7 +1466,7 @@ "raw": "" }, "url": { - "raw": "{{api-url}}/v4/projects?filter=type%3Dgeneric", + "raw": "{{api-url}}/v4/projects?filter=type%3Dapp", "host": [ "{{api-url}}" ], @@ -1477,7 +1477,7 @@ "query": [ { "key": "filter", - "value": "type%3Dgeneric" + "value": "type%3Dapp" } ] }, @@ -1486,10 +1486,21 @@ "response": [] }, { - "name": "List projects with filters - name, code, customer, manager", + "name": "List projects with filters - id (exact)", "request": { + "method": "GET", + "header": [ + { + "key": "Authorization", + "value": "Bearer {{jwt-token}}" + } + ], + "body": { + "mode": "raw", + "raw": "" + }, "url": { - "raw": "{{api-url}}/v4/projects?filter=id%3D1*%26name%3Dtes*%26code=test*%26customer%3DDiya*%26manager=first*", + "raw": "{{api-url}}/v4/projects?filter=id%3D1", "host": [ "{{api-url}}" ], @@ -1500,34 +1511,30 @@ "query": [ { "key": "filter", - "value": "id%3D1*%26name%3Dtes*%26code=test*%26customer%3DDiya*%26manager=first*", - "equals": true, - "description": "" + "value": "id%3D1" } - ], - "variable": [] + ] }, + "description": "List all the project with filters applied. The filter string should be url encoded. Default limit and offset is applicable" + }, + "response": [] + }, + { + "name": "List projects with filters - name, code, customer, manager", + "request": { "method": "GET", "header": [ { "key": "Authorization", - "value": "Bearer {{jwt-token}}", - "description": "" + "value": "Bearer {{jwt-token}}" } ], "body": { "mode": "raw", "raw": "" }, - "description": "List all the project with filters applied. The filter string should be url encoded. Default limit and offset is applicable" - }, - "response": [] - }, - { - "name": "List projects with filters - id", - "request": { "url": { - "raw": "{{api-url}}/v4/projects?filter=id%3D2", + "raw": "{{api-url}}/v4/projects?filter=id%3D1*%26name%3Dtes*%26code=test*%26customer%3DDiya*%26manager=first*", "host": [ "{{api-url}}" ], @@ -1538,32 +1545,28 @@ "query": [ { "key": "filter", - "value": "id%3D2", - "equals": true, - "description": "" + "value": "id%3D1*%26name%3Dtes*%26code=test*%26customer%3DDiya*%26manager=first*" } - ], - "variable": [] + ] }, + "description": "List all the project with filters applied. The filter string should be url encoded. Default limit and offset is applicable" + }, + "response": [] + }, + { + "name": "List projects with filters - code", + "request": { "method": "GET", "header": [ { "key": "Authorization", - "value": "Bearer {{jwt-token}}", - "description": "" + "value": "Bearer {{jwt-token}}" } ], "body": { "mode": "raw", "raw": "" }, - "description": "List all the project with filters applied. The filter string should be url encoded. Default limit and offset is applicable" - }, - "response": [] - }, - { - "name": "List projects with filters - code", - "request": { "url": { "raw": "{{api-url}}/v4/projects?filter=code%3Dtest*", "host": [ @@ -1576,24 +1579,9 @@ "query": [ { "key": "filter", - "value": "code%3Dtest*", - "equals": true, - "description": "" + "value": "code%3Dtest*" } - ], - "variable": [] - }, - "method": "GET", - "header": [ - { - "key": "Authorization", - "value": "Bearer {{jwt-token}}", - "description": "" - } - ], - "body": { - "mode": "raw", - "raw": "" + ] }, "description": "List all the project with filters applied. The filter string should be url encoded. Default limit and offset is applicable" }, @@ -5509,4 +5497,4 @@ ] } ] -} +} \ No newline at end of file diff --git a/src/routes/projects/list.js b/src/routes/projects/list.js index 691d7d81..4b841f60 100755 --- a/src/routes/projects/list.js +++ b/src/routes/projects/list.js @@ -241,8 +241,6 @@ const parseElasticSearchCriteria = (criteria, fields, order) => { values: criteria.filters.id.$in, }, }); - } else if (_.has(criteria, 'filters.id') && criteria.filters.id.indexOf('*') > -1) { - mustQuery = _.concat(mustQuery, buildEsQueryWithFilter('id', criteria.filters.id, MATCH_TYPE_WILDCARD, ['id'])); } else if (_.has(criteria, 'filters.id')) { boolQuery.push({ term: { @@ -287,6 +285,21 @@ const parseElasticSearchCriteria = (criteria, fields, order) => { }); } + if (_.has(criteria, 'filters.type.$in')) { + // type is an array + boolQuery.push({ + terms: { + type: criteria.filters.type.$in, + }, + }); + } else if (_.has(criteria, 'filters.type')) { + // type is simple string + boolQuery.push({ + term: { + type: criteria.filters.type, + }, + }); + } if (_.has(criteria, 'filters.keyword')) { // keyword is a full text search // escape special fields from keyword search @@ -400,7 +413,7 @@ module.exports = [ 'type', 'type asc', 'type desc', ]; if (!util.isValidFilter(filters, - ['id', 'status', 'memberOnly', 'keyword', 'name', 'code', 'customer', 'manager']) || + ['id', 'status', 'memberOnly', 'keyword', 'type', 'name', 'code', 'customer', 'manager']) || (sort && _.indexOf(sortableProps, sort) < 0)) { return util.handleError('Invalid filters or sort', null, req, next); } diff --git a/src/routes/projects/list.spec.js b/src/routes/projects/list.spec.js index 8eee10df..dfecec90 100644 --- a/src/routes/projects/list.spec.js +++ b/src/routes/projects/list.spec.js @@ -513,9 +513,9 @@ describe('LIST Project', () => { }); }); - it('should return project that match when filtering by id', (done) => { + it('should return project that match when filtering by id (exact)', (done) => { request(server) - .get('/v4/projects/?filter=id%3D1*') + .get('/v4/projects/?filter=id%3D1') .set({ Authorization: `Bearer ${testUtil.jwts.admin}`, }) From db19f455b14ac5ce3a6e118623b77f36bdfb7d42 Mon Sep 17 00:00:00 2001 From: Samir Date: Wed, 20 Feb 2019 09:20:07 +0100 Subject: [PATCH 05/18] update permissions for admin --- src/routes/projectMembers/create.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/routes/projectMembers/create.js b/src/routes/projectMembers/create.js index 43f754c7..40432300 100644 --- a/src/routes/projectMembers/create.js +++ b/src/routes/projectMembers/create.js @@ -44,9 +44,9 @@ module.exports = [ err.status = 401; return next(err); } - } else if (util.hasRoles(req, [USER_ROLE.MANAGER])) { + } else if (util.hasRoles(req, [USER_ROLE.MANAGER, USER_ROLE.CONNECT_ADMIN])) { targetRole = PROJECT_MEMBER_ROLE.MANAGER; - } else if (util.hasRoles(req, [USER_ROLE.COPILOT])) { + } else if (util.hasRoles(req, [USER_ROLE.COPILOT, USER_ROLE.CONNECT_ADMIN])) { targetRole = PROJECT_MEMBER_ROLE.COPILOT; } else { const err = new Error('Only copilot or manager is able to call this endpoint'); From 8e2dc03a69c3ca0669130949097a96b90162f6c3 Mon Sep 17 00:00:00 2001 From: Samir Date: Thu, 21 Feb 2019 10:57:28 +0100 Subject: [PATCH 06/18] account manager topcoder role --- src/constants.js | 1 + src/permissions/project.view.js | 1 + src/routes/projectMembers/create.js | 11 ++++++++++- src/routes/projects/list.js | 3 ++- 4 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/constants.js b/src/constants.js index 8e01042c..f4be9be5 100644 --- a/src/constants.js +++ b/src/constants.js @@ -26,6 +26,7 @@ export const PROJECT_MEMBER_MANAGER_ROLES = [PROJECT_MEMBER_ROLE.MANAGER, PROJEC export const USER_ROLE = { TOPCODER_ADMIN: 'administrator', MANAGER: 'Connect Manager', + TOPCODER_ACCOUNT_MANAGER: 'Connect Account Manager', COPILOT: 'Connect Copilot', CONNECT_ADMIN: 'Connect Admin', }; diff --git a/src/permissions/project.view.js b/src/permissions/project.view.js index b82cd1dc..b32aec38 100644 --- a/src/permissions/project.view.js +++ b/src/permissions/project.view.js @@ -22,6 +22,7 @@ module.exports = freq => new Promise((resolve, reject) => { // check if auth user has acecss to this project const hasAccess = util.hasAdminRole(req) || util.hasRole(req, USER_ROLE.MANAGER) + || util.hasRole(req, USER_ROLE.ACCOUNT_MANAGER) || !_.isUndefined(_.find(members, m => m.userId === currentUserId)); // if user is co-pilot and the project doesn't have any copilots then diff --git a/src/routes/projectMembers/create.js b/src/routes/projectMembers/create.js index 40432300..201b4ece 100644 --- a/src/routes/projectMembers/create.js +++ b/src/routes/projectMembers/create.js @@ -32,13 +32,20 @@ module.exports = [ if (_.get(req, 'body.param.role')) { targetRole = _.get(req, 'body.param.role'); - if ([PROJECT_MEMBER_ROLE.MANAGER, PROJECT_MEMBER_ROLE.ACCOUNT_MANAGER].includes(targetRole) && + if (PROJECT_MEMBER_ROLE.MANAGER === targetRole && !util.hasRoles(req, [USER_ROLE.MANAGER])) { const err = new Error(`Only manager is able to join as ${targetRole}`); err.status = 401; return next(err); } + if (PROJECT_MEMBER_ROLE.ACCOUNT_MANAGER === targetRole && + !util.hasRoles(req, [USER_ROLE.MANAGER, USER_ROLE.ACCOUNT_MANAGER])) { + const err = new Error(`Only manager or account manager is able to join as ${targetRole}`); + err.status = 401; + return next(err); + } + if (targetRole === PROJECT_MEMBER_ROLE.COPILOT && !util.hasRoles(req, [USER_ROLE.COPILOT])) { const err = new Error(`Only copilot is able to join as ${targetRole}`); err.status = 401; @@ -46,6 +53,8 @@ module.exports = [ } } else if (util.hasRoles(req, [USER_ROLE.MANAGER, USER_ROLE.CONNECT_ADMIN])) { targetRole = PROJECT_MEMBER_ROLE.MANAGER; + } else if (util.hasRoles(req, [USER_ROLE.ACCOUNT_MANAGER])) { + targetRole = PROJECT_MEMBER_ROLE.ACCOUNT_MANAGER; } else if (util.hasRoles(req, [USER_ROLE.COPILOT, USER_ROLE.CONNECT_ADMIN])) { targetRole = PROJECT_MEMBER_ROLE.COPILOT; } else { diff --git a/src/routes/projects/list.js b/src/routes/projects/list.js index 4b841f60..cc58465a 100755 --- a/src/routes/projects/list.js +++ b/src/routes/projects/list.js @@ -430,7 +430,8 @@ module.exports = [ if (!memberOnly && (util.hasAdminRole(req) - || util.hasRole(req, USER_ROLE.MANAGER))) { + || util.hasRole(req, USER_ROLE.MANAGER) + || util.hasRole(req, USER_ROLE.ACCOUNT_MANAGER))) { // admins & topcoder managers can see all projects return retrieveProjects(req, criteria, sort, req.query.fields) .then(result => res.json(util.wrapResponse(req.id, result.rows, result.count))) From 601543126490823ab905dbc2bd85e1497c53b28d Mon Sep 17 00:00:00 2001 From: Samir Date: Thu, 21 Feb 2019 11:45:56 +0100 Subject: [PATCH 07/18] update role name --- src/permissions/project.view.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/permissions/project.view.js b/src/permissions/project.view.js index b32aec38..5fb10ff5 100644 --- a/src/permissions/project.view.js +++ b/src/permissions/project.view.js @@ -22,7 +22,7 @@ module.exports = freq => new Promise((resolve, reject) => { // check if auth user has acecss to this project const hasAccess = util.hasAdminRole(req) || util.hasRole(req, USER_ROLE.MANAGER) - || util.hasRole(req, USER_ROLE.ACCOUNT_MANAGER) + || util.hasRole(req, USER_ROLE.TOPCODER_ACCOUNT_MANAGER) || !_.isUndefined(_.find(members, m => m.userId === currentUserId)); // if user is co-pilot and the project doesn't have any copilots then From 85db099aaf78daa47f357c1b46dfb65577c8916e Mon Sep 17 00:00:00 2001 From: gondzo Date: Thu, 21 Feb 2019 12:03:59 +0100 Subject: [PATCH 08/18] Update list.js --- src/routes/projects/list.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/routes/projects/list.js b/src/routes/projects/list.js index cc58465a..faca2fb5 100755 --- a/src/routes/projects/list.js +++ b/src/routes/projects/list.js @@ -431,7 +431,7 @@ module.exports = [ if (!memberOnly && (util.hasAdminRole(req) || util.hasRole(req, USER_ROLE.MANAGER) - || util.hasRole(req, USER_ROLE.ACCOUNT_MANAGER))) { + || util.hasRole(req, USER_ROLE.TOPCODER_ACCOUNT_MANAGER))) { // admins & topcoder managers can see all projects return retrieveProjects(req, criteria, sort, req.query.fields) .then(result => res.json(util.wrapResponse(req.id, result.rows, result.count))) From 721d68e6f562da2b15169055ffade54672bf2759 Mon Sep 17 00:00:00 2001 From: gondzo Date: Thu, 21 Feb 2019 12:47:49 +0100 Subject: [PATCH 09/18] Update create.js --- src/routes/projectMembers/create.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/routes/projectMembers/create.js b/src/routes/projectMembers/create.js index 201b4ece..56fb6986 100644 --- a/src/routes/projectMembers/create.js +++ b/src/routes/projectMembers/create.js @@ -40,7 +40,7 @@ module.exports = [ } if (PROJECT_MEMBER_ROLE.ACCOUNT_MANAGER === targetRole && - !util.hasRoles(req, [USER_ROLE.MANAGER, USER_ROLE.ACCOUNT_MANAGER])) { + !util.hasRoles(req, [USER_ROLE.MANAGER, USER_ROLE.TOPCODER_ACCOUNT_MANAGER])) { const err = new Error(`Only manager or account manager is able to join as ${targetRole}`); err.status = 401; return next(err); @@ -54,7 +54,7 @@ module.exports = [ } else if (util.hasRoles(req, [USER_ROLE.MANAGER, USER_ROLE.CONNECT_ADMIN])) { targetRole = PROJECT_MEMBER_ROLE.MANAGER; } else if (util.hasRoles(req, [USER_ROLE.ACCOUNT_MANAGER])) { - targetRole = PROJECT_MEMBER_ROLE.ACCOUNT_MANAGER; + targetRole = PROJECT_MEMBER_ROLE.TOPCODER_ACCOUNT_MANAGER; } else if (util.hasRoles(req, [USER_ROLE.COPILOT, USER_ROLE.CONNECT_ADMIN])) { targetRole = PROJECT_MEMBER_ROLE.COPILOT; } else { From 42a788e6bddbd3e455614ab74751aecebe5d50ad Mon Sep 17 00:00:00 2001 From: gondzo Date: Thu, 21 Feb 2019 13:02:13 +0100 Subject: [PATCH 10/18] Update create.js --- src/routes/projectMembers/create.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/routes/projectMembers/create.js b/src/routes/projectMembers/create.js index 56fb6986..f3a1fc0c 100644 --- a/src/routes/projectMembers/create.js +++ b/src/routes/projectMembers/create.js @@ -53,8 +53,8 @@ module.exports = [ } } else if (util.hasRoles(req, [USER_ROLE.MANAGER, USER_ROLE.CONNECT_ADMIN])) { targetRole = PROJECT_MEMBER_ROLE.MANAGER; - } else if (util.hasRoles(req, [USER_ROLE.ACCOUNT_MANAGER])) { - targetRole = PROJECT_MEMBER_ROLE.TOPCODER_ACCOUNT_MANAGER; + } else if (util.hasRoles(req, [USER_ROLE.TOPCODER_ACCOUNT_MANAGER])) { + targetRole = PROJECT_MEMBER_ROLE.ACCOUNT_MANAGER; } else if (util.hasRoles(req, [USER_ROLE.COPILOT, USER_ROLE.CONNECT_ADMIN])) { targetRole = PROJECT_MEMBER_ROLE.COPILOT; } else { From b1737e5d5c838211b08487398f33fb578306c256 Mon Sep 17 00:00:00 2001 From: gondzo Date: Thu, 21 Feb 2019 16:24:34 +0100 Subject: [PATCH 11/18] Copilot workflow changes (#254) * copilot workflow changes * Lint-fix * update events naming --- src/constants.js | 4 ++ src/events/busApi.js | 68 ++++++++++++++----- src/routes/projectMemberInvites/create.js | 2 +- src/routes/projectMemberInvites/update.js | 14 ++-- .../projectMemberInvites/update.spec.js | 64 ++++++++++++++++- 5 files changed, 128 insertions(+), 24 deletions(-) diff --git a/src/constants.js b/src/constants.js index f4be9be5..9aa3a803 100644 --- a/src/constants.js +++ b/src/constants.js @@ -29,6 +29,7 @@ export const USER_ROLE = { TOPCODER_ACCOUNT_MANAGER: 'Connect Account Manager', COPILOT: 'Connect Copilot', CONNECT_ADMIN: 'Connect Admin', + COPILOT_MANAGER: 'Connect Copilot Manager', }; export const ADMIN_ROLES = [USER_ROLE.CONNECT_ADMIN, USER_ROLE.TOPCODER_ADMIN]; @@ -132,7 +133,10 @@ export const BUS_API_EVENT = { // Project Member Invites PROJECT_MEMBER_INVITE_CREATED: 'notifications.connect.project.member.invite.created', + PROJECT_MEMBER_INVITE_REQUESTED: 'notifications.connect.project.member.invite.requested', PROJECT_MEMBER_INVITE_UPDATED: 'notifications.connect.project.member.invite.updated', + PROJECT_MEMBER_INVITE_APPROVED: 'notifications.connect.project.member.invite.approved', + PROJECT_MEMBER_INVITE_REJECTED: 'notifications.connect.project.member.invite.rejected', PROJECT_MEMBER_EMAIL_INVITE_CREATED: 'connect.action.email.project.member.invite.created', }; diff --git a/src/events/busApi.js b/src/events/busApi.js index 141ee3e4..51bc6d45 100644 --- a/src/events/busApi.js +++ b/src/events/busApi.js @@ -1,6 +1,7 @@ import _ from 'lodash'; import config from 'config'; -import { EVENT, BUS_API_EVENT, PROJECT_STATUS, PROJECT_PHASE_STATUS, PROJECT_MEMBER_ROLE, MILESTONE_STATUS } +import { EVENT, BUS_API_EVENT, PROJECT_STATUS, PROJECT_PHASE_STATUS, PROJECT_MEMBER_ROLE, MILESTONE_STATUS, + INVITE_STATUS } from '../constants'; import { createEvent } from '../services/busApi'; import models from '../models'; @@ -696,30 +697,61 @@ module.exports = (app, logger) => { } }); - app.on(EVENT.ROUTING_KEY.PROJECT_MEMBER_INVITE_CREATED, ({ req, userId, email }) => { + app.on(EVENT.ROUTING_KEY.PROJECT_MEMBER_INVITE_CREATED, ({ req, userId, email, role }) => { logger.debug('receive PROJECT_MEMBER_INVITE_CREATED event'); const projectId = _.parseInt(req.params.projectId); - // send event to bus api - createEvent(BUS_API_EVENT.PROJECT_MEMBER_INVITE_CREATED, { - projectId, - userId, - email, - initiatorUserId: req.authUser.userId, - }, logger); + if (role === PROJECT_MEMBER_ROLE.COPILOT) { + createEvent(BUS_API_EVENT.PROJECT_MEMBER_INVITE_REQUESTED, { + projectId, + userId, + email, + initiatorUserId: req.authUser.userId, + }, logger); + } else { + // send event to bus api + createEvent(BUS_API_EVENT.PROJECT_MEMBER_INVITE_CREATED, { + projectId, + userId, + email, + initiatorUserId: req.authUser.userId, + }, logger); + } }); - app.on(EVENT.ROUTING_KEY.PROJECT_MEMBER_INVITE_UPDATED, ({ req, userId, email, status }) => { + app.on(EVENT.ROUTING_KEY.PROJECT_MEMBER_INVITE_UPDATED, ({ req, userId, email, status, role, createdBy }) => { logger.debug('receive PROJECT_MEMBER_INVITE_UPDATED event'); const projectId = _.parseInt(req.params.projectId); - // send event to bus api - createEvent(BUS_API_EVENT.PROJECT_MEMBER_INVITE_UPDATED, { - projectId, - userId, - email, - status, - initiatorUserId: req.authUser.userId, - }, logger); + if (role === PROJECT_MEMBER_ROLE.COPILOT && status === INVITE_STATUS.ACCEPTED) { + // send event to bus api + createEvent(BUS_API_EVENT.PROJECT_MEMBER_INVITE_APPROVED, { + projectId, + userId, + originator: createdBy, + email, + status, + initiatorUserId: req.authUser.userId, + }, logger); + } else if (role === PROJECT_MEMBER_ROLE.COPILOT && status === INVITE_STATUS.REFUSED) { + // send event to bus api + createEvent(BUS_API_EVENT.PROJECT_MEMBER_INVITE_REJECTED, { + projectId, + userId, + originator: createdBy, + email, + status, + initiatorUserId: req.authUser.userId, + }, logger); + } else { + // send event to bus api + createEvent(BUS_API_EVENT.PROJECT_MEMBER_INVITE_UPDATED, { + projectId, + userId, + email, + status, + initiatorUserId: req.authUser.userId, + }, logger); + } }); }; diff --git a/src/routes/projectMemberInvites/create.js b/src/routes/projectMemberInvites/create.js index 3214c493..9143fcf2 100644 --- a/src/routes/projectMemberInvites/create.js +++ b/src/routes/projectMemberInvites/create.js @@ -250,7 +250,7 @@ module.exports = [ { correlationId: req.id }, ); // send email invite (async) - if (v.email && !v.userId) { + if (v.email && !v.userId && v.role !== PROJECT_MEMBER_ROLE.COPILOT) { sendInviteEmail(req, projectId, v); } }); diff --git a/src/routes/projectMemberInvites/update.js b/src/routes/projectMemberInvites/update.js index 300ba0ad..a73f2cbf 100644 --- a/src/routes/projectMemberInvites/update.js +++ b/src/routes/projectMemberInvites/update.js @@ -4,7 +4,7 @@ import Joi from 'joi'; import { middleware as tcMiddleware } from 'tc-core-library-js'; import models from '../../models'; import util from '../../util'; -import { PROJECT_MEMBER_ROLE, MANAGER_ROLES, INVITE_STATUS, EVENT } from '../../constants'; +import { PROJECT_MEMBER_ROLE, MANAGER_ROLES, INVITE_STATUS, EVENT, USER_ROLE } from '../../constants'; /** * API to update invite member to project. @@ -66,8 +66,12 @@ module.exports = [ if (!util.hasRoles(req, MANAGER_ROLES) && invite.role !== PROJECT_MEMBER_ROLE.CUSTOMER) { error = `Project members can cancel invites only for ${PROJECT_MEMBER_ROLE.CUSTOMER}`; } - } else if ((!!putInvite.userId && putInvite.userId !== req.authUser.userId) || - (!!putInvite.email && putInvite.email !== req.authUser.email)) { + } else if ((!!invite.role && invite.role === PROJECT_MEMBER_ROLE.COPILOT) && + !req.authUser.roles.includes(USER_ROLE.COPILOT_MANAGER)) { + error = 'Only Connect copilot manager can add copilots'; + } else if (((!!putInvite.userId && putInvite.userId !== req.authUser.userId) || + (!!putInvite.email && putInvite.email !== req.authUser.email)) && + (!!invite.role && invite.role !== PROJECT_MEMBER_ROLE.COPILOT)) { error = 'Project members can only update invites for themselves'; } @@ -88,6 +92,8 @@ module.exports = [ userId: updatedInvite.userId, email: updatedInvite.email, status: updatedInvite.status, + role: updatedInvite.role, + createdBy: updatedInvite.createdBy, }); req.app.services.pubsub.publish(EVENT.ROUTING_KEY.PROJECT_MEMBER_INVITE_UPDATED, updatedInvite, { correlationId: req.id, @@ -103,7 +109,7 @@ module.exports = [ const member = { projectId, role: updatedInvite.role, - userId: req.authUser.userId, + userId: updatedInvite.userId, createdBy: req.authUser.userId, updatedBy: req.authUser.userId, }; diff --git a/src/routes/projectMemberInvites/update.spec.js b/src/routes/projectMemberInvites/update.spec.js index a137e57e..c2f61df0 100644 --- a/src/routes/projectMemberInvites/update.spec.js +++ b/src/routes/projectMemberInvites/update.spec.js @@ -16,6 +16,7 @@ describe('Project member invite update', () => { let project1; let invite1; let invite2; + let invite3; beforeEach((done) => { testUtil.clearDb() @@ -73,7 +74,22 @@ describe('Project member invite update', () => { invite2 = in2.get({ plain: true, }); - done(); + models.ProjectMemberInvite.create({ + projectId: project1.id, + userId: 40051332, + email: null, + role: PROJECT_MEMBER_ROLE.COPILOT, + status: INVITE_STATUS.PENDING, + createdBy: 1, + updatedBy: 1, + createdAt: '2016-06-30 00:33:07+00', + updatedAt: '2016-06-30 00:33:07+00', + }).then((in3) => { + invite3 = in3.get({ + plain: true, + }); + done(); + }); }); }); }); @@ -243,6 +259,52 @@ describe('Project member invite update', () => { }); }); + it('should return 403 if try to update COPILOT role invite with copilot', (done) => { + const mockHttpClient = _.merge(testUtil.mockHttpClient, { + get: () => Promise.resolve({ + status: 200, + data: { + id: 'requesterId', + version: 'v3', + result: { + success: true, + status: 200, + content: [{ + roleName: USER_ROLE.COPILOT, + }], + }, + }, + }), + }); + sandbox.stub(util, 'getHttpClient', () => mockHttpClient); + request(server) + .put(`/v4/projects/${project1.id}/members/invite`) + .set({ + Authorization: `Bearer ${testUtil.jwts.copilot}`, + }) + .send({ + param: { + userId: invite3.userId, + status: INVITE_STATUS.ACCEPTED, + }, + }) + .expect('Content-Type', /json/) + .expect(403) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body.result.content; + should.exist(resJson); + res.body.result.status.should.equal(403); + const errorMessage = _.get(resJson, 'message', ''); + sinon.assert.match(errorMessage, 'Only Connect copilot manager can add copilots'); + done(); + } + }); + }); + + describe('Bus api', () => { let createEventSpy; From e14627c4f8bc183b3f6238ec5ff43d72f0932667 Mon Sep 17 00:00:00 2001 From: Samir Date: Thu, 21 Feb 2019 19:26:41 +0100 Subject: [PATCH 12/18] add role to invite created event --- src/routes/projectMemberInvites/create.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/routes/projectMemberInvites/create.js b/src/routes/projectMemberInvites/create.js index 9143fcf2..0f95c770 100644 --- a/src/routes/projectMemberInvites/create.js +++ b/src/routes/projectMemberInvites/create.js @@ -243,6 +243,7 @@ module.exports = [ req, userId: v.userId, email: v.email, + role: v.role, }); req.app.services.pubsub.publish( EVENT.ROUTING_KEY.PROJECT_MEMBER_INVITE_CREATED, From b5b574cef92407b3b0f3ccdcb05af33c86c4cb92 Mon Sep 17 00:00:00 2001 From: Samir Date: Sun, 24 Feb 2019 19:10:07 +0100 Subject: [PATCH 13/18] copilot manager updates --- src/constants.js | 10 ++++++++- src/events/busApi.js | 13 ++++++++---- src/models/projectMemberInvite.js | 19 +++++++++++++++++ src/permissions/project.edit.js | 4 ++-- src/permissions/project.view.js | 5 ++--- src/routes/projectMemberInvites/create.js | 11 +++++++--- src/routes/projectMemberInvites/update.js | 21 ++++++++++++------- .../projectMemberInvites/update.spec.js | 6 +++--- src/routes/projects/create.js | 5 +++-- src/routes/projects/get.js | 2 +- src/routes/projects/list-db.js | 4 ++-- src/routes/projects/list.js | 5 ++--- 12 files changed, 74 insertions(+), 31 deletions(-) diff --git a/src/constants.js b/src/constants.js index 9aa3a803..583cfce1 100644 --- a/src/constants.js +++ b/src/constants.js @@ -34,7 +34,12 @@ export const USER_ROLE = { export const ADMIN_ROLES = [USER_ROLE.CONNECT_ADMIN, USER_ROLE.TOPCODER_ADMIN]; -export const MANAGER_ROLES = [...ADMIN_ROLES, USER_ROLE.MANAGER, PROJECT_MEMBER_ROLE.ACCOUNT_MANAGER]; +export const MANAGER_ROLES = [ + ...ADMIN_ROLES, + USER_ROLE.MANAGER, + USER_ROLE.TOPCODER_ACCOUNT_MANAGER, + USER_ROLE.COPILOT_MANAGER, +]; export const EVENT = { ROUTING_KEY: { @@ -162,5 +167,8 @@ export const INVITE_STATUS = { PENDING: 'pending', ACCEPTED: 'accepted', REFUSED: 'refused', + REQUESTED: 'requested', + REQUEST_REJECTED: 'request_rejected', + REQUEST_APPROVED: 'request_approved', CANCELED: 'canceled', }; diff --git a/src/events/busApi.js b/src/events/busApi.js index 51bc6d45..b1b69dc2 100644 --- a/src/events/busApi.js +++ b/src/events/busApi.js @@ -697,15 +697,16 @@ module.exports = (app, logger) => { } }); - app.on(EVENT.ROUTING_KEY.PROJECT_MEMBER_INVITE_CREATED, ({ req, userId, email, role }) => { + app.on(EVENT.ROUTING_KEY.PROJECT_MEMBER_INVITE_CREATED, ({ req, userId, email, status, role }) => { logger.debug('receive PROJECT_MEMBER_INVITE_CREATED event'); const projectId = _.parseInt(req.params.projectId); - if (role === PROJECT_MEMBER_ROLE.COPILOT) { + if (status === INVITE_STATUS.REQUESTED) { createEvent(BUS_API_EVENT.PROJECT_MEMBER_INVITE_REQUESTED, { projectId, userId, email, + role, initiatorUserId: req.authUser.userId, }, logger); } else { @@ -714,6 +715,7 @@ module.exports = (app, logger) => { projectId, userId, email, + role, initiatorUserId: req.authUser.userId, }, logger); } @@ -723,23 +725,25 @@ module.exports = (app, logger) => { logger.debug('receive PROJECT_MEMBER_INVITE_UPDATED event'); const projectId = _.parseInt(req.params.projectId); - if (role === PROJECT_MEMBER_ROLE.COPILOT && status === INVITE_STATUS.ACCEPTED) { + if (status === INVITE_STATUS.REQUEST_APPROVED) { // send event to bus api createEvent(BUS_API_EVENT.PROJECT_MEMBER_INVITE_APPROVED, { projectId, userId, originator: createdBy, email, + role, status, initiatorUserId: req.authUser.userId, }, logger); - } else if (role === PROJECT_MEMBER_ROLE.COPILOT && status === INVITE_STATUS.REFUSED) { + } else if (status === INVITE_STATUS.REQUEST_REJECTED) { // send event to bus api createEvent(BUS_API_EVENT.PROJECT_MEMBER_INVITE_REJECTED, { projectId, userId, originator: createdBy, email, + role, status, initiatorUserId: req.authUser.userId, }, logger); @@ -749,6 +753,7 @@ module.exports = (app, logger) => { projectId, userId, email, + role, status, initiatorUserId: req.authUser.userId, }, logger); diff --git a/src/models/projectMemberInvite.js b/src/models/projectMemberInvite.js index ac33ee2d..ae27d0cb 100644 --- a/src/models/projectMemberInvite.js +++ b/src/models/projectMemberInvite.js @@ -55,6 +55,15 @@ module.exports = function defineProjectMemberInvite(sequelize, DataTypes) { raw: true, }); }, + getPendingAndReguestedInvitesForProject(projectId) { + return this.findAll({ + where: { + projectId, + status: { $in: [INVITE_STATUS.PENDING, INVITE_STATUS.REQUESTED] }, + }, + raw: true, + }); + }, getPendingInviteByEmailOrUserId(projectId, email, userId) { const where = { projectId, status: INVITE_STATUS.PENDING }; @@ -69,6 +78,16 @@ module.exports = function defineProjectMemberInvite(sequelize, DataTypes) { where, }); }, + getRequestedInvite(projectId, userId) { + const where = { projectId, status: INVITE_STATUS.REQUESTED }; + + if (userId) { + _.assign(where, { userId }); + } + return this.findOne({ + where, + }); + }, getProjectInvitesForUser(email, userId) { const where = { status: INVITE_STATUS.PENDING }; diff --git a/src/permissions/project.edit.js b/src/permissions/project.edit.js index 760e672e..a9ab5d51 100644 --- a/src/permissions/project.edit.js +++ b/src/permissions/project.edit.js @@ -3,7 +3,7 @@ import _ from 'lodash'; import util from '../util'; import models from '../models'; -import { USER_ROLE } from '../constants'; +import { MANAGER_ROLES } from '../constants'; /** * Super admin, Topcoder Managers are allowed to edit any project @@ -20,7 +20,7 @@ module.exports = freq => new Promise((resolve, reject) => { req.context.currentProjectMembers = members; // check if auth user has acecss to this project const hasAccess = util.hasAdminRole(req) - || util.hasRole(req, USER_ROLE.MANAGER) + || util.hasRoles(req, MANAGER_ROLES) || !_.isUndefined(_.find(members, m => m.userId === req.authUser.userId)); if (!hasAccess) { diff --git a/src/permissions/project.view.js b/src/permissions/project.view.js index 5fb10ff5..61e4ebed 100644 --- a/src/permissions/project.view.js +++ b/src/permissions/project.view.js @@ -2,7 +2,7 @@ import _ from 'lodash'; import util from '../util'; import models from '../models'; -import { USER_ROLE, PROJECT_STATUS, PROJECT_MEMBER_ROLE } from '../constants'; +import { USER_ROLE, PROJECT_STATUS, PROJECT_MEMBER_ROLE, MANAGER_ROLES } from '../constants'; /** * Super admin, Topcoder Managers are allowed to view any projects @@ -21,8 +21,7 @@ module.exports = freq => new Promise((resolve, reject) => { req.context.currentProjectMembers = members; // check if auth user has acecss to this project const hasAccess = util.hasAdminRole(req) - || util.hasRole(req, USER_ROLE.MANAGER) - || util.hasRole(req, USER_ROLE.TOPCODER_ACCOUNT_MANAGER) + || util.hasRoles(req, MANAGER_ROLES) || !_.isUndefined(_.find(members, m => m.userId === currentUserId)); // if user is co-pilot and the project doesn't have any copilots then diff --git a/src/routes/projectMemberInvites/create.js b/src/routes/projectMemberInvites/create.js index 0f95c770..90817f2c 100644 --- a/src/routes/projectMemberInvites/create.js +++ b/src/routes/projectMemberInvites/create.js @@ -8,7 +8,7 @@ import { middleware as tcMiddleware } from 'tc-core-library-js'; import models from '../../models'; import util from '../../util'; import { PROJECT_MEMBER_ROLE, PROJECT_MEMBER_MANAGER_ROLES, - MANAGER_ROLES, INVITE_STATUS, EVENT, BUS_API_EVENT } from '../../constants'; + MANAGER_ROLES, INVITE_STATUS, EVENT, BUS_API_EVENT, USER_ROLE } from '../../constants'; import { createEvent } from '../../services/busApi'; @@ -224,7 +224,11 @@ module.exports = [ const data = { projectId, role: invite.role, - status: INVITE_STATUS.PENDING, + // invite directly if user is admin or copilot manager + status: (invite.role !== PROJECT_MEMBER_ROLE.COPILOT || + util.hasRoles(req, [USER_ROLE.CONNECT_ADMIN, USER_ROLE.COPILOT_MANAGER])) + ? INVITE_STATUS.PENDING + : INVITE_STATUS.REQUESTED, createdBy: req.authUser.userId, updatedBy: req.authUser.userId, }; @@ -243,6 +247,7 @@ module.exports = [ req, userId: v.userId, email: v.email, + status: v.status, role: v.role, }); req.app.services.pubsub.publish( @@ -251,7 +256,7 @@ module.exports = [ { correlationId: req.id }, ); // send email invite (async) - if (v.email && !v.userId && v.role !== PROJECT_MEMBER_ROLE.COPILOT) { + if (v.email && !v.userId && v.status === INVITE_STATUS.PENDING) { sendInviteEmail(req, projectId, v); } }); diff --git a/src/routes/projectMemberInvites/update.js b/src/routes/projectMemberInvites/update.js index a73f2cbf..a5ba6b5a 100644 --- a/src/routes/projectMemberInvites/update.js +++ b/src/routes/projectMemberInvites/update.js @@ -44,13 +44,17 @@ module.exports = [ } let invite; + let requestedInvite; return models.ProjectMemberInvite.getPendingInviteByEmailOrUserId( projectId, putInvite.email, putInvite.userId, ).then((_invite) => { invite = _invite; - if (!invite) { + }).then(() => models.ProjectMemberInvite.getRequestedInvite(projectId, putInvite.userId)) + .then((_requestedInvite) => { + requestedInvite = _requestedInvite; + if (!invite && !requestedInvite) { // check there is an existing invite for the user with status PENDING // handle 404 const err = new Error( @@ -60,18 +64,20 @@ module.exports = [ return next(err); } + invite = invite || requestedInvite; + req.log.debug('Chekcing user permission for updating invite'); let error = null; - if (putInvite.status === INVITE_STATUS.CANCELED) { + if (invite.status === INVITE_STATUS.REQUESTED && + !util.hasRoles(req, [USER_ROLE.CONNECT_ADMIN, USER_ROLE.COPILOT_MANAGER])) { + error = 'Requested invites can only be updated by Copilot manager'; + } else if (putInvite.status === INVITE_STATUS.CANCELED) { if (!util.hasRoles(req, MANAGER_ROLES) && invite.role !== PROJECT_MEMBER_ROLE.CUSTOMER) { error = `Project members can cancel invites only for ${PROJECT_MEMBER_ROLE.CUSTOMER}`; } - } else if ((!!invite.role && invite.role === PROJECT_MEMBER_ROLE.COPILOT) && - !req.authUser.roles.includes(USER_ROLE.COPILOT_MANAGER)) { - error = 'Only Connect copilot manager can add copilots'; } else if (((!!putInvite.userId && putInvite.userId !== req.authUser.userId) || (!!putInvite.email && putInvite.email !== req.authUser.email)) && - (!!invite.role && invite.role !== PROJECT_MEMBER_ROLE.COPILOT)) { + !util.hasRoles(req, [USER_ROLE.CONNECT_ADMIN, USER_ROLE.COPILOT_MANAGER])) { error = 'Project members can only update invites for themselves'; } @@ -101,7 +107,8 @@ module.exports = [ req.log.debug('Adding user to project'); // add user to project if accept invite - if (updatedInvite.status === INVITE_STATUS.ACCEPTED) { + if (updatedInvite.status === INVITE_STATUS.ACCEPTED || + updatedInvite.status === INVITE_STATUS.REQUEST_APPROVED) { return models.ProjectMember.getActiveProjectMembers(projectId) .then((members) => { req.context = req.context || {}; diff --git a/src/routes/projectMemberInvites/update.spec.js b/src/routes/projectMemberInvites/update.spec.js index c2f61df0..0383291b 100644 --- a/src/routes/projectMemberInvites/update.spec.js +++ b/src/routes/projectMemberInvites/update.spec.js @@ -62,7 +62,7 @@ describe('Project member invite update', () => { }); models.ProjectMemberInvite.create({ projectId: project1.id, - userId: 40051332, + userId: 40051334, email: null, role: PROJECT_MEMBER_ROLE.MANAGER, status: INVITE_STATUS.PENDING, @@ -79,7 +79,7 @@ describe('Project member invite update', () => { userId: 40051332, email: null, role: PROJECT_MEMBER_ROLE.COPILOT, - status: INVITE_STATUS.PENDING, + status: INVITE_STATUS.REQUESTED, createdBy: 1, updatedBy: 1, createdAt: '2016-06-30 00:33:07+00', @@ -298,7 +298,7 @@ describe('Project member invite update', () => { should.exist(resJson); res.body.result.status.should.equal(403); const errorMessage = _.get(resJson, 'message', ''); - sinon.assert.match(errorMessage, 'Only Connect copilot manager can add copilots'); + sinon.assert.match(errorMessage, 'Requested invites can only be updated by Copilot manager'); done(); } }); diff --git a/src/routes/projects/create.js b/src/routes/projects/create.js index 2600203b..61a9e299 100644 --- a/src/routes/projects/create.js +++ b/src/routes/projects/create.js @@ -7,7 +7,8 @@ import config from 'config'; import moment from 'moment'; import models from '../../models'; -import { PROJECT_MEMBER_ROLE, PROJECT_STATUS, PROJECT_PHASE_STATUS, USER_ROLE, EVENT, REGEX } from '../../constants'; +import { PROJECT_MEMBER_ROLE, MANAGER_ROLES, PROJECT_STATUS, PROJECT_PHASE_STATUS, + EVENT, REGEX } from '../../constants'; import fieldLookupValidation from '../../middlewares/fieldLookupValidation'; import util from '../../util'; import directProject from '../../services/directProject'; @@ -197,7 +198,7 @@ module.exports = [ (req, res, next) => { const project = req.body.param; // by default connect admin and managers joins projects as manager - const userRole = util.hasRoles(req, [USER_ROLE.CONNECT_ADMIN, USER_ROLE.MANAGER]) + const userRole = util.hasRoles(req, MANAGER_ROLES) ? PROJECT_MEMBER_ROLE.MANAGER : PROJECT_MEMBER_ROLE.CUSTOMER; // set defaults diff --git a/src/routes/projects/get.js b/src/routes/projects/get.js index a6f8cabe..8942237d 100644 --- a/src/routes/projects/get.js +++ b/src/routes/projects/get.js @@ -64,7 +64,7 @@ module.exports = [ if (attachments) { project.attachments = attachments; } - return models.ProjectMemberInvite.getPendingInvitesForProject(projectId); + return models.ProjectMemberInvite.getPendingAndReguestedInvitesForProject(projectId); }) .then((invites) => { project.invites = invites; diff --git a/src/routes/projects/list-db.js b/src/routes/projects/list-db.js index a8ed05d2..3e10736f 100644 --- a/src/routes/projects/list-db.js +++ b/src/routes/projects/list-db.js @@ -1,7 +1,7 @@ import _ from 'lodash'; import Promise from 'bluebird'; import models from '../../models'; -import { USER_ROLE } from '../../constants'; +import { USER_ROLE, MANAGER_ROLES } from '../../constants'; import util from '../../util'; /** @@ -125,7 +125,7 @@ module.exports = [ if (!memberOnly && (util.hasAdminRole(req) - || util.hasRole(req, USER_ROLE.MANAGER))) { + || util.hasRoles(req, MANAGER_ROLES))) { // admins & topcoder managers can see all projects return retrieveProjects(req, criteria, sort, req.query.fields) .then(result => res.json(util.wrapResponse(req.id, result.rows, result.count))) diff --git a/src/routes/projects/list.js b/src/routes/projects/list.js index faca2fb5..d9be9f66 100755 --- a/src/routes/projects/list.js +++ b/src/routes/projects/list.js @@ -5,7 +5,7 @@ import _ from 'lodash'; import config from 'config'; import models from '../../models'; -import { USER_ROLE } from '../../constants'; +import { USER_ROLE, MANAGER_ROLES } from '../../constants'; import util from '../../util'; const ES_PROJECT_INDEX = config.get('elasticsearchConfig.indexName'); @@ -430,8 +430,7 @@ module.exports = [ if (!memberOnly && (util.hasAdminRole(req) - || util.hasRole(req, USER_ROLE.MANAGER) - || util.hasRole(req, USER_ROLE.TOPCODER_ACCOUNT_MANAGER))) { + || util.hasRoles(req, MANAGER_ROLES))) { // admins & topcoder managers can see all projects return retrieveProjects(req, criteria, sort, req.query.fields) .then(result => res.json(util.wrapResponse(req.id, result.rows, result.count))) From a3190cf3e779271c7a2780585cdba99fec3d7876 Mon Sep 17 00:00:00 2001 From: Maksym Mykhailenko Date: Tue, 5 Mar 2019 15:44:02 +0800 Subject: [PATCH 14/18] winning submission from challenge 30085188 - Topcoder Connect - Handle invitation errors --- postman.json | 27 ++++- src/routes/projectMemberInvites/create.js | 95 +++++++-------- .../projectMemberInvites/create.spec.js | 109 ++++++++++++++---- swagger.yaml | 12 +- 4 files changed, 170 insertions(+), 73 deletions(-) diff --git a/postman.json b/postman.json index 96995389..8c74a585 100644 --- a/postman.json +++ b/postman.json @@ -1041,6 +1041,31 @@ }, "response": [] }, + { + "name": "Invite with userIds and emails - both success and failed", + "request": { + "url": "{{api-url}}/v4/projects/1/members/invite", + "method": "POST", + "header": [ + { + "key": "Authorization", + "value": "Bearer {{jwt-token-manager-40051334}}", + "description": "" + }, + { + "key": "Content-Type", + "value": "application/json", + "description": "" + } + ], + "body": { + "mode": "raw", + "raw": "{\n\t\"param\": {\n\t\t\"userIds\": [40051331, 40051334],\n\t\t\"emails\": [\"divyalife526@gmail.com\"],\n\t\t\"role\": \"manager\"\n\t}\n}" + }, + "description": "" + }, + "response": [] + }, { "name": "Update invite status with userId", "request": { @@ -5497,4 +5522,4 @@ ] } ] -} \ No newline at end of file +} diff --git a/src/routes/projectMemberInvites/create.js b/src/routes/projectMemberInvites/create.js index 90817f2c..862c1f7c 100644 --- a/src/routes/projectMemberInvites/create.js +++ b/src/routes/projectMemberInvites/create.js @@ -35,12 +35,12 @@ const addMemberValidations = { * @param {Object} invite invite to process * @param {Array} invites existent invites from DB * @param {Object} data template for new invites to be put in DB + * @param {Array} failed failed invites error message * * @returns {Promise} list of promises */ -const buildCreateInvitePromises = (req, invite, invites, data) => { +const buildCreateInvitePromises = (req, invite, invites, data, failed) => { const invitePromises = []; - if (invite.userIds) { // remove invites for users that are invited already _.remove(invite.userIds, u => _.some(invites, i => i.userId === u)); @@ -91,15 +91,15 @@ const buildCreateInvitePromises = (req, invite, invites, data) => { invitePromises.push(models.ProjectMemberInvite.create(dataNew)); }); - - return Promise.resolve(invitePromises); + return invitePromises; }).catch((error) => { req.log.error(error); - return Promise.reject(invitePromises); + _.forEach(invite.emails, email => failed.push(_.assign({}, { email, message: error.statusText }))); + return invitePromises; }); } - return Promise.resolve(invitePromises); + return invitePromises; }; const sendInviteEmail = (req, projectId, invite) => { @@ -157,6 +157,7 @@ module.exports = [ validate(addMemberValidations), permissions('projectMemberInvite.create'), (req, res, next) => { + let failed = []; const invite = req.body.param; if (!invite.userIds && !invite.emails) { @@ -192,16 +193,16 @@ module.exports = [ if (invite.emails) { // email invites can only be used for CUSTOMER role if (invite.role !== PROJECT_MEMBER_ROLE.CUSTOMER) { // eslint-disable-line no-lonely-if - const err = new Error(`Emails can only be used for ${PROJECT_MEMBER_ROLE.CUSTOMER}`); - err.status = 400; - return next(err); + const message = `Emails can only be used for ${PROJECT_MEMBER_ROLE.CUSTOMER}`; + failed = _.concat(failed, _.map(invite.emails, email => _.assign({}, { email, message }))); + delete invite.emails; } } - if (promises.length === 0) { promises.push(Promise.resolve()); } return Promise.all(promises).then((rolesList) => { + const updatedInvite = invite; if (!!invite.userIds && _.includes(PROJECT_MEMBER_MANAGER_ROLES, invite.role)) { req.log.debug('Chekcing if userId is allowed as manager'); const forbidUserList = []; @@ -209,23 +210,23 @@ module.exports = [ const [userId, roles] = data; req.log.debug(roles); - if (!util.hasIntersection(MANAGER_ROLES, roles)) { + if (roles && !util.hasIntersection(MANAGER_ROLES, roles)) { forbidUserList.push(userId); } }); if (forbidUserList.length > 0) { - const err = new Error(`${forbidUserList.join()} cannot be added with a Manager role to the project`); - err.status = 403; - return next(err); + const message = 'cannot be added with a Manager role to the project'; + failed = _.concat(failed, _.map(forbidUserList, id => _.assign({}, { id, message }))); + updatedInvite.userIds = _.filter(invite.userIds, userId => !_.includes(forbidUserList, userId)); } } return models.ProjectMemberInvite.getPendingInvitesForProject(projectId) .then((invites) => { const data = { projectId, - role: invite.role, + role: updatedInvite.role, // invite directly if user is admin or copilot manager - status: (invite.role !== PROJECT_MEMBER_ROLE.COPILOT || + status: (updatedInvite.role !== PROJECT_MEMBER_ROLE.COPILOT || util.hasRoles(req, [USER_ROLE.CONNECT_ADMIN, USER_ROLE.COPILOT_MANAGER])) ? INVITE_STATUS.PENDING : INVITE_STATUS.REQUESTED, @@ -233,39 +234,39 @@ module.exports = [ updatedBy: req.authUser.userId, }; - return buildCreateInvitePromises(req, invite, invites, data) - .then((invitePromises) => { - if (invitePromises.length === 0) { - return []; - } - - req.log.debug('Creating invites'); - return models.sequelize.Promise.all(invitePromises) - .then((values) => { - values.forEach((v) => { - req.app.emit(EVENT.ROUTING_KEY.PROJECT_MEMBER_INVITE_CREATED, { - req, - userId: v.userId, - email: v.email, - status: v.status, - role: v.role, - }); - req.app.services.pubsub.publish( - EVENT.ROUTING_KEY.PROJECT_MEMBER_INVITE_CREATED, - v, - { correlationId: req.id }, - ); - // send email invite (async) - if (v.email && !v.userId && v.status === INVITE_STATUS.PENDING) { - sendInviteEmail(req, projectId, v); - } - }); - return values; - }); // models.sequelize.Promise.all - }); // buildCreateInvitePromises + req.log.debug('Creating invites'); + return models.sequelize.Promise.all(buildCreateInvitePromises(req, updatedInvite, invites, data, failed)) + .then((values) => { + values.forEach((v) => { + req.app.emit(EVENT.ROUTING_KEY.PROJECT_MEMBER_INVITE_CREATED, { + req, + userId: v.userId, + email: v.email, + status: v.status, + role: v.role, + }); + req.app.services.pubsub.publish( + EVENT.ROUTING_KEY.PROJECT_MEMBER_INVITE_CREATED, + v, + { correlationId: req.id }, + ); + // send email invite (async) + if (v.email && !v.userId && v.status === INVITE_STATUS.PENDING) { + sendInviteEmail(req, projectId, v); + } + }); + return values; + }); // models.sequelize.Promise.all }); // models.ProjectMemberInvite.getPendingInvitesForProject }) - .then(values => res.status(201).json(util.wrapResponse(req.id, values, null, 201))) + .then((values) => { + const success = _.assign({}, { success: values }); + if (failed.length) { + res.status(403).json(util.wrapResponse(req.id, _.assign({}, success, { failed }), null, 403)); + } else { + res.status(201).json(util.wrapResponse(req.id, success, null, 201)); + } + }) .catch(err => next(err)); }, ]; diff --git a/src/routes/projectMemberInvites/create.spec.js b/src/routes/projectMemberInvites/create.spec.js index e648a06f..74f6be9c 100644 --- a/src/routes/projectMemberInvites/create.spec.js +++ b/src/routes/projectMemberInvites/create.spec.js @@ -42,6 +42,15 @@ describe('Project Member Invite create', () => { createdBy: 1, updatedBy: 1, }); + + models.ProjectMember.create({ + userId: 40051334, + projectId: project1.id, + role: 'manager', + isPrimary: true, + createdBy: 1, + updatedBy: 1, + }); }).then(() => models.Project.create({ type: 'generic', @@ -87,6 +96,7 @@ describe('Project Member Invite create', () => { sinon.stub(server.services.pubsub, 'init', () => {}); sinon.stub(server.services.pubsub, 'publish', () => {}); // by default mock lookupUserEmails return nothing so all the cases are not broken + sandbox.stub(util, 'getUserRoles', () => Promise.resolve([])); sandbox.stub(util, 'lookupUserEmails', () => Promise.resolve([])); sandbox.stub(util, 'getMemberDetailsByUserIds', () => Promise.resolve([{ userId: 40051333, @@ -246,9 +256,11 @@ describe('Project Member Invite create', () => { result: { success: true, status: 200, - content: [{ - roleName: USER_ROLE.COPILOT, - }], + content: { + success: [{ + roleName: USER_ROLE.COPILOT, + }], + }, }, }, }), @@ -271,7 +283,7 @@ describe('Project Member Invite create', () => { if (err) { done(err); } else { - const resJson = res.body.result.content[0]; + const resJson = res.body.result.content.success[0]; should.exist(resJson); resJson.role.should.equal('customer'); resJson.projectId.should.equal(project2.id); @@ -292,9 +304,11 @@ describe('Project Member Invite create', () => { result: { success: true, status: 200, - content: [{ - roleName: USER_ROLE.COPILOT, - }], + content: { + success: [{ + roleName: USER_ROLE.COPILOT, + }], + }, }, }, }), @@ -322,7 +336,7 @@ describe('Project Member Invite create', () => { if (err) { done(err); } else { - const resJson = res.body.result.content[0]; + const resJson = res.body.result.content.success[0]; should.exist(resJson); resJson.role.should.equal('customer'); resJson.projectId.should.equal(project2.id); @@ -344,9 +358,11 @@ describe('Project Member Invite create', () => { result: { success: true, status: 200, - content: [{ - roleName: USER_ROLE.COPILOT, - }], + content: { + success: [{ + roleName: USER_ROLE.COPILOT, + }], + }, }, }, }), @@ -369,7 +385,7 @@ describe('Project Member Invite create', () => { if (err) { done(err); } else { - const resJson = res.body.result.content[0]; + const resJson = res.body.result.content.success[0]; should.exist(resJson); resJson.role.should.equal('customer'); resJson.projectId.should.equal(project2.id); @@ -390,9 +406,11 @@ describe('Project Member Invite create', () => { result: { success: true, status: 200, - content: [{ - roleName: USER_ROLE.COPILOT, - }], + content: { + success: [{ + roleName: USER_ROLE.COPILOT, + }], + }, }, }, }), @@ -415,7 +433,7 @@ describe('Project Member Invite create', () => { if (err) { done(err); } else { - const resJson = res.body.result.content; + const resJson = res.body.result.content.success; should.exist(resJson); resJson.length.should.equal(0); server.services.pubsub.publish.neverCalledWith('project.member.invite.created').should.be.true; @@ -484,16 +502,18 @@ describe('Project Member Invite create', () => { it('should return 201 if try to create manager with MANAGER_ROLES', (done) => { const mockHttpClient = _.merge(testUtil.mockHttpClient, { get: () => Promise.resolve({ - status: 200, + status: 403, data: { id: 'requesterId', version: 'v3', result: { success: true, - status: 200, - content: [{ - roleName: USER_ROLE.MANAGER, - }], + status: 403, + content: { + failed: [{ + message: 'cannot be added with a Manager role to the project', + }], + }, }, }, }), @@ -511,16 +531,57 @@ describe('Project Member Invite create', () => { }, }) .expect('Content-Type', /json/) + .expect(403) + .end((err, res) => { + const failed = res.body.result.content.failed[0]; + should.exist(failed); + failed.message.should.equal('cannot be added with a Manager role to the project'); + done(); + }); + }); + + it('should return 201 if try to create customer with COPILOT', (done) => { + const mockHttpClient = _.merge(testUtil.mockHttpClient, { + get: () => Promise.resolve({ + status: 200, + data: { + id: 'requesterId', + version: 'v3', + result: { + success: true, + status: 200, + content: { + success: [{ + roleName: USER_ROLE.COPILOT, + }], + }, + }, + }, + }), + }); + sandbox.stub(util, 'getHttpClient', () => mockHttpClient); + request(server) + .post(`/v4/projects/${project1.id}/members/invite`) + .set({ + Authorization: `Bearer ${testUtil.jwts.manager}`, + }) + .send({ + param: { + userIds: [40051331], + role: 'copilot', + }, + }) + .expect('Content-Type', /json/) .expect(201) .end((err, res) => { if (err) { done(err); } else { - const resJson = res.body.result.content[0]; + const resJson = res.body.result.content.success[0]; should.exist(resJson); - resJson.role.should.equal('manager'); + resJson.role.should.equal('copilot'); resJson.projectId.should.equal(project1.id); - resJson.userId.should.equal(40152855); + resJson.userId.should.equal(40051331); server.services.pubsub.publish.calledWith('project.member.invite.created').should.be.true; done(); } diff --git a/swagger.yaml b/swagger.yaml index eb8d063f..fcd02b60 100644 --- a/swagger.yaml +++ b/swagger.yaml @@ -4071,6 +4071,16 @@ definitions: description: READ-ONLY. User that last updated this task readOnly: true + ProjectMemberInviteSuccessAndFailure: + type: object + properties: + success: + $ref: "#/definitions/ProjectMemberInvite" + failed: + type: array + items: + type: object + AddProjectMemberInvitesRequest: title: Add project member invites request object type: object @@ -4133,4 +4143,4 @@ definitions: metadata: $ref: "#/definitions/ResponseMetadata" content: - $ref: "#/definitions/ProjectMemberInvite" \ No newline at end of file + $ref: "#/definitions/ProjectMemberInviteSuccessAndFailure" From 7a24574c940aa01fd49ca855d47991382307ef8a Mon Sep 17 00:00:00 2001 From: Maksym Mykhailenko Date: Tue, 5 Mar 2019 16:06:04 +0800 Subject: [PATCH 15/18] updated so individual failed invitataions have 'userId' field instead of 'id' to make it clear --- src/routes/projectMemberInvites/create.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/routes/projectMemberInvites/create.js b/src/routes/projectMemberInvites/create.js index 862c1f7c..a4b0860e 100644 --- a/src/routes/projectMemberInvites/create.js +++ b/src/routes/projectMemberInvites/create.js @@ -216,7 +216,7 @@ module.exports = [ }); if (forbidUserList.length > 0) { const message = 'cannot be added with a Manager role to the project'; - failed = _.concat(failed, _.map(forbidUserList, id => _.assign({}, { id, message }))); + failed = _.concat(failed, _.map(forbidUserList, id => _.assign({}, { userId: id, message }))); updatedInvite.userIds = _.filter(invite.userIds, userId => !_.includes(forbidUserList, userId)); } } From 5106789fd1b11959f6a136046670b8a06bb075d5 Mon Sep 17 00:00:00 2001 From: Maksym Mykhailenko Date: Tue, 5 Mar 2019 16:16:24 +0800 Subject: [PATCH 16/18] remove unnecessary variable to avoid confusion --- src/routes/projectMemberInvites/create.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/routes/projectMemberInvites/create.js b/src/routes/projectMemberInvites/create.js index a4b0860e..4839ba45 100644 --- a/src/routes/projectMemberInvites/create.js +++ b/src/routes/projectMemberInvites/create.js @@ -202,7 +202,6 @@ module.exports = [ promises.push(Promise.resolve()); } return Promise.all(promises).then((rolesList) => { - const updatedInvite = invite; if (!!invite.userIds && _.includes(PROJECT_MEMBER_MANAGER_ROLES, invite.role)) { req.log.debug('Chekcing if userId is allowed as manager'); const forbidUserList = []; @@ -217,16 +216,16 @@ module.exports = [ if (forbidUserList.length > 0) { const message = 'cannot be added with a Manager role to the project'; failed = _.concat(failed, _.map(forbidUserList, id => _.assign({}, { userId: id, message }))); - updatedInvite.userIds = _.filter(invite.userIds, userId => !_.includes(forbidUserList, userId)); + invite.userIds = _.filter(invite.userIds, userId => !_.includes(forbidUserList, userId)); } } return models.ProjectMemberInvite.getPendingInvitesForProject(projectId) .then((invites) => { const data = { projectId, - role: updatedInvite.role, + role: invite.role, // invite directly if user is admin or copilot manager - status: (updatedInvite.role !== PROJECT_MEMBER_ROLE.COPILOT || + status: (invite.role !== PROJECT_MEMBER_ROLE.COPILOT || util.hasRoles(req, [USER_ROLE.CONNECT_ADMIN, USER_ROLE.COPILOT_MANAGER])) ? INVITE_STATUS.PENDING : INVITE_STATUS.REQUESTED, @@ -235,7 +234,7 @@ module.exports = [ }; req.log.debug('Creating invites'); - return models.sequelize.Promise.all(buildCreateInvitePromises(req, updatedInvite, invites, data, failed)) + return models.sequelize.Promise.all(buildCreateInvitePromises(req, invite, invites, data, failed)) .then((values) => { values.forEach((v) => { req.app.emit(EVENT.ROUTING_KEY.PROJECT_MEMBER_INVITE_CREATED, { From 1f2718487555d25652527f0b6e4d92fed701fc23 Mon Sep 17 00:00:00 2001 From: Vikas Agarwal Date: Tue, 5 Mar 2019 16:15:44 +0530 Subject: [PATCH 17/18] Added more exception fields for skipping the merge to avoid absolute keys in the updated JSON. --- src/routes/projectTemplates/update.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/routes/projectTemplates/update.js b/src/routes/projectTemplates/update.js index acc7e741..0e0bc780 100644 --- a/src/routes/projectTemplates/update.js +++ b/src/routes/projectTemplates/update.js @@ -64,7 +64,11 @@ module.exports = [ } // Merge JSON fields - entityToUpdate.scope = util.mergeJsonObjects(projectTemplate.scope, entityToUpdate.scope, ['priceConfig']); + entityToUpdate.scope = util.mergeJsonObjects( + projectTemplate.scope, + entityToUpdate.scope, + ['priceConfig', 'addonPriceConfig', 'preparedConditions', 'buildingBlocks'] + ); entityToUpdate.phases = util.mergeJsonObjects(projectTemplate.phases, entityToUpdate.phases); // removes null phase templates entityToUpdate.phases = _.omitBy(entityToUpdate.phases, _.isNull); From 0536be11978e9008eadfbd9e76ffecf81569a9ff Mon Sep 17 00:00:00 2001 From: RishiRaj Date: Wed, 6 Mar 2019 10:50:16 +0530 Subject: [PATCH 18/18] lint fix ; --- src/routes/projectTemplates/update.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/routes/projectTemplates/update.js b/src/routes/projectTemplates/update.js index 0e0bc780..10d55d70 100644 --- a/src/routes/projectTemplates/update.js +++ b/src/routes/projectTemplates/update.js @@ -67,7 +67,7 @@ module.exports = [ entityToUpdate.scope = util.mergeJsonObjects( projectTemplate.scope, entityToUpdate.scope, - ['priceConfig', 'addonPriceConfig', 'preparedConditions', 'buildingBlocks'] + ['priceConfig', 'addonPriceConfig', 'preparedConditions', 'buildingBlocks'], ); entityToUpdate.phases = util.mergeJsonObjects(projectTemplate.phases, entityToUpdate.phases); // removes null phase templates