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): Create endpoint for fetching all revisions of a variable #304

Merged
merged 4 commits into from
Jul 29, 2024

Conversation

yogesh1801
Copy link
Contributor

@yogesh1801 yogesh1801 commented Jun 29, 2024

User description

Description

Adds a getRevisionsOfVariable method to get revisions of a variable

Fixes #271

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:

  • 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 endpoint getRevisionsOfVariable to fetch revisions of a variable with pagination support.
  • Implemented the corresponding method in the service layer with necessary authority checks.
  • Added end-to-end tests for the new endpoint covering multiple scenarios including no revisions, non-existent variable/environment, and access control.

Changes walkthrough 📝

Relevant files
Enhancement
variable.controller.ts
Add endpoint to fetch variable revisions with pagination 

apps/api/src/variable/controller/variable.controller.ts

  • Added getRevisionsOfVariable endpoint to fetch variable revisions.
  • Implemented pagination for the new endpoint.
  • +18/-0   
    variable.service.ts
    Implement method to retrieve variable revisions with authority checks

    apps/api/src/variable/service/variable.service.ts

  • Added getRevisionsOfVariable method to retrieve variable revisions.
  • Included authority checks for variable and environment access.
  • +33/-0   
    Tests
    variable.e2e.spec.ts
    Add tests for getRevisionsOfVariable endpoint                       

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

  • Added tests for getRevisionsOfVariable endpoint.
  • Included tests for various scenarios like no revisions, non-existent
    variable/environment, and access control.
  • +105/-1 

    💡 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 [1-5] 3
    🧪 Relevant tests Yes
    🔒 Security concerns No
    ⚡ Key issues to review None

    Copy link
    Contributor

    codiumai-pr-agent-free bot commented Jun 29, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Enhancement
    Add validation for pagination parameters to ensure they are non-negative and within a reasonable range

    Consider validating the page and limit query parameters to ensure they are non-negative
    integers. This prevents potential issues with pagination logic where negative values could
    lead to unexpected behavior or errors.

    apps/api/src/variable/controller/variable.controller.ts [109-110]

    -@Query('page') page: number = 0,
    -@Query('limit') limit: number = 10
    +@Query('page', new ParseIntPipe({ min: 0 })) page: number = 0,
    +@Query('limit', new ParseIntPipe({ min: 1, max: 100 })) limit: number = 10
     
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: This suggestion improves the robustness of the API by ensuring that pagination parameters are within a valid range, preventing potential issues with negative values or excessively large limits.

    9
    Error handling
    Handle the case where no revisions are found by throwing a NotFoundException

    Implement error handling for the scenario where no revisions are found for the given
    variable and environment IDs. This could involve returning a specific message or an empty
    array, depending on the desired API behavior.

    apps/api/src/variable/service/variable.service.ts [628-635]

     const revisions = await this.prisma.variableVersion.findMany({
       where: {
         variableId: variableId,
         environmentId: environmentId
       },
       skip: page * limit,
       take: limit
     })
    +if (revisions.length === 0) {
    +  throw new NotFoundException(`No revisions found for variable ID ${variableId} in environment ID ${environmentId}.`);
    +}
     
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Implementing error handling for cases where no revisions are found improves the API's clarity and user experience by providing specific feedback when no data is available.

    8
    Performance
    Suggest adding a database index to improve query performance for fetching variable revisions

    To optimize database queries, consider adding an index on the variableId and environmentId
    columns in the variableVersion table if not already present. This can significantly
    improve the performance of the query fetching variable revisions.

    apps/api/src/variable/service/variable.service.ts [628-635]

    +// Ensure your database has an index on `variableId` and `environmentId` for the `variableVersion` table.
     const revisions = await this.prisma.variableVersion.findMany({
       where: {
         variableId: variableId,
         environmentId: environmentId
       },
       skip: page * limit,
       take: limit
     })
     
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Adding an index can significantly improve query performance, especially for large datasets. However, this suggestion is more of a general optimization tip and may not be directly applicable to the PR code diff.

    7
    Maintainability
    Enhance method signature by adding explicit return type annotations

    Add explicit type annotations for the parameters in the getRevisionsOfVariable method to
    enhance code readability and maintainability.

    apps/api/src/variable/controller/variable.controller.ts [105-110]

     async getRevisionsOfVariable(
       @CurrentUser() user: User,
       @Param('variableId') variableId: string,
       @Param('environmentId') environmentId: string,
       @Query('page') page: number = 0,
       @Query('limit') limit: number = 10
    -)
    +): Promise<VariableRevision[]>
     
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    Why: Adding explicit return type annotations improves code readability and maintainability, but it is a minor enhancement compared to functional improvements or bug fixes.

    6

    @rajdip-b rajdip-b added the foss hack Clustering all the curated issues for Foss Hack 2024 label Jun 30, 2024
    @yogesh1801 yogesh1801 requested a review from rajdip-b July 1, 2024 10:16
    @rajdip-b rajdip-b changed the title feat(api): Added getRevisionsOfVariable method feat(api): Create endpoint for fetching all revisions of a variable Jul 29, 2024
    @rajdip-b rajdip-b merged commit 13ebc0d into keyshade-xyz:develop Jul 29, 2024
    4 checks passed
    rajdip-b pushed a commit that referenced this pull request Jul 29, 2024
    ## [2.3.0](v2.2.0...v2.3.0) (2024-07-29)
    
    ### 🚀 Features
    
    * **api:** Add pagination metadata to Environment module ([#382](#382)) ([9baa344](9baa344))
    * **api:** Add pagination metadata to Event module ([#394](#394)) ([60010b4](60010b4))
    * **api:** Add pagination metadata to Integration module ([#391](#391)) ([0372e36](0372e36))
    * **api:** Add pagination metadata to Project module ([#393](#393)) ([bc274fd](bc274fd))
    * **api:** Add pagination metadata to Secret module ([#389](#389)) ([c4cc667](c4cc667))
    * **api:** Add pagination metadata to Variable module ([#390](#390)) ([be6aabf](be6aabf))
    * **api:** Add pagination metadata to Workspace module  ([#387](#387)) ([a08c924](a08c924))
    * **api:** Add pagination metadata to Workspace Role module ([#388](#388)) ([d8e8f49](d8e8f49))
    * **api:** Create a paginate method ([#379](#379)) ([09576f1](09576f1))
    * **api:** Create endpoint for fetching all revisions of a [secure] ([#303](#303)) ([de2b602](de2b602))
    * **api:** Create endpoint for fetching all revisions of a variable ([#304](#304)) ([9abddc1](9abddc1))
    * **cli:** Improved the DX for list profile ([#334](#334)) ([6bff496](6bff496))
    * **platform:** Add warning sonner toast for invalid otp ([#335](#335)) ([21513f5](21513f5))
    
    ### 🐛 Bug Fixes
    
    * **cli:** Added parent directory check ([#359](#359)) ([538ea7f](538ea7f))
    * **platform:** Platform types fixes ([#374](#374)) ([8e9d9ff](8e9d9ff))
    
    ### 📚 Documentation
    
    * Added docker details in setting-things-up.md ([#358](#358)) ([ed5093a](ed5093a))
    * Update postman workspace link ([d6aba27](d6aba27))
    * Updated env and cli docs ([1213d2a](1213d2a))
    
    ### 🔧 Miscellaneous Chores
    
    * Added next backend url in .env.example ([5695254](5695254))
    * **api-client:** Added pagination structure ([a70e957](a70e957))
    * **api-client:** Fixed test script ([ad70819](ad70819))
    * **api-client:** Removed try-catch from tests in environment ([a64e48c](a64e48c))
    * **api:** Add user cache for optimization ([#386](#386)) ([8d730b5](8d730b5))
    * **api:** Alter cache rehydration interval ([f5f9eec](f5f9eec))
    * **api:** Fixed naming error in variable controller ([0c5a380](0c5a380))
    * **api:** Improve handling of edge cases for paginate module ([#402](#402)) ([8591487](8591487))
    * **api:** Minor updates to user service ([249d778](249d778))
    * **api:** Skip workspace creation when user is admin ([#376](#376)) ([13f6c59](13f6c59))
    * **ci:** Add docker check   ([#383](#383)) ([3119001](3119001))
    * **ci:** Add names to CI files ([1a7e5f6](1a7e5f6))
    * **ci:** Add validate CLI pipeline ([#373](#373)) ([a91df6c](a91df6c))
    * **ci:** Adding validate pipeline ([#372](#372)) ([23cf3b3](23cf3b3))
    * **ci:** Disabled platform and api deployments ([74d601a](74d601a))
    * **ci:** Fixed deployment scripts ([12e35db](12e35db))
    * **ci:** Fixed platform script ([d783f2a](d783f2a))
    * **CI:** Include migration deployment in API deploy pipeline ([dbd5222](dbd5222))
    * **CI:** Separated deployment and docker build jobs ([090e193](090e193))
    * **CI:** Setup inter-job dependency ([1756727](1756727))
    * **ci:** Update auto-assign.yaml ([#375](#375)) ([91e0ec1](91e0ec1))
    * **cli:** Changed objects to classes ([#306](#306)) ([c83f2db](c83f2db))
    * Removed Minio config ([8feb83a](8feb83a))
    * Updated deployment scripts and added health check in platform ([fcc1c3f](fcc1c3f))
    
    ### 🔨 Code Refactoring
    
    * **api:** Updated path of some endpoints in project controller ([9502678](9502678))
    * **api:** Updated Redis provider ([33491a1](33491a1))
    rajdip-b pushed a commit that referenced this pull request Jul 29, 2024
    ## [2.3.0](v2.2.0...v2.3.0) (2024-07-29)
    
    ### 🚀 Features
    
    * **api:** Add pagination metadata to Environment module ([#382](#382)) ([9baa344](9baa344))
    * **api:** Add pagination metadata to Event module ([#394](#394)) ([60010b4](60010b4))
    * **api:** Add pagination metadata to Integration module ([#391](#391)) ([0372e36](0372e36))
    * **api:** Add pagination metadata to Project module ([#393](#393)) ([bc274fd](bc274fd))
    * **api:** Add pagination metadata to Secret module ([#389](#389)) ([c4cc667](c4cc667))
    * **api:** Add pagination metadata to Variable module ([#390](#390)) ([be6aabf](be6aabf))
    * **api:** Add pagination metadata to Workspace module  ([#387](#387)) ([a08c924](a08c924))
    * **api:** Add pagination metadata to Workspace Role module ([#388](#388)) ([d8e8f49](d8e8f49))
    * **api:** Create a paginate method ([#379](#379)) ([09576f1](09576f1))
    * **api:** Create endpoint for fetching all revisions of a [secure] ([#303](#303)) ([de2b602](de2b602))
    * **api:** Create endpoint for fetching all revisions of a variable ([#304](#304)) ([9abddc1](9abddc1))
    * **cli:** Improved the DX for list profile ([#334](#334)) ([6bff496](6bff496))
    * **platform:** Add warning sonner toast for invalid otp ([#335](#335)) ([21513f5](21513f5))
    
    ### 🐛 Bug Fixes
    
    * **cli:** Added parent directory check ([#359](#359)) ([538ea7f](538ea7f))
    * **platform:** Platform types fixes ([#374](#374)) ([8e9d9ff](8e9d9ff))
    
    ### 📚 Documentation
    
    * Added docker details in setting-things-up.md ([#358](#358)) ([ed5093a](ed5093a))
    * Update postman workspace link ([d6aba27](d6aba27))
    * Updated env and cli docs ([1213d2a](1213d2a))
    
    ### 🔧 Miscellaneous Chores
    
    * Added next backend url in .env.example ([5695254](5695254))
    * **api-client:** Added pagination structure ([a70e957](a70e957))
    * **api-client:** Fixed test script ([ad70819](ad70819))
    * **api-client:** Removed try-catch from tests in environment ([a64e48c](a64e48c))
    * **api:** Add user cache for optimization ([#386](#386)) ([8d730b5](8d730b5))
    * **api:** Alter cache rehydration interval ([f5f9eec](f5f9eec))
    * **api:** Fixed naming error in variable controller ([0c5a380](0c5a380))
    * **api:** Improve handling of edge cases for paginate module ([#402](#402)) ([8591487](8591487))
    * **api:** Minor updates to user service ([249d778](249d778))
    * **api:** Skip workspace creation when user is admin ([#376](#376)) ([13f6c59](13f6c59))
    * **ci:** Add docker check   ([#383](#383)) ([3119001](3119001))
    * **ci:** Add names to CI files ([1a7e5f6](1a7e5f6))
    * **ci:** Add validate CLI pipeline ([#373](#373)) ([a91df6c](a91df6c))
    * **ci:** Adding validate pipeline ([#372](#372)) ([23cf3b3](23cf3b3))
    * **ci:** Disabled platform and api deployments ([74d601a](74d601a))
    * **ci:** Fixed deployment scripts ([12e35db](12e35db))
    * **ci:** Fixed platform script ([d783f2a](d783f2a))
    * **CI:** Include migration deployment in API deploy pipeline ([dbd5222](dbd5222))
    * **CI:** Separated deployment and docker build jobs ([090e193](090e193))
    * **CI:** Setup inter-job dependency ([1756727](1756727))
    * **ci:** Update auto-assign.yaml ([#375](#375)) ([91e0ec1](91e0ec1))
    * **cli:** Changed objects to classes ([#306](#306)) ([c83f2db](c83f2db))
    * Removed Minio config ([8feb83a](8feb83a))
    * Updated deployment scripts and added health check in platform ([fcc1c3f](fcc1c3f))
    
    ### 🔨 Code Refactoring
    
    * **api:** Updated path of some endpoints in project controller ([9502678](9502678))
    * **api:** Updated Redis provider ([33491a1](33491a1))
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    enhancement foss hack Clustering all the curated issues for Foss Hack 2024 Review effort [1-5]: 3 Tests
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    API: Create endpoint for fetching all revisions of a variable
    2 participants