Nodebb: Notifications.merge results in "new posts made to topic" notifications going to end of topic

[BenLubar]

Ideally, the first unread post should be the target of the merged notification.

Ref: apapadimoulis/what-bugs#54

[julianlam]

Ugh, right, that's a silly mistake on my part.

[julianlam]

The related code is here in notifications.js. Because I remove duplicates by comparing .indexOf, it always preserves the first, which is the newest.

The code calling Notifications.merge could pass in the reversed array and reverse the output again, but that's... kind of a silly way of doing it. That's 2am thinking for you :coffee:

[julianlam]

As it turns out, it was a one-line fix :thought_balloon:

[LB--]

This seems to still be slightly out of whack. I've noticed times where the noficiation for someone replying directly to me gets overridden by the notification of someone replying to that person when I am watching the thread. E.g., I post to thread, someone replies to me, then someone replies to them -> I only get notification for the last post and unless I scroll up I'll have no idea someone replied to me.

I've also seen times where it just flat out starts at a later post for no discernable reason.

[LB--]

@BenLubar This is currently still happening on what.thedailywtf, did we ever merge the fix or was there a regression?

[BenLubar]

@LB-- we merged the fix back in 2016: https://github.com/boomzillawtf/tdwtf/commit/f07518feb62355e0178269fbdb5fa34fe39f8042