Add filter site__user_in on wp site list
#438
Conversation
danielbachhuber
left a comment
There was a problem hiding this comment.
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.
We should present an empty list here, instead of erroring.
src/Site_Command.php
Outdated
| } | ||
|
|
||
| if ( isset( $assoc_args['site__user_in'] ) ) { | ||
| $user = get_user_by( 'login', $assoc_args['site__user_in'] ); |
There was a problem hiding this comment.
_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.
Il'll take a look at WP_CLI\Fetchers\User.
There was a problem hiding this comment.
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.
@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.
I needed to change the flag to
site_usersince 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 <daniel@bachhuber.co>
|
@marksabbath Are you still planning to finish this up? |
Yes! I'll make sure to spend some time this week. |
danielbachhuber
left a comment
There was a problem hiding this comment.
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