Skip to:
Content

BuddyPress.org

Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#5271 closed defect (bug) (fixed)

BP_User_Query - user_ids parameter breaks pagination

Reported by: shanebp's profile shanebp Owned by: boonebgorges's profile boonebgorges
Milestone: 1.9 Priority: normal
Severity: normal Version: 1.8.1
Component: Core Keywords: has-patch
Cc: sam3dus@…

Description

When using the bp_pre_user_query_construct hook to pass ids to the 'user_ids' parameter, the results on the Members page are as expected:

$query_array->query_vars['user_ids'] = $custom_ids_array;

But the pagination says:

Viewing member 1 to 0 (of 0 active members)

Attachments (2)

5271.diff (435 bytes) - added by imath 10 years ago.
5271.02.patch (594 bytes) - added by boonebgorges 10 years ago.

Download all attachments as: .zip

Change History (12)

#1 @shanebp
10 years ago

  • Cc sam3dus@… added

#2 @shanebp
10 years ago

If it's the case that using the 'user_ids' parameters assumes you are not on the Members page and are using the return for some other use case, then I will note that on the codex page for the class.

#3 @imath
10 years ago

  • Keywords has-patch added

hi @shanebp,

I confirm. I suggest 5271.diff to set $this->total_users if not in function do_wp_user_query()

@imath
10 years ago

#4 @shanebp
10 years ago

@imath -

I confirm your patch works on BP 1.8.1

Thank you!

Hopefully the patch will make it into 1.9 if not already there.

#5 @DJPaul
10 years ago

  • Keywords reporter-feedback added
  • Milestone changed from Awaiting Review to 1.9

I think there's merit in the patch but Boone's the expert on this part of the code, as he wrote it.

Boone/JJJ/Ray: do we want this for BP 1.9 or 2.0?

#6 follow-up: @boonebgorges
10 years ago

  • Keywords reporter-feedback removed

Good catch, imath. The technique seems good. I guess it makes more sense to me to do something like 5271.02.diff - same idea, but done in a place where I think it's easier to trace. imath, do you have any thoughts about it? Can you make sure it does what your patch does?

#7 in reply to: ↑ 6 @imath
10 years ago

Replying to boonebgorges:

Good catch, imath.

Thanks Boone, actually at first i did what you did in 5271.02.patch. The problem with this technique is that in this case if the array of user ids contains an id that doesn't exist, then the number of members displayed can be different than the total.

Example : the array( 1, 2, 9999999) will give you a total of 3 with your technique, but if 9999999 id doesn't exist it won't be displayed so the total is 3 and the number of displayed members is 2.

That's why i've chosen this part because we're sure to have a consistent total.

#8 follow-up: @boonebgorges
10 years ago

Aha, good call. It's certainly an edge case that people would be passing invalid user IDs to BP_User_Query, but I suppose we should account for it. Thanks!

#9 @boonebgorges
10 years ago

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

In 7660:

Calculate fallback value for BP_User_Query::total_users

BP_User_Query normally calculates the total_users count with a separate COUNT
query; see BP_User_Query::do_user_ids_query(). However, this method is not
called when a $user_ids parameter is passed to BP_User_Query. This changeset
introduces an alternative method for calculating total_users in this case,
ensuring that pagination works as expected.

Fixes #5271

Props imath

#10 in reply to: ↑ 8 @imath
10 years ago

Replying to boonebgorges:

Aha, good call. It's certainly an edge case that people would be passing invalid user IDs to BP_User_Query, but I suppose we should account for it. Thanks!

you're welcome :)

Note: See TracTickets for help on using tickets.