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(platfrom): add delete method in api client #225

Merged
merged 1 commit into from
May 18, 2024
Merged

Conversation

kriptonian1
Copy link
Contributor

@kriptonian1 kriptonian1 commented May 18, 2024

User description

Description

Give a summary of the change that you have made

Fixes #[ISSUENO]

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

  • Added a delete method to the APIClient class in api-client.ts to support DELETE HTTP requests.
  • The new method enhances the API client by allowing deletion operations, configured to handle credentials and JSON content types.

Changes walkthrough 📝

Relevant files
Enhancement
api-client.ts
Add DELETE Method to APIClient Class                                         

apps/platform/src/lib/api-client.ts

  • Added a new delete method to the APIClient class.
  • The delete method sends a DELETE request to a specified URL and
    returns a promise with the response data.
  • Configured the method to include credentials and set the
    'Content-Type' header to 'application/json'.
  • +16/-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 Description updated to latest commit (2125b11)

    Copy link

    sonarcloud bot commented May 18, 2024

    Quality Gate Passed Quality Gate passed

    Issues
    0 New issues
    0 Accepted issues

    Measures
    0 Security Hotspots
    No data about Coverage
    0.0% Duplication on New Code

    See analysis details on SonarCloud

    @kriptonian1 kriptonian1 requested a review from rajdip-b May 18, 2024 10:25
    @rajdip-b rajdip-b merged commit 55cf09f into develop May 18, 2024
    6 checks passed
    @rajdip-b rajdip-b deleted the api-client-delete branch May 18, 2024 10:28
    Copy link
    Contributor

    PR Review 🔍

    ⏱️ Estimated effort to review [1-5]

    2, because the PR introduces a single method with a clear purpose and straightforward implementation. The changes are localized and do not seem to involve complex logic.

    🧪 Relevant tests

    No

    ⚡ Possible issues

    Missing Error Handling: The new delete method does not include any specific error handling. It would be beneficial to handle potential errors that could arise during the DELETE request.

    🔒 Security concerns

    No

    Copy link
    Contributor

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Enhancement
    Add error handling to the delete method to improve robustness

    Consider adding error handling for the delete method to manage exceptions or failed HTTP
    requests gracefully. This can be done using try-catch blocks or by chaining a .catch()
    method to the returned promise.

    apps/platform/src/lib/api-client.ts [80-87]

     delete<T>(url: string): Promise<T> {
       return this.request<T>(url, {
         method: 'DELETE',
         headers: {
           'Content-Type': 'application/json'
         },
         credentials: 'include'
    -  })
    +  }).catch(error => {
    +    console.error('Failed to delete:', error);
    +    throw error;
    +  });
     }
     
    Suggestion importance[1-10]: 8

    Why: Adding error handling to the delete method is crucial for robustness and to manage exceptions or failed HTTP requests gracefully. This suggestion correctly identifies a significant improvement area in the new code.

    8
    Possible issue
    Handle potential 204 No Content responses in the delete method

    To ensure the delete method can handle cases where no content is returned (204 No
    Content), consider checking the response status and handling it accordingly before parsing
    the response as JSON.

    apps/platform/src/lib/api-client.ts [80-87]

     delete<T>(url: string): Promise<T> {
       return this.request<T>(url, {
         method: 'DELETE',
         headers: {
           'Content-Type': 'application/json'
         },
         credentials: 'include'
    -  })
    +  }).then(response => {
    +    if (response.status === 204) {
    +      return null; // or appropriate handling
    +    }
    +    return response.json();
    +  });
     }
     
    Suggestion importance[1-10]: 7

    Why: Handling 204 No Content responses in the delete method is important for correct API function, especially when no content is expected as a response. This suggestion correctly targets a new method implementation and improves its reliability.

    7

    rajdip-b pushed a commit that referenced this pull request May 24, 2024
    ## [1.4.0](v1.3.0...v1.4.0) (2024-05-24)
    
    ### 🚀 Features
    
    * add example for health and email auth ([b834d25](b834d25))
    * **api:** Add `minio-client` provider ([#237](#237)) ([cd71c5a](cd71c5a))
    * **api:** Add feature to fork projects ([#239](#239)) ([3bab653](3bab653))
    * **api:** Added feedback form module ([#210](#210)) ([ae1efd8](ae1efd8))
    * **api:** Added Project Level Access  ([#221](#221)) ([564f5ed](564f5ed))
    * **api:** Added support for changing email of users ([#233](#233)) ([5ea9a10](5ea9a10))
    * implemented auth, ui for most, and fixed cors ([#217](#217)) ([feace86](feace86))
    * **platfrom:** add delete method in api client ([#225](#225)) ([55cf09f](55cf09f))
    * **postman:** add example for get_self and update_self ([e015acf](e015acf))
    * **web:** Add and link privacy and tnc page ([#226](#226)) ([ec81eb9](ec81eb9))
    
    ### 🐛 Bug Fixes
    
    * **web:** docker next config not found ([#228](#228)) ([afe3160](afe3160))
    
    ### 📚 Documentation
    
    * Added docs regarding postman, and refactored architecture diagrams ([f1c9777](f1c9777))
    * Fix typo in organization-of-code.md ([#234](#234)) ([11244a2](11244a2))
    
    ### 🔧 Miscellaneous Chores
    
    * **api:** Get feedback forward email from process.env ([#236](#236)) ([204c9d1](204c9d1))
    * **postman:** Initialized postman ([bb76384](bb76384))
    * **release:** Update changelog config ([af91283](af91283))
    * Remove swagger docs ([#220](#220)) ([7640299](7640299))
    
    ### 🔨 Code Refactoring
    
    * **api:** Replaced OTP code from alphanumeric to numeric ([#230](#230)) ([f16162a](f16162a))
    @rajdip-b
    Copy link
    Member

    🎉 This PR is included in version 1.4.0 🎉

    The release is available on GitHub release

    Your semantic-release bot 📦🚀

    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.

    2 participants