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] Deeplink – Infinite skeleton loading when navigate via Concierge link and log in as new user #39907

Closed
4 of 6 tasks
izarutskaya opened this issue Apr 9, 2024 · 30 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@izarutskaya
Copy link

izarutskaya commented Apr 9, 2024

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: v1.4.61-0
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4480351
Email or phone of affected tester (no customers): ponikarchuks+28424@gmail.com
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

  1. Go to https://staging.new.expensify.com/concierge
  2. Log in as a new user.

Expected Result:

Concierge chat opens when navigate via Concierge link and log in as a new user

Actual Result:

Infinite skeleton loading when navigate via Concierge link and log in as a new user.
The same when navigate to IOU while logged out.

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

Bug6442758_1712606543105.loading.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01eb559dd4b55cb40a
  • Upwork Job ID: 1777728606282014720
  • Last Price Increase: 2024-04-09
@izarutskaya izarutskaya added DeployBlockerCash This issue or pull request should block deployment Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Apr 9, 2024
Copy link

melvin-bot bot commented Apr 9, 2024

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

Copy link

melvin-bot bot commented Apr 9, 2024

Triggered auto assignment to @cristipaval (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Apr 9, 2024
Copy link
Contributor

github-actions bot commented Apr 9, 2024

👋 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.

@izarutskaya
Copy link
Author

@bfitzexpensify I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors.

@izarutskaya
Copy link
Author

Production

Recording.2356.mp4

@cristipaval
Copy link
Contributor

I suspect this PR is the offending one

@cristipaval
Copy link
Contributor

Well, I think this isn't a blocker and I'm not even sure it is an issue either. So the chat in the background doesn't load while the onboarding modal is shown, but everything works fine after the user closes/finishes the onboarding modal 🤷

@cristipaval
Copy link
Contributor

I'm demoting this one, given that it is a small inconvenience, and I don't think it is worth reverting the offending PR. It is huge and brings more value to the product.

@cristipaval cristipaval added Daily KSv2 External Added to denote the issue can be worked on by a contributor and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 labels Apr 9, 2024
@melvin-bot melvin-bot bot changed the title Deeplink – Infinite skeleton loading when navigate via Concierge link and log in as new user [$250] Deeplink – Infinite skeleton loading when navigate via Concierge link and log in as new user Apr 9, 2024
Copy link

melvin-bot bot commented Apr 9, 2024

Job added to Upwork: https://www.upwork.com/jobs/~01eb559dd4b55cb40a

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 9, 2024
Copy link

melvin-bot bot commented Apr 9, 2024

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

@mountiny mountiny changed the title [$250] Deeplink – Infinite skeleton loading when navigate via Concierge link and log in as new user [$500] Deeplink – Infinite skeleton loading when navigate via Concierge link and log in as new user Apr 9, 2024
Copy link

melvin-bot bot commented Apr 9, 2024

Upwork job price has been updated to $500

@mountiny mountiny moved this to Release 1: Spring 2024 (May) in [#whatsnext] #wave-collect Apr 9, 2024
@mananjadhav
Copy link
Collaborator

mananjadhav commented Apr 9, 2024

I can confirm @hoangzinh's fix is working. We just finished another deploy blocker and did come across this issue. Using hasSelectedPurpose works.

@rushatgabhane
Copy link
Member

We just finished another deploy blocker and did come across this issue

@mananjadhav wait is this being fixed somewhere else?

@rushatgabhane
Copy link
Member

ah okay must be another blocker from the same PR

@mananjadhav
Copy link
Collaborator

mananjadhav commented Apr 9, 2024

No we fixed another issue, and while testing I came across this issue. Saw that this was already tracked here.

ah okay must be another blocker from the same PR

Yes.

@nkdengineer
Copy link
Contributor

nkdengineer commented Apr 9, 2024

Proposal

Please re-state the problem that we are trying to solve in this issue.

Infinite skeleton loading when navigate via Concierge link and log in as a new user.

What is the root cause of that problem?

From this change, we've decoupled the server is ready status from the ONYXKEYS.NVP_INTRO_SELECTED.

If we look at other places where checkOnReady is called, here, here, and here, they're all related to a dependency of isReady status.

However, after the change above the checkOnReady is no longer dependent on ONYXKEYS.NVP_INTRO_SELECTED, thus checkOnReady is no longer called in the Onyx.connect of ONYXKEYS.NVP_INTRO_SELECTED, as also confirmed here.

What changes do you think we should make in order to solve the problem?

We should remove the legacy hasSelectedChoice from here because it's no longer used.

But we should not add the hasSelectedPurpose because the is ready status is not dependent on hasSelectedPurpose, so there'll be a race condition where if ONYXKEYS.NVP_INTRO_SELECTED key here is updated last in Onyx.connect, resolveIsReadyPromise will never be called (because at the 3 points checkOnReady is triggered above, the hasSelectedPurpose is still undefined at that time). This race condition won't happen for other dependencies of is ready like isFirstTimeNewExpensifyUser, because whenever such dependency is updated, it will always call checkOnReady again

What alternative solutions did you explore? (Optional)

NA

@mountiny
Copy link
Contributor

mountiny commented Apr 9, 2024

@nkdengineer Would you be able to raise a PR quickly?

@nkdengineer
Copy link
Contributor

@mountiny Sure, please assign me so I can raise the PR.

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 10, 2024
Copy link

melvin-bot bot commented Apr 10, 2024

📣 @nkdengineer You have been assigned to this job!
Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

@nkdengineer
Copy link
Contributor

@rushatgabhane The PR is here.

@rushatgabhane
Copy link
Member

PR in production for > 7 days. #39996 (comment)

@bfitzexpensify please attach payment summary when you can, thank you! 🙇

@rushatgabhane
Copy link
Member

created a manual request here - https://staging.new.expensify.com/r/7722843441959164

@bfitzexpensify
Copy link
Contributor

Payment summary: $500 for @rushatgabhane for PR review

@JmillsExpensify
Copy link

$500 approved for @rushatgabhane

@nkdengineer
Copy link
Contributor

nkdengineer commented Jul 1, 2024

@JmillsExpensify @bfitzexpensify I was the assigned contributor and haven't been paid, would you mind reopening the issue to handle this?

TIA

@bfitzexpensify
Copy link
Contributor

Confirmed this wasn't paid against the previous Upwork job.

@nkdengineer I just sent you an offer

@bfitzexpensify bfitzexpensify reopened this Jul 1, 2024
@nkdengineer
Copy link
Contributor

@bfitzexpensify Offer accepted, thanks 🙇

@bfitzexpensify
Copy link
Contributor

All sorted now!

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. Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2
Projects
No open projects
Archived in project
Development

No branches or pull requests

9 participants