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 Integration module #397

Closed vr-varad closed 1 month ago

vr-varad commented 2 months ago

User description

Description

Api-client Controller for Integration

Fixes #353

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

Description

Changes walkthrough 📝

Relevant files
Enhancement
integration.ts
Added IntegrationController with CRUD methods                       
packages/api-client/src/controllers/integration/integration.ts
  • Created IntegrationController class with static methods for CRUD
    operations.
  • Imported client from '../../client'.
    		•  +11/-0   
    integrayion.types.d.ts
    Added TypeScript interfaces for integration operations     
    packages/api-client/src/types/integrayion.types.d.ts
  • Defined interfaces for request and response types for integration
    operations.
    		•  +19/-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: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    Empty Methods
    The methods `createIntegration`, `updateIntegration`, `getIntegration`, `getAllIntegrations`, and `deleteIntegration` are defined but not implemented. This could be intentional for a stub implementation, but it's important to either implement these or provide a clear TODO comment if they are to be completed in the future.
    codiumai-pr-agent-free[bot] commented 2 months ago

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Enhancement
    ✅ Define method parameters and return types for better type safety ___
    Suggestion Impact:The commit added method parameters and return types to the methods in IntegrationController, enhancing type safety and clarifying the expected inputs and outputs. code diff: ```diff - static async createIntegration() {} - static async updateIntegration() {} - static async getIntegration() {} - static async getAllIntegrations() {} - static async deleteIntegration() {} + static async createIntegration( + request: CreateIntegrationRequest, + headers?: Record + ): Promise { + return this.apiClient.post( + `api/integration/${request.workspaceId}`, + request, + headers + ) + } + + static async updateIntegration( + request: UpdateIntegrationRequest, + headers?: Record + ): Promise { + return this.apiClient.put( + `/api/integration/${request.workspaceId}`, + request, + headers + ) + } + + static async getIntegration( + request: GetIntegrationRequest, + headers?: Record + ): Promise { + return this.apiClient.get( + `/api/integration/${request.integrationId}`, + headers + ) + } + + static async getAllIntegrations( + request: GetAllIntegrationRequest, + headers?: Record + ): Promise { + let url = `/api/integration/all/${request.workspaceId}` + 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}&`) + + return this.apiClient.get(url, headers) + } + + static async deleteIntegration( + request: DeleteIntegrationRequest, + headers?: Record + ): Promise { + return this.apiClient.delete( + `/api/integration/${request.integrationId}`, + headers + ) + } ```
    ___ **Consider adding method parameters and return types to the methods in
    IntegrationController to enhance type safety and clarify the expected inputs and
    outputs.** [packages/api-client/src/controllers/integration/integration.ts [6-10]](https://github.com/keyshade-xyz/keyshade/pull/397/files#diff-4c6a75c90cc3c944a2848c76539d9513e351da13006375ff5b3d11c9a6ea9b17R6-R10) ```diff -static async createIntegration() {} -static async updateIntegration() {} -static async getIntegration() {} -static async getAllIntegrations() {} -static async deleteIntegration() {} +static async createIntegration(data: CreateIntegrationRequest): Promise {} +static async updateIntegration(data: UpdateIntegrationRequest): Promise {} +static async getIntegration(id: string): Promise {} +static async getAllIntegrations(): Promise {} +static async deleteIntegration(id: string): Promise {} ``` - [ ] **Apply this suggestion**
    Suggestion importance[1-10]: 9 Why: Adding method parameters and return types enhances type safety and clarifies the expected inputs and outputs, significantly improving code robustness and readability.
    		9
    Possible bug
    ✅ Add initialization check for apiClient ___
    Suggestion Impact:The suggestion led to the initialization of the apiClient within the constructor of the IntegrationController class, ensuring it is properly set up before use. code diff: ```diff - private static apiClient = client + private apiClient: APIClient - static async createIntegration() {} - static async updateIntegration() {} - static async getIntegration() {} - static async getAllIntegrations() {} - static async deleteIntegration() {} + constructor(private readonly backendUrl: string) { + this.apiClient = new APIClient(this.backendUrl) ```
    ___ **Ensure that the apiClient is properly initialized before using it in the
    IntegrationController methods to avoid runtime errors.** [packages/api-client/src/controllers/integration/integration.ts [4]](https://github.com/keyshade-xyz/keyshade/pull/397/files#diff-4c6a75c90cc3c944a2848c76539d9513e351da13006375ff5b3d11c9a6ea9b17R4-R4) ```diff -private static apiClient = client +private static apiClient = client || throw new Error('ApiClient is not initialized'); ``` - [ ] **Apply this suggestion**
    Suggestion importance[1-10]: 8 Why: Ensuring `apiClient` is properly initialized before use can prevent potential runtime errors, which is crucial for the stability of the application.
    		8
    Maintainability
    Add implementation or placeholder comments to empty methods ___ **Consider implementing actual logic inside the methods of IntegrationController or at
    least adding a placeholder comment to indicate that implementation is pending. This
    will help other developers understand that these methods are not yet functional.** [packages/api-client/src/controllers/integration/integration.ts [6-10]](https://github.com/keyshade-xyz/keyshade/pull/397/files#diff-4c6a75c90cc3c944a2848c76539d9513e351da13006375ff5b3d11c9a6ea9b17R6-R10) ```diff -static async createIntegration() {} -static async updateIntegration() {} -static async getIntegration() {} -static async getAllIntegrations() {} -static async deleteIntegration() {} +static async createIntegration() { + // TODO: Implement this method +} +static async updateIntegration() { + // TODO: Implement this method +} +static async getIntegration() { + // TODO: Implement this method +} +static async getAllIntegrations() { + // TODO: Implement this method +} +static async deleteIntegration() { + // TODO: Implement this method +} ``` - [ ] **Apply this suggestion**
    Suggestion importance[1-10]: 7 Why: Adding placeholder comments helps other developers understand that the methods are not yet implemented, improving code maintainability and clarity.
    		7
    Best practice
    ✅ Convert static methods to instance methods for better flexibility ___
    Suggestion Impact:The static methods in the IntegrationController were converted to instance methods, aligning with the suggestion to use instance methods for better flexibility. code diff: ```diff export default class IntegrationController { - private static apiClient = client + private apiClient: APIClient - static async createIntegration() {} - static async updateIntegration() {} - static async getIntegration() {} - static async getAllIntegrations() {} - static async deleteIntegration() {} + constructor(private readonly backendUrl: string) { + this.apiClient = new APIClient(this.backendUrl) + } + + async createIntegration( + request: CreateIntegrationRequest, + headers?: Record + ): Promise> { + const response = await this.apiClient.post( + `/api/integration/${request.workspaceId}`, + request, + headers + ) + return await parseResponse(response) + } + + async updateIntegration( + request: UpdateIntegrationRequest, + headers?: Record + ): Promise> { + const response = await this.apiClient.put( + `/api/integration/${request.integrationId}`, + request, + headers + ) + return await parseResponse(response) + } + + async getIntegration( + request: GetIntegrationRequest, + headers?: Record + ): Promise> { + const response = await this.apiClient.get( + `/api/integration/${request.integrationId}`, + headers + ) + return await parseResponse(response) + } + + async getAllIntegrations( + request: GetAllIntegrationRequest, + headers?: Record + ): Promise> { + let url = `/api/integration/all/${request.workspaceId}` + 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 response = await this.apiClient.get(url, headers) + return await parseResponse(response) + } + + async deleteIntegration( + request: DeleteIntegrationRequest, + headers?: Record + ): Promise> { + const response = await this.apiClient.delete( + `/api/integration/${request.integrationId}`, + headers + ) + return await parseResponse(response) + } ```
    ___ **It is recommended to use instance methods rather than static methods for the
    IntegrationController unless there is a specific reason to prefer static methods.
    This allows easier management of state and dependencies if needed in the future.** [packages/api-client/src/controllers/integration/integration.ts [6-10]](https://github.com/keyshade-xyz/keyshade/pull/397/files#diff-4c6a75c90cc3c944a2848c76539d9513e351da13006375ff5b3d11c9a6ea9b17R6-R10) ```diff -static async createIntegration() {} -static async updateIntegration() {} -static async getIntegration() {} -static async getAllIntegrations() {} -static async deleteIntegration() {} +async createIntegration() {} +async updateIntegration() {} +async getIntegration() {} +async getAllIntegrations() {} +async deleteIntegration() {} ``` - [ ] **Apply this suggestion**
    Suggestion importance[1-10]: 6 Why: Using instance methods can provide better flexibility for managing state and dependencies, but the necessity depends on the specific use case of the controller.
    		6
    rajdip-b commented 1 month ago

    @vr-varad Hey bro, any updates?