Issue #3346953 by alexpott, acbramley, GoZ, larowlan, bircher: Role...

Issue #3346953 by alexpott, acbramley, GoZ, larowlan, bircher: Role permissions are not sorted when saving via admin/people/permissions

core/modules/user/src/Entity/Role.php

@@ -185,6 +185,13 @@ public function preSave(EntityStorageInterface $storage) {
       });
       $this->weight = $max + 1;
     }
+
+    if (!$this->isSyncing() && $this->hasTrustedData()) {
+      // Permissions are always ordered alphabetically to avoid conflicts in the
+      // exported configuration. If the save is not trusted then the
+      // configuration will be sorted by StorableConfigBase.
+      sort($this->permissions);
+    }
   }
 
   /**

core/modules/user/tests/src/Functional/UserPermissionsAdminTest.php

+<?php
+
+namespace Drupal\Tests\user\Functional;
+
+use Drupal\Tests\BrowserTestBase;
+use Drupal\user\Entity\Role;
+
+/**
+ * Tests adding and removing permissions via the UI.
+ *
+ * @group user
+ */
+class UserPermissionsAdminTest extends BrowserTestBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  protected $defaultTheme = 'stark';
+
+  /**
+   * Tests granting and revoking permissions via the UI sorts permissions.
+   */
+  public function testPermissionsSorting() {
+    $role = Role::create(['id' => 'test_role', 'label' => 'Test role']);
+    // Start the role with a permission that is near the end of the alphabet.
+    $role->grantPermission('view user email addresses');
+    $role->save();
+
+    $this->drupalLogin($this->drupalCreateUser([
+      'administer permissions',
+    ]));
+    $this->drupalGet('admin/people/permissions');
+
+    $this->assertSession()->statusCodeEquals(200);
+
+    // Add a permission that is near the start of the alphabet.
+    $this->submitForm([
+      'test_role[change own username]' => 1,
+    ], 'Save permissions');
+
+    // Check that permissions are sorted alphabetically.
+    $storage = \Drupal::entityTypeManager()->getStorage('user_role');
+    /** @var \Drupal\user\Entity\Role $role */
+    $role = $storage->loadUnchanged($role->id());
+    $this->assertEquals([
+      'change own username',
+      'view user email addresses',
+    ], $role->getPermissions());
+
+    // Remove the first permission, resulting in a single permission in the first
+    // key of the array.
+    $this->submitForm([
+      'test_role[change own username]' => 0,
+    ], 'Save permissions');
+    /** @var \Drupal\user\Entity\Role $role */
+    $role = $storage->loadUnchanged($role->id());
+    $this->assertEquals([
+      'view user email addresses',
+    ], $role->getPermissions());
+  }
+
+}

core/modules/user/user.post_update.php

@@ -19,9 +19,15 @@ function user_removed_post_updates() {
 }
 
 /**
- * Ensure permissions stored in role configuration are sorted using the schema.
+ * No-op update.
  */
 function user_post_update_sort_permissions(&$sandbox = NULL) {
+}
+
+/**
+ * Ensure permissions stored in role configuration are sorted using the schema.
+ */
+function user_post_update_sort_permissions_again(&$sandbox = NULL) {
   \Drupal::classResolver(ConfigEntityUpdater::class)->update($sandbox, 'user_role', function (Role $role) {
     $permissions = $role->getPermissions();
     sort($permissions);