Conversation
fix(PM-2435): identity service response structure
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| } | ||
|
|
||
| const opportunity = await models.CopilotOpportunity.create(data, { transaction }); | ||
| req.log.debug('Created new copilot opportunity', { opportunityId: opportunity.id }); |
There was a problem hiding this comment.
[correctness]
Consider adding a check to ensure that opportunity is not null or undefined before logging its id. This will prevent potential runtime errors if create fails silently.
| subjects.forEach(subject => sendNotification(subject.handle, subject.email)); | ||
|
|
||
| // send email to notify via slack | ||
| sendNotification('Copilots', config.copilotsSlackEmail); |
There was a problem hiding this comment.
[security]
The email address config.copilotsSlackEmail should be validated to ensure it is correctly configured. If this value is incorrect, the notification will fail silently.
| throw err; | ||
| } | ||
| try { | ||
| const copilotRequest = await models.sequelize.transaction(async (transaction) => { |
There was a problem hiding this comment.
[performance]
Consider adding a timeout or a retry mechanism for the transaction to handle potential deadlocks or long-running operations.
|
|
||
| return res.status(201).json(copilotRequest); | ||
| } catch (err) { | ||
| req.log.error('Error creating copilot request', { error: err }); |
There was a problem hiding this comment.
[❗❗ security]
Logging the error message directly might expose sensitive information. Consider sanitizing the error message before logging.
| return _.get(res, 'data.result.content', []) | ||
| const roles = res.data; | ||
| logger.debug(`Roles by ${roleName}: ${JSON.stringify(roles)}`); | ||
| return roles.result.content |
There was a problem hiding this comment.
[❗❗ correctness]
The roles.result.content is accessed without checking if roles.result is defined. This could lead to a runtime error if the response structure changes or if the expected data is not present. Consider adding a check to ensure roles.result is defined before accessing roles.result.content.
Reduced the branches for deployDev to only 'develop'.
| filters: | ||
| branches: | ||
| only: ['develop', 'migration-setup', 'PM-1612', 'fix-project-exposing'] | ||
| only: ['develop'] |
There was a problem hiding this comment.
[design]
The branch filter for the deployDev job has been changed to only include the develop branch. Ensure that this aligns with the intended deployment strategy, as it removes the ability to deploy from other branches like migration-setup, PM-1612, fix-2321, and pm-2435. This could impact the flexibility of testing and deploying changes from feature or bug-fix branches.
3 participants
https://topcoder.atlassian.net/browse/PM-2321