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.32k stars 2.75k forks source link

[$500] Onboarding - Changing the URL during onboarding takes back to the first flow #46104

Open lanitochka17 opened 1 month ago

lanitochka17 commented 1 month 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.11-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: N/A Issue reported by: Applause - Internal Team

Action Performed:

  1. Sign out of the staging app if signed in already
  2. Log in with a new Gmail address > Continue > Join
  3. When the onboarding flow opens select any option in the "What do you want to do today?" flow
  4. Click continue
  5. Enter a first name in the next flow
  6. Before clicking on continue, copy this link ( https://staging.new.expensify.com/settings/profile ) on your browser and hit enter

Expected Result:

After the page re-loads the user is taken to the current process of the onboarding flow ("What's your name?")

Actual Result:

The user is taken one step back in the onboarding flow("What do you want to do today?") and the previously filled-out name is displayed after clicking on continue

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/691d57c5-a21c-4357-8449-5a88f16fd232

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01f2e620ccc728274d
  • Upwork Job ID: 1818010709256699718
  • Last Price Increase: 2024-09-06
  • Automatic offers:
    • tsa321 | Contributor | 103498083
melvin-bot[bot] commented 1 month ago

Triggered auto assignment to @alexpensify (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 commented 1 month ago

We think that this bug might be related to #wave-collect - Release 1

lanitochka17 commented 1 month ago

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

tsa321 commented 1 month ago

Proposal

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

Onboarding reset to first page if user change URL address

What is the root cause of that problem?

When user change the url while in onboarding, in here:

https://github.com/Expensify/App/blob/a79189f2294a07a737bebbf5423f7d19ebf9d467/src/libs/Navigation/AppNavigator/createCustomBottomTabNavigator/BottomTabBar.tsx#L59-L64

https://github.com/Expensify/App/blob/a79189f2294a07a737bebbf5423f7d19ebf9d467/src/libs/actions/Welcome.ts#L56-L57

the problem occurs because the Welcome.isOnboardingFlowCompleted() function is triggered again when the URL changes, causing navigation back to the first page of onboarding. This happens because we do not store the onboarding progress in onyx and use ROUTES.ONBOARDING_ROOT on the onNotCompleted parameter of isOnboardingFlowCompleted

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

We could add onboardingDraft in onyx and store the current address in the onboardingDraft. Then the onNotComppleted will navigate to the last route stored in onBoardingDraft. So in each Welcome.isOnboardingFlowCompleted function call we need to adjust the onNotCompleted parameter to navigate to the stored route in onboardingDraft. Or if it is possible we could directly navigate do the navigation in onNotCompleted, but it seems the onNotCompleted parameter is different on each code that use it. and if during onboarding process we need to store some data, we could do that in the onyx onboardingDraft too.

alexpensify commented 1 month ago

I'll test it soon; this GH is on my testing list.

alexpensify commented 1 month ago

I've tested this one and can confirm it happens. When you get to the name part, I think nothing has been saved, so I'm wondering if we can account for the page without the save action without clicking Continue on the first and last name tab.

2024-07-29_12-41-42

melvin-bot[bot] commented 1 month ago

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

melvin-bot[bot] commented 1 month ago

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

alexpensify commented 1 month ago

Assigning External, @mollfpr please review if the proposal will fix this issue. I think we should still take the person to the Name tab since they would click continue in the first tab of options for what their account will be used for. Thanks for reviewing!

2024-07-29_12-51-11

mollfpr commented 1 month ago

@tsa321 Thanks for the proposal! I did a test but it seems the Welcome.isOnboardingCompletedFlow is not called after refresh.

Instead I found out that the route is set here. https://github.com/Expensify/App/blob/ba25cb3400ab92ea3c2b618631da8e715b2c6644/src/libs/Navigation/NavigationRoot.tsx#L97-L99

tsa321 commented 1 month ago

Thanks, @mollfpr . I’ll need to change that line to use onboard draft -> current route if the visited URL/route is not related to onboarding. I’ll also check all occurrences of ROUTES.ONBOARDING_ROOT in the codebase and adjust the code as needed. I’ll create a branch to test my proposed solution.

mollfpr commented 1 month ago

@tsa321 That will be great. Thank you!

tsa321 commented 1 month ago

@mollfpr, here is the test branch.

I also found a bug: when a user is in the onboarding flow and enters a random URL, the onboarding process shows the first page and then closes without completing the flow. The fix are are in the getAdaptedStateFromPath file.

https://github.com/user-attachments/assets/72fa87bb-7e42-404e-91c1-af43e56c045d

melvin-bot[bot] commented 1 month ago

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

alexpensify commented 1 month ago

@mollfpr, when you get a chance, can you review the latest text? Thanks!

melvin-bot[bot] commented 1 month ago

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

mollfpr commented 1 month ago

Reviewing!

mollfpr commented 1 month ago

@tsa321 Thank you for the test branch. It result great! I'm wondering if we can use ONYXKEYS.LAST_VISITED_PATH instead of creating a new one?

tsa321 commented 1 month ago

@mollfpr oh right, we can use ONYXKEYS.LAST_VISITED_PATH. This will simplify the code. I will update the test branch

tsa321 commented 1 month ago

@mollfpr I have updated my test branch to use last visited path.

melvin-bot[bot] commented 1 month ago

@alexpensify @mollfpr 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!

mollfpr commented 1 month ago

Thanks @tsa321 for the update and it looks good now!

Let's go with @tsa321 proposal then!

🎀 👀 🎀 C+ reviewed!

melvin-bot[bot] commented 1 month ago

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

chiragsalian commented 4 weeks ago

Cool, proposal LGTM. Feel free to push up a PR @tsa321

melvin-bot[bot] commented 4 weeks ago

📣 @tsa321 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

tsa321 commented 4 weeks ago

I will try to submit a PR tomorrow.

tsa321 commented 3 weeks ago

@mollfpr PR is ready

alexpensify commented 2 weeks ago

Weekly Update: PR is under review

alexpensify commented 2 weeks ago

Next Steps

This PR is going through the review process.

Heads up, I will be offline until Tuesday, September 3, 2024, and will not actively watch over this GitHub during that period.

If anything urgent is needed here, please ask for help in the #expensify-open-source Slack Room-- thanks!

alexpensify commented 1 week ago

Catching up from being OOO, I see the PR is moving along.

chiragsalian commented 5 days ago

@alexpensify, can we increase the pricing of this PR to $500. The scope of the PR changed during development (discussed here) to solve two additional bugs on main. Contributor decided to solve it in this PR.

Does that sound fair @tsa321?

melvin-bot[bot] commented 5 days ago

Upwork job price has been updated to $500

alexpensify commented 5 days ago

Thanks for this context and done!