Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chat- Incorrect unread messages shown in green badge after "marked as unread" for any message #5106

Closed
kavimuru opened this issue Sep 6, 2021 · 24 comments
Assignees
Labels
Engineering Improvement Item broken or needs improvement. Weekly KSv2

Comments

@kavimuru
Copy link

kavimuru commented Sep 6, 2021

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:

  1. Go to https://staging.new.expensify.com
  2. Log in with any account
  3. Go to any user conversation
  4. Scroll to the top 10-15 messages and select "Mark as unread"
  5. Check green badge

Expected Result:

Numbers of unread messages are matching the green badge after marked messages unreaded

Actual Result:

Numbers of unread messages aren't matching the green badge after marked messages unreaded

Workaround:

Unknown

Platform:

Where is this issue occurring?

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

Version Number: 1.0.94.0
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

Bug5225474_Recording__325.mp4

Expensify/Expensify Issue URL:

View all open jobs on GitHub

@MelvinBot
Copy link

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

@pecanoro
Copy link
Contributor

pecanoro commented Sep 8, 2021

I tested it and in my case, I always get an extra one, so I can reproduce it. Assigning external label.

@pecanoro pecanoro added the Improvement Item broken or needs improvement. label Sep 8, 2021
@pecanoro pecanoro removed their assignment Sep 8, 2021
@pecanoro pecanoro added the External Added to denote the issue can be worked on by a contributor label Sep 8, 2021
@MelvinBot
Copy link

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

@MitchExpensify
Copy link
Contributor

Upwork job

@kidroca
Copy link
Contributor

kidroca commented Sep 8, 2021

I think this is related to #4273 and the problem I've found with scrolling up: #4273 (comment)

The steps to recreate are pretty much the same: #4273 (comment), with the addition of marking a comment as unread

@harshmanwani
Copy link

I reproduced it on stage and this looked like a similar problem I had received in a previous project.

The count is wrong due to the index we are splicing it from in the chat message list. or if that's not the case, probable the issue in the looping over function.

Sending a proposal on the Upwork listing to work on the fix right away.

@Christinadobrzyn
Copy link
Contributor

Thanks for the input @kidroca - I'm thinking we should combine this issue with 4273.

Which to keep open is the question?

@Christinadobrzyn Christinadobrzyn added Weekly KSv2 and removed Daily KSv2 External Added to denote the issue can be worked on by a contributor labels Sep 10, 2021
@Christinadobrzyn
Copy link
Contributor

Christinadobrzyn commented Sep 10, 2021

Taking this over from Mitch - since it's related to a GH I've been holding for a while!

Based on @kidroca's great suggestions, I think we should hold off on hiring for this issue while we implement a fix for this issue which will likely fix this one.

@Christinadobrzyn
Copy link
Contributor

Hired @kidroca for this related GH - we'll see it the PR resolves this issue.

@isagoico
Copy link

Issue not reproducible during KI retests.

@kidroca
Copy link
Contributor

kidroca commented Sep 13, 2021

@isagoico
I was able to recreate with slightly different steps

  1. Fresh login
  2. Select and mark a message near the top end
  3. Scroll up soon after that
New.Expensify.-.Google.Chrome.2021-09-13.18-29-07.mp4

It seems to resolves itself if you scroll down and then back up agan, or maybe if you just scroll up

@kidroca
Copy link
Contributor

kidroca commented Sep 13, 2021

Another thing I've noticed is we're counting deleted comments

  1. Login with 2 accounts side by side
  2. Tag a comment as unread and mark the count of unread comments (in my case 18)
  3. Delete some comments from the sender's account (I delete 4 messages around the 0:18 mark)
  4. Refresh the page
  5. Mark the same comment as you did in step 2. Observe the count is still the same even though the messages on screen are less (In my case the count remained 18)
Mark.as.unread.mp4

I've noticed the same thing happens with your own messages

  1. When you have a convo with mix of you and other participants
  2. When you mark a point in time that includes comments from you
  3. If you delete you messages and mark the same comment from step 2. You still get the same count that includes the deleted messages

@isagoico
Copy link

Issue reproducible during KI retests.

@Christinadobrzyn
Copy link
Contributor

Not overdue, I'm tracking this

@MelvinBot MelvinBot removed the Overdue label Sep 22, 2021
@isagoico
Copy link

Issue reproducible during KI retests. (2nd week)

@kidroca
Copy link
Contributor

kidroca commented Sep 27, 2021

PR regarding incorrect unread count was just merged: #5302

I believe it also addresses the current issue - I can't recreate it anymore
Note: if you have deleted comments when you mark something as unread the count might be wrong - but this is handled in a separate issue: #5237

@Christinadobrzyn
Copy link
Contributor

Amazing!!! Thank you so much for working on this @kidroca! We'll keep this open for some more testing from @isagoico to see if it's fixed!

@isagoico
Copy link

isagoico commented Oct 4, 2021

Issue not reproducible during KI retests

@Christinadobrzyn
Copy link
Contributor

Amazing! I'm going to keep this open while we wait for the 7 day mark of #5302 to make sure this stays resolved.

@Christinadobrzyn
Copy link
Contributor

Closing - because #5302 is good!

@mvtglobally
Copy link

Issue reproducible during KI retests.

@kidroca
Copy link
Contributor

kidroca commented Oct 11, 2021

Since the bug covered in #5302 caused an incorrect last read action to be saved on the backend, it's possible to occur even after the fix is applied
To ensure this doesn't happen it would be best to:

  1. Log out
  2. Sign in. If there are any chats with unread notifications - open them to clear unread messages (don't scroll up, just open the convo) (Incorrectly stored data appears and is fixed at this step)
  3. Finally log out again

@mvtglobally
Copy link

Issue is not reproducible during KI retests. (First Week)

@Christinadobrzyn
Copy link
Contributor

Since the issue isn't reproducible in latest retests, going to close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Engineering Improvement Item broken or needs improvement. Weekly KSv2
Projects
None yet
Development

No branches or pull requests

10 participants