Skip to content

Copilot workflow changes #254

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export const USER_ROLE = {
MANAGER: 'Connect 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];
Expand Down Expand Up @@ -130,7 +131,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',
};

Expand Down
68 changes: 50 additions & 18 deletions src/events/busApi.js
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gondzo same as the issue mentioned in case of updated event below, should we bypass create event in case of requested event event thrown ?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we should skip raising the created event in this case.

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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gondzo should we send only copilot events only in this case and bypass the event on line https://github.com/topcoder-platform/tc-project-service/pull/254/files#diff-42ae3999a1a28855656e72e3e1e1ea71R727 ? otherwise I think it would result in duplicate emails being sent from tc-notifications ?

// 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);
}
});
};
2 changes: 1 addition & 1 deletion src/routes/projectMemberInvites/create.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we better check the status (v.status === 'pending') of the invite itself rather than depending of the role? We might have some future requirements where we might need to add more roles where we need workflow like this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, we don't send the email for copilots only - invite status is still the same regardless of role

sendInviteEmail(req, projectId, v);
}
});
Expand Down
14 changes: 10 additions & 4 deletions src/routes/projectMemberInvites/update.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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';
}

Expand All @@ -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,
Expand All @@ -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,
};
Expand Down
64 changes: 63 additions & 1 deletion src/routes/projectMemberInvites/update.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ describe('Project member invite update', () => {
let project1;
let invite1;
let invite2;
let invite3;

beforeEach((done) => {
testUtil.clearDb()
Expand Down Expand Up @@ -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();
});
});
});
});
Expand Down Expand Up @@ -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;

Expand Down