[HOLD for payment 2025-01-15] [$250] Chat - Inconsistent behavior on deleting mark as unread message

[IuliiaHerets]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: V9. 0.73-6
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Yes, reproducible on both
Issue reported by: Applause Internal Team

Action Performed:

1. Go to https://staging.new.expensify.com/home
2. Open a chat with no conversation
3. Send 2 messages
4. Mark as unread the first message & then delete it
5. Note mark as unread green line shown in second message sent
6. Now delete the message in chat page
7. Send a new message and mark as unread
8. Send a second message
9. Delete the message sent in step 7
10. Now note unread marker not shown for second message

Expected Result:

There must be consistent behavior on deleting mark as unread message.

Actual Result:

Inconsistent behavior on deleting mark as unread message. After sending 2 messages, mark us unread first message & deleting it shows unread marker on second message. But if we send a message, mark it as unread and then if we send a second message and delete first one, mark us unread was not shown on second message.

Workaround:

Unknown

Platforms:

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6689662_1733809687907.Screenrecorder-2024-12-10-11-01-28-613_compress_1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021866516056985004536
• Upwork Job ID: 1866516056985004536
• Last Price Increase: 2024-12-10
• Automatic offers:
• ahmedGaber93 | Reviewer | 105359601

Issue Owner

Current Issue Owner: @lschurr

[melvin-bot]

Triggered auto assignment to @lschurr (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[bernhardoj]

Proposal

Please re-state the problem that we are trying to solve in this issue.

The next message doesn't show a New message green line when the previous unread message is deleted.

What is the root cause of that problem?

The New message green line will show above the message if the unreadMarkerTime is smaller than the message/action created time.

App/src/pages/home/report/ReportActionsList.tsx

Lines 278 to 285 in 3982ed0

	const isWithinVisibleThreshold = scrollingVerticalOffset.current < MSG_VISIBLE_THRESHOLD ? message.created < (userActiveSince.current ?? '') : true;
	// If the unread marker should be hidden or is not within the visible area, don't show the unread marker.
	if (ReportActionsUtils.shouldHideNewMarker(message) || !isWithinVisibleThreshold) {
	return false;
	}
	const isCurrentMessageUnread = ReportActionsUtils.isReportActionUnread(message, unreadMarkerTime);

unreadMarkerTime is initially set with the report lastReadTime and is updated on these 3 cases listed below.

App/src/pages/home/report/ReportActionsList.tsx

Lines 221 to 229 in 3982ed0

	/**
	* The timestamp for the unread marker.
	*
	* This should ONLY be updated when the user
	* - switches reports
	* - marks a message as read/unread
	* - reads a new message as it is received
	*/
	const [unreadMarkerTime, setUnreadMarkerTime] = useState(lastReadTime);

When we send message A and then mark it as unread, it will update the unreadMarkerTime to be the marked message created time - 1ms. This will show the New message green line above the message A.

App/src/pages/home/report/ReportActionsList.tsx

Lines 328 to 331 in 3982ed0

	const unreadActionSubscription = DeviceEventEmitter.addListener(`unreadAction_${report.reportID}`, (newLastReadTime: string) => {
	setUnreadMarkerTime(newLastReadTime);
	userActiveSince.current = DateUtils.getDBTime();
	});

Notice that it also updates the userActiveSince to the current time. When we send message B, unreadMarkerTime stays unchanged. When we delete message A, unreadMarkerTime stays unchanged too and we expect the New message green line to be shown above message B because the unreadMarkerTime is smaller than the message B created time.

However, the isWithinVisibleThreshold value is false because the message B created time is bigger than the userActiveSince value, preventing the New message green line from showing.

What changes do you think we should make in order to solve the problem?

The isWithinVisibleThreshold was added a long time ago in #31627. The root cause as explained on the issue was because we compared the message/action created time with the report lastReadTime which is updated every time we receive a new message from the other user. But now, the lastReadTime is only used as the initial value as I explained on the root cause above. I believe it's a workaround which we don't need it anymore given the current state of our code, so I suggest removing the isWithinVisibleThreshold condition from shouldDisplayNewMarker.

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

We already have UnreadIndicatorsTest, so we can add this case where we first send a message, mark it as unread, send another message, delete the first message, and assert that the New green line is shown for the second message. I removed the isWithinVisibleThreshold condition and run the test and it still passes.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~021866516056985004536

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @ahmedGaber93 (External)

[ahmedGaber93]

The root cause as explained on the issue was because we compared the message/action created time with the report lastReadTime which is updated every time we receive a new message from the other user.

@bernhardoj Can you please refer to this part?

[bernhardoj]

It's from this sentences

[ahmedGaber93]

@bernhardoj's proposal LGTM! The RCA is correct, and the solution by removing isWithinVisibleThreshold works fine. Also, I have tested #27456 which isWithinVisibleThreshold is added for it, and it works good now after deleted it.

🎀 👀 🎀 C+ reviewed