henvo / ra-jsonapi-client

JSON API data provider for react-admin.
MIT License
72 stars 70 forks source link

Support for DELETE_MANY request type #16

Open mrclayman opened 5 years ago

mrclayman commented 5 years ago

Hi @henvo ,

I have just hit an issue where I wanted to delete multiple records in a list by checking their boxes and hitting Delete. React-admin issues a DELETE_MANY request in that case but it appears to not be supported by ra-jsonapi-client.

According to the documentation at https://marmelab.com/react-admin/DataProviders.html#usage the task for the JSON API client is to issue multiple DELETE requests, but I would probably suggest to use the filter approach like in the case of the GET_MANY request. I would happily parse the query string on my end in the server and go from there.

Thanks. :slightly_smiling_face:

henvo commented 5 years ago

Hi @mrclayman

thanks for your input! You are right - the DELETE_MANY action is currently not supported by ra-jsonapi-client. And of course it would be great if it would. :+1:

but I would probably suggest to use the filter approach like in the case of the GET_MANY request

As far as I know the jsonapi specification (v1.0 or v1.1) does not officially support the filter approach. So I think the best and widely supported way would be also to use multiple DELETE requests.

I'll leave this open for discussion and as always: PR are welcome.

mrclayman commented 4 years ago

Hi @henvo ,

to follow up on your response, I have gotten to the point where I would like ra-jsonapi-client to have support for this particular request type so I am thinking about picking up the mantle and implement this feature myself. I am not a Javascript pro (always been more of a backend server dev), but I have been dabbling in it for some time and I would like to undertake this with your supervision if you don't mind. :blush:

First thing I would like to ask you is what do you think is the best way to implement this feature? I was thinking about it for a bit already and I can essentially see two ways to do it -- either with a single request with a batch of id's, or a bunch of requests, one per each id. Which way do you think is preferable? Or maybe provide both and make the use configurable?

Let me know, and cheers. :slightly_smiling_face:

mrclayman commented 4 years ago

Just to follow up on my previous message, I have noticed in your last response that you said the preferred way would be to send a bunch of DELETE requests. I understand that the jsonapi spec is unclear on this topic and, after a short googling session, it appears there have been many ideas on how to tackle this issue. I don't even know if the jsonapi standardization committee is anywhere close to a decision on this at this point. My overall opinion, after skimming through some of those discussions, is that there is a tendency to make the eventual solution all-encompassing and thus overly complicated (IMO, of course) given the breadth of cases that may arise in practice.

Therefore, I am more in favor of the filter approach. It appears that if I were to implement the solution using multiple self-contained DELETE requests, I would then have to keep track of multiple promises and somehow aggregate the responses and form something that react-admin can work with when data provider function returns (if my understanding is correct, to react-admin, "delete many" is still considered a single request and so it does not expect to receive multiple promises). Given the current state of the code ra-jsonapi-client, a substantial extension/rewrite would be necessary to accomplish this. Moreover, my personal preference when making bulk DELETE requests is to make their processing transactional, i.e. either all requested resources are deleted and 2xx status is returned, or none is and a 4xx/5xx status is returned.

I forked this repository and have already implemented the filter approach. If you want to take a look at it, I can create a PR. If you are not in favor of this solution, then I am going to have to keep it for myself for now as I need that functionality for a project I have been working on and I don't have the time to refactor the client to implement this feature the proper way.

Let me know what you think. Cheers.

Vaflan commented 1 year ago

Hi @henvo ,

FYI and missed UPDATE_MANY -> updateMany https://marmelab.com/react-admin/Architecture.html