diff --git a/.github/changelog/2532-from-description b/.github/changelog/2532-from-description new file mode 100644 index 000000000..15fa5fd9d --- /dev/null +++ b/.github/changelog/2532-from-description @@ -0,0 +1,4 @@ +Significance: patch +Type: fixed + +False mention email notifications for users in CC field without actual mention tags. diff --git a/includes/class-mailer.php b/includes/class-mailer.php index 1acd0e229..052cb6b5a 100644 --- a/includes/class-mailer.php +++ b/includes/class-mailer.php @@ -327,8 +327,8 @@ public static function direct_message( $activity, $user_ids ) { * @param int|int[] $user_ids The id(s) of the local blog-user(s). */ public static function mention( $activity, $user_ids ) { - // Early return if activity has no cc recipients. - if ( empty( $activity['cc'] ) ) { + // Early return if activity has no mentions. + if ( empty( $activity['object']['tag'] ) ) { return; } @@ -337,19 +337,17 @@ public static function mention( $activity, $user_ids ) { return; } - // Normalize to array. - $user_ids = (array) $user_ids; - - // Build a map of user_id => actor_id and filter to only users in the "cc" field. $recipients = array(); - foreach ( $user_ids as $user_id ) { + $mentions = wp_list_filter( (array) $activity['object']['tag'], array( 'type' => 'Mention' ) ); + $mentions = array_map( '\Activitypub\object_to_uri', $mentions ); + foreach ( (array) $user_ids as $user_id ) { $actor = Actors::get_by_id( $user_id ); if ( \is_wp_error( $actor ) ) { continue; } $actor_id = $actor->get_id(); - if ( \in_array( $actor_id, (array) $activity['cc'], true ) ) { + if ( \in_array( $actor_id, $mentions, true ) ) { $recipients[ $user_id ] = $actor_id; } } diff --git a/includes/functions.php b/includes/functions.php index 20a786b7c..71f73e4b6 100644 --- a/includes/functions.php +++ b/includes/functions.php @@ -809,9 +809,12 @@ function object_to_uri( $data ) { case 'Video': // See https://www.w3.org/TR/activitystreams-vocabulary/#dfn-video. $data = object_to_uri( $data['url'] ); break; + case 'Link': // See https://www.w3.org/TR/activitystreams-vocabulary/#dfn-link. + case 'Mention': // See https://www.w3.org/TR/activitystreams-vocabulary/#dfn-mention. $data = $data['href']; break; + default: $data = $data['id']; break; diff --git a/tests/phpunit/tests/includes/class-test-mailer.php b/tests/phpunit/tests/includes/class-test-mailer.php index 09aecbcdf..c3a747008 100644 --- a/tests/phpunit/tests/includes/class-test-mailer.php +++ b/tests/phpunit/tests/includes/class-test-mailer.php @@ -814,6 +814,13 @@ public function test_mention_with_array() { 'object' => array( 'id' => 'https://example.com/post/1', 'content' => 'Test mention', + 'tag' => array( + array( + 'type' => 'Mention', + 'href' => Actors::get_by_id( $user_id )->get_id(), + 'name' => '@test', + ), + ), ), 'cc' => array( Actors::get_by_id( $user_id )->get_id() ), ); @@ -935,6 +942,13 @@ public function test_mention_filters_recipients() { 'object' => array( 'id' => 'https://example.com/post/1', 'content' => 'Test mention', + 'tag' => array( + array( + 'type' => 'Mention', + 'href' => Actors::get_by_id( $user_id )->get_id(), + 'name' => '@test', + ), + ), ), // Only user_id is in CC, not other_user_id. 'cc' => array( Actors::get_by_id( $user_id )->get_id() ), @@ -1115,4 +1129,122 @@ public function test_respect_existing_notification_settings() { // Clean up. \delete_option( 'activitypub_create_posts' ); } + + /** + * Test that users in CC without actual mention tags do not receive mention notifications. + * + * This tests the bug fix where users added to CC (e.g., because they follow the actor) + * were incorrectly receiving mention notifications even when not actually mentioned. + * + * @covers ::mention + */ + public function test_mention_requires_tag_not_just_cc() { + $user_id = self::$user_id; + + // Activity with user in CC but NOT mentioned in tags. + $activity = array( + 'actor' => 'https://example.com/sports-account', + 'object' => array( + 'id' => 'https://example.com/sports-account/posts/123', + 'type' => 'Note', + 'content' => '

Join @user1 and @user2 on our stream...

', + 'tag' => array( + // Other users mentioned, but NOT the local user. + array( + 'type' => 'Mention', + 'href' => 'https://example.com/user1', + 'name' => 'user1@example.com', + ), + array( + 'type' => 'Mention', + 'href' => 'https://example.com/user2', + 'name' => 'user2@example.com', + ), + ), + ), + // User is in CC (e.g., because they follow the actor). + 'cc' => array( Actors::get_by_id( $user_id )->get_id() ), + ); + + // Mock remote metadata. + $metadata_filter = function () { + return array( + 'name' => 'Sports Account', + 'url' => 'https://example.com/sports-account', + ); + }; + \add_filter( 'pre_get_remote_metadata_by_actor', $metadata_filter ); + + $mock = new \MockAction(); + \add_filter( 'wp_mail', array( $mock, 'filter' ), 1 ); + + // Trigger mention notification. + Mailer::mention( $activity, $user_id ); + + // Should NOT send any email because user is not actually mentioned in tags. + $this->assertEquals( 0, $mock->get_call_count(), 'User in CC without mention tag should not receive notification' ); + + // Clean up. + \remove_filter( 'pre_get_remote_metadata_by_actor', $metadata_filter ); + \remove_filter( 'wp_mail', array( $mock, 'filter' ), 1 ); + } + + /** + * Test that users with actual mention tags DO receive mention notifications. + * + * @covers ::mention + */ + public function test_mention_with_tag_sends_notification() { + $user_id = self::$user_id; + + // Activity with user properly mentioned in both CC and tags. + $activity = array( + 'actor' => 'https://example.com/author', + 'object' => array( + 'id' => 'https://example.com/post/1', + 'type' => 'Note', + 'content' => '

Hello @testuser, how are you?

', + 'tag' => array( + array( + 'type' => 'Mention', + 'href' => Actors::get_by_id( $user_id )->get_id(), + 'name' => '@testuser', + ), + ), + ), + 'cc' => array( Actors::get_by_id( $user_id )->get_id() ), + ); + + // Mock remote metadata. + $metadata_filter = function () { + return array( + 'name' => 'Test Author', + 'url' => 'https://example.com/author', + ); + }; + \add_filter( 'pre_get_remote_metadata_by_actor', $metadata_filter ); + + $mock = new \MockAction(); + \add_filter( 'wp_mail', array( $mock, 'filter' ), 1 ); + + // Capture email. + $mail_filter = function ( $args ) use ( $user_id ) { + $this->assertStringContainsString( 'Mention', $args['subject'] ); + $this->assertStringContainsString( 'Test Author', $args['subject'] ); + $this->assertEquals( \get_user_by( 'id', $user_id )->user_email, $args['to'] ); + return $args; + }; + \add_filter( 'wp_mail', $mail_filter ); + + // Trigger mention notification. + Mailer::mention( $activity, $user_id ); + + // Should send 1 email because user is properly mentioned. + $this->assertEquals( 1, $mock->get_call_count(), 'User properly mentioned in tags should receive notification' ); + + // Clean up. + \remove_filter( 'pre_get_remote_metadata_by_actor', $metadata_filter ); + \remove_filter( 'wp_mail', array( $mock, 'filter' ), 1 ); + \remove_filter( 'wp_mail', $mail_filter ); + } }