MEDIUM: [$500] Unread indicator is showing in the fevicon even though there is nothing in LHN

[m-natarajan]

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:
Reproducible in staging?: needs reproduction
Reproducible in production?: needs reproduction
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: @puneetlath
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1699899029212629

Action Performed:

1. Open app and login
2. Make a IOU request
3. Open the IOU thread to see the details
4. Observe the unread indicator in expensify fevicon

Expected Result:

If there is an unread indicator, LHN should have one unread message

Actual Result:

But nothing in LHN with unread message.

it was caused by this transaction thread, which wasn’t showing up in my LHN, but was considered unread by the logic being used to compute the unread indicator here. We should update the logic of the unread indicator to make sure it matches the logic of the LHN (or even better use shared logic). So that it’s never possible to see the unread indicator when you don’t have anything unread in the LHN.

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01d98ea368e18737bf
• Upwork Job ID: 1724168903941550080
• Last Price Increase: 2024-01-03

Issue Owner

Current Issue Owner: @muttmuure

[melvin-bot]

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

[melvin-bot]

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

[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 - @situchan (External)

[ericuldall]

Proposal

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

When requesting money, after completing your request the new notification generated by a logged in user is marked as and unread notification for that user who generated it. This results in a badge being generated unnecessarily on the app icon.

What is the root cause of that problem?

The root cause seems to be that a user is being notified of their own activity when the onyx event is received
The call to requestMoney triggers a new report notify action which is subsequently handled by the unread indicator updater which I believe uses faulty logic implemented in the ReportUtil.isUnread method

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

Assuming there are no scenarios where a user should be notified of their own activity, it may be the best course of action to simply check that we are not generating notification badges to a user from themself.

What alternate solutions did you explore (updated)

• It may also be good to use lastActorAccountID as the checked value so we make sure the report owner can still get notifications on other actions.

[dukenv0307]

Proposal

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

If there is an unread indicator, LHN should have one unread message, but nothing in LHN with unread message.

What is the root cause of that problem?

We updated unreadReports here to sync with LHN but we missed the case is report has notification is hidden

App/src/libs/UnreadIndicatorUpdater/index.js

Line 14 in a2cf591

const unreadReports = _.filter(allReports, (report) => ReportUtils.isUnread(report) && ReportUtils.shouldReportBeInOptionList(report, currentReportID));

App/src/components/LHNOptionsList/OptionRowLHN.js

Lines 92 to 94 in a2cf591

	if (isHidden && !props.isFocused && !optionItem.isPinned) {
	return null;
	}

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

We should check for this case the same way we do in OptionRowLHN

const unreadReports = _.filter(allReports, (report) => ReportUtils.isUnread(report) && ReportUtils.shouldReportBeInOptionList(report, currentReportID) && (report.notificationPreference !== CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN || report.reportID === currentReportID || report.isPinned));

App/src/libs/UnreadIndicatorUpdater/index.js

Line 14 in a2cf591

const unreadReports = _.filter(allReports, (report) => ReportUtils.isUnread(report) && ReportUtils.shouldReportBeInOptionList(report, currentReportID));

App/src/components/LHNOptionsList/OptionRowLHN.js

Lines 92 to 94 in a2cf591

	if (isHidden && !props.isFocused && !optionItem.isPinned) {
	return null;
	}

What alternative solutions did you explore? (Optional)

There're 2 places that currently determine whether a report will show in LHN or not, here and here, we can try to combine to one in shouldReportBeInOptionList as a refactor for clarify

[paultsimura]

@puneetlath could you please double-check this was not happening simply because of the Concierge chat being unread? This can be easily overseen for the new accounts.

[AmjedNazzal]

I think this should be held until this is completed because it will apply changes to UnreadIndicatorUpdater

[cubuspl42]

I think that this issue is a possible duplicate of #28536, or at least has a high chance of getting solved by the associated PR

[puneetlath]

@cubuspl42 indeed it does sound very similar to that. @muttmuure perhaps we can put it on hold for #28536 that and then we can have @shawnborton and others who are experiencing this re-test after #28536 is live.

[cubuspl42]

@puneetlath We're discussing in #31276 right now, it's probably a mistake

[puneetlath]

Lol whops, I meant #28536

[cubuspl42]

In case this doesn't move forward, please ping also in PR #30133, as Melvin won't remind me about that issue anymore

[MonilBhavsar]

Commented in the PR

[muttmuure]

Still in review here: #34025

[muttmuure]

Still in review

[muttmuure]

Still open

[muttmuure]

Not overdue

[muttmuure]

Not overdue!

[muttmuure]

Held

[MonilBhavsar]

PR merged and deployed to production. Taking this off hold.

[MonilBhavsar]

Have asked if bug is fixed or not in slack thread https://expensify.slack.com/archives/C049HHMV9SM/p1706532316571099?thread_ts=1699899029.212629&cid=C049HHMV9SM

[melvin-bot]

@MonilBhavsar, @muttmuure, @situchan Eep! 4 days overdue now. Issues have feelings too...

[iwiznia]

This has been working for me for a while now https://expensify.slack.com/archives/C03U7DCU4/p1706112016224459

[muttmuure]

It seems like there are a few PRs that will be eligible for payment here, I need to comb through and figure it out

[muttmuure]

OK, so the linked issue was a fix for a regression - #33506 so no payment there.

We did get a proposal for this issue above, but since there was an overlap with the regression fix, there was already something solid and we actually didn't need the second proposal.

So I am going to close this, but if you think that your work was eligible for payment despite this issue not having a linked PR, please send me a DM!