fix(api): Add NotFound exception on passing an invalid roleId while inviting user in workspace

[PriyobrotoKar]

Description

Added validation to ensure that If a workspace role is provided in the roleIds array doesn't exist, a NotFoundException is thrown.

Fixes #405

Dependencies

No new dependencies were added.

Future Improvements

Mention any improvements to be done in future related to any file/feature

Mentions

Mention and tag the people

Screenshots of relevant screens

Add screenshots of relevant screens

Developer's checklist

• My PR follows the style guidelines of this project
• I have performed a self-check on my work

If changes are made in the code:

• I have followed the coding guidelines
• My changes in code generate no new warnings
• My changes are breaking another fix/feature of the project
• I have added test cases to show that my feature works
• I have added relevant screenshots in my PR
• There are no UI/UX issues

Documentation Update

• This PR requires an update to the documentation at docs.keyshade.xyz
• I have made the necessary updates to the documentation, or no documentation changes are required.

[codiumai-pr-agent-free]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review Code Duplication The code for checking valid roles is duplicated in two different methods. Consider extracting this logic into a separate method to improve maintainability. Performance Concern The role validation involves multiple database queries and array operations. For large workspaces with many roles, this could potentially impact performance.

[codiumai-pr-agent-free]

PR Code Suggestions ✨

Category	Suggestion	Score
Enhancement	✅ Extract role validation logic into a separate method for better code organization and reusability Suggestion Impact: The role validation logic was extracted into a separate method named `findInvalidWorkspaceRoles`, improving code reusability and maintainability. code diff: + const invalidRoles = await this.findInvalidWorkspaceRoles( + workspace.id, + roleIds ) if (invalidRoles.length > 0) { @@ -1000,18 +991,9 @@ ) } - //Check if the member has valid roles - const roles = await this.prisma.workspaceRole.findMany({ - where: { - id: { - in: member.roleIds - }, - workspaceId: workspace.id - } - }) - - const invalidRoles = member.roleIds.filter( - (id) => !roles.map((role) => role.id).includes(id) + const invalidRoles = await this.findInvalidWorkspaceRoles( + workspace.id, + member.roleIds ) if (invalidRoles.length > 0) { @@ -1085,6 +1067,26 @@ } } + private async findInvalidWorkspaceRoles( + workspaceId: string, + roleIds: string[] + ) { + const roles = await this.prisma.workspaceRole.findMany({ + where: { + id: { + in: roleIds + }, + workspaceId: workspaceId + } + }) + + const roleIdSet = new Set(roles.map((role) => role.id)) + + const invalidRoles = roleIds.filter((id) => !roleIdSet.has(id)) + + return invalidRoles Consider extracting the role validation logic into a separate method to improve code reusability and maintainability. This method can be used in both the update and invite scenarios. apps/api/src/workspace/service/workspace.service.ts [418-435] -const roles = await this.prisma.workspaceRole.findMany({ - where: { - id: { - in: roleIds - }, - workspaceId: workspace.id +await this.validateWorkspaceRoles(workspace, roleIds); + +// New method to be added to the class: +private async validateWorkspaceRoles(workspace: Workspace, roleIds: string[]): Promise<void> { + const roles = await this.prisma.workspaceRole.findMany({ + where: { + id: { in: roleIds }, + workspaceId: workspace.id + } + }); + + const invalidRoles = roleIds.filter((id) => !roles.map((role) => role.id).includes(id)); + + if (invalidRoles.length > 0) { + throw new NotFoundException( + `Workspace ${workspace.name} (${workspace.id}) does not have roles ${invalidRoles.join(', ')}` + ); } -}) - -const invalidRoles = roleIds.filter( - (id) => !roles.map((role) => role.id).includes(id) -) - -if (invalidRoles.length > 0) { - throw new NotFoundException( - `Workspace ${workspace.name} (${workspace.id}) does not have roles ${invalidRoles.join(', ')}` - ) } Apply this suggestion Suggestion importance[1-10]: 7 Why:	7
Performance	✅ Use Set for more efficient role ID comparison Use Set data structure for more efficient role ID comparison instead of using Array.prototype.includes(). apps/api/src/workspace/service/workspace.service.ts [427-429] -const invalidRoles = roleIds.filter( - (id) => !roles.map((role) => role.id).includes(id) -) +const roleIdSet = new Set(roles.map((role) => role.id)); +const invalidRoles = roleIds.filter((id) => !roleIdSet.has(id)); [Suggestion has been applied] Suggestion importance[1-10]: 7 Why:	7
Best practice	Use a more appropriate exception type for invalid role IDs Consider using a more specific error type like BadRequestException instead of NotFoundException when invalid role IDs are provided, as it's more appropriate for this scenario. apps/api/src/workspace/service/workspace.service.ts [431-435] if (invalidRoles.length > 0) { - throw new NotFoundException( - `Workspace ${workspace.name} (${workspace.id}) does not have roles ${invalidRoles.join(', ')}` + throw new BadRequestException( + `Invalid role IDs provided for workspace ${workspace.name} (${workspace.id}): ${invalidRoles.join(', ')}` ) } Apply this suggestion Suggestion importance[1-10]: 7 Why:	7
Security	Add a limit to the number of role IDs that can be processed at once Consider adding a limit to the number of role IDs that can be processed at once to prevent potential performance issues or abuse. apps/api/src/workspace/service/workspace.service.ts [418-425] +const MAX_ROLES = 50; // Adjust this value as needed +if (roleIds.length > MAX_ROLES) { + throw new BadRequestException(`Cannot process more than ${MAX_ROLES} roles at once.`); +} const roles = await this.prisma.workspaceRole.findMany({ where: { id: { in: roleIds }, workspaceId: workspace.id } }) Apply this suggestion Suggestion importance[1-10]: 7 Why:	7

[PriyobrotoKar]

@rajdip-b I have made those changes.