Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| COPILOTS_URL: 'https://copilots.topcoder-dev.com', | ||
|
|
||
| // Projects API v6: keep dev default (switch to LOCAL_PROJECTS_API when needed) | ||
| PROJECT_API_URL: `${DEV_API_HOSTNAME}/v6/projects`, |
There was a problem hiding this comment.
[maintainability]
The PROJECT_API_URL is set to use the development API host (DEV_API_HOSTNAME). Consider using the local endpoint LOCAL_PROJECTS_API for consistency with other local services, unless there's a specific reason to keep it on the dev environment.
| checkAdmin: jest.fn(), | ||
| checkReadOnlyRoles: jest.fn(), | ||
| checkAdminOrCopilotOrManager: jest.fn(), | ||
| checkCanManageProject: jest.fn(), |
There was a problem hiding this comment.
[❗❗ correctness]
The function checkCanManageProject is replacing checkAdminOrCopilotOrManager. Ensure that the new function provides equivalent or improved functionality, as this change may affect permission checks.
| checkCanViewProjectAssets: jest.fn(), | ||
| checkManager: jest.fn(), | ||
| getProjectMemberByUserId: jest.fn((project, userId) => { | ||
| getProjectMemberRole: jest.fn((project, userId) => { |
There was a problem hiding this comment.
[❗❗ correctness]
The function getProjectMemberRole now returns the role directly instead of the member object. Verify that all usages of this function are updated accordingly, as this change alters the expected return value.
| const roles = _.get(tokenData, 'roles') | ||
| const talentManagerRoles = ['talent manager', 'topcoder talent manager'] | ||
| return roles.some(val => talentManagerRoles.indexOf(val.toLowerCase()) > -1) | ||
| return roles.some(val => TALENT_MANAGER_ROLES.indexOf(val.toLowerCase()) > -1) |
There was a problem hiding this comment.
[maintainability]
The constant TALENT_MANAGER_ROLES is defined at the top of the file and is used here. Ensure that this constant is imported from a shared constants file if it is used across multiple files to avoid duplication and improve maintainability.
| return project && !_.isEmpty(project) && (_.find( | ||
| project.invites, | ||
| d => ( | ||
| d.status === PROJECT_MEMBER_INVITE_STATUS_PENDING && |
There was a problem hiding this comment.
[❗❗ correctness]
The addition of d.status === PROJECT_MEMBER_INVITE_STATUS_PENDING improves the correctness of the invitation check by ensuring only pending invites are considered. This is a necessary change to prevent processing non-actionable invites.
| }) | ||
|
|
||
| describe('getProjectMemberByUserId', () => { | ||
| it('matches project members even when ids differ by string vs number types', () => { |
There was a problem hiding this comment.
[correctness]
The test case for getProjectMemberByUserId relies on implicit type coercion between string and number for userId. Consider explicitly converting userId to a consistent type to avoid potential issues with type mismatches in the future.
| decodeToken.mockReset() | ||
| }) | ||
|
|
||
| it('returns the pending invite for the authenticated user', () => { |
There was a problem hiding this comment.
[correctness]
The test case for checkIsUserInvitedToProject assumes that the decodeToken function will always return a valid object with userId and email. Consider adding a test case to handle scenarios where decodeToken might return an unexpected value or throw an error, to ensure robustness.
1 participant
https://topcoder.atlassian.net/browse/PM-4376