-
Notifications
You must be signed in to change notification settings - Fork 55
feat(PM-1610): Directly add copilot to project #845
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
Conversation
@@ -172,51 +173,41 @@ module.exports = [ | |||
return; | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The removal of the check for existing pending invites may lead to duplicate invites being created. Consider implementing a mechanism to prevent adding a user as a copilot if they already have a pending invite.
source: 'copilot_portal', | ||
}), | ||
); | ||
await util.addUserToProject(req, member, t) | ||
|
||
await application.update({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The status update from COPILOT_APPLICATION_STATUS.INVITED
to COPILOT_APPLICATION_STATUS.ACCEPTED
should be verified to ensure it aligns with the intended business logic, as this change may have implications on the application workflow.
}, { | ||
transaction: t, | ||
}); | ||
|
||
// Cancel other applications | ||
const otherApplications = await models.CopilotApplication.findAll({ |
There was a problem hiding this comment.
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 findAll
query to manage potential database access issues or unexpected results when fetching other applications.
}); | ||
|
||
for (const otherApplication of otherApplications) { | ||
await otherApplication.update({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The loop updating the status of other applications to CANCELED
should include error handling to manage potential failures during the update process, ensuring that the transaction can be rolled back if necessary.
projectId, | ||
applicationId: application.id, | ||
role: USER_ROLE.TC_COPILOT, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that USER_ROLE.TC_COPILOT
is defined and correctly imported or available in the scope. If this is a new role, verify that it is handled appropriately throughout the application.
What's in this PR?
Ticket link - https://topcoder.atlassian.net/browse/PM-1610