Skip to:
Content

BuddyPress.org

Opened 10 years ago

Closed 10 years ago

#6099 closed defect (bug) (fixed)

groups/favorites/friends activity scopes when displayed user has not any 'scope' items

Reported by: imath's profile imath Owned by: r-a-y's profile r-a-y
Milestone: 2.2 Priority: high
Severity: major Version:
Component: Activity Keywords: has-patch commit
Cc:

Description

We have a trouble in adjusting scopes for BP_Activity_Activity::get_scope_query_sql when the displayed user does not have any items in the active scope. In this case all user's activities are fetched instead of none.

Way to reproduce for the favorites scope for instance : simply make sure the displayed user has not favorited any items.

I've built unit tests and realized it's also appearing on groups and friends scopes.

To solve this i suggest to set dummy arrays having a 0 value as the item id when the user is not a part of any groups/did not favorited any activities or has no friends.

Attachments (2)

6099.patch (5.2 KB) - added by imath 10 years ago.
6099.no-results.patch (4.4 KB) - added by r-a-y 10 years ago.

Download all attachments as: .zip

Change History (8)

@imath
10 years ago

This ticket was mentioned in Slack in #buddypress by imath. View the logs.


10 years ago

#2 @boonebgorges
10 years ago

  • Keywords commit added; 2nd-opinion removed

These fixes look correct to me. There was some discussion https://wordpress.slack.com/archives/buddypress/p1420808854002450 about whether array( 0 ) is hackish, but IMO it is not - I think it's the right way to solve this. (The alternative would be to introduce a 0=1 type no-results clause, but the way that we build our SQL makes this difficult to do in a consistent way across components. The array( 0 ) fix is much more straightforward, and I'd argue more semantically correct when it comes time to debug.)

#3 @r-a-y
10 years ago

Nice catch, imath!

I forgot to port over no results from my feature-as-a-plugin. In my plugin, I used a no-results clause like in no-results.patch. But based on the way that custom scopes are handled in each respective scope filter function, I can understand the approach in 6099.patch as well.

IMO, both patches are about equal when it comes to debugging.

If I were debugging the activity query's WHERE conditions and saw this:

[scope_query_sql] =>
(
        (
                a.component = 'groups'
                AND
                a.item_id IN ( 0 )
        )

)

I would wonder where a.item_id IN ( 0 ) came from.

Whereas, this might be more explicit:

[scope_no_results] => 0 = 1

Happy to go with either patch.

Last edited 10 years ago by r-a-y (previous) (diff)

#4 @r-a-y
10 years ago

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

In 9330:

Activity: Scopes with no generated SQL statement should match nothing.

Commit also adds unit tests demonstrating the problem.

Props imath.

Fixes #6099.

#5 @r-a-y
10 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening due to #6140.

imath's patch is the right way to address this issue. #6140 is an example where r9330 encroaches on older scopes that are not defined as scope arguments -- home and potentially other plugins adding a subpage under a member's activity such as BP Follow.

#6 @johnjamesjacoby
10 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 9372:

Revert r9330 in lieu of original patch on #6099.

When querying for various types and scopes of activity stream items, each BuddyPress component having a unique type or scope also has unique keys and filters responsible for handling the custom queries necessary to get the correct entries from the database.

In some instances (like single groups, member favorites, etc...) these scopes are returning invalid (or empty) results, causing no activity entries to be returned from the database when entries do exist and should be displayed.

Unit tests still apply (though they get some variable clean-up here.)

Hat-tip hnla for discovery. Props r-a-y, imath. Fixes #6140. Fixes #6099.

Note: See TracTickets for help on using tickets.