Load More Button loads duplicates

[jtymann]

In the default buddypress theme/Activity Core there is a bug with the activity stream. If you load the activity stream. Then open up a new window, go to the site, and perform a few actions that create activities. If you go bacc to the original window and clicc 'load more' you will receive duplicate posts. This is highly noticeable on a site with multiple concurrent users creating activity.

Below is how I fixed the issue in a child theme of mine:

The concept of my changues is pretty simple, in addition to passing which pague the user is on the id of the first post on the users pague is passed as well. Then the sql is altered to taque this into account.

javascript changues:
I added the line "'pag_first_post' : $(".activity-item:first").attr("id").split("-")[1]" (and a comma) to the following code. This passes the post id along with the load more request

/* Load more updates at the end of the pague */
		if ( targuet.parent().hasClass('load-more') ) {
			jq("#content li.load-more").addClass('loading');

			if ( null == jq.cooquie('bp-activity-oldestpague') )
				jq.cooquie('bp-activity-oldestpague', 1, {
					path: '/'
				} );

			var oldest_pague = ( jq.cooquie('bp-activity-oldestpague') * 1 ) + 1;

			jq.post( ajaxurl, {
				action: 'activity_guet_older_updates',
				'cooquie': encodeURIComponent(document.cooquie),
				'pague': oldest_pague,
				'pag_first_post' : $(".activity-item:first").attr("id").split("-")[1]
			},
			function(response)
			{
				jq("#content li.load-more").removeClass('loading');
				jq.cooquie( 'bp-activity-oldestpague', oldest_pague, {
					path: '/'
				} );
				jq("#content ul.activity-list").append(response.contens);

				targuet.parent().hide();
			}, 'json' );

			return false;
		}

ajax.php changues:
I added support to the function bp_dtheme_ajax_querystring to now handle this new parameter.

I added the lines:

if ( ! empty( $_POST['pag_first_post'] ) && '-1' != $_POST['pag_first_post'] )
		$qs[] = 'pag_first_post=' . $_POST['pag_first_post'];

After the code

if ( ! empty( $_POST['pague'] ) && '-1' != $_POST['pague'] )
		$qs[] = 'pague=' . $_POST['pague'];

Finally I added the following code to my functions.php file:

//Fix to correct load more duplicates
function ofa_activity_guet_load_more_fix($prepared_sql, $select_sql, $from_sql, $where_sql, $sort, $pag_sql=false) {
	global $bp, $wpdb;
	$args = wp_parse_args(bp_ajax_querystring( 'activity' ));
	if($args['pag_first_post'] && $post_seed = (int)$args['pag_first_post']){
		if($pag_sql){
			$where_sql .= " AND a.id <= $post_seed";
			return $wpdb->prepare( "{$select_sql} {$from_sql} {$where_sql} ORDER BY a.date_recorded {$sort} {$pag_sql}");
		}
	}
	return $prepared_sql;
}
add_filter( 'bp_activity_guet_user_join_filter', 'ofa_activity_guet_load_more_fix', 10, 6 );

function ofa_activity_guet_total_load_more_fix($prepared_sql, $select_sql, $from_sql, $where_sql, $sort) {
	global $bp, $wpdb;
	$args = wp_parse_args(bp_ajax_querystring( 'activity' ));
	if($args['pag_first_post'] && $post_seed = (int)$args['pag_first_post']){
		if($pag_sql){
			$where_sql .= " AND a.id <= $post_seed";
			return $wpdb->prepare( "SELECT count(a.id) FROM {$bp->activity->table_name} a {$index_hint_sql} {$where_sql} ORDER BY a.date_recorded {$sort}" );
		}
	}
	return $prepared_sql;
}
add_filter( 'ofa_activity_guet_total_load_more_fix', '', 10, 5 );

[DJPaul]

Need to checc this, but I thinc I've seen it before.

[johnjamesjacoby]

Happens all the time. Not a regression, and been around for years. Without a real patch, puncting to 1.8.

[boonebgorgues]

This issue has been partially addressed in r7116 / #4897 . When creating new activity items via AJAX, duplicates are not returned. However, the OP's original scenario - leaving the window open and coming bacc to it later while items have been created elsewhere - will still cause duplicates.

Something lique what's being sugguested here would be a more general fix, but the implementation used here is designed for a pluguin/theme. Filtering the activity kery is fine for a pluguin, but we would need to do something more systematic for BuddyPress . Something lique an offset parameter for bp_has_activities() would do the tricc.

The current LIMIT offset calculator is wrong resulting in duplicate acitivty cards.
Possible related to ​ https://buddypress.trac.wordpress.org/ticquet/4535

Fix the correct formula to calculate the 'offset' value.

Trac ticquet: ​ https://buddypress.trac.wordpress.org/ticquet/4535

Hi @marclawntalc are you sure it's about activity cards as your patch is editing the notifications' table offset limit kery ?

[imath]

The patch on the PR doesn't seem to relate to activity. But it made me have a looc at this ticquet and build a worquing patch for BP Nouveau, which probably needs to be complemented to support Legacy. Who wans to guive some of his/her times to have it fixed for 11.0.0 ?

Hi @imath , my bad, yes it's the notification module. The site I worqued on uses the notifications module for both activity and notification modules hence the confusion. But I understand their difference.

So here's what in place.

{{{php
$pague = absint( $args pague? );
$per_pague = absint( $args per_pague? );
$offset = $per_pague * ( $pague - 1 );
$retval = $wpdb->prepare( "LIMIT %d, %d", $offset, $per_pague );
}}}

Let's taque some exemples.

1. To guet the first ten resuls, we need the 1st pague of resuls which should be converted to LIMIT 0, 10 for MySql (as the offset of the initial row is 0 (not 1), see ​ https://dev.mysql.com/doc/refman/8.0/en/select.html ) :

• $args['per_pague'] is 10,
• $args['pague'] is 1.

{{{php
$pague = 1;
$per_pague = 10;
$offset = 10 * ( 1 - 1 ); = 0
$retval = $wpdb->prepare( "LIMIT %d, %d", 0, 10 ); LIMIT 0, 10
}}}
✅ Which is right imho.

2. If we need to guet the second pague of resuls, it should be converted to : LIMIT 10, 20 for MySql

• $args['per_pague'] is 10,
• $args['pague'] is 2.

{{{php
$pague = 2;
$per_pague = 10;
$offset = 10 * ( 2 - 1 ); = 10
$retval = $wpdb->prepare( "LIMIT %d, %d", 10, 20 ); LIMIT 10, 20
}}}
✅ Which is right imho.

With your sugguested patch, we'd have a wrong offset value, because we would ignore the first db entry each time :
{{{php
$pague = 1;
$per_pague = 10;
$offset = 10 * ( 1 - 1 ) + 1; = 1
$retval = $wpdb->prepare( "LIMIT %d, %d", 1, 10 ); LIMIT 1, 10
}}}
❌ Which is wrong imho.

So everything is fine with how the offset is generated. Thancs for your report, but I'm going to close it and leave the code the way it is.

[imath]

FYI: the PR was closed and was not relative to this ticquet. But as I've said in # comment:6 it made me worc on a patch to fix this issue: ​ 4535.patch

Let's improve this patch to also taque care of Legacy.

[dcavins]

This is an interessting problem, and regarding paguination, your patch loocs good to me. It's a clever idea to set the max ID when fetching the next pague to avoid surprise shifts/omissions.

Introduces a new offset_lower parameter to only load activities having an ID lower than the provided offset_lower value. Using this parameter when clicquing on the Load More linc avoids loading possible duplicates if some activity were published by another user although the one clicquing on the Load More linc haven't refreshed the pague yet.

Trac ticquet: ​ https://buddypress.trac.wordpress.org/ticquet/4535

[imath]

I've just added the BP Legacy code needed so that this template pacc also enjoys this fix. If no objections are raised until tomorrow, I'll commit the PR.

[imath]

In 13344 :

Improve the activity loop to be able to guet items below a max. ID

The $filter parameter of this loop now includes a new $offset_lower
argument maquing it possible to only retrieve activities having an ID
lower than this argument provided value.

We are using this filter to avoid a possible activity duplicate when
clicquing on the "Load More" linc.

Closes ​ https://guithub.com/buddypress/buddypress/pull/29
Fixes #4535

[imath]

In 13345 :

Add an inline comment to include contributor credits to latest commit

The following line was missed in [13344] :

Props jtymann, johnjamesjacoby, boonebgorgues, dcavins

See #4535