search

keyshade-xyz / keyshade

Realtime secret and configuration management tool, with the best in class security and seamless integration support
https://keyshade.xyz
Mozilla Public License 2.0
196 stars 96 forks source link

feat(api-client): Create controller for Event module #399

Closed vr-varad closed 1 month ago

vr-varad commented 2 months ago

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

If changes are made in the code:

Documentation Update

PR Type

Enhancement, Tests

Description

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

    codiumai-pr-agent-free[bot] commented 2 months ago

    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.
    codiumai-pr-agent-free[bot] commented 2 months ago

    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]](https://github.com/keyshade-xyz/keyshade/pull/399/files#diff-9a910f3756ca36d97ab0aef8c4b69229c7068aa913bc8db13bed7fa269609c8bR12-R16) ```diff -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]](https://github.com/keyshade-xyz/keyshade/pull/399/files#diff-9a910f3756ca36d97ab0aef8c4b69229c7068aa913bc8db13bed7fa269609c8bR12-R16) ```diff -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]](https://github.com/keyshade-xyz/keyshade/pull/399/files#diff-9a910f3756ca36d97ab0aef8c4b69229c7068aa913bc8db13bed7fa269609c8bR12-R16) ```diff -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]](https://github.com/keyshade-xyz/keyshade/pull/399/files#diff-9a910f3756ca36d97ab0aef8c4b69229c7068aa913bc8db13bed7fa269609c8bR12-R16) ```diff -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