Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[$250] mWeb on Chrome and mobile app experience differs on signup #46672

Closed
1 of 6 tasks
m-natarajan opened this issue Aug 1, 2024 · 17 comments
Closed
1 of 6 tasks

[$250] mWeb on Chrome and mobile app experience differs on signup #46672

m-natarajan opened this issue Aug 1, 2024 · 17 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@m-natarajan
Copy link

m-natarajan commented Aug 1, 2024

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.15-5
Reproducible in staging?: Y
Reproducible in production?: Y
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: @davidcardoza
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1722471696514569

Action Performed:

  1. Sign up for account
  2. UsertTaps track expenses > taps get started after watching the video
    A. On mWeb the user is dropped into LHN with a GBR to Concierge
    B. On mobile iOS, the user is dropped into the Concierge chat

Expected Result:

The user should always be dropped into LHN with a GBR to Concierge

Actual Result:

Sign ups see two different experiences

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

RPReplay_Final1722469307.MP4
RPReplay_Final1722527646.MP4

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01dc1e03761805d373
  • Upwork Job ID: 1820461993908929902
  • Last Price Increase: 2024-08-05
  • Automatic offers:
    • rojiphil | Reviewer | 103494199
Issue OwnerCurrent Issue Owner: @rojiphil
@m-natarajan m-natarajan added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Aug 1, 2024
Copy link

melvin-bot bot commented Aug 1, 2024

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

@bernhardoj
Copy link
Contributor

Proposal

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

The concierge page opens after completing onboarding in native.

What is the root cause of that problem?

In the onboarding page, we have an effect that prevent the user to access the onboarding page again by redirecting them to the concierge page. (it was added in #42087)

useEffect(() => {
if (!hasCompletedGuidedSetupFlow) {
return;
}
Navigation.isNavigationReady().then(() => {
// Need to go back to previous route and then redirect to Concierge,
// otherwise going back on Concierge will go to onboarding and then redirected to Concierge again
Navigation.goBack();
Report.navigateToConciergeChat();
});
}, [hasCompletedGuidedSetupFlow]);

When we complete the onboarding, we call the API and dismiss the modal.

Report.completeOnboarding(
onboardingPurposeSelected,
CONST.ONBOARDING_MESSAGES[onboardingPurposeSelected],
{
firstName,
lastName,
},
onboardingAdminsChatReportID ?? undefined,
onboardingPolicyID,
);
Welcome.setOnboardingAdminsChatReportID();
Welcome.setOnboardingPolicyID();
Navigation.dismissModal();

Looks like on native, the request is completed first before the navigator is completely closed, so the effect is triggered because the effect depends on hasCompletedGuidedSetupFlow.

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

Remove hasCompletedGuidedSetupFlow from the dependency so it's only triggered on mount which will still prevent the user to access the page if it's already completed.

useEffect(() => {
if (!hasCompletedGuidedSetupFlow) {
return;
}
Navigation.isNavigationReady().then(() => {
// Need to go back to previous route and then redirect to Concierge,
// otherwise going back on Concierge will go to onboarding and then redirected to Concierge again
Navigation.goBack();
Report.navigateToConciergeChat();
});
}, [hasCompletedGuidedSetupFlow]);

Copy link
Contributor

github-actions bot commented Aug 2, 2024

true

@melvin-bot melvin-bot bot added the Overdue label Aug 5, 2024
@abekkala abekkala added the External Added to denote the issue can be worked on by a contributor label Aug 5, 2024
@melvin-bot melvin-bot bot changed the title mWeb on Chrome and mobile app experience differs on signup [$250] mWeb on Chrome and mobile app experience differs on signup Aug 5, 2024
Copy link

melvin-bot bot commented Aug 5, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 5, 2024
Copy link

melvin-bot bot commented Aug 5, 2024

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

@melvin-bot melvin-bot bot removed the Overdue label Aug 5, 2024
@abekkala
Copy link
Contributor

abekkala commented Aug 5, 2024

@rojiphil there is already a proposal above ready for review! 🎉

Copy link

melvin-bot bot commented Aug 8, 2024

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

@melvin-bot melvin-bot bot added the Overdue label Aug 8, 2024
@abekkala
Copy link
Contributor

abekkala commented Aug 8, 2024

@rojiphil can you please review the proposal above?

@rojiphil
Copy link
Contributor

rojiphil commented Aug 9, 2024

Sorry for the delay here. I will review and provide an update by tomorrow my day.

@melvin-bot melvin-bot bot removed the Overdue label Aug 9, 2024
@rojiphil
Copy link
Contributor

Yeah. An unnecessary trigger of the effect due to hasCompletedGuidedSetupFlow dependency is causing this issue. Removing the hasCompletedGuidedSetupFlow dependency makes sense.
@bernhardoj proposal LGTM.
🎀👀🎀 C+ reviewed

Copy link

melvin-bot bot commented Aug 10, 2024

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

@dangrous
Copy link
Contributor

cool, makes sense! we should just make sure that this still works appropriately on desktop, as well as fixing mobile. But sounds good to me. Will assign!

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 12, 2024
Copy link

melvin-bot bot commented Aug 12, 2024

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

Offer link
Upwork job

@bernhardoj
Copy link
Contributor

@rojiphil @dangrous I can't reproduce this anymore after this PR. Looks like the issue that PR is trying to solve is the same as this one but with a different solution.

@dangrous
Copy link
Contributor

oh interesting! @abekkala do you have a moment to retest this? we might be able to just close it out

@rojiphil
Copy link
Contributor

yeah. Just tested the latest and this is fixed now. We can close this.

@dangrous
Copy link
Contributor

Nice, closing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
No open projects
Archived in project
Development

No branches or pull requests

5 participants