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

[$500] [MEDIUM] LHN - Green dot(IOU) disappears from WS conversation in LHN when going offline-online #33544

Closed
6 tasks done
izarutskaya opened this issue Dec 23, 2023 · 52 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review

Comments

@izarutskaya
Copy link

izarutskaya commented Dec 23, 2023

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: 1.4.16-3
Reproducible in staging?: Y
Reproducible in production?: N
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: Applause-Internal Team
Slack conversation: @

Action Performed:

⚠️ To reproduce you need an account that has the beta: excludedFromReliableUpdates.

  1. Open New Expensify app
  2. Log in with any account
  3. Create new workspace
  4. Request money on that workspace
  5. Go to offline mode and after a few seconds return to online mode

Expected Result:

The IOU green dot must be present in the LHN in any mode of operation until the IOU request is paid or an IOU error occurs.

Actual Result:

The green dot IOU disappears from the LHN if the user goes offline and comes back online.

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

Bug6324826_1703328627225.Recording__981.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~019c2e500b47b7def2
  • Upwork Job ID: 1738525487918645248
  • Last Price Increase: 2024-01-31
@izarutskaya izarutskaya added DeployBlockerCash This issue or pull request should block deployment External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Dec 23, 2023
Copy link

melvin-bot bot commented Dec 23, 2023

Job added to Upwork: https://www.upwork.com/jobs/~019c2e500b47b7def2

@melvin-bot melvin-bot bot changed the title LHN - Green dot(IOU) disappears from WS conversation in LHN when going offline-online [$500] LHN - Green dot(IOU) disappears from WS conversation in LHN when going offline-online Dec 23, 2023
Copy link

melvin-bot bot commented Dec 23, 2023

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 23, 2023
Copy link

melvin-bot bot commented Dec 23, 2023

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

Copy link

melvin-bot bot commented Dec 23, 2023

Triggered auto assignment to Contributor-plus team member for initial proposal review - @alitoshmatov (External)

Copy link
Contributor

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

Copy link

melvin-bot bot commented Dec 23, 2023

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

@paultsimura
Copy link
Contributor

First of all, this is not a deploy blocker – it's happening in prod as well:

image

We're potentially facing the out-of-order Onyx updates again:
When coming back online, 2 conflicting responses set report.hasOutstandingChildRequest:

First comes the ReconnectApp that sets it to false via the mergeCollection action:
image

Second comes the ReconnectToReport, which should set the value to true, but this value is ignored. I suppose it gets overwritten by the mergeCollection from above similar to what was happening in #28737

image

This particular issue can be fixed by making sure the ReconnectApp returns report.hasOutstandingChildRequest: true (which is correct), but the Onyx issue should be addressed as well.

cc: @Julesssss for the out-of-order updates

Copy link

melvin-bot bot commented Dec 25, 2023

📣 @aim-salam! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
    Screen Shot 2022-11-16 at 4 42 54 PM
    Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@aim-salam
Copy link

Contributor details
Your Expensify account email: aimsalam.com@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~0165dd5f4e0e952fdd

Copy link

melvin-bot bot commented Dec 25, 2023

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

@melvin-bot melvin-bot bot added the Overdue label Dec 25, 2023
@Christinadobrzyn Christinadobrzyn added Daily KSv2 and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 labels Dec 25, 2023
@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Dec 25, 2023
@aldo-expensify aldo-expensify added Internal Requires API changes or must be handled by Expensify staff and removed Help Wanted Apply this label when an issue is open to proposals by contributors labels Feb 6, 2024
Copy link

melvin-bot bot commented Feb 6, 2024

Current assignee @alitoshmatov is eligible for the Internal assigner, not assigning anyone new.

@BartoszGrajdek
Copy link
Contributor

BartoszGrajdek commented Feb 6, 2024

Hi! I'm Bartosz from Software Mansion, I was working on this issue yesterday (I wanted to run a few more tests today) and I think that it may have been fixed with navigation rework. Following these steps here's the result:

lhn-test.mp4

@aldo-expensify
Copy link
Contributor

oh, nice! thanks for taking a look @BartoszGrajdek

@melvin-bot melvin-bot bot added the Overdue label Feb 9, 2024
@aldo-expensify
Copy link
Contributor

👋 @BartoszGrajdek where you able to run more tests?

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Feb 9, 2024
@BartoszGrajdek
Copy link
Contributor

Oh, I took it as if you didn't need any more help, so I just shared what I have found out. I tested out several scenarios with going online/offline and it really looks like it got fixed. I can investigate what might have fixed this problem if you want to - just let me know 😄

@aldo-expensify
Copy link
Contributor

I can investigate what might have fixed this problem if you want to - just let me know 😄

No need @BartoszGrajdek , thank you again. If we can't reproduce, we can just close this.

@lanitochka17
Copy link

Issue is still reproducible on the latest build 1.4.40-1

Recording.1331.mp4

@lanitochka17 lanitochka17 reopened this Feb 12, 2024
@BartoszGrajdek
Copy link
Contributor

Well, that's strange. I tested it by trying both steps found here and here. I also tried it on multiple browsers just to be sure and I still couldn't reproduce the problem.

Maybe I'm missing something, or this is some edge case and we still haven't figured out all the necessary steps. I'm attaching a video of how it behaves for me (build 1.4.40-4) @lanitochka17 let me know if there's anything I have done incorrectly there.

@aldo-expensify
Copy link
Contributor

For some reason I'm not being able to reproduce in main in my dev environment but I was able to reproduce in https://staging.new.expensify.com/

Screen.Recording.2024-02-13.at.11.14.10.AM.mov

@aldo-expensify
Copy link
Contributor

From looking at ReconnectApp and OpenReport response, I agree with @paultsimura 's findings here and it does look like ReconnectApp calculates hasOutstandingChildRequest incorrectly.

@aldo-expensify
Copy link
Contributor

Reproduced in dev, the key is that the account has to have the beta excludedFromReliableUpdates. Accounts under expensifail.com , applause.expensifail.com and some big domains have this, so I guess that explain why @BartoszGrajdek can't reproduce.

@aldo-expensify
Copy link
Contributor

I have a fix here https://github.com/Expensify/Auth/pull/9929, I just need to add the tests and I'll make it ready for review

@aldo-expensify aldo-expensify added the Reviewing Has a PR in review label Feb 13, 2024
@Christinadobrzyn
Copy link
Contributor

Yay! Exciting @aldo-expensify!

@aldo-expensify
Copy link
Contributor

PR was deployed

@aldo-expensify
Copy link
Contributor

Tested and now it is working:

Screen.Recording.2024-02-14.at.12.42.17.PM.mov

In the video we can see that the policyExpenseChat (reportID: 2130476157825373) comes with hasOutstandingChildRequest: true in ReconnectApp

@Christinadobrzyn
Copy link
Contributor

hey @aldo-expensify or @alitoshmatov do we need to pay out anyone for this?

@aldo-expensify
Copy link
Contributor

@Christinadobrzyn not as far as I know, the backend PR wasn't reviewed by C+

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review
Projects
No open projects
Development

No branches or pull requests

10 participants