New activity check

[SGr33n]

Hi,

I don't know if you wanna fix this, but there is a difference between the check for activity by PHP and the one by ajax ( bp_activity_newest_activities ).

Infact the first one maybe orders via date_recorded DESC, the second one by id.

In the most of cases it's ok, but if we mention somebody on bbPress and then we don't pass by the page displaying it can create a bug.

In this case the activity will have a date lower than the id, and then, if we are on an activity page, php will show as last new item the mention, because it has a lower date than last item, and will force it, everytime, as new item because it has an higher id.

[boonebgorges]

imath, would you mind taking a look at this?

[imath]

Replying to boonebgorges:

imath, would you mind taking a look at this?

Not at all, i'll check this asap.

[imath]

Replying to SGr33n:

but if we mention somebody on bbPress and then we don't pass by the page displaying it can create a bug.

I'm not sure to understand what means "don't pass by the page", can you explain step by step how you've done ?

In this case the activity will have a date lower than the id, and then, if we are on an activity page, php will show as last new item the mention, because it has a lower date than last item, and will force it, everytime, as new item because it has an higher id.

I'm sorry, i've tried to reproduce but didn't succeed :(

And i'm a bit lost, mentioning a user doesn't create an update, so as there's only the topic activity created.. i wonder how the trouble happened in your config..

If/When it happens again, can you share the activity id and date recorded fields of activity that is problematic with the one before and after in the db ?

[SGr33n]

Hi imath!

Actually if you mention somebody on bbpress, buddypress creates a new_mention activity, but not on the same moment you post data, when the next time the page containing the mentioning will be visited... that's happen to my installations.

[imath]

Replying to SGr33n:

Hi imath!

Actually if you mention somebody on bbpress, buddypress creates a new_mention activity, but not on the same moment you post data, when the next time the page containing the mentioning will be visited... that's happen to my installations.

Actually, i really don't think BuddyPress is creating an activity typed "new_mention" when a mention is made. I've made a search on all BuddyPress (trunk) and bbPress (2.5.3) and wasn't able to find the "new_mention" type you're talking about.

I've tried to reproduce by submitting a topic where i mention a user and no activity typed "new_mention" was created in the database, (i've refreshed the topic page a bunch of time).

Is it possible that a plugin is creating this activity ?

[SGr33n]

Sorry, my mistake... it was a notification, and it was component_action = 'new_at_mention'. When, then, you'll view the page with that mention, buddypress creates a new record inside wp_postmeta, meta_key = _bbp_activity_id, referring to this activity type: bbp_reply_create, that's the one.

[SGr33n]

To replicate just edit an old bbPress post, inserting a mention, e.g. @imath. Visit again that page, then wait a few seconds on the activity page.

[imath]

Replying to SGr33n:

To replicate just edit an old bbPress post, inserting a mention, e.g. @imath. Visit again that page, then wait a few seconds on the activity page.

done and nothing went wrong ? I'm sorry, i really don't manage to reproduce, there's no activity generated when mentioning a user. I don't know what to think. Does anybody manage to reproduce ?

[SGr33n]

I'm uploading a webcast on YouTube reproducing the bug.

[SGr33n]

On the start you can see that i publish a new activity.
Then I modify an oldest bbPress reply inserting a new mention.

I go to the activity page but the ajax updater shows it always as newest, also if i reload the page many times.

Actually this stops only if a new activity is published.

This happen because the activity page order them by date, but the ajax checker looks if there are activities with an id higher than the last one's. Since that the last one have an higher id but a lower date, this will be always the newest one.

[SGr33n]

Here it is :)

​http://youtu.be/Myx3A2O2V7I

[imath]

Replying to SGr33n:

Here it is :)

​http://youtu.be/Myx3A2O2V7I

Yeah, this shouldn't happen actually. I think the mention has nothing to do with it. When a bbPress topic/reply is updated, bbPress is updating the activity for this topic/reply. But it doesn't update the date of this activity :
see ​https://bbpress.trac.wordpress.org/browser/trunk/src/includes/extend/buddypress/activity.php#L434

The only way i've managed to reproduce was to change this line using bp_core_current_time() instead of get_post_time( 'Y-m-d H:i:s', true, $topic_id )

So i'm a bit amazed by the video, maybe there's a bbPress setting i've missed that would automatically update the topic date in case it has been edited..

So. If an activity for some reason has his date updated to current time then all activity with a greater id will load again in the stream. Trying to build a patch to avoid this.

[boonebgorges]

imath - Hold on, I'm almost finished with a patch

[boonebgorges]

So, I'm running out of time to finish up the patch I'm writing, so I'm going to post what I have with some thoughts. imath, since you wrote the JS, maybe you can finish the work (or tell me if you disagree with what I've started)

• I agree that this has nothing to do with the mention in bbPress.
• This only happens when the date_recorded value of an existing activity item is modified. I actually couldn't reproduce this with bbPress in the interface. Like imath, the only way I could do it was to manually force the discrepancy in the database (I did it with a direct SQL query, but the same idea)
• Still, I think this needs to be fixed, because it's a fairly serious interface issue if there is a plugin that bumps old behavior in this way
• I've opted to add a 'since' parameter alongside 'offset'. See 5505.patch.
• IMO, it's most logical for 'since' to accept a Y-m-d H:i:s value (see 5505.patch). But it's safest from a JS point of view to pass an integer value back and forth. I started to work on this in the patch but ran out of time.
• If we go with this, we're going to need another CSS class to activity items - something like date-recorded-{$date_recorded} (where $date_recorded will be a unix timestamp)

imath, do you agree with this take on things? Is there any chance that you can finish what I've started in 5505.patch? If not, I can try to get to it tomorrow. Many thanks.

[imath]

Replying to boonebgorges:

imath, do you agree with this take on things? Is there any chance that you can finish what I've started in 5505.patch? If not, I can try to get to it tomorrow. Many thanks.

Thanks a lot for your work on this patch boonebgorges :) I'd say that's a really nice option, i had something else in mind. So i'll try to end your option and eventually add another patch containing my option so that we can choose (just realized my idea wouldn't work)

[SGr33n]

I think that checking new updates based on the time, not on the ID, could solve this issue... date_recorded is also an index, so it should not take much time than search on last id.

[SGr33n]

Replying to imath:

Yeah, this shouldn't happen actually. I think the mention has nothing to do with it. When a bbPress topic/reply is updated, bbPress is updating the activity for this topic/reply. But it doesn't update the date of this activity :

If there wasn't a mention, bbPress didn't create the activity, that's why the mention has the newest date.

[imath]

I think 5505.01.patch is completing 5505.patch for the javascript part and some other stuff in ajax functions. It creates a specific class if heartbeat feature is on.

I think we should test this a bit more. I'll run some more test tomorrow. @SGr33n if you could check if applying the patch solves your issue, that would be awesome ;)

[SGr33n]

Everything is ok, it's not happening anymore :)

[imath]

Replying to SGr33n:

Everything is ok, it's not happening anymore :)

Thanks for your feedback :)

My pattern wasn't good to get the timestamp. As soon as an activity was posted, it stopped the feature as the activity_last_recorded is reset and the class which contains the timestamp is not the last one (there's just-posted).

5505.02.diff changes the pattern used to match the timestamp.

[boonebgorges]

These changes look good to me, imath. If they're fixing the problem for everyone, and you feel good about them, go ahead and commit. Thanks.

[imath]

Replying to boonebgorges:

These changes look good to me, imath. If they're fixing the problem for everyone, and you feel good about them, go ahead and commit. Thanks.

Thanks boonebgorges :) Actually something was making feel uncomfortable. I've tested it and retested it, found something i was doing wrong so corrected it and then i imagine the case where a plugin updated the recorded_date. The fix will load this activity as a new one, but we could have the 2 same activity on the stream.
In the example of SGr33n:
let's say i edit the topic a first time (although i still don't understand how this is happening as bbPress is not updating the recorded date..), go to the activity stream, load newest and it's on top.
Now i go back on the topic and edit it again, it will load another time and so on and so on....

So to avoid this, i've added a new check that parses the newest_activities to eventually remove potential duplicates. Before committing, i'd like your opinions this new version (other opinions are welcome too :) ).

The trick will be in buddypress.js at line 418 once the 5505.03.patch applied. Thanks in advance ;)

[SGr33n]

@imath
In my opinion it's enough to look for newest activities (date) than the last one, instead of look for highest id than the last one.

Regarding bbPress the activity is created as soon as a visitor visits the page containing the mention, not on the new post $POST. The mention is connected to the postmeta meta_key _bbp_activity_id.

So... if the page of the mention is visited in a second moment that mention will have an higher id but the lowest date. I don't know how really explain this :P

I think it would be simplest if you could reproduce it. Please try again using the following steps:

1) create a new bbPress Topic
2) create a new status update
3) edit the bbPress Topic inserting a mention
4) go to the activity page and wait

[imath]

Replying to SGr33n:

@imath
In my opinion it's enough to look for newest activities (date) than the last one, instead of look for highest id than the last one.

Actually i'm not looking for the last one. I'm making sure the activity which recorded date has been updated is not already on the stream before displaying newest activities. The idea is to avoid duplicate activities.

1) create a new bbPress Topic
2) create a new status update
3) edit the bbPress Topic inserting a mention
4) go to the activity page and wait

Just tested again, still waiting for what you've described to happen ;) Maybe another plugin is playing in this area in your config?

Anyway, as boonebgorges said: your feedback is really useful as it demonstrated it can happen.

[boonebgorges]

i imagine the case where a plugin updated the recorded_date.

This seems like very much an edge case to me, but your fix for it looks solid.

[imath]

In 8251:

Insert a new "since" filter for the activity stream to be used by HeartBeat check feature

In rare cases, the field date_recorded of the activity table can be updated to a date greater than the most recent activity in terms of id.
The Heartbeat feature was based on the id field so far, while the activity stream is sorting items according to the date_recorded field. As a result, in this very particular case, activities with an id greater than the activity (which date has been updated) will be loaded again in the stream and this until a new activity is published.
To prevent this to happen, we introduce this new "since" filter to be used by the HeartBeat feature once it gets the timestamp of the most recent activity displayed in the stream.

props SGr33n, boonebgorges, imath

Fixes #5505

[SGr33n]

Replying to imath:

Replying to SGr33n:
Just tested again, still waiting for what you've described to happen ;) Maybe another plugin is playing in this area in your config?

Lol :P but it's happening on a clean installation, just bbPress and BuddyPress enabled and TwentyFourteen theme.

[SGr33n]

Still happen... :\ to me on the search page, but I really don't know.

Searching, the first row is:

<li id="activity-148132" class="activity activity_update activity-item date-recorded-1406197697">

Then, after heartbeat pulsed, it loads:

<li id="activity-148140" class="activity activity_update activity-item just-posted">
<li id="activity-148139" class="activity activity_update activity-item just-posted">
<li id="activity-148138" class="activity activity_update activity-item just-posted">
<li id="activity-148137" class="activity activity_update activity-item just-posted">
<li id="activity-148135" class="activity activity_update activity-item just-posted">
<li id="activity-148134" class="xprofile updated_profile activity-item mini just-posted">
<li id="activity-148133" class="activity activity_update activity-item just-posted">
<li class="load-newest" style="display: none;">

[imath]

Replying to SGr33n:

Still happen... :\ to me on the search page, but I really don't know.

Thanks for the feedback, i'll check it soon and will open a new ticket.

[SGr33n]

If you need somebody to test, I'm here.

[imath]

Replying to SGr33n:

If you need somebody to test, I'm here.

Thanks a lot :) here's the new ticket #5779