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

[$250] Onboarding - Onboarding Modal opens when returning back online #49499

Open IuliiaHerets opened 2 days ago

IuliiaHerets commented 2 days 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.38-0 Reproducible in staging?: Y Reproducible in production?: Y Issue reported by: Applause Internal Team

Action Performed:

  1. Sign in with a new user
  2. On the Onboarding modal turn off the internet
  3. Go through the onboarding flow and click continue
  4. Refresh the page to kill the app
  5. Go back online and reload

Expected Result:

Reloading after going back online should not bring the Onboarding flow back

Actual Result:

Reloading after going back online bring back the Onboarding flow for a moment

Workaround:

Unknown

Platforms:

Screenshots/Videos

https://github.com/user-attachments/assets/29ca040f-6dff-4547-8102-88ecf81324e0

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021836961456209861456
  • Upwork Job ID: 1836961456209861456
  • Last Price Increase: 2024-09-20
Issue OwnerCurrent Issue Owner: @eVoloshchak
melvin-bot[bot] commented 2 days ago

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

IuliiaHerets commented 2 days ago

@jliexpensify 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

jliexpensify commented 1 day ago

Able to repro this one

melvin-bot[bot] commented 1 day ago

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

melvin-bot[bot] commented 1 day ago

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

dominictb commented 1 day ago

Proposal

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

Reloading after going back online bring back the Onboarding flow for a moment

What is the root cause of that problem?

When calling CompleteGuidedSetup API, we didn't set hasCompletedGuidedSetupFlow: true

https://github.com/Expensify/App/blob/6510ae44fa35163d53e5e9f93047cdff6082790f/src/libs/actions/Report.ts#L3692

so when users reload, the onboarding flow will be shown again

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

set hasCompletedGuidedSetupFlow: true optimistically when calling CompleteGuidedSetup API. We should reset hasCompletedGuidedSetupFlow: false in failureData

What alternative solutions did you explore? (Optional)

huult commented 1 day ago

Proposal

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

Onboarding Modal opens when returning back online

What is the root cause of that problem?

CompleteGuidedSetup will be stored locally when there is no internet connection, and hasCompletedGuidedSetupFlow will not be updated because CompleteGuidedSetup has not been executed yet. Once the connection is restored, the condition checking hasCompletedGuidedSetupFlow will still return false until the CompleteGuidedSetup API is fully executed. As a result, the onboarding process will reappear because the condition to show onboarding is checked before CompleteGuidedSetup is completed. https://github.com/Expensify/App/blob/00d2e327ab2a8cd5dfdae785eb2cc8c5b962c2bd/src/libs/actions/Report.ts#L3692

https://github.com/Expensify/App/blob/00d2e327ab2a8cd5dfdae785eb2cc8c5b962c2bd/src/libs/hasCompletedGuidedSetupFlowSelector.ts#L10

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

We need to add a condition for displaying onboarding. If there is a CompleteGuidedSetup pending for the account, onboarding will not be displayed. By using this method, we can avoid the scenario where, if the CompleteGuidedSetup API call encounters an error, onboarding will still follow the correct flow

// .src/libs/hasCompletedGuidedSetupFlowSelector.ts#L6
function hasCompletedGuidedSetupFlowSelector(onboarding: OnyxValue<typeof ONYXKEYS.NVP_ONBOARDING>): boolean {
    // onboarding is an array for old accounts and accounts created from olddot
    if (Array.isArray(onboarding)) {
        return true;
    }

+    const persistedRequests = PersistedRequests.getAll();
+    const hasPendingCompleteGuidedSetupRequest = persistedRequests.some((item) => item.command === WRITE_COMMANDS.COMPLETE_GUIDED_SETUP);

+    // If there's a pending request to complete guided setup, return false
+    if (hasPendingCompleteGuidedSetupRequest) {
+        return false;
+    }

    return onboarding?.hasCompletedGuidedSetupFlow ?? true;
}
POC https://github.com/user-attachments/assets/d62f32b1-07b7-41de-868c-aff6c132e975
CyberAndrii commented 16 hours ago

This is a regression from https://github.com/Expensify/App/pull/49263 and was fixed in https://github.com/Expensify/App/pull/49516 by removing the hasCompletedGuidedSetupFlowSelector call. Ideally we should still revert https://github.com/Expensify/App/pull/49263 to prevent bugs in other places