Problem/Motivation
I'm using InlineParagraphsWidget
with a custom entity type, not Paragraphs. As soon as I create an entity I see Paragraph's You are not allowed to remove this Paragraph. message. This is because the access check ends up in \Drupal\Core\Entity\EntityAccessControlHandler::checkAccess()
which does:
if ($operation == 'delete' && $entity->isNew()) {
return AccessResult::forbidden()->addCacheableDependency($entity);
}
Since #2733535: Better Paragraph deletion permission logics, upon $parent_access this no longer happens for Paragraphs itself because that check is never reached. I could implement a similar access logic for my own entity type, but I think the logic in the widget is actually incorrect. delete
is a storage-related operation so checking that for new entities is simply wrong, I think.
Proposed resolution
Check $entity->isNew()
in addition to $entity->access('delete')
Remaining tasks
User interface changes
API changes
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#9 | 2851492-9.patch | 5.64 KB | tstoeckler |
|
Comments
Comment #2
BerdirNot sure I understand this 100% yet, but makes sense I think, patch? :)
Comment #3
tstoecklerKind of lost track of this, here we go.
This replaces
$paragraphs_entity->access('delete')
with$paragraphs_entity->isNew() || $paragraphs_entity->access('delete')
consistently inInlineParagrapsWidget
andParagraphsWidget
(including some boolean logic simplification. I also removed one obsolete instantiation of the$button_access
variable, since that seemed reasonably in scope since we're modifying that variable above.This works for me and properly shows a Remove button for newly created paragraphs with my custom paragraphs entity type.
Comment #4
tstoecklerRerolled for 1.2
I decided to put the two delete-related checks in a variable as that makes things slightly more readable.
Comment #6
tstoecklerOh, so I guess 8.x-1.x is moving too fast for me ;-) This one should apply
Comment #7
tstoecklerNeeded a re-roll again.
Not sure if it's intentional that the new
removeButtonAccess()
is not used everywhere.In any case would be great to get some feedback here, on what's needed to get this in.
Comment #9
tstoecklerHere we go, this should be green.
Comment #10
DigitalCatalyst CreditAttribution: DigitalCatalyst as a volunteer commentedI checked out this patch, it does hide the message "You are not allowed to remove this Paragraph." on a paragraph, however, it reverts the hiding of the Remove button, for paragraphs when you have only 1 paragraph, from this thread, https://www.drupal.org/project/paragraphs/issues/2919186
Comment #11
gg24 CreditAttribution: gg24 as a volunteer and commentedHi @DigitalCatalyst,
Yes, the patch works perfectly fine for me and it serves the purpose of this issue. The point raised that it reverts the hiding of the Remove button, for paragraphs when you have only 1 paragraph, from this thread, https://www.drupal.org/project/paragraphs/issues/2919186 is not related to this fix and here just the new entity check is added along with the delete access. I think we can move this to RTBC.
Thanks, @tstoeckler for the patch. Patch works as expected.
Hence moving it to RTBC.
Thanks!
Comment #12
gg24 CreditAttribution: gg24 as a volunteer and commentedComment #13
BerdirYes, I don't see how this reverts the other issues, please provide steps to reproduce. We have test coverage for that and it doesn't fail.
Comment #15
BerdirNo feedback, so committed. If someone can reproduce, feel free to create a new issue.