-
Notifications
You must be signed in to change notification settings - Fork 91
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
Add filter site__user_in
on wp site list
#438
Conversation
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.
Thanks for working on this, @marksabbath ! A few things to fix up.
src/Site_Command.php
Outdated
} | ||
|
||
if ( ! isset( $where['blog_id'] ) ) { | ||
WP_CLI::error( 'Could not find a site with the user provided.' ); |
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.
We should present an empty list here, instead of erroring.
src/Site_Command.php
Outdated
@@ -607,6 +610,24 @@ public function list_( $args, $assoc_args ) { | |||
$where['site_id'] = $assoc_args['network']; | |||
} | |||
|
|||
if ( isset( $assoc_args['site__user_in'] ) ) { | |||
$user = get_user_by( 'login', $assoc_args['site__user_in'] ); |
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.
_in
implies the argument supports a CSV of users. Should we accommodate that here?
Also, we should use WP_CLI\Fetchers\User
instead of directly calling get_user_by()
. It will handle the "invalid user" case for us.
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.
Il'll take a look at WP_CLI\Fetchers\User
.
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.
I've updated the code to use WP_CLI\Fetchers\User
and also updated the name of the flag to --user
.
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.
@danielbachhuber I needed to change the flag to site_user
since it was not properly working with user
. I guess it conflicts with some global flag or something.
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.
I needed to change the flag to
site_user
since it was not properly working withuser
. I guess it conflicts with some global flag or something.
@marksabbath Yep, it does. site_user
is a fine alternative.
Co-authored-by: Daniel Bachhuber <[email protected]>
@marksabbath Are you still planning to finish this up? |
Yes! I'll make sure to spend some time this week. |
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.
Looks great, @marksabbath ! Thanks for your work on this.
I added another test case with b016207
site__user_in
on wp site list
This is a HackDay PR 🎉
This PR should cover wp-cli/ideas#153