Add wp user update_meta #117

Closed
wants to merge 8 commits into
from

Conversation

2 participants
Contributor

mwilliamson-red-gate commented May 31, 2012

Adds the command wp user update_meta to allow meta fields on a user to be updated.

@scribu scribu commented on an outdated diff May 31, 2012

src/php/wp-cli/commands/internals/user.php
+ WP_CLI::success( "Updated user $user_id." );
+ } else {
+ WP_CLI::error( "Failed to update meta field" );
+ }
+ }
+
+ private function get_numeric_arg_or_error( $args, $index, $name ) {
+ $value = self::get_arg_or_error( $args, $index, $name );
+ if ( ! is_numeric( $value ) ) {
+ self::error_see_help( "$name must be numeric" );
+ }
+ return $value;
+ }
+
+ private function get_arg_or_error( $args, $index, $name ) {
+ $value = $args[$index];
@scribu

scribu May 31, 2012

Owner

This will produce a notice. Should use isset().

Owner

scribu commented May 31, 2012

How about instead of update_meta, we use meta set?

wp user meta set 123 key value

It would be more consistent with wp option set key value.

Contributor

mwilliamson-red-gate commented May 31, 2012

wp user meta set sounds good to me, but is there a straightforward way to add a sub-sub-command? (Where user is the command, meta is the sub-command, and set is the sub-sub-command)

Owner

scribu commented May 31, 2012

You can treat 'set' as just another parameter to the 'meta' sub-command.

The alternative would be to introduce a user-meta command:

wp user-meta set 123 key value

wp user-meta get 123 key

In this case, we could have a helper class which we could reuse for post-meta etc.

Contributor

mwilliamson-red-gate commented May 31, 2012

You can treat 'set' as just another parameter to the 'meta' sub-command.

True, but then you don't get the nice error messages for free when you leave off the sub-sub-command, like you do when you leave off a sub-command. You'd have to implement those messages yourself, which doesn't seem like a great solution.

Adding a user-meta command feels slightly neater in terms of implementation, but not quite as nice when you're calling code. Might be the simplest solution though.

Owner

scribu commented May 31, 2012

Even if we implemented sub-sub-commands, we'd still likely need them to be in a separate class, so it's worth having user-meta as a first step.

scribu commented on 9e685e2 May 31, 2012

What does this change fix?

What does this change fix?

wp core install. Before the change, WP_CLI::_set_url would be called, causing the url parameter to be unset. If we were running wp core install, we'd then check that the url parameter was set, which would always be false, causing an early exit with an error code.

Ok, but this doesn't seem related to #117 at all. More like with #115.

Also, wp core install does not need to do any further handling for --url parameter. Like I explained in #115, it relies on wp_guess_url().

Ok, but this doesn't seem related to #117 at all. More like with #115.

It's unrelated to either (my fault for using the same branch). Before this change, even if you run wp core install with an url parameter, the check for the url parameter will always fail. Either it was never set, or it was set, in which case it promptly gets unset by WP_CLI::_set_url before the check for the url parameter.

Oh, I think this was caused while fixing wp-cli/wp-cli#103

Should be fixed now: wp-cli/wp-cli@a02a3f9

@scribu scribu added a commit that referenced this pull request Jun 1, 2012

@scribu scribu generate man page. see #117 5a69b91

@scribu scribu added a commit that referenced this pull request Jun 1, 2012

@scribu scribu add user-meta get. see #117 33fb1c6

@scribu scribu added a commit that referenced this pull request Jun 1, 2012

@scribu scribu Complete user-meta command:
* rename 'user-meta set' to 'user-meta update'
* add 'user-meta add' and 'user-meta delete'

See #117
94c2cde
Owner

scribu commented Jun 1, 2012

I merged everything except the --url commit.

scribu closed this Jun 1, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment