Skip to:
Content

BuddyPress.org

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's profile 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)

beforepost.jpg (96.1 KB) - added by xyhavoc 10 years ago.
BP showing one activity post like it should
afterpost.jpg (191.2 KB) - added by xyhavoc 10 years ago.
After posting a new activity update
5603.patch (600 bytes) - added by boonebgorges 10 years ago.
5603.02.patch (819 bytes) - added by imath 10 years ago.

Download all attachments as: .zip

Change History (25)

#1 @xyhavoc
10 years ago

Other scope parameters like &max=1 are also removed when posting a new update.

@xyhavoc
10 years ago

BP showing one activity post like it should

@xyhavoc
10 years ago

After posting a new activity update

#2 follow-up: @boonebgorges
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 @xyhavoc
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: @boonebgorges
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 @xyhavoc
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.

Last edited 10 years ago by xyhavoc (previous) (diff)

#6 in reply to: ↑ 4 @xyhavoc
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 @xyhavoc
10 years ago

This is definitely a Buddypress 2.0 thing. I just recreated this using another fresh buddypress install on another wordpress directory.

Version 0, edited 10 years ago by xyhavoc (next)

#8 @boonebgorges
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.

@boonebgorges
10 years ago

#9 @xyhavoc
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 @imath
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 ?

@imath
10 years ago

#11 follow-up: @boonebgorges
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 @imath
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 this else 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 "javascript 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

Last edited 10 years ago by imath (previous) (diff)

#13 @imath
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: @boonebgorges
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 @imath
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: @xyhavoc
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 @imath
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.

#19 @imath
10 years ago

In 8340:

Only load the activity just posted when on a user activity page

HeartBeat activity check feature introduced a "since" argument to load the activity that are newest. In a user activity page, this feature is not used but a timestamp is created using a "0" value causing all user activities to be reloaded. Checking if the value is empty before creating the timestamp will make sure the activity just posted will be the only one loaded into user stream.

props boonebgorges, imath

See #5603

#20 @imath
10 years ago

In 8341:

Only load the activity just posted when on a user activity page

HeartBeat activity check feature introduced a "since" argument to load the activity that are newest. In a user activity page, this feature is not used but a timestamp is created using a "0" value causing all user activities to be reloaded. Checking if the value is empty before creating the timestamp will make sure the activity just posted will be the only one loaded into user stream.

props boonebgorges, imath

See #5603

#21 @boonebgorges
10 years ago

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

I'd like to clear out the 2.0.1 milestone, so I'm going to close the ticket. xyhavoc, if you're still having issues with the activity box expanding improperly, please open a separate ticket for discussion. Thanks!

Note: See TracTickets for help on using tickets.