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
In #2096717: Add parameter to User::getRoles() to exclude built in rids, (authenticated, anonymous), a regression was introduced for anonymous user sessions for getRoles. This happened because the constants for RID weren't used. It wasn't caught because test coveraged didn't cover anonymous roles.
Proposed resolution
Switch to use constants.
Add test coverage
Remaining tasks
none
User interface changes
n/a
API changes
n/a
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | interdiff.txt | 3.55 KB | heddn |
| #11 | 2277547-user_getRoles-11.patch | 6.52 KB | heddn |
| #11 | 2277547-user_getRoles-11-TEST_ONLY.patch | 5.34 KB | heddn |
| #8 | interdiff.txt | 1.81 KB | heddn |
| #8 | 2277547-user_getRoles-8.patch | 3.05 KB | heddn |
Comments
Comment #1
heddnComment #2
heddnComment #4
heddnAdded constants for RIDs to the unit tests.
Comment #5
heddnAdded constants for RIDs to the unit tests.
Comment #7
heddnAnother attempt to make the testbot happy.
Comment #8
heddnRe-worked the tests and added a tests only patch.
Comment #9
heddnComment #11
heddnIncluding UserSession in this patch.
Comment #13
jcfiala CreditAttribution: jcfiala commentedThis looks good to me. The use of the constants instead of the string "anonymous" and "authenticated" helps both fix the unfortunate misspelling of anonymous in the code and helps to future-proof the code in case the roles are renamed for Drupal 9. The tests look good and cover the case that bootstrap isn't called, necessitating the re-declaration of the role constants.
And I've tried applying the patch to the latest HEAD, and it applies cleanly.
Comment #14
alexpottCommitted 35bc4a2 and pushed to 8.x. Thanks!