[2.x] fix(mention): fix post mention notification jumping

[zxbmmmmmmmmm]

Fixes #4524

Changes proposed in this pull request:

This pull request updates the way the post mentioned notification determines which discussion to link to, making it more reliable by explicitly passing the discussion ID from the backend to the frontend.

Backend changes:

• The getData method in PostMentionedBlueprint.php now includes the discussionId in the notification payload, ensuring the frontend always receives the correct discussion reference.

Frontend changes:

• The PostMentionedNotification component now uses the discussionId from the notification content instead of extracting it from the post, improving robustness and consistency.

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)

[imorland]

Thanks for digging into this — the diagnosis is spot on and the backend change is exactly right. Passing discussionId from the reply through the payload is the correct destination, and the scope is appropriately small.

There's a blocker on the frontend side though that I think will break new notifications, so I can't merge this as-is — especially as we're in the RC cycle and need to be careful here.

Blocker: passing a raw ID where a model is expected

const discussion = content?.discussionId ?? post.discussion();
return app.route.discussion(discussion, content?.replyNumber);

app.route.discussion() expects a Discussion model — it calls discussion.slug() internally:

discussion: (discussion: Discussion, near?: number) => {
  return app.route(..., { id: discussion.slug(), ... });
}

But content.discussionId is a plain integer ((int) $this->reply->discussion_id, JSON-encoded). So for every notification created after this ships, discussion will be a number and (3).slug() throws TypeError: discussion.slug is not a function, which breaks rendering of the notification.

This probably looked fine locally because existing notifications in the DB don't have discussionId yet, so the ?? post.discussion() fallback returns a model — but that's also the path that doesn't actually fix the bug. The fixed path (new notifications) is the one that crashes.

Suggested fix

A numeric id resolves fine via the slug driver (/d/3 → canonicalised by the resolver), so we can build the route directly without needing the model in the store:

href() {
  const notification = this.attrs.notification;
  const post = notification.subject();
  const content = notification.content();
  const near = content?.replyNumber;

  if (content?.discussionId) {
    return app.route(near && near !== 1 ? 'discussion.near' : 'discussion', {
      id: content.discussionId,
      near: near && near !== 1 ? near : undefined,
    });
  }

  // Fallback for notifications created before discussionId was added to the payload.
  return app.route.discussion(post.discussion(), near);
}

Resolving the model via app.store.getById('discussions', content.discussionId) is an option too, but the reply's discussion isn't guaranteed to be loaded in the store, so it can come back undefined and we'd be back to a crash. The numeric-id approach avoids that.

Tests

We'd like to see test coverage for this before it goes in. A test that creates a mention from a different discussion and asserts the notification points at the reply's discussion (not the original post's) would lock the behaviour and stop it regressing again. The "Tests have been added" box is currently unchecked — please add one here.

Requesting changes for now. Once the frontend is sorted and there's a test, this is a clean fix and I'm happy to take it. Thanks again!

[zxbmmmmmmmmm]

Thanks for the detailed review!

I've updated the code based on your suggestions and added tests for both backend and frontend.

• Backend: added a test that ensures post mention notification content includes discussion id

• Frontend: added tests to ensure the link is generated with discussion id and fallbacks correctly without it.