Skip to:

Opened 9 years ago

Closed 9 years ago

#6577 closed defect (bug) (fixed)

friends_check_friendship_status() problem when using legacy user query

Reported by: r-a-y's profile r-a-y Owned by: r-a-y's profile r-a-y
Milestone: 2.4 Priority: normal
Severity: normal Version: 2.1
Component: Friends Keywords: has-patch commit


This is a follow-up to #5703.

boonebgorges alerted me of a problem when using the legacy user query - add_filter( 'bp_use_legacy_user_query', '__return_true' ) - and checking the friendship status in the members loop.

When using the legacy user query, the friendship button would always return "Add Friend" instead of using the actual friendship status.

Bug is due to me using has_filter( 'bp_user_query_populate_extras', 'bp_friends_filter_user_query_populate_extras' ).

I've moved the logic for setting the 'not_friends' status directly to the bp_friends_filter_user_query_populate_extras() filter.

This should fix the problem. Existing unit tests should cover this test (@group friends_check_friendship_status).

Attachments (2)

6577.01.patch (2.2 KB) - added by r-a-y 9 years ago.
6577.user_ids.patch (2.2 KB) - added by r-a-y 9 years ago.

Download all attachments as: .zip

Change History (10)

9 years ago

#1 @boonebgorges
9 years ago

This seems like the right fix, at a glance. Thanks, r-a-y!

#2 @r-a-y
9 years ago

  • Owner set to r-a-y
  • Resolution set to fixed
  • Status changed from new to closed

In 10037:

Friends: Fix issue with using friends_check_friendship_status() during the members loop when the legacy user query is enabled.

Previously, if the legacy user query was enabled, the friendship button in
the members loop would always return "Add Friend" regardless of the actual
friendship status.

This commit fixes this by setting the 'not_friends' status inside the
the bp_friends_filter_user_query_populate_extras() function instead of
in friends_check_friendship_status().

Fixes #6577.

#3 @boonebgorges
9 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

After [10037], I noticed PHP notices being thrown in the Members widgets on one of my installs. It turns out that one of the users identified in $user_query->user_ids had been deleted from the installation. As a result, the ID showed up in the user_ids array, but not in results; and so the ID showed up as a false positive in the array_diff() added in [10037], which means "creating default object from empty value" notices on the line $user_query->results[ $nf ]->friendship_status = 'not_friends'.

A quick look suggests that this issue may not be reproducible through normal means. Under normal circumstances, deleted users will have their last_activity deleted as well, so that they shouldn't show up in the user_ids array in the first place. On my dev installation, it must be the case that the user was deleted manually, perhaps via SQL query.

Reopening this ticket to get your opinion, r-a-y: should we be doing one of the following?

  1. checking isset( $user_query->results[ $nf ] ) - or, better still, using array_keys( $user_query->results ) instead of $user_query->user_ids) - when adding the 'not_friends' flag
  2. In BP_User_Query, after performing the WP_User_Query in do_wp_user_query(), unset any IDs in user_ids that are not found in the WP_User_Query object?
  3. Do nothing, because the PHP notice in this case is a helpful reminder to developers that their data is somehow corrupted?

Option b seems the most elegant somehow, but it's also the farthest reaching (and is perhaps outside the scope of the current ticket - your commit only made it visible). I could be sold on any of these three options, though.

9 years ago

#4 @r-a-y
9 years ago

Thanks for uncovering this, boonebgorges.

I think we should do both option 1 and 2; see user_ids.patch.

I've included a unit test for option 2 and will commit bp-friends-filters.php separately.

#5 @boonebgorges
9 years ago

Cool, this looks good!

#6 @DJPaul
9 years ago

  • Keywords commit added

#7 @r-a-y
9 years ago

In 10043:

BP User Query: Remove invalid IDs from $user_ids property after query is run.

If the user_ids parameter is passed to BP_User_Query with invalid user
IDs, after the user query is run, the user_ids property still references
these invalid user IDs. This can lead to devs to assume that the user ID
exists when it does not.

This commit removes these invalid user IDs and includes a unit test.

See #6577.

#8 @r-a-y
9 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed
Note: See TracTickets for help on using tickets.