Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
In #540008: Add a container parameter that can remove the special behavior of UID#1 an approach was taken where we can simply flag tests that are failing if we turn off user 1's super user powers, so that they can be taken care of in a followup. This issue is to collect all of these followups.
The goal is to have no tests in Drupal core that rely on UID1's special privileges so that we:
- Know these tests are correctly assigning the necessary permissions to run
- Can turn off the super user access policy in D11, knowing it won't break core
- Can remove the super user access policy in D12, providing an admin account recovery tool to replace it
Steps to reproduce
Go into any of the tests flagged with:
/**
* {@inheritdoc}
*
* @todo Remove and fix test to not rely on super user.
* @see https://www.drupal.org/project/drupal/issues/3437620
*/
And:
- Remove the code below that sets the usesSuperUserAccessPolicy to TRUE.
- Run the test to see which test methods are failing
Proposed resolution
Assign the right permissions to make the test go green without the super user access policy. Those few tests that specifically test said policy can obviously stay, but will be removed along with the policy in D12.
Remaining tasks
- core/modules/content_moderation/tests/src/
- Functional/ModerationContentTranslationTest.php
- Functional/ModerationFormTest.php
- Functional/ModerationLocaleTest.php
- Functional/ModerationStateBlockTest.php
- Functional/WorkspaceContentModerationIntegrationTest.php
- Kernel/EntityStateChangeValidationTest.php
Issue fork drupal-3439832
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:
changes
Comments
Comment #2
vensiresComment #3
quietone commentedThe issue title reads like this will be fixing all the tests, when it really is just those in content moderation. Maybe someth8ing like, "Fix content_moderation tests that rely on UID1's super user behavior"
Comment #4
vensiresNice catch! Just changed. Will change the other issues too. Thanks for the remark!
Comment #5
vensiresComment #13
pooja_sharma commentedAddressed the superuser code needs to remove & added respective permissions.
Facing issue with existing MR, so created new branch , it also have conflicts , so fixed those & raise the MR.
Please review MR 8643, move NR
Comment #14
pooja_sharma commentedComment #17
smustgrave commentedLeft some comments on the MR.
Comment #18
pooja_sharma commentedI have replied on the queries, Please review & passing for NR.
Comment #19
smustgrave commentedSaving credit for those who have worked on the issue thus far.
Left a response to the thread.
Comment #20
pooja_sharma commented@smustgrave, I can't see new comment on MR, on the basis of prev comment , I 'm trying to reply on query:
Goal of ticket is adding permissions only, however permission can be assign to user only if module install code added before user login, if module install & respective config install (if required/added) code added after user login code in that case scenario permission can be assign only by adding role (checked in core as well)
PLease review, moving NR
Comment #21
smustgrave commentedInstead of creating a bunch of roles for single permissions
Why not move $this->setCurrentUser($this->adminUser); to below when the workflow is created and change to $this->setCurrentUser($this->createUser(['single permissiono']));
Should result in less calls being made.
Comment #22
pooja_sharma commentedThanks for reviewing, tried to address the feedback, Please review, moving NR.
Comment #23
smustgrave commentedHad some threads I forgot to save from earlier that's my mistake.
Comment #24
pooja_sharma commentedYeah I have similar thoughts, u didn't mention similar changes here then also skip from my end as well.
Applied suggestions, PLease review, moving NR.
Comment #25
smustgrave commentedFeedback appears to have been addressed
Comment #26
quietone commentedThe diff does not apply to 11.x HEAD. And while does that I think it would make it easier to maintain if all the lists of permissions were sorted. Some of these in this MR are quite long. Thanks.
Comment #27
quietone commentedI don't know why but the diff applies now. The only commands I executed in my working directory was 'git log' and 'git status' . I'm restoring the status while I review.
Comment #28
quietone commentedFour of the functional tests extend from \Drupal\Tests\content_moderation\Functional\ModerationStateTestBase which defines a user and many permissions. Why are these four tests creating a new user with different permissions? Why not use the existing user and grant any permissions needed? It would simplify the tests.
Comment #29
quietone commentedOh, and a reminder that we work collaboratively. Typically, there is no need to create a new branch and separate MR. Thanks.
Comment #30
pooja_sharma commentedThanks for reviewing it, Verifying to address #28 feedback.
Comment #31
pooja_sharma commentedAddressed #28 feedback, sorry it was missed from my end, now code looks clean & tried to compare the old & changed generated test html files output - both are same.
Please review, moving NR.
Comment #32
smustgrave commentedFeedback appears to be addressed.
Comment #33
alexpottCommitted and pushed 56186f9070b to 11.x and 743c2e68f70 to 11.1.x. Thanks!