feat(API-CLIENT): Added workspace-membership and it's tests

Nil2000 commented 2 months ago

User description

Description

Created controller for Workspace Membership module

Fixes #446

Dependencies

Mention any dependencies/packages used

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

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

If changes are made in the code:

[x] I have followed the coding guidelines
[x] 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
[x] 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.

PR Type

Enhancement, Tests

Description

Implemented a new WorkspaceMembershipController to manage workspace memberships, including methods for transferring ownership, inviting/removing users, updating roles, and handling invitations.
Defined TypeScript types for workspace membership operations, including request and response interfaces and an Authority enum.
Updated the API client to export the new WorkspaceMembershipController.
Added comprehensive tests for the WorkspaceMembershipController to ensure correct functionality of all methods.

Changes walkthrough 📝

Relevant files

Enhancement

workspace-membership.ts `Implement Workspace Membership Controller with API Methods` packages/api-client/src/controllers/workspace-membership.ts Added `WorkspaceMembershipController` class with methods for managing workspace memberships. Implemented methods for transferring ownership, inviting/removing users, updating roles, and handling invitations. Integrated response parsing for API interactions.	+149/-0
index.ts `Export Workspace Membership Controller in API Client` packages/api-client/src/index.ts - Imported and exported `WorkspaceMembershipController`.	+4/-2
workspace-membership.types.d.ts `Define Types for Workspace Membership Operations` packages/api-client/src/types/workspace-membership.types.d.ts Defined types for workspace membership requests and responses. Included `Authority` enum for role permissions.	+145/-0

Tests

workspace-membership.spec.ts

Add Tests for Workspace Membership Controller

packages/api-client/tests/workspace-membership.spec.ts

Added tests for WorkspaceMembershipController methods.

Covered scenarios for ownership transfer, user invitations, and role
updates.

+158/-0

💡 PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

codiumai-pr-agent-free[bot] commented 2 months ago

PR Reviewer Guide 🔍

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Key issues to review

Error Handling
The controller methods don't include explicit error handling. Consider adding try-catch blocks or error handling middleware to manage potential API errors gracefully. Test Coverage
While the tests cover the basic functionality, they lack assertions for specific response properties and error scenarios. Consider adding more detailed assertions and error case tests.

codiumai-pr-agent-free[bot] commented 2 months ago

PR Code Suggestions ✨

Category	Suggestion	Score
Input validation	Add input validation for request parameters before making API calls ___ Consider implementing input validation for the request parameters before making API calls. This can help prevent unnecessary API calls with invalid data and provide better error messages to the client. [packages/api-client/src/controllers/workspace-membership.ts [35-44]](https://github.com/keyshade-xyz/keyshade/pull/452/files#diff-9cefca6b94d518b82ce149083ebd84ef032fe360df595f1cba6d91fb85ddbc62R35-R44) ```diff async transferOwnership( request: TransferOwnershipRequest, headers?: Record ): Promise> { + if (!request.workspaceSlug \|\| !request.userEmail) { + throw new Error('Invalid request: workspaceSlug and userEmail are required') + } const response = await this.apiClient.put( `/api/workspace-membership/${request.workspaceSlug}/transfer-ownership/${request.userEmail}`, request, headers ) return await parseResponse(response) } ``` - [ ] Apply this suggestion Suggestion importance[1-10]: 9 Why: Implementing input validation is crucial for preventing invalid data from being sent to the API, which can reduce unnecessary API calls and improve error messaging. This is a significant improvement in terms of data integrity and user experience.	9
Error handling	Implement specific error handling for API requests to provide more detailed error information ___ Consider using a more specific error handling mechanism for API requests. Instead of relying on the generic error handling provided by the APIClient, you could wrap each API call in a try-catch block to handle specific error cases and provide more detailed error information to the caller. [packages/api-client/src/controllers/workspace-membership.ts [39-44]](https://github.com/keyshade-xyz/keyshade/pull/452/files#diff-9cefca6b94d518b82ce149083ebd84ef032fe360df595f1cba6d91fb85ddbc62R39-R44) ```diff -const response = await this.apiClient.put( - `/api/workspace-membership/${request.workspaceSlug}/transfer-ownership/${request.userEmail}`, - request, - headers -) -return await parseResponse(response) +try { + const response = await this.apiClient.put( + `/api/workspace-membership/${request.workspaceSlug}/transfer-ownership/${request.userEmail}`, + request, + headers + ) + return await parseResponse(response) +} catch (error) { + // Handle specific error cases + if (error instanceof APIError) { + // Handle API-specific errors + throw new WorkspaceMembershipError('Failed to transfer ownership', error) + } + throw error +} ``` - [ ] Apply this suggestion Suggestion importance[1-10]: 8 Why: This suggestion enhances error handling by providing more specific error information, which can improve debugging and user feedback. It addresses a significant aspect of robustness in API interactions.	8
Testing	Add negative test cases to ensure proper error handling ___ Consider adding negative test cases to ensure the controller handles error scenarios correctly. This could include tests for invalid inputs, network errors, or API errors. [packages/api-client/tests/workspace-membership.spec.ts [33-43]](https://github.com/keyshade-xyz/keyshade/pull/452/files#diff-51217021dfbc8a7925c518b52884af0daf5c34df7757913e229aca1dd4dbd3caR33-R43) ```diff it('should transfer ownership', async () => { const request = { workspaceSlug: workspaceSlug!, userEmail: userEmail } const response = await workspaceMembershipController.transferOwnership( request, { 'x-e2e-user-email': userEmail } ) expect(response).toBeTruthy() }) +it('should handle invalid input when transferring ownership', async () => { + const invalidRequest = { workspaceSlug: '', userEmail: '' } + await expect(workspaceMembershipController.transferOwnership(invalidRequest, { + 'x-e2e-user-email': userEmail + })).rejects.toThrow('Invalid request') +}) + ``` - [ ] Apply this suggestion Suggestion importance[1-10]: 8 Why: Adding negative test cases is important for ensuring that the application handles error scenarios correctly, which is crucial for maintaining robustness and reliability in production environments.	8
Performance	Implement a rate limiting mechanism to prevent API abuse ___ Consider implementing a rate limiting mechanism to prevent abuse of the API. This could be done by adding a delay between consecutive API calls or by tracking the number of calls made within a certain time frame. [packages/api-client/src/controllers/workspace-membership.ts [28-33]](https://github.com/keyshade-xyz/keyshade/pull/452/files#diff-9cefca6b94d518b82ce149083ebd84ef032fe360df595f1cba6d91fb85ddbc62R28-R33) ```diff export default class WorkspaceMembershipController { private apiClient: APIClient + private lastCallTime: number = 0 + private readonly minTimeBetweenCalls: number = 1000 // 1 second constructor(private readonly backendUrl: string) { this.apiClient = new APIClient(this.backendUrl) } + private async rateLimit() { + const now = Date.now() + const timeSinceLastCall = now - this.lastCallTime + if (timeSinceLastCall < this.minTimeBetweenCalls) { + await new Promise(resolve => setTimeout(resolve, this.minTimeBetweenCalls - timeSinceLastCall)) + } + this.lastCallTime = Date.now() + } + ``` - [ ] Apply this suggestion Suggestion importance[1-10]: 7 Why: While rate limiting is important for preventing API abuse, the suggestion is more of a performance optimization rather than a critical fix. It is beneficial but not immediately necessary for the functionality.	7

💡 Need additional feedback ? start a PR chat

Nil2000 commented 2 months ago

@rajdip-b Let me know if it's fine or require any changes

rajdip-b commented 3 weeks ago

:tada: This PR is included in version 2.6.0 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket:

keyshade-xyz / keyshade