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

Automatically support patch vs put updates #84

Open
optmsp opened this issue May 29, 2023 · 1 comment
Open

Automatically support patch vs put updates #84

optmsp opened this issue May 29, 2023 · 1 comment

Comments

@optmsp
Copy link

optmsp commented May 29, 2023

Hey @aidan-casey! Okay, I see there is still an open item for patch support?

I think there are two approaches to this:

  • Caller makes an explicit decision every time on using PUT/PATCH (e.g., update(), updatePatch())
  • Lib makes a runtime decision based on count of supplied attributes vs total possible attribute count

Correct me if I'm wrong, but the only time we must use a PUT is if we want to unset a value in Autotask, meaning we have to use a PUT with an empty value in an attribute.

Before digging in too deep, will the lib reject if we supply an entity to update() that is missing some/most of the attributes? I haven't looked at your validation code in a while. If you are explicitly throwing an exception on that, I would understand. Perhaps we could supply an option like enablePatch=true on the update() which would let the lib decide if it wanted to use a PUT or PATCH?

I'm trying to reduce changes to your API while supporting PATCH updates as transparently as possible.

@aidan-casey
Copy link
Member

Hey, @optmsp! That is correct.

Currently the update method requires a fully filled out entity, including any properties required for a create request. Any properties unset or changed will attempt to be unset or changed in the request. In this sense, a PUT request basically equates to a replacement, consistent with the PUT specification.

Since most users of this package are retrieving an entity prior to updating it, I was of the mind to defer this to the next version with an API change of update being a PATCH request and replace being a PUT request. This helps ensure the most efficient method is chosen as well.

The problem with trying to support this on the existing update method is that we need a way to only accept some of the properties. This isn't very doable with a DTO since we are requiring certain properties.

I don't necessarily have a specific solution for this version, but I am open to suggestions. 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants