[HOLD for payment 2024-02-01] [HOLD for payment 2024-01-31] [$250] Chat-No unread marker when chat with the latest text is from you and marked as unread from LHN

[lanitochka17]

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: 1.4.2-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

1. Launch New Expensify app
2. Mark a chat where the latest message is not from you, as unread
3. Open the chat
4. Note that it shows the unread marker
5. Send a few messages to the chat
6. Return to LHN
7. Long press on the chat and mark it as unread
8. Open the chat

Expected Result:

The unread marker will be displayed

Actual Result:

The unread marker is not displayed. This only happens when the latest message comes from the current app user

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

Bug6287687_1700684028156.RPReplay_Final1700671844.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01c2b3a63cdf2318c4
• Upwork Job ID: 1727427415139188736
• Last Price Increase: 2024-01-24
• Automatic offers:
• mollfpr | Reviewer | 28032969
• dukenv0307 | Contributor | 28032971

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @miljakljajic (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[melvin-bot]

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

[akamefi202]

Proposal

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

No unread marker when chat with the latest text is from you and marked as unread from LHN.

What is the root cause of that problem?

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

Lines 341 to 343 in 14abfa1

	if (!messageManuallyMarkedUnread) {
	shouldDisplay = shouldDisplay && reportAction.actorAccountID !== Report.getCurrentUserAccountID();
	}

Currently, shouldDisplayNewMarker function prevents showing unread marker if the sender of last message is the current app user.
As a result, we can't see unread marker if the current user sent the last message.

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

We might delete this code part to show unread marker although the last messgae is sent by the current user.

What alternative solutions did you explore? (Optional)

N/A

[dukenv0307]

Proposal

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

The unread marker is not displayed. This only happens when the latest message comes from the current app user

What is the root cause of that problem?

When we mark a message as unread from LHN, it will mark the last message as unread as in here. In case the logged in user is the one sending the last message, that message will be marked as unread, which is not correct because the it doesn't make sense for the user to "unread" their own message. So based on this condition, the marker will not be visible on user's own message.

In this case we should do the same as Slack does, which is to mark the "latest message that doesn't come from current user" as unread.

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

In here, if reportActionCreated is not available, we should find the latest message that doesn't come from current user to get the actionCreationTime, and use that as the basis for marking the lastReadTime.

We might need BE change as well to follow the same logic.

What alternative solutions did you explore? (Optional)

If we want to fix the same way for marking unread from report action context menu, we need to pass the reportActionId to markCommentAsUnread and find the "latest report action doesn't come from current user" from that reportActionId backwards. This is also the same as how Slack does it.

Or we can disable Mark as unread in report action context menu for report actions that come from current user.

[melvin-bot]

@mollfpr, @miljakljajic Eep! 4 days overdue now. Issues have feelings too...

[miljakljajic]

@mollfpr any thoughts on the initial proposals?

[mollfpr]

Sorry for the delay; I forgot to reply here.

I'm still confused about the expected result. I think we shouldn't allow the user to unmark their message.

@miljakljajic Could you confirm if we want to do that, or maybe we want to change the expected result here? Thanks!

[situchan]

cc: @MonilBhavsar

[MonilBhavsar]

Thanks for the mention. I'll take a look

[MonilBhavsar]

This issue is only reproducible on Mobile devices

I'm still confused about the expected result. I think we shouldn't allow the user to unmark their message.

We need to allow users to unmark their own messages. Many chatting apps including slack has this ability. Reason: in case user wants to catch up from that later

[melvin-bot]

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

[mollfpr]

Thanks @MonilBhavsar, for confirming it.

@akamefi202 @dukenv0307 Why is it working fine on the Web and Desktop?

I found a different result for the messageManuallyMarkedUnread value between Web and mWeb. On the Web, we have the value timestamp, but on the mWeb it's remains 0.

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

Lines 341 to 343 in 089ddf4

	if (!messageManuallyMarkedUnread) {
	shouldDisplay = shouldDisplay && reportAction.actorAccountID !== Report.getCurrentUserAccountID();
	}

I think that might be why it's checking the marker it's not displayed.

[akamefi202]

@mollfpr Because LHN and Report page is separated in mobile app.
So Report component dismounts & remounts when the user navigates to LHN and back to Report page.
And this resets messageManuallyMarkedUnread as 0.

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

Lines 219 to 220 in 0863b6a

	lastReadTimeRef.current = report.lastReadTime;
	setMessageManuallyMarkedUnread(0);

[dukenv0307]

We need to allow users to unmark their own messages. Many chatting apps including slack has this ability. Reason: in case user wants to catch up from that later

@MonilBhavsar Can you also confirm that once unmarking their own message, we should do the same as Slack does, which is to mark the "latest message that doesn't come from current user" as unread?
(Slack will not show the marker above current user's message)

@mollfpr Because LHN and Report page is separated in mobile app.

Yes this is right

[MonilBhavsar]

Can you also confirm that once unmarking their own message, we should do the same as Slack does, which is to mark the "latest message that doesn't come from current user" as unread?
(Slack will not show the marker above current user's message)

Pretty sure this has changed recently with Slack. One was able to mark any message as unread in slack previously. I think we should keep that out of scope for now, and handle in new issue. Not a priority though.

For this issue, we can mark the last message as unread like Web

[mollfpr]

@akamefi202 Do you propose removing the check for the current user accountID?

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

Lines 340 to 343 in 1682be4

	if (shouldDisplay && !messageManuallyMarkedUnread) {
	const isWithinVisibleThreshold = scrollingVerticalOffset.current < MSG_VISIBLE_THRESHOLD ? reportAction.created < userActiveSince.current : true;
	shouldDisplay = reportAction.actorAccountID !== Report.getCurrentUserAccountID() && isWithinVisibleThreshold;
	}

[MonilBhavsar]

Proposals in review...

[youssef-lr]

@dukenv0307 please check the regression linked by @situchan. Thank you!

[dukenv0307]

@youssef-lr @mollfpr The PR to fix regression is ready for review.

[melvin-bot]

Upwork job price has been updated to $250

[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 1.4.31-7 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fallback report action created if reportActions is empty #34551

If no regressions arise, payment will be issued on 2024-02-01. 🎊

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

• @mollfpr requires payment automatic offer (Reviewer)
• @dukenv0307 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:

• [@mollfpr] The PR that introduced the bug has been identified. Link to the PR:
• [@mollfpr] 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:
• [@mollfpr] 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:
• [@mollfpr] Determine if we should create a regression test for this bug.
• [@mollfpr] 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.
• [@miljakljajic] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[Santhosh-Sellavel]

Seems this issue is not fully fixed please this issue #35122

[miljakljajic]

@MonilBhavsar what's your thoughts on this one?

[miljakljajic]

@MonilBhavsar can I get some feedback on @Santhosh-Sellavel 's comment above - is #35122 a regression from this issue?

[MonilBhavsar]

Seems this issue is not fully fixed please this issue #35122
@Santhosh-Sellavel this issue involves marking the rpeort as unread from LHN and other issue does not involve marking as unread flow?
@dukenv0307 @mollfpr any thoughts on this.
Also cc @situchan since you're looking at the other issue.

[dukenv0307]

@MonilBhavsar The issue #35122 isn't related to our PR since we updated markCommentAsUnread function meanwhile the other issue above is focused on the the report in LHN that is returned from pusher isn't bold.

[MonilBhavsar]

Yes exactly, that's what I thought.

[dukenv0307]

I think we're good to close this now.

[MonilBhavsar]

Okay thanks!