Opened 13 years ago
Closed 8 years ago
#4062 closed defect (bug) (fixed)
Activity makes profile avtivity filtered aswel
Reported by: | gogger | Owned by: | boonebgorges |
---|---|---|---|
Milestone: | 2.8 | Priority: | normal |
Severity: | minor | Version: | 1.2 |
Component: | Activity | Keywords: | has-patch needs-testing |
Cc: |
Description
Iv tested this on the testbp.org website so its not just my website. But if you go to the activity page, filter the activity by say ‘New Members’ then go to a user page in another tab it only shows activity that matches the filter on the original activity page – but the drop down always says ‘everything’ even though its not. Even if you then close the activity page and refresh the user page it comes up filtered. If you dont open it in a new tab but browse straight to a user profile from the filtered activity feed the user feed shows up with the same filter – only this time the correct filter is displayed in the drop down.
Im not sure if the later is meant to happen – and im not sure if theres much that can be done about the former, but i noticed it none the less. Its a bit annoying, but far from a deal breaker.
Once again, Buddypress freaking rocks – thanks a million.
Attachments (2)
Change History (11)
#4
@
8 years ago
- Keywords has-patch dev-feedback added
As far as I can tell the dropdown always have the correct filter selected as long as the filter that's selected (via the cookie bp-activity-filter) also has a context that exists in the current scope, i.e. if the bp-activity-filter cookie has a value of new_member and you're viewing a profile, "everything" is shown in the dropdown because the context of new_member is != member and therefore it's not shown in the dropdown generated by bp_activity_show_filters() (see: https://buddypress.trac.wordpress.org/browser/trunk/src/bp-activity/bp-activity-template.php#L3830).
The problem is that the activity is still filtered by new_member in https://buddypress.trac.wordpress.org/browser/trunk/src/bp-templates/bp-legacy/buddypress-functions.php#L689 even though no activities of that type exists in the context of viewing a profile page.
I've whipped up a quick patch as a proof of concept for how this could be fixed, not sure though if it's attacking the problem in the right place. This patch might need some cleanup, for example it might be better to abstract out the $context setting functionality into its own function bp_get_activity_context( $context ) or something since it's just copypasted from https://buddypress.trac.wordpress.org/browser/trunk/src/bp-activity/bp-activity-template.php#L3803
But other than that, as far as I can tell this solves the problem by not filtering activities if the $context of the filter is not a part of the current scope.
#6
@
8 years ago
The patch looks pretty good to me, though I haven't tested it or looked into the issue further. Once someone has some time to review this and see if there's any other places to fix this (as you suggest in your comment, @lakrisgubben), we should be able to move this along. Thanks for your work on the patch so far!
#7
@
8 years ago
- Keywords needs-testing added; dev-feedback removed
@lakrisgubben Thanks so much for working on this ticket (and sorry for the delay in review!).
In some ideal world, it'd be nice if we had more fine-grained logic for setting the current filter based on context and cookie. I mean, what's the value in sharing the cookie between the global activity stream and a group activity stream, even when the filter is valid in both contexts? If I had my druthers, I'd yank out the entire set-filter-based-on-cookie system altogether - it causes more problems than it solves.
Given the state of things, I think the approach in your patch is appropriate. In 4062.2.diff, I've taken up your suggestion to break the shared logic into a new function. Two, actually: one to get the "current context", and one to return a list of valid actions for a given context. It seems to me that having this level of granularity in the functions will allow devs to do interesting things in the future.
Anyway, would you have a look to see if it's close to what you had in mind, and to be sure that I haven't butchered the fix itself?
I thought we had a recent ticket about this, but I can't find it. It's not a trivial thing to change so it'll be fixed in a future release.