Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Sep 20, 2024

Conversation

Nil2000
Copy link
Contributor

@Nil2000 Nil2000 commented Sep 19, 2024

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

  • 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.

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

    Copy link
    Contributor

    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.

    Copy link
    Contributor

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    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]

     async transferOwnership(
       request: TransferOwnershipRequest,
       headers?: Record<string, string>
     ): Promise<ClientResponse<TransferOwnershipResponse>> {
    +  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<TransferOwnershipResponse>(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]

    -const response = await this.apiClient.put(
    -  `/api/workspace-membership/${request.workspaceSlug}/transfer-ownership/${request.userEmail}`,
    -  request,
    -  headers
    -)
    -return await parseResponse<TransferOwnershipResponse>(response)
    +try {
    +  const response = await this.apiClient.put(
    +    `/api/workspace-membership/${request.workspaceSlug}/transfer-ownership/${request.userEmail}`,
    +    request,
    +    headers
    +  )
    +  return await parseResponse<TransferOwnershipResponse>(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]

     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]

     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 Nil2000 changed the title feat(API-CLIENT) : Added workspace-membership and it's tests feat(API-CLIENT) :Added workspace-membership and it's tests Sep 19, 2024
    @Nil2000 Nil2000 changed the title feat(API-CLIENT) :Added workspace-membership and it's tests feat(API-CLIENT): Added workspace-membership and it's tests Sep 19, 2024
    @Nil2000
    Copy link
    Contributor Author

    Nil2000 commented Sep 19, 2024

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

    @rajdip-b rajdip-b merged commit 6a1c091 into keyshade-xyz:develop Sep 20, 2024
    4 of 8 checks passed
    @Nil2000 Nil2000 deleted the nilabhra-issue-446 branch September 20, 2024 07:00
    Kiranchaudhary537 pushed a commit to Kiranchaudhary537/keyshade that referenced this pull request Oct 13, 2024
    rajdip-b pushed a commit that referenced this pull request Oct 24, 2024
    ## [2.6.0](v2.5.0...v2.6.0) (2024-10-24)
    
    ### 🚀 Features
    
    * **api:**  Add icon and remove description field from workspace ([#435](#435)) ([a99c0db](a99c0db))
    * **api-client:** Added workspace-membership and related tests ([#452](#452)) ([6a1c091](6a1c091))
    * **api-client:** Create controller for User module ([#484](#484)) ([f9d8e83](f9d8e83))
    * **api:** Add prod env schema in env file ([#436](#436)) ([21c3004](21c3004))
    * **api:** Add resend otp implementation ([#445](#445)) ([4dc6aa1](4dc6aa1))
    * **api:** Fetch total count of environments, [secure]s and variables in project ([#434](#434)) ([0c9e50a](0c9e50a))
    * **api:** Replace `projectId` with `name` and `slug` in workspace-role response.  ([#432](#432)) ([af06071](af06071))
    * **cli:** Add functionality to operate on Secrets ([#504](#504)) ([1b4bf2f](1b4bf2f))
    * **cli:** Add project command ([#451](#451)) ([70448e1](70448e1))
    * **cli:** Add workspace operations ([#441](#441)) ([ed38d22](ed38d22))
    * **cli:** implement commands to get, list, update, and delete, workspace roles ([#469](#469)) ([957ea8d](957ea8d))
    * **cli:** Implemented pagination support ([#453](#453)) ([feb1806](feb1806))
    * **cli:** Secret scan ([#438](#438)) ([85cb8ab](85cb8ab))
    * **cli:** Update environment command outputs ([f4af874](f4af874))
    * **platform:** Clearing email field after waitlisting the user email ([#481](#481)) ([256d659](256d659))
    * Remove project IDs from workspace role export data and update tests ([#448](#448)) ([8fdb328](8fdb328))
    * **web:** Configured extra check for waitlisted users already in the list and created toast message for them ([#492](#492)) ([2ddd0ef](2ddd0ef))
    * **web:** show the toast only when email add successfully ([#490](#490)) ([783c411](783c411))
    
    ### 🐛 Bug Fixes
    
    * **api,api-client:** Add environmentSlug in multiple places across the variable module ([#468](#468)) ([d970aff](d970aff))
    * **api:** Replace the id with slug in the global-search service ([#455](#455)) ([74804b1](74804b1))
    * **platform:** Fixed duplicate Google Logo UI fix  ([#450](#450)) ([fb0d6f7](fb0d6f7))
    * resolve footer website name cut-off or overlap issue ([#444](#444)) ([fe03ba2](fe03ba2))
    * **web:** Horizontal Scrolling issue on the website ([#440](#440)) ([655177b](655177b))
    
    ### 📚 Documentation
    
    * Add documentation for environment in CLI ([#462](#462)) ([dad7394](dad7394))
    * Add documentation for project in CLI ([#466](#466)) ([341fb32](341fb32))
    * Add documentation for scan in CLI ([#461](#461)) ([72281e6](72281e6))
    * Add documentation for workspace command ([#464](#464)) ([4aad8a2](4aad8a2))
    * Add instructions for resetting the local Prisma database ([#495](#495)) ([#501](#501)) ([b07ea17](b07ea17))
    * Added docker support documentation ([#465](#465)) ([bc04be4](bc04be4))
    * Added documentation for running the platform ([#473](#473)) ([8b8386b](8b8386b))
    * Added missing mappings to pages ([5de9fd8](5de9fd8))
    * Fix Documentation Hyperlink and update expired Discord invite link ([#496](#496)) ([5a10e39](5a10e39))
    * Updated CLI docs ([#460](#460)) ([c7e0f13](c7e0f13))
    
    ### 🔧 Miscellaneous Chores
    
    * Add more logging to Sentry init ([#470](#470)) ([de4925d](de4925d))
    * **api:** Optimise API docker image size ([#360](#360)) ([ea40dc1](ea40dc1))
    * **api:** Updated lockfile ([a968e78](a968e78))
    * **CI:** Add [secure] scan validation ([f441262](f441262))
    * **cli:** Update controller invocation in environment commands ([#477](#477)) ([596bd1a](596bd1a))
    * Minor changes to variables ([fe01ca6](fe01ca6))
    * **[secure]-scan:** Failing lint issues ([#507](#507)) ([48f45df](48f45df))
    * **[secure]-scan:** Formatted files ([5884833](5884833))
    * Update .env.example ([70ad4f7](70ad4f7))
    * Updated scripts ([9eb76a7](9eb76a7))
    * **web:** email validation ([#487](#487)) ([e8e737a](e8e737a))
    @rajdip-b
    Copy link
    Member

    🎉 This PR is included in version 2.6.0 🎉

    The release is available on GitHub release

    Your semantic-release bot 📦🚀

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    API-CLIENT: Create controller for Workspace Membership module
    2 participants