-
Notifications
You must be signed in to change notification settings - Fork 55
[PROD] - Copilot Portal fixes and updates #838
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
base: master
Are you sure you want to change the base?
Changes from all commits
99220ae
9b7b337
8f2fbd5
2218b14
c7578c5
9783ab1
85bc8da
b360f9d
4895bce
473b060
b63b2ad
191cc98
7ead77e
1a8d9f2
e6f3595
f63c580
2f46d05
2201ec4
36c6e7b
cf50f64
56eb2b6
fed04b8
64091d5
eaf2116
5bf08be
a2a668b
bd92a3b
65e943e
624cad5
f79f42b
7b4edde
8a823f2
7b853be
682c2b3
9c9edce
97becbb
cc15920
8218b87
25b8b34
7831f54
57a8d00
1b21d3b
fa13330
0536910
b67a6ba
ff30126
f38a2b6
2071d45
f26de03
50d2dac
347caae
d4e4033
ecb836c
279d2f1
3998114
b7c5f95
bebe265
b7c7f34
30ea264
9385156
fdb09e3
2dc0ea2
13c8c39
f73f444
7fce661
1f2cba4
1266652
c701b1e
1d19d15
af842b0
b2ac65f
0d02782
ce575c7
2f2e162
287e14f
21977ef
b4dbde5
fc0bffe
68522e0
ccceb70
d56f233
37f740c
993ea57
b3e4f8e
45629b5
5025fa1
7075d3f
4a2a671
683b3d6
f0a889f
4823330
26dc288
59c6077
2ba33ac
04b4e37
f2a8c89
76a22c1
cf120a8
471d5e1
1559e31
44ac842
de8c14f
40076d3
d40373d
8b2fa3e
e7a328b
ab268ca
e82e6b2
dad92bc
ada10e8
c2df9ee
6122446
65d9a7e
54bff16
f8cf3b4
ab5e2f0
eb88f39
8587a8e
8082112
82dee4e
c63ba9c
b1d7b40
b9f8e59
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -56,6 +56,7 @@ | |
"workManagerUrl": "https://challenges.topcoder-dev.com", | ||
"copilotPortalUrl": "https://copilots.topcoder-dev.com", | ||
"accountsAppUrl": "https://accounts.topcoder-dev.com", | ||
"copilotsSlackEmail": "[email protected]", | ||
"MAX_REVISION_NUMBER": 100, | ||
"UNIQUE_GMAIL_VALIDATION": false, | ||
"pageSize": 20, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,5 +4,6 @@ | |
"copilotPortalUrl": "https://copilots.topcoder.com", | ||
"sfdcBillingAccountNameField": "Billing_Account_name__c", | ||
"sfdcBillingAccountMarkupField": "Mark_up__c", | ||
"sfdcBillingAccountActiveField": "Active__c" | ||
"sfdcBillingAccountActiveField": "Active__c", | ||
"copilotsSlackEmail": "[email protected]" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,15 @@ | ||
import _ from 'lodash'; | ||
import validate from 'express-validation'; | ||
import Joi from 'joi'; | ||
import config from 'config'; | ||
|
||
import models from '../../models'; | ||
import util from '../../util'; | ||
import { PERMISSION } from '../../permissions/constants'; | ||
import { COPILOT_APPLICATION_STATUS, COPILOT_OPPORTUNITY_STATUS, COPILOT_REQUEST_STATUS, EVENT, INVITE_STATUS, PROJECT_MEMBER_ROLE, RESOURCES } from '../../constants'; | ||
import { CONNECT_NOTIFICATION_EVENT, COPILOT_APPLICATION_STATUS, COPILOT_OPPORTUNITY_STATUS, COPILOT_REQUEST_STATUS, EVENT, INVITE_STATUS, PROJECT_MEMBER_ROLE, RESOURCES, TEMPLATE_IDS } from '../../constants'; | ||
import { getCopilotTypeLabel } from '../../utils/copilot'; | ||
import { createEvent } from '../../services/busApi'; | ||
import moment from 'moment'; | ||
|
||
const assignCopilotOpportunityValidations = { | ||
body: Joi.object().keys({ | ||
|
@@ -45,11 +49,17 @@ module.exports = [ | |
throw err; | ||
} | ||
|
||
const copilotRequest = await models.CopilotRequest.findOne({ | ||
where: { id: opportunity.copilotRequestId }, | ||
transaction: t, | ||
}); | ||
|
||
const application = await models.CopilotApplication.findOne({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure that the |
||
where: { id: applicationId, opportunityId: copilotOpportunityId }, | ||
transaction: t, | ||
}); | ||
|
||
|
||
if (!application) { | ||
const err = new Error('No such application available'); | ||
err.status = 400; | ||
|
@@ -65,12 +75,101 @@ module.exports = [ | |
const projectId = opportunity.projectId; | ||
const userId = application.userId; | ||
const activeMembers = await models.ProjectMember.getActiveProjectMembers(projectId, t); | ||
|
||
const existingUser = activeMembers.find(item => item.userId === userId); | ||
if (existingUser && existingUser.role === 'copilot') { | ||
const err = new Error(`User is already a copilot of this project`); | ||
err.status = 400; | ||
throw err; | ||
const updateCopilotOpportunity = async () => { | ||
const transaction = await models.sequelize.transaction(); | ||
const memberDetails = await util.getMemberDetailsByUserIds([application.userId], req.log, req.id); | ||
const member = memberDetails[0]; | ||
req.log.debug(`Updating opportunity: ${JSON.stringify(opportunity)}`); | ||
await opportunity.update({ | ||
status: COPILOT_OPPORTUNITY_STATUS.COMPLETED, | ||
}, { | ||
transaction, | ||
}); | ||
req.log.debug(`Updating application: ${JSON.stringify(application)}`); | ||
await application.update({ | ||
status: COPILOT_APPLICATION_STATUS.ACCEPTED, | ||
}, { | ||
transaction, | ||
}); | ||
|
||
req.log.debug(`Updating request: ${JSON.stringify(copilotRequest)}`); | ||
await copilotRequest.update({ | ||
status: COPILOT_REQUEST_STATUS.FULFILLED, | ||
}, { | ||
transaction, | ||
}); | ||
|
||
req.log.debug(`Updating other applications: ${JSON.stringify(copilotRequest)}`); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The debug log message here is misleading. It mentions updating 'other applications' but logs the |
||
await models.CopilotApplication.update({ | ||
status: COPILOT_APPLICATION_STATUS.CANCELED, | ||
}, { | ||
where: { | ||
opportunityId: opportunity.id, | ||
id: { | ||
$ne: application.id, | ||
}, | ||
} | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider adding a |
||
|
||
req.log.debug(`All updations done`); | ||
transaction.commit(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider using |
||
|
||
req.log.debug(`Sending email notification`); | ||
const emailEventType = CONNECT_NOTIFICATION_EVENT.EXTERNAL_ACTION_EMAIL; | ||
const copilotPortalUrl = config.get('copilotPortalUrl'); | ||
const requestData = copilotRequest.data; | ||
createEvent(emailEventType, { | ||
data: { | ||
opportunity_details_url: `${copilotPortalUrl}/opportunity/${opportunity.id}`, | ||
work_manager_url: config.get('workManagerUrl'), | ||
opportunity_type: getCopilotTypeLabel(requestData.projectType), | ||
opportunity_title: requestData.opportunityTitle, | ||
start_date: moment.utc(requestData.startDate).format('DD-MM-YYYY'), | ||
user_name: member ? member.handle : "", | ||
}, | ||
sendgrid_template_id: TEMPLATE_IDS.COPILOT_ALREADY_PART_OF_PROJECT, | ||
recipients: [member.email], | ||
version: 'v3', | ||
}, req.log); | ||
|
||
req.log.debug(`Email sent`); | ||
}; | ||
|
||
const existingMember = activeMembers.find(item => item.userId === userId); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The variable |
||
if (existingMember) { | ||
req.log.debug(`User already part of project: ${JSON.stringify(existingMember)}`); | ||
if (['copilot', 'manager'].includes(existingMember.role)) { | ||
req.log.debug(`User is a copilot or manager`); | ||
await updateCopilotOpportunity(); | ||
} else { | ||
req.log.debug(`User has read/write role`); | ||
await models.ProjectMember.update({ | ||
role: 'copilot', | ||
}, { | ||
where: { | ||
id: existingMember.id, | ||
}, | ||
}); | ||
|
||
const projectMember = await models.ProjectMember.findOne({ | ||
where: { | ||
id: existingMember.id, | ||
}, | ||
}); | ||
|
||
req.log.debug(`Updated project member: ${JSON.stringify(projectMember.get({plain: true}))}`); | ||
|
||
util.sendResourceToKafkaBus( | ||
req, | ||
EVENT.ROUTING_KEY.PROJECT_MEMBER_UPDATED, | ||
RESOURCES.PROJECT_MEMBER, | ||
projectMember.get({ plain: true }), | ||
existingMember); | ||
req.log.debug(`Member updated in kafka`); | ||
await updateCopilotOpportunity(); | ||
} | ||
res.status(200).send({ id: applicationId }); | ||
return; | ||
} | ||
|
||
const existingInvite = await models.ProjectMemberInvite.findAll({ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,12 @@ | ||
import _ from 'lodash'; | ||
import { Op } from 'sequelize'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
|
||
import models from '../../models'; | ||
import util from '../../util'; | ||
import { COPILOT_APPLICATION_STATUS, COPILOT_OPPORTUNITY_STATUS, COPILOT_REQUEST_STATUS } from '../../constants'; | ||
import { COPILOT_APPLICATION_STATUS, COPILOT_OPPORTUNITY_STATUS, COPILOT_REQUEST_STATUS, EVENT, INVITE_STATUS, RESOURCES } from '../../constants'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The constants |
||
import { PERMISSION } from '../../permissions/constants'; | ||
|
||
|
||
module.exports = [ | ||
(req, res, next) => { | ||
if (!util.hasPermissionByReq(PERMISSION.CANCEL_COPILOT_OPPORTUNITY, req)) { | ||
|
@@ -54,6 +56,14 @@ module.exports = [ | |
})); | ||
}); | ||
|
||
const allInvites = await models.ProjectMemberInvite.findAll({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider adding error handling for the database query to ensure that any issues with fetching |
||
where: { | ||
applicationId: { | ||
[Op.in]: applications.map(item => item.id), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure that |
||
}, | ||
}, | ||
}); | ||
|
||
await Promise.all(promises); | ||
|
||
await copilotRequest.update({ | ||
|
@@ -68,6 +78,21 @@ module.exports = [ | |
transaction, | ||
}); | ||
|
||
// update all the existing invites which are | ||
// associated to the copilot opportunity | ||
// with cancel status | ||
for (const invite of allInvites) { | ||
await invite.update({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider adding error handling for the |
||
status: INVITE_STATUS.CANCELED, | ||
}); | ||
await invite.reload(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider adding error handling for the |
||
util.sendResourceToKafkaBus( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider adding error handling for the |
||
req, | ||
EVENT.ROUTING_KEY.PROJECT_MEMBER_INVITE_UPDATED, | ||
RESOURCES.PROJECT_MEMBER_INVITE, | ||
invite.toJSON()); | ||
} | ||
|
||
res.status(200).send({ id: opportunity.id }); | ||
}) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,25 @@ module.exports = [ | |
const pageSize = parseInt(req.query.pageSize, 10) || DEFAULT_PAGE_SIZE; | ||
const offset = (page - 1) * pageSize; | ||
const limit = pageSize; | ||
const noGroupingByStatus = req.query.noGrouping === 'true'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider using a more descriptive variable name than |
||
|
||
const baseOrder = []; | ||
|
||
// If grouping is enabled (default), add custom ordering based on status | ||
if (!noGroupingByStatus) { | ||
baseOrder.push([ | ||
models.Sequelize.literal(` | ||
CASE | ||
WHEN "CopilotOpportunity"."status" = 'active' THEN 0 | ||
WHEN "CopilotOpportunity"."status" = 'cancelled' THEN 1 | ||
WHEN "CopilotOpportunity"."status" = 'completed' THEN 2 | ||
ELSE 3 | ||
END | ||
`), | ||
'ASC', | ||
]); | ||
} | ||
baseOrder.push([sortParams[0], sortParams[1]]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure |
||
|
||
return models.CopilotOpportunity.findAll({ | ||
include: [ | ||
|
@@ -34,7 +53,7 @@ module.exports = [ | |
attributes: ['name'], | ||
}, | ||
], | ||
order: [[sortParams[0], sortParams[1]]], | ||
order: baseOrder, | ||
limit, | ||
offset, | ||
}) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,10 +8,11 @@ import util from '../../util'; | |
import { PERMISSION } from '../../permissions/constants'; | ||
import { CONNECT_NOTIFICATION_EVENT, COPILOT_OPPORTUNITY_STATUS, TEMPLATE_IDS, USER_ROLE } from '../../constants'; | ||
import { createEvent } from '../../services/busApi'; | ||
import { getCopilotTypeLabel } from '../../utils/copilot'; | ||
|
||
const applyCopilotRequestValidations = { | ||
body: Joi.object().keys({ | ||
notes: Joi.string().optional(), | ||
notes: Joi.string().required(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
}), | ||
}; | ||
|
||
|
@@ -41,6 +42,12 @@ module.exports = [ | |
where: { | ||
id: copilotOpportunityId, | ||
}, | ||
include: [ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider verifying that the |
||
{ | ||
model: models.CopilotRequest, | ||
as: 'copilotRequest', | ||
}, | ||
], | ||
}).then(async (opportunity) => { | ||
if (!opportunity) { | ||
const err = new Error('No opportunity found'); | ||
|
@@ -92,12 +99,15 @@ module.exports = [ | |
|
||
const emailEventType = CONNECT_NOTIFICATION_EVENT.EXTERNAL_ACTION_EMAIL; | ||
const copilotPortalUrl = config.get('copilotPortalUrl'); | ||
const requestData = opportunity.copilotRequest.data; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider checking if |
||
listOfSubjects.forEach((subject) => { | ||
createEvent(emailEventType, { | ||
data: { | ||
user_name: subject.handle, | ||
opportunity_details_url: `${copilotPortalUrl}/opportunity/${opportunity.id}#applications`, | ||
work_manager_url: config.get('workManagerUrl'), | ||
opportunity_type: getCopilotTypeLabel(requestData.projectType), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure that |
||
opportunity_title: requestData.opportunityTitle, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider validating |
||
}, | ||
sendgrid_template_id: TEMPLATE_IDS.APPLY_COPILOT, | ||
recipients: [subject.email], | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,19 +31,68 @@ module.exports = [ | |
canAccessAllApplications ? {} : { createdBy: userId }, | ||
); | ||
|
||
return models.CopilotApplication.findAll({ | ||
where: whereCondition, | ||
include: [ | ||
{ | ||
model: models.CopilotOpportunity, | ||
as: 'copilotOpportunity', | ||
}, | ||
], | ||
order: [[sortParams[0], sortParams[1]]], | ||
return models.CopilotOpportunity.findOne({ | ||
where: { | ||
id: opportunityId, | ||
} | ||
}).then((opportunity) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider adding a timeout or handling for the promise returned by |
||
if (!opportunity) { | ||
const err = new Error('No opportunity found'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The error message 'No opportunity found' could be more descriptive by including the |
||
err.status = 404; | ||
throw err; | ||
} | ||
return models.CopilotApplication.findAll({ | ||
where: whereCondition, | ||
include: [ | ||
{ | ||
model: models.CopilotOpportunity, | ||
as: 'copilotOpportunity', | ||
}, | ||
], | ||
order: [[sortParams[0], sortParams[1]]], | ||
}) | ||
.then(copilotApplications => { | ||
req.log.debug(`CopilotApplications ${JSON.stringify(copilotApplications)}`); | ||
return models.ProjectMember.getActiveProjectMembers(opportunity.projectId).then((members) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider adding error handling for the promise returned by |
||
req.log.debug(`Fetched existing active members ${JSON.stringify(members)}`); | ||
req.log.debug(`Applications ${JSON.stringify(copilotApplications)}`); | ||
const enrichedApplications = copilotApplications.map(application => { | ||
const m = members.find(m => m.userId === application.userId); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The variable |
||
|
||
// Using spread operator fails in lint check | ||
// While Object.assign fails silently during run time | ||
// So using this method | ||
const enriched = { | ||
id: application.id, | ||
opportunityId: application.opportunityId, | ||
notes: application.notes, | ||
status: application.status, | ||
userId: application.userId, | ||
deletedAt: application.deletedAt, | ||
createdAt: application.createdAt, | ||
updatedAt: application.updatedAt, | ||
deletedBy: application.deletedBy, | ||
createdBy: application.createdBy, | ||
updatedBy: application.updatedBy, | ||
copilotOpportunity: application.copilotOpportunity, | ||
}; | ||
|
||
if (m) { | ||
enriched.existingMembership = m; | ||
} | ||
|
||
req.log.debug(`Existing member to application ${JSON.stringify(enriched)}`); | ||
|
||
return enriched; | ||
}); | ||
|
||
req.log.debug(`Enriched Applications ${JSON.stringify(enrichedApplications)}`); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider using |
||
res.status(200).send(enrichedApplications); | ||
}); | ||
}) | ||
}) | ||
.then(copilotApplications => res.json(copilotApplications)) | ||
.catch((err) => { | ||
util.handleError('Error fetching copilot applications', err, req, next); | ||
}); | ||
.catch((err) => { | ||
util.handleError('Error fetching copilot applications', err, req, next); | ||
}); | ||
}, | ||
]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider handling the case where
copilotRequest
might benull
orundefined
after thefindOne
query. This could prevent potential runtime errors if the expected data is not found.