-
Notifications
You must be signed in to change notification settings - Fork 67
Introduce wp db users command for user management #303
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
…utput.txt Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces a new wp db users command to simplify database user management tasks, specifically adding the ability to create MySQL user accounts with optional privileges on the WordPress database.
Changes:
- Added
DB_Users_Commandclass extendingDB_Commandwith acreatesubcommand for creating MySQL users - Modified
DB_Commandto makeesc_sql_identmethod protected for reuse in child classes - Added comprehensive Behat feature tests covering various user creation scenarios
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/DB_Users_Command.php | Implements the new DB_Users_Command class with create subcommand, SQL escaping methods, and user/privilege management |
| src/DB_Command.php | Changes visibility of esc_sql_ident from private to protected to allow child class access |
| phpcs.xml.dist | Excludes DB_Users_Command from WordPress prefix naming rule for consistency with DB_Command |
| features/db-users.feature | Adds comprehensive test scenarios for user creation with/without privileges and various configurations |
| db-command.php | Registers the new 'db users' command with WP-CLI |
| composer.json | Adds 'db users' and 'db users create' to the list of available commands |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a new wp db users create command for managing MySQL users, which is a valuable addition to WP-CLI's database management capabilities. The implementation is well-structured, extending the existing DB_Command and reusing its functionality where appropriate. Security has been carefully considered, with proper SQL escaping for user-provided identifiers and passwords. The new command is also well-tested with Behat scenarios covering various use cases. My review includes a couple of minor suggestions to improve code clarity by removing redundant PHPDoc comments. Overall, this is a high-quality contribution.
| /** @var string $username_escaped */ | ||
| /** @var string $host_escaped */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These @var annotations are redundant. The self::esc_sql_ident() method's PHPDoc already specifies that it returns a string when a string is passed as an argument. Modern static analysis tools can infer the types of $username_escaped and $host_escaped correctly without these comments. Removing them will make the code cleaner.
| if ( $grant_privileges ) { | ||
| $database = DB_NAME; | ||
| $database_escaped = self::esc_sql_ident( $database ); | ||
| /** @var string $database_escaped */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implementation Plan
Summary
Implemented
wp db users createcommand that allows database administrators to easily create MySQL user accounts with optional privileges on the WordPress database. The implementation extendsDB_Commandfor consistency, follows WP-CLI patterns, and includes comprehensive tests. Latest changes remove assert() calls and improve SQL string escaping to follow MySQL's documented string literal escaping rules, handling special characters like null bytes, newlines, carriage returns, quotes, and control characters.Original prompt
wp db users#138💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.