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

Fix: patch & post store update (missing delete) #42

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

JuraJuki
Copy link
Contributor

There were 2 ways of going about this:

  1. Update the store from the response: I couldn't make it work dynamically, all the resources found on this topic use a specific object name which is updated after the request itself.
  2. Update the store by fetching in the success scope after the action: this is the way it is implemented in this PR

- missing update for delete request. This is because when a certain item is deleted, you cannot fetch it

@JuraJuki JuraJuki requested a review from renatoruk August 26, 2020 09:24
@JuraJuki JuraJuki changed the title Fix: patch & post store update Fix: patch & post store update (missing delete) Aug 26, 2020
@renatoruk
Copy link
Contributor

Thanks for your effort on implementing this. I see you considered two possible options — that's awesome.

However, I don't think this would be a viable way of doing the store update as it introduces unnecessary load to the server.
The json:api doc states this about successful patch response:

200 OK
If a server accepts an update but also changes the resource(s) in ways other than those specified by the request (for example, updating the updated-at attribute or a computed sha), it MUST return a 200 OK response. The response document MUST include a representation of the updated resource(s) as if a GET request was made to the request URL.
A server MUST return a 200 OK status code if an update is successful, the client’s current fields remain up to date, and the server responds only with top-level meta data. In this case the server MUST NOT include a representation of the updated resource(s).

That means that we can get the whole resource updated in store.
The challenge we may face is that the updated record needs to update some relationship (this may require some dependency tracking).

We can start small, by starting adding tests for this and working only on PATCH as this is the most common scenario. What do you think?

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

Successfully merging this pull request may close these issues.

2 participants