WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 19 months ago

Last modified 19 months ago

#28631 closed defect (bug) (fixed)

WP_User_Query should support -1 for the number parameter

Reported by: mordauk Owned by: wonderboymusic
Milestone: 4.4 Priority: normal
Severity: normal Version: 4.0
Component: Users Keywords: has-patch
Focuses: Cc:

Description

In order to be consistent with WP_Query, I think WP_User_Query should accept -1 as a valid value for the number parameter.

If you want to perform a query that returns all posts, you pass -1 for posts_per_page to WP_Query.

If you try to do this with WP_User_Query, however, you get an SQL syntax error:

WordPress database error: [You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '-1' at line 2]
SELECT SQL_CALC_FOUND_ROWS wp_users.* FROM wp_users INNER JOIN wp_usermeta ON (wp_users.ID = wp_usermeta.user_id) WHERE 1=1 AND ( (CAST(wp_usermeta.meta_value AS CHAR) LIKE '%\"pending\\_vendor\"%') ) AND (wp_usermeta.meta_key = 'wp_capabilities' ) ORDER BY user_login ASC LIMIT -1

LIMIT can't be -1 in SQL.

I'd expect both of these to return the same results:

$query = new WP_User_Query( array(
    'number' => -1
) );

$query = new WP_User_Query( array(
    'number' => 99999999
) );

Attachments (4)

28631.diff (496 bytes) - added by nofearinc 3 years ago.
28631-tests.diff (500 bytes) - added by jesin 3 years ago.
Unit tests for 'number' => -1
28631-2.diff (508 bytes) - added by mordauk 3 years ago.
28631-tests-2.diff (1.2 KB) - added by mordauk 3 years ago.

Download all attachments as: .zip

Change History (17)

@nofearinc
3 years ago

#1 @nofearinc
3 years ago

  • Keywords has-patch added

WP_Query has a nopaging argument that's true or false which defines whether we add or not the LIMIT statement, a simple check for a positive number could probably solve that. I'm not sure if a intval call should be used as well to prevent floating point numbers, but that quick check ignores the limits for all non-negative numbers and zero.

#2 @mordauk
3 years ago

28631.diff looks good to me.

#3 @nacin
3 years ago

  • Keywords needs-unit-tests added
  • Milestone changed from Awaiting Review to Future Release

needs-unit-tests :-)

@jesin
3 years ago

Unit tests for 'number' => -1

#4 @jesin
3 years ago

28631-tests.diff is my first unit test patch. Hope things are in the right place.

@mordauk
3 years ago

#5 @mordauk
3 years ago

28631-2.diff is a refreshed patch for the /src/ directory.

28631-tests-2.diff adds an additional test that checks for proper counts when passing the number parameter as 12, 10, and -1.

#6 @mordauk
3 years ago

  • Keywords needs-unit-tests removed

#7 @mordauk
2 years ago

  • Keywords needs-testing added

#8 @mordauk
2 years ago

  • Type changed from enhancement to defect (bug)

#9 @wonderboymusic
19 months ago

  • Keywords needs-testing removed
  • Milestone changed from Future Release to 4.4

#10 @wonderboymusic
19 months ago

  • Owner set to wonderboymusic
  • Resolution set to fixed
  • Status changed from new to closed

In 35114:

Users: allow -1 (no limit, use with extreme caution on large sites) as the value for number in WP_User_Query - similar to posts_per_page => -1 in WP_Query.

Adds unit tests.

Props mordauk, jesin, nofearinc.
Fixes #28631.

#11 @boonebgorges
19 months ago

In 35123:

Correct expected counts in WP_User_Query 'number' test.

The create_many() number should be padded by just one, to account for the
user created by the test suite. Introduced in [35114].

We also don't have to create so many fixtures to run this test.

See #28631.

#12 @DrewAPicture
19 months ago

In 35135:

Docs: Add a changelog entry mentioning that the number argument now supports -1 (all).

Also fixes the argument description.

-1 support was added in [35114].

See #28631.

#13 @DrewAPicture
19 months ago

In 35138:

Users: Restore changes from [35114] accidentally reverted in [35135].

See #28631.

Note: See TracTickets for help on using tickets.