[HOLD for payment 2021-10-07] Notification badge showed 100 unread messages when user only had 1 unread chat

[isagoico]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

There are no solid reproduction steps yet will investigate further and update.

1. Open the New Expensify app in Desktop
2. From another device and account sent a message to the account logged in desktop
3. Check the notification badge.

Expected Result:

Notification badge should display the correct amount of unread messages.

Actual Result:

Notification badge is showing incorrect amount of unread messages.

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number: 1.0.80-2

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

Expensify/Expensify Issue URL:

View all open jobs on Upwork

---

From @alex-mechler https://expensify.slack.com/archives/C01GTK53T8Q/p1627488282388000

I have 1 unread message, but e.cash is telling me I have 100

[MelvinBot]

Triggered auto assignment to @madmax330 (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[alex-mechler]

Of note, when I read the 1 unread message I actually did have, it cleared all 100 unnreads.

[MelvinBot]

Triggered auto assignment to @Christinadobrzyn (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[isagoico]

@rafecolton Was able to reproduce this. Here's the new intel on the issue:

I seem to be stuck on 36 notifications on both desktop and mobile.
Ok, so this is even weirder. I think this was due to chronos - when I scrolled down, I saw it marked unread, so I clicked in and the badge went away. However, I never got any notifications for it, and there aren't even 36 messages in the chat.

[parasharrajat]

I faced one more instance of it. On Web. I have 126 notifications on the title but I never got that many messages. There is something really bad going on but I may know what. I'll try to debug this by the far end of the next week.

[Christinadobrzyn]

Posted in Upwork:

Internal job: https://www.upwork.com/ab/applicants/1422014297908846592/job-details
External job: https://www.upwork.com/jobs/~0100d479f219f067dd

Invited @parasharrajat to it!

[MelvinBot]

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

[twisterdotcom]

I had this too. I navigated to my Chronos DM and it cleared but I've no idea why. Also @parasharrajat won't even have that DM, so it can't be specific to it.

[parasharrajat]

If you have any single unread message and you go to that chat. All unread message count is reset to the updated one. So navigating to any unread chat should clear the count.

[isagoico]

@laurenreidexpensify Was able to reproduce this issue. Thread here https://expensify.slack.com/archives/C01GTK53T8Q/p1627890930045000

She had 1390 "unread messages". When opening chat with Chronos the number went down, messages from thread:

ah interesting - nah chronos just takes it down to 1378
BUT it does make sense that the old chronos to new chronos migration might be the culprit here
and that my old chronos messages are the other 1378

[Christinadobrzyn]

It looks like we're still waiting to hire a contributor for this. @parasharrajat would you like to 'officially' take this job or would you prefer I leave it open for other contributors to take a peek? Not sure how full your plate is right now but I know you've been working on lots!

[Christinadobrzyn]

Hey, @timszot, this job has been open for over a week without a taker.

Should I raise the price to $500, or should we put this on pause and see if a fix for this issue resolves it.

[rafecolton]

I think we should still pursue this, as I'm pretty sure it's related to Chronos. I'm seeing pretty consistently when I have one message from Chronos the badge shows two unread messages, and when I stay in the Chronos chat for a few seconds, the badge goes away. cc @deetergp in case the relation to Chronos means this needs to be solved internally.

[mananjadhav]

@Christinadobrzyn I don't think fixing #4425 will resolve this. The negative count comes in one specific scenario.

[kidroca]

@kadiealexander
I don't want to bother you, but maybe you can consider prioritizing this ticket as well as it's the root of few or even several issues related to unread message count or the app incorrectly interpreting that there are such. (See #4273 (comment) and #4273 (comment))

Published a PR about it with 6 line changes and should be easy to review and see from the change alone that it's unlikely to cause a regression
Whatever is decided the PR is here: #5302
Tomorrow I'll update it with videos for the rest of the platforms

[kadiealexander]

Checking with the team on this one @kidroca, will let you know!

[kadiealexander]

@kidroca the team agrees! Removing the hold on this one, please proceed with the fix.

[kidroca]

I've tested on all platforms and updated the PR with videos for all of them.
The PR is ready for review.

[isagoico]

@iwiznia was able to reproduce the negative notification just now

Unread counter is showing -1. This happened when testing the free plan flow following the instructions in this PR #5443

[kidroca]

@isagoico
I don't think the negative notification is caused by the problems I've found here
Most people reported an unusually high and incorrect number of unread messages and that's what I've investigated

The -1 seems to be a special case

App/src/libs/UnreadIndicatorUpdater/index.js

Lines 15 to 18 in b4c876a

	// If all of our nonzero unread action counts show -1, update the unread count to be -1 as well so we don't show a
	// number in the indicator badge.
	if (_.every(unreadActionCounts, count => count < 1) && _.some(unreadActionCounts, count => count === -1)) {
	updateUnread(-1);

It's intended to be an underlying value and not shown to the user, but for some reason it is displayed
As such it doesn't look like a problem for the current ticket

[kidroca]

The problem seems to be right here:

App/src/libs/UnreadIndicatorUpdater/updateUnread/index.website.js

Lines 11 to 15 in b4c876a

	function updateUnread(totalCount) {
	const hasUnread = totalCount !== 0;
	document.title = hasUnread ? `(${totalCount}) ${CONFIG.SITE_TITLE}` : CONFIG.SITE_TITLE;
	document.getElementById('favicon').href = hasUnread ? CONFIG.FAVICON.UNREAD : CONFIG.FAVICON.DEFAULT;
	}

As long as the totalCount is not 0 - display it in the tab title

And we can see it's sometimes getting called with -1:

App/src/libs/UnreadIndicatorUpdater/index.js

Line 18 in b4c876a

updateUnread(-1);

[kidroca]

I think we should reopen the original issue: #4425 as we're intentionally using the value -1 and we're handling it differently per platform.
In order to fix it we need to clarify what's expected to be displayed when the underlying value is -1 #4425 (comment) (for example we're only handling this value on iOS where we would display it as 1 in the app icon badge)

[mallenexpensify]

@timszot , can you review @kidroca 's post above when you have a min, thx

[timszot]

@mallenexpensify @kidroca I think reopening the original issue here makes sense for the -1 problem since it's different than showing a huge unread count.

[Christinadobrzyn]

PR deployed today so holding this for 7 days before payment!

[parasharrajat]

PR is not yet deployed to PROD

[botify]

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

• https://github.com/Expensify/App/pulls/5302\n

If no regressions arise, payment will be issued on 2021-10-07. 🎊

[Christinadobrzyn]

PR has been in Prod for over 7 days so paid @kidroca in Upwork!

I'm currently ooo until 10-17 so I will close this GH/close the Upwork job when I'm back.

[mallenexpensify]

@kidroca has been paid, closing this issue and the Upwork job

[Julesssss]

A similar issue was just reported: #7914 (comment)

[kidroca]

The problem in this task (#4273) was caused by maxSequenceNumber being incorrectly overwritten during paging
The problem in #7914 and #6591 is there's a change to set all the messages in a conversation as unread: #6591 (comment)