Problem/Motivation

Split from #2025089: Deprecate user_role_grant_permissions() and user_role_revoke_permissions().

Proposed resolution

user_role_permissions() should be deprecated for removal. There's only one use-case in core and it should be replaced by something like:

  $entities = Role::loadMultiple($roles);
  $role_permissions = [];
  foreach ($roles as $rid) {
    $role_permissions[$rid] = $entities[$rid]?->getPermissions() : [];
  }

Remaining tasks

- revoew/commit

User interface changes

no

API changes

- deprecated user_role_permissions() and internal _user_role_permissions_update()
- PermissionsHashGenerator now require EntityTypeManagerInterface as 4th constructor argument

Data model changes

no

Release notes snippet

Comments

alexpott created an issue. See original summary.

alexpott’s picture

Status: Postponed » Active

There no need to postpone this.

ranjit1032002’s picture

Status: Active » Needs review
StatusFileSize
new1.6 KB

Uploaded a patch for the issue mentioned, please review.
Thank You.

alexpott’s picture

Status: Needs review » Needs work
  1. +++ b/core/modules/user/user.module
    @@ -255,9 +255,10 @@ function user_role_permissions(array $roles) {
     function _user_role_permissions_update($roles) {
    +  $entities = Role::loadMultiple($roles);
       $role_permissions = [];
       foreach ($roles as $rid) {
    -    $role_permissions[$rid] = \Drupal::config("user.role.$rid")->get('permissions') ?: [];
    +    $role_permissions[$rid] = $entities[$rid]->getPermissions() ?: [];
       }
       return $role_permissions;
     }
    

    This function shouldn't be changing at all. It will be reomved in Drupal 11.

  2. +++ b/core/tests/Drupal/Tests/Core/Session/PermissionsHashGeneratorTest.php
    @@ -252,9 +253,10 @@ public function testGenerateNoCache() {
     if (!function_exists('user_role_permissions')) {
     
       function user_role_permissions(array $roles) {
    +    $entities = Role::loadMultiple($roles);
         $role_permissions = [];
         foreach ($roles as $rid) {
    -      $role_permissions[$rid] = [];
    +        $role_permissions[$rid] = $entities[$rid]->getPermissions() ?: [];
         }
         return $role_permissions;
       }
    

    This code should be removed because we change the PermissionsHashGenerator

alexpott’s picture

Status: Needs work » Needs review
StatusFileSize
new6.59 KB

Here's the deprecations and the uses in core replaced. Now we need to add test coverage of the deprecated methods. To test the deprecation and to ensure we don't break them until they are removed.

No interdiff because of #4.

smustgrave’s picture

Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative
andypost’s picture

Status: Needs work » Needs review
StatusFileSize
new1.6 KB
new7.24 KB

fix failure

smustgrave’s picture

Status: Needs review » Needs work

Thanks @andypost!

Change looks good to me. Only moving to NW for the deprecation tests of the functions that were deprecated. per #5

andypost’s picture

Status: Needs work » Needs review
StatusFileSize
new2.73 KB
new9.18 KB

Added test

andypost’s picture

StatusFileSize
new1.28 KB
new9.17 KB

and clean-up

andypost’s picture

StatusFileSize
new1.07 KB
new9.17 KB

2 more typos

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Test coverage looks good.

alexpott’s picture

We need to complete the issue summary and change record.

andypost’s picture

Issue summary: View changes
Status: Reviewed & tested by the community » Needs review
Issue tags: -Needs issue summary update, -Needs change record updates
StatusFileSize
new1.17 KB
new9.15 KB

updated IS and CR also fixed nit in patch

smustgrave’s picture

Status: Needs review » Needs work

Wonder if we could provide examples in the CR for _user_role_permissions_update also please.

andypost’s picture

Status: Needs work » Needs review

Added to CR that _user_role_permissions_update() has no replacements (it was not API as function name starts with underscore) and config should not be read directly

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Thanks!

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 10.1.x, thanks! Nice to see constructor property promotion here.

catch’s picture

andypost’s picture

Looks not pushed and CR is not published

  • catch committed 0fadc153 on 10.1.x
    Issue #3348092 by andypost, alexpott, Ranjit1032002, smustgrave, catch:...
catch’s picture

Oops, now pushed and CR published.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.