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
The postSave method for Role is not needed anymore. It used to help with cache invalidation, but Drupal 8 now has correct contexts and tags everywhere, so it should be obsolete now.
Proposed resolution
Remove the method from core/modules/user/src/Entity/Role.php
Remaining tasks
None
User interface changes
None
API changes
None
| Comment | File | Size | Author |
|---|---|---|---|
| #17 | removePostsaveMethod_2475263_12-interdiff.txt | 3.06 KB | LKS90 |
| #13 | removePostsaveMethod_2475263_12.patch | 13 KB | LKS90 |
| #11 | removePostsaveMethod_2475263_11-interdiff.txt | 9.42 KB | Berdir |
| #11 | removePostsaveMethod_2475263_11.patch | 9.94 KB | Berdir |
| #5 | removePostsaveMethod_2475263_2.patch | 532 bytes | LKS90 |
Comments
Comment #1
LKS90 CreditAttribution: LKS90 at MD Systems GmbH commentedHere is the patch that removes the method.
Comment #3
Berdirthe fatal error'd tests are unrelated, but CommentAnonymousTest looks like an actual issue that is missing cache tags somewhere...
Comment #4
Wim LeersComment #5
LKS90 CreditAttribution: LKS90 at MD Systems GmbH commentedCan't reproduce the Test fail locally, reupload.
Comment #6
LKS90 CreditAttribution: LKS90 at MD Systems GmbH commentedComment #8
LKS90 CreditAttribution: LKS90 at MD Systems GmbH commentedComment #11
BerdirSo, there are apparently two problems that were hidden by invalidating all render caches when saving a role:
a) the access checks on the reply form redirect and do not add cacheability metadata. This patch converts it to an actual access check with proper metadata. The downside is that users no longer get redirect back and don't see those messages anymore. I think that's OK, but not sure. The alternative would be to add a CacheableRedirectResponse. Would be a good idea anyway to have that (I have 2 contrib modules that can use that i think), but would also make the logic in that method even more convoluted I think, as we need to collect all the relevant metadata for every redirect.
Comment #13
LKS90 CreditAttribution: LKS90 at MD Systems GmbH commentedAlso changed the two other failing tests to assert a 403 response instead of looking for some text.
Comment #14
LKS90 CreditAttribution: LKS90 at MD Systems GmbH commentedComment #15
catchThere was another issue dealing with the reply form that ran into similar issues, can't remember which one it was though atm.
Comment #16
Wim Leers#13: could you provide an interdiff? That makes it easier to see what you changed relative to the previous patch. Thanks :)
Comment #17
LKS90 CreditAttribution: LKS90 at MD Systems GmbH commentedAn interdiff, as requested.
Comment #18
Fabianx CreditAttribution: Fabianx as a volunteer and at Tag1 Consulting commentedOpened #2491545: Bring back the error messages for the 403 page of the comment handler to bring those messages back potentially with a clean interface.
Overall this is much better, RTBC.
Comment #19
catchCommitted/pushed to 8.0.x, thanks!