Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.58k stars 2.92k forks source link

[$250] No popup to open desktop app for 'Try New Expensify' link #52450

Open IuliiaHerets opened 2 weeks ago

IuliiaHerets commented 2 weeks ago

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: v9.0.60-2 Reproducible in staging?: Y Reproducible in production?: Y Issue was found when executing this PR: https://github.com/Expensify/App/pull/52255 Email or phone of affected tester (no customers): applausetester+bp2410d@app;ause.expensifail.com Issue reported by: Applause Internal Team

Action Performed:

Precondition: user is logged in OD.

  1. OD: Go to Settings > Account
  2. Click on Credit Card Import > Import Card/Bank
  3. Click on Privacy at the bottom
  4. Go back to OD
  5. Click on 'Try New Expensify'

Expected Result:

User is redirected to NewDot application. Popup with suggestion to open desktop app appears.

Actual Result:

User is redirected to NewDot application. No popup with suggestion to open desktop app.

Workaround:

Unknown

Platforms:

Screenshots/Videos

https://github.com/user-attachments/assets/7735fcf3-75af-4a95-92de-2760f1dfa248

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021859311638145427434
  • Upwork Job ID: 1859311638145427434
  • Last Price Increase: 2024-11-27
Issue OwnerCurrent Issue Owner: @Pujan92
melvin-bot[bot] commented 2 weeks ago

Triggered auto assignment to @JmillsExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

bernhardoj commented 2 weeks ago

Proposal

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

There is no prompt to open in desktop app.

What is the root cause of that problem?

When we open ND from OD, the link will have a transition path. If it's a transition URL, the prompt will be pending by App.beginDeepLinkRedirectAfterTransition(). https://github.com/Expensify/App/blob/67d7299079a14ee9dc22c17a5174209b70e9e61c/src/components/DeeplinkWrapper/index.website.tsx#L19-L31

beginDeepLinkRedirectAfterTransition will only resolved after signOnTransitionToFinishPromise promise is resolved. https://github.com/Expensify/App/blob/67d7299079a14ee9dc22c17a5174209b70e9e61c/src/libs/actions/App.ts#L501-L503

It will be resolved in setUpPoliciesAndNavigate. https://github.com/Expensify/App/blob/67d7299079a14ee9dc22c17a5174209b70e9e61c/src/libs/actions/App.ts#L441-L447

setUpPoliciesAndNavigate is called when the AuthScreen is mounted. https://github.com/Expensify/App/blob/67d7299079a14ee9dc22c17a5174209b70e9e61c/src/libs/Navigation/AppNavigator/AuthScreens.tsx#L303

But as seen from the code above, the promise is only resolved if exitTo param is not empty. Currently, the exitTo param from OD is empty. exitTo=. So, the promise is never resolved.

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

Fix the OD to provide the exitTo

OR

I think we should set a fallback to the exitTo when calling navigate. If it's empty, we use HOME.

https://github.com/Expensify/App/blob/67d7299079a14ee9dc22c17a5174209b70e9e61c/src/libs/actions/App.ts#L425

Then remove the check for exitTo. https://github.com/Expensify/App/blob/67d7299079a14ee9dc22c17a5174209b70e9e61c/src/libs/actions/App.ts#L441 https://github.com/Expensify/App/blob/67d7299079a14ee9dc22c17a5174209b70e9e61c/src/libs/actions/App.ts#L419-L421

Also, when the exitTo is empty, it will show infinite loading

image

It's because when we navigate to HOME, it won't go anywhere because we are already at HOME, but the transition page is still at the nav stack. https://github.com/Expensify/App/blob/67d7299079a14ee9dc22c17a5174209b70e9e61c/src/libs/actions/App.ts#L441-L446

So, we need to call Navigation.goBack too, but only if exitTo is empty because when exitTo is available, we already popped it out from LogOutPreviousUserPage. (basically we have 2 places of handling transition)

https://github.com/Expensify/App/blob/67d7299079a14ee9dc22c17a5174209b70e9e61c/src/pages/LogOutPreviousUserPage.tsx#L77-L87

if (!isLoggingInAsNewUser) {
    Navigation.waitForProtectedRoutes()
        .then(() => {
            if (isTransitioning && !exitTo) {
                Navigation.goBack();
            }
            Navigation.navigate(exitTo);
        })
        .then(endSignOnTransition);
}

OR

We can immediately call endSignOnTransition when exitTo isn't available.

https://github.com/Expensify/App/blob/67d7299079a14ee9dc22c17a5174209b70e9e61c/src/libs/actions/App.ts#L419-L421 https://github.com/Expensify/App/blob/67d7299079a14ee9dc22c17a5174209b70e9e61c/src/libs/actions/App.ts#L441-L447

if (!session || !currentUrl?.includes('exitTo')) {
    endSignOnTransition();
    return;
}
if (!isLoggingInAsNewUser && exitTo) {
    Navigation.waitForProtectedRoutes()
        .then(() => {
            Navigation.navigate(exitTo);
        })
        .then(endSignOnTransition);
} else {
    endSignOnTransition();
}
melvin-bot[bot] commented 1 week ago

@JmillsExpensify Eep! 4 days overdue now. Issues have feelings too...

melvin-bot[bot] commented 1 week ago

@JmillsExpensify 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

melvin-bot[bot] commented 1 week ago

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

melvin-bot[bot] commented 1 week ago

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

JmillsExpensify commented 1 week ago

Opened up to the community since we have a proposal

melvin-bot[bot] commented 3 days ago

@JmillsExpensify, @Pujan92 Eep! 4 days overdue now. Issues have feelings too...

JmillsExpensify commented 2 days ago

@Pujan92 proposal above when you have a moment.

melvin-bot[bot] commented 2 days ago

@JmillsExpensify @Pujan92 this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

melvin-bot[bot] commented 1 day ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

Pujan92 commented 1 day ago

I think the last option from @bernhardoj's proposal of calling endSignOnTransition when exitTo isn't available looks good to me.

🎀👀🎀 C+ reviewed

melvin-bot[bot] commented 1 day ago

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

lakchote commented 23 hours ago

Let's go with @bernhardoj's proposal, by calling endSignOnTransition when exitTo isn't available.