fix(mentions): missed post mentions UI changes

[iPurpl3x]

Patches #3780

Changes proposed in this pull request:

• Fix UI to actually use the new mentionedByCount instead of checking the length of the repliers array.
• Fix the API, so that it serializes the correct value of mentionedByCount instead of 0 (else the UI doesn't behave as expected)

Reviewers should focus on:
The issues are actually fixed, no new performance issue has been introduced.

QA

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)
• Related core extension PRs: (Remove if irrelevant)

[SychO9]

@iPurpl3x can you explain what issue this is trying to fix? the repliers which is post.mentionedBy() is always the limit, so repliers.length is the same as mentionedByCount().

mentionedByCount() is serialized to be able to determine if there are more mentions that the limit loaded.

[iPurpl3x]

@SychO9 so as I understand the code, repliers.length is dependent on replies.length and replies.length comes from the post.mentionedBy() relationship, which usually only contains 4 items as this is the limit for performance. So if a post has 200 replies, repliers.length == 4 & post.mentionedByCount() == 200. Or am I misinterpreting something?...

Now this PR isn't complete yet, @imorland still needs to fix the API, so that post.mentionedByCount() == 200 when the discussion is loaded, and not post.mentionedByCount() == 0 which breaks the UI.

[SychO9]

@iPurpl3x oh my bad, I mistakenly assumed it was modifying the dropdown listing logic. Looking closely now I see it is to do with the the replier names logic which I hadn't updated in my previous PR, which now makes it dead code.

Still not sure how it breaks the Ui apart from not showing the more count though 🤔

[imorland]

Actually, no other changes are required here, other than what @iPurpl3x has already pushed. The correct values are returned correctly after a fix to fof/merge-discussions was made (it was overriding ShowDiscussionController, which in turn was not updated after the recent core changes.

Before this change:

After this change:

[SychO9]

I'm 99% confident the limit still applies, so there is no performance impact. Just a missing count to be able to open the modal and see all mentions, which is good cause it makes this into more trivial bug

@iPurpl3x can you please allow maintainers to push to this PR branch?

[imorland]

I've pushed the changes suggested by @SychO9

To clarify for the record, if an extension modifies ShowDiscussionController to include firstPost (or lastPost), the counts are not loaded for them, causing the API to return 0, which in turn messes up the UI.

[iPurpl3x]

@SychO9 I've added you with admin rights to our repo. I didn't find out how to allow all maintainers at once, but as admin you can now add other people.