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.47k stars 2.82k forks source link

[$250] Hybrid app - "Welcome to Expensify" modal when signing into the existing account on a new device. #49611

Open m-natarajan opened 3 weeks ago

m-natarajan commented 3 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: 9.0.39-0 Reproducible in staging?: Y Reproducible in production?: If this was caught during regression testing, add the test name, ID and link from TestRail: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: @puneetlath Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1726864963819969

Action Performed:

  1. Go to staging.new.expensify.com and create an account (Any other platform)
  2. Signin with the same credentials on Hybrid app
  3. Tap Try New Expensify from menu

    Expected Result:

    No welcome modal as user already signed in another platform

    Actual Result:

    "Welcome to Expensify" modal when signing in

    Workaround:

    Unknown

    Platforms:

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

    • [ ] Android: Native
    • [ ] Android: mWeb Chrome
    • [x] iOS: Native
    • [ ] iOS: mWeb Safari
    • [ ] MacOS: Chrome / Safari
    • [ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021838490385865012073
  • Upwork Job ID: 1838490385865012073
  • Last Price Increase: 2024-10-08
Issue OwnerCurrent Issue Owner: @bfitzexpensify
melvin-bot[bot] commented 3 weeks ago

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

bfitzexpensify commented 2 weeks ago

Agree that the Welcome to Expensify modal shouldn't appear in this scenario. Adding the external label.

melvin-bot[bot] commented 2 weeks ago

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

melvin-bot[bot] commented 2 weeks ago

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

klajdipaja commented 2 weeks ago

Proposal

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

The user has already logged into the New Expensify on a web browser but when he logs into the hybrid app he is shown the welcome to new Expensify modal even though he has already been using it in the web.

What is the root cause of that problem?

We have not taken into account that the user might login first through the web and then in the app. The functions that check if the user should see the welcome modal use the onyx key nvp_tryNewDot.classicRedirect.completedHybridAppOnboarding which is only set when the user logs on the native app on the first time.

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

We can use the nvp_onboarding.hasCompletedGuidedSetupFlow flag from onyx to detect that the user has already used the NewDot. This flag when an account is used in the web for the first time is set to to true but on the native app is set to true only once the guided welcome flow has completed. If we add this into the isFirstTimeHybridAppUser function in https://github.com/Expensify/App/blob/main/src/libs/actions/Welcome/index.ts we can make sure that the welcome message does not show. The first check in that function should be the following:

   const hasCompletedGuidedSetupFlow = !(Array.isArray(onboarding) || onboarding?.hasCompletedGuidedSetupFlow === undefined) && onboarding?.hasCompletedGuidedSetupFlow
        if (hasCompletedGuidedSetupFlow) {
            return;
        }

We can also put this block before isFirstTimeHybridAppUser is called:

function handleHybridAppOnboarding() {
    if (!NativeModules.HybridAppModule) {
        return;
    }
       const hasCompletedGuidedSetupFlow = !(Array.isArray(onboarding) || onboarding?.hasCompletedGuidedSetupFlow === undefined) && onboarding?.hasCompletedGuidedSetupFlow
        if (hasCompletedGuidedSetupFlow) {
            return;
        }

What alternative solutions did you explore? (Optional)

melvin-bot[bot] commented 2 weeks ago

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

melvin-bot[bot] commented 1 week ago

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

melvin-bot[bot] commented 1 week ago

@bfitzexpensify, @alitoshmatov Still overdue 6 days?! Let's take care of this!

bfitzexpensify commented 1 week ago

Proposal ready for review in https://github.com/Expensify/App/issues/49611#issuecomment-2370707723 @alitoshmatov

melvin-bot[bot] commented 1 week ago

@bfitzexpensify, @alitoshmatov Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!

alitoshmatov commented 1 week ago

I couldn't reproduce it in our existing app. As far as I understand this is only happening in hybrid app(which I don't have much info about). But based on this C+ should get access soon.

@bfitzexpensify I am not sure what is the next step. If it is not an urgent issue I think we can hold till C+ get access and we can continue then

melvin-bot[bot] commented 1 week ago

@bfitzexpensify @alitoshmatov 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!

bfitzexpensify commented 6 days ago

If it is not an urgent issue I think we can hold till C+ get access and we can continue then

Agreed. Setting to weekly and starring myself to keep checking in on this. We can revisit once C+ have access

melvin-bot[bot] commented 6 days ago

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