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
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:
- 3582050-add-test-for
changes, plain diff MR !15262
Comments
Comment #2
prudloff commentedComment #3
prudloff commentedComment #5
prudloff commentedComment #6
berdirTechnically, 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.
Comment #7
borisson_One very small nitpick, but I think this is looking great.
Comment #9
andypostNice to see commited
Comment #10
godotislateA nit for whitespace, but also a question/request for more documentation on the MR.
Comment #11
berdirReplied in the MR.
Comment #15
godotislateCommitted and pushed b62f3ab to main and 1903248 to 11.x. Thanks!