Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
683b3d6
added existing membership to the copilot application
hentrymartin Jul 30, 2025
f0a889f
added existing membership to the copilot application
hentrymartin Jul 30, 2025
4823330
added existing membership to the copilot application
hentrymartin Jul 30, 2025
26dc288
added existing membership to the copilot application
hentrymartin Jul 30, 2025
59c6077
added existing membership to the copilot application
hentrymartin Jul 30, 2025
2ba33ac
added existing membership to the copilot application
hentrymartin Jul 30, 2025
04b4e37
added existing membership to the copilot application
hentrymartin Jul 30, 2025
f2a8c89
added existing membership to the copilot application
hentrymartin Jul 30, 2025
76a22c1
added existing membership to the copilot application
hentrymartin Jul 30, 2025
cf120a8
added existing membership to the copilot application
hentrymartin Jul 30, 2025
471d5e1
added existing membership to the copilot application
hentrymartin Jul 30, 2025
1559e31
added existing membership to the copilot application
hentrymartin Jul 30, 2025
44ac842
added existing membership to the copilot application
hentrymartin Jul 30, 2025
de8c14f
added existing membership to the copilot application
hentrymartin Jul 30, 2025
40076d3
added existing membership to the copilot application
hentrymartin Jul 30, 2025
d40373d
added existing membership to the copilot application
hentrymartin Jul 30, 2025
8b2fa3e
added existing membership to the copilot application
hentrymartin Jul 30, 2025
e7a328b
added existing membership to the copilot application
hentrymartin Jul 30, 2025
ab268ca
added existing membership to the copilot application
hentrymartin Jul 30, 2025
e82e6b2
added existing membership to the copilot application
hentrymartin Jul 30, 2025
dad92bc
fix: show already member modal
hentrymartin Jul 30, 2025
ada10e8
fix: show already member modal
hentrymartin Jul 30, 2025
c2df9ee
fix: show already member modal
hentrymartin Jul 30, 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
78 changes: 72 additions & 6 deletions src/routes/copilotOpportunity/assign.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,17 @@ module.exports = [
throw err;
}

const copilotRequest = await models.CopilotRequest.findOne({

Choose a reason for hiding this comment

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

Consider checking if copilotRequest is null or undefined after fetching it from the database. This will help prevent potential runtime errors if the request is not found.

where: { id: opportunity.copilotRequestId },
transaction: t,
});

const application = await models.CopilotApplication.findOne({
where: { id: applicationId, opportunityId: copilotOpportunityId },
transaction: t,
});


Choose a reason for hiding this comment

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

Remove the extra blank line here to maintain consistent formatting and readability.

if (!application) {
const err = new Error('No such application available');
err.status = 400;
Expand All @@ -65,12 +71,72 @@ module.exports = [
const projectId = opportunity.projectId;
const userId = application.userId;
const activeMembers = await models.ProjectMember.getActiveProjectMembers(projectId, t);

const existingUser = activeMembers.find(item => item.userId === userId);
if (existingUser && existingUser.role === 'copilot') {
const err = new Error(`User is already a copilot of this project`);
err.status = 400;
throw err;
const updateCopilotOpportunity = async () => {
await opportunity.update({
status: COPILOT_OPPORTUNITY_STATUS.COMPLETED,
}, {
transaction: t,
});
await application.update({
status: COPILOT_APPLICATION_STATUS.ACCEPTED,
}, {
transaction: t,
});

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

await models.CopilotApplication.update({
status: COPILOT_APPLICATION_STATUS.CANCELED,
}, {
where: {
projectId,
opportunityId: opportunity.id,

Choose a reason for hiding this comment

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

The removal of projectId from the where clause may affect the specificity of the query. Ensure that this change is intentional and that the query will still correctly identify the intended records without this condition.

id: {
$ne: application.id,
},
}
});
};

const existingMember = activeMembers.find(item => item.userId === userId);
if (existingMember) {

Choose a reason for hiding this comment

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

The logic for checking existingMember and its role could be optimized. Consider combining the role check with the initial find operation to reduce complexity and improve readability.

req.log.debug(`User already part of project: ${JSON.stringify(existingMember)}`);

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 JSON.stringify(existingMember) might throw an error if existingMember contains circular references. You could use a try-catch block or a safe stringification method.

if (['copilot', 'manager'].includes(existingMember.role)) {
req.log.debug(`User is a copilot or manager`);

Choose a reason for hiding this comment

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

The debug log message User is a copilot or manager could be more descriptive by including the user's role. Consider logging the actual role for better traceability.

updateCopilotOpportunity();

Choose a reason for hiding this comment

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

The function updateCopilotOpportunity() is called without await. If this function performs asynchronous operations, consider using await to ensure it completes before proceeding.

} else {
req.log.debug(`User has read/write role`);

Choose a reason for hiding this comment

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

The debug log message User has read/write role might be misleading if the user has other roles. Consider specifying the exact role or roles the user has for clarity.

await models.ProjectMember.update({

Choose a reason for hiding this comment

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

The await keyword is used here, but the surrounding code does not handle potential errors that might arise from this asynchronous operation. Consider adding error handling to manage any exceptions that may occur.

role: 'copilot',
}, {
where: {
id: existingMember.id,
},
});

const projectMember = await models.ProjectMember.findOne({
where: {
id: existingMember.id,
},
});

req.log.debug(`Updated project member: ${JSON.stringify(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 handling potential errors that might occur during the JSON.stringify operation. If projectMember.get({plain: true}) contains circular references or other non-serializable data, JSON.stringify could throw an error.


util.sendResourceToKafkaBus(
req,
EVENT.ROUTING_KEY.PROJECT_MEMBER_UPDATED,
RESOURCES.PROJECT_MEMBER,
projectMember.get({ plain: true }),
existingMember);
req.log.debug(`Member updated in kafka`);

Choose a reason for hiding this comment

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

The log message Member updated in kafka could be more descriptive. Consider including additional context, such as the member ID or other relevant details, to make the log more informative.

updateCopilotOpportunity();

Choose a reason for hiding this comment

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

The removal of t.commit(); suggests that the transaction might not be committed anymore. Ensure that the transaction is being properly handled to avoid potential data inconsistencies.

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

const existingInvite = await models.ProjectMemberInvite.findAll({
Expand Down
55 changes: 42 additions & 13 deletions src/routes/copilotOpportunityApply/list.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,48 @@ module.exports = [
canAccessAllApplications ? {} : { createdBy: userId },
);

Choose a reason for hiding this comment

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

The opportunityId variable is used here, but it is not clear from the diff where it is defined or assigned. Ensure that opportunityId is properly defined and assigned a value before this line.

return models.CopilotApplication.findAll({
where: whereCondition,
include: [
{
model: models.CopilotOpportunity,
as: 'copilotOpportunity',
},
],
order: [[sortParams[0], sortParams[1]]],
return models.CopilotOpportunity.findOne({
where: {
id: opportunityId,
}
}).then((opportunity) => {
if (!opportunity) {
const err = new Error('No opportunity found');
err.status = 404;
throw err;
}
return models.CopilotApplication.findAll({
where: whereCondition,
include: [
{
model: models.CopilotOpportunity,
as: 'copilotOpportunity',
},
],
order: [[sortParams[0], sortParams[1]]],
})
.then(copilotApplications => {
req.log.debug(`CopilotApplications ${JSON.stringify(copilotApplications)}`);

Choose a reason for hiding this comment

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

Consider using a more descriptive log message to clarify what copilotApplications represents in this context.

return models.ProjectMember.getActiveProjectMembers(opportunity.projectId).then((members) => {
req.log.debug(`Fetched existing active members ${JSON.stringify(members)}`);
req.log.debug(`Applications ${JSON.stringify(copilotApplications)}`);

Choose a reason for hiding this comment

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

The log message for Applications is redundant with the previous log message for CopilotApplications. Consider removing one of them to avoid unnecessary duplication.

const enrichedApplications = copilotApplications.map(application => {

Choose a reason for hiding this comment

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

Consider using const for enrichedApplications only if it is not reassigned later in the code. If it is reassigned, let would be more appropriate.

const m = members.find(m => m.userId === application.userId);

Choose a reason for hiding this comment

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

The debug log statement was removed but the comment remains. Consider removing the comment as it is not necessary to keep commented-out code.

req.log.debug(`Existing member to application ${JSON.stringify(Object.assign({}, application, {

Choose a reason for hiding this comment

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

Consider using the spread operator instead of Object.assign for better readability and consistency with modern JavaScript practices. Example: { ...application, existingMembership: m }.

existingMembership: m,
}))}`);
return Object.assign({}, application, {
existingMembership: m,
});
});

req.log.debug(`Enriched Applications ${JSON.stringify(enrichedApplications)}`);

Choose a reason for hiding this comment

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

The debug log message could be more informative by including the number of enriched applications. Consider adding enrichedApplications.length to the log message.

res.status(200).send(enrichedApplications);

Choose a reason for hiding this comment

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

The response method has been changed from res.json to res.status(200).send. Ensure that this change is intentional and that send is the appropriate method for sending the enriched applications. If res.json was previously used to automatically set the content-type to application/json, verify that send achieves the same result.

});
})
})
.then(copilotApplications => res.json(copilotApplications))
.catch((err) => {
util.handleError('Error fetching copilot applications', err, req, next);
});
.catch((err) => {
util.handleError('Error fetching copilot applications', err, req, next);
});
},
];