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(schema, api-client): Migrate workspace-membership schemas and types to @keyshade/schema #569

Merged

Conversation

muntaxir4
Copy link
Contributor

@muntaxir4 muntaxir4 commented Nov 30, 2024

User description

Description

@keyshade/schema

  • Add schemas and types for workspace-membership.
  • Add tests for workspace-membership schemas.
  • Export workpsace-membership schemas from index and its types from index.types .

@keyshade/api-client

  • Remove existing workspace-membership.types.d.ts.
  • Change imports for workpsace-membership types to @keyshade/schema. And include userEmails for removeUsers.

api

  • In workspace-membership.controller, moved userEmails from request body to query params in order to follow HTTP's DELETE standards.

api-collection

  • Update docs for remove user from workspace.

Related to #519

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, documentation, dependencies


Description

  • Migrated workspace-membership schemas and types to @keyshade/schema.
  • Added comprehensive test cases for workspace-membership and UserSchema.
  • Enhanced DELETE request method to support optional request body in api-client.
  • Updated import paths in workspace-membership controller to use new schemas.
  • Documented request body for removing users from workspace.
  • Updated package dependencies and versions, including eslint-config-turbo.
  • Exported workspace membership schemas and types from the main index.

Changes walkthrough 📝

Relevant files
Tests
2 files
workspace-membership.spec.ts
Add tests for workspace membership schemas                             

packages/schema/tests/workspace-membership.spec.ts

  • Added comprehensive test cases for various workspace membership
    schemas.
  • Validated both valid and invalid scenarios for each schema.
  • Ensured coverage for all request and response schemas related to
    workspace membership.
  • +431/-0 
    user.spec.ts
    Enhance user schema tests with additional cases                   

    packages/schema/tests/user.spec.ts

  • Added tests for UserSchema validation.
  • Included scenarios for valid, invalid, and missing fields in
    UserSchema.
  • Updated existing tests to include new fields in user-related schemas.
  • +68/-2   
    Enhancement
    7 files
    index.ts
    Define workspace membership schemas using zod                       

    packages/schema/src/workspace-membership/index.ts

  • Introduced new schemas for workspace membership operations.
  • Defined request and response schemas for membership actions.
  • Utilized zod for schema validation.
  • +99/-0   
    index.types.ts
    Add TypeScript types for workspace membership schemas       

    packages/schema/src/workspace-membership/index.types.ts

  • Added TypeScript types inferred from workspace membership schemas.
  • Ensured type definitions align with the newly defined schemas.
  • +89/-0   
    index.ts
    Introduce UserSchema and refactor related schemas               

    packages/schema/src/user/index.ts

  • Introduced UserSchema for user-related data validation.
  • Refactored existing schemas to extend from UserSchema.
  • +7/-9     
    client.ts
    Enhance DELETE request method to support request body       

    packages/api-client/src/core/client.ts

  • Modified DELETE request method to accept optional body data.
  • Updated method signature to include data parameter.
  • +8/-1     
    workspace-membership.ts
    Update workspace membership controller with new schema imports

    packages/api-client/src/controllers/workspace-membership.ts

  • Updated import paths to use @keyshade/schema.
  • Modified remove users method to include request body data.
  • +3/-2     
    index.types.ts
    Export workspace membership types from index                         

    packages/schema/src/index.types.ts

    • Exported workspace membership types from the main index.
    +1/-0     
    index.ts
    Export workspace membership schemas from index                     

    packages/schema/src/index.ts

    • Exported workspace membership schemas from the main index.
    +1/-0     
    Configuration changes
    2 files
    setup.ts
    Add delay in test setup for database migrations                   

    packages/api-client/tests/config/setup.ts

  • Added a delay before deploying database migrations.
  • Ensured proper setup timing for test environment.
  • +1/-1     
    package.json
    Update scripts to use scoped package names                             

    package.json

    • Updated lint and build scripts to use scoped package names.
    +4/-4     
    Dependencies
    2 files
    package.json
    Update eslint-config-turbo version                                             

    packages/eslint-config-custom/package.json

    • Updated eslint-config-turbo to a newer version.
    +10/-10 
    pnpm-lock.yaml
    Update package dependencies and versions in pnpm-lock.yaml

    pnpm-lock.yaml

  • Removed @swc/helpers from various package dependencies.
  • Updated turbo package versions from 1.13.4 to 2.3.3.
  • Updated eslint-config-turbo package versions from 1.13.4 to 2.3.3.
  • Adjusted TypeScript versions in some dependencies.
  • +140/-278
    Documentation
    1 files
    Remove users from workspace.bru
    Document request body for removing users from workspace   

    api-collection/Workspace Membership Controller/Remove users from workspace.bru

  • Added request body example for removing users from workspace.
  • Documented the request body format and usage.
  • +14/-0   

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

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ✅

    519 - Fully compliant

    Compliant requirements:

    • Created workspace-membership folder with schema and types files
    • Added zod schemas and inferred types for workspace-membership
    • Updated api-client to use types from schema package
    • Added exports to index files
    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    API Change
    The removeUsers endpoint now accepts a request body. Verify this change is compatible with the API implementation and documented properly.

    Copy link
    Contributor

    codiumai-pr-agent-free bot commented Nov 30, 2024

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    General
    ✅ Move DELETE request payload from body to URL query parameters to follow HTTP standards

    The DELETE request to remove users should include the request body in the URL query
    parameters instead of the request body, as DELETE requests typically should not have
    a body according to HTTP standards.

    packages/api-client/src/controllers/workspace-membership.ts [63-67]

     const response = await this.apiClient.delete(
    -  `/api/workspace-membership/${request.workspaceSlug}/remove-users`,
    -  headers,
    -  request.userEmails
    +  `/api/workspace-membership/${request.workspaceSlug}/remove-users?emails=${request.userEmails.join(',')}`,
    +  headers
     )

    [Suggestion has been applied]

    Suggestion importance[1-10]: 9

    Why: This suggestion addresses a significant architectural issue by moving the DELETE request payload from the body to query parameters, which is the standard HTTP practice and improves compatibility with servers and proxies.

    9
    ✅ Remove request body support from DELETE method to comply with HTTP standards
    Suggestion Impact:The commit removed the request body support from the DELETE method, aligning with the suggestion to comply with HTTP standards.

    code diff:

    -   * @param data - Optional data to be sent in the request body.
        * @returns A Promise that resolves to the response data.
        */
       delete(
    @@ -86,7 +85,6 @@
             'Content-Type': 'application/json',
             ...headers
           },
    -      body: JSON.stringify(data),

    The DELETE method should not accept a request body parameter as it violates HTTP
    standards and may cause compatibility issues with some servers and proxies.

    packages/api-client/src/core/client.ts [78-92]

     delete(
       url: string,
    -  headers?: Record<string, string>,
    -  data?: any
    +  headers?: Record<string, string>
     ): Promise<Response> {
       return this.request(url, {
         method: 'DELETE',
         headers: {
           'Content-Type': 'application/json',
           ...headers
         },
    -    body: JSON.stringify(data),
         credentials: 'include'
       })
     }
    Suggestion importance[1-10]: 9

    Why: The suggestion fixes a fundamental HTTP compliance issue by removing body support from DELETE requests, which could prevent compatibility issues with various servers and middleware.

    9

    💡 Need additional feedback ? start a PR chat

    @muntaxir4 muntaxir4 requested a review from rajdip-b December 2, 2024 19:51
    Copy link

    codecov bot commented Dec 3, 2024

    Codecov Report

    All modified and coverable lines are covered by tests ✅

    Project coverage is 87.53%. Comparing base (ce50743) to head (04b4a6d).
    Report is 233 commits behind head on develop.

    Additional details and impacted files
    @@             Coverage Diff             @@
    ##           develop     #569      +/-   ##
    ===========================================
    - Coverage    91.71%   87.53%   -4.18%     
    ===========================================
      Files          111      106       -5     
      Lines         2510     2776     +266     
      Branches       469      416      -53     
    ===========================================
    + Hits          2302     2430     +128     
    - Misses         208      346     +138     
    Flag Coverage Δ
    api-e2e-tests 87.53% <100.00%> (-4.18%) ⬇️

    Flags with carried forward coverage won't be shown. Click here to find out more.

    ☔ View full report in Codecov by Sentry.
    📢 Have feedback on the report? Share it here.

    @rajdip-b rajdip-b merged commit 4398969 into keyshade-xyz:develop Dec 3, 2024
    12 checks passed
    @muntaxir4 muntaxir4 deleted the workspace-membership-schemas branch December 3, 2024 11:16
    muntaxir4 added a commit to muntaxir4/keyshade that referenced this pull request Dec 3, 2024
    rajdip-b pushed a commit that referenced this pull request Dec 3, 2024
    ## [2.8.0](v2.7.0...v2.8.0) (2024-12-03)
    
    ### 🚀 Features
    
    * **api:** Add workspace removal notification email template ([#476](#476)) ([40b754f](40b754f))
    * **cli:** Store `metrics_enabled` key in profile config ([#536](#536)) ([9283b22](9283b22))
    * **package, api, cli:** Add api-key schemas and types; Fix schema inconsistencies; Minor fix for CLI build errors  ([#557](#557)) ([126d024](126d024))
    * **platform:** Added screen for CREATE NEW PROJECT ([#540](#540)) ([b644633](b644633))
    * **platform:** Updated the empty state of dashboard ([#522](#522)) ([28739d9](28739d9))
    * **schema, api-client:** Migrate auth types to @keyshade/schema ([#532](#532)) ([d880098](d880098))
    * **schema, api-client:** Migrate event schemas and types to @keyshade/schema ([#546](#546)) ([a3267de](a3267de))
    * **schema, api-client:** Migrate integration schemas and types to @keyshade/schema ([#547](#547)) ([08868c3](08868c3))
    * **schema, api-client:** Migrate project schemas and environment schemas along with their types to @keyshade/schema ([#538](#538)) ([c468af0](c468af0))
    * **schema, api-client:** Migrate [secure] types and schemas to @keyshade/schema ([#539](#539)) ([bc3100b](bc3100b))
    * **schema, api-client:** Migrate user types and schemas to @keyshade/schema ([#535](#535)) ([c24695e](c24695e))
    * **schema, api-client:** Migrate variable schemas and types to @keyshade/schema ([#545](#545)) ([0ee8f9a](0ee8f9a))
    * **schema, api-client:** Migrate workspace-membership schemas and types to @keyshade/schema ([#569](#569)) ([4398969](4398969))
    * **schema, api-client:** Migrate workspace-role schemas and types to @keyshade/schema ([#568](#568)) ([9efbf2d](9efbf2d))
    * **schema:** Add User type inference from UserSchema ([#574](#574)) ([84c1db5](84c1db5))
    
    ### 🐛 Bug Fixes
    
    * **api:** Incorrect oauth redirect url ([58d96e5](58d96e5))
    * **platform:** Resolve loading SVG blocking input field interaction ([#571](#571)) ([30f4f65](30f4f65))
    
    ### 📚 Documentation
    
    * Add pictures to Bruno setup ([#541](#541)) ([210c0fd](210c0fd))
    * Migrate to Bruno ([#525](#525)) ([1793d92](1793d92))
    
    ### 🔧 Miscellaneous Chores
    
    * **ci:** Add script to validate schema package ([59e4280](59e4280))
    * Fixed codecov client version ([a998ae4](a998ae4))
    * **package:** Fixed tests and did housekeeping ([#544](#544)) ([40008e3](40008e3))
    * Update test coverage settings ([5b27e32](5b27e32))
    * Update Turbo to 2.3.1 ([#564](#564)) ([3a63823](3a63823))
    * **web:** Update dockerfile ([10d9cc5](10d9cc5))
    
    ### 🔨 Code Refactoring
    
    * **api-client, schema:** Add workspace's schemas and types in @keyshade/schema ([#520](#520)) ([7c8ee5d](7c8ee5d))
    muntaxir4 added a commit to muntaxir4/keyshade that referenced this pull request Jan 1, 2025
    muntaxir4 pushed a commit to muntaxir4/keyshade that referenced this pull request Jan 1, 2025
    ## [2.8.0](keyshade-xyz/keyshade@v2.7.0...v2.8.0) (2024-12-03)
    
    ### 🚀 Features
    
    * **api:** Add workspace removal notification email template ([keyshade-xyz#476](keyshade-xyz#476)) ([40b754f](keyshade-xyz@40b754f))
    * **cli:** Store `metrics_enabled` key in profile config ([keyshade-xyz#536](keyshade-xyz#536)) ([9283b22](keyshade-xyz@9283b22))
    * **package, api, cli:** Add api-key schemas and types; Fix schema inconsistencies; Minor fix for CLI build errors  ([keyshade-xyz#557](keyshade-xyz#557)) ([126d024](keyshade-xyz@126d024))
    * **platform:** Added screen for CREATE NEW PROJECT ([keyshade-xyz#540](keyshade-xyz#540)) ([b644633](keyshade-xyz@b644633))
    * **platform:** Updated the empty state of dashboard ([keyshade-xyz#522](keyshade-xyz#522)) ([28739d9](keyshade-xyz@28739d9))
    * **schema, api-client:** Migrate auth types to @keyshade/schema ([keyshade-xyz#532](keyshade-xyz#532)) ([d880098](keyshade-xyz@d880098))
    * **schema, api-client:** Migrate event schemas and types to @keyshade/schema ([keyshade-xyz#546](keyshade-xyz#546)) ([a3267de](keyshade-xyz@a3267de))
    * **schema, api-client:** Migrate integration schemas and types to @keyshade/schema ([keyshade-xyz#547](keyshade-xyz#547)) ([08868c3](keyshade-xyz@08868c3))
    * **schema, api-client:** Migrate project schemas and environment schemas along with their types to @keyshade/schema ([keyshade-xyz#538](keyshade-xyz#538)) ([c468af0](keyshade-xyz@c468af0))
    * **schema, api-client:** Migrate [secure] types and schemas to @keyshade/schema ([keyshade-xyz#539](keyshade-xyz#539)) ([bc3100b](keyshade-xyz@bc3100b))
    * **schema, api-client:** Migrate user types and schemas to @keyshade/schema ([keyshade-xyz#535](keyshade-xyz#535)) ([c24695e](keyshade-xyz@c24695e))
    * **schema, api-client:** Migrate variable schemas and types to @keyshade/schema ([keyshade-xyz#545](keyshade-xyz#545)) ([0ee8f9a](keyshade-xyz@0ee8f9a))
    * **schema, api-client:** Migrate workspace-membership schemas and types to @keyshade/schema ([keyshade-xyz#569](keyshade-xyz#569)) ([4398969](keyshade-xyz@4398969))
    * **schema, api-client:** Migrate workspace-role schemas and types to @keyshade/schema ([keyshade-xyz#568](keyshade-xyz#568)) ([9efbf2d](keyshade-xyz@9efbf2d))
    * **schema:** Add User type inference from UserSchema ([keyshade-xyz#574](keyshade-xyz#574)) ([84c1db5](keyshade-xyz@84c1db5))
    
    ### 🐛 Bug Fixes
    
    * **api:** Incorrect oauth redirect url ([58d96e5](keyshade-xyz@58d96e5))
    * **platform:** Resolve loading SVG blocking input field interaction ([keyshade-xyz#571](keyshade-xyz#571)) ([30f4f65](keyshade-xyz@30f4f65))
    
    ### 📚 Documentation
    
    * Add pictures to Bruno setup ([keyshade-xyz#541](keyshade-xyz#541)) ([210c0fd](keyshade-xyz@210c0fd))
    * Migrate to Bruno ([keyshade-xyz#525](keyshade-xyz#525)) ([1793d92](keyshade-xyz@1793d92))
    
    ### 🔧 Miscellaneous Chores
    
    * **ci:** Add script to validate schema package ([59e4280](keyshade-xyz@59e4280))
    * Fixed codecov client version ([a998ae4](keyshade-xyz@a998ae4))
    * **package:** Fixed tests and did housekeeping ([keyshade-xyz#544](keyshade-xyz#544)) ([40008e3](keyshade-xyz@40008e3))
    * Update test coverage settings ([5b27e32](keyshade-xyz@5b27e32))
    * Update Turbo to 2.3.1 ([keyshade-xyz#564](keyshade-xyz#564)) ([3a63823](keyshade-xyz@3a63823))
    * **web:** Update dockerfile ([10d9cc5](keyshade-xyz@10d9cc5))
    
    ### 🔨 Code Refactoring
    
    * **api-client, schema:** Add workspace's schemas and types in @keyshade/schema ([keyshade-xyz#520](keyshade-xyz#520)) ([7c8ee5d](keyshade-xyz@7c8ee5d))
    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.

    2 participants