Opened 10 years ago
Closed 10 years ago
#5603 closed defect (bug) (fixed)
Posting a new Activity causes activity-loop.php scope filters to be removed
Reported by: | xyhavoc | Owned by: | |
---|---|---|---|
Milestone: | 2.0.1 | Priority: | normal |
Severity: | major | Version: | 2.0 |
Component: | Activity | Keywords: | reporter-feedback has-patch 2nd-opinion |
Cc: |
Description
In BP 2.0, limiting the scope within activity-loop.php doesn't work consistently when posting a new activity update.
For instance:
-activity-loop.php I have added the filter “&action=activity_update” so that the stream only shows user’s activity posts, as per the codex (http://codex.buddypress.org/developer/loops-reference/the-activity-stream-loop/).
-I Pull up the activity feed on the web site, everything looks A-OK, the scope filters are working as they should.
-Then I post a new activity update, when Buddypress adds the new activity via javascript, the filter is removed and the user can now scroll and see activities that should not be visible because of the activity-loop.php scope filter that have been set.
imath recommended adding a new way of limiting the scope through bp-custom.php but that doesn't make sense if there's already a documented way of limiting the scope through buddypress templates. However his fix did not work.
Link to his fix: https://gist.github.com/imath/5c7e4db74513ca4b30b8
I'm comparing BP 1.9 buddypress.js against BP 2.0 buddypress.js to see if this is where the problem lies and I think it may have something to do with the "load newest activities" functions, but I could be wrong. Any thoughts? Can anyone else reproduce this?
Attachments (4)
Change History (25)
#2
follow-up:
↓ 3
@
10 years ago
- Keywords reporter-feedback added
I've spent some time trying to reproduce this, but I've been unsuccessful. Clicking "Load More" always loads the correct items. But maybe I'm not understanding your report correctly:
-Then I post a new activity update, when Buddypress adds the new activity via javascript, the filter is removed and the user can now scroll and see activities that should not be visible because of the activity-loop.php scope filter that have been set.
Can you walk me through this a little more? "The user can now scroll"? Do you have something enabled on your site to allow for infinite scroll? BP, out of the box, requires you to click a "Load More" button to show more items.
#3
in reply to:
↑ 2
@
10 years ago
When a user makes a post, Buddypress adds the activity to the feed and updates in realtime using javacsript, correct?
Well when the user profile template is loaded, it reads "&action=activity_update&max=1", so the activity feed is populated with ONLY what the scope allows, which is great.
However, once a user types out and submits a post, the same feed is updated in realtime except now with results that should be hidden because of the scope that has been set.
Posting an update breaks the scope limiting consistency and I am not sure why.
No I do not have any infinite scroll plugins.
#4
follow-up:
↓ 6
@
10 years ago
the same feed is updated in realtime
I guess by this you mean that BuddyPress detects that new posts are available in the background, and then gives you a 'Load Newest' button at the top that you can click? That's the only sense in which I think that BuddyPress does any "realtime" feed updating. If this is right, then part of reproducing your issue involves going into a separate window and creating new activity?
#5
@
10 years ago
I've tried to reproduce this on another WP installation and it seems this is only affecting that one site. I'll post my solution as soon as I find one.
Edit: Never mind this is a BP 2.0 bug.
#6
in reply to:
↑ 4
@
10 years ago
It seems the problem is only happening when posting from the current user's page, and not when posting under sitewide /activity.
#7
@
10 years ago
This is definitely a Buddypress 2.0 bug. I just recreated this using another fresh buddypress install on another wordpress directory.
#8
@
10 years ago
- Keywords has-patch added
- Milestone changed from Awaiting Review to 2.1
It seems the problem is only happening when posting from the current user's page
Confirmed. Thanks for this extra info.
imath - Can you chime in here? It looks like the 'date-recorded' class is not being added, because bp_activity_do_heartbeat()
is not returning true on user activity streams. It seems to me that the easiest fix is just to add the date-recorded class *all the time* - it won't hurt anything in cases where heartbeat is inactive, and it seems to prevent the problem. See 5603.patch.
xyhavoc - Have a look at 5603.patch to see if it fixes the problem for you.
#9
@
10 years ago
- Keywords has-patch removed
- Milestone changed from 2.1 to Awaiting Review
Yes this seems to have done the trick. However after posting, the comment box automatically opened up for the new activity. It's not a big deal but I'm not sure why that is.
#10
@
10 years ago
- Keywords has-patch 2nd-opinion added
Hi boonebgorges, xyhavoc
I've checked the trouble, in bp.org forum the description xyhavoc made into this post was a bit unclear to me, i wasn't sure the problem was appearing when on the user's activity profile page.
1/ I've just edited the codex, to remove the bp_has_activities( 'object=status' )
thing that was described in the filtering examples chapter in favor of the example of the activity updates see:
http://codex.buddypress.org/developer/loops-reference/the-activity-stream-loop/#filtering-examples
I've done this because the object "status" does not exist anymore as it has been replaced by the last activity update a user made. Moreover i think this part was confusing, because it gave the impression we could use the activity loop without the bp_ajax_querystring( 'activity' )
argument. This argument needs to be there otherwise the pagination is broken for instance. If you take this example :
bp_has_activities( '&action=activity_update' )
You will end up loading the same activities that are displayed on the stream when clicking on the load more button.
Now @xyhavoc About the gist i've shared with you, as i've said it's my personal preference to not rely on templates to customize the activity loop in order to have more control on it. With your method all activity loops will be restricted with activity updates, with mine i can choose to restrict the user's profile one, the directory or the group's home one.
I agree it didn't solve the issue and you're free to use the method of your choice.
2/ The issue
I chose not to enable heartbeat when on a user's profile because i thought it doesn't make sense to surcharge this part as to be usable it would require the user to open a new browser and post from this browser to see on the first browser that he posted a new activity. As a result i think heartbeat is only interesting when more than one user can post > Directory & in activity's single group page.
boonebgorges, i saw & tested your patch, it seems to work fine, but i have the feeling this is not the best way to solve this. Explaining why:
In bp-templates/bp-legacy/buddypress/buddypress-functions.php at line 714 there's :
} else { $activity_args = array( 'include' => $activity_id ); }
And using your patch we'll never set this $activity_args the way it's described in the else part. So i think the problem is the isset
check that we should replace by a ! empty
one, this way we'll "arrive" in the else part described above. And concerning the user's profile i think we need to only get the activity the user just posted instead of all the activities the user posted since (we never know if the user is playing with 2 browser at the same time).
Because this single update will dynamically update the "user status" or latest activity in member's header. So we need to be sure there's only one activity.
So i'm attaching 5603.02.patch because i strongly think it's the right move.
xyhavoc, can you confirm 5603.02.patch is also fixing your issue ?
boonebgorges, what do you think of my alternative approach ?
#11
follow-up:
↓ 12
@
10 years ago
I chose not to enable heartbeat when on a user's profile because i thought it doesn't make sense to surcharge this part as to be usable it would require the user to open a new browser and post from this browser to see on the first browser that he posted a new activity.
Yes, I think this original decision was a good one, and I don't think we should change it (definitely not for 2.0.1).
In bp-templates/bp-legacy/buddypress/buddypress-functions.php at line 714
What's the natural situation where this will happen? When *should* $last_recorded = 0
? When an activity stream is empty? (Just trying to be clear on why we'd try to preserve this else
condition.)
5603.02.patch looks fine to me, in any case.
#12
in reply to:
↑ 11
@
10 years ago
Replying to boonebgorges:
What's the natural situation where this will happen? When *should*
$last_recorded = 0
? When an activity stream is empty? (Just trying to be clear on why we'd try to preserve thiselse
condition.)
in buddypress.js, i init last_date_recorded to 0. And if there's a timestamp class (needed by the heartbeat feature) in the first displayed activity, then last_date_recorded takes the corresponding timestamp value.
So if $last_recorded is empty it means we're on the user's profile activity page as no timestamp class are added in this part.
I really think the else condition is only usefull for the user's profile member's header. Because when a user is posting an update from his profile, the content of this update is also "copied" into
<div id="latest-update"> <?php bp_activity_latest_update( bp_displayed_user_id() ); ?> </div>
So using the "else" part array( 'include' => $activity_id )
, we make sure there can be only one latest_update in member's header
#13
@
10 years ago
boonebgorges, I've seen you've chosen the 2.1 milestone (then xyhavoc reverted to Awaiting review). I think if we don't have time to include a fix before 2.0.1, as it's something introduced by 2.0, maybe we should still add the fix into 2.0 branch, for an eventual 2.0.2 release.
#14
follow-up:
↓ 15
@
10 years ago
- Milestone changed from Awaiting Review to 2.0.1
Oops, I meant to put in 2.0.1. Let's go ahead with your fix.
#15
in reply to:
↑ 14
@
10 years ago
Replying to boonebgorges:
Oops, I meant to put in 2.0.1.
;)
Let's go ahead with your fix.
Should i wait for xyhavoc's feedback before committing ?
#17
follow-up:
↓ 18
@
10 years ago
Thank you guys for looking into this situation and I applaud your knowledge with web development, you guys are light years ahead of me. :)
With that said, the patch fix does seem to be working with the exception of the comment box opening up on it's own after the post. And deleting the activity causes it to slide up and come back again.
As far as the scope being consistent with before and after posting, it's appears to be working correctly.
I need to really wrap my head around filtering at the functions.php level. It's still a bit beyond my knowledge but I'm learning over time.
#18
in reply to:
↑ 17
@
10 years ago
Replying to xyhavoc:
Thanks a lot for your feedback
With that said, the patch fix does seem to be working with the exception of the comment box opening up on it's own after the post. And deleting the activity causes it to slide up and come back again.
I don't have this behavior, using twentytwelve theme the comment box is not opened up and deleting the activity doesn't cause it to come back.. This is strange. Could it be the theme you are using or a specific plugin ? If you use the default activity loop is this also happening ?
As far as the scope being consistent with before and after posting, it's appears to be working correctly.
Fine, i'm going to commit 5603.02.patch, leaving this ticket open. I think we really need this fix in 2.0.1.
Other scope parameters like &max=1 are also removed when posting a new update.