Skip to content

Copilot workflow changes #89

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 4 commits into from
Feb 21, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions connect/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ module.exports = {
// These variables are currently being used to retrieve above role members using API V3 `/roles` endpoint.
// As soon as this endpoint is replaced with more suitable one, these variables has to be removed if no need anymore.
CONNECT_MANAGER_ROLE_ID: 8,
CONNECT_COPILOT_MANAGER_ROLE_ID: 113,

Choose a reason for hiding this comment

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

@gondzo I forgot how do we manage these ids when we test the application in dev env?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We just use what is set in the admin app - role ids should be consistent between dev and prod

CONNECT_COPILOT_ROLE_ID: 4,
ADMINISTRATOR_ROLE_ID: 1,
// id of the BOT user which creates post with various events in discussions
Expand Down
31 changes: 30 additions & 1 deletion connect/connectNotificationServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,34 @@ const getNotificationsForMentionedUser = (eventConfig, content) => {
});
};

/**
* Get notifications for users obtained from createdBy
*
* @param {Object} eventConfig event configuration
* @param {String} createdBy created by
*
* @return {Promise} resolves to a list of notifications
*/
const getNotificationsForCreatedBy = (eventConfig, createdBy) => {
// if event doesn't have to be notified to creator, just ignore
if (!eventConfig.creator) {
return Promise.resolve([]);
}

// if we have to send notification to the creator,
// but it's not provided in the message, then throw error
if (!createdBy) {
return Promise.reject(new Error('Missing createdBy in the event message.'));
}

return Promise.resolve([{
userId: createdBy.toString(),
contents: {
creator: true,
},
}]);
};

/**
* Get project members notifications
*
Expand Down Expand Up @@ -307,6 +335,7 @@ const handler = (topic, message, logger, callback) => {
// - check that event has everything required or throw error
getNotificationsForTopicStarter(eventConfig, message.topicId),

Choose a reason for hiding this comment

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

@gondzo we might reuse the logic of originator, which I mentioned in other comment, here to be generic enough to send notifications to the originator.

getNotificationsForUserId(eventConfig, message.userId),
getNotificationsForCreatedBy(eventConfig, message.createdBy),
getNotificationsForMentionedUser(eventConfig, message.postContent),
getProjectMembersNotifications(eventConfig, project),
getTopCoderMembersNotifications(eventConfig),
Expand All @@ -321,7 +350,7 @@ const handler = (topic, message, logger, callback) => {
)).then((notifications) => {
allNotifications = _.filter(notifications, notification => notification.userId !== `${message.initiatorUserId}`);

if (eventConfig.includeUsers && message[eventConfig.includeUsers] && message[eventConfig.includeUsers].length>0){
if (eventConfig.includeUsers && message[eventConfig.includeUsers] && message[eventConfig.includeUsers].length > 0) {
allNotifications = _.filter(allNotifications, notification => message[eventConfig.includeUsers].contains(notification.userId));
}

Expand Down
3 changes: 3 additions & 0 deletions connect/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ module.exports = {
ASSIGNED_AS_OWNER: 'notifications.connect.project.member.assignedAsOwner',
INVITE_CREATED: 'notifications.connect.project.member.invite.created',
INVITE_UPDATED: 'notifications.connect.project.member.invite.updated',
INVITE_REQUESTED: 'notifications.connect.project.member.invite.requested',
COPILOT_ADDED: 'notifications.connect.project.member.copilot.added',

Choose a reason for hiding this comment

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

@gondzo I would like to rename this event to INVITE_APPROVED : notifications.connect.project.member.invite.approved

COPILOT_REFUSED: 'notifications.connect.project.member.copilot.refused',

Choose a reason for hiding this comment

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

I would like to rename this event to INVITE_REJECTED : notifications.connect.project.member.invite.rejected

},
PROJECT: {
ACTIVE: 'notifications.connect.project.active',
Expand Down
25 changes: 20 additions & 5 deletions connect/events-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,14 @@ const PROJECT_ROLE_RULES = {
// TopCoder roles
const ROLE_CONNECT_COPILOT = 'Connect Copilot';
const ROLE_CONNECT_MANAGER = 'Connect Manager';
const ROLE_CONNECT_COPILOT_MANAGER = 'Connect Copilot Manager';
const ROLE_ADMINISTRATOR = 'administrator';

// TopCoder role rules
const TOPCODER_ROLE_RULES = {
[ROLE_CONNECT_COPILOT]: { id: config.CONNECT_COPILOT_ROLE_ID },
[ROLE_CONNECT_MANAGER]: { id: config.CONNECT_MANAGER_ROLE_ID },
[ROLE_CONNECT_COPILOT_MANAGER]: { id: config.CONNECT_COPILOT_MANAGER_ROLE_ID },
[ROLE_ADMINISTRATOR]: { id: config.ADMINISTRATOR_ROLE_ID },
};

Expand Down Expand Up @@ -107,6 +109,16 @@ const EVENTS = [
type: BUS_API_EVENT.CONNECT.MEMBER.INVITE_CREATED,
projectRoles: [],
toUserHandle: true,
}, {
type: BUS_API_EVENT.CONNECT.MEMBER.INVITE_REQUESTED,
topcoderRoles: [ROLE_CONNECT_COPILOT_MANAGER],
}, {
type: BUS_API_EVENT.CONNECT.MEMBER.COPILOT_ADDED,
toUserHandle: true,
creator: true

Choose a reason for hiding this comment

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

@gondzo Can we have better naming for creator? I mean creator is too generic to relate to any event. My example would be something like originator (not be confused with initiatorId which is the user who is performing the current operation)

}, {
type: BUS_API_EVENT.CONNECT.MEMBER.COPILOT_REFUSED,
creator: true
},

// Project activity
Expand Down Expand Up @@ -149,7 +161,7 @@ const EVENTS = [
type: BUS_API_EVENT.CONNECT.PROJECT.FILE_UPLOADED,
version: 2,
projectRoles: [PROJECT_ROLE_OWNER, PROJECT_ROLE_COPILOT, PROJECT_ROLE_MANAGER, PROJECT_ROLE_MEMBER],
includeUsers: 'allowedUsers'
includeUsers: 'allowedUsers',
}, {
type: BUS_API_EVENT.CONNECT.PROJECT.SPECIFICATION_MODIFIED,
version: 2,
Expand All @@ -160,12 +172,12 @@ const EVENTS = [
}, {
type: BUS_API_EVENT.CONNECT.PROJECT_PLAN.MODIFIED,
projectRoles: [PROJECT_ROLE_OWNER, PROJECT_ROLE_COPILOT, PROJECT_ROLE_MANAGER, PROJECT_ROLE_MEMBER],
includeUsers: 'allowedUsers'
includeUsers: 'allowedUsers',
}, {
type: BUS_API_EVENT.CONNECT.PROJECT_PLAN.PROGRESS_UPDATED,
projectRoles: [PROJECT_ROLE_OWNER, PROJECT_ROLE_COPILOT, PROJECT_ROLE_MANAGER, PROJECT_ROLE_MEMBER],
},

// Phase activity
{
type: BUS_API_EVENT.CONNECT.PROJECT_PLAN.PHASE_ACTIVATED,
Expand Down Expand Up @@ -200,8 +212,8 @@ const EVENTS = [
}, {
type: BUS_API_EVENT.CONNECT.PROJECT_PLAN.TIMELINE_ADJUSTED,
projectRoles: [PROJECT_ROLE_OWNER, PROJECT_ROLE_COPILOT, PROJECT_ROLE_MANAGER, PROJECT_ROLE_MEMBER],
includeUsers: 'allowedUsers'
}
includeUsers: 'allowedUsers',
},
];

const EVENT_BUNDLES = {
Expand Down Expand Up @@ -263,6 +275,9 @@ const EVENT_BUNDLES = {
BUS_API_EVENT.CONNECT.MEMBER.MANAGER_JOINED,
BUS_API_EVENT.CONNECT.MEMBER.REMOVED,
BUS_API_EVENT.CONNECT.MEMBER.INVITE_CREATED,
BUS_API_EVENT.CONNECT.MEMBER.INVITE_REQUESTED,
BUS_API_EVENT.CONNECT.MEMBER.COPILOT_ADDED,
BUS_API_EVENT.CONNECT.MEMBER.COPILOT_REFUSED,
],
},
PROJECT_PLAN: {
Expand Down
19 changes: 16 additions & 3 deletions docs/tc-notification-server-api.postman_collection.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
"description": "",
"auth": null,
"events": null,
"collection": "3f30c4e3-3b7a-491b-bdb2-6629d081a452",
"collectionId": "3f30c4e3-3b7a-491b-bdb2-6629d081a452",
"folder": null,
"order": [
"1b3b6480-ea94-4027-8898-f82f28e2bea6",
Expand All @@ -40,6 +40,7 @@
"requests": [
{
"id": "19332a51-03e8-4f5c-8f85-4d28d6dfe6f4",
"collectionId": "3f30c4e3-3b7a-491b-bdb2-6629d081a452",
"name": "getSettings",
"url": "{{URL}}/settings",
"description": "",
Expand Down Expand Up @@ -71,6 +72,7 @@
},
{
"id": "1b3b6480-ea94-4027-8898-f82f28e2bea6",
"collectionId": "3f30c4e3-3b7a-491b-bdb2-6629d081a452",
"name": "listNotifications - invalid read filter",
"url": "{{URL}}/list?offset=0&limit=20&type=notifications.connect.project.updated&read=yes",
"description": "",
Expand Down Expand Up @@ -131,6 +133,7 @@
},
{
"id": "543cab06-2c7d-4aed-8cf3-0808463254d5",
"collectionId": "3f30c4e3-3b7a-491b-bdb2-6629d081a452",
"name": "markAllRead",
"url": "{{URL}}/read",
"description": "",
Expand Down Expand Up @@ -162,6 +165,7 @@
},
{
"id": "59fc9f2b-28c5-4cff-b21b-11ab51bf67d8",
"collectionId": "3f30c4e3-3b7a-491b-bdb2-6629d081a452",
"name": "getSettings - invalid token",
"url": "{{URL}}/settings",
"description": "",
Expand Down Expand Up @@ -193,6 +197,7 @@
},
{
"id": "76779830-a8a4-4636-8c03-1801b3d1863d",
"collectionId": "3f30c4e3-3b7a-491b-bdb2-6629d081a452",
"name": "markAsRead",
"url": "{{URL}}/1/read",
"description": "",
Expand Down Expand Up @@ -226,6 +231,7 @@
"id": "cb2299a5-dac7-4c40-80c4-7b1694138354",
"name": "TC API - get project",
"url": "https://api.topcoder-dev.com/v4/projects/1936",
"collectionId": "3f30c4e3-3b7a-491b-bdb2-6629d081a452",
"description": "",
"data": [],
"dataMode": "raw",
Expand Down Expand Up @@ -339,7 +345,7 @@
],
"cookies": [],
"request": "cb2299a5-dac7-4c40-80c4-7b1694138354",
"collection": "3f30c4e3-3b7a-491b-bdb2-6629d081a452"
"collectionId": "3f30c4e3-3b7a-491b-bdb2-6629d081a452"
}
],
"rawModeData": "",
Expand All @@ -351,6 +357,7 @@
"name": "markAsRead - not found",
"url": "{{URL}}/1111111/read",
"description": "",
"collectionId": "3f30c4e3-3b7a-491b-bdb2-6629d081a452",
"data": [],
"dataMode": "raw",
"headerData": [
Expand Down Expand Up @@ -380,6 +387,7 @@
{
"id": "d293d2c5-230d-4f34-8c97-1adc1f2f89b4",
"name": "listNotifications - invalid limit",
"collectionId": "3f30c4e3-3b7a-491b-bdb2-6629d081a452",
"url": "{{URL}}/list?offset=0&limit=abc&type=notifications.connect.project.updated",
"description": "",
"data": [],
Expand Down Expand Up @@ -441,6 +449,7 @@
"id": "d57ba947-a5e7-410a-b978-76882f33c86e",
"name": "updateSettings",
"url": "{{URL}}/settings",
"collectionId": "3f30c4e3-3b7a-491b-bdb2-6629d081a452",
"description": "",
"data": [],
"dataMode": "raw",
Expand Down Expand Up @@ -471,6 +480,7 @@
{
"id": "da23d550-55b3-4f7d-9131-735956d62f6d",
"name": "markAllRead - missing token",
"collectionId": "3f30c4e3-3b7a-491b-bdb2-6629d081a452",
"url": "{{URL}}/read",
"description": "",
"data": [],
Expand All @@ -495,6 +505,7 @@
},
{
"id": "f2246cf7-7aae-4ea0-9d92-1d932d340302",
"collectionId": "3f30c4e3-3b7a-491b-bdb2-6629d081a452",
"name": "updateSettings - invalid body",
"url": "{{URL}}/settings",
"description": "",
Expand Down Expand Up @@ -527,6 +538,7 @@
{
"id": "f3f3a847-46f6-4059-b167-b436078fb112",
"name": "listNotifications - invalid offset",
"collectionId": "3f30c4e3-3b7a-491b-bdb2-6629d081a452",
"url": "{{URL}}/list?offset=-1&limit=20&type=notifications.connect.project.updated",
"description": "",
"data": [],
Expand Down Expand Up @@ -586,6 +598,7 @@
},
{
"id": "fce69847-5bf8-4b07-bcaf-6352db4ba923",
"collectionId": "3f30c4e3-3b7a-491b-bdb2-6629d081a452",
"name": "listNotifications",
"url": "{{URL}}/list?offset=0&limit=20",
"description": "",
Expand Down Expand Up @@ -645,4 +658,4 @@
"pathVariables": {}
}
]
}
}
23 changes: 18 additions & 5 deletions emails/src/partials/project-team.html
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
<td class="main-td">
<table class="main-child">
<tr><td></td></tr>
</table>
</table>
</td>
</tr>

Expand Down Expand Up @@ -60,12 +60,25 @@
{{#if [notifications.connect.project.member.invite.created]}}
Hi <strong>{{userFullName}}</strong>, you are invited to join the project {{projectName}}. Please click on the button ("View project on Connect") below to join.
{{/if}}
{{#if [notifications.connect.project.member.invite.requested]}}
You are requested to add <strong>{{userFullName}}</strong> as a copilot
{{/if}}
{{#if [notifications.connect.project.member.copilot.added]}}

Choose a reason for hiding this comment

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

Just for keep track of, this would be updated as per event rename suggested above.

{{#if [creator]}}
Copy link
Contributor

Choose a reason for hiding this comment

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

@gondzo I think we need to rename this creator as well, need not we ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is done already @RishiRajSahu

Your request to add invite the copilot was approved
{{else}}
Hi <strong>{{userFullName}}</strong>, you are added as a copilot
{{/if}}
{{/if}}
{{#if [notifications.connect.project.member.copilot.refused]}}
Your request to add the copilot was refused
{{/if}}
</td>
</tr>
</table>
</td>
</tr>
</table>
</table>
</td>
</tr>
{{/each}}
Expand All @@ -76,7 +89,7 @@
<td class="main-td">
<table class="main-child">
<tr><td></td></tr>
</table>
</table>
</td>
</tr>
<tr class="button-row button-one">
Expand All @@ -98,7 +111,7 @@
<td class="main-td">
<table class="main-child">
<tr><td></td></tr>
</table>
</table>
</td>
</tr>
{{/if}}
{{/if}}