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

Merged
merged 18 commits into from
Aug 9, 2024

Conversation

vr-varad
Copy link
Contributor

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

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

  • 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


Description

  • Created IntegrationController class in integration.ts with static methods for creating, updating, retrieving, and deleting integrations.
  • Added TypeScript interfaces in integrayion.types.d.ts for request and response types related to integration operations.

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

    Copy link
    Contributor

    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.

    Copy link
    Contributor

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

    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:

    -  static async createIntegration() {}
    -  static async updateIntegration() {}
    -  static async getIntegration() {}
    -  static async getAllIntegrations() {}
    -  static async deleteIntegration() {}
    +  static async createIntegration(
    +    request: CreateIntegrationRequest,
    +    headers?: Record<string, string>
    +  ): Promise<CreateIntegrationResponse> {
    +    return this.apiClient.post(
    +      `api/integration/${request.workspaceId}`,
    +      request,
    +      headers
    +    )
    +  }
    +
    +  static async updateIntegration(
    +    request: UpdateIntegrationRequest,
    +    headers?: Record<string, string>
    +  ): Promise<UpdateIntegrationResponse> {
    +    return this.apiClient.put(
    +      `/api/integration/${request.workspaceId}`,
    +      request,
    +      headers
    +    )
    +  }
    +
    +  static async getIntegration(
    +    request: GetIntegrationRequest,
    +    headers?: Record<string, string>
    +  ): Promise<GetIntegrationResponse> {
    +    return this.apiClient.get(
    +      `/api/integration/${request.integrationId}`,
    +      headers
    +    )
    +  }
    +
    +  static async getAllIntegrations(
    +    request: GetAllIntegrationRequest,
    +    headers?: Record<string, string>
    +  ): Promise<GetAllIntegrationResponse> {
    +    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<string, string>
    +  ): Promise<DeleteIntegrationResponse> {
    +    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]

    -static async createIntegration() {}
    -static async updateIntegration() {}
    -static async getIntegration() {}
    -static async getAllIntegrations() {}
    -static async deleteIntegration() {}
    +static async createIntegration(data: CreateIntegrationRequest): Promise<CreateIntegrationResponse> {}
    +static async updateIntegration(data: UpdateIntegrationRequest): Promise<UpdateIntegrationResponse> {}
    +static async getIntegration(id: string): Promise<GetIntegrationResponse> {}
    +static async getAllIntegrations(): Promise<GetAllIntegrationResponse[]> {}
    +static async deleteIntegration(id: string): Promise<DeleteIntegrationResponse> {}
     
    • 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:

    -  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]

    -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]

    -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:

     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<string, string>
    +  ): Promise<ClientResponse<CreateIntegrationResponse>> {
    +    const response = await this.apiClient.post(
    +      `/api/integration/${request.workspaceId}`,
    +      request,
    +      headers
    +    )
    +    return await parseResponse<CreateIntegrationResponse>(response)
    +  }
    +
    +  async updateIntegration(
    +    request: UpdateIntegrationRequest,
    +    headers?: Record<string, string>
    +  ): Promise<ClientResponse<UpdateIntegrationResponse>> {
    +    const response = await this.apiClient.put(
    +      `/api/integration/${request.integrationId}`,
    +      request,
    +      headers
    +    )
    +    return await parseResponse<UpdateIntegrationResponse>(response)
    +  }
    +
    +  async getIntegration(
    +    request: GetIntegrationRequest,
    +    headers?: Record<string, string>
    +  ): Promise<ClientResponse<GetIntegrationResponse>> {
    +    const response = await this.apiClient.get(
    +      `/api/integration/${request.integrationId}`,
    +      headers
    +    )
    +    return await parseResponse<GetIntegrationResponse>(response)
    +  }
    +
    +  async getAllIntegrations(
    +    request: GetAllIntegrationRequest,
    +    headers?: Record<string, string>
    +  ): Promise<ClientResponse<GetAllIntegrationResponse>> {
    +    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<GetAllIntegrationResponse>(response)
    +  }
    +
    +  async deleteIntegration(
    +    request: DeleteIntegrationRequest,
    +    headers?: Record<string, string>
    +  ): Promise<ClientResponse<DeleteIntegrationResponse>> {
    +    const response = await this.apiClient.delete(
    +      `/api/integration/${request.integrationId}`,
    +      headers
    +    )
    +    return await parseResponse<DeleteIntegrationResponse>(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]

    -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

    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.

    I found quite a few type errors in your code. Please refer to the DTOs of the specific controllers and get them fixed in the input. as for the output, we dont have postman updated for this, so you would need to test it.

    packages/api-client/src/types/integrayion.types.d.ts Outdated Show resolved Hide resolved
    packages/api-client/src/types/integrayion.types.d.ts Outdated Show resolved Hide resolved
    packages/api-client/src/types/integrayion.types.d.ts Outdated Show resolved Hide resolved
    packages/api-client/src/types/integrayion.types.d.ts Outdated Show resolved Hide resolved
    packages/api-client/src/types/integrayion.types.d.ts Outdated Show resolved Hide resolved
    packages/api-client/src/types/integrayion.types.d.ts Outdated Show resolved Hide resolved
    packages/api-client/src/types/integrayion.types.d.ts Outdated Show resolved Hide resolved
    packages/api-client/src/types/integrayion.types.d.ts Outdated Show resolved Hide resolved
    @vr-varad vr-varad requested a review from rajdip-b July 29, 2024 05:27
    @rajdip-b rajdip-b force-pushed the api-client/integration branch from 18fc9bc to 831cb8b Compare July 29, 2024 05:31
    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.

    Include the IntegrationController in index.ts exports list

    packages/api-client/tests/integration.spec.ts Outdated Show resolved Hide resolved
    packages/api-client/src/types/integration.types.d.ts Outdated Show resolved Hide resolved
    packages/api-client/tests/integration.spec.ts Show resolved Hide resolved
    packages/api-client/tests/integration.spec.ts Outdated Show resolved Hide resolved
    packages/api-client/tests/integration.spec.ts Show resolved Hide resolved
    @rajdip-b
    Copy link
    Member

    rajdip-b commented Aug 7, 2024

    @vr-varad Hey bro, any updates?

    @vr-varad vr-varad force-pushed the api-client/integration branch from 93b98c4 to c048e03 Compare August 8, 2024 15:03
    @rajdip-b rajdip-b merged commit ce16b14 into keyshade-xyz:develop Aug 9, 2024
    4 checks passed
    rajdip-b added a commit that referenced this pull request Sep 5, 2024
    Co-authored-by: vr-varad <varadgupta21#gmail.com>
    Co-authored-by: Rajdip Bhattacharya <agentR47@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 Integration module
    2 participants