Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
25b8b34
fix: removed logic to allow inviting user even if they are already a …
hentrymartin Jul 23, 2025
7831f54
fix: allow copilots to be added even if the existing member
hentrymartin Jul 23, 2025
57a8d00
fix: allow copilots to be added even if the existing member
hentrymartin Jul 23, 2025
1b21d3b
fix: allow copilots to be added even if the existing member
hentrymartin Jul 23, 2025
fa13330
fix: allow copilots to be added even if the existing member
hentrymartin Jul 23, 2025
0536910
fix: allow copilots to be added even if the existing member
hentrymartin Jul 23, 2025
b67a6ba
fix: allow copilots to be added even if the existing member
hentrymartin Jul 23, 2025
f26de03
fix: just switch role if user is already a member
hentrymartin Jul 24, 2025
50d2dac
fix: just switch role if user is already a member
hentrymartin Jul 24, 2025
347caae
fix: debug logs
hentrymartin Jul 24, 2025
d4e4033
fix: debug logs
hentrymartin Jul 24, 2025
ecb836c
fix: debug logs
hentrymartin Jul 24, 2025
279d2f1
fix: update kafka
hentrymartin Jul 24, 2025
3998114
fix: update kafka
hentrymartin Jul 24, 2025
b7c5f95
revert
hentrymartin Jul 24, 2025
bebe265
fix: build
hentrymartin Jul 24, 2025
9385156
fix: added error string and already assigned role
hentrymartin Jul 25, 2025
fdb09e3
fix: added error string and already assigned role
hentrymartin Jul 25, 2025
2dc0ea2
fix: added error string and already assigned role
hentrymartin Jul 25, 2025
13c8c39
fix: added error string and already assigned role
hentrymartin Jul 25, 2025
f73f444
feat: modifications on copilot addition to project
hentrymartin Jul 27, 2025
7fce661
feat: modifications on copilot addition to project
hentrymartin Jul 27, 2025
1f2cba4
feat: modifications on copilot addition to project
hentrymartin Jul 27, 2025
1266652
feat: modifications on copilot addition to project
hentrymartin Jul 27, 2025
c701b1e
fix: complete the copilot requests if the incoming role is observer o…
hentrymartin Jul 27, 2025
1d19d15
fix: complete the copilot requests if the incoming role is observer o…
hentrymartin Jul 27, 2025
af842b0
fix: complete the copilot requests if the incoming role is observer o…
hentrymartin Jul 27, 2025
0d02782
fix: complete the copilot requests if the incoming role is observer o…
hentrymartin Jul 28, 2025
ce575c7
fix: action string
hentrymartin Jul 28, 2025
2f2e162
fix: send email to project manager on copilot invite acceptation
hentrymartin Jul 28, 2025
287e14f
fix: send email to project manager on copilot invite acceptation
hentrymartin Jul 28, 2025
21977ef
fix: send email to project manager on copilot invite acceptation
hentrymartin Jul 28, 2025
b4dbde5
fix: send email to project manager on copilot invite acceptation
hentrymartin Jul 28, 2025
fc0bffe
fix: send email to project manager on copilot invite acceptation
hentrymartin Jul 28, 2025
ccceb70
send email to copilot who got selected
hentrymartin Jul 29, 2025
d56f233
send email to copilot who got selected
hentrymartin Jul 29, 2025
37f740c
fix: allow action to be empty string
hentrymartin Jul 29, 2025
993ea57
fix: allow action to be empty string
hentrymartin Jul 29, 2025
b3e4f8e
fix: allow action to be empty string
hentrymartin Jul 29, 2025
45629b5
fix: allow action to be empty string
hentrymartin Jul 29, 2025
5025fa1
fix: allow action to be empty string
hentrymartin Jul 29, 2025
7075d3f
fix: allow action to be empty string
hentrymartin Jul 29, 2025
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-1497']
only: ['develop', 'migration-setup', 'pm-1510']
- 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 @@ -311,6 +311,8 @@ export const TEMPLATE_IDS = {
APPLY_COPILOT: 'd-d7c1f48628654798a05c8e09e52db14f',
CREATE_REQUEST: 'd-3efdc91da580479d810c7acd50a4c17f',
PROJECT_MEMBER_INVITED: 'd-b47a25b103604bc28fc0ce77e77fb681',
INFORM_PM_COPILOT_APPLICATION_ACCEPTED: 'd-b35d073e302b4279a1bd208fcfe96f58',
COPILOT_ALREADY_PART_OF_PROJECT: 'd-003d41cdc9de4bbc9e14538e8f2e0585',
}
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
31 changes: 29 additions & 2 deletions src/routes/projectMemberInvites/create.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import _ from 'lodash';
import Joi from 'joi';
import config from 'config';
import { middleware as tcMiddleware } from 'tc-core-library-js';
import { Op } from 'sequelize';
import models from '../../models';
import util from '../../util';
import {
Expand Down Expand Up @@ -299,7 +300,7 @@ module.exports = [

return [];
})
.then((inviteUsers) => {
.then(async (inviteUsers) => {

Choose a reason for hiding this comment

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

Using async in a .then() callback can lead to unexpected behavior and is generally not recommended. Consider refactoring this part of the code to use async/await syntax throughout for better readability and error handling.

const members = req.context.currentProjectMembers;
const projectId = _.parseInt(req.params.projectId);
// check user handle exists in returned result
Expand All @@ -322,13 +323,39 @@ module.exports = [
const errorMessageForAlreadyMemberUser = 'User with such handle is already a member of the team.';

if (inviteUserIds) {
// remove members already in the team
const existingMembers = _.filter(members, (m) => {
return inviteUserIds.includes(m.userId);
});

req.log.debug(`Existing members: ${JSON.stringify(existingMembers)}`);

const projectMembers = await models.ProjectMember.findAll({
where: {
userId: {
[Op.in]: existingMembers.map(item => item.userId),
},
projectId,
}
});

req.log.debug(`Existing Project Members: ${JSON.stringify(projectMembers)}`);

const existingProjectMembersMap = projectMembers.reduce((acc, current) => {
return Object.assign({}, acc, {
[current.userId]: current,

Choose a reason for hiding this comment

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

Using Object.assign in this way creates a new object for each iteration, which can be inefficient. Consider using the spread operator to merge objects: return { ...acc, [current.userId]: current };

});
}, {});

req.log.debug(`Existing Project Members Map: ${JSON.stringify(existingProjectMembersMap)}`);

_.remove(inviteUserIds, u => _.some(members, (m) => {
const isPresent = m.userId === u;
if (isPresent) {
failed.push(_.assign({}, {
handle: getUserHandleById(m.userId, inviteUsers),
message: errorMessageForAlreadyMemberUser,
error: "ALREADY_MEMBER",
role: existingProjectMembersMap[m.userId].role,

Choose a reason for hiding this comment

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

Ensure that existingProjectMembersMap[m.userId] exists before accessing .role to avoid potential runtime errors. Consider adding a check or using optional chaining: existingProjectMembersMap[m.userId]?.role.

}));
}
return isPresent;
Expand Down
68 changes: 67 additions & 1 deletion src/routes/projectMemberInvites/update.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,15 @@ import validate from 'express-validation';
import _ from 'lodash';
import Joi from 'joi';
import { Op } from 'sequelize';
import config from 'config';

import { middleware as tcMiddleware } from 'tc-core-library-js';
import models from '../../models';
import util from '../../util';
import { INVITE_STATUS, EVENT, RESOURCES, COPILOT_APPLICATION_STATUS, COPILOT_OPPORTUNITY_STATUS, COPILOT_REQUEST_STATUS, INVITE_SOURCE } from '../../constants';
import { INVITE_STATUS, EVENT, RESOURCES, COPILOT_APPLICATION_STATUS, COPILOT_OPPORTUNITY_STATUS, COPILOT_REQUEST_STATUS, INVITE_SOURCE, CONNECT_NOTIFICATION_EVENT, TEMPLATE_IDS, USER_ROLE } from '../../constants';
import { PERMISSION } from '../../permissions/constants';
import { getCopilotTypeLabel } from '../../utils/copilot';
import { createEvent } from '../../services/busApi';


/**
Expand Down Expand Up @@ -263,6 +267,68 @@ module.exports = [
})
}

Choose a reason for hiding this comment

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

The removal of await t.commit(); might lead to uncommitted transactions if the logic depends on this commit. Ensure that the transaction is committed appropriately elsewhere in the code or verify that its removal does not affect the transaction handling.


if (source === 'copilot_portal' && invite.applicationId) {
const application = await models.CopilotApplication.findOne({
where: {
id: invite.applicationId,
},
});

const opportunity = await models.CopilotOpportunity.findOne({

Choose a reason for hiding this comment

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

The variable opportunity is redeclared here, which can lead to confusion. Consider renaming one of the variables to avoid shadowing and improve code clarity.

where: {
id: application.opportunityId,

Choose a reason for hiding this comment

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

Consider adding a null check for application before accessing application.opportunityId to prevent potential runtime errors if application is not found.

},
include: [

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 database query to ensure that any issues with fetching the opportunity are properly managed.

{
model: models.CopilotRequest,
as: 'copilotRequest',
},
],
});
const pmRole = await util.getRolesByRoleName(USER_ROLE.PROJECT_MANAGER, req.log, req.id);
const { subjects = [] } = await util.getRoleInfo(pmRole[0], req.log, req.id);

const creatorDetails = await util.getMemberDetailsByUserIds([opportunity.createdBy], req.log, req.id);
const inviteeDetails = await util.getMemberDetailsByUserIds([application.userId], req.log, req.id);

Choose a reason for hiding this comment

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

The variable inviteeDetails is assigned using application.userId, but the previous code used invite.userId. Ensure that application.userId is the correct identifier for the invitee in this context.

const creator = creatorDetails[0];
const invitee = inviteeDetails[0];
const listOfSubjects = subjects;
if (creator && creator.email) {

Choose a reason for hiding this comment

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

The variable application is removed but is still referenced in the code. Ensure that the removal of this variable does not affect other parts of the code that might rely on it.

const isCreatorPartofSubjects = subjects.find(item => {
if (!item.email) {
return false;
}

return item.email.toLowerCase() === creator.email.toLowerCase();

Choose a reason for hiding this comment

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

The variable creator is being accessed as an object instead of an array. Ensure that creator is always an object and not an array to avoid runtime errors.

Choose a reason for hiding this comment

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

The variable opportunity is removed but is still referenced in the code. Ensure that the removal of this variable does not affect other parts of the code that might rely on it.

});
if (!isCreatorPartofSubjects) {
listOfSubjects.push({
email: creator.email,
handle: creator.handle,
});
}
}

const emailEventType = CONNECT_NOTIFICATION_EVENT.EXTERNAL_ACTION_EMAIL;
const copilotPortalUrl = config.get('copilotPortalUrl');
const requestData = opportunity.copilotRequest.data;
listOfSubjects.forEach((subject) => {

Choose a reason for hiding this comment

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

Consider using for...of loop instead of forEach for asynchronous operations inside the loop to ensure proper handling of asynchronous code execution.

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),
opportunity_title: requestData.opportunityTitle,
copilot_handle: invitee ? invitee.handle : "",
},
sendgrid_template_id: TEMPLATE_IDS.INFORM_PM_COPILOT_APPLICATION_ACCEPTED,
recipients: [subject.email],
version: 'v3',
}, req.log);
});
}

await t.commit();

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 t.commit() operation to ensure that any issues during the commit process are properly caught and managed. This will help prevent potential issues if the commit fails.

return res.json(util.postProcessInvites('$.email', updatedInvite, req));
} catch (e) {
Expand Down
157 changes: 151 additions & 6 deletions src/routes/projectMembers/update.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,17 @@
import validate from 'express-validation';
import _ from 'lodash';
import Joi from 'joi';
import config from 'config';
import moment from 'moment';
import { Op } from 'sequelize';
import { middleware as tcMiddleware } from 'tc-core-library-js';
import models from '../../models';
import util from '../../util';
import { EVENT, RESOURCES, PROJECT_MEMBER_ROLE } from '../../constants';
import { EVENT, RESOURCES, PROJECT_MEMBER_ROLE, COPILOT_REQUEST_STATUS, COPILOT_OPPORTUNITY_STATUS, COPILOT_APPLICATION_STATUS, USER_ROLE, CONNECT_NOTIFICATION_EVENT, TEMPLATE_IDS } from '../../constants';
import { PERMISSION, PROJECT_TO_TOPCODER_ROLES_MATRIX } from '../../permissions/constants';
import { createEvent } from '../../services/busApi';
import { getCopilotTypeLabel } from '../../utils/copilot';


/**
* API to update a project member.
Expand All @@ -27,12 +33,140 @@ const updateProjectMemberValdiations = {
PROJECT_MEMBER_ROLE.SOLUTION_ARCHITECT,

Choose a reason for hiding this comment

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

Typo in the variable name updateProjectMemberValdiations. It should be updateProjectMemberValidations.

PROJECT_MEMBER_ROLE.PROJECT_MANAGER,
).required(),
action: Joi.string().allow('').optional(),
}),
query: {
fields: Joi.string().optional(),
},
};

const completeAllCopilotRequests = async (req, projectId, _transaction, _member) => {

Choose a reason for hiding this comment

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

The _member parameter is added to the function signature but is not used within the function body. Consider removing it if it's not needed, or ensure it is utilized appropriately.

const allCopilotRequests = await models.CopilotRequest.findAll({
where: {
projectId,
status: {

Choose a reason for hiding this comment

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

Consider checking if the Op.in condition is necessary for the specific logic you are implementing. If the logic requires handling only specific statuses, ensure that the statuses included are correct and comprehensive for the intended functionality.

[Op.in]: [
COPILOT_REQUEST_STATUS.APPROVED,
COPILOT_REQUEST_STATUS.NEW,
COPILOT_REQUEST_STATUS.SEEKING,
],
}
},
transaction: _transaction,
});

req.log.debug(`all copilot requests ${JSON.stringify(allCopilotRequests)}`);

Choose a reason for hiding this comment

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

The debug log message all copilot requests could be more descriptive, such as Fetched all copilot requests for projectId: ${projectId}.


await models.CopilotRequest.update({
status: COPILOT_REQUEST_STATUS.FULFILLED,
}, {
where: {
id: {
[Op.in]: allCopilotRequests.map(item => item.id),
}
},
transaction: _transaction,
});

req.log.debug(`updated all copilot requests`);

Choose a reason for hiding this comment

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

The debug log message updated all copilot requests could be more descriptive, such as Updated status to FULFILLED for all copilot requests related to projectId: ${projectId}.


const copilotOpportunites = await models.CopilotOpportunity.findAll({
where: {
copilotRequestId: {
[Op.in]: allCopilotRequests.map(item => item.id),
},
},
transaction: _transaction,
});

req.log.debug(`all copilot opportunities ${JSON.stringify(copilotOpportunites)}`);

Choose a reason for hiding this comment

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

The debug log message all copilot opportunities could be more descriptive, such as Fetched all copilot opportunities for copilotRequestIds: ${allCopilotRequests.map(item => item.id)}.


await models.CopilotOpportunity.update({
status: COPILOT_OPPORTUNITY_STATUS.COMPLETED,
}, {
where: {
id: {
[Op.in]: copilotOpportunites.map(item => item.id),
}
},
transaction: _transaction,
});

req.log.debug(`updated all copilot opportunities`);

Choose a reason for hiding this comment

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

The debug log message updated all copilot opportunities could be more descriptive, such as Updated status to COMPLETED for all copilot opportunities related to copilotRequestIds: ${allCopilotRequests.map(item => item.id)}.


const allCopilotApplications = await models.CopilotApplication.findAll({
where: {
opportunityId: {
[Op.in]: copilotOpportunites.map(item => item.id),
},
},
transaction: _transaction,
});

const memberApplication = allCopilotApplications.find(app => app.userId === _member.userId);
const applicationsWithoutMemberApplication = allCopilotApplications.filter(app => app.userId !== _member.userId);

req.log.debug(`all copilot applications ${JSON.stringify(allCopilotApplications)}`);

Choose a reason for hiding this comment

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

The debug log message all copilot applications could be more descriptive, such as Fetched all copilot applications for opportunityIds: ${copilotOpportunites.map(item => item.id)}.


await models.CopilotApplication.update({
status: COPILOT_APPLICATION_STATUS.CANCELED,
}, {
where: {
id: {
[Op.in]: applicationsWithoutMemberApplication.map(item => item.id),
},
},
transaction: _transaction,
});

// If the invited member

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 case when memberApplication is not found or the update fails. This will help ensure that the application status update process is robust and can handle unexpected scenarios gracefully.

if (memberApplication) {
await models.CopilotApplication.update({
status: COPILOT_APPLICATION_STATUS.ACCEPTED,
}, {
where: {
id: memberApplication.id,
},
transaction: _transaction,
});
}

req.log.debug(`updated all copilot applications`);

Choose a reason for hiding this comment

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

The debug log message updated all copilot applications could be more descriptive, such as Updated status to CANCELED for all copilot applications related to opportunityIds: ${copilotOpportunites.map(item => item.id)}.

Choose a reason for hiding this comment

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

The log message updated all copilot applications might be misleading since the update operation for memberApplication is separate from allCopilotApplications. Consider revising the log message to accurately reflect the operations performed.


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

Choose a reason for hiding this comment

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

Consider checking if _member is defined before accessing userId to avoid potential runtime errors.

Choose a reason for hiding this comment

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

The removal of _transaction.commit() might lead to uncommitted changes in the database. Ensure that the transaction is being committed elsewhere or that this removal is intentional and safe.

const member = memberDetails[0];

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 memberDetails array to ensure it contains elements before accessing the first element. This will prevent potential runtime errors if the array is empty.


req.log.debug(`member details: ${JSON.stringify(member)}`);

const emailEventType = CONNECT_NOTIFICATION_EVENT.EXTERNAL_ACTION_EMAIL;
const copilotPortalUrl = config.get('copilotPortalUrl');
allCopilotRequests.forEach((request) => {
const requestData = request.data;

req.log.debug(`Copilot request data: ${JSON.stringify(requestData)}`);
const opportunity = copilotOpportunites.find(item => item.copilotRequestId === request.id);

Choose a reason for hiding this comment

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

Ensure opportunity is defined before accessing opportunity.id to prevent errors if find returns undefined.

req.log.debug(`Opportunity: ${JSON.stringify(opportunity)}`);
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 : "",

Choose a reason for hiding this comment

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

Consider adding a null check for member before accessing member.handle to prevent potential runtime errors if member is undefined.

},
sendgrid_template_id: TEMPLATE_IDS.COPILOT_ALREADY_PART_OF_PROJECT,
recipients: [member.email],

Choose a reason for hiding this comment

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

Consider adding a null check for member before accessing member.email to prevent potential runtime errors if member is undefined.

version: 'v3',
}, req.log);

req.log.debug(`Sent email to ${member.email}`);

Choose a reason for hiding this comment

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

Consider adding a null check for member before accessing member.email to prevent potential runtime errors if member is undefined.

});

await _transaction.commit();

Choose a reason for hiding this comment

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

Consider adding error handling around the _transaction.commit() call to ensure that any issues during the commit process are properly managed. This could help prevent potential data inconsistencies or application crashes.

};

module.exports = [
// handles request validations
validate(updateProjectMemberValdiations),
Expand All @@ -45,15 +179,16 @@ module.exports = [
let updatedProps = req.body;

Choose a reason for hiding this comment

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

Consider checking if req.body is defined before assigning it to updatedProps to prevent potential runtime errors.

const projectId = _.parseInt(req.params.projectId);
const memberRecordId = _.parseInt(req.params.id);
const action = updatedProps.action;

Choose a reason for hiding this comment

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

The variable action is assigned but never used in the code. Consider removing it if it's not needed or using it appropriately.

updatedProps = _.pick(updatedProps, ['isPrimary', 'role']);
const fields = req.query.fields ? req.query.fields.split(',') : null;

let previousValue;

Choose a reason for hiding this comment

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

Consider using await with models.ProjectMember.findOne since the transaction function is now asynchronous. This will ensure that the promise is resolved before proceeding.

// let newValue;
models.sequelize.transaction(() => models.ProjectMember.findOne({
models.sequelize.transaction(async (_transaction) => models.ProjectMember.findOne({
where: { id: memberRecordId, projectId },
})
.then((_member) => {
.then(async (_member) => {

Choose a reason for hiding this comment

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

Consider handling the promise rejection for findOne to manage potential database errors more effectively.

if (!_member) {
// handle 404
const err = new Error(`project member not found for project id ${projectId} ` +
Expand All @@ -76,10 +211,13 @@ module.exports = [
return Promise.reject(err);
}

req.log.debug(`updated props ${JSON.stringify(updatedProps)}`);

Choose a reason for hiding this comment

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

Consider removing or adjusting the log level for this debug statement if it's not necessary for production, as excessive logging can impact performance and clutter logs.

req.log.debug(`previous values ${JSON.stringify(previousValue)}`);

Choose a reason for hiding this comment

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

Similar to the previous line, evaluate the necessity of this debug log statement for production environments.

// no updates if no change
if (updatedProps.role === previousValue.role &&
if ((updatedProps.role === previousValue.role || action === 'complete-copilot-requests') &&

Choose a reason for hiding this comment

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

The condition action === 'complete-copilot-requests' is added here. Ensure that this logic is correct and that it doesn't unintentionally bypass other necessary checks or conditions.

(_.isUndefined(updatedProps.isPrimary) ||
updatedProps.isPrimary === previousValue.isPrimary)) {
await completeAllCopilotRequests(req, projectId, _transaction, _member);

Choose a reason for hiding this comment

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

The function completeAllCopilotRequests now has an additional parameter _member. Ensure that this change is reflected in the function definition and that _member is being used appropriately within the function. If _member is not needed, consider removing it to maintain clarity and simplicity.

return Promise.resolve();
}

Expand Down Expand Up @@ -121,9 +259,13 @@ module.exports = [
});
})
.then(() => projectMember.reload(projectMember.id))
.then(() => {
.then(async () => {
projectMember = projectMember.get({ plain: true });

Choose a reason for hiding this comment

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

Consider using const instead of let for projectMember since it is reassigned but not mutated after this point.

projectMember = _.omit(projectMember, ['deletedAt']);

Choose a reason for hiding this comment

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

The function completeAllCopilotRequests now has an additional parameter _member. Ensure that this parameter is necessary for the function's operation and that the function definition is updated accordingly to handle this new argument.


if (['observer', 'customer'].includes(updatedProps.role)) {

Choose a reason for hiding this comment

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

The condition ['observer', 'customer'].includes(updatedProps.role) could be extracted into a named function or variable for better readability and maintainability.

await completeAllCopilotRequests(req, projectId, _transaction, _member);
}
})
.then(() => (
util.getObjectsWithMemberDetails([projectMember], fields, req)
Expand All @@ -145,6 +287,9 @@ module.exports = [
req.log.debug('updated project member', projectMember);
res.json(memberWithDetails || projectMember);
})
.catch(err => next(err)));
.catch(async (err) => {
await _transaction.rollback();

Choose a reason for hiding this comment

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

Consider checking the result of _transaction.rollback() for errors or exceptions to ensure that the rollback was successful.

return next(err);
}));
},
];