[$500] LHN - Report is not marked as unread in LHN when the IOU is received

[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.501
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

Issue found when executing PR #35928

Action Performed:

Preconditions:
Set up an OldDot admin account, invite the employee and approver to the policy https://sites.google.com/applausemail.com/applause-expensifyproject/wiki-guides/newdot-categories?authuser=0

1. Open https://staging.new.expensify.com/
2. Log in with the account of the employee added to the policy
3. Open https://staging.new.expensify.com/
   in incognito mode
4. Log in to the policy approver account
5. Under the approver account, go to the WS room.
6. Under the employee account, create a manual request and send it to the WS room
7. Click on the "Submit" button in the created IOU

Expected Result:

When an IOU is received, the Report in the LHN should be marked as unread

Actual Result:

Report is not marked as unread in LHN when the IOU is received

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

Bug6410215_1710188687689.Recording__1416.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~013810a6929404cade
• Upwork Job ID: 1769899997275127808
• Last Price Increase: 2024-07-18

[melvin-bot]

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

[lanitochka17]

@MitchExpensify 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

[MitchExpensify]

@dylanexpensify you've been super close to message unread/read behavior - Should a new IOU show as unread? I think "yes", right?

[melvin-bot]

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

[melvin-bot]

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

[MitchExpensify]

Lets get this fixed

[MitchExpensify]

Waiting for proposals

[MitchExpensify]

Advertising for proposals here https://expensify.slack.com/archives/C01GTK53T8Q/p1711408083227239

[melvin-bot]

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

[kmbcook]

I think this is a regression from #36950, because of adding the !!report.lastActorAccountID condition here:

App/src/libs/SidebarUtils.ts

Lines 270 to 272 in e542ba7

	// When the only message of a report is deleted lastVisibileActionCreated is not reset leading to wrongly
	// setting it Unread so we add additional condition here to avoid empty chat LHN from being bold.
	result.isUnread = ReportUtils.isUnread(report) && !!report.lastActorAccountID;

If that is correct, it might be better to revert the change. ReportUtils.isUnread should return the correct answer to the question "Is the report unread". According to the line above, it appears that ReportUtils.isUnread doesn't always return the correct answer. If ReportUtils.isUnread does not always return the correct answer then it should be changed. However, in this issue we see an example where ReportUtils.isUnread is returning the correct answer and adding the condition is incorrect.

If lastActorAccountID is needed to determine if a report is unread, then let's put that check inside ReportUtils.isUnread.

[DylanDylann]

I can take over this issue as a C+ reviewer because @situchan is OOO in April

[MitchExpensify]

Holding on #38778

[MitchExpensify]

Reassigning while I'm on leave 🙇

[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.

[twisterdotcom]

@MitchExpensify #38778 is closed. Is there anything else this should be held on?

[MitchExpensify]

Ah closed yesterday, nice. No that's all that was holding this up

[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]

Okay, I see this is still a thing:

38186.mp4

[twisterdotcom]

@kmbcook or @bernhardoj - do you fancy a go at a proposal for this now? @trjExpensify I'm correct in thinking that a new green dot request should be an unread comment right?

[bernhardoj]

Proposal

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

The chat item in LHN doesn't show as bold for the workspace admin/approver when there is an expense preview

What is the root cause of that problem?

#36950 changes the unread logic a bit by checking whether the report has a lastActorAccountID or not.

App/src/libs/SidebarUtils.ts

Lines 339 to 341 in c2ded1d

	// When the only message of a report is deleted lastVisibileActionCreated is not reset leading to wrongly
	// setting it Unread so we add additional condition here to avoid empty chat LHN from being bold.
	result.isUnread = ReportUtils.isUnread(report) && !!report.lastActorAccountID;

But in case the last action is a report preview, the last actor account ID is 0.

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

First of all, we should revert #36950 as it doesn't fix the root cause of the issue. The issue that the PR is trying to fix is that when we delete the last message in a room from device A, the lastVisibleActionCreated property of the report isn't updated in device B, so the unread status stays true.

App/src/libs/ReportUtils.ts

Line 4182 in 4e6c52c

return lastReadTime < lastVisibleActionCreated || lastReadTime < lastMentionedTime;

It will be updated once the device B opens the room. So, the problem is in the pusher that doesn't send the update to device B. Instead of patching up the issue, we should fix the pusher.

fyi, when we delete the last message, we set the lastVisibleActionCreated to empty optimistically, that's why the device A doesn't experience the issue. I think it should be set to the CREATED action created value, but I guess that's another topic to discuss

[bernhardoj]

The issue is actually the same, but because the other issue is closed, I reposted my proposal here

[situchan]

@twisterdotcom I can take this back as C+ while @jjcoffee is OOO

[trjExpensify]

@trjExpensify I'm correct in thinking that a new green dot request should be an unread comment right?

Go from 1.30 in the video. You were in the chat when the IOU came through. So I wouldn't expect it to be unread in that case, would you?

[twisterdotcom]

Ahhh good point. Okay, tested again and this is fixed: