feat: add delete endpoint (#345) #346

psankhe28 · 2024-08-13T19:36:39Z

Description

It adds an endpoint to delete the drs object based on the object id

Fixes #(issue)
#333

Checklist

My code follows the contributing guidelines of this project.
I am aware that all my commits will be squashed into a single commit, using the PR title as the commit message.
I have performed a self-review of my own code.
I have commented my code in hard-to-understand areas.
I have updated the user-facing documentation to describe any new or changed behavior.
My changes generate no new warnings.
I have added tests that prove my fix is effective or that my feature works.
I have not reduced the existing code coverage.

Comments

Summary by Sourcery

Add a delete endpoint to the API for removing DRS objects by ID and update the documentation to reflect this new functionality.

New Features:

Introduce a new endpoint to delete a DRS object based on its ID.

Documentation:

Update the README to include documentation for the new deleteObject function, detailing its parameters and usage.

Signed-off-by: Pratiksha Sankhe <sankhepratiksha3@gmail.com> Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>

changeset-bot · 2024-08-13T19:36:42Z

⚠️ No Changeset found

Latest commit: 7b0a6b8

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

vercel · 2024-08-13T19:36:43Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
elixir-cloud-components	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Aug 13, 2024 7:38pm

sourcery-ai · 2024-08-13T19:36:48Z

Reviewer's Guide by Sourcery

This pull request implements a new endpoint to delete DRS objects by their ID. The changes include adding a new deleteObject function in the drsAPI.ts file and updating the README.md to document this new functionality. The implementation follows RESTful principles and includes error handling.

File-Level Changes

Files	Changes
`packages/ecc-client-elixir-drs-filer/src/API/Object/drsAPI.ts`	Implemented a new `deleteObject` function to handle DELETE requests for DRS objects
`packages/ecc-client-elixir-drs-filer/src/API/Object/drsAPI.ts`	Added JSDoc comments to explain the `postObject` and `deleteObject` functions
`packages/ecc-client-elixir-drs-filer/src/API/Object/drsAPI.ts`	Updated the export statement to include the new `deleteObject` function
`packages/ecc-client-elixir-drs-filer/src/API/Object/README.md`	Added documentation for the new `deleteObject` endpoint in the README

Tips

Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
Continue your discussion with Sourcery by replying directly to review comments.
You can change your review settings at any time by accessing your dashboard:
- Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
- Change the review language;
You can always contact us if you have any questions or feedback.

sourcery-ai

Hey @psankhe28 - I've reviewed your changes - here's some feedback:

Overall Comments:

Consider improving error handling in the deleteObject function. The current implementation returns different structures for errors vs. success, which could lead to inconsistent behavior. Also, consider checking and handling specific HTTP status codes.
The JSDoc for the deleteObject function is less detailed compared to postObject. Please enhance the documentation to maintain consistency and provide more information about parameters and return values.
No tests have been added for the new deleteObject endpoint. Given that this is a data-modifying operation, it's crucial to have comprehensive tests to ensure correct functionality and handle various scenarios.

Here's what I looked at during the review

🟡 General issues: 4 issues found
🟢 Security: all looks good
🟢 Testing: all looks good
🟢 Complexity: all looks good
🟡 Documentation: 2 issues found

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.}

packages/ecc-client-elixir-drs-filer/src/API/Object/drsAPI.ts

sourcery-ai · 2024-08-13T19:38:14Z

packages/ecc-client-elixir-drs-filer/src/API/Object/drsAPI.ts

+ * @param {object} objectData - The data of the drs object to be posted.
+ *                           This should be an object containing the necessary fields for the drs object.
+ *                           Modify the structure according to your object requirements.
+ * @returns {string} - A string that denotes the object id


issue: Update JSDoc to match actual return type of postObject

The JSDoc indicates that postObject returns a string, but the implementation suggests it returns an object. Please update the documentation to accurately reflect the function's behavior.

@psankhe28, please take a look at this

packages/ecc-client-elixir-drs-filer/src/API/Object/drsAPI.ts

packages/ecc-client-elixir-drs-filer/src/API/Object/README.md

SalihuDickson · 2024-08-14T22:25:20Z

packages/ecc-client-elixir-drs-filer/src/API/Object/drsAPI.ts

+      method: "DELETE",
+    });
+
+    if (!response) {


consider some more robust error handling like checking the status text, as checking for just the response object won't catch many errors that may occur.

Also consider this for the other postObject function.

SalihuDickson · 2024-08-27T17:17:20Z

@psankhe28, please address the request changes and resolve conflicts with main so I can re-review.

feat: add delete endpoint (#345)

b38957d

Signed-off-by: Pratiksha Sankhe <sankhepratiksha3@gmail.com> Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>

Merge branch 'main' into delete-endpoint

7b0a6b8

sourcery-ai bot reviewed Aug 13, 2024

View reviewed changes

vercel bot deployed to Preview August 13, 2024 19:38 View deployment

SalihuDickson requested changes Aug 14, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add delete endpoint (#345) #346

feat: add delete endpoint (#345) #346

psankhe28 commented Aug 13, 2024 •

edited by sourcery-ai bot

Loading

changeset-bot bot commented Aug 13, 2024 •

edited

Loading

vercel bot commented Aug 13, 2024 •

edited

Loading

sourcery-ai bot commented Aug 13, 2024 •

edited

Loading

sourcery-ai bot left a comment

sourcery-ai bot Aug 13, 2024

SalihuDickson Aug 14, 2024

SalihuDickson Aug 14, 2024

SalihuDickson commented Aug 27, 2024

feat: add delete endpoint (#345) #346

Are you sure you want to change the base?

feat: add delete endpoint (#345) #346

Conversation

psankhe28 commented Aug 13, 2024 • edited by sourcery-ai bot Loading

Description

Checklist

Comments

Summary by Sourcery

changeset-bot bot commented Aug 13, 2024 • edited Loading

⚠️ No Changeset found

vercel bot commented Aug 13, 2024 • edited Loading

sourcery-ai bot commented Aug 13, 2024 • edited Loading

Reviewer's Guide by Sourcery

File-Level Changes

sourcery-ai bot left a comment

Choose a reason for hiding this comment

sourcery-ai bot Aug 13, 2024

Choose a reason for hiding this comment

SalihuDickson Aug 14, 2024

Choose a reason for hiding this comment

SalihuDickson Aug 14, 2024

Choose a reason for hiding this comment

SalihuDickson commented Aug 27, 2024

psankhe28 commented Aug 13, 2024 •

edited by sourcery-ai bot

Loading

changeset-bot bot commented Aug 13, 2024 •

edited

Loading

vercel bot commented Aug 13, 2024 •

edited

Loading

sourcery-ai bot commented Aug 13, 2024 •

edited

Loading