- Status changed to RTBC
about 2 years ago 3:25pm 2 March 2023 - ๐ฉ๐ชGermany jurgenhaas Gottmadingen
Unfortunately, the approach with
PathValidator::isValid
doesn't really achieve what's needed. The permission check only looks for the permissionlink to any page
, but that doesn't necessarily mean that the current user is allowed to edit the current entity. The rest of the patch looks ok, but the permission check needs a different approach. - ๐ฉ๐ชGermany jurgenhaas Gottmadingen
I have started an MR which uses the simple entity access controller to see if the user has edit permissions.
- Status changed to Needs review
almost 2 years ago 1:46pm 26 April 2023 - @jurgenhaas opened merge request.
- First commit to issue fork.
- @elgandoz opened merge request.
- Merge request !30Issue #3319445: Edit link in breadcrumb region shows even when user has no permission โ (Open) created by elgandoz
- ๐ฆ๐บAustralia elgandoz Canberra
#11: the merge was blocked by a conflict and I (unsuccessfully) tried to rebase from upstream, sorry about the mess
#14: this is the new branch with the cherry picked commit by @jurgenhaas, with the merge fix.
I had to change the fix given that I tried with different roles and one of them (implementer) wasn't able to see the breadcrumb despite having the permission "[CT]: Edit its own content".See here the differences between a moderator and a implementer:
The issue was basically the same as this one https://www.drupal.org/project/drupal/issues/3204423 โ , caused by the confusion of the names between "edit" and "update" for the entity access check.
Before:$entity->access('edit')
After:$entity->access('update')
Now it works nicely has expected, please review.
- ๐ฉ๐ชGermany jurgenhaas Gottmadingen
Thanks @elgandoz for updating the MR. Unfortunately, it only applies to 1.0.x-dev but not to 1.0.0-RC3 because of this commit: https://git.drupalcode.org/project/gin_toolbar/-/commit/4e66cd7c20b249be...
But there isn't much we can do about that.
- ๐ฆ๐บAustralia elgandoz Canberra
Hey @jurgenhaas, I removed the commits past your one and amended it, in order to have the 'update' access check, in the original
3319445-edit-link-in
branch you created, and force-pushed.
So now the diff from MR!25 should apply on1.0.0-RC3
, while MR!30 goes on1.0.x-dev
in order to get merged.
Please check since I didn't test it. - Status changed to RTBC
over 1 year ago 8:56am 28 July 2023 - ๐ฉ๐ชGermany jurgenhaas Gottmadingen
Thanks @elgandoz that's a nice solution as well. So MR !25 is for the latest RC3 and MR !30 is for the dev release. This is working great.
- ๐จ๐ฆCanada Liam Morland Ontario, CA ๐จ๐ฆ
Liam Morland โ made their first commit to this issueโs fork.
- ๐ฉ๐ชGermany jurgenhaas Gottmadingen
MR!30 didn't apply any longer and I've done a rebase. However, one part of the fix needs to be handled in Gin now, since the part of the template where this is required has moved over there.
- First commit to issue fork.
- ๐ฆ๐ทArgentina hanoii ๐ฆ๐ทUTC-3
hanoii โ changed the visibility of the branch 3319445-edit-link-in to hidden.
- ๐ฆ๐ทArgentina hanoii ๐ฆ๐ทUTC-3
hanoii โ changed the visibility of the branch 3319445-edit-link-rebased to hidden.
- ๐ฆ๐ทArgentina hanoii ๐ฆ๐ทUTC-3
The rebased one was missing some twig changes, I happened to worked on fixing this before having found this issue :facepalm:
I a very similar thing and went ahead and push it, it makes it cleaner in the the twig and uses the proper drupal services.
- First commit to issue fork.
- ๐ฆ๐บAustralia dpi Perth, Australia
Overall proposal looks fine, however I've modified the MR to be a little more correct.
- First commit to issue fork.
- Assigned to jurgenhaas
- Status changed to Needs review
2 months ago 5:59am 21 January 2025