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): Add endpoint to fetch all workspace invitations for a user #586

Merged

Conversation

muntaxir4
Copy link
Contributor

@muntaxir4 muntaxir4 commented Dec 9, 2024

User description

Description

Created an endpoint /invitations under workspace to retrieve all unaccepted invitations. Three E2E Tests were also added.

Fixes #550
Fixes #551

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

  • Added a new API endpoint /invitations to fetch all workspace invitations for a user, with support for pagination and filtering by acceptance status.
  • Implemented the getInvitationsOfUser method in the WorkspaceService to handle the logic for fetching and paginating invitations.
  • Added end-to-end tests to verify the functionality of the new endpoint, including tests for filtering by accepted invitations.

Changes walkthrough 📝

Relevant files
Enhancement
workspace.controller.ts
Add endpoint to fetch workspace invitations                           

apps/api/src/workspace/controller/workspace.controller.ts

  • Added a new endpoint GET /invitations to fetch workspace invitations.
  • Implemented query parameters for pagination and filtering.
  • +22/-0   
    workspace.service.ts
    Implement service method for fetching user invitations     

    apps/api/src/workspace/service/workspace.service.ts

  • Added getInvitationsOfUser method to fetch user invitations.
  • Implemented pagination and filtering logic for invitations.
  • +97/-0   
    Tests
    workspace.e2e.spec.ts
    Add E2E tests for workspace invitations endpoint                 

    apps/api/src/workspace/workspace.e2e.spec.ts

  • Added end-to-end tests for the new invitations endpoint.
  • Tested fetching invitations with and without acceptance filter.
  • +74/-1   

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    @muntaxir4 muntaxir4 requested a review from rajdip-b as a code owner December 9, 2024 21:32
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ✅

    404 - Fully compliant

    Compliant requirements:

    • Added endpoint /invitations under workspace.controller.ts
    • Added getInvitationsOfUser function with pagination in workspace.service.ts
    • Added e2e tests in workspace.e2e.spec.ts
    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Query Performance
    The query to fetch invitations performs two separate database calls - one for items and one for count. Consider using Prisma's count in a single query for better performance.

    Input Validation
    The endpoint accepts query parameters without validation for page, limit, sort and order. Consider adding validation to ensure these values are within acceptable ranges.

    Copy link
    Contributor

    codiumai-pr-agent-free bot commented Dec 9, 2024

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Security
    Add input validation for query parameters to prevent invalid values and potential security issues

    Add input validation for query parameters. The 'page' and 'limit' should be positive
    integers, 'sort' should be restricted to valid field names, and 'order' should only
    accept 'asc' or 'desc'.

    apps/api/src/workspace/controller/workspace.controller.ts [51-54]

    -@Query('page') page: number = 0,
    -@Query('limit') limit: number = 10,
    -@Query('sort') sort: string = 'name',
    -@Query('order') order: string = 'asc',
    +@Query('page', new ParseIntPipe({ min: 0 })) page: number = 0,
    +@Query('limit', new ParseIntPipe({ min: 1 })) limit: number = 10,
    +@Query('sort', new EnumPipe(['name', 'slug'])) sort: string = 'name',
    +@Query('order', new EnumPipe(['asc', 'desc'])) order: string = 'asc',
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: The suggestion addresses a significant security concern by adding proper input validation for query parameters, preventing potential injection attacks and ensuring data integrity through type checking and value constraints.

    8
    Possible issue
    ✅ Implement the search functionality in the database query to properly filter results based on the search parameter

    The search parameter is collected but not used in the query filter, which could lead
    to unexpected results when users try to search for workspaces.

    apps/api/src/workspace/service/workspace.service.ts [417-429]

     where: {
       userId: user.id,
       invitationAccepted: isAccepted,
    +  workspace: {
    +    name: { contains: search, mode: 'insensitive' }
    +  },
       roles: {
         none: {
           role: {
             authorities: {
               has: Authority.WORKSPACE_ADMIN
             }
           }
         }
       }
     },

    [Suggestion has been applied]

    Suggestion importance[1-10]: 7

    Why: The suggestion fixes a functional bug where the search parameter is collected but not utilized in the query, which would lead to incorrect filtering behavior in the API endpoint.

    7

    💡 Need additional feedback ? start a PR chat

    @muntaxir4
    Copy link
    Contributor Author

    muntaxir4 commented Dec 9, 2024

    Something fishy with the api e2e tests. I pushed the changes only after testing it locally.

    @muntaxir4
    Copy link
    Contributor Author

    Related to #550

    @muntaxir4
    Copy link
    Contributor Author

    Any idea how to get the invitationDate? The WorkSpaceMember Table has no Date property

    @rajdip-b
    Copy link
    Member

    Any idea how to get the invitationDate? The WorkSpaceMember Table has no Date property

    You can add an invitedOn field to it i suppose.

    @muntaxir4
    Copy link
    Contributor Author

    You can add an invitedOn field to it i suppose.

    I added createdOn in the WorkspaceMember model, as I felt admin entries should not have invitedOn.

    @rajdip-b
    Copy link
    Member

    @muntaxir4 can you please add the bruno docs aswell?

    @rajdip-b rajdip-b merged commit d45417a into keyshade-xyz:develop Dec 13, 2024
    8 checks passed
    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.

    DOCS: Add "Get All Workspace Invitations" request to Bruno API: Get all memberships of user
    2 participants