Skip to content

fix(PM-1168): invite user when assigning as copilot #809

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 17 commits into from
May 22, 2025
Merged
Show file tree
Hide file tree
Changes from 10 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-1168']
only: ['develop', 'migration-setup']
- deployProd:
context : org-global
filters:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
module.exports = {
up: async (queryInterface, Sequelize) => {
await queryInterface.addColumn('project_member_invites', 'applicationId', {

Choose a reason for hiding this comment

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

Consider specifying an index on the applicationId column for improved query performance, especially if this column will be frequently used in WHERE clauses or joins.

type: Sequelize.BIGINT,
allowNull: true,
references: {
model: 'copilot_applications',
key: 'id',
},
onUpdate: 'CASCADE',
onDelete: 'SET NULL',
});
},

down: async (queryInterface) => {
await queryInterface.removeColumn('project_member_invites', 'applicationId');
},
};
88 changes: 75 additions & 13 deletions src/routes/copilotOpportunity/assign.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
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 } from '../../constants';
import { createEvent } from '../../services/busApi';
import { CONNECT_NOTIFICATION_EVENT, COPILOT_APPLICATION_STATUS, COPILOT_OPPORTUNITY_STATUS, COPILOT_REQUEST_STATUS, EVENT, INVITE_STATUS, PROJECT_MEMBER_ROLE, RESOURCES } from '../../constants';

const assignCopilotOpportunityValidations = {
body: Joi.object().keys({
Expand Down Expand Up @@ -73,20 +75,80 @@ module.exports = [
throw err;
}

await models.CopilotRequest.update(
{ status: COPILOT_REQUEST_STATUS.FULFILLED },
{ where: { id: opportunity.copilotRequestId }, transaction: t },
);
const project = await models.Project.findOne({
where: {
id: projectId,
},
});

const existingInvite = await models.ProjectMemberInvite.findAll({
where: {
userId,
projectId,
role: PROJECT_MEMBER_ROLE.COPILOT,
status: INVITE_STATUS.PENDING,
},
});

if (existingInvite) {

Choose a reason for hiding this comment

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

The findAll method returns an array, so existingInvite will always be truthy even if it's empty. Consider using findOne if you expect a single result, or check if existingInvite.length > 0.

const err = new Error(`User already has an pending invite to the project`);

Choose a reason for hiding this comment

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

The error message contains a grammatical error. It should be 'a pending invite' instead of 'an pending invite'.

err.status = 400;
throw err;
}

const applicationUser = await util.getMemberDetailsByUserIds([userId], req.log, req.id);

const invite = await models.ProjectMemberInvite.create({
status: INVITE_STATUS.PENDING,
role: PROJECT_MEMBER_ROLE.COPILOT,
userId,
projectId,
email: applicationUser[0].email,
createdBy: req.authUser.userId,
createdAt: new Date(),
updatedBy: req.authUser.userId,
updatedAt: new Date(),
}, {
transaction: t,
})

util.sendResourceToKafkaBus(
req,
EVENT.ROUTING_KEY.PROJECT_MEMBER_INVITE_CREATED,
RESOURCES.PROJECT_MEMBER_INVITE,
invite.toJSON());

await opportunity.update(
{ status: COPILOT_OPPORTUNITY_STATUS.COMPLETED },
{ transaction: t },
);
const authUserDetails = await util.getMemberDetailsByUserIds([req.authUser.userId], req.log, req.id);

await models.CopilotApplication.update(
{ status: COPILOT_APPLICATION_STATUS.ACCEPTED },
{ where: { id: applicationId }, transaction: t },
);
const emailEventType = CONNECT_NOTIFICATION_EVENT.PROJECT_MEMBER_EMAIL_INVITE_CREATED;
await createEvent(emailEventType, {
data: {
workManagerUrl: config.get('workManagerUrl'),
accountsAppURL: config.get('accountsAppUrl'),
subject: config.get('inviteEmailSubject'),
projects: [{
name: project.name,
projectId,
sections: [
{
EMAIL_INVITES: true,
title: config.get('inviteEmailSectionTitle'),
projectName: project.name,
projectId,
initiator: authUserDetails[0],

Choose a reason for hiding this comment

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

The removal of the code block that updates the status of CopilotRequest, opportunity, and CopilotApplication might lead to inconsistencies in the database. Ensure that these status updates are handled elsewhere if they are still necessary for the application's logic.

isSSO: util.isSSO(project),
},
],
}],
},
recipients: [applicationUser[0].email],
version: 'v3',
from: {
name: config.get('EMAIL_INVITE_FROM_NAME'),
email: config.get('EMAIL_INVITE_FROM_EMAIL'),
},
categories: [`${process.env.NODE_ENV}:${emailEventType}`.toLowerCase()],
}, req.log);

res.status(200).send({ id: applicationId });
}).catch(err => next(err));
Expand Down
4 changes: 3 additions & 1 deletion src/routes/projectMemberInvites/update.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,9 @@ module.exports = [
};
return util
.addUserToProject(req, member)
.then(() => res.json(util.postProcessInvites('$.email', updatedInvite, req)))
.then(async () => {

Choose a reason for hiding this comment

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

The use of async in the then block seems unnecessary here since there is no await expression inside the block. Consider removing async to simplify the code.

return res.json(util.postProcessInvites('$.email', updatedInvite, req))
})
.catch(err => next(err));
});
}
Expand Down