regression: unread banner appears for thread messages after channel creation #36331

abhinavkrin · 2025-06-30T16:48:54Z

Proposed changes (including videos or screenshots)

When you create a brand-new channel and immediately reply to your own message in a thread, the unread-messages banner was showing up even though you hadn’t missed anything. This happened because the baseline timestamp (lastMessageDate) stayed undefined until an off-screen message was detected, so every reply looked like “new” content. Now, on first load—when the message count goes from zero to some number—we set lastMessageDate to the newest message’s timestamp, giving the unread-count logic a proper anchor and preventing self-sent thread replies from ever triggering the banner.

Issue(s)

Steps to test or reproduce

Further comments

CORE-1214

…reation Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>

dionisio-bot · 2025-06-30T16:48:58Z

Looks like this PR is ready to merge! 🎉
If you have any trouble, please check the PR guidelines

changeset-bot · 2025-06-30T16:48:59Z

⚠️ No Changeset found

Latest commit: 18d7420

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

abhinavkrin · 2025-06-30T16:49:55Z

apps/meteor/client/views/room/body/hooks/useUnreadMessages.ts

 			return;
 		}
 		setMessageJumpQueryStringParameter(message?._id);
+		chat.readStateManager.markAsRead();


This seemed to be a fix for me. While clicking on the banner, It would take me to message. But when page is refreshed, the banner would reapper.

github-actions · 2025-06-30T17:03:42Z

PR Preview Action v1.6.1
🚀 View preview at https://RocketChat.github.io/Rocket.Chat/pr-preview/pr-36331/
Built to branch `gh-pages` at 2025-07-01 19:49 UTC. Preview will be ready when the GitHub Pages deployment is complete.

apps/meteor/tests/e2e/threads.spec.ts

Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>

codecov · 2025-06-30T17:55:34Z

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 64.54%. Comparing base (a14410b) to head (8340250).

Additional details and impacted files

@@                Coverage Diff                @@
##           release-7.8.0   #36331      +/-   ##
=================================================
- Coverage          64.55%   64.54%   -0.01%     
=================================================
  Files               3147     3147              
  Lines             104629   104629              
  Branches           19766    19766              
=================================================
- Hits               67544    67538       -6     
- Misses             34401    34406       +5     
- Partials            2684     2685       +1

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

MartinSchoeler

Do we know the root cause of the issue (in what PR the bug was introduced)? Here it seems we are only patching the symptom, not actually fixing the root cause of the issue.
I think we can dive a little deeper and come up with a more elegant solution.

MartinSchoeler · 2025-07-01T14:37:50Z

apps/meteor/client/views/room/body/hooks/useUnreadMessages.ts

 		[getMessage, setUnreadCount],
 	);

+	useEffect(() => {


Could we find an alternative to this use effect? As it is, this is going to run every time the messagesCount prop changes (basically on every message). It seems costly for an operation that only needs to happen when the room is initialized.

Actually this will run only once. In first run, the lastMessageDate will be set and there is a check to return early when lastMessageDate is set.

Also, this is not expensive tasks. It is used only at one or two places all over the client. Plus it only runs once and loops around few messages. Next runs, it returns early. I dont think it is expensive.

MartinSchoeler · 2025-07-01T14:39:17Z

apps/meteor/client/views/room/body/hooks/useUnreadMessages.ts

+				(a, b) => b.ts.getTime() - a.ts.getTime(),
+			);
+			if (newest) {
+				setLastMessageDate(newest.ts);


We should avoid changing the value of a state that is in the dependency list

abhinavkrin · 2025-07-01T18:27:59Z

Do we know the root cause of the issue (in what PR the bug was introduced)? Here it seems we are only patching the symptom, not actually fixing the root cause of the issue. I think we can dive a little deeper and come up with a more elegant solution.

The root cause is as explained, we are using the last message which is hidden behind the scroll or near the scroll to calculate the lastMessageDate. But when a room is new, there arent a lot of messages so that a message is behind the scroll or near to it. This made lastMessageDate undefined.
Earlier the count code was:

const count = Messages.find({
			rid: room._id,
			ts: { $lte: lastMessageDate, $gt: subscription?.ls },
		}).count();

This will always return nothing sinc lastMessageDate is undefined.
But now the code is

const count = filterMessages(
			(record) =>
				record.rid === room._id &&
				record.ts.getTime() <= (lastMessageDate?.getTime() ?? Infinity) &&
				record.ts.getTime() > (subscription?.ls?.getTime() ?? -Infinity),
		).length;

This will always return a number > 0 if lastMessageDate is undefined.
So this code change made the already existing bug to appear now.

MartinSchoeler · 2025-07-01T19:12:39Z

const count = filterMessages(
			(record) =>
				record.rid === room._id &&
				record.ts.getTime() <= (lastMessageDate?.getTime() ?? Infinity) &&
				record.ts.getTime() > (subscription?.ls?.getTime() ?? -Infinity),
		).length;

I think we could fix the issue here, add an exception before we filter the messages since when the lastMessage is undefined, the filter does nothing (since infinity will always be larger).

just by doing

const count = filterMessages(
			(record) =>
				record.rid === room._id && !!lastMessageDate &&
				record.ts.getTime() <= lastMessageDate?.getTime() &&
				record.ts.getTime() > (subscription?.ls?.getTime() ?? -Infinity),
		).length;

the issue seems to go away (just an example, need testing), so I think we don't need another useEffect, just to adjust the existing one

Also it seems that this issue was introduced in #36158

abhinavkrin · 2025-07-01T19:17:52Z

const count = filterMessages(
			(record) =>
				record.rid === room._id &&
				record.ts.getTime() <= (lastMessageDate?.getTime() ?? Infinity) &&
				record.ts.getTime() > (subscription?.ls?.getTime() ?? -Infinity),
		).length;
I think we could fix the issue here, add an exception before we filter the messages since when the lastMessage is undefined, the filter does nothing (since infinity will always be larger).

just by doing
const count = filterMessages(
			(record) =>
				record.rid === room._id && !!lastMessageDate &&
				record.ts.getTime() <= lastMessageDate?.getTime() &&
				record.ts.getTime() > (subscription?.ls?.getTime() ?? -Infinity),
		).length;
the issue seems to go away (just an example, need testing), so I think we don't need another useEffect, just to adjust the existing one

Also it seems that this issue was introduced in #36158

Yes it will go back to the previous behavior. But still the issue with how we calculate lastMessageDate will remain same. It will be undefined for new rooms. Since there wont be enough messages which will cause a message to go invisible. And hence unread message banner might not show for new rooms.

Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>

MartinSchoeler · 2025-07-01T22:24:19Z

Behavior matches version 7.7.0

regression: unread banner appears for thread messages after channel c…

79e66ff

…reation Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>

abhinavkrin requested a review from a team as a code owner June 30, 2025 16:48

abhinavkrin commented Jun 30, 2025

View reviewed changes

jessicaschelly requested changes Jun 30, 2025

View reviewed changes

apps/meteor/tests/e2e/threads.spec.ts Outdated Show resolved Hide resolved

apps/meteor/tests/e2e/threads.spec.ts Show resolved Hide resolved

requested changes

4fbf993

Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>

abhinavkrin requested a review from jessicaschelly June 30, 2025 17:51

jessicaschelly previously approved these changes Jun 30, 2025

View reviewed changes

MartinSchoeler requested changes Jul 1, 2025

View reviewed changes

Al2Tul2 added this to the 7.8.0 milestone Jul 1, 2025

abhinavkrin requested a review from MartinSchoeler July 1, 2025 18:28

requested changes

8340250

Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>

abhinavkrin dismissed jessicaschelly’s stale review via 8340250 July 1, 2025 19:35

MartinSchoeler approved these changes Jul 1, 2025

View reviewed changes

MartinSchoeler added the stat: QA assured Means it has been tested and approved by a company insider label Jul 1, 2025

dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Jul 1, 2025

Merge branch 'release-7.8.0' into regression/unread-banner-thread

18d7420

ggazzo merged commit a20ca2c into release-7.8.0 Jul 2, 2025
9 of 10 checks passed

ggazzo deleted the regression/unread-banner-thread branch July 2, 2025 04:08

regression: unread banner appears for thread messages after channel creation #36331

regression: unread banner appears for thread messages after channel creation #36331

Uh oh!

Conversation

abhinavkrin commented Jun 30, 2025 • edited by atlassian bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes (including videos or screenshots)

Issue(s)

Steps to test or reproduce

Further comments

Uh oh!

dionisio-bot bot commented Jun 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

changeset-bot bot commented Jun 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

abhinavkrin Jun 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jun 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Built to branch gh-pages at 2025-07-01 19:49 UTC. Preview will be ready when the GitHub Pages deployment is complete.

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Jun 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

MartinSchoeler left a comment

Choose a reason for hiding this comment

Uh oh!

MartinSchoeler Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

abhinavkrin Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

abhinavkrin Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

MartinSchoeler Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

abhinavkrin Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

abhinavkrin commented Jul 1, 2025

Uh oh!

MartinSchoeler commented Jul 1, 2025

Uh oh!

abhinavkrin commented Jul 1, 2025

Uh oh!

MartinSchoeler commented Jul 1, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

abhinavkrin commented Jun 30, 2025 •

edited by atlassian bot

Loading

dionisio-bot bot commented Jun 30, 2025 •

edited

Loading

changeset-bot bot commented Jun 30, 2025 •

edited

Loading

abhinavkrin Jun 30, 2025 •

edited

Loading

github-actions bot commented Jun 30, 2025 •

edited

Loading

Built to branch `gh-pages` at 2025-07-01 19:49 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

codecov bot commented Jun 30, 2025 •

edited

Loading