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

CRITICAL: Sometimes messages you receive while in focus mode do not make the chat show up in the LHN #41112

Closed
iwiznia opened this issue Apr 26, 2024 · 15 comments
Assignees
Labels
Daily KSv2 Engineering Reviewing Has a PR in review

Comments

@iwiznia
Copy link
Contributor

iwiznia commented Apr 26, 2024

Context https://expensify.slack.com/archives/C03SDMF9YJ2/p1713913891166219

Steps to reproduce (sometimes, can't reliably reproduce it but maybe in dev is easier as we are not receiving "random" push updates like in prod):

  • Be on focus mode
  • Delete from onyx the report_X and reportMetaData_X keys for a report
  • Have the other participant send you a message
  • You will see the notification, but no item in LHN
Copy link

melvin-bot bot commented Apr 29, 2024

@iwiznia, @tgolen Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@tgolen tgolen removed their assignment Apr 30, 2024
@melvin-bot melvin-bot bot removed the Overdue label Apr 30, 2024
@danielrvidal
Copy link
Contributor

This is happening to me. I put more info in here. I'm also going to add this to #vip-vsb for @quinthar to track.

Here is what I was currently experiencing:

  1. I know I have unread messages on mobile. (Image 1)
  2. When I go into web I cannot see them (Image 2)
  3. Signing in and out makes it so I can see the messages (Image 3)

image
image
image

@melvin-bot melvin-bot bot added the Overdue label May 1, 2024
@quinthar quinthar changed the title Sometimes messages you receive while in focus mode do not make the chat show up in the LHN HIGH: [UX Reliability] Sometimes messages you receive while in focus mode do not make the chat show up in the LHN May 2, 2024
@melvin-bot melvin-bot bot removed the Overdue label May 3, 2024
@danieldoglas
Copy link
Contributor

firefighting Today, but this had a lot of discussions here: https://expensify.slack.com/archives/C05LX9D6E07/p1715013025244169?thread_ts=1714752429.173899&cid=C05LX9D6E07

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels May 6, 2024
Copy link

melvin-bot bot commented May 10, 2024

@danieldoglas Whoops! This issue is 2 days overdue. Let's get this updated quick!

@quinthar quinthar changed the title HIGH: [UX Reliability] Sometimes messages you receive while in focus mode do not make the chat show up in the LHN CRITICAL: Sometimes messages you receive while in focus mode do not make the chat show up in the LHN May 12, 2024
@danieldoglas
Copy link
Contributor

This is being discussed

@muttmuure
Copy link
Contributor

Unassigning Dan as he can't make this his core focus right now, please reassign yourself if you can

@muttmuure
Copy link
Contributor

@kidroca could you comment on this issue and I'll assign you?

@kidroca
Copy link
Contributor

kidroca commented May 17, 2024

@kidroca could you comment on this issue and I'll assign you?

Yep, here we go

@marcaaron marcaaron self-assigned this May 17, 2024
@marcaaron
Copy link
Contributor

I'll work with @kidroca on this.

@quinthar quinthar assigned tgolen and unassigned tgolen May 20, 2024
@marcaaron
Copy link
Contributor

Action plan (based on the conversation here)

Backend part: Create GetMissingReport command that returns the full report minus any existing fields the user already has (can be in review by EOD)

Frontend part: Create some detection logic in the ReportIDsContextProvider to check if we have a partial report and then call GetMissingReport if we do. We will provide the existing report properties as existingPropertyList param so that the backend doesn't send them again. We will also add a check to look at allReportActions to see if there are any which do not have a corresponding report at all. We should call the API to get the missing report for this case. However, we should treat it as unexpected and log a warning that includes what priorityMode the user has. Seems we do not understand the case where a report does not exist at all very well yet, but has been reported by @AndrewGable so curious to learn more about how it can happen.

Not completely closing the door on sending the data at a lower level (e.g. when creating any report action). But we need a better idea of the minimum required data that we should send before we do this (which I think @kidroca is currently looking into and thinking of alternatives).

@melvin-bot melvin-bot bot added the Overdue label May 23, 2024
@marcaaron
Copy link
Contributor

Ok, changed the plan again.

Now, we are gonna do a backend only solution where we send the partial report update needed to show the LHN and see how far it gets us. We changed this up after doing a deep dive on the required fields and realizing there's not that many of them to send.

@melvin-bot melvin-bot bot removed the Overdue label May 23, 2024
@marcaaron marcaaron added the Reviewing Has a PR in review label May 24, 2024
@marcaaron
Copy link
Contributor

Auth PR is ready. There are two Web-Expensify tests that will need to be fixed first.

@marcaaron
Copy link
Contributor

@Beamanator will be reviewing the PRs ASAP.

@marcaaron
Copy link
Contributor

Auth PR has been merged here. Once site is stable I will prod QA this one and we can discuss next steps (or close for now).

@marcaaron
Copy link
Contributor

Just re-tested and appears to not be happening anymore. 🎉

2024-06-03_14-18-50

If we run into a new reproduction - let's create a new issue? From my conversations with @AndrewGable it sounded like there is similar/same bug with a different root cause, but we never identified how it can happen and this might solve that one too. Though, I suspect most reported cases are due to the reproduction listed in the description of this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Daily KSv2 Engineering Reviewing Has a PR in review
Projects
Status: Done
Development

No branches or pull requests

7 participants