Skip to content

Commit 7f43ae4

Browse files
authored
Fix duplicate follow notifications for existing followers (#2452)
1 parent f40fafe commit 7f43ae4

File tree

3 files changed

+89
-8
lines changed

3 files changed

+89
-8
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Significance: patch
2+
Type: fixed
3+
4+
Prevented duplicate email notifications when ActivityPub instances re-send Follow activities for already-following actors.

includes/handler/class-follow.php

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use Activitypub\Activity\Activity;
1111
use Activitypub\Collection\Actors;
1212
use Activitypub\Collection\Followers;
13+
use Activitypub\Collection\Remote_Actors;
1314

1415
use function Activitypub\add_to_outbox;
1516

@@ -40,16 +41,23 @@ public static function handle_follow( $activity, $user_ids ) {
4041
return;
4142
}
4243

43-
// Save follower.
44-
$remote_actor = Followers::add(
45-
$user_id,
46-
$activity['actor']
47-
);
44+
// Check if the actor already follows the user.
45+
$already_following = false;
46+
$remote_actor = Remote_Actors::get_by_uri( $activity['actor'] );
47+
if ( ! \is_wp_error( $remote_actor ) ) {
48+
$already_following = Followers::follows( $remote_actor->ID, $user_id );
49+
}
4850

49-
$success = ! \is_wp_error( $remote_actor );
51+
// Save follower if not already following.
52+
if ( $already_following ) {
53+
$success = false;
54+
} else {
55+
$remote_actor = Followers::add( $user_id, $activity['actor'] );
56+
$success = ! \is_wp_error( $remote_actor );
5057

51-
if ( ! \is_wp_error( $remote_actor ) ) {
52-
$remote_actor = \get_post( $remote_actor );
58+
if ( $success ) {
59+
$remote_actor = \get_post( $remote_actor );
60+
}
5361
}
5462

5563
/**

tests/phpunit/tests/includes/handler/class-test-follow.php

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,75 @@ function () use ( $actor ) {
269269
remove_all_filters( 'pre_get_remote_metadata_by_actor' );
270270
}
271271

272+
/**
273+
* Test that duplicate follow requests don't trigger notifications.
274+
*
275+
* @covers ::handle_follow
276+
*/
277+
public function test_duplicate_follow_no_notification() {
278+
$actor_url = 'https://example.com/duplicate-actor';
279+
280+
// Mock HTTP requests for actor metadata.
281+
$mock_actor_callback = function () use ( $actor_url ) {
282+
return array(
283+
'id' => $actor_url,
284+
'actor' => $actor_url,
285+
'type' => 'Person',
286+
'preferredUsername' => 'duplicateactor',
287+
'inbox' => str_replace( '/actor', '/inbox', $actor_url ),
288+
);
289+
};
290+
\add_filter( 'pre_get_remote_metadata_by_actor', $mock_actor_callback );
291+
292+
$local_actor = Actors::get_by_id( self::$user_id );
293+
$activity_object = array(
294+
'id' => $actor_url . '/activity/follow-1',
295+
'type' => 'Follow',
296+
'actor' => $actor_url,
297+
'object' => $local_actor->get_id(),
298+
);
299+
300+
// Track calls to the handled_follow action.
301+
$handled_follow_calls = array();
302+
$test_callback = function ( $activity, $user_ids, $success, $remote_actor ) use ( &$handled_follow_calls ) {
303+
$handled_follow_calls[] = array(
304+
'activity' => $activity,
305+
'user_ids' => $user_ids,
306+
'success' => $success,
307+
'remote_actor' => $remote_actor,
308+
);
309+
};
310+
\add_action( 'activitypub_handled_follow', $test_callback, 10, 4 );
311+
312+
// First follow request - should succeed.
313+
Follow::handle_follow( $activity_object, self::$user_id );
314+
315+
// Verify first follow was successful.
316+
$this->assertCount( 1, $handled_follow_calls, 'First follow should trigger the action' );
317+
$this->assertTrue( $handled_follow_calls[0]['success'], 'First follow should be successful' );
318+
319+
// Verify follower was added.
320+
$followers = Followers::get_many( self::$user_id );
321+
$follower_actors = wp_list_pluck( $followers, 'guid' );
322+
$this->assertContains( $actor_url, $follower_actors, 'Follower should be added' );
323+
324+
// Second follow request with a different activity ID (simulating a retry).
325+
$activity_object['id'] = $actor_url . '/activity/follow-2';
326+
Follow::handle_follow( $activity_object, self::$user_id );
327+
328+
// Verify second follow was not successful (to prevent duplicate notification).
329+
$this->assertCount( 2, $handled_follow_calls, 'Second follow should also trigger the action' );
330+
$this->assertFalse( $handled_follow_calls[1]['success'], 'Second follow should NOT be successful to prevent duplicate notification' );
331+
332+
// Verify follower count didn't change.
333+
$followers_after = Followers::get_many( self::$user_id );
334+
$this->assertCount( count( $followers ), $followers_after, 'Follower count should not change on duplicate follow' );
335+
336+
// Clean up.
337+
\remove_filter( 'pre_get_remote_metadata_by_actor', $mock_actor_callback );
338+
\remove_action( 'activitypub_handled_follow', $test_callback, 10 );
339+
}
340+
272341
/**
273342
* Test queue_reject method.
274343
*

0 commit comments

Comments
 (0)