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-1613']
only: ['develop', 'migration-setup', 'pm-1611']
- deployProd:
context : org-global
filters:
Expand Down
2 changes: 2 additions & 0 deletions src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,8 @@ export const TEMPLATE_IDS = {
INFORM_PM_COPILOT_APPLICATION_ACCEPTED: 'd-b35d073e302b4279a1bd208fcfe96f58',
COPILOT_ALREADY_PART_OF_PROJECT: 'd-003d41cdc9de4bbc9e14538e8f2e0585',
COPILOT_APPLICATION_ACCEPTED: 'd-eef5e7568c644940b250e76d026ced5b',
COPILOT_OPPORTUNITY_COMPLETED: 'd-dc448919d11b4e7d8b4ba351c4b67b8b',
COPILOT_OPPORTUNITY_CANCELED: 'd-2a67ba71e82f4d70891fe6989c3522a3'
}
export const REGEX = {
URL: /^(http(s?):\/\/)?(www\.)?[a-zA-Z0-9\.\-\_]+(\.[a-zA-Z]{2,15})+(\:[0-9]{2,5})?(\/[a-zA-Z0-9\_\-\s\.\/\?\%\#\&\=;]*)?$/, // eslint-disable-line
Expand Down
9 changes: 5 additions & 4 deletions src/models/copilotRequest.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import _ from 'lodash';
import { COPILOT_REQUEST_STATUS } from '../constants';

module.exports = function defineCopilotRequest(sequelize, DataTypes) {
const CopliotRequest = sequelize.define('CopilotRequest', {
const CopilotRequest = sequelize.define('CopilotRequest', {
id: { type: DataTypes.BIGINT, primaryKey: true, autoIncrement: true },
status: {
type: DataTypes.STRING(16),
Expand Down Expand Up @@ -30,9 +30,10 @@ module.exports = function defineCopilotRequest(sequelize, DataTypes) {
indexes: [],
});

CopliotRequest.associate = (models) => {
CopliotRequest.hasMany(models.CopilotOpportunity, { as: 'copilotOpportunity', foreignKey: 'copilotRequestId' });
CopilotRequest.associate = (models) => {

Choose a reason for hiding this comment

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

Typo correction needed: The model name CopliotRequest should be corrected to CopilotRequest.

CopilotRequest.hasMany(models.CopilotOpportunity, { as: 'copilotOpportunity', foreignKey: 'copilotRequestId' });
CopilotRequest.belongsTo(models.Project, { as: 'project', foreignKey: 'projectId' });
};

return CopliotRequest;
return CopilotRequest;

Choose a reason for hiding this comment

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

Typo correction needed: The model name CopliotRequest should be corrected to CopilotRequest.

};
30 changes: 30 additions & 0 deletions src/routes/copilotOpportunity/assign.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,33 @@ module.exports = [
return next(err);
}

const sendEmailToAllApplicants = async (copilotRequest, allApplications) => {

Choose a reason for hiding this comment

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

The function signature of sendEmailToAllApplicants has changed, but the logic for fetching allApplications has been removed. Ensure that allApplications is correctly populated before calling this function, as it is now expected to be passed as an argument.


const userIds = allApplications.map(item => item.userId);

const users = await util.getMemberDetailsByUserIds(userIds, req.log, req.id);

users.forEach(async (user) => {

Choose a reason for hiding this comment

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

Consider using a Promise.all to handle asynchronous operations in the forEach loop. This will ensure that all emails are sent before proceeding, and can help catch any errors in the email sending process.

req.log.debug(`Sending email notification to copilots who are not accepted`);
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_title: requestData.opportunityTitle,
user_name: user ? user.handle : "",

Choose a reason for hiding this comment

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

The user_name field is being set to an empty string if user is falsy. Consider handling cases where user might be undefined or null more explicitly to avoid sending incomplete data.

},
sendgrid_template_id: TEMPLATE_IDS.COPILOT_OPPORTUNITY_COMPLETED,
recipients: [user.email],
version: 'v3',
}, req.log);

req.log.debug(`Email sent to copilots who are not accepted`);
});

};

return models.sequelize.transaction(async (t) => {
const opportunity = await models.CopilotOpportunity.findOne({
where: { id: copilotOpportunityId },
Expand Down Expand Up @@ -238,6 +265,9 @@ module.exports = [
transaction: t,
});

// Send email to all applicants about opportunity completion
await sendEmailToAllApplicants(copilotRequest, otherApplications);

Choose a reason for hiding this comment

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

Consider handling potential errors from the sendEmailToAllApplicants function. If this function fails, it might be beneficial to log the error or take corrective action to ensure the application process continues smoothly.


for (const otherApplication of otherApplications) {
await otherApplication.update({
status: COPILOT_APPLICATION_STATUS.CANCELED,
Expand Down
31 changes: 30 additions & 1 deletion src/routes/copilotOpportunity/delete.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import _ from 'lodash';
import { Op } from 'sequelize';
import config from 'config';

import models from '../../models';
import util from '../../util';
import { COPILOT_APPLICATION_STATUS, COPILOT_OPPORTUNITY_STATUS, COPILOT_REQUEST_STATUS, EVENT, INVITE_STATUS, RESOURCES } from '../../constants';
import { CONNECT_NOTIFICATION_EVENT, COPILOT_APPLICATION_STATUS, COPILOT_OPPORTUNITY_STATUS, COPILOT_REQUEST_STATUS, EVENT, INVITE_STATUS, RESOURCES, TEMPLATE_IDS } from '../../constants';
import { createEvent } from '../../services/busApi';
import { PERMISSION } from '../../permissions/constants';


Expand All @@ -20,6 +22,31 @@ module.exports = [
// default values
const opportunityId = _.parseInt(req.params.id);

const sendEmailToAllApplicants = async (copilotRequest, applications) => {
const userIds = applications.map(item => item.userId);
const users = await util.getMemberDetailsByUserIds(userIds, req.log, req.id);

users.forEach(async (user) => {

Choose a reason for hiding this comment

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

Consider using Promise.all to handle asynchronous operations within the forEach loop. This ensures that all emails are sent before proceeding, and can help avoid potential issues with unhandled promises.

req.log.debug(`Sending email notification to copilots who applied`);
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_title: requestData.opportunityTitle,
user_name: user ? user.handle : "",

Choose a reason for hiding this comment

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

Consider using a default value or handling the case where user.handle might be null or undefined to prevent potential issues with the email content.

},
sendgrid_template_id: TEMPLATE_IDS.COPILOT_OPPORTUNITY_CANCELED,
recipients: [user.email],
version: 'v3',
}, req.log);

req.log.debug(`Email sent to copilots who applied`);
});

};

return models.sequelize.transaction(async (transaction) => {
req.log.debug('Canceling Copilot opportunity transaction', opportunityId);
const opportunity = await models.CopilotOpportunity.findOne({
Expand Down Expand Up @@ -93,6 +120,8 @@ module.exports = [
invite.toJSON());
}

await sendEmailToAllApplicants(copilotRequest, applications)

Choose a reason for hiding this comment

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

The sendEmailToAllApplicants function call has been modified to remove the req parameter. Ensure that this change does not affect the function's behavior or cause any issues, as the req parameter might have been used within the function for accessing request-specific data.


res.status(200).send({ id: opportunity.id });
})

Expand Down
1 change: 0 additions & 1 deletion src/routes/copilotOpportunity/get.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import util from '../../util';
module.exports = [
(req, res, next) => {
const { id } = req.params;

if (!id || isNaN(id)) {
return util.handleError('Invalid opportunity ID', null, req, next, 400);
}
Expand Down
29 changes: 19 additions & 10 deletions src/routes/copilotRequest/list.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import _ from 'lodash';
import { Op, Sequelize } from 'sequelize';

import models from '../../models';
import util from '../../util';
import { PERMISSION } from '../../permissions/constants';
import { DEFAULT_PAGE_SIZE } from '../../constants';

module.exports = [
(req, res, next) => {
Expand All @@ -15,6 +17,10 @@ module.exports = [
return next(err);
}

const page = parseInt(req.query.page, 10) || 1;

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 req.query.page is not a valid integer. Currently, parseInt will return NaN for invalid inputs, which could lead to unexpected behavior when calculating offset.

const pageSize = parseInt(req.query.pageSize, 10) || DEFAULT_PAGE_SIZE;

Choose a reason for hiding this comment

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

Similar to the previous comment, ensure that req.query.pageSize is a valid integer before using it. This will prevent potential issues with offset calculation if pageSize is NaN.

const offset = (page - 1) * pageSize;

const projectId = _.parseInt(req.params.projectId);

let sort = req.query.sort ? decodeURIComponent(req.query.sort) : 'createdAt desc';
Expand All @@ -29,19 +35,22 @@ module.exports = [

const whereCondition = projectId ? { projectId } : {};

return models.CopilotRequest.findAll({
return models.CopilotRequest.findAndCountAll({

Choose a reason for hiding this comment

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

Consider checking if findAndCountAll is necessary for your use case. If you only need the count for pagination headers, ensure that this change is intentional as it may impact performance.

where: whereCondition,
include: [
{
model: models.CopilotOpportunity,
as: 'copilotOpportunity',
},
{ model: models.CopilotOpportunity, as: 'copilotOpportunity', required: false },
{ model: models.Project, as: 'project', required: false },

Choose a reason for hiding this comment

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

Ensure that the inclusion of the Project model is necessary for the functionality you are implementing. If not, it could lead to unnecessary data fetching.

],
order: [[sortParams[0], sortParams[1]]],
})
.then(copilotRequests => res.json(copilotRequests))
.catch((err) => {
util.handleError('Error fetching copilot requests', err, req, next);
});
limit: pageSize,
offset,
distinct: true,
subQuery: false,
}).then(({rows: copilotRequests, count}) => util.setPaginationHeaders(req, res, {
count: count,
rows: copilotRequests,
page,
pageSize,
}));
},
];