Skip to content

[2.x] fix(mentions): prevent deleted fallback for unsynced posts during formatting#4824

Merged
imorland merged 4 commits into
flarum:2.xfrom
huoxin233:huoxin/2.x-fix-mentions-unparse-saving
Jul 23, 2026
Merged

[2.x] fix(mentions): prevent deleted fallback for unsynced posts during formatting#4824
imorland merged 4 commits into
flarum:2.xfrom
huoxin233:huoxin/2.x-fix-mentions-unparse-saving

Conversation

@huoxin233

@huoxin233 huoxin233 commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Fixes #4823

Changes proposed in this pull request:

This PR modifies all Format*Mentions and Unparse*Mentions classes to:

  • Use the relation's loaded collection if it has been eager loaded (via relationLoaded), executing 0 queries.
  • Fallback directly to the base model query (e.g., Post::query()->find()) rather than querying the pivot table if the relation isn't loaded.

This completely bypasses the pivot table requirement during text evaluation, allowing the formatters to accurately retrieve display names for edited posts.

This bug also caused false positives in the Flarum test suite:

  • editing_a_post_with_deleted_author_that_has_a_mention_works: The test edited a post by sending the exact same text that the post already had. Because the broken unparse() method previously evaluated the old post content as [deleted], Flarum thought the content had changed and dispatched the Revised event. With unparse() fixed, Flarum correctly recognizes the text did not change and skips the Revised event, causing the test's pivot table assertion to fail. This PR updates the test payloads to actually mutate the content so that the Revised event fires properly.
  • deleted_tag_mentions_relation_unparse_and_render_as_expected: The test verified that a post mentioning a Tag would render as [deleted] if the pivot row was missing, despite the Tag actually existing in the database fixtures. Before our format fixes, Flarum wrongly rendered this as deleted because it strictly trusted the empty pivot table. With our format logic fixed, Flarum rightfully discovers the Tag still exists and refuses to render it as deleted, causing the test to fail. This PR updates the test to target a genuinely non-existent Tag ID (999) to accurately test the deleted fallback.

Output of $post->content

Content use to test:

@"huoxin"#p89 abc
@"huoxin"#1 abc

Output:

Before:
[2026-07-22T18:44:19.913160+00:00] flarum.INFO: [Test] Content observed during Saving event: {"content":"@\"[unknown]\"#p89 abc
@\"[deleted]\"#1 abc"} 

After:
[2026-07-22T18:42:36.145018+00:00] flarum.INFO: [Test] Content observed during Saving event: {"content":"@\"huoxin\"#p89 abc
@\"huoxin\"#1 abc"} 

Also tested with email notification which relates to #3454, but was not able to reproduce the issue, the username is showing correctly before and after the fix:

Notification

Hey huoxin,

huoxin233 mentioned you in a post in cccddaaas.

https://xxxxx

---

@"huoxin"#p89 abc
@"huoxin"#1 abc

- The test team -
....

Necessity

  • Has the problem that is being solved here been clearly explained?
  • If applicable, have various options for solving this problem been considered?
  • For core PRs, does this need to be in core, or could it be in an extension?
  • Are we willing to maintain this for years / potentially forever?

Confirmed

  • Frontend changes: tested on a local Flarum installation.
  • Backend changes: tests are green (run composer test).
  • Core developer confirmed locally this works as intended.
  • Tests have been added, or are not appropriate here.

Required changes:

  • Related documentation PR: (Remove if irrelevant)

huoxin233 and others added 2 commits July 23, 2026 03:33
…matting

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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
@huoxin233
huoxin233 requested a review from a team as a code owner July 22, 2026 20:57
@huoxin233 huoxin233 changed the title fix(mentions): prevent deleted fallback for unsaved posts during unparse [2.x] fix(mentions): prevent deleted fallback for unsaved posts during unparse Jul 22, 2026
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 <noreply@anthropic.com>
@huoxin233 huoxin233 changed the title [2.x] fix(mentions): prevent deleted fallback for unsaved posts during unparse [2.x] fix(mentions): prevent deleted fallback for unsynced posts during formatting Jul 23, 2026
@imorland imorland added this to the 2.0.0-rc.6 milestone Jul 23, 2026
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.
@imorland
imorland merged commit 4013db4 into flarum:2.x Jul 23, 2026
19 checks passed
@huoxin233
huoxin233 deleted the huoxin/2.x-fix-mentions-unparse-saving branch July 24, 2026 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[1.x & 2.x] Mentions unparsing resolves to [deleted] or [unknown] during Saving event for unsaved posts

2 participants