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): Create controller for Event module #399

Merged
merged 5 commits into from
Aug 3, 2024

Conversation

vr-varad
Copy link
Contributor

@vr-varad vr-varad commented Jul 28, 2024

User description

Description

controller for Event module

Fixes #356

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

  • Created EventController class with getEvents method to fetch events based on request parameters.
  • Defined GetEventsRequest and GetEventsResponse interfaces for type safety.
  • Added comprehensive tests for EventController covering various event sources such as project, environment, secret, and variable.

Changes walkthrough 📝

Relevant files
Enhancement
event.ts
Add EventController with getEvents method                               

packages/api-client/src/controllers/event/event.ts

  • Created EventController class.
  • Added getEvents method to fetch events based on request parameters.
  • Constructed URL dynamically based on request parameters.
  • +19/-0   
    event.types.d.ts
    Define types for Event module requests and responses         

    packages/api-client/src/types/event.types.d.ts

  • Defined GetEventsRequest interface.
  • Defined GetEventsResponse interface.
  • +11/-0   
    Tests
    event.spec.ts
    Add tests for EventController methods                                       

    packages/api-client/tests/event.spec.ts

  • Added tests for EventController.
  • Included tests for fetching project, environment, secret, and variable
    events.
  • Set up and tore down test data for workspace, project, environment,
    secret, and variable.
  • +131/-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

    URL Construction
    The URL construction in getEvents method might lead to malformed URLs due to missing '&' before 'page', 'limit', 'sort', 'order', and 'search' parameters. This can be fixed by ensuring each parameter addition starts with '&'.

    Error Handling
    The getEvents method does not handle any errors that might occur during the API call. It is recommended to add error handling to improve the robustness of the method.

    Copy link
    Contributor

    codiumai-pr-agent-free bot commented Jul 28, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible bug
    Improve URL query parameter concatenation to avoid malformed URLs

    Ensure that the URL query parameters are correctly concatenated with an '&' only
    when necessary to avoid malformed URLs.

    packages/api-client/src/controllers/event/event.ts [12-16]

    -request.page && (url += `page=${request.page}&`)
    -request.limit && (url += `limit=${request.limit}&`)
    -request.sort && (url += `sort=${request.sort}&`)
    -request.order && (url += `order=${request.order}&`)
    -request.search && (url += `search=${request.search}&`)
    +const queryParams = [
    +  request.page ? `page=${request.page}` : '',
    +  request.limit ? `limit=${request.limit}` : '',
    +  request.sort ? `sort=${request.sort}` : '',
    +  request.order ? `order=${request.order}` : '',
    +  request.search ? `search=${request.search}` : ''
    +].filter(param => param).join('&');
    +url += queryParams ? `&${queryParams}` : '';
     
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: This suggestion addresses a potential bug where the URL could be malformed due to incorrect concatenation of query parameters. The improved code ensures that parameters are only added when necessary, enhancing robustness.

    9
    Possible issue
    Prevent potential request errors by avoiding a trailing '&' when no parameters are added

    Consider handling the case where no query parameters are added, which currently
    results in a trailing '&' that could lead to request errors.

    packages/api-client/src/controllers/event/event.ts [12-16]

    -request.page && (url += `page=${request.page}&`)
    -request.limit && (url += `limit=${request.limit}&`)
    -request.sort && (url += `sort=${request.sort}&`)
    -request.order && (url += `order=${request.order}&`)
    -request.search && (url += `search=${request.search}&`)
    +const queryParams = [
    +  request.page ? `page=${request.page}` : '',
    +  request.limit ? `limit=${request.limit}` : '',
    +  request.sort ? `sort=${request.sort}` : '',
    +  request.order ? `order=${request.order}` : '',
    +  request.search ? `search=${request.search}` : ''
    +].filter(param => param).join('&');
    +url += queryParams ? `&${queryParams}` : '';
     
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: This suggestion effectively prevents potential request errors by ensuring no trailing '&' is added when no query parameters are present, addressing a significant issue.

    9
    Maintainability
    Refactor URL construction to a functional style for better readability and maintainability

    Refactor the method to use a more functional style for constructing the URL, which
    improves readability and maintainability.

    packages/api-client/src/controllers/event/event.ts [12-16]

    -request.page && (url += `page=${request.page}&`)
    -request.limit && (url += `limit=${request.limit}&`)
    -request.sort && (url += `sort=${request.sort}&`)
    -request.order && (url += `order=${request.order}&`)
    -request.search && (url += `search=${request.search}&`)
    +const params = { page: request.page, limit: request.limit, sort: request.sort, order: request.order, search: request.search };
    +const queryString = Object.entries(params)
    +  .filter(([_, value]) => value !== undefined)
    +  .map(([key, value]) => `${key}=${value}`)
    +  .join('&');
    +url += queryString ? `&${queryString}` : '';
     
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: This suggestion enhances code readability and maintainability by using a more functional approach to URL construction, making the code easier to understand and maintain.

    8
    Enhancement
    Use optional chaining for cleaner and more efficient URL parameter handling

    Use TypeScript's optional chaining to simplify the conditional checks and
    concatenation of URL parameters.

    packages/api-client/src/controllers/event/event.ts [12-16]

    -request.page && (url += `page=${request.page}&`)
    -request.limit && (url += `limit=${request.limit}&`)
    -request.sort && (url += `sort=${request.sort}&`)
    -request.order && (url += `order=${request.order}&`)
    -request.search && (url += `search=${request.search}&`)
    +url += [
    +  request.page && `page=${request.page}`,
    +  request.limit && `limit=${request.limit}`,
    +  request.sort && `sort=${request.sort}`,
    +  request.order && `order=${request.order}`,
    +  request.search && `search=${request.search}`
    +].filter(Boolean).join('&') + '&';
     
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: This suggestion improves code readability and efficiency by using optional chaining, but it does not address the potential issue of trailing '&' in the URL.

    7

    packages/api-client/src/types/event.types.d.ts Outdated Show resolved Hide resolved
    packages/api-client/src/controllers/event/event.ts Outdated Show resolved Hide resolved
    @vr-varad vr-varad requested a review from rajdip-b August 3, 2024 05:22
    Copy link
    Member

    @rajdip-b rajdip-b left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    LGTM

    @rajdip-b rajdip-b merged commit 6bb71a6 into keyshade-xyz:develop Aug 3, 2024
    4 checks passed
    rajdip-b pushed a commit that referenced this pull request Sep 5, 2024
    Co-authored-by: vr-varad <varadgupta21#gmail.com>
    rajdip-b pushed a commit that referenced this pull request Sep 5, 2024
    ## [2.4.0](v2.3.0...v2.4.0) (2024-09-05)
    
    ### 🚀 Features
    
    * **api-client:** Create controller for Event module ([#399](#399)) ([122df35](122df35))
    * **api-client:** Create controller for Integration module ([#397](#397)) ([697d38b](697d38b))
    * **api-client:** Create controller for Project module ([#370](#370)) ([fa25866](fa25866))
    * **api-client:** Create controller for Secret module ([#396](#396)) ([7e929c0](7e929c0))
    * **api-client:** Create controller for Variable module ([#395](#395)) ([3e114d9](3e114d9))
    * **api:** Add global search in workspace ([c49962b](c49962b))
    * **api:** Add max page size ([#377](#377)) ([ed18eb0](ed18eb0))
    * **cli:** Add functionality to operate on Environments ([#324](#324)) ([4c6f3f8](4c6f3f8))
    * **cli:** Quit on decryption failure ([#381](#381)) ([1349d15](1349d15))
    
    ### 🐛 Bug Fixes
    
    * **api-client:** Fixed broken export ([096df2c](096df2c))
    * **api:** Add NotFound exception on passing an invalid roleId while inviting user in workspace ([#408](#408)) ([ab441db](ab441db))
    * **cli:** Fixed missing module ([f7a091f](f7a091f))
    * **platform:**  Build failure in platform ([#385](#385)) ([90dcb2c](90dcb2c))
    
    ### 🔧 Miscellaneous Chores
    
    * Add api client build script and updated CI ([da0e27a](da0e27a))
    * **api:** Reorganized import using path alias ([d5befd1](d5befd1))
    * **ci:** Update CLI CI name ([8f4c456](8f4c456))
    * **cli:** Add Zod validation to parseInput function ([#362](#362)) ([34e6c39](34e6c39))
    * Fixed api client tests and rearranged controllers ([1307604](1307604))
    * Housekeeping ([c5f1330](c5f1330))
    * **platform:** Added strict null check ([072254f](072254f))
    * **web:** Added strict null check ([7e12b47](7e12b47))
    
    ### 🔨 Code Refactoring
    
    * **api:** Update logic for forking projects ([#398](#398)) ([4cf3838](4cf3838))
    Kiranchaudhary537 pushed a commit to Kiranchaudhary537/keyshade that referenced this pull request Oct 13, 2024
    ## [2.4.0](keyshade-xyz/keyshade@v2.3.0...v2.4.0) (2024-09-05)
    
    ### 🚀 Features
    
    * **api-client:** Create controller for Event module ([keyshade-xyz#399](keyshade-xyz#399)) ([122df35](keyshade-xyz@122df35))
    * **api-client:** Create controller for Integration module ([keyshade-xyz#397](keyshade-xyz#397)) ([697d38b](keyshade-xyz@697d38b))
    * **api-client:** Create controller for Project module ([keyshade-xyz#370](keyshade-xyz#370)) ([fa25866](keyshade-xyz@fa25866))
    * **api-client:** Create controller for Secret module ([keyshade-xyz#396](keyshade-xyz#396)) ([7e929c0](keyshade-xyz@7e929c0))
    * **api-client:** Create controller for Variable module ([keyshade-xyz#395](keyshade-xyz#395)) ([3e114d9](keyshade-xyz@3e114d9))
    * **api:** Add global search in workspace ([c49962b](keyshade-xyz@c49962b))
    * **api:** Add max page size ([keyshade-xyz#377](keyshade-xyz#377)) ([ed18eb0](keyshade-xyz@ed18eb0))
    * **cli:** Add functionality to operate on Environments ([keyshade-xyz#324](keyshade-xyz#324)) ([4c6f3f8](keyshade-xyz@4c6f3f8))
    * **cli:** Quit on decryption failure ([keyshade-xyz#381](keyshade-xyz#381)) ([1349d15](keyshade-xyz@1349d15))
    
    ### 🐛 Bug Fixes
    
    * **api-client:** Fixed broken export ([096df2c](keyshade-xyz@096df2c))
    * **api:** Add NotFound exception on passing an invalid roleId while inviting user in workspace ([keyshade-xyz#408](keyshade-xyz#408)) ([ab441db](keyshade-xyz@ab441db))
    * **cli:** Fixed missing module ([f7a091f](keyshade-xyz@f7a091f))
    * **platform:**  Build failure in platform ([keyshade-xyz#385](keyshade-xyz#385)) ([90dcb2c](keyshade-xyz@90dcb2c))
    
    ### 🔧 Miscellaneous Chores
    
    * Add api client build script and updated CI ([da0e27a](keyshade-xyz@da0e27a))
    * **api:** Reorganized import using path alias ([d5befd1](keyshade-xyz@d5befd1))
    * **ci:** Update CLI CI name ([8f4c456](keyshade-xyz@8f4c456))
    * **cli:** Add Zod validation to parseInput function ([keyshade-xyz#362](keyshade-xyz#362)) ([34e6c39](keyshade-xyz@34e6c39))
    * Fixed api client tests and rearranged controllers ([1307604](keyshade-xyz@1307604))
    * Housekeeping ([c5f1330](keyshade-xyz@c5f1330))
    * **platform:** Added strict null check ([072254f](keyshade-xyz@072254f))
    * **web:** Added strict null check ([7e12b47](keyshade-xyz@7e12b47))
    
    ### 🔨 Code Refactoring
    
    * **api:** Update logic for forking projects ([keyshade-xyz#398](keyshade-xyz#398)) ([4cf3838](keyshade-xyz@4cf3838))
    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 Event module
    2 participants