Problem/Motivation
An HTTP DELETE request to a JSONAPI Resource gets a 500 error response if the resource (a drupal entity) is configured by this module to not be deleted if other entities reference it.
This happens because EntityPredelete throws an error. It would be great if that error message could bubble up and be included in the JSONAPI response.
Steps to reproduce
Create entities that reference each other, configure them to not be deleted if referenced, and perform a DELETE request the JSONAPI endpoint: https://www.drupal.org/docs/8/modules/jsonapi/removing-existing-resource...
Proposed resolution
One solution would be to implement hook_entity_access
, check that $operation == 'delete'
and deny access to entities that have the referenced integrity constraint. This will return a 403 response with an optional AccessResultForbidden reason included in the response.
With this approach it's important to only deny access when the request is to a JSON:API resource. Otherwise the user will not have access to delete the entity in any other context - a good example is the "Delete" button on the Entity edit form which is not displayed if the user does not have access to delete the entity.
Remaining tasks
Include test coverage.
User interface changes
N/A
API changes
N/A
Data model changes
N/A
Comment | File | Size | Author |
---|
Issue fork entity_reference_integrity-3195039
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3195039-500-error-when changes, plain diff MR !2
Comments
Comment #3
paul121 CreditAttribution: paul121 commentedAdded a fix as proposed above and included a functional test.
What's nice is that this approach does not depend on the JSONAPI module;
hook_entity_access
simply checks if the request is coming from the JSONAPI controller and returns otherwise. As noted in the JSONAPI classes, it is possible that their internal implementation could change and that we should not depend on its PHP code. This is why I re-implemented their logic rather than actually use their helper method which could change.Comment #4
paul121 CreditAttribution: paul121 commentedThanks again for the review @Sam152 . I took a pass at implementing your requested changes, they make sense to me.
Comment #5
m.stentaAttaching a patch for use with
cweagans/composer-patches
until the MR gets merged. Thanks for your work on this @paul121 and @sam152!Comment #6
m.stentaAh, so we actually want to use both this patch AND the patch for #2831365: Make action confirm_form_route_name forms that delete entities disabled when protected, but applying them together creates a small conflict due to the
use
statements inentity_reference_integrity_enforce.module
. So here is one more patch that can be applied AFTER applying the patch from #2831365. Again, though, disregard this in favor of the MR. :-)Comment #8
Sam152 CreditAttribution: Sam152 as a volunteer and commentedCommitted, thanks.
I do wonder if perhaps it belonged in a sub-module before tagging a new release, but I don't feel that strongly about it.
Comment #9
m.stentaThanks @Sam152!! For this and #2831365!
Comment #11
m.stenta@Sam152 - Any chance we can release 8.x-1.1 with these commits so that we can pin to it and remove the patches from our project?
Not urgent of course - if you have other things you'd like to include. Worth asking at least. :-)
Comment #12
Sam152 CreditAttribution: Sam152 as a volunteer and commentedDone, cheers: https://www.drupal.org/project/entity_reference_integrity/releases/8.x-1.1