Problem/Motivation

The test was initially added to #2910353: Prevent saving config entities when configuration overrides are applied but we are trying to reduce the scope of this issue.
(It was historically about both detecting when configuration overrides are saved and fixing places where this happens.)
The test passes on main because the code that was incorrectly saving overrides has been fixed in another issue (probably #3497325: Config entity static cache is not cleared correctly when multiple language overrides are used) but the test still has value so we could commit it.

Steps to reproduce

The initial problem (now fixed) was that saving permissions could result in a translated role name being saved to the original untranslated config.
I don't have exact steps to reproduce because the bug has been fixed on main.

However it is possible to make the test fail with this change:

diff --git a/core/modules/user/user.module b/core/modules/user/user.module
index 53f0cdd5226..52b37c7265d 100644
--- a/core/modules/user/user.module
+++ b/core/modules/user/user.module
@@ -10,6 +10,7 @@
 use Drupal\Core\Session\AnonymousUserSession;
 use Drupal\Core\Site\Settings;
 use Drupal\Core\Url;
+use Drupal\user\Entity\Role;
 use Drupal\user\Entity\User;
 use Drupal\user\UserInterface;
 
@@ -465,8 +466,7 @@ function user_role_change_permissions($rid, array $permissions = []): void {
  */
 function user_role_grant_permissions($rid, array $permissions = []): void {
   // Grant new permissions for the role.
-  $role_storage = \Drupal::entityTypeManager()->getStorage('user_role');
-  if ($role = $role_storage->loadOverrideFree($rid)) {
+  if ($role = Role::load($rid)) {
     foreach ($permissions as $permission) {
       $role->grantPermission($permission);
     }
@@ -487,8 +487,7 @@ function user_role_grant_permissions($rid, array $permissions = []): void {
  */
 function user_role_revoke_permissions($rid, array $permissions = []): void {
   // Revoke permissions for the role.
-  $role_storage = \Drupal::entityTypeManager()->getStorage('user_role');
-  $role = $role_storage->loadOverrideFree($rid);
+  $role = Role::load($rid);
   foreach ($permissions as $permission) {
     $role->revokePermission($permission);
   }

(This is a revert from this commit: https://git.drupalcode.org/project/drupal/-/commit/30fb2bde8c479d9fd3be0...)

Proposed resolution

Add a functional test that checks if /admin/people/permissions works correctly with translated role labels.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3582050

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

prudloff created an issue. See original summary.

prudloff’s picture

Issue summary: View changes
prudloff’s picture

Issue summary: View changes

prudloff’s picture

Status: Active » Needs review
berdir’s picture

Technically, I wrote this test back in 2018 and it probably hasn't changed much since then, so not sure if I can RTBC it, but the MR looks good to me, so if someone else wants to give it a sanity check then it should be fine to be RTBC.

Even though the bug was fixed, I think it's useful to have some functional test coverage for this whole problem space, if we at some point manage to deprecate/disallow saving config entities without load without overrides first then it becomes kind of moot, but that's likely still a long time away.

borisson_’s picture

One very small nitpick, but I think this is looking great.

smustgrave made their first commit to this issue’s fork.

andypost’s picture

Status: Needs review » Reviewed & tested by the community

Nice to see commited

godotislate’s picture

Status: Reviewed & tested by the community » Needs work

A nit for whitespace, but also a question/request for more documentation on the MR.

berdir’s picture

Status: Needs work » Reviewed & tested by the community

Replied in the MR.

  • godotislate committed b62f3ab8 on main
    test: #3582050 Add test for role UI when using config translation
    
    By:...

  • godotislate committed 19032488 on 11.x
    test: #3582050 Add test for role UI when using config translation
    
    By:...
godotislate’s picture

Version: main » 11.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed b62f3ab to main and 1903248 to 11.x. Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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