fix(PM-1273): Send canApplyAsCopilot to check if the user can apply for the opportunity #824
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -20,6 +20,13 @@ module.exports = [ | |||
| model: models.Project, | |||
| as: 'project', | |||
| attributes: ['name'], | |||
There was a problem hiding this comment.
The attributes array on line 22 originally included 'members', which has now been moved to a separate include block. Ensure that this change aligns with the intended data structure and that the 'members' attribute is correctly populated in the response.
| @@ -34,6 +34,10 @@ module.exports = function defineProjectMember(sequelize, DataTypes) { | |||
| ], | |||
| }); | |||
|
|
|||
| ProjectMember.associate = (models) => { | |||
| ProjectMember.belongsTo(models.Project, { foreignKey: 'projectId' }); | |||
| }; | |||
There was a problem hiding this comment.
Consider adding validation or error handling for the association to ensure that the projectId foreign key is valid and exists in the Project model. This can help prevent potential runtime errors if the association is used with invalid data.
src/routes/copilotOpportunity/get.js
Outdated
| let canApplyAsCopilot = false; | ||
| if (plainOpportunity && plainOpportunity.project && plainOpportunity.project.members && req.authUser) { | ||
| const existingMember = plainOpportunity.project.members.find(item => item.userId === req.authUser.userId); | ||
| canApplyAsCopilot = !!!existingMember; |
There was a problem hiding this comment.
Using !!!existingMember is confusing and can lead to readability issues. Consider using !existingMember to clearly express the intent of checking for non-existence.
| const existingMember = plainOpportunity.project.members.find(item => item.userId === req.authUser.userId); | ||
| canApplyAsCopilot = !!!existingMember; | ||
| } | ||
| // This shouldn't be exposed to the clientside |
There was a problem hiding this comment.
The comment on line 40 suggests that plainOpportunity.project.members should not be exposed to the client-side. Ensure that this deletion is handled securely and that no sensitive information is inadvertently exposed elsewhere in the code.
src/routes/copilotOpportunity/get.js
Outdated
| }, | ||
| ], | ||
| }) | ||
| .then((copilotOpportunity) => { | ||
| const plainOpportunity = copilotOpportunity.get({ plain: true }); | ||
| const formattedOpportunity = Object.assign({}, plainOpportunity, | ||
| let canApplyAsCopilot = false; | ||
| req.log.info(plainOpportunity.project.members); |
There was a problem hiding this comment.
Logging sensitive information such as project members can lead to security issues. Consider removing or sanitizing this log statement.
src/routes/copilotOpportunity/get.js
Outdated
| const formattedOpportunity = Object.assign({}, plainOpportunity, | ||
| let canApplyAsCopilot = false; | ||
| req.log.info(plainOpportunity.project.members); | ||
| req.log.info(req.authUser, 'authuser'); |
There was a problem hiding this comment.
Logging the entire authUser object may expose sensitive user information. Consider logging only non-sensitive parts of the object.
src/routes/copilotOpportunity/get.js
Outdated
| req.log.info(req.authUser, 'authuser'); | ||
| if (plainOpportunity && plainOpportunity.project && plainOpportunity.project.members && req.authUser) { | ||
| const existingMember = plainOpportunity.project.members.find(item => item.userId === req.authUser.userId); | ||
| req.log.info(existingMember, 'existingMember'); |
There was a problem hiding this comment.
Ensure that logging existingMember does not expose any sensitive information. Consider logging only necessary details or removing this log statement.
src/routes/copilotOpportunity/get.js
Outdated
| }, | ||
| ], | ||
| }) | ||
| .then((copilotOpportunity) => { | ||
| const plainOpportunity = copilotOpportunity.get({ plain: true }); | ||
| const formattedOpportunity = Object.assign({}, plainOpportunity, | ||
| const memberIds = plainOpportunity.project.members.map((member) => member.userId); |
There was a problem hiding this comment.
The variable memberIds is being created by mapping over plainOpportunity.project.members. Ensure that plainOpportunity.project.members is defined before attempting to map over it to avoid potential runtime errors.
| const formattedOpportunity = Object.assign({}, plainOpportunity, | ||
| const memberIds = plainOpportunity.project.members.map((member) => member.userId); | ||
| // This shouldn't be exposed to the clientside | ||
| delete plainOpportunity.project.members; |
There was a problem hiding this comment.
Consider adding a check to ensure plainOpportunity.project exists before attempting to delete plainOpportunity.project.members to prevent potential errors if project is undefined.
hentrymartin
changed the title
| }, | ||
| ], | ||
| }) | ||
| .then((copilotOpportunity) => { | ||
| const plainOpportunity = copilotOpportunity.get({ plain: true }); | ||
| const formattedOpportunity = Object.assign({}, plainOpportunity, | ||
| req.log.info("authUser", req.authUser); |
There was a problem hiding this comment.
Logging sensitive information such as authUser can pose security risks if it contains personal or sensitive data. Consider reviewing what information is being logged and ensure it complies with security best practices.
src/routes/copilotOpportunity/get.js
Outdated
| const formattedOpportunity = Object.assign({}, plainOpportunity, | ||
| req.log.info("authUser", req.authUser); | ||
| const memberIds = plainOpportunity.project.members.map((member) => member.userId); | ||
| const canApplyAsCopilot = !memberIds.includes(req.authUser.userId) |
There was a problem hiding this comment.
Consider adding a check to ensure req.authUser and req.authUser.userId are defined before attempting to access userId. This will prevent potential runtime errors if req.authUser is undefined.
| }, | ||
| ], | ||
| }) | ||
| .then((copilotOpportunity) => { | ||
| const plainOpportunity = copilotOpportunity.get({ plain: true }); | ||
| const formattedOpportunity = Object.assign({}, plainOpportunity, | ||
| req.log.info("authUser", req.authUser); | ||
| const memberIds = plainOpportunity.project.members && plainOpportunity.project.members.map((member) => member.userId); |
There was a problem hiding this comment.
Consider adding a null check for plainOpportunity.project before accessing members to prevent potential runtime errors if project is undefined.
| const memberIds = plainOpportunity.project.members && plainOpportunity.project.members.map((member) => member.userId); | ||
| let canApplyAsCopilot = false; | ||
| if (req.authUser) { | ||
| canApplyAsCopilot = !memberIds.includes(req.authUser.userId) |
There was a problem hiding this comment.
Ensure memberIds is properly initialized before calling includes to avoid errors if plainOpportunity.project.members is null or undefined.
hentrymartin
changed the title
What's in this PR?
Ticket link - https://topcoder.atlassian.net/browse/PM-1273