From db3d73fb0ab103866a764d260556dfdfd6c243e1 Mon Sep 17 00:00:00 2001 From: huoxin <23447157+huoxin233@users.noreply.github.com> Date: Thu, 23 Jul 2026 03:33:28 +0800 Subject: [PATCH 1/4] fix(mentions): prevent deleted fallback for unsynced posts during formatting During the Saving/Revised lifecycle (such as editing a post), the mentions pivot tables may not be fully synced or accessible before the API serializes the response. When formatContent() or unparse() is called, the fallback logic in the formatting classes was attempting to execute a relationship pivot query ($context->mentionsPosts()->find()) if the relation wasn't eager loaded. Because the pivot table was not completely synced, this query failed, causing the formatter to incorrectly resolve valid mentions to the deleted_text fallback. This fix removes the pivot table constraint when the relationship is not eagerly loaded, directly querying the underlying model instead. This also resolves an N+1 query bottleneck when unparsing posts without eager loaded mentions. Co-Authored-By: Claude Opus 4.6 --- extensions/mentions/src/Formatter/FormatGroupMentions.php | 4 +--- extensions/mentions/src/Formatter/FormatPostMentions.php | 4 +--- extensions/mentions/src/Formatter/FormatTagMentions.php | 4 +--- extensions/mentions/src/Formatter/FormatUserMentions.php | 4 +--- extensions/mentions/src/Formatter/UnparsePostMentions.php | 4 +--- extensions/mentions/src/Formatter/UnparseTagMentions.php | 4 +--- extensions/mentions/src/Formatter/UnparseUserMentions.php | 4 +--- 7 files changed, 7 insertions(+), 21 deletions(-) diff --git a/extensions/mentions/src/Formatter/FormatGroupMentions.php b/extensions/mentions/src/Formatter/FormatGroupMentions.php index b6ad163170..be8bddcd63 100644 --- a/extensions/mentions/src/Formatter/FormatGroupMentions.php +++ b/extensions/mentions/src/Formatter/FormatGroupMentions.php @@ -27,9 +27,7 @@ public function __invoke(Renderer $renderer, mixed $context, string $xml): strin return Utils::replaceAttributes($xml, 'GROUPMENTION', function ($attributes) use ($context) { /** @var Group|null $group */ $group = match (true) { - $context instanceof AbstractModel && $context->isRelation('mentionsGroups') => $context->relationLoaded('mentionsGroups') - ? $context->mentionsGroups->find($attributes['id']) // @phpstan-ignore-line - : $context->mentionsGroups()->find($attributes['id']), // @phpstan-ignore-line + $context instanceof AbstractModel && $context->relationLoaded('mentionsGroups') => $context->mentionsGroups->find($attributes['id']), // @phpstan-ignore-line default => Group::query()->find($attributes['id']), }; diff --git a/extensions/mentions/src/Formatter/FormatPostMentions.php b/extensions/mentions/src/Formatter/FormatPostMentions.php index 75a20ba368..9140d74fca 100644 --- a/extensions/mentions/src/Formatter/FormatPostMentions.php +++ b/extensions/mentions/src/Formatter/FormatPostMentions.php @@ -33,9 +33,7 @@ public function __invoke(Renderer $renderer, mixed $context, string $xml): strin return Utils::replaceAttributes($xml, 'POSTMENTION', function ($attributes) use ($context) { /** @var Post|null $post */ $post = match (true) { - $context instanceof AbstractModel && $context->isRelation('mentionsPosts') => $context->relationLoaded('mentionsPosts') - ? $context->mentionsPosts->find($attributes['id']) // @phpstan-ignore-line - : $context->mentionsPosts()->find($attributes['id']), // @phpstan-ignore-line + $context instanceof AbstractModel && $context->relationLoaded('mentionsPosts') => $context->mentionsPosts->find($attributes['id']), // @phpstan-ignore-line default => Post::query()->find($attributes['id']), }; diff --git a/extensions/mentions/src/Formatter/FormatTagMentions.php b/extensions/mentions/src/Formatter/FormatTagMentions.php index 6cad73ca6c..ca64181786 100644 --- a/extensions/mentions/src/Formatter/FormatTagMentions.php +++ b/extensions/mentions/src/Formatter/FormatTagMentions.php @@ -22,9 +22,7 @@ public function __invoke(Renderer $renderer, mixed $context, string $xml, ?Reque return Utils::replaceAttributes($xml, 'TAGMENTION', function ($attributes) use ($context) { /** @var Tag|null $tag */ $tag = match (true) { - $context instanceof AbstractModel && $context->isRelation('mentionsTags') => $context->relationLoaded('mentionsTags') - ? $context->mentionsTags->find($attributes['id']) // @phpstan-ignore-line - : $context->mentionsTags()->find($attributes['id']), // @phpstan-ignore-line + $context instanceof AbstractModel && $context->relationLoaded('mentionsTags') => $context->mentionsTags->find($attributes['id']), // @phpstan-ignore-line default => Tag::query()->find($attributes['id']), }; diff --git a/extensions/mentions/src/Formatter/FormatUserMentions.php b/extensions/mentions/src/Formatter/FormatUserMentions.php index fbbbe28f73..9658b98e1e 100644 --- a/extensions/mentions/src/Formatter/FormatUserMentions.php +++ b/extensions/mentions/src/Formatter/FormatUserMentions.php @@ -29,9 +29,7 @@ public function __invoke(Renderer $renderer, mixed $context, string $xml): strin return Utils::replaceAttributes($xml, 'USERMENTION', function ($attributes) use ($context) { /** @var User|null $user */ $user = match (true) { - $context instanceof AbstractModel && $context->isRelation('mentionsUsers') => $context->relationLoaded('mentionsUsers') - ? $context->mentionsUsers->find($attributes['id']) // @phpstan-ignore-line - : $context->mentionsUsers()->find($attributes['id']), // @phpstan-ignore-line + $context instanceof AbstractModel && $context->relationLoaded('mentionsUsers') => $context->mentionsUsers->find($attributes['id']), // @phpstan-ignore-line default => User::query()->find($attributes['id']), }; diff --git a/extensions/mentions/src/Formatter/UnparsePostMentions.php b/extensions/mentions/src/Formatter/UnparsePostMentions.php index ee9f0d24d4..d1a59d5bf4 100644 --- a/extensions/mentions/src/Formatter/UnparsePostMentions.php +++ b/extensions/mentions/src/Formatter/UnparsePostMentions.php @@ -40,9 +40,7 @@ protected function updatePostMentionTags(mixed $context, string $xml): string return Utils::replaceAttributes($xml, 'POSTMENTION', function ($attributes) use ($context) { /** @var Post|null $post */ $post = match (true) { - $context instanceof AbstractModel && $context->isRelation('mentionsPosts') => $context->relationLoaded('mentionsPosts') - ? $context->mentionsPosts->find($attributes['id']) // @phpstan-ignore-line - : $context->mentionsPosts()->find($attributes['id']), // @phpstan-ignore-line + $context instanceof AbstractModel && $context->relationLoaded('mentionsPosts') => $context->mentionsPosts->find($attributes['id']), // @phpstan-ignore-line default => Post::query()->find($attributes['id']), }; diff --git a/extensions/mentions/src/Formatter/UnparseTagMentions.php b/extensions/mentions/src/Formatter/UnparseTagMentions.php index 8623f47985..d4a19c71dd 100644 --- a/extensions/mentions/src/Formatter/UnparseTagMentions.php +++ b/extensions/mentions/src/Formatter/UnparseTagMentions.php @@ -34,9 +34,7 @@ protected function updateTagMentionTags(mixed $context, string $xml): string return Utils::replaceAttributes($xml, 'TAGMENTION', function (array $attributes) use ($context) { /** @var Tag|null $tag */ $tag = match (true) { - $context instanceof AbstractModel && $context->isRelation('mentionsTags') => $context->relationLoaded('mentionsTags') - ? $context->mentionsTags->find($attributes['id']) // @phpstan-ignore-line - : $context->mentionsTags()->find($attributes['id']), // @phpstan-ignore-line + $context instanceof AbstractModel && $context->relationLoaded('mentionsTags') => $context->mentionsTags->find($attributes['id']), // @phpstan-ignore-line default => Tag::query()->find($attributes['id']), }; diff --git a/extensions/mentions/src/Formatter/UnparseUserMentions.php b/extensions/mentions/src/Formatter/UnparseUserMentions.php index b40b1a5615..14bc1834dc 100644 --- a/extensions/mentions/src/Formatter/UnparseUserMentions.php +++ b/extensions/mentions/src/Formatter/UnparseUserMentions.php @@ -40,9 +40,7 @@ protected function updateUserMentionTags(mixed $context, string $xml): string return Utils::replaceAttributes($xml, 'USERMENTION', function ($attributes) use ($context) { /** @var User|null $user */ $user = match (true) { - $context instanceof AbstractModel && $context->isRelation('mentionsUsers') => $context->relationLoaded('mentionsUsers') - ? $context->mentionsUsers->find($attributes['id']) // @phpstan-ignore-line - : $context->mentionsUsers()->find($attributes['id']), // @phpstan-ignore-line + $context instanceof AbstractModel && $context->relationLoaded('mentionsUsers') => $context->mentionsUsers->find($attributes['id']), // @phpstan-ignore-line default => User::query()->find($attributes['id']), }; From 5f6aab8cfab865e47269ab4623094ce6309cc4ed Mon Sep 17 00:00:00 2001 From: huoxin <23447157+huoxin233@users.noreply.github.com> Date: Thu, 23 Jul 2026 04:20:29 +0800 Subject: [PATCH 2/4] test(mentions): fix false positive in deleted author edit tests The `editing_a_post_with_deleted_author_that_has_a_mention_works` tests were incorrectly submitting a `PATCH` payload with the exact same content as the unparsed original post text. Because the text was identical, Flarum correctly optimized away the `Revised` event, which meant the database pivot tables were never synced and the test failed on the database assertion. This fixes the test by actually mutating the content string during the edit, ensuring the `Revised` event is fired and the pivot tables are populated so they can be properly asserted. Co-Authored-By: Claude Opus 4.6 --- .../mentions/tests/integration/api/PostMentionsTest.php | 4 ++-- .../mentions/tests/integration/api/UserMentionsTest.php | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/extensions/mentions/tests/integration/api/PostMentionsTest.php b/extensions/mentions/tests/integration/api/PostMentionsTest.php index 0b9cd9c8e1..c07a77089a 100644 --- a/extensions/mentions/tests/integration/api/PostMentionsTest.php +++ b/extensions/mentions/tests/integration/api/PostMentionsTest.php @@ -480,7 +480,7 @@ public function editing_a_post_with_deleted_author_that_has_a_mention_works() 'data' => [ 'type' => 'posts', 'attributes' => [ - 'content' => '@"Bad _ User"#p9', + 'content' => '@"Bad _ User"#p9 edited', ], ], ], @@ -494,7 +494,7 @@ public function editing_a_post_with_deleted_author_that_has_a_mention_works() $response = json_decode($body, true); $this->assertStringContainsString('Bad "#p6 User', $response['data']['attributes']['contentHtml']); - $this->assertEquals('@"Bad _ User"#p9', $response['data']['attributes']['content']); + $this->assertEquals('@"Bad _ User"#p9 edited', $response['data']['attributes']['content']); $this->assertStringContainsString('PostMention', $response['data']['attributes']['contentHtml']); $this->assertNotNull(CommentPost::find($response['data']['id'])->mentionsPosts->find(9)); } diff --git a/extensions/mentions/tests/integration/api/UserMentionsTest.php b/extensions/mentions/tests/integration/api/UserMentionsTest.php index 962eb59df8..db0b3983d4 100644 --- a/extensions/mentions/tests/integration/api/UserMentionsTest.php +++ b/extensions/mentions/tests/integration/api/UserMentionsTest.php @@ -465,7 +465,7 @@ public function editing_a_post_with_deleted_author_that_has_a_mention_works() 'data' => [ 'type' => 'posts', 'attributes' => [ - 'content' => '@"Bad _ User"#5', + 'content' => '@"Bad _ User"#5 edited', ], ], ], @@ -479,7 +479,7 @@ public function editing_a_post_with_deleted_author_that_has_a_mention_works() $response = json_decode($body, true); $this->assertStringContainsString('Bad "#p6 User', $response['data']['attributes']['contentHtml']); - $this->assertEquals('@"Bad _ User"#5', $response['data']['attributes']['content']); + $this->assertEquals('@"Bad _ User"#5 edited', $response['data']['attributes']['content']); $this->assertStringContainsString('UserMention', $response['data']['attributes']['contentHtml']); $this->assertNotNull(CommentPost::find($response['data']['id'])->mentionsUsers->find(5)); } From bec30a1b26778328732b7bc6c1e0cdf3324bedf2 Mon Sep 17 00:00:00 2001 From: huoxin <23447157+huoxin233@users.noreply.github.com> Date: Thu, 23 Jul 2026 17:59:50 +0800 Subject: [PATCH 3/4] test(mentions): fix false positive asserting buggy deleted tag rendering The `deleted_tag_mentions_relation_unparse_and_render_as_expected` test verified that a post mentioning a Tag (ID 3) would render as `[deleted]` if the relationship pivot row was missing, despite Tag 3 actually existing in the database fixtures. Because we fixed the format logic to correctly query the base table when the pivot is empty, Flarum rightfully discovers that Tag 3 still exists and refuses to render it as deleted if it isn't eager-loaded. This commit fixes the test by changing the mention to target Tag ID 999 (which truly does not exist in the database fixtures), ensuring the deleted rendering logic is accurately tested against an actually missing tag. Co-Authored-By: Claude Opus 4.6 --- extensions/mentions/tests/integration/api/TagMentionsTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/mentions/tests/integration/api/TagMentionsTest.php b/extensions/mentions/tests/integration/api/TagMentionsTest.php index 3515e1045e..5134f748f4 100644 --- a/extensions/mentions/tests/integration/api/TagMentionsTest.php +++ b/extensions/mentions/tests/integration/api/TagMentionsTest.php @@ -40,7 +40,7 @@ protected function setUp(): void ], Post::class => [ ['id' => 4, 'number' => 2, 'discussion_id' => 2, 'created_at' => Carbon::now(), 'user_id' => 3, 'type' => 'comment', 'content' => '#test_old_slug'], - ['id' => 7, 'number' => 5, 'discussion_id' => 2, 'created_at' => Carbon::now(), 'user_id' => 2021, 'type' => 'comment', 'content' => '#deleted_relation'], + ['id' => 7, 'number' => 5, 'discussion_id' => 2, 'created_at' => Carbon::now(), 'user_id' => 2021, 'type' => 'comment', 'content' => '#deleted_relation'], ['id' => 8, 'number' => 6, 'discussion_id' => 2, 'created_at' => Carbon::now(), 'user_id' => 4, 'type' => 'comment', 'content' => '#i_am_a_deleted_tag'], ['id' => 10, 'number' => 11, 'discussion_id' => 2, 'created_at' => Carbon::now(), 'user_id' => 4, 'type' => 'comment', 'content' => '#laravel'], ], From 04b0a407c86ae7aadefd82f84c1970802216d14f Mon Sep 17 00:00:00 2001 From: IanM Date: Thu, 23 Jul 2026 18:51:51 +0100 Subject: [PATCH 4/4] test(mentions): cover mention resolution during Saving (#4823) Add a regression test asserting that reading $post->content in a Saving listener resolves mentions to the real entity rather than the [deleted]/ [unknown] fallback, for both new and edited posts. Fails against the pre-fix pivot-query behaviour, passes with the fix. --- .../api/MentionsDuringSavingTest.php | 121 ++++++++++++++++++ 1 file changed, 121 insertions(+) create mode 100644 extensions/mentions/tests/integration/api/MentionsDuringSavingTest.php diff --git a/extensions/mentions/tests/integration/api/MentionsDuringSavingTest.php b/extensions/mentions/tests/integration/api/MentionsDuringSavingTest.php new file mode 100644 index 0000000000..9562c275e4 --- /dev/null +++ b/extensions/mentions/tests/integration/api/MentionsDuringSavingTest.php @@ -0,0 +1,121 @@ +content` is read during the `Saving` lifecycle event (before + * the post's mention pivot tables have been populated), the mention + * formatters must still resolve the mentioned entity by its own existence + * rather than the not-yet-synced pivot — otherwise the mention is wrongly + * unparsed to the `[deleted]` / `[unknown]` fallback text. + */ +class MentionsDuringSavingTest extends TestCase +{ + use RetrievesAuthorizedUsers; + + /** + * Content observed by the Saving listener, captured for assertion. + */ + protected ?string $observedContent = null; + + protected function setUp(): void + { + parent::setUp(); + + $this->extension('flarum-mentions'); + + $this->extend( + (new Extend\Event()) + ->listen(Saving::class, function (Saving $event) { + // Reading content here triggers unparse() while the post's + // mention pivots are not yet synced — the #4823 scenario. + $this->observedContent = $event->post->content; + }) + ); + + $this->prepareDatabase([ + User::class => [ + $this->normalUser(), + ['id' => 3, 'username' => 'mentioned_user', 'email' => 'mentioned_user@machine.local', 'is_email_confirmed' => 1], + ], + Discussion::class => [ + ['id' => 1, 'title' => 'Test discussion', 'created_at' => Carbon::now(), 'last_posted_at' => Carbon::now(), 'user_id' => 1, 'first_post_id' => 1, 'comment_count' => 1], + ], + Post::class => [ + ['id' => 1, 'number' => 1, 'discussion_id' => 1, 'created_at' => Carbon::now(), 'user_id' => 1, 'type' => 'comment', 'content' => '

Original

'], + ], + ]); + } + + #[Test] + public function user_mention_resolves_during_saving_of_a_new_post(): void + { + $response = $this->send( + $this->request('POST', '/api/posts', [ + 'authenticatedAs' => 1, + 'json' => [ + 'data' => [ + 'attributes' => [ + 'content' => '@"mentioned_user"#3', + ], + 'relationships' => [ + 'discussion' => ['data' => ['type' => 'discussions', 'id' => '1']], + ], + ], + ], + ]) + ); + + $this->assertEquals(201, $response->getStatusCode(), $response->getBody()->getContents()); + + // The Saving listener must have seen the real mention, NOT the deleted + // fallback — this is the regression from #4823. + $this->assertNotNull($this->observedContent); + $this->assertStringContainsString('mentioned_user', $this->observedContent); + $this->assertStringNotContainsString('[deleted]', $this->observedContent); + $this->assertStringNotContainsString('[unknown]', $this->observedContent); + } + + #[Test] + public function user_mention_resolves_during_saving_when_editing_a_post(): void + { + $response = $this->send( + $this->request('PATCH', '/api/posts/1', [ + 'authenticatedAs' => 1, + 'json' => [ + 'data' => [ + 'attributes' => [ + 'content' => '@"mentioned_user"#3 edited', + ], + ], + ], + ]) + ); + + $this->assertEquals(200, $response->getStatusCode(), $response->getBody()->getContents()); + + $this->assertNotNull($this->observedContent); + $this->assertStringContainsString('mentioned_user', $this->observedContent); + $this->assertStringNotContainsString('[deleted]', $this->observedContent); + $this->assertStringNotContainsString('[unknown]', $this->observedContent); + } +}