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

[RFR] Support API empty response on DELETE and DELETE_MANY actions #3441

Merged
merged 6 commits into from
Aug 21, 2019

Conversation

Kmaschta
Copy link
Contributor

@Kmaschta Kmaschta commented Jul 21, 2019

Ok, I checked for both DELETE and DELETE_MANY actions.

  • DELETE doesn't require to have an identifier ({ data: { id: 'id' } }) but needs to have a data key, which should be returned by the data provider
  • DELETE_MANY should return a data as a array, but do not require to have something in that array. If the API doesn't return a response, the data provider will return an empty array

Note : It is possible for the data provider to return the ids or resources sent with the request thanks to the params argument, but since react admin does nothing with this data, I preferred to remove not require it.

@Kmaschta Kmaschta changed the title Avoid error for empty response on DELETE and DELETE_MANY actions [RFR] Avoid error for empty response on DELETE and DELETE_MANY actions Jul 21, 2019
@Kmaschta Kmaschta changed the title [RFR] Avoid error for empty response on DELETE and DELETE_MANY actions [RFR] Support API empty response on DELETE and DELETE_MANY actions Jul 21, 2019
packages/ra-data-json-server/src/index.js Outdated Show resolved Hide resolved
packages/ra-data-simple-rest/src/index.js Outdated Show resolved Hide resolved
docs/DataProviders.md Outdated Show resolved Hide resolved
@@ -467,8 +467,8 @@ Request Type | Response format
`CREATE` | `{ data: {Record} }`
`UPDATE` | `{ data: {Record} }`
`UPDATE_MANY` | `{ data: {mixed[]} }` The ids which have been updated
`DELETE` | `{ data: {Record} }`
`DELETE_MANY` | `{ data: {mixed[]} }` The ids which have been deleted
`DELETE` | `{ data: {Record} }` The resource that had been deleted or nothing
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
`DELETE` | `{ data: {Record} }` The resource that had been deleted or nothing
`DELETE` | `{ data: {Record} }` The record that has been deleted

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would precise, to the data provider maintainers that this data isn't required.

`DELETE` | `{ data: {Record} }`
`DELETE_MANY` | `{ data: {mixed[]} }` The ids which have been deleted
`DELETE` | `{ data: {Record} }` The resource that had been deleted or nothing
`DELETE_MANY` | `{ data: {mixed[]} }` The ids which have been deleted or an empty array
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
`DELETE_MANY` | `{ data: {mixed[]} }` The ids which have been deleted or an empty array
`DELETE_MANY` | `{ data: {mixed[]} }` The ids of the deleted records

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Idem, I would precise, to the data provider maintainers that this data isn't required.

@djhi
Copy link
Contributor

djhi commented Aug 5, 2019

Any update ?

Kmaschta and others added 5 commits August 5, 2019 14:23
Co-Authored-By: Francois Zaninotto <francois@marmelab.com>
Co-Authored-By: Francois Zaninotto <francois@marmelab.com>
Co-Authored-By: Francois Zaninotto <francois@marmelab.com>
@Kmaschta Kmaschta force-pushed the avoid-error-messages-when-empty-response branch from 95d3ccf to f67c27d Compare August 5, 2019 12:23
@Kmaschta Kmaschta force-pushed the avoid-error-messages-when-empty-response branch from f67c27d to 29d1670 Compare August 5, 2019 12:24
@Kmaschta
Copy link
Contributor Author

Kmaschta commented Aug 5, 2019

Reviews applied and rebased

@djhi djhi added this to the 2.9.6 milestone Aug 5, 2019
@fzaninotto fzaninotto merged commit 853c807 into master Aug 21, 2019
@fzaninotto fzaninotto deleted the avoid-error-messages-when-empty-response branch August 21, 2019 13:31
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.

None yet

3 participants