[Awaiting checklist] [$250] 'Mark Unread' button unresponsive despite green line in composer

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 1.4.85-6
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause- Internal Team

Action Performed:

1. Open any chat and compose a message
2. Reply to the message
3. Delete the original message (parent message)
4. Navigate to the left-hand navigation (LHN), left-click, and select 'Mark unread'

Expected Result:

The 'Mark unread' button should be responsive and change to 'Mark read' if a green line appears. It should become unresponsive if no message is in the composer and no green line appears

Actual Result:

Despite a message appearing in the LHN and a green line appearing in the composer, the 'Mark unread' button becomes unresponsive

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6518238_1718792230152.Screen_Recording_2024-06-19_at_3.14.19_AM.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~011a7209db54618c37
• Upwork Job ID: 1803567026422660932
• Last Price Increase: 2024-06-26
• Automatic offers:
• bernhardoj | Contributor | 102893020

Issue Owner

Current Issue Owner: @fedirjh

[melvin-bot]

Triggered auto assignment to @twisterdotcom (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.

[lanitochka17]

@twisterdotcom FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[lanitochka17]

We think that this bug might be related to #vip-vsp

[melvin-bot]

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

[melvin-bot]

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

[daledah]

Proposal

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

Despite a message appearing in the LHN and a green line appearing in the composer, the 'Mark unread' button becomes unresponsive

What is the root cause of that problem?

We're considering the empty report is read report

App/src/libs/ReportUtils.ts

Lines 5170 to 5172 in 7561439

	if (isEmptyReport(report)) {
	return false;
	}

If the last message is [Deleted message], lastMessageText, lastMessageTranslationKey are empty -> isEmptyReport is true

App/src/libs/ReportUtils.ts

Line 5162 in 7561439

return !report.lastMessageText && !report.lastMessageTranslationKey && !lastVisibleMessage.lastMessageText && !lastVisibleMessage.lastMessageTranslationKey;

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

We should check the childVisibleActionCount along with lastMessageText, lastMessageTranslationKey

1. In getLastVisibleMessage, return additional childVisibleActionCount: lastVisibleAction?.childVisibleActionCount
2. In isEmptyReport, add childVisibleActionCount to

App/src/libs/ReportUtils.ts

Line 5162 in 7561439

return !report.lastMessageText && !report.lastMessageTranslationKey && !lastVisibleMessage.lastMessageText && !lastVisibleMessage.lastMessageTranslationKey;

return !report.lastMessageText && !report.lastMessageTranslationKey && !lastVisibleMessage.lastMessageText && !lastVisibleMessage.lastMessageTranslationKey && !lastVisibleMessage.childVisibleActionCount;

What alternative solutions did you explore? (Optional)

NA

[bernhardoj]

Proposal

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

The mark as unread doesn't make the LHN bold even though there is a green unread line.

What is the root cause of that problem?

The LHN will be bold if the isUnread returns true.

App/src/libs/ReportUtils.ts

Lines 5165 to 5172 in 7561439

	function isUnread(report: OnyxEntry<Report>): boolean {
	if (!report) {
	return false;
	}
	if (isEmptyReport(report)) {
	return false;
	}

However, in our case, isEmptyReport returns true, so isUnread becomes false.

App/src/libs/ReportUtils.ts

Lines 5157 to 5162 in 7561439

	function isEmptyReport(report: OnyxEntry<Report>): boolean {
	if (!report) {
	return true;
	}
	const lastVisibleMessage = ReportActionsUtils.getLastVisibleMessage(report.reportID);
	return !report.lastMessageText && !report.lastMessageTranslationKey && !lastVisibleMessage.lastMessageText && !lastVisibleMessage.lastMessageTranslationKey;

The lastMessageText of a report is empty for our case, but lastVisibleMessage is also empty.

ReportActionsUtils.getLastVisibleMessage gets the last visible action and returns its message. In our case, the last visible action is the [Deleted message], but the message is empty because it's deleted.

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

Replace ReportActionsUtils.getLastVisibleMessage with ReportUtils.getLastVisibleMessage.

In ReportUtils.getLastVisibleMessage, we return [Deleted message] if it's a deleted parent action which is what we need for our case.

App/src/libs/ReportUtils.ts

Lines 2242 to 2255 in 7561439

	function getLastVisibleMessage(reportID: string | undefined, actionsToMerge: ReportActions = {}): LastVisibleMessage {
	const report = getReport(reportID);
	const lastVisibleAction = ReportActionsUtils.getLastVisibleAction(reportID ?? '-1', actionsToMerge);
	// For Chat Report with deleted parent actions, let us fetch the correct message
	if (ReportActionsUtils.isDeletedParentAction(lastVisibleAction) && !isEmptyObject(report) && isChatReport(report)) {
	const lastMessageText = getDeletedParentActionMessageForChatReport(lastVisibleAction);
	return {
	lastMessageText,
	};
	}
	// Fetch the last visible message for report represented by reportID and based on actions to merge.
	return ReportActionsUtils.getLastVisibleMessage(reportID ?? '-1', actionsToMerge);

[daledah]

Hey @bernhardoj, I see your solution is quite similar to mine. I'm new in EPSF and I spent a lot of time to investigate this issue. I checked the implementation of ReportUtils.getLastVisibleMessage, it uses ReportActionsUtils.getLastVisibleMessage along with childVisibleActionCount (same as what I propose). So I think using ReportUtils.getLastVisibleMessage is just a small improvement (I didn't notice this function). What is the chosen proposal in this case @fedirjh @twisterdotcom?

[fedirjh]

Thank you for the proposal, @daledah and @bernhardoj. Both proposals are good. I prefer Bernard's solution for the simplicity.

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[twisterdotcom]

Shall we get an internal engineer on this @fedirjh?

[fedirjh]

Oh my bad missed the tag: 🎀 👀 🎀 C+ reviewed

Let's proceed with @bernhardoj's solution

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @chiragsalian, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[chiragsalian]

Proposal LGTM. Feel free to create the PR @bernhardoj.

[melvin-bot]

📣 @bernhardoj 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[bernhardoj]

PR is ready

cc: @fedirjh

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.5-13 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• LHN item stay as read after mark as unread #44519

If no regressions arise, payment will be issued on 2024-07-17. 🎊

For reference, here are some details about the assignees on this issue:

• @fedirjh requires payment through NewDot Manual Requests
• @bernhardoj requires payment automatic offer (Contributor)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@fedirjh] The PR that introduced the bug has been identified. Link to the PR:
• [@fedirjh] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@fedirjh] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@fedirjh] Determine if we should create a regression test for this bug.
• [@fedirjh] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@twisterdotcom] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[bernhardoj]

I'll request in ND once payment is due.

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.6-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• LHN item stay as read after mark as unread #44519

If no regressions arise, payment will be issued on 2024-07-22. 🎊

For reference, here are some details about the assignees on this issue:

• @fedirjh requires payment through NewDot Manual Requests
• @bernhardoj requires payment through NewDot Manual Requests (Contributor)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@fedirjh] The PR that introduced the bug has been identified. Link to the PR:
• [@fedirjh] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@fedirjh] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@fedirjh] Determine if we should create a regression test for this bug.
• [@fedirjh] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@twisterdotcom] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[bernhardoj]

The payment should be due tomorrow (07-17)

[bernhardoj]

Requested in ND.

[twisterdotcom]

Payment Summary:

• @fedirjh paid $250 via NewDot Manual Request
• @bernhardoj paid $250 via NewDot Manual Request (Contributor)

Just waiting on the checklist now @fedirjh.

[JmillsExpensify]

$250 approved for @bernhardoj

[fedirjh]

BugZero Checklist:

• Link to the PR: fix empty chat displayed in focus mode #40336
• Link to comment: fix empty chat displayed in focus mode #40336 (comment)
• Link to discussion: N/A
• Determine if we should create a regression test for this bug: N/A, this is an edge case bug, no need for regression test.

[JmillsExpensify]

$250 approved for @fedirjh