Skip to content

Commit 58c223b

Browse files
pfefferlepfefferle
authored
Merge branch 'trunk' into fix/tag-name-handling
2 parents 57bb02c + ba72c37 commit 58c223b

File tree

12 files changed

+163
-26
lines changed

12 files changed

+163
-26
lines changed

‎.github/changelog/2532-from-description‎

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+
False mention email notifications for users in CC field without actual mention tags.

‎includes/class-mailer.php‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -327,8 +327,8 @@ public static function direct_message( $activity, $user_ids ) {
327327
* @param int|int[] $user_ids The id(s) of the local blog-user(s).
328328
*/
329329
public static function mention( $activity, $user_ids ) {
330-
// Early return if activity has no cc recipients.
331-
if ( empty( $activity['cc'] ) ) {
330+
// Early return if activity has no mentions.
331+
if ( empty( $activity['object']['tag'] ) ) {
332332
return;
333333
}
334334

@@ -337,19 +337,17 @@ public static function mention( $activity, $user_ids ) {
337337
return;
338338
}
339339

340-
// Normalize to array.
341-
$user_ids = (array) $user_ids;
342-
343-
// Build a map of user_id => actor_id and filter to only users in the "cc" field.
344340
$recipients = array();
345-
foreach ( $user_ids as $user_id ) {
341+
$mentions = wp_list_filter( (array) $activity['object']['tag'], array( 'type' => 'Mention' ) );
342+
$mentions = array_map( '\Activitypub\object_to_uri', $mentions );
343+
foreach ( (array) $user_ids as $user_id ) {
346344
$actor = Actors::get_by_id( $user_id );
347345
if ( \is_wp_error( $actor ) ) {
348346
continue;
349347
}
350348

351349
$actor_id = $actor->get_id();
352-
if ( \in_array( $actor_id, (array) $activity['cc'], true ) ) {
350+
if ( \in_array( $actor_id, $mentions, true ) ) {
353351
$recipients[ $user_id ] = $actor_id;
354352
}
355353
}

‎includes/functions.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -809,9 +809,12 @@ function object_to_uri( $data ) {
809809
case 'Video': // See https://www.w3.org/TR/activitystreams-vocabulary/#dfn-video.
810810
$data = object_to_uri( $data['url'] );
811811
break;
812+
812813
case 'Link': // See https://www.w3.org/TR/activitystreams-vocabulary/#dfn-link.
814+
case 'Mention': // See https://www.w3.org/TR/activitystreams-vocabulary/#dfn-mention.
813815
$data = $data['href'];
814816
break;
817+
815818
default:
816819
$data = $data['id'];
817820
break;

‎tests/phpunit/tests/includes/class-test-mailer.php‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -814,6 +814,13 @@ public function test_mention_with_array() {
814814
'object' => array(
815815
'id' => 'https://example.com/post/1',
816816
'content' => 'Test mention',
817+
'tag' => array(
818+
array(
819+
'type' => 'Mention',
820+
'href' => Actors::get_by_id( $user_id )->get_id(),
821+
'name' => '@test',
822+
),
823+
),
817824
),
818825
'cc' => array( Actors::get_by_id( $user_id )->get_id() ),
819826
);
@@ -935,6 +942,13 @@ public function test_mention_filters_recipients() {
935942
'object' => array(
936943
'id' => 'https://example.com/post/1',
937944
'content' => 'Test mention',
945+
'tag' => array(
946+
array(
947+
'type' => 'Mention',
948+
'href' => Actors::get_by_id( $user_id )->get_id(),
949+
'name' => '@test',
950+
),
951+
),
938952
),
939953
// Only user_id is in CC, not other_user_id.
940954
'cc' => array( Actors::get_by_id( $user_id )->get_id() ),
@@ -1115,4 +1129,122 @@ public function test_respect_existing_notification_settings() {
11151129
// Clean up.
11161130
\delete_option( 'activitypub_create_posts' );
11171131
}
1132+
1133+
/**
1134+
* Test that users in CC without actual mention tags do not receive mention notifications.
1135+
*
1136+
* This tests the bug fix where users added to CC (e.g., because they follow the actor)
1137+
* were incorrectly receiving mention notifications even when not actually mentioned.
1138+
*
1139+
* @covers ::mention
1140+
*/
1141+
public function test_mention_requires_tag_not_just_cc() {
1142+
$user_id = self::$user_id;
1143+
1144+
// Activity with user in CC but NOT mentioned in tags.
1145+
$activity = array(
1146+
'actor' => 'https://example.com/sports-account',
1147+
'object' => array(
1148+
'id' => 'https://example.com/sports-account/posts/123',
1149+
'type' => 'Note',
1150+
'content' => '<p>Join @user1 and @user2 on our stream...</p>',
1151+
'tag' => array(
1152+
// Other users mentioned, but NOT the local user.
1153+
array(
1154+
'type' => 'Mention',
1155+
'href' => 'https://example.com/user1',
1156+
'name' => '[email protected]',
1157+
),
1158+
array(
1159+
'type' => 'Mention',
1160+
'href' => 'https://example.com/user2',
1161+
'name' => '[email protected]',
1162+
),
1163+
),
1164+
),
1165+
// User is in CC (e.g., because they follow the actor).
1166+
'cc' => array( Actors::get_by_id( $user_id )->get_id() ),
1167+
);
1168+
1169+
// Mock remote metadata.
1170+
$metadata_filter = function () {
1171+
return array(
1172+
'name' => 'Sports Account',
1173+
'url' => 'https://example.com/sports-account',
1174+
);
1175+
};
1176+
\add_filter( 'pre_get_remote_metadata_by_actor', $metadata_filter );
1177+
1178+
$mock = new \MockAction();
1179+
\add_filter( 'wp_mail', array( $mock, 'filter' ), 1 );
1180+
1181+
// Trigger mention notification.
1182+
Mailer::mention( $activity, $user_id );
1183+
1184+
// Should NOT send any email because user is not actually mentioned in tags.
1185+
$this->assertEquals( 0, $mock->get_call_count(), 'User in CC without mention tag should not receive notification' );
1186+
1187+
// Clean up.
1188+
\remove_filter( 'pre_get_remote_metadata_by_actor', $metadata_filter );
1189+
\remove_filter( 'wp_mail', array( $mock, 'filter' ), 1 );
1190+
}
1191+
1192+
/**
1193+
* Test that users with actual mention tags DO receive mention notifications.
1194+
*
1195+
* @covers ::mention
1196+
*/
1197+
public function test_mention_with_tag_sends_notification() {
1198+
$user_id = self::$user_id;
1199+
1200+
// Activity with user properly mentioned in both CC and tags.
1201+
$activity = array(
1202+
'actor' => 'https://example.com/author',
1203+
'object' => array(
1204+
'id' => 'https://example.com/post/1',
1205+
'type' => 'Note',
1206+
'content' => '<p>Hello @testuser, how are you?</p>',
1207+
'tag' => array(
1208+
array(
1209+
'type' => 'Mention',
1210+
'href' => Actors::get_by_id( $user_id )->get_id(),
1211+
'name' => '@testuser',
1212+
),
1213+
),
1214+
),
1215+
'cc' => array( Actors::get_by_id( $user_id )->get_id() ),
1216+
);
1217+
1218+
// Mock remote metadata.
1219+
$metadata_filter = function () {
1220+
return array(
1221+
'name' => 'Test Author',
1222+
'url' => 'https://example.com/author',
1223+
);
1224+
};
1225+
\add_filter( 'pre_get_remote_metadata_by_actor', $metadata_filter );
1226+
1227+
$mock = new \MockAction();
1228+
\add_filter( 'wp_mail', array( $mock, 'filter' ), 1 );
1229+
1230+
// Capture email.
1231+
$mail_filter = function ( $args ) use ( $user_id ) {
1232+
$this->assertStringContainsString( 'Mention', $args['subject'] );
1233+
$this->assertStringContainsString( 'Test Author', $args['subject'] );
1234+
$this->assertEquals( \get_user_by( 'id', $user_id )->user_email, $args['to'] );
1235+
return $args;
1236+
};
1237+
\add_filter( 'wp_mail', $mail_filter );
1238+
1239+
// Trigger mention notification.
1240+
Mailer::mention( $activity, $user_id );
1241+
1242+
// Should send 1 email because user is properly mentioned.
1243+
$this->assertEquals( 1, $mock->get_call_count(), 'User properly mentioned in tags should receive notification' );
1244+
1245+
// Clean up.
1246+
\remove_filter( 'pre_get_remote_metadata_by_actor', $metadata_filter );
1247+
\remove_filter( 'wp_mail', array( $mock, 'filter' ), 1 );
1248+
\remove_filter( 'wp_mail', $mail_filter );
1249+
}
11181250
}

‎tests/phpunit/tests/includes/class-test-search.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ class Test_Search extends \WP_UnitTestCase {
1818
*/
1919
public function test_regular_search_unchanged() {
2020
// Create a test post.
21-
$post_id = $this->factory->post->create(
21+
$post_id = self::factory()->post->create(
2222
array(
2323
'post_title' => 'Test Post About Cats',
2424
'post_content' => 'This is a test post about cats and dogs.',

‎tests/phpunit/tests/includes/handler/class-test-accept.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ public function test_handle_accept_moves_user_from_pending_to_following() {
122122
$object_guid = 'https://example.com/actor/123';
123123
$outbox_guid = 'https://example.com/outbox/123';
124124

125-
$outbox_post_id = $this->factory->post->create(
125+
$outbox_post_id = self::factory()->post->create(
126126
array(
127127
'post_type' => 'ap_outbox',
128128
'post_status' => 'publish',
@@ -133,7 +133,7 @@ public function test_handle_accept_moves_user_from_pending_to_following() {
133133
\add_post_meta( $outbox_post_id, '_activitypub_activity_type', 'Follow' );
134134

135135
// Create remote actor post.
136-
$post_id = $this->factory->post->create(
136+
$post_id = self::factory()->post->create(
137137
array(
138138
'post_type' => Remote_Actors::POST_TYPE,
139139
'post_status' => 'publish',

‎tests/phpunit/tests/includes/handler/class-test-reject.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ public function test_handle_reject_keeps_user_in_pending() {
123123
$object_guid = 'https://example.com/actor/123';
124124
$outbox_guid = 'https://example.com/outbox/123';
125125

126-
$outbox_post_id = $this->factory->post->create(
126+
$outbox_post_id = self::factory()->post->create(
127127
array(
128128
'post_type' => Outbox::POST_TYPE,
129129
'post_status' => 'publish',
@@ -134,7 +134,7 @@ public function test_handle_reject_keeps_user_in_pending() {
134134
\add_post_meta( $outbox_post_id, '_activitypub_activity_type', 'Follow' );
135135

136136
// Create remote actor post.
137-
$post_id = $this->factory->post->create(
137+
$post_id = self::factory()->post->create(
138138
array(
139139
'post_type' => Remote_Actors::POST_TYPE,
140140
'post_status' => 'publish',

‎tests/phpunit/tests/includes/handler/class-test-undo.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ public function test_handle_undo_comment_activities( $actor_url, $activity_type,
175175
\add_filter( 'pre_get_remote_metadata_by_actor', $mock_actor_metadata );
176176

177177
// Create a test post.
178-
$post_id = $this->factory->post->create(
178+
$post_id = self::factory()->post->create(
179179
array(
180180
'post_author' => self::$user_id,
181181
'post_title' => 'Test Post for ' . $description,

‎tests/phpunit/tests/includes/transformer/class-test-base.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ public function get_object_var_keys() {
8484
* @param array $expected_audience The expected audience.
8585
*/
8686
public function test_set_audience( $content_visibility, $object_attributes, $expected_audience ) {
87-
$post_id = $this->factory->post->create(
87+
$post_id = self::factory()->post->create(
8888
array(
8989
'post_title' => 'Test Post',
9090
'post_content' => 'Test content that is longer than the note length limit',

‎tests/phpunit/tests/includes/transformer/class-test-post.php‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public function tear_down() {
5555
public function test_get_type_returns_configured_type_when_option_set() {
5656
update_option( 'activitypub_object_type', 'Article' );
5757

58-
$post_id = $this->factory->post->create(
58+
$post_id = self::factory()->post->create(
5959
array(
6060
'post_title' => 'Test Post',
6161
'post_content' => 'Test content that is longer than the note length limit',
@@ -81,7 +81,7 @@ public function test_get_type_returns_configured_type_when_option_set() {
8181
* @param string $description Description of the test case.
8282
*/
8383
public function test_get_type( $post_data, $post_format, $expected_type, $description ) {
84-
$post_id = $this->factory->post->create( $post_data );
84+
$post_id = self::factory()->post->create( $post_data );
8585

8686
if ( $post_format ) {
8787
set_post_format( $post_id, $post_format );
@@ -180,7 +180,7 @@ public function test_get_type_respects_post_type_title_support() {
180180
)
181181
);
182182

183-
$post_id = $this->factory->post->create(
183+
$post_id = self::factory()->post->create(
184184
array(
185185
'post_title' => 'Test Post',
186186
'post_content' => str_repeat( 'Long content. ', 100 ),
@@ -213,7 +213,7 @@ public function test_get_type_respects_post_format_support() {
213213
)
214214
);
215215

216-
$post_id = $this->factory->post->create(
216+
$post_id = self::factory()->post->create(
217217
array(
218218
'post_title' => 'Test Post',
219219
'post_content' => str_repeat( 'Long content. ', 100 ),
@@ -496,7 +496,7 @@ public function test_get_media_from_blocks_handles_multiple_blocks() {
496496
* @covers ::get_icon
497497
*/
498498
public function test_get_icon() {
499-
$post_id = $this->factory->post->create(
499+
$post_id = self::factory()->post->create(
500500
array(
501501
'post_title' => 'Test Post',
502502
'post_content' => 'Test content',
@@ -620,7 +620,7 @@ public function create_upload_object( $file, $parent_id = 0 ) {
620620
*/
621621
public function test_preview_property() {
622622
// Create a test post of type "Article".
623-
$post = $this->factory->post->create_and_get(
623+
$post = self::factory()->post->create_and_get(
624624
array(
625625
'post_title' => 'Test Article',
626626
'post_content' => str_repeat( 'Long content. ', 100 ),
@@ -638,7 +638,7 @@ public function test_preview_property() {
638638
$this->assertNotEmpty( $preview['content'] );
639639

640640
// Create a test post of type "Note" (short content).
641-
$note_post = $this->factory->post->create_and_get(
641+
$note_post = self::factory()->post->create_and_get(
642642
array(
643643
'post_title' => '',
644644
'post_content' => 'Short note content',

0 commit comments

Comments
 (0)