Skip to content
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
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ workflows:
context : org-global
filters:
branches:
only: ['develop', 'migration-setup', 'PM-1612', 'fix-project-exposing']
only: ['develop']

Choose a reason for hiding this comment

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

[⚠️ design]
The branch filter for the deployDev job has been changed to only include the develop branch. Ensure that this aligns with the intended deployment strategy, as it removes the ability to deploy from other branches like migration-setup, PM-1612, fix-2321, and pm-2435. This could impact the flexibility of testing and deploying changes from feature or bug-fix branches.

- deployProd:
context : org-global
filters:
Expand Down
2 changes: 1 addition & 1 deletion config/development.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"copilotPortalUrl": "https://copilots.topcoder-dev.com",
"fileServiceEndpoint": "https://api.topcoder-dev.com/v5/files",
"memberServiceEndpoint": "https://api.topcoder-dev.com/v5/members",
"identityServiceEndpoint": "https://api.topcoder-dev.com/v3/",
"identityServiceEndpoint": "https://api.topcoder-dev.com/v6/",
"taasJobApiUrl": "https://api.topcoder-dev.com/v5/jobs",
"sfdcBillingAccountNameField": "Billing_Account_name__c",
"sfdcBillingAccountMarkupField": "Mark_Up__c",
Expand Down
168 changes: 87 additions & 81 deletions src/routes/copilotRequest/approveRequest.service.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
import _ from 'lodash';
import config from 'config';
import moment from 'moment';
import { Op } from 'sequelize';

import models from '../../models';
import { CONNECT_NOTIFICATION_EVENT, COPILOT_OPPORTUNITY_STATUS, COPILOT_REQUEST_STATUS, TEMPLATE_IDS, USER_ROLE } from '../../constants';
import {
CONNECT_NOTIFICATION_EVENT,
COPILOT_OPPORTUNITY_STATUS,
COPILOT_REQUEST_STATUS,
TEMPLATE_IDS,
USER_ROLE,
} from '../../constants';
import util from '../../util';
import { createEvent } from '../../services/busApi';
import { getCopilotTypeLabel } from '../../utils/copilot';
Expand All @@ -17,85 +22,86 @@ const resolveTransaction = (transaction, callback) => {
return models.sequelize.transaction(callback);
};

module.exports = (req, data, existingTransaction) => {
module.exports = async (req, data, existingTransaction) => {
const { projectId, copilotRequestId, opportunityTitle, type, startDate } = data;

return resolveTransaction(existingTransaction, transaction =>
models.Project.findOne({
where: { id: projectId, deletedAt: { $eq: null } },
}, { transaction })
.then((existingProject) => {
if (!existingProject) {
const err = new Error(`active project not found for project id ${projectId}`);
err.status = 404;
throw err;
}
return models.CopilotRequest.findByPk(copilotRequestId, { transaction })
.then((existingCopilotRequest) => {
if (!existingCopilotRequest) {
const err = new Error(`no active copilot request found for copilot request id ${copilotRequestId}`);
err.status = 404;
throw err;
}

return existingCopilotRequest.update({
status: COPILOT_REQUEST_STATUS.APPROVED,
}, { transaction }).then(() => models.CopilotOpportunity
.findOne({
where: {
projectId,
type: data.type,
status: {
[Op.in]: [COPILOT_OPPORTUNITY_STATUS.ACTIVE],
}
},
})
.then((existingCopilotOpportunityOfSameType) => {
if (existingCopilotOpportunityOfSameType) {
const err = new Error('There\'s an active opportunity of same type already!');
_.assign(err, {
status: 403,
});
throw err;
}
return models.CopilotOpportunity
.create(data, { transaction });
}))
.then(async (opportunity) => {
const roles = await util.getRolesByRoleName(USER_ROLE.TC_COPILOT, req.log, req.id);
const { subjects = [] } = await util.getRoleInfo(roles[0], req.log, req.id);
const emailEventType = CONNECT_NOTIFICATION_EVENT.EXTERNAL_ACTION_EMAIL;
const copilotPortalUrl = config.get('copilotPortalUrl');
req.log.info("Sending emails to all copilots about new opportunity");

const sendNotification = (userName, recipient) => createEvent(emailEventType, {
data: {
user_name: userName,
opportunity_details_url: `${copilotPortalUrl}/opportunity/${opportunity.id}`,
work_manager_url: config.get('workManagerUrl'),
opportunity_type: getCopilotTypeLabel(type),
opportunity_title: opportunityTitle,
start_date: moment(startDate).format("DD-MM-YYYY"),
},
sendgrid_template_id: TEMPLATE_IDS.CREATE_REQUEST,
recipients: [recipient],
version: 'v3',
}, req.log);

subjects.forEach(subject => sendNotification(subject.handle, subject.email));

// send email to notify via slack
sendNotification('Copilots', config.copilotsSlackEmail);

req.log.info("Finished sending emails to copilots");

return opportunity;
})
.catch((err) => {
transaction.rollback();
return Promise.reject(err);
});
});
}),
);
return resolveTransaction(existingTransaction, async (transaction) => {
try {
const existingProject = await models.Project.findOne({
where: { id: projectId, deletedAt: { $eq: null } },
transaction,
});

if (!existingProject) {
const err = new Error(`active project not found for project id ${projectId}`);
err.status = 404;
throw err;
}

const copilotRequest = await models.CopilotRequest.findByPk(copilotRequestId, { transaction });

if (!copilotRequest) {
const err = new Error(`no active copilot request found for copilot request id ${copilotRequestId}`);
err.status = 404;
throw err;
}

await copilotRequest.update({ status: COPILOT_REQUEST_STATUS.APPROVED }, { transaction });

const existingOpportunity = await models.CopilotOpportunity.findOne({
where: {
projectId,
type: data.type,
status: { [Op.in]: [COPILOT_OPPORTUNITY_STATUS.ACTIVE] },
},
transaction,
});

if (existingOpportunity) {
const err = new Error('There\'s an active opportunity of same type already!');
err.status = 403;
throw err;
}

const opportunity = await models.CopilotOpportunity.create(data, { transaction });
req.log.debug('Created new copilot opportunity', { opportunityId: opportunity.id });

Choose a reason for hiding this comment

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

[⚠️ correctness]
Consider adding a check to ensure that opportunity is not null or undefined before logging its id. This will prevent potential runtime errors if create fails silently.


// Send notifications
try {
const roles = await util.getRolesByRoleName(USER_ROLE.TC_COPILOT, req.log, req.id);

const { subjects = [] } = await util.getRoleInfo(roles[0], req.log, req.id);
const emailEventType = CONNECT_NOTIFICATION_EVENT.EXTERNAL_ACTION_EMAIL;
const copilotPortalUrl = config.get('copilotPortalUrl');
req.log.info('Sending emails to all copilots about new opportunity');

const sendNotification = (userName, recipient) => createEvent(emailEventType, {
data: {
user_name: userName,
opportunity_details_url: `${copilotPortalUrl}/opportunity/${opportunity.id}`,
work_manager_url: config.get('workManagerUrl'),
opportunity_type: getCopilotTypeLabel(type),
opportunity_title: opportunityTitle,
start_date: moment(startDate).format('DD-MM-YYYY'),
},
sendgrid_template_id: TEMPLATE_IDS.CREATE_REQUEST,
recipients: [recipient],
version: 'v3',
}, req.log);

subjects.forEach(subject => sendNotification(subject.handle, subject.email));

// send email to notify via slack
sendNotification('Copilots', config.copilotsSlackEmail);

Choose a reason for hiding this comment

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

[⚠️ security]
The email address config.copilotsSlackEmail should be validated to ensure it is correctly configured. If this value is incorrect, the notification will fail silently.

req.log.info('Finished sending emails to copilots');
} catch (emailErr) {
req.log.error('Error sending notifications', { error: emailErr });
}

return opportunity;
} catch (err) {
req.log.error('approveRequest failed', { error: err });
throw err; // let outer transaction handle rollback
}
});
};
114 changes: 56 additions & 58 deletions src/routes/copilotRequest/create.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const addCopilotRequestValidations = {

module.exports = [
validate(addCopilotRequestValidations),
(req, res, next) => {
async (req, res, next) => {
const data = req.body;
if (!util.hasPermissionByReq(PERMISSION.MANAGE_COPILOT_REQUEST, req)) {
const err = new Error('Unable to create copilot request');
Expand All @@ -58,66 +58,64 @@ module.exports = [
updatedBy: req.authUser.userId,
});

return models.sequelize.transaction((transaction) => {
req.log.debug('Create Copilot request transaction', data);
return models.Project.findOne({
where: { id: projectId, deletedAt: { $eq: null } },
})
.then((existingProject) => {
if (!existingProject) {
const err = new Error(`active project not found for project id ${projectId}`);
err.status = 404;
throw err;
}
return models.CopilotRequest.findOne({
where: {
createdBy: req.authUser.userId,
projectId,
status: {
[Op.in]: [COPILOT_REQUEST_STATUS.NEW, COPILOT_REQUEST_STATUS.APPROVED, COPILOT_REQUEST_STATUS.SEEKING],
},
},
}).then((copilotRequest) => {
if (copilotRequest && copilotRequest.data.projectType === data.data.projectType) {
const err = new Error('There\'s a request of same type already!');
_.assign(err, {
status: 400,
});
throw err;
}
try {
const copilotRequest = await models.sequelize.transaction(async (transaction) => {

Choose a reason for hiding this comment

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

[⚠️ performance]
Consider adding a timeout or a retry mechanism for the transaction to handle potential deadlocks or long-running operations.

req.log.debug('Create copilot request transaction', { data });

const existingProject = await models.Project.findOne({
where: { id: projectId, deletedAt: { $eq: null } },
transaction,
});

return models.CopilotRequest
.create(data, { transaction });
}).then((copilotRequest) => {
/**
* Automatically approve the copilot request.
*/
const approveData = _.assign({
projectId,
copilotRequestId: copilotRequest.id,
createdBy: req.authUser.userId,
updatedBy: req.authUser.userId,
type: copilotRequest.data.projectType,
opportunityTitle: copilotRequest.data.opportunityTitle,
startDate: copilotRequest.data.startDate,
});
return approveRequest(req, approveData, transaction).then(() => copilotRequest);
}).then(copilotRequest => res.status(201).json(copilotRequest))
.catch((err) => {
try {
transaction.rollback();
} catch (e) {
_.noop(e);
}
return Promise.reject(err);
});
if (!existingProject) {
const err = new Error(`Active project not found for project id ${projectId}`);
err.status = 404;
throw err;
}

const existingRequest = await models.CopilotRequest.findOne({
where: {
createdBy: req.authUser.userId,
projectId,
status: {
[Op.in]: [
COPILOT_REQUEST_STATUS.NEW,
COPILOT_REQUEST_STATUS.APPROVED,
COPILOT_REQUEST_STATUS.SEEKING,
],
},
},
transaction,
});
})
.catch((err) => {
if (err.message) {
_.assign(err, { details: err.message });

if (existingRequest && existingRequest.data.projectType === data.data.projectType) {
const err = new Error('There\'s a request of same type already!');
err.status = 400;
throw err;
}
util.handleError('Error creating copilot request', err, req, next);

const newRequest = await models.CopilotRequest.create(data, { transaction });

await approveRequest(req, {
projectId,
copilotRequestId: newRequest.id,
createdBy: req.authUser.userId,
updatedBy: req.authUser.userId,
type: newRequest.data.projectType,
opportunityTitle: newRequest.data.opportunityTitle,
startDate: newRequest.data.startDate,
}, transaction);

return newRequest;
});

return res.status(201).json(copilotRequest);
} catch (err) {
req.log.error('Error creating copilot request', { error: err });

Choose a reason for hiding this comment

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

[❗❗ security]
Logging the error message directly might expose sensitive information. Consider sanitizing the error message before logging.

if (err.message) _.assign(err, { details: err.message });
util.handleError('Error creating copilot request', err, req, next);
return undefined;
}
},
];

11 changes: 6 additions & 5 deletions src/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -820,10 +820,10 @@ const projectServiceUtils = {
const token = yield this.getM2MToken();
const httpClient = this.getHttpClient({ id: requestId, log: logger });
httpClient.defaults.timeout = 6000;
logger.debug(`${config.identityServiceEndpoint}roles/${roleId}`, "fetching role info");
logger.debug(`${config.identityServiceEndpoint}roles/${roleId}`, 'fetching role info');
return httpClient.get(`${config.identityServiceEndpoint}roles/${roleId}`, {
params: {
fields: `subjects`,
fields: 'subjects',
},
headers: {
'Content-Type': 'application/json',
Expand All @@ -834,7 +834,7 @@ const projectServiceUtils = {
return _.get(res, 'data.result.content', []);
});
} catch (err) {
logger.debug(err, "error on getting role info");
logger.debug(err, 'error on getting role info');
return Promise.reject(err);
}
}),
Expand All @@ -853,8 +853,9 @@ const projectServiceUtils = {
Authorization: `Bearer ${token}`,
},
}).then((res) => {
logger.debug(`Roles by ${roleName}: ${JSON.stringify(res.data.result.content)}`);
return _.get(res, 'data.result.content', [])
const roles = res.data;
logger.debug(`Roles by ${roleName}: ${JSON.stringify(roles)}`);
return roles.result.content

Choose a reason for hiding this comment

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

[❗❗ correctness]
The roles.result.content is accessed without checking if roles.result is defined. This could lead to a runtime error if the response structure changes or if the expected data is not present. Consider adding a check to ensure roles.result is defined before accessing roles.result.content.

.filter(item => item.roleName === roleName)
.map(r => r.id);
});
Expand Down