Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
If a user does not have access to perform a bulk operation on any of the selected entities and the selected action requires a redirect to a confirmation form, the redirect is still performed, even though it does not make sense anyway and also results in an Access denied page being shown to the user.
Proposed resolution
Do not perform the redirect if the user does not have access to perform the bulk operation on any of the entities. We already show a proper error message for the non-allowed entities anyway, so it should be clear what is happening.
The if ($count) check in \Drupal\views\Plugin\views\field\BulkForm::viewsFormSubmit() can simply be moved before the check for the confirm route to solve this issue.
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | 3018148-9.patch | 2.72 KB | Lendude |
| #9 | interdiff-3018148-6-9.txt | 1001 bytes | Lendude |
| #6 | 3018148-6.patch | 2.71 KB | Lendude |
| #6 | interdiff-3018148-3-6.txt | 2.17 KB | Lendude |
| #3 | 3018148-3.patch | 2.7 KB | Lendude |
Comments
Comment #2
LendudeNice find.
Test and proposed fix, let's see if this breaks anything else...
Comment #3
Lendudequick cleanup
Comment #5
rgpublicPatch #3 works great for me under 8.7.10. Thanks!
Comment #6
LendudeFixed a minor typo.
@rgpublic feel free to set to RTBC if you feel this is ready
Comment #7
rgpublic@Lendude: Well for me it works without any problems (tested this by manually trying to delete files where some or all of them are inaccessible). In addition, I see you have added a dedicated test. Wonderful! But as this is quite a low-level change on a core-component, it might perhaps be useful to wait for another confirmation. I feel a bit reluctant to declare myself alone as the "community" (the C in RTBC) ;-) So.... Hello, you other 3 followers lurking in the dark... Could you verify/check if this is also working for you?
Comment #9
LendudeFixed the test, now that it's using stark and not classy anymore we can't use the class to find the views form.
@rgpublic you are the "C", never doubt it. You, along with the patch roller, and the core committer, means 3 pairs of eyes will have seen this once it gets committed, and that (plus automated test coverage) is sufficient. If it is something significantly hard it can be tagged for 'needs subsystem maintainer review', but this isn't (and I happen to be a Views subsystem maintainer, so you got that anyway :).
Would it be nice to always get more eyes on it? Sure, but it's not required, your eyes and your great manual testing of this as described in #7 is exactly what was missing for this to be RTBC'd.
Comment #10
rgpublicOkay, great! RTBC'ing per #9. Let's see what happens :-)
Comment #12
LendudeUnrelated fail, back to RTBC
Comment #13
alexpottCommitted and pushed 78f8ee05bb to 9.0.x and 7e844b90b6 to 8.9.x. Thanks!
Going to ask other committers about backporting to 8.8.x as I feel this is a risk free bugfix.
Comment #16
alexpott@catch +1'd the backport