Skip to:
Content

Opened 8 years ago

Closed 7 years ago

Last modified 16 months ago

#2000 closed defect (bug) (fixed)

Duplicate cookie of bp-activity-oldest-page leads to duplicate posts in activity stream

Reported by: erich73 Owned by: kunalb
Milestone: 1.5 Priority: major
Severity: Version: 1.5
Component: Templates Keywords: has-patch close
Cc: kunalb

Description

when being at the homepage of testbp.org, then seing some posts in the first section (without opening the "Load More section").

Then when opening another section (first time of clicking onto "Load More"), it is showing some post DOUBLE.

So the same posts are showing in the default section and in the opened section of "Load More".

See Screenshot attached.

Attachments (3)

double_post_load_more.gif (61.8 KB) - added by erich73 8 years ago.
activity.diff (9.0 KB) - added by kunalb 7 years ago.
in_the_year_2000.patch (560 bytes) - added by r-a-y 7 years ago.

Download all attachments as: .zip

Change History (24)

#1 @erich73
8 years ago

this issue appeared today (18th February 2010) at the website of testbp.org

#2 @cnorris23
8 years ago

  • Keywords needs-patch added

Good catch. I tested this on testbp.org, and was able to reproduce it. Here's what appears to be happening. When you visit a page with an activity stream, the most recent 20 activities and their comments are retrieved. If a new post is added to the activity stream before you click "load more," you will get a duplicate post after clicking "load more." Here's what's going on. Say there are 40 posts in the stream:

Newest Activity (post-id-40)
...
Oldest Activity (post-id-21)
Load More

When you click load more, you expect to see the second page of activity, post-id-20 through post-id-1. Let's say two new posts have been added to the stream before you click load more, for a total of 42 posts. Instead of seeing post-id-20 through post-id-1 after clicking load more, you get this:

post-id-22
...
post-id-3
Load More

Your full stream will now show:

post-id-40
...
post-id-21
post-id-22
post-id-21
...
post-id-3
Load More

I haven't looked at the code to see what, exactly, is happening, but it looks the offending function only iterates through stream posts in blocks of 20, without regard for the the last post shown.

#3 @boonebgorges
8 years ago

I fished through the code a little bit based on cnorris23's suggestions. When you click "Load More", a cookie is stored (bp-activity-oldestpage) saying which page you had gone back to. So the calculation about which activity items to load on "Load More" is made based on the page # cookie x the per_page, which in the default theme is set as the default value in bp_has_activities, ie 20.

Not sure what the best way is to solve the problem. One possible setup: When "Load More" is clicked, get the oldest activity id number from the last child of ul#activity-stream. Then in ajax.php use that number to calculate a value of $include (specific activity id numbers) to pass along to bp_has_activities in the template. Is that a reasonable way to do things?

#4 @kunalb
7 years ago

  • Cc kunalb added
  • Component set to Core
  • Owner set to kunalb
  • Status changed from new to accepted

I was thinking that going through the ul#activity-stream might cause problems across themes (correct me if I'm wrong.) Maybe we can create another cookie along the lines of bp-activity-postsadded which is updated everytime posts are added to the current page? That can be used to push the number from which posts are read and returned on the load-more request to the correct number.

#5 @kunalb
7 years ago

  • Component changed from Core to Activity

#6 @kunalb
7 years ago

  • Keywords has-patch needs-testing added; needs-patch removed
  • Priority changed from major to normal

Created a basic patch to solve this: creates a cookie bp-activity-offset which is used to control any offsets required. Accordingly modified the bp_activity_template, has_activity, get, bp_activity_get to accept offset as a variable. This might break other stuff, definitely needs testing.

@kunalb
7 years ago

#7 @erich73
7 years ago

  • Milestone changed from 1.3 to 1.2.6
  • Priority changed from normal to major

#8 @boonebgorges
7 years ago

  • Keywords tested added; needs-testing removed

Tested and works on latest 1.2 branch. When you load an activity page that shows 20 items, and post another to that page through the ajax update box, hitting Load More no longer reloads items 21-41 (where 21 is the old 20 and thus a duplicate) but instead loads 22-42. Works well for any number of updates as far as I can tell.

#9 @johnjamesjacoby
7 years ago

Rock on. Looks like we'll give this a go and I'll put it up on testbp.org soon, for us to give some good laps on.

#10 @johnjamesjacoby
7 years ago

Have been looking at this, and creating another cookie specifically for this 1 thing doesn't feel like the correct solution to me. It would be nice to not drop too many cookies if they can be prevented in any other way.

I'm inclined to bump this back to 1.3 for further review, since it sounds like the circumstances that cause this to happen are pretty edge case unless the activity stream is highly active. In that case, a better solution would be live updates anyhow, which are on the roadmap for 1.4 I believe.

If someone can come up with a solution that doesn't involve the extra cookie, that would be awesome. I'll leave this until the end of the week before punting it if there's no solution.

#11 @pisanojm
7 years ago

This was happening in triplicate on BP.org today... I posted on BP at the time I was seeing it... Not sure if the repliation was related to the above...

#12 @johnjamesjacoby
7 years ago

  • Milestone changed from 1.2.6 to 1.3

Per todays dev chat, punting this to 1.3 for further review.

#13 @paulhastings0
7 years ago

  • Summary changed from again showing double-post in activity-stream to [patch] again showing double-post in activity-stream

#14 @r-a-y
7 years ago

  • Component changed from Activity to Theme
  • Keywords tested removed
  • Summary changed from [patch] again showing double-post in activity-stream to Duplicate cookie of bp-activity-oldest-page leads to duplicate posts in activity stream
  • Version set to 1.3

Was debugging the ajax querystring in a BP theme when I came across this bug; the fix is easier than you think!

The jq.cookie was missing the path declaration, so jQuery was never looking for the previous "bp-activity-oldestpage" cookie, it created a new one and hence double posts galore!

The attached patch fixes this; patch is against 1.3-trunk.

#15 @djpaul
7 years ago

(In [4039]) Add path argument to bp-activity-oldestpage cookie. See #2000.

#16 @DJPaul
7 years ago

Sorry for forgetting your props, r-a-y. I couldn't confirm that it fixes this issue, but it didn't look like it would break anything and it is makes it consistant with the nearby jq.cookie calls.

Last edited 7 years ago by DJPaul (previous) (diff)

#17 @cnorris23
7 years ago

I ran across this issue on a 1.2.8 test install recently. Adding the path seemed to fix the issue. No more double cookies. FWIW.

#18 @r-a-y
7 years ago

  • Keywords close added

Thanks for verifiying, cnorris23.

Close? Or one more FTW?

#19 @cnorris23
7 years ago

I think closing would be safe. I'll let you do the honors, since you found the issue :)

#20 @r-a-y
7 years ago

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

Alright, I'm closing the ticket!

*closes ticket and runs away from core devs*

#21 @DJPaul
16 months ago

  • Component changed from Appearance - Template Parts to Templates
Note: See TracTickets for help on using tickets.