Make WordPress Core

Opened 11 years ago

Last modified 3 years ago

#22921 new enhancement

Allow get_users() to return array of values via 'fields' parameter

Reported by: chipbennett's profile chipbennett Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.5
Component: Users Keywords: has-patch needs-testing dev-feedback needs-refresh
Focuses: Cc:

Description

Currently, the 'fields' parameter in the arguments array for get_users() only accepts either an array of user data fields, or the string 'all'. Both of these options cause get_users() to return an array of object comprised of the specified user data. Passing any other string value to 'fields' causes get_users() to return an array of user IDs.

Per the Codex:

Array of objects, except when fields specifies a single field to be returned, then an array of values is returned. If fields is set to all_with_meta, it will return an array of WP_User objects.

This statement appears to be untrue, but would be incredibly useful if it were true.

Patch adds this functionality to get_users().

Potentially related: #18581

Attachments (1)

user.php.diff (652 bytes) - added by chipbennett 11 years ago.

Download all attachments as: .zip

Change History (19)

#1 @scribu
11 years ago

Example usage?

#2 @chipbennett
11 years ago

Return an array of user email addresses, such as:

$user_emails = get_users( array( 'fields' => 'user_email' ) );

This usage eliminates the step of looping through the array of objects otherwise returned, to pluck out the email addresses.

#3 @DavidAnderson
11 years ago

-1 from me.

I also had to waste some time when dealing with this function, and the difference between Codex and the actual behaviour.

However, once I worked the actual behaviour out, I wrote my code to deal with that reality. If you change the behaviour, you'll break lots of existing code, and people will be confused/annoyed when their sites break and they have to re-write. Better solution is to change Codex to describe the current real behaviour.

#4 follow-up: @dd32
11 years ago

If you change the behaviour, you'll break lots of existing code

If you take a look at the patch, the only code that will break, are those passing the fields parameter as one of the following, and expecting back a list of user_id's, a broken assumption and code to start with..
array( 'ID', 'user_login', 'user_pass', 'user_nicename', 'user_email', 'user_url', 'user_registered', 'user_activation_key', 'user_status', 'display_name' )

Last edited 11 years ago by dd32 (previous) (diff)

#5 in reply to: ↑ 4 ; follow-up: @DavidAnderson
11 years ago

dd32: Yes, that's right - it'll break code that uses the existing function. I've written such code. The existing function does something useful, and works whilst doing it, and people are using it. There's a real-world cost to breaking that.

#6 in reply to: ↑ 5 ; follow-up: @chipbennett
11 years ago

Replying to DavidAnderson:

dd32: Yes, that's right - it'll break code that uses the existing function. I've written such code. The existing function does something useful, and works whilst doing it, and people are using it. There's a real-world cost to breaking that.

Let's consider the purpose of the 'fields' array parameter: to return only the fields specified, instead of all user data. The code provides two forms of output, based on the type of input:

  1. An array of objects comprised of one or more values, if 'fields' => array( $fields ) is passed
  2. An array of values, if 'fields' => $field is passed

Thus, the only way this patch would break existing code is if someone passed a field other than 'ID' for 'fields', e.g.:

$user_IDs = get_users( array( 'fields' => 'user_email' ) );

...and then used the output as if an array of IDs was the expected return value.

That just doesn't make logical sense. Why would someone pass 'user_email' as the value for 'fields', and then subsequently use the output as if 'ID' had been passed?

Any normal developer would look at that output and do one of two things:

  1. Change the function call to 'fields' => 'ID', if the desired return value is 'ID'
  2. Change the function call to pass 'fields' => array( 'user_email' ) if the desired return value is 'user_email'

I contend that if a developer leaves the code as-is, the user code is at fault, and such a use case shouldn't be something that keeps the core function itself from being improved.

#7 in reply to: ↑ 6 ; follow-up: @DavidAnderson
11 years ago

Aah - gotcha. Sorry, I was confusing myself before. You are completely right ...

... up to a point (unless I'm still confused!).

What you desire can already be done, using this syntax (which I use in the "Use Admin Password" plugin):

$desired_fields = get_users("fields[]=email");

So I think we just need to change Codex to give an accurate description, rather than re-add the same feature?

Last edited 11 years ago by DavidAnderson (previous) (diff)

#8 in reply to: ↑ 7 ; follow-ups: @chipbennett
11 years ago

Replying to DavidAnderson:

Aah - gotcha. Sorry, I was confusing myself before. You are completely right ...

... up to a point (unless I'm still confused!).

What you desire can already be done, using this syntax (which I use in the "Use Admin Password" plugin):

$desired_fields = get_users("fields[]=email");

So I think we just need to change Codex to give an accurate description, rather than re-add the same feature?

Is that any different than:

$desired_fields = get_users( array( 'fields' => array( 'user_email' ) ) );

If so, then that still doesn't return the desired output. That returns an array of objects, each of which contains one entity, 'user_email'.

So, when 'ID' is passed as the single field, the output is an array of user IDs; but if any other field is passed as the single field, an array of objects is returned, since the only possible way to pass a single field other than ID is by passing 'fields' as an array.

So, user IDs can be returned as a simple array, like so:

$user_IDs = get_users( array( 'fields' => 'ID' ) );

$user IDs = array(
    [0] => '1',
    [1] => '2',
    [2] => '3'
);

But to get a single-field value for any other field, you can only get an array of objects:

$user_emails = get_users( array( 'fields' => array( 'user_email' ) ) );

$user_emails = array(
    [0] => object( 'user_email' => 'email@...' ),
    [1] => object( 'user_email' => 'email@...' ),
    [2] => object( 'user_email' => 'email@...' )
);

The purpose of this ticket is to allow for consistent return for single-field values for fields other than 'ID'; e.g. so that the following return is possible:

$user_IDs = get_users( array( 'fields' => 'email' ) );

$user IDs = array(
    [0] => 'email@...',
    [1] => 'email@...',
    [2] => 'email@...'
);

Currently this return is not possible, requiring a wasteful, second step to loop through the array of objects to pluck the field values.

#9 in reply to: ↑ 8 @DavidAnderson
11 years ago

OK... but I'm still missing something... the original bug report spoke of adding something "incredibly useful"... but the difference is just that you can get back an array of strings instead of an array of objects (with each object containing the string as a property)? I feel entirely indifferent to that change and leave it to others better qualified to judge on what would be usual WP practice on such things.

#10 @nacin
11 years ago

  • Milestone changed from Awaiting Review to 3.6

3.6 for review.

#11 @ryan
11 years ago

  • Milestone changed from 3.6 to Future Release

#12 in reply to: ↑ 8 @akforsyt
10 years ago

Replying to chipbennett:

Replying to DavidAnderson:

Aah - gotcha. Sorry, I was confusing myself before. You are completely right ...

... up to a point (unless I'm still confused!).

What you desire can already be done, using this syntax (which I use in the "Use Admin Password" plugin):

$desired_fields = get_users("fields[]=email");

So I think we just need to change Codex to give an accurate description, rather than re-add the same feature?

Is that any different than:

$desired_fields = get_users( array( 'fields' => array( 'user_email' ) ) );

If so, then that still doesn't return the desired output. That returns an array of objects, each of which contains one entity, 'user_email'.

So, when 'ID' is passed as the single field, the output is an array of user IDs; but if any other field is passed as the single field, an array of objects is returned, since the only possible way to pass a single field other than ID is by passing 'fields' as an array.

So, user IDs can be returned as a simple array, like so:

$user_IDs = get_users( array( 'fields' => 'ID' ) );

$user IDs = array(
    [0] => '1',
    [1] => '2',
    [2] => '3'
);

But to get a single-field value for any other field, you can only get an array of objects:

$user_emails = get_users( array( 'fields' => array( 'user_email' ) ) );

$user_emails = array(
    [0] => object( 'user_email' => 'email@...' ),
    [1] => object( 'user_email' => 'email@...' ),
    [2] => object( 'user_email' => 'email@...' )
);

The purpose of this ticket is to allow for consistent return for single-field values for fields other than 'ID'; e.g. so that the following return is possible:

$user_IDs = get_users( array( 'fields' => 'email' ) );

$user IDs = array(
    [0] => 'email@...',
    [1] => 'email@...',
    [2] => 'email@...'
);

Currently this return is not possible, requiring a wasteful, second step to loop through the array of objects to pluck the field values.

Thank you for providing this fix. This was driving me crazy as the codex says it returns an array of values, but this is simply not true for any field except for user id. There is not much discussion on this, although I do agree, it is very useful.

#13 follow-ups: @mbcreation
10 years ago

Any update on this. (since the Codex is still wrong) ?
Thanks.

#14 in reply to: ↑ 13 @uuf6429
10 years ago

Wordpress 3.9 and 4.0 still has this issue and the Codex is still wrong.

Last edited 10 years ago by uuf6429 (previous) (diff)

#15 in reply to: ↑ 13 ; follow-up: @DrewAPicture
10 years ago

  • Keywords dev-feedback added

Replying to mbcreation:

Any update on this. (since the Codex is still wrong) ?
Thanks.

The Codex articles for get_users() and WP_User_Query have been updated to reflect that either can return an array of IDs, stdClass objects or WP_User objects depending on the value of the fields parameter.

As for the purpose of the ticket, the patch as-is would constitute a breaking change. I think we need a lot more data on whether plugin devs are using the fields argument in this way, if so how many and how. We also need to determine whether that usage constitutes "doing it wrong" as we've unfortunately been pigeon-holing all individual fields as 'ID' this whole time.

In the absence of more data, it seems like a good alternative might be to consider a new output argument (a la get_post()'s second parameter) that would set the format of the return container (an array of arrays for ARRAY_A, an array of objects for OBJECT (default)).

#16 in reply to: ↑ 15 @chipbennett
10 years ago

Replying to DrewAPicture:

As for the purpose of the ticket, the patch as-is would constitute a breaking change. I think we need a lot more data on whether plugin devs are using the fields argument in this way, if so how many and how. We also need to determine whether that usage constitutes "doing it wrong" as we've unfortunately been pigeon-holing all individual fields as 'ID' this whole time.

Theoretical issues should not constitute blockers for improvement. Are there any live, real-world examples of Plugins for which this patch would cause breakage? If so, your assessment may have merit. Even so, I refer you back to @dd32's comment above:

If you take a look at the patch, the only code that will break, are those passing the fields parameter as one of the following, and expecting back a list of user_id's, a broken assumption and code to start with.

array( 'ID', 'user_login', 'user_pass', 'user_nicename', 'user_email', 'user_url', 'user_registered', 'user_activation_key', 'user_status', 'display_name' )

I would agree with @dd32, and would argue that such breakage would indeed constitute _doing_it_wrong(). Querying for e.g. 'user_email' should not reasonably be expected to return 'ID'.

Changing the parameters, or the output, of the function would almost certainly be overkill here, and would potentially introduce far more compatibility/breakage issues than would the proposed patch.

#17 @chriscct7
8 years ago

  • Keywords needs-refresh added

#18 @lucasbustamante
3 years ago

+1, I just ran this query and got an array of user IDs, which is counter-intuitive:

<?php
$adminEmails = get_users([
    'role' => 'administrator',
    'fields' => 'user_email',
]);

// Result: [ '1' ]
// Where `1` is the ID of the admin user.

If instead I query:

<?php
$adminEmails = get_users([
    'role' => 'administrator',
    'fields' => [
        'user_email'
    ],
]);

// I get an array of stdClass objects:

// Result: [ (object) [ 'user_email' => 'dev@wp-foo.com' ]

// So to get the e-mail I need to:
// $adminEmails[0]->user_email === 'dev@wp-foo.com';

Which is essentially the same problem as running get_post_meta() with the single true or false. I expect a single result, but instead I get a collection of results that I need to parse afterwards.

So my suggestion is that:

<?php
$adminEmails = get_users([
    'role' => 'administrator',
    'fields' => 'user_email',
]);

// Would return an array of single results: ['dev@wp-foo.com']

While keeping the query by array the same as it is today, to keep backwards compatibility. The only change is that when asking for a single result like user_email it would return the actual email instead of a list of IDs.

Note: See TracTickets for help on using tickets.