Skip to content

Tests: Cover user role changes and user search in admin E2E - #15779

Closed
shail-mehta wants to merge 4 commits into
WordPress:trunkfrom
shail-mehta:tests/52895-users-role-search-e2e
Closed

shail-mehta wants to merge 4 commits into
WordPress:trunkfrom
shail-mehta:tests/52895-users-role-search-e2e

Conversation

@shail-mehta

Copy link
Copy Markdown
Member

Trac ticket: #52895

Adds remaining Users e2e coverage from ticket #52895.

What

  • Changes another user’s role from the user edit screen and asserts it persists in the UI and via REST
  • Confirms an administrator cannot change their own role to Subscriber (no Role field on Profile; bulk Change shows the error notice and role is unchanged)
  • Searches for an existing user and asserts the row is listed
  • Searches for a missing user and asserts the empty-state message (with a non-matching fixture so ignoring search cannot pass)

Why

Ticket #52895 lists Users coverage for changing roles, preventing an admin from demoting itself, and searching for users. Existing users e2e covered create/login and bulk delete, but not these cases.

How

Extends tests/e2e/specs/users.test.js using the same create/cleanup helpers. Role is found by label; top bulk role controls keep their IDs because top/bottom share an accessible name (r65275).

Testing instructions

  1. Start the local environment if needed: npm run env:start
  2. Run: npm run test:e2e -- tests/e2e/specs/users.test.js
  3. Confirm the new role-change, self-demotion, and search tests pass

Use of AI Tools

  • Cursor

Add coverage for editing another user's role, blocking self-demotion to subscriber, and searching for existing and missing users.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The test can fail when the administrator is beyond the first page and, if the self-demotion guard regresses, can demote the shared administrator, breaking cleanup and other tests.

2 open issues
What changed in this PR

Extends the admin Users end-to-end tests to cover role management and user search, addressing remaining Users scenarios from Trac #52895.

Changes:

  • Tests role changes through the edit screen, checking persistence in the UI and REST API.
  • Checks that administrators cannot demote themselves through profile or bulk role controls.
  • Covers searches for existing and missing users, including the empty-state message.
File Description
tests/​e2e/​specs/​users.test.js Adds four role-management and user-search tests using existing fixture and cleanup helpers.

Review effort: Balanced

Comment thread tests/e2e/specs/users.test.js Resolved
Comment thread tests/e2e/specs/users.test.js Resolved
Run the own-role change check as a disposable administrator in an isolated browser context, and filter the Users list by username before selecting the row.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

There are no active review issues for the added user-management end-to-end coverage.

No open issues 🎉

2 resolved since last review

Review effort: Balanced

@shail-mehta
shail-mehta marked this pull request as ready for review November 7, 2026 15:05
@github-actions

github-actions Bot commented Nov 7, 2026 •

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs.

Core Committers: Use this line as a base for the props when committing in SVN:

Props shailu25, lancewillett.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.


Props Bot commands

You can trigger Props Bot actions by commenting on this PR:

  • @propsbot add <username> to give props to a GitHub user. Requires write access.
  • @propsbot remove <username> to leave a user out of the props. Requires write access.
  • @propsbot refresh to refresh this list. Adding the props-bot label works too.

@shail-mehta shail-mehta self-assigned this Nov 7, 2026

@lancewillett lancewillett left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Two of these four are worth keeping, and two I'd drop. CI is green at 06eec79 with no retries (parallel 41, serial 72, both jobs).

Keep:

  • Self-demotion via bulk Change role. The guard in wp-admin/users.php (the err_admin_role branch of the promote action) has no other test. Tests_User::test_edit_user_role_update() covers edit_user(), not this bulk path. If the guard goes, the role changes and the "Changed roles." notice appears, so this fails. Locating the bulk controls and notice by ID is fine.
  • Empty search for a missing user. With a non-matching user present, it fails if s is ignored.

Drop:

  • Changing another user's role from the edit screen. Tests_User::test_edit_user_role_update() (#42564) already promotes a user through edit_user(), which is what this form posts to.
  • Searching for an existing user. If search is ignored, the row still appears, since the test site has far fewer than a page of users. So this can't catch a search regression; the empty-result test does that.

One small thing in the self-demotion test: await expect( usernameField ).toBeFocused(); checks login autofocus, which isn't what the test is about. I'd remove it so the test can only fail on the role guard.

AI review · claude-opus-5-5

Drop edit-screen role change and existing-user search cases already covered elsewhere, and remove the unrelated login autofocus assert from the self-demotion test.
@shail-mehta

Copy link
Copy Markdown
Member Author

Thanks! Addressed in b891c75

kept the self-demotion bulk Change role and empty missing-user search tests, dropped the edit-screen role change and existing-user search cases, and removed the login autofocus assert from the self-demotion test.

@lancewillett lancewillett left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this is what I was after. Re-checked at b891c75: the PR now adds just the self-demotion and empty-search tests, Prettier passes, and it applies cleanly to trunk.

CI is green (parallel 45, serial 74). One parallel job retried general-settings-invalid-timezone.test.js (the "" case), a known retry that this PR doesn't touch.

Ready to land.

AI review · claude-opus-5-5

wporg-sync pushed a commit that referenced this pull request Nov 7, 2026
Log in as a second administrator and try to change their own role to Subscriber with the bulk Change role control on the Users screen. Check for the error notice and that the role is unchanged. Search users for a missing name and check for the empty result, with another user present so the test fails if search is ignored.

Developed in: #15779

Props shailu25.
See #52895.


git-svn-id: https://develop.svn.wordpress.org/trunk@65334 602fd350-edb4-49c9-b593-d223f7449a82
@lancewillett

Copy link
Copy Markdown
Member

wporg-sync pushed a commit to WordPress/WordPress that referenced this pull request Nov 7, 2026
Log in as a second administrator and try to change their own role to Subscriber with the bulk Change role control on the Users screen. Check for the error notice and that the role is unchanged. Search users for a missing name and check for the empty result, with another user present so the test fails if search is ignored.

Developed in: WordPress/wordpress-develop#15779

Props shailu25.
See #52895.

Built from https://develop.svn.wordpress.org/trunk@65334


git-svn-id: http://core.svn.wordpress.org/trunk@64474 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants