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.34k stars 2.77k forks source link

Sign in - Deep linking to onboarding and signing up with new gmail bypasses onboarding flow #49404

Open izarutskaya opened 20 hours ago

izarutskaya commented 20 hours 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: 9.0.37-0 Reproducible in staging?: Y Reproducible in production?: Y Email or phone of affected tester (no customers): nathanmulugetatesting+1553@gmail.com Logs: https://stackoverflow.com/c/expensify/questions/4856 Issue reported by: #Applause-Internal team

Action Performed:

  1. Sign out from the app
  2. Deep link/navigate to staging.new.expensify.com/onboarding/purpose
  3. Sign up with a new gmail account

Expected Result:

Onboarding flow continues after signing up

Actual Result:

Onboarding flow gets bypassed after signing up

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

https://github.com/user-attachments/assets/dc79e078-2db5-4a84-9b0d-2840bf495d54

View all open jobs on GitHub

melvin-bot[bot] commented 20 hours ago

Triggered auto assignment to @johncschuster (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.

izarutskaya commented 20 hours ago

We think this issue might be related to the #collect project

NJ-2020 commented 6 hours ago

Edited by proposal-police: This proposal was edited at 2024-09-19 12:23:26 UTC.

Proposal

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

Sign in - Deep linking to onboarding and signing up with new gmail bypasses onboarding flow

What is the root cause of that problem?

After we sign in with new account, the startOnboardingFlow function will get invoked 2 times, first inside the BottomTabBar file and second is inside openReportFromDeepLink function because we open the app from deep link

But inside the openReportFromDeepLink function we always redirect the user to onboarding if the user not yet complete the onboarding before redirecting the user to the deep link https://github.com/Expensify/App/blob/f7a4aa13979ae71631df79a6ac181c2e9b08f17f/src/libs/actions/Report.ts#L2702-L2706 BottomTabBar file https://github.com/Expensify/App/blob/f7a4aa13979ae71631df79a6ac181c2e9b08f17f/src/libs/Navigation/AppNavigator/createCustomBottomTabNavigator/BottomTabBar.tsx#L96-L98

So when startOnboardingFlow got invoked 2 times it causes conflict issue and we do not wrap inside Navigation.isNavigationReady when invoking the startOnboardingFlow function

Because after we sign in with new account inside the BottomTabBar file we invoke the startOnboardingFlow function and inside the openReportFromDeepLink function we also invoke the startOnboardingFlow function which causes conflict

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

We can wrap inside Navigation.isNavigationReady when invoking the startOnboardingFlow function

Welcome.isOnboardingFlowCompleted({
    onNotCompleted: () => {
        Navigation.isNavigationReady().then(() => {
            OnboardingFlow.startOnboardingFlow();
        });
    },
});

Or we can wrap inside the startOnboardingFlow function https://github.com/Expensify/App/blob/f7a4aa13979ae71631df79a6ac181c2e9b08f17f/src/libs/actions/Welcome/OnboardingFlow.ts#L90-L98

function startOnboardingFlow() {
    const currentRoute = navigationRef.getCurrentRoute();
    const {adaptedState} = getAdaptedStateFromPath(getOnboardingInitialPath(), linkingConfig.config, false);
    const focusedRoute = findFocusedRoute(adaptedState as PartialState<NavigationState<RootStackParamList>>);
    if (focusedRoute?.name === currentRoute?.name) {
        return;
    }
    Navigation.isNavigationReady().then(() => {
        navigationRef.resetRoot(adaptedState);
    })
}

What alternative solutions did you explore? (Optional)

blazejkustra commented 2 hours ago

I can't reproduce it locally, am I doing something wrong? I used both gmail and swmansion domains, and the onboarding modal was always displayed.

https://github.com/user-attachments/assets/73a0f14f-0210-4a64-bcc1-58a0083fc4da

blazejkustra commented 2 hours ago

Two additional questions @izarutskaya:

  1. Why would we deeplink to the onboarding?
  2. Should I sign in as a completely new user or a user that logged in before but haven't finished onboarding flow yet?
bernhardoj commented 2 hours ago

Edited by proposal-police: This proposal was edited at 2024-09-19 11:32:57 UTC.

Proposal

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

The onboarding modal doesn't show when deep linking to the onboarding and sign up as a new user

What is the root cause of that problem?

When we deep link to the onboarding page, we first check whether the user has completed the onboarding flow or not. https://github.com/Expensify/App/blob/37bf6b2fef128786dfcd83257a56f56c0cd1eb3d/src/libs/actions/Report.ts#L2699-L2707

After sign up, the onboarding data isn't ready yet and hasCompletedGuidedSetupFlowSelector by default returns true. So, the if is passed and the user is navigated to the onboarding page.

In the onboarding modal navigator, if the user has completed the flow (which is true because the default value is true), they will be redirected back to concierge chat, https://github.com/Expensify/App/blob/37bf6b2fef128786dfcd83257a56f56c0cd1eb3d/src/libs/Navigation/AppNavigator/Navigators/OnboardingModalNavigator.tsx#L36-L48

but it will wait for the app loading to complete first. https://github.com/Expensify/App/blob/37bf6b2fef128786dfcd83257a56f56c0cd1eb3d/src/libs/actions/Report.ts#L2164-L2170

When the data is ready, the OnboardingFlow.startOnboardingFlow() is also triggered which calls resetRoot on the navigation container. https://github.com/Expensify/App/blob/37bf6b2fef128786dfcd83257a56f56c0cd1eb3d/src/libs/actions/Welcome/OnboardingFlow.ts#L90-L97

Based on my log, the calls to reset happens first before the navigation to the concierge chat and looks like the reset is ignored.

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

Because we default hasCompletedGuidedSetupFlowSelector to true, we need to check it only if the onboarding is available. https://github.com/Expensify/App/blob/37bf6b2fef128786dfcd83257a56f56c0cd1eb3d/src/libs/actions/Report.ts#L2677-L2689

if (onboarding === undefined) {
    return;
}
...

What alternative solutions did you explore? (Optional)

Remove hasCompletedGuidedSetupFlow check and move the rest of the logic to onCompleted callback because isOnboardingFlowCompleted only trigger the callback if the data is ready. https://github.com/Expensify/App/blob/37bf6b2fef128786dfcd83257a56f56c0cd1eb3d/src/libs/actions/Report.ts#L2702-L2707

bernhardoj commented 2 hours ago

@blazejkustra you need to sign in as a new user.

NJ-2020 commented 1 hour ago

Proposal Updated