App shows an unread count of 1 while no unreads appear in LHN

[shawnborton]

@fedirjh Here is the relevant Slack conversation.

It seems like the logic in this PR filters what is shown in the LHN as unread/read. However, I have noticed that a room that I have muted might have a new message, causing my unread count to go to 1, but this message does not show up in the LHN.

However, I see the unread message in Expensify.com:

[shawnborton]

cc @joelbettner @thesahindia @luacmartins @Beamanator as you were reviewers of the potentially offending PR.

and cc @fedirjh for conversation in Slack.

[melvin-bot]

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

[hungvu193]

I don't think the linked PR is the RCA of this issue, it actually came from this PR:
#22717

App/src/components/LHNOptionsList/OptionRowLHN.js

Lines 71 to 74 in 00efce4

	const isMuted = optionItem.notificationPreference === CONST.REPORT.NOTIFICATION_PREFERENCE.MUTE;
	if (isMuted && !props.isFocused && !optionItem.isPinned) {
	return null;
	}

So if a report is muted, not pinned or focused it will disappear from LHN because it rendered null.

We also didn't send notification if current notification preference of the room is daily or mute (but still received the pusher event), that's why

App/src/libs/actions/Report.js

Lines 1547 to 1552 in 00efce4

	// We don't want to send a local notification if the user preference is daily or mute
	const notificationPreference = lodashGet(allReports, [reportID, 'notificationPreference'], CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS);
	if (notificationPreference === CONST.REPORT.NOTIFICATION_PREFERENCE.MUTE || notificationPreference === CONST.REPORT.NOTIFICATION_PREFERENCE.DAILY) {
	Log.info(`${tag} No notification because user preference is to be notified: ${notificationPreference}`);
	return false;
	}

That's a little bit weird, I think we should still show the mute room but give it the lowest priority and not bold the text when we have new messages also update the logic here for not counting the mute report.

App/src/libs/UnreadIndicatorUpdater/index.js

Lines 10 to 13 in 00efce4

	callback: (reportsFromOnyx) => {
	const unreadReports = _.filter(reportsFromOnyx, ReportUtils.isUnread);
	updateUnread(_.size(unreadReports));
	},

[fedirjh]

it actually came from this PR:
#22717

That PR seems to be reverted, then this PR #23521 reintroduced the same logic

---

There is another open PR #25618 that may fix this bug.

[hungvu193]

There is another open PR #25618 that may fix this bug.

Yeah, but it seems still in progress, I didn't see the logic to handle unread for hidden report, or may be there's a backend fix for that 🤔

[shawnborton]

@chiragsalian can you take a look here? Looks like your PR might be the offending PR that caused this.

[chiragsalian]

Yeah i think this will be solved once this is live. Perhaps we can revisit this issue once my PR is live. With my changes even if you muted a chat it will show up in your LHN so this way you should be able to see those unread muted chats.

[luacmartins]

Thanks for confirming @chiragsalian! Added a hold to this issue.

[luacmartins]

on hold

[luacmartins]

still on hold

[laurenreidexpensify]

#25618 is deployed to prod as of Thursday, are we good to take off hold?

[luacmartins]

Yes, we can remove the hold. Is this issue still reproducible?

[laurenreidexpensify]

@shawnborton @chiragsalian can you confirm if this is still reproducible, or list the repro steps so I can test. thanks!

[shawnborton]

Hmm well now what's happening is that I have the #announce room for the Lounge Workspace muted, but I still get unreads shown in my LHN for that room. Is that intended?

[hungvu193]

Hmm well now what's happening is that I have the #announce room for the Lounge Workspace muted, but I still get unreads shown in my LHN for that room. Is that intended?

According to the PR, I think yes. But I think it's little weird, how about my suggestion here?

That's a little bit weird, I think we should still show the mute room but give it the lowest priority and not bold the text when we have new messages also update the logic here for not counting the mute report.

App/src/libs/UnreadIndicatorUpdater/index.js

Lines 10 to 13 in 00efce4

	callback: (reportsFromOnyx) => {
	const unreadReports = _.filter(reportsFromOnyx, ReportUtils.isUnread);
	updateUnread(_.size(unreadReports));
	},

[shawnborton]

Curious for @chiragsalian's thoughts on that one.

[melvin-bot]

@luacmartins, @fedirjh, @laurenreidexpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

@luacmartins, @fedirjh, @laurenreidexpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[luacmartins]

@chiragsalian can you check this comment please?

[luacmartins]

Hmm well now what's happening is that I have the #announce room for the Lounge Workspace muted, but I still get unreads shown in my LHN for that room. Is that intended?

@shawnborton so you don't get notifications for it but the LHN still shows something as unread?

[luacmartins]

Maybe we need to update the logic in isUnread to return false if the report is muted?

[luacmartins]

Just chatted with Jason and apparently this is intended. According to E.cash: Default Rooms & Room Notifications design doc, muted chats should count towards the unread count:

Chat room is bolded as “unread” in LHN, and the chat room moves in the LHN in-line with the existing lastMessage sorting logic in place for DMs

[luacmartins]

So I think there's nothing to do here since this is the expected behavior. I'm gonna close the issue. Please reopen if I missed something!