From 8080019928ca8cb98d2d1d22e754beae860889f2 Mon Sep 17 00:00:00 2001 From: yoution Date: Fri, 28 Feb 2020 18:54:12 +0800 Subject: [PATCH 1/4] #488 Project List endpoint returns member email for some projects --- src/routes/projects/get.js | 27 ++- src/routes/projects/get.spec.js | 293 +++++++++++++++++++++++++++++- src/routes/projects/list.js | 13 +- src/routes/projects/list.spec.js | 297 ++++++++++++++++++++++++++++++- src/util.js | 57 ++++-- 5 files changed, 668 insertions(+), 19 deletions(-) diff --git a/src/routes/projects/get.js b/src/routes/projects/get.js index c76b433a..5625dcef 100644 --- a/src/routes/projects/get.js +++ b/src/routes/projects/get.js @@ -22,9 +22,18 @@ const ES_PROJECT_TYPE = config.get('elasticsearchConfig.docType'); // var permissions = require('tc-core-library-js').middleware.permissions const permissions = tcMiddleware.permissions; const PROJECT_ATTRIBUTES = _.without(_.keys(models.Project.rawAttributes), 'utm', 'deletedAt'); -const PROJECT_MEMBER_ATTRIBUTES = _.without(_.keys(models.ProjectMember.rawAttributes), 'deletedAt'); +const PROJECT_MEMBER_ATTRIBUTES = _.concat(_.without(_.keys(models.ProjectMember.rawAttributes), 'deletedAt'), + ['firstName', 'lastName', 'handle', 'email']); const PROJECT_MEMBER_INVITE_ATTRIBUTES = _.without(_.keys(models.ProjectMemberInvite.rawAttributes), 'deletedAt'); const PROJECT_ATTACHMENT_ATTRIBUTES = _.without(_.keys(models.ProjectAttachment.rawAttributes), 'deletedAt'); +const PROJECT_PHASE_ATTRIBUTES = _.without( + _.keys(models.ProjectPhase.rawAttributes), + 'deletedAt', +); +const PROJECT_PHASE_PRODUCTS_ATTRIBUTES = _.without( + _.keys(models.PhaseProduct.rawAttributes), + 'deletedAt', +); /** * Parse the ES search criteria and prepare search request body @@ -52,13 +61,21 @@ const parseElasticSearchCriteria = (projectId, fields) => { sourceInclude = sourceInclude.concat(_.map(memberFields, single => `invites.${single}`)); } + if (_.get(fields, 'project_phases', null)) { + const phaseFields = _.get(fields, 'project_phases'); + sourceInclude = sourceInclude.concat(_.map(phaseFields, single => `phases.${single}`)); + } + if (_.get(fields, 'project_phases_products', null)) { + const phaseFields = _.get(fields, 'project_phases_products'); + sourceInclude = sourceInclude.concat(_.map(phaseFields, single => `phases.products.${single}`)); + } if (_.get(fields, 'attachments', null)) { const attachmentFields = _.get(fields, 'attachments'); sourceInclude = sourceInclude.concat(_.map(attachmentFields, single => `attachments.${single}`)); } if (sourceInclude) { - searchCriteria._sourceInclude = sourceInclude; // eslint-disable-line no-underscore-dangle + searchCriteria._sourceIncludes = sourceInclude; // eslint-disable-line no-underscore-dangle } @@ -87,9 +104,14 @@ const retrieveProjectFromES = (projectId, req) => { projects: PROJECT_ATTRIBUTES, project_members: PROJECT_MEMBER_ATTRIBUTES, project_member_invites: PROJECT_MEMBER_INVITE_ATTRIBUTES, + project_phases: PROJECT_PHASE_ATTRIBUTES, + project_phases_products: PROJECT_PHASE_PRODUCTS_ATTRIBUTES, attachments: PROJECT_ATTACHMENT_ATTRIBUTES, }); + // if user is not admin, ignore email field for project_members + fields = util.ignoreEmailField(req, fields); + const searchCriteria = parseElasticSearchCriteria(projectId, fields) || {}; return new Promise((accept, reject) => { const es = util.getElasticSearchClient(); @@ -108,6 +130,7 @@ const retrieveProjectFromDB = (projectId, req) => { projects: PROJECT_ATTRIBUTES, project_members: PROJECT_MEMBER_ATTRIBUTES, }); + return models.Project .findOne({ where: { id: projectId }, diff --git a/src/routes/projects/get.spec.js b/src/routes/projects/get.spec.js index 94e6e225..bff5fd56 100644 --- a/src/routes/projects/get.spec.js +++ b/src/routes/projects/get.spec.js @@ -3,7 +3,7 @@ import chai from 'chai'; import sinon from 'sinon'; import request from 'supertest'; - +import _ from 'lodash'; import config from 'config'; import models from '../../models'; import util from '../../util'; @@ -23,7 +23,8 @@ const data = [ billingAccountId: 1, name: 'test1', description: 'es_project', - status: 'active', + cancelReason: 'price/cost', + status: 'draft', details: { utm: { code: 'code1', @@ -42,6 +43,7 @@ const data = [ firstName: 'es_member_1_firstName', lastName: 'Lastname', handle: 'test_tourist_handle', + email: 'test@test.com', isPrimary: true, createdBy: 1, updatedBy: 1, @@ -56,6 +58,30 @@ const data = [ updatedBy: 1, }, ], + invites: [ + { + id: 1, + userId: 40051335, + email: 'test@topcoder.com', + status: 'pending', + }, + ], + phases: [ + + { + id: 45, + name: 'test phases', + spentBudget: 0, + products: [ + { + + phaseId: 45, + id: 3, + name: 'tet product', + }, + ], + }, + ], attachments: [ { id: 1, @@ -80,6 +106,7 @@ describe('GET Project', () => { .then(() => testUtil.clearES()) .then(() => { const p1 = models.Project.create({ + id: 5, type: 'generic', billingAccountId: 1, name: 'test1', @@ -98,6 +125,10 @@ describe('GET Project', () => { projectId: project1.id, role: 'customer', isPrimary: true, + firstName: 'Firstname', + lastName: 'Lastname', + handle: 'test_tourist_handle', + email: 'test@test.com', createdBy: 1, updatedBy: 1, }); @@ -343,5 +374,263 @@ describe('GET Project', () => { }); }); }); + + describe('URL Query fields', () => { + it('should not return "email" for project members when "fields" query param is not defined (to non-admin users)', (done) => { + request(server) + .get(`/v5/projects/${project1.id}?fields=members.handle`) + .set({ + Authorization: `Bearer ${testUtil.jwts.member}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body; + should.exist(resJson); + resJson.members[0].should.have.property('handle'); + resJson.members[0].should.not.have.property('email'); + done(); + } + }); + }); + + it('should not return "email" for project members even if it\'s defined in "fields" query param (to non-admin users)', (done) => { + request(server) + .get(`/v5/projects/${project1.id}?fields=members.email,members.handle`) + .set({ + Authorization: `Bearer ${testUtil.jwts.member}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body; + should.exist(resJson); + resJson.members[0].should.have.property('handle'); + resJson.members[0].should.not.have.property('email'); + done(); + } + }); + }); + + + it('should not return "cancelReason" if it is not listed in "fields" query param ', (done) => { + request(server) + .get(`/v5/projects/${project1.id}?fields=description`) + .set({ + Authorization: `Bearer ${testUtil.jwts.member}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body; + should.exist(resJson); + resJson.should.have.property('description'); + resJson.description.should.be.eq('es_project'); + resJson.should.not.have.property('cancelReason'); + done(); + } + }); + }); + + it('should not return "email" for project members when "fields" query param is not defined (to admin users)', (done) => { + request(server) + .get(`/v5/projects/${project1.id}?fields=description,members.id`) + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body; + should.exist(resJson); + resJson.members.should.have.lengthOf(2); + resJson.members[0].should.not.have.property('email'); + done(); + } + }); + }); + + it('should return "email" for project members if it\'s defined in "fields" query param (to admin users', (done) => { + request(server) + .get(`/v5/projects/${project1.id}?fields=description,members.id,members.email`) + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body; + should.exist(resJson); + resJson.members.should.have.lengthOf(2); + resJson.members[0].should.have.property('email'); + resJson.members[0].email.should.be.eq('test@test.com'); + done(); + } + }); + }); + + + it('should only return "id" field, when it\'s defined in "fields" query param', (done) => { + request(server) + .get(`/v5/projects/${project1.id}?fields=id`) + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body; + should.exist(resJson); + resJson.should.have.property('id'); + _.keys(resJson).length.should.be.eq(1); + done(); + } + }); + }); + + it('should only return "invites.userId" field, when it\'s defined in "fields" query param', (done) => { + request(server) + .get(`/v5/projects/${project1.id}?fields=invites.userId`) + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body; + should.exist(resJson); + resJson.invites[0].should.have.property('userId'); + _.keys(resJson.invites[0]).length.should.be.eq(1); + done(); + } + }); + }); + + it('should only return "members.role" field, when it\'s defined in "fields" query param', (done) => { + request(server) + .get(`/v5/projects/${project1.id}?fields=members.role`) + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body; + should.exist(resJson); + resJson.members[0].should.have.property('role'); + _.keys(resJson.members[0]).length.should.be.eq(1); + done(); + } + }); + }); + + it('should only return "attachments.title" field, when it\'s defined in "fields" query param', (done) => { + request(server) + .get(`/v5/projects/${project1.id}?fields=attachments.title`) + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body; + should.exist(resJson); + resJson.attachments[0].should.have.property('title'); + _.keys(resJson.attachments[0]).length.should.be.eq(1); + done(); + } + }); + }); + + it('should only return "phases.name" field, when it\'s defined in "fields" query param', (done) => { + request(server) + .get(`/v5/projects/${project1.id}?fields=phases.name`) + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body; + should.exist(resJson); + resJson.phases[0].should.have.property('name'); + _.keys(resJson.phases[0]).length.should.be.eq(1); + done(); + } + }); + }); + + it('should only return "phases.products.name" field, when it\'s defined in "fields" query param and "phases" is also defined', (done) => { + request(server) + .get(`/v5/projects/${project1.id}?fields=phases.products.name,phases.name`) + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body; + should.exist(resJson); + resJson.phases[0].products[0].should.have.property('name'); + _.keys(resJson.phases[0].products[0]).length.should.be.eq(1); + done(); + } + }); + }); + + it('should not return "phases.products.name" field, when it\'s defined in "fields" query param but "phases" is not defined', (done) => { + request(server) + + .get(`/v5/projects/${project1.id}?fields=id,phases.products.name`) + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body; + should.exist(resJson); + resJson.should.not.have.property('phases'); + done(); + } + }); + }); + }); }); }); diff --git a/src/routes/projects/list.js b/src/routes/projects/list.js index fc58c03b..8ca1279a 100755 --- a/src/routes/projects/list.js +++ b/src/routes/projects/list.js @@ -26,10 +26,10 @@ const PROJECT_ATTRIBUTES = _.without(_.keys(models.Project.rawAttributes), 'utm', 'deletedAt', ); -const PROJECT_MEMBER_ATTRIBUTES = _.without( +const PROJECT_MEMBER_ATTRIBUTES = _.concat(_.without( _.keys(models.ProjectMember.rawAttributes), 'deletedAt', -); +), ['firstName', 'lastName', 'handle', 'email']); const PROJECT_MEMBER_INVITE_ATTRIBUTES = _.without( _.keys(models.ProjectMemberInvite.rawAttributes), 'deletedAt', @@ -314,7 +314,7 @@ const parseElasticSearchCriteria = (criteria, fields, order) => { } if (sourceInclude) { - searchCriteria._sourceInclude = sourceInclude; // eslint-disable-line no-underscore-dangle + searchCriteria._sourceIncludes = sourceInclude; // eslint-disable-line no-underscore-dangle } // prepare the elasticsearch filter criteria const boolQuery = []; @@ -481,6 +481,8 @@ const retrieveProjectsFromDB = (req, criteria, sort, ffields) => { projects: PROJECT_ATTRIBUTES, project_members: PROJECT_MEMBER_ATTRIBUTES, }); + + // make sure project.id is part of fields if (_.indexOf(fields.projects, 'id') < 0) fields.projects.push('id'); const retrieveAttachments = !req.query.fields || req.query.fields.indexOf('attachments') > -1; @@ -548,6 +550,11 @@ const retrieveProjects = (req, criteria, sort, ffields) => { project_phases_products: PROJECT_PHASE_PRODUCTS_ATTRIBUTES, attachments: PROJECT_ATTACHMENT_ATTRIBUTES, }); + + + // if user is not admin, ignore email field for project_members + fields = util.ignoreEmailField(req, fields); + // make sure project.id is part of fields if (_.indexOf(fields.projects, 'id') < 0) { fields.projects.push('id'); diff --git a/src/routes/projects/list.spec.js b/src/routes/projects/list.spec.js index 0cbb4fc6..6d342923 100644 --- a/src/routes/projects/list.spec.js +++ b/src/routes/projects/list.spec.js @@ -1,6 +1,7 @@ /* eslint-disable no-unused-expressions */ /* eslint-disable max-len */ import chai from 'chai'; +import _ from 'lodash'; import request from 'supertest'; // import sleep from 'sleep'; import config from 'config'; @@ -29,6 +30,7 @@ const data = [ }, createdBy: 1, updatedBy: 1, + cancelReason: 'price/cost', lastActivityAt: 1, lastActivityUserId: '1', members: [ @@ -40,6 +42,7 @@ const data = [ firstName: 'Firstname', lastName: 'Lastname', handle: 'test_tourist_handle', + email: 'test@test.com', isPrimary: true, createdBy: 1, updatedBy: 1, @@ -62,6 +65,22 @@ const data = [ status: 'pending', }, ], + phases: [ + + { + id: 45, + name: 'test phases', + spentBudget: 0, + products: [ + { + + phaseId: 45, + id: 3, + name: 'tet product', + }, + ], + }, + ], attachments: [ { id: 1, @@ -109,6 +128,18 @@ const data = [ status: 'requested', }, ], + attachments: [ + { + id: 1, + title: 'Spec', + projectId: 1, + description: 'specification', + filePath: 'projects/1/spec.pdf', + contentType: 'application/pdf', + createdBy: 1, + updatedBy: 1, + }, + ], }, { id: 3, @@ -402,7 +433,7 @@ describe('LIST Project', () => { it('should return the project for administrator with field description and billingAccountId', (done) => { request(server) - .get('/v5/projects/?fields=description,billingAccountId&sort=id asc') + .get('/v5/projects/?fields=description,billingAccountId,attachments&sort=id asc') .set({ Authorization: `Bearer ${testUtil.jwts.admin}`, }) @@ -968,5 +999,269 @@ describe('LIST Project', () => { }); }); }); + + describe('URL Query fields', () => { + it('should not return "email" for project members when "fields" query param is not defined (to non-admin users)', (done) => { + request(server) + .get('/v5/projects/') + .set({ + Authorization: `Bearer ${testUtil.jwts.member2}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body; + should.exist(resJson); + resJson.should.have.lengthOf(1); + resJson[0].members[0].should.not.have.property('email'); + done(); + } + }); + }); + + + it('should not return "email" for project members even if it\'s defined in "fields" query param (to non-admin users)', (done) => { + request(server) + .get('/v5/projects/?fields=members.email,members.id') + .set({ + Authorization: `Bearer ${testUtil.jwts.member2}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body; + should.exist(resJson); + resJson.should.have.lengthOf(1); + resJson[0].members[0].should.not.have.property('email'); + done(); + } + }); + }); + + + it('should not return "cancelReason" if it is not listed in "fields" query param ', (done) => { + request(server) + .get('/v5/projects/?fields=description') + .set({ + Authorization: `Bearer ${testUtil.jwts.member2}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body; + should.exist(resJson); + resJson.should.have.lengthOf(1); + resJson[0].should.have.property('description'); + resJson[0].should.not.have.property('cancelReason'); + resJson[0].description.should.be.eq('test project1'); + done(); + } + }); + }); + + it('should not return "email" for project members when "fields" query param is not defined (to admin users)', (done) => { + request(server) + .get('/v5/projects/?fields=description,members.id') + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body; + should.exist(resJson); + const project = _.find(resJson, p => p.id === 1); + const member = _.find(project.members, m => m.id === 1); + member.should.not.have.property('email'); + done(); + } + }); + }); + + + it('should return "email" for project members if it\'s defined in "fields" query param (to admin users', (done) => { + request(server) + .get('/v5/projects/?fields=description,members.id,members.email') + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body; + should.exist(resJson); + const project = _.find(resJson, p => p.id === 1); + const member = _.find(project.members, m => m.id === 1); + member.should.have.property('email'); + member.email.should.be.eq('test@test.com'); + done(); + } + }); + }); + + it('should only return "id" field, when it\'s defined in "fields" query param', (done) => { + request(server) + .get('/v5/projects/?fields=id') + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body; + should.exist(resJson); + resJson[0].should.have.property('id'); + _.keys(resJson[0]).length.should.be.eq(1); + done(); + } + }); + }); + + it('should only return "invites.userId" field, when it\'s defined in "fields" query param', (done) => { + request(server) + .get('/v5/projects/?fields=invites.userId') + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body; + should.exist(resJson); + resJson[0].invites[0].should.have.property('userId'); + _.keys(resJson[0].invites[0]).length.should.be.eq(1); + done(); + } + }); + }); + + it('should only return "members.role" field, when it\'s defined in "fields" query param', (done) => { + request(server) + .get('/v5/projects/?fields=members.role') + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body; + should.exist(resJson); + resJson[0].members[0].should.have.property('role'); + _.keys(resJson[0].members[0]).length.should.be.eq(1); + done(); + } + }); + }); + + it('should only return "attachments.title" field, when it\'s defined in "fields" query param', (done) => { + request(server) + .get('/v5/projects/?fields=attachments.title') + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body; + should.exist(resJson); + resJson[0].attachments[0].should.have.property('title'); + _.keys(resJson[0].attachments[0]).length.should.be.eq(1); + done(); + } + }); + }); + + it('should only return "phases.name" field, when it\'s defined in "fields" query param', (done) => { + request(server) + .get('/v5/projects/?fields=phases.name') + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body; + should.exist(resJson); + const project = _.find(resJson, p => p.id === 1); + project.phases[0].should.have.property('name'); + _.keys(project.phases[0]).length.should.be.eq(1); + done(); + } + }); + }); + + it('should only return "phases.products.name" field, when it\'s defined in "fields" query param and "phases" is also defined', (done) => { + request(server) + .get('/v5/projects/?fields=phases.products.name,phases.name') + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body; + should.exist(resJson); + const project = _.find(resJson, p => p.id === 1); + project.phases[0].products[0].should.have.property('name'); + _.keys(project.phases[0].products[0]).length.should.be.eq(1); + done(); + } + }); + }); + + it('should not return "phases.products.name" field, when it\'s defined in "fields" query param but "phases" is not defined', (done) => { + request(server) + .get('/v5/projects/?fields=phases.products.name') + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body; + should.exist(resJson); + const project = _.find(resJson, p => p.id === 1); + project.should.not.have.property('phases'); + done(); + } + }); + }); + }); }); }); diff --git a/src/util.js b/src/util.js index 08e10e51..2c48d48e 100644 --- a/src/util.js +++ b/src/util.js @@ -230,22 +230,57 @@ _.assignIn(util, { if (queryFields.length) { // remove any inavlid fields fields.projects = _.intersection(queryFields, allowedFields.projects); - fields.project_members = _.filter(queryFields, f => f.indexOf('members.') === 0); - // remove members. prefix - fields.project_members = _.map(fields.project_members, f => f.substring(8)); - // remove any errorneous fields - fields.project_members = _.intersection(fields.project_members, allowedFields.project_members); - if (fields.project_members.length === 0 && _.indexOf(queryFields, 'members') > -1) { - fields.project_members = allowedFields.project_members; + + const parseSubFields = (name, strName) => { + fields[name] = _.filter(queryFields, f => f.indexOf(`${strName}.`) === 0); + fields[name] = _.map(fields[name], f => f.substring(strName.length + 1)); + fields[name] = _.intersection(fields[name], allowedFields[name]); + if (fields[name].length === 0 && _.indexOf(queryFields, strName) > -1) { + fields[name] = allowedFields[name]; + } + }; + + if (allowedFields.project_members) { + parseSubFields('project_members', 'members'); + } + if (allowedFields.project_member_invites) { + parseSubFields('project_member_invites', 'invites'); } - // remove attachments if not requested - if (fields.attachments && _.indexOf(queryFields, 'attachments') === -1) { - fields.attachments = null; + + if (allowedFields.attachments) { + parseSubFields('attachments', 'attachments'); + } + + if (allowedFields.project_phases) { + parseSubFields('project_phases', 'phases'); + } + + if (allowedFields.project_phases_products) { + if (fields.project_phases.length > 0) { + parseSubFields('project_phases_products', 'phases.products'); + } else { + // if donot have 'phases', so hide 'phases.products' + fields.project_phases_products = []; + } } } return fields; }, - + /** + * Remove email field for PROJECT_MEMBER_ATTRIBUTES, if user is not admin + * @param {object} req request object + * @param {object} fields fields object + * @return {object} the parsed array + */ + ignoreEmailField: (req, fields) => { + if (!fields.project_members) { return fields; } + const isAdmin = util.hasPermission({ topcoderRoles: ADMIN_ROLES }, req.authUser); + if (isAdmin) { + return fields; + } + _.assign(fields, { project_members: _.filter(fields.project_members, f => f !== 'email') }); + return fields; + }, /** * Parse the query filters * @param {String} fqueryFilter the query filter string From da40ec6eb98e552a0fda48eba94af725bb4b6ec3 Mon Sep 17 00:00:00 2001 From: Maksym Mykhailenko Date: Mon, 2 Mar 2020 11:51:55 +0800 Subject: [PATCH 2/4] fix: unit tests stability --- src/routes/projects/list.spec.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/routes/projects/list.spec.js b/src/routes/projects/list.spec.js index 6d342923..72acf1cc 100644 --- a/src/routes/projects/list.spec.js +++ b/src/routes/projects/list.spec.js @@ -282,6 +282,7 @@ describe('LIST Project', () => { type: ES_PROJECT_TYPE, id: project1.id, body: data[0], + refresh: 'wait_for', }); const esp2 = server.services.es.index({ @@ -289,6 +290,7 @@ describe('LIST Project', () => { type: ES_PROJECT_TYPE, id: project2.id, body: data[1], + refresh: 'wait_for', }); const esp3 = server.services.es.index({ @@ -296,6 +298,7 @@ describe('LIST Project', () => { type: ES_PROJECT_TYPE, id: project3.id, body: data[2], + refresh: 'wait_for', }); return Promise.all([esp1, esp2, esp3]); }).then(() => { From 4331abc44f9ad203f3304f57d38139e84398bb7e Mon Sep 17 00:00:00 2001 From: Maksym Mykhailenko Date: Mon, 2 Mar 2020 11:57:27 +0800 Subject: [PATCH 3/4] feat: return products if requested even if phases are not listed in fields --- src/routes/projects/get.spec.js | 21 --------------------- src/routes/projects/list.spec.js | 21 --------------------- src/util.js | 7 +------ 3 files changed, 1 insertion(+), 48 deletions(-) diff --git a/src/routes/projects/get.spec.js b/src/routes/projects/get.spec.js index bff5fd56..066c0b23 100644 --- a/src/routes/projects/get.spec.js +++ b/src/routes/projects/get.spec.js @@ -610,27 +610,6 @@ describe('GET Project', () => { } }); }); - - it('should not return "phases.products.name" field, when it\'s defined in "fields" query param but "phases" is not defined', (done) => { - request(server) - - .get(`/v5/projects/${project1.id}?fields=id,phases.products.name`) - .set({ - Authorization: `Bearer ${testUtil.jwts.admin}`, - }) - .expect('Content-Type', /json/) - .expect(200) - .end((err, res) => { - if (err) { - done(err); - } else { - const resJson = res.body; - should.exist(resJson); - resJson.should.not.have.property('phases'); - done(); - } - }); - }); }); }); }); diff --git a/src/routes/projects/list.spec.js b/src/routes/projects/list.spec.js index 72acf1cc..9ebb3e13 100644 --- a/src/routes/projects/list.spec.js +++ b/src/routes/projects/list.spec.js @@ -1244,27 +1244,6 @@ describe('LIST Project', () => { } }); }); - - it('should not return "phases.products.name" field, when it\'s defined in "fields" query param but "phases" is not defined', (done) => { - request(server) - .get('/v5/projects/?fields=phases.products.name') - .set({ - Authorization: `Bearer ${testUtil.jwts.admin}`, - }) - .expect('Content-Type', /json/) - .expect(200) - .end((err, res) => { - if (err) { - done(err); - } else { - const resJson = res.body; - should.exist(resJson); - const project = _.find(resJson, p => p.id === 1); - project.should.not.have.property('phases'); - done(); - } - }); - }); }); }); }); diff --git a/src/util.js b/src/util.js index 2c48d48e..91a584f3 100644 --- a/src/util.js +++ b/src/util.js @@ -256,12 +256,7 @@ _.assignIn(util, { } if (allowedFields.project_phases_products) { - if (fields.project_phases.length > 0) { - parseSubFields('project_phases_products', 'phases.products'); - } else { - // if donot have 'phases', so hide 'phases.products' - fields.project_phases_products = []; - } + parseSubFields('project_phases_products', 'phases.products'); } } return fields; From b3a128ad7016e8204e938179a2162070e94d41cf Mon Sep 17 00:00:00 2001 From: Maksym Mykhailenko Date: Mon, 2 Mar 2020 13:11:00 +0800 Subject: [PATCH 4/4] fix: return emails for members - return email for members, but only to Topcoder Admins - fix logic for not returning emails in invites by handle - emails are returned only to Topcoder Admins but not Connect Admins --- src/util.js | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/util.js b/src/util.js index 91a584f3..13158538 100644 --- a/src/util.js +++ b/src/util.js @@ -28,6 +28,7 @@ import { VALUE_TYPE, ESTIMATION_TYPE, RESOURCES, + USER_ROLE, } from './constants'; const tcCoreLibAuth = require('tc-core-library-js').auth; @@ -268,12 +269,18 @@ _.assignIn(util, { * @return {object} the parsed array */ ignoreEmailField: (req, fields) => { - if (!fields.project_members) { return fields; } - const isAdmin = util.hasPermission({ topcoderRoles: ADMIN_ROLES }, req.authUser); - if (isAdmin) { + if (!fields.project_members) { + return fields; + } + + // Only Topcoder Admins can get all the fields + if (util.hasPermission({ topcoderRoles: [USER_ROLE.TOPCODER_ADMIN] }, req.authUser)) { return fields; } + + // for non topcoder admins remove emails from the field list _.assign(fields, { project_members: _.filter(fields.project_members, f => f !== 'email') }); + return fields; }, /** @@ -628,7 +635,7 @@ _.assignIn(util, { // uncomment code below, to enable masking emails again /* - const isAdmin = util.hasPermission({ topcoderRoles: ADMIN_ROLES }, req.authUser); + const isAdmin = util.hasPermission({ topcoderRoles: [USER_ROLE.TOPCODER_ADMIN] }, req.authUser); if (isAdmin) { return data; } @@ -660,6 +667,11 @@ _.assignIn(util, { const memberTraitFields = ['photoURL', 'workingHourStart', 'workingHourEnd', 'timeZone']; const memberDetailFields = ['handle', 'firstName', 'lastName']; + // Only Topcoder admins can get emails for users + if (util.hasPermission({ topcoderRoles: [USER_ROLE.TOPCODER_ADMIN] }, req.authUser)) { + memberDetailFields.push('email'); + } + let allMemberDetails = []; if (_.intersection(fields, _.union(memberDetailFields, memberTraitFields)).length > 0) { const userIds = _.reject(_.map(members, 'userId'), _.isNil); // some invites may have no `userId` @@ -711,15 +723,16 @@ _.assignIn(util, { // pick valid fields from fetched member details return _.map(members, (member) => { let memberDetails = _.find(allMemberDetails, ({ userId }) => userId === member.userId); - memberDetails = _.assign({}, member, memberDetails); + memberDetails = _.assign({}, member, _.pick(memberDetails, _.union(memberDetailFields, memberTraitFields))); // this case would be only valid for invites: // don't return `email` for non-admins if invitation has `userId` // if invitation doesn't have `userId` means it is invitation by email // then we are still returning emails to all users if ( + memberDetails.status && // this is how we identify that the object is "invite" and not a "member" memberDetails.email && memberDetails.userId && - !util.hasPermission({ topcoderRoles: ADMIN_ROLES }, req.authUser) + !util.hasPermission({ topcoderRoles: [USER_ROLE.TOPCODER_ADMIN] }, req.authUser) ) { delete memberDetails.email; }