Fix: Resolve admin user dynamically instead of hardcoding user ID 1#6270
Conversation
Replace hardcoded user ID 1 fallback in Admin context with smart user resolution. On multisite, queries get_super_admins() to find a valid super admin. On single site, queries for the first user with the administrator role. Emits a clear error if no suitable user is found. Fixes wp-cli#6269
|
Hello! 👋 Thanks for opening this pull request! Please check out our contributing guidelines. We appreciate you taking the initiative to contribute to this project. Contributing isn't limited to just code. We encourage you to contribute in the way that best fits your abilities, by writing tutorials, giving a demo at your local meetup, helping other users with their support questions, or revising our documentation. Here are some useful Composer commands to get you started:
To run a single Behat test, you can use the following command: # Run all tests in a single file
composer behat features/some-feature.feature
# Run only a specific scenario (where 123 is the line number of the "Scenario:" title)
composer behat features/some-feature.feature:123You can find a list of all available Behat steps in our handbook. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request does a great job of removing the hardcoded admin user ID 1 and replacing it with a dynamic lookup. The new find_admin_user_id() method correctly handles both single-site and multisite installations, with clear error messages when no suitable user can be found. The added Behat tests cover the main scenarios for this new logic. I have a couple of suggestions to further improve the code: one for performance in the user lookup on multisite, and another to add a test case for the error condition on single-site installations.
Use get_users() with login__in for single DB query instead of looping get_user_by(). Add single-site error case test. Replace wp super-admin commands with direct option manipulation in tests.
Use single quotes for strings per PHPCS rules. Fix gherkin use-and lint violations by replacing consecutive When steps with And.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request effectively addresses the issue of a hardcoded admin user ID by implementing dynamic user resolution for the admin context. The logic correctly handles both single-site and multisite installations, with clear error messages for cases where a suitable admin user cannot be found. The addition of comprehensive Behat tests ensures the new functionality is well-covered. I have one suggestion to slightly simplify the code for finding a super admin on multisite installations.
There was a problem hiding this comment.
Pull request overview
This PR updates the Admin context user resolution logic to avoid hardcoding user ID 1, selecting an appropriate admin user dynamically (super admin on multisite; administrator on single-site) and adding Behat coverage for the new behavior.
Changes:
- Replaces the Admin context fallback user selection with
find_admin_user_id()resolution logic. - Adds error messages when no suitable admin user can be found for the current environment.
- Adds new Behat scenarios to cover multisite/single-site resolution and error paths.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| php/WP_CLI/Context/Admin.php | Introduces dynamic admin user resolution logic (super admin / administrator) instead of hardcoded ID 1. |
| features/context.feature | Adds Behat scenarios intended to cover the new resolution behavior and error handling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Revert multisite super admin lookup from get_users()
back to foreach + get_user_by('login') loop because
get_users() only fetches users on the current site
but a super admin might not be a member of any site.
Add debug logging after resolving admin user ID.
…e phar-bundled versions (wp-cli#6218) Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Pascal Birchler <pascalb@google.com>
…6169) Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Pascal Birchler <pascalb@google.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Pascal Birchler <pascalb@google.com> Co-authored-by: Pascal Birchler <pascal.birchler@gmail.com>
* Initial plan * Add WP-CLI handbook URL reference to wp help output Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
… config key (wp-cli#6274) * Initial plan * Fix SSH alias path not forwarded to remote when path is a separate config key Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
* Initial plan * Fix vagrant SSH strict host key checking failure When ssh: vagrant is configured and vagrant ssh-config is parsed, add -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null to the SSH command to match what vagrant itself sets. This prevents failures when the vagrant VM has been recreated and has a different host key than what is stored in ~/.ssh/known_hosts. Adds a Behat test scenario to verify the fix. Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Apply suggestion from @swissspidy --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> Co-authored-by: Pascal Birchler <pascalb@google.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…e()` (wp-cli#6276) * Initial plan * Add optional $newline parameter to WP_CLI::log(), WP_CLI::line(), and logger info() methods Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> * Undo Base class change --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> Co-authored-by: Pascal Birchler <pascalb@google.com>
Summary
$admin_user_id = 1fallback in Admin context with dynamic user resolutionget_super_admins()viaget_users()withlogin__in(single DB query) to find a valid super adminadministratorroleFixes #6269
Note
This PR was authored with the help of Claude Code (claude-opus-4-6).
Test plan
wp plugin listworks on multisite where user ID 1 is not a super adminwp --context=admin eval ''works on single site (resolves administrator)features/context.feature)