[$250] Log in – Welcome modal is missing when login as a new user from public room login modal

[lanitochka17]

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.69-2 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4531005 Email or phone of affected tester (no customers): ponikarchuks+131524@gmail.com Issue reported by: Applause - Internal Team

Action Performed:

1. Log out of New Expensify
2. Navigate to this link - https://staging.new.expensify.com/r/2652281908731856
3. Verify you can see the announce room as an anonymous user
4. Click on Sign In
5. Log in with a new account

Expected Result: Welcome modal appears and user should enter the name

Actual Result: Welcome modal is missing when login as a new user from public room login modal, the user's name remains Guest

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 Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/d4216f0b-42c0-4d8b-a777-e78c94fa1e4c

View all open jobs on GitHub

[melvin-bot[bot]]

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

[lanitochka17]

@dylanexpensify FYI 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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[c3024]

Proposal

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

Welcome modal does not open for a new account when logged in from right hand sign modal for a public room

What is the root cause of that problem?

The useEffect here https://github.com/Expensify/App/blob/cca1952e18ea67a587d25d8ff5064268b9425758/src/libs/Navigation/AppNavigator/createCustomBottomTabNavigator/BottomTabBar.tsx#L53-L55 for showing the onboarding modal triggers only when isLoadingApp changes and isLoadingApp changes only when openApp is called. https://github.com/Expensify/App/blob/387ab0edd9d2c01c2a0be72950490c94b1b7d9d9/src/libs/actions/App.ts#L216-L225 so, when we visit a public room openApp is called and after logging in from RHP sign in modal, it is not called again and isLoadingApp value does not change and the useEffect does not trigger.

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

We can have an Onyx value to check if the RHP SignIn modal closed after signing in and set it to true here https://github.com/Expensify/App/blob/387ab0edd9d2c01c2a0be72950490c94b1b7d9d9/src/pages/signin/SignInModal.tsx#L29-L30 like

            Navigation.isNavigationReady().then(() => Navigation.dismissModal());
         +++Onyx.set(ONYXKEYS.SIGNIN_MODAL_CLOSED, true);
        }

after the dismissing the RHP. Subscribe to this key in BottomTabBar and add it as a dependency to the useEffect mentioned in the RCA so that when the value changes the useEffect triggers again and the Welcome.isOnboardingFlowCompleted is called.

What alternative solutions did you explore? (Optional)

[NJ-2020]

Proposal

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

The welcome modal is missing when login as a new user from a public room

What is the root cause of that problem?

The BottomTabBar file(Inside Report page) will invoke the Welcome onboarding functionality after the user logs in as a new user from the Login Page, but If I try to log in as a new user from the Public room the BottomTabBar won't be invoked, I think because the BottomTabBar is being cached on the Report Page as I've already opened it so it won't call the Welcome onboarding functionality.

I've tried to hard refresh to remove the cache and the Welcome onboarding is showing up but after a while when I tried to refresh the page again and again it won't show up.

For the Guest input, the issue comes from the backend, and when a user logs in as a new user from the public room we do not remove the Guest input value.

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

We can add the Welcome onboarding functionality inside the LoginForm directly so that when the user logs in as a new account, the Welcome onboarding will be triggered automatically.

https://github.com/Expensify/App/blob/2a6d63d7f17b1e4fba94f26103f13c6afe294b91/src/libs/Navigation/AppNavigator/createCustomBottomTabNavigator/BottomTabBar.tsx#L53

And for the Guest input issue, we can fix it inside LoginForm and Report Utils (for rendering the participant name) directly by checking if the user has logged in as a new user and has not yet completed the Welcome onboarding and the Account Display name as a guest, we can set it the user display name to empty which will show the user email.

I've fixed the issue and it is working successfully, below is the video demo(updated):

https://github.com/Expensify/App/assets/76243292/4b73f3fd-cac5-45e9-9eeb-b6c12464b1a7

What alternative solutions did you explore? (Optional)

N/A

[tsa321]

Proposal

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

Welcome modal doesn't show up when user sign in in rigth panel if the user visit public room.

What is the root cause of that problem?

Welcome step is executed by this function:

https://github.com/Expensify/App/blob/550e83b2f55f0998feece27b1768ad7148d03398/src/libs/Navigation/AppNavigator/createCustomBottomTabNavigator/BottomTabBar.tsx#L53-L55

But it will only executed right after App is finished loading. the use effect depend on isLoadingApp which is based on onyxkey: IS_LOADING_APP.

When user visit public room the app is finished loading and the useEffect function is executed but won't reach welcome function because the user is anonymous user. So when the user sign in using right panel login form, the Welcome.isOnboardingFlowCompleted won't be executed again because there is no change in IS_LOADING_APP value.

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

We can change or add the useEffect dependency using other onyx value for example LAST_VISITED_PATH. So the bottom part of the BottomTabBar could be :

export default withOnyx<PurposeForUsingExpensifyModalProps, PurposeForUsingExpensifyModalOnyxProps>({
    isLoadingApp: {
        key: ONYXKEYS.IS_LOADING_APP,
    },
    lastVisitedPath: {
      key: ONYXKEYS.LAST_VISITED_PATH,
    },
})(BottomTabBar);

the the dependency of useEffect will be:

+ }, [lastVisitedPath, isLoadingApp]);

and to prevent the welcome modal shown two times right after user completed welcome steps and app send nvp onboarding to server then the app navigate to welcome video. We could add:

+ let isOnboardingInProgress : boolean = false;
...
...
function isOnboardingFlowCompleted({onCompleted, onNotCompleted}: HasCompletedOnboardingFlowProps) {
    isOnboardingFlowStatusKnownPromise.then(() => {
        if (Array.isArray(onboarding) || onboarding?.hasCompletedGuidedSetupFlow === undefined) {
            return;
        }

        if (onboarding?.hasCompletedGuidedSetupFlow) {
+            isOnboardingInProgress = false;
            onCompleted?.();
        } else {
+            if (isOnboardingInProgress) {
+             return;
+           }
+            isOnboardingInProgress = true;
            onNotCompleted?.();
        }
    });
}

The solution is not fixed to certain onyxkey but can use other onyxkey based on other consideration. Or the dependency can use navigation variable which store the value of useNavigation.

What alternative solutions did you explore?

Adding dependency on useEffect will cause unnecessary re-trigger of Welcome.isOnboardingFlowCompleted multiples times for existing user. So we can remove the useEffect clause of bottomTabBar where Welcome.isOnboardingFlowCompleted is being triggered and remove the Welcome.isOnboardingFlowCompleted completely. The new logic is based on promise that contains onyx connect session, pathchange, nvp_onboarding and then immediately resolve the promise and disconnect the onyx connection when condition match. This logic is already inside Welcome.ts and I will add promise to session (to check whether user is logged in) and last_path_change (to check on current navigation / route name). This could be done by adding isRouteReadyPromise isUserLoggedInPromise similar to isServerDataReadyPromise and isOnboardingFlowStatusKnownPromise.

function startOnBoarding(){
  listenToSessionChange();
  isUserLoggedInPromise
  .then(() => isServerDataReadyPromise)
  .then(() => {
      listenToNvpOnboardingChange();
      return isOnboardingFlowStatusKnownPromise;
  }).then(() => {
      listenToPathChange();
      return isRouteReadyPromise;
  }).then(() => {
      if (Array.isArray(onboarding) || onboarding?.hasCompletedGuidedSetupFlow === undefined) {
          return;
      }

      if (onboarding?.hasCompletedGuidedSetupFlow) {
          onCompleted?.();
      } else {
          Navigation.navigate(ROUTES.ONBOARDING_ROOT)
          //onNotCompleted?.();
      }
      return;
  });
}
startOnBoarding();

and in callback of LAST_ViSITED_PATH onyx:

          const rootRoute = navigationRef.getRootState()?.routes;
          const currentRouteName = rootRoute?.[rootRoute.length - 1]?.name;
          if (Boolean(currentRouteName && currentRouteName !== NAVIGATORS.BOTTOM_TAB_NAVIGATOR && currentRouteName !== NAVIGATORS.CENTRAL_PANE_NAVIGATOR)) {
            return;
          }

          resolveRouteIsReady();
          Onyx.disconnect(lastVistedConnectID);

Complete rough code:

```javascript let resolveIsReadyPromise: (value?: Promise) => void | undefined; let isServerDataReadyPromise = new Promise((resolve) => { resolveIsReadyPromise = resolve; }); let resolveOnboardingFlowStatus: (value?: Promise) => void | undefined; let isOnboardingFlowStatusKnownPromise = new Promise((resolve) => { resolveOnboardingFlowStatus = resolve; }); let resolveRouteIsReady: (value?: Promise) => void | undefined; let isRouteReadyPromise = new Promise((resolve) => { resolveRouteIsReady = resolve; }); let resolveUserIsLoggedIn: (value?: Promise) => void | undefined; let isUserLoggedInPromise = new Promise((resolve) => { resolveUserIsLoggedIn = resolve; }); function onServerDataReady(): Promise { return isServerDataReadyPromise; } const listenToSessionChange = function() { const sessionConeectionId = Onyx.connect({ key: ONYXKEYS.SESSION, initWithStoredValues: false, callback: (session) => { if (session?.accountID === undefined || session?.authTokenType === CONST.AUTH_TOKEN_TYPES.ANONYMOUS) { return; } resolveUserIsLoggedIn?.(); Onyx.disconnect(sessionConeectionId); }, }); } const listenToPathChange = function() { const connectionId = Onyx.connect({ key: ONYXKEYS.LAST_VISITED_PATH, initWithStoredValues: true, callback: (cc) => { const rootRoute = navigationRef.getRootState()?.routes; const currentRouteName = rootRoute?.[rootRoute.length - 1]?.name; if (Boolean(currentRouteName && currentRouteName !== NAVIGATORS.BOTTOM_TAB_NAVIGATOR && currentRouteName !== NAVIGATORS.CENTRAL_PANE_NAVIGATOR)) { return; } resolveRouteIsReady(); Onyx.disconnect(lastVistedConnectID); }, }); } const listenToNvpOnboardingChange = function() { const connectionId = Onyx.connect({ key: ONYXKEYS.NVP_ONBOARDING, initWithStoredValues: true, callback: (value) => { if (value === null || value === undefined) { return; } onboarding = value; resolveOnboardingFlowStatus(); Onyx.disconnect(connectionId); }, }); } function startOnBoarding(){ listenToSessionChange(); isUserLoggedInPromise .then(() => isServerDataReadyPromise) .then(() => { listenToNvpOnboardingChange(); return isOnboardingFlowStatusKnownPromise; }).then(() => { listenToPathChange(); return isRouteReadyPromise; }).then(() => { if (Array.isArray(onboarding) || onboarding?.hasCompletedGuidedSetupFlow === undefined) { return; } if (onboarding?.hasCompletedGuidedSetupFlow) { onCompleted?.(); } else { Navigation.navigate(ROUTES.ONBOARDING_ROOT) //onNotCompleted?.(); } return; }); } startOnBoarding(); ```

[NJ-2020]

Proposal

Updated

[dylanexpensify]

@rojiphil to review updated proposal

[rojiphil]

Reviewing today

[rojiphil]

We can add the Welcome onboarding functionality inside useEffect and trigger it when the user logs in as a new user from a public room I've fixed the issue and it is working successfully,

@NJ-2020 Can you please add relevant code to your proposal so we know how you fixed it?

[NJ-2020]

Proposal

Updated

[NJ-2020]

@rojiphil Done

[rojiphil]

Thanks for your proposals. But I don’t think the RCA is good enough to proceed with any proposal. Simply triggering the Welcome Onboarding flow will not help as it is not required for existing accounts. Further, the proposals do not address the Guest user name issue after a new account login. This is part of the scope of this issue as mentioned in the OP. Looking forward to reviewing your revised proposals.

[NJ-2020]

Ok, I will look into it. Thanks

[c3024]

@rojiphil

Simply triggering the Welcome Onboarding flow will not help as it is not required for existing accounts.

Logic to show the onboarding modal or not is within the isOnboardingFlowCompleted of Welcome. Presently Welcome.isOnboardingFlowCompleted is invoked every time in a normal flow when a user ((including an existing user who completed onboarding already)) logs in because openApp is called on login and the useEffect in BottomTabBar gets triggered due to change of isLoadingApp on calling openApp. But showing the onboarding modal or not is based on the hasCompletedGuidedSetupFlow and this logic is in Welcome here.

https://github.com/Expensify/App/blob/5730ba4dced381adc473e502f675e7cf4e974a19/src/libs/actions/Welcome.ts#L38

For users who already completed the onboarding, the backend returns the hasCompletedGuidedSetupFlow values as true. Nothing happens in this case and no onboarding modal shows up because we are not passing any onCompleted in the Welcome.isOnboardingFlowCompleted.

The proposal extends this logic to trigger it similarly when a user logs in from RHP. So, with the suggested solution Welcome.isOnboardingFlowCompleted is called but the modal is shown only if the guided setup has not been completed yet due to the existing logic in isOnboardingFlowCompleted of Welcome.

About the Guest name, once the welcome modal shows up, changing the name is part of the flow and it will be overwritten.

If we want the display name to be login for new sign-ins from RHP as well similar to new logins in normal flow, that needs to be fixed from backend because Guest is sent from backend.

Let me know if I am missing something or something is unclear in my comments.

[tsa321]

Proposal

updated on alternative solution

But I don’t think the RCA is good enough to proceed with any proposal. Simply triggering the Welcome Onboarding flow will not help as it is not required for existing accounts.

@rojiphil yes adding dependency on useEffect will cause unnecessary re-trigger of Welcome.isOnboardingFlowCompleted multiples times for existing user. So we can remove the useEffect clause of bottomTabBar where Welcome.isOnboardingFlowCompleted is being triggered and remove the Welcome.isOnboardingFlowCompleted completely. The new logic is based on promise that contains onyx connect session, pathchange, nvp_onboarding and then immediately resolve the promise and disconnect the onyx connection when condition match. This logic is already inside Welcome.ts and I will add promise to session (to check whether user is logged in) and last_path_change (to check on current navigation / route name). This could be done by adding isRouteReadyPromise isUserLoggedInPromise similar to isServerDataReadyPromise and isOnboardingFlowStatusKnownPromise.

Complete rough code:

```javascript let resolveIsReadyPromise: (value?: Promise) => void | undefined; let isServerDataReadyPromise = new Promise((resolve) => { resolveIsReadyPromise = resolve; }); let resolveOnboardingFlowStatus: (value?: Promise) => void | undefined; let isOnboardingFlowStatusKnownPromise = new Promise((resolve) => { resolveOnboardingFlowStatus = resolve; }); let resolveRouteIsReady: (value?: Promise) => void | undefined; let isRouteReadyPromise = new Promise((resolve) => { resolveRouteIsReady = resolve; }); let resolveUserIsLoggedIn: (value?: Promise) => void | undefined; let isUserLoggedInPromise = new Promise((resolve) => { resolveUserIsLoggedIn = resolve; }); function onServerDataReady(): Promise { return isServerDataReadyPromise; } const listenToSessionChange = function() { const sessionConeectionId = Onyx.connect({ key: ONYXKEYS.SESSION, initWithStoredValues: false, callback: (session) => { if (session?.accountID === undefined || session?.authTokenType === CONST.AUTH_TOKEN_TYPES.ANONYMOUS) { return; } resolveUserIsLoggedIn?.(); Onyx.disconnect(sessionConeectionId); }, }); } const listenToPathChange = function() { const connectionId = Onyx.connect({ key: ONYXKEYS.LAST_VISITED_PATH, initWithStoredValues: true, callback: (cc) => { const rootRoute = navigationRef.getRootState()?.routes; const currentRouteName = rootRoute?.[rootRoute.length - 1]?.name; if (Boolean(currentRouteName && currentRouteName !== NAVIGATORS.BOTTOM_TAB_NAVIGATOR && currentRouteName !== NAVIGATORS.CENTRAL_PANE_NAVIGATOR)) { return; } resolveRouteIsReady(); Onyx.disconnect(connectionId); }, }); } const listenToNvpOnboardingChange = function() { const connectionId = Onyx.connect({ key: ONYXKEYS.NVP_ONBOARDING, initWithStoredValues: true, callback: (value) => { if (value === null || value === undefined) { return; } onboarding = value; resolveOnboardingFlowStatus(); Onyx.disconnect(connectionId); }, }); } function startOnBoarding(){ listenToSessionChange(); isUserLoggedInPromise .then(() => isServerDataReadyPromise) .then(() => { listenToNvpOnboardingChange(); return isOnboardingFlowStatusKnownPromise; }).then(() => { listenToPathChange(); return isRouteReadyPromise; }).then(() => { if (Array.isArray(onboarding) || onboarding?.hasCompletedGuidedSetupFlow === undefined) { return; } if (onboarding?.hasCompletedGuidedSetupFlow) { onCompleted?.(); } else { Navigation.navigate(ROUTES.ONBOARDING_ROOT) //onNotCompleted?.(); } return; }); } startOnBoarding(); ```

function startOnBoarding(){
  listenToSessionChange();
  isUserLoggedInPromise
  .then(() => isServerDataReadyPromise)
  .then(() => {
      listenToNvpOnboardingChange();
      return isOnboardingFlowStatusKnownPromise;
  }).then(() => {
      listenToPathChange();
      return isRouteReadyPromise;
  }).then(() => {
      if (Array.isArray(onboarding) || onboarding?.hasCompletedGuidedSetupFlow === undefined) {
          return;
      }

      if (onboarding?.hasCompletedGuidedSetupFlow) {
          onCompleted?.();
      } else {
          Navigation.navigate(ROUTES.ONBOARDING_ROOT)
          //onNotCompleted?.();
      }
      return;
  });
}
startOnBoarding();

and in callback of LAST_ViSITED_PATH onyx:

          const rootRoute = navigationRef.getRootState()?.routes;
          const currentRouteName = rootRoute?.[rootRoute.length - 1]?.name;
          if (Boolean(currentRouteName && currentRouteName !== NAVIGATORS.BOTTOM_TAB_NAVIGATOR && currentRouteName !== NAVIGATORS.CENTRAL_PANE_NAVIGATOR)) {
            return;
          }

          resolveRouteIsReady();
          Onyx.disconnect(lastVistedConnectID);

Complete rough code:

```javascript let resolveIsReadyPromise: (value?: Promise) => void | undefined; let isServerDataReadyPromise = new Promise((resolve) => { resolveIsReadyPromise = resolve; }); let resolveOnboardingFlowStatus: (value?: Promise) => void | undefined; let isOnboardingFlowStatusKnownPromise = new Promise((resolve) => { resolveOnboardingFlowStatus = resolve; }); let resolveRouteIsReady: (value?: Promise) => void | undefined; let isRouteReadyPromise = new Promise((resolve) => { resolveRouteIsReady = resolve; }); let resolveUserIsLoggedIn: (value?: Promise) => void | undefined; let isUserLoggedInPromise = new Promise((resolve) => { resolveUserIsLoggedIn = resolve; }); function onServerDataReady(): Promise { return isServerDataReadyPromise; } const listenToSessionChange = function() { const sessionConeectionId = Onyx.connect({ key: ONYXKEYS.SESSION, initWithStoredValues: false, callback: (session) => { if (session?.accountID === undefined || session?.authTokenType === CONST.AUTH_TOKEN_TYPES.ANONYMOUS) { return; } resolveUserIsLoggedIn?.(); Onyx.disconnect(sessionConeectionId); }, }); } const listenToPathChange = function() { const connectionId = Onyx.connect({ key: ONYXKEYS.LAST_VISITED_PATH, initWithStoredValues: true, callback: (cc) => { const rootRoute = navigationRef.getRootState()?.routes; const currentRouteName = rootRoute?.[rootRoute.length - 1]?.name; if (Boolean(currentRouteName && currentRouteName !== NAVIGATORS.BOTTOM_TAB_NAVIGATOR && currentRouteName !== NAVIGATORS.CENTRAL_PANE_NAVIGATOR)) { return; } resolveRouteIsReady(); Onyx.disconnect(lastVistedConnectID); }, }); } const listenToNvpOnboardingChange = function() { const connectionId = Onyx.connect({ key: ONYXKEYS.NVP_ONBOARDING, initWithStoredValues: true, callback: (value) => { if (value === null || value === undefined) { return; } onboarding = value; resolveOnboardingFlowStatus(); Onyx.disconnect(connectionId); }, }); } function startOnBoarding(){ listenToSessionChange(); isUserLoggedInPromise .then(() => isServerDataReadyPromise) .then(() => { listenToNvpOnboardingChange(); return isOnboardingFlowStatusKnownPromise; }).then(() => { listenToPathChange(); return isRouteReadyPromise; }).then(() => { if (Array.isArray(onboarding) || onboarding?.hasCompletedGuidedSetupFlow === undefined) { return; } if (onboarding?.hasCompletedGuidedSetupFlow) { onCompleted?.(); } else { Navigation.navigate(ROUTES.ONBOARDING_ROOT) //onNotCompleted?.(); } return; }); } startOnBoarding(); ```

---

Further, the proposals do not address the Guest user name issue after a new account login.

For the guest user name, The guest name will change when user complete the onboarding processes, after user change the user name.

[NJ-2020]

Proposal

Updated

• Updated video
• Updated Proposal

[NJ-2020]

@rojiphil Done. I've updated my proposal

Looking forward to hearing from you

[NJ-2020]

Proposal

Updated

[melvin-bot[bot]]

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

[rojiphil]

I will review the updated proposals tomorrow

[dylanexpensify]

@rojiphil any update?

[rojiphil]

Thanks for all your proposals. @tsa321 The proposal and follow-up comment proposes additional logic or to reimplement the feature to fix this issue but I don’t think it is required as we can resolve this in simpler steps. @NJ-2020 The updated proposal root cause does not seem correct as I don’t think it has to do anything with caching. The earlier proposed root cause was more correct but it was already shared here. Further, directly calling the welcome onboard flow from within RHP signin modal could solve the problem but is just a minor enhancement to the solution already proposed here.
@c3024 proposal is the closest to the correct RCA as mentioned below and the first to arrive at that. But, do we need an additional Onyx value to solve this?

when we visit a public room openApp is called and after logging in from RHP sign in modal, it is not called again

@c3024 Instead of relying on an additional Onyx value, would it not work if we call App.openApp()? This will also trigger the Welcome onboard flow.

[c3024]

@rojiphil

Yes, it does not need a Onyx value. Calling App.openApp() works just fine.

[rojiphil]

@c3024 proposal LGTM along with comment here. 🎀👀🎀 C+ reviewed

[melvin-bot[bot]]

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

[rojiphil]

@c3024 And oh! I forgot to mention about Guest user name issue. I agree that this is a BE issue but this is a different issue and the details of why this is a BE issue also need to be shared with the internal team.

[melvin-bot[bot]]

@rojiphil @grgia @dylanexpensify 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!