React-admin: Deleting a review in the demo crashes the app

[fzaninotto]

What you were expecting:

No crash on the example demo

What happened instead:

Steps to reproduce:

1. Open the demo example
2. display the review list
3. Select one or several rows
4. Click on "delete" in the bulk actions toolbar
5. Something went wrong

Other information:

Environment

• React-admin version: online demo https://marmelab.com/react-admin-demo/#/reviews
• Browser: Browser
• Stack trace (in case of a JS error):

react-dom.production.min.js:196 TypeError: Cannot read property 'id' of undefined
    at rowStyle.js:7
    at DatagridBody.js:52
    at Array.map (<anonymous>)
    at Qu (DatagridBody.js:31)
    at la (react-dom.production.min.js:154)
    at Su (react-dom.production.min.js:259)
    at vc (react-dom.production.min.js:230)
    at dc (react-dom.production.min.js:229)
    at ic (react-dom.production.min.js:223)
    at react-dom.production.min.js:121

[sshyam-gupta]

@fzaninotto can I pick this issue?
I think its because rowStyle is getting deleted record and so coming up as undefined

[fzaninotto]

Sure!

[tiagoschenkel]

Apparently, the problem is coming from this line of code:
https://github.com/marmelab/react-admin/blob/master/packages/ra-core/src/reducer/admin/resource/list/selectedIds.ts#L51

The action.payload is undefined. Actually, the payload is coming with the action.requestPayload property, probably this line of code was not updated.

[fzaninotto]

This was fixed sometime since the bug was filed. I cannot reproduce the issue anymore. So I'm closing it.

[tiagoschenkel]

@fzaninotto, I'm checking the source code of the FakeRest implementation and noticed that the removeOne method is actually collecting the element to be removed, removing it from the list and returning the deleted element:
https://github.com/marmelab/FakeRest/blob/v2.1.0/src/Collection.js#L285

Then the implementation of the method getResponse of ra-data-fakerest for delete request is returning this element as the data:
https://github.com/marmelab/react-admin/blob/v3.8.4/packages/ra-data-fakerest/src/index.ts#L104

So, when ra-core is handling the DELETE effect, it will expect to have the action.payload.id filled:
https://github.com/marmelab/react-admin/blob/v3.8.4/packages/ra-core/src/reducer/admin/resource/list/selectedIds.ts#L62

I'm unsure now if this is one assumption made by react-admin about how a generic REST API should work, or this is a particular case, and this would explain why you can't reproduce this error.

On my case, I'm using the Springboot as the REST API, and they don't return anything when the call to delete one element is processed:

@DeleteMapping("/employees/{id}")
ResponseEntity<?> deleteEmployee(@PathVariable Long id) {

  repository.deleteById(id);

  return ResponseEntity.noContent().build();
}

Debugging the ra-core code, the property action.payload will be udefined on this case, but we still have the action.requestPayload.

Perhaps this change in the selectedIds.ts file would handle this case:

const index = previousState.indexOf(action.payload ? action.payload.data.id : action.requestPayload.id);

Perhaps this change will make the reducer handler more generic.

[fzaninotto]

I'm unsure now if this is one assumption made by react-admin about how a generic REST API should work

Unfortunately, there is no such thing as a generic REST API ;) In order to accommodate various API dialects, we had to design our own. The dataProvider is the place to map react-admin's dialect with your server's dialect - in that case, copying the deleted record from the request to the response.

[tiagoschenkel]

Got it, thanks for your answer @fzaninotto.