[HOLD for payment 2024-07-10] [$250] Concierge - User is not landed on Concierge chat via deeplink

[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.83-0
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4630882&group_by=cases:section_id&group_order=asc&group_id=229066
Email or phone of affected tester (no customers): gocemate+a2533@gmail.com
Issue reported by: Applause - Internal Team

Action Performed:

1. Log out of NewDot
2. Navigate to https://staging.new.expensify.com/concierge
3. Sign-up with a new account

Expected Result:

User should navigate to the concierge chat

Actual Result:

User is not landed on Concierge chat via deeplink

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

Bug6512346_1718306485456.Recording__3213.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01d4c5cf8f620e9130
• Upwork Job ID: 1801355071704746276
• Last Price Increase: 2024-06-13
• Automatic offers:
• allgandalf | Reviewer | 102733790
• bernhardoj | Contributor | 102797658

Issue Owner

Current Issue Owner: @slafortune

[melvin-bot]

Triggered auto assignment to @AndrewGable (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[lanitochka17]

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

[lanitochka17]

We think that this bug might be related to #vip-vsp

[AndrewGable]

I wonder if the new onboarding broke this, looking

[AndrewGable]

Can't reproduce on web

[AndrewGable]

Reproduced on staging Android

screen-20240613-142635.mp4

[AndrewGable]

This happens on Android production for me too (using version 1.4.82-4)

screen-20240613-143449.mp4

Going to remove the deploy blocker label, but keep it as a bug

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[rvenky125]

I want to work on this

[bernhardoj]

Proposal

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

Concierge from deeplink doesn't open when sign up as a new user.

What is the root cause of that problem?

When we sign up, an onboarding page shows up. After we complete the onboarding, close the onboarding page and we navigate to HOME (LHN).

App/src/pages/OnboardingPersonalDetails/BaseOnboardingPersonalDetails.tsx

Line 69 in 43cd0f7

Navigation.dismissModal();

App/src/pages/OnboardingPersonalDetails/BaseOnboardingPersonalDetails.tsx

Lines 71 to 74 in 43cd0f7

	// Only navigate to concierge chat when central pane is visible
	// Otherwise stay on the chats screen.
	if (isSmallScreenWidth) {
	Navigation.navigate(ROUTES.HOME);

So the concierge page is actually in the nav stack,
[Home, Concierge, Onboarding]

but because we navigate to home, the LHN is shown instead of the concierge. I believe this happens to every deep link page.

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

If the expected is to show the deeplinked page after the onboarding, then we shouldn't navigate the user to home after completing the onboarding, but instead, just do nothing.

App/src/pages/OnboardingPersonalDetails/BaseOnboardingPersonalDetails.tsx

Lines 73 to 79 in 43cd0f7

	if (isSmallScreenWidth) {
	Navigation.navigate(ROUTES.HOME);
	} else if (AccountUtils.isAccountIDOddNumber(accountID ?? 0)) {
	Report.navigateToSystemChat();
	} else {
	Report.navigateToConciergeChat();
	}

if (!isSmallScreenWidth) {
    if (AccountUtils.isAccountIDOddNumber(accountID ?? 0)) {
        Report.navigateToSystemChat();
    } else {
        Report.navigateToConciergeChat();
    }
}

This way, if we have this nav stack,
[Home, Concierge, Onboarding]

and we complete the onboarding, we arrive at the concierge page.
[Home, Concierge]

[allgandalf]

haha, no worries! We all have odd days 😆

[allgandalf]

@slafortune This bug only occurs on small screens, I can see that this was done intentionally, I have asked the original author of the PR whether this is intentional or an actual bug

[allgandalf]

@slafortune , the author said that the current behaviour is as per the design doc, do we need to change that then?

IMO, yes, the user should be taken to the concierge after the flow is completed

[bernhardoj]

I think this one is different because the user uses a deep link to /concierge.

[allgandalf]

I think this one is different because the user uses a deep link to /concierge.

I agree if we deeplink to a page, then we should be able to get to that page itself.

@bernhardoj about your proposal:

With your suggested solution:

if (!isSmallScreenWidth) {
if (AccountUtils.isAccountIDOddNumber(accountID ?? 0)) {
Report.navigateToSystemChat();
} else {
Report.navigateToConciergeChat();
}
}

What about the case when we are on small screen when the onboarding process has been completed? From the code it seems that we will never navigate the user for small screens

[bernhardoj]

I mentioned it in the proposal that we already close the onboarding page when completing the process. So, no need to navigate anywhere after closing the onboarding page for a small screen.

[allgandalf]

Ahh that's indeed right,

I believe this happens to every deep link page.

You're right over here, if we deeplink to profiles page, the same bug occurs. This has to do with the way we navigate after the onboarding has been completed, we do not differentiate deeplinks here.

@bernhardoj's proposal looks good to me.

Note

Their RCA is correct (This is an edge case of the onboarding flow) and their solution would work fine and makes sense to me.

🎀👀🎀 C+ reviewed

[melvin-bot]

Current assignee @AndrewGable is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[melvin-bot]

📣 @bernhardoj 🎉 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 📖

[bernhardoj]

PR is ready

cc: @allgandalf

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.3-7 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix user is not landed on Concierge chat via deeplink after completing onboarding in small screen #44063

If no regressions arise, payment will be issued on 2024-07-10. 🎊

For reference, here are some details about the assignees on this issue:

• @bernhardoj requires payment automatic offer (Contributor)
• @allgandalf requires payment automatic offer (Reviewer)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@allgandalf] The PR that introduced the bug has been identified. Link to the PR:
• [@allgandalf] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@allgandalf] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@allgandalf] Determine if we should create a regression test for this bug.
• [@allgandalf] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@slafortune] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[slafortune]

@allgandalf can you please complete the checklist?

[allgandalf]

• The PR that introduced the bug has been identified. Link to the PR: Tasks for guided setup #39951

• The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: This was actually by design, we had that discussion before the PR implementation https://github.com/Expensify/App/pull/39951/files#r1642259155

• A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A

• If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Proposal

1. Sign out
2. Deep link to /concierge (or any other page)
3. Sign in as a new user
4. Complete the onboarding process
5. In small screen, verify the deep linked page, in this case concierge, is shown (if deep linked to /settings, then settings page should be shown)

Do we agree 👍 or 👎

[bernhardoj]

Requested payment in ND.

[slafortune]

@allgandalf Role C+ - Owed $250 via Upworks - Paid ✅
@bernhardoj, ended contract via Upworks, eligible for NewDot payment as of 5/5. Role C - owed $250 via NewDot

[JmillsExpensify]

$250 approved for @bernhardoj