Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 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', 'migration-setup', 'PM-1612', 'PM-2321']
- deployProd:
context : org-global
filters:
Expand Down
26 changes: 19 additions & 7 deletions src/routes/copilotRequest/approveRequest.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@ 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';
Copy link

Choose a reason for hiding this comment

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

Consider formatting the import statement so that each constant is on a separate line for better readability, like the other constants in this block.

import util from '../../util';
import { createEvent } from '../../services/busApi';
import { getCopilotTypeLabel } from '../../utils/copilot';
Expand Down Expand Up @@ -47,7 +51,7 @@ module.exports = (req, data, existingTransaction) => {
type: data.type,
status: {
[Op.in]: [COPILOT_OPPORTUNITY_STATUS.ACTIVE],
}
},
},
})
.then((existingCopilotOpportunityOfSameType) => {
Expand All @@ -66,7 +70,7 @@ module.exports = (req, data, existingTransaction) => {
const { subjects = [] } = await util.getRoleInfo(roles[0], req.log, req.id);
Copy link

Choose a reason for hiding this comment

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

Consider adding error handling for the await util.getRoleInfo(roles[0], req.log, req.id); call to manage potential promise rejections.

const emailEventType = CONNECT_NOTIFICATION_EVENT.EXTERNAL_ACTION_EMAIL;
const copilotPortalUrl = config.get('copilotPortalUrl');
req.log.info("Sending emails to all copilots about new opportunity");
req.log.info('Sending emails to all copilots about new opportunity');

const sendNotification = (userName, recipient) => createEvent(emailEventType, {
data: {
Expand All @@ -75,20 +79,28 @@ module.exports = (req, data, existingTransaction) => {
work_manager_url: config.get('workManagerUrl'),
opportunity_type: getCopilotTypeLabel(type),
opportunity_title: opportunityTitle,
start_date: moment(startDate).format("DD-MM-YYYY"),
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));
const notificationPromises = subjects.map(subject =>
sendNotification(subject.handle, subject.email),
);

notificationPromises.push(
sendNotification('Copilots', config.copilotsSlackEmail),
);

await Promise.all(notificationPromises);

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

Choose a reason for hiding this comment

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

The call to sendNotification('Copilots', config.copilotsSlackEmail); is redundant here because it is already included in the notificationPromises array and awaited with Promise.all(notificationPromises);. Consider removing this line to avoid sending duplicate notifications.


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

return opportunity;
})
.catch((err) => {
Expand Down