[Hold for Payment: 2022-12-09] [$250] LHN - Opening <EXPENSIFY_URL>/concierge, system redirected to LHN

[kbecciv]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Issue was found when executing PR #12588

Action Performed:

1. Go to URL https://staging.new.expensify.com/
2. Login with any account
3. Go to any chat
4. Minimize the window
5. Open <EXPENSIFY_URL>/concierge

Expected Result:

The concierge chat should be displayed

Actual Result:

The system redirected to LNH when opening <EXPENSIFY_URL>/concierge

Workaround:

Unknown

Platform:

Where is this issue occurring?

• Web
• Mobile Web

Version Number: 1.2.28.1

Reproducible in staging?: Yes

Reproducible in production?: Yes

Email or phone of affected tester (no customers):

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

Bug5823119_Recording__2962.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

[melvin-bot]

Triggered auto assignment to @dylanexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[roryabraham]

This is a regression that's reproducible on staging and production

[melvin-bot]

Current assignee @dylanexpensify is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[melvin-bot]

Current assignee @roryabraham is eligible for the External assigner, not assigning anyone new.

[dylanexpensify]

Upwork: https://www.upwork.com/jobs/~013d413cd690798c5c

[flodnv]

This is very strange, as it worked in #12588 (which was fixing #11892). To me, this isn't as much a regression as it should've been a deploy blocker -- that PR should not have been marked as passing tests, as tests were not passing. Why was this not a deploy blocker @kbecciv ?

[flodnv]

Oh I see: #12753 (comment)
As per my above message, I disagree with what you said there @roryabraham

[flodnv]

cc @Ollyws maybe you can take a look^ ?

[Ollyws]

@flodnv Had a quick poke around and seems to be this PR which broke the concierge navigation functionality.

[melvin-bot]

Looks like something related to react-navigation may have been mentioned in this issue discussion.

As a reminder, please make sure that all proposals are not workarounds and that any and all attempt to fix the issue holistically have been made before proceeding with a solution. Proposals to change our DeprecatedCustomActions.js files should not be accepted.

Feel free to drop a note in #expensify-open-source with any questions.

[flodnv]

Thanks @Ollyws ❤️
@stitesExpensify can you take a look please?

[tlerias]

The Problem

This is happening because /concierge redirects to the concierge route with the report ID. When the redirect happens, by default the drawer is open on small screens. You can see the code that does this here.
This shows the redirect in the URL:

The Proposed Solution

I propose fixing this by adding this code to the ReportScreen Page componentDidMount

if (ReportUtils.isConciergeChatReport(this.props.report) && this.props.isSmallScreenWidth) {
            Navigation.closeDrawer();
        }

By putting it in the componentDidMount it will not do this check on every rerender, forcing the LHN Drawer to be closed all the time. It will only run this code when the page has been mounted.

Looking forward to your thoughts on this solution.

[bernhardoj]

Proposal

Problem
Actually, this issue also happens on native when we open the deep link (<EXPENSIFY_URL>/concierge) from another app while the Expensify app is closed. It will throw a development-only error below.

ReportScreen should be the one that handles the routes, but the navigator could not find it as it is not available. After doing some investigation, it turns out that when we are trying to navigate to concierge chat on launch, MainDrawerNavigator (which contains ReportScreen) is not ready/mounted yet. Then why the old code can handle it?

It is already mentioned the PR that caused the issue here specifically at this line. The new code changes call Report.navigateToConciergeChat() which will open concierge chat immediately, compared to the old code that calls Report.fetchOrCreateChatReport() which will do an api request before doing the navigation, so there is some delay for the drawer navigator to be ready.

Solution
We can wait for the drawer navigator to be ready first, then do the navigation action.

const ConciergePage = (props) => {
    if (_.has(props.session, 'authToken')) {
-       Report.navigateToConciergeChat();
+       Navigation.isDrawerReady().then(() => {
+           Report.navigateToConciergeChat();
+       });
    } else {
        Navigation.navigate();
    }

    return <FullScreenLoadingIndicator />;
};

Result

327800.t.mp4

Screen.Recording.2022-11-19.at.02.44.16.mov

[tlerias]

@bernhardoj This feels like a different bug to me. You should report it so you get reporting $ too 😉.

(i deleted my last comment because I meant to take back the second part of my comment that mentioned something about a useEffect but that was wrong).

[melvin-bot]

@rushatgabhane, @roryabraham, @dylanexpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[rushatgabhane]

all proposals feel like a hack. still investigating

[s77rt]

Proposal

diff --git a/src/libs/actions/App.js b/src/libs/actions/App.js
index 1faeae4bb..97cc46f16 100644
--- a/src/libs/actions/App.js
+++ b/src/libs/actions/App.js
@@ -115,6 +115,7 @@ function openApp() {
     const connectionID = Onyx.connect({
         key: ONYXKEYS.COLLECTION.POLICY,
         waitForCollectionCallback: true,
+        initWithStoredValues: false,
         callback: (policies) => {
             Onyx.disconnect(connectionID);
             API.read('OpenApp', {policyIDList: getNonOptimisticPolicyIDs(policies)}, {
diff --git a/src/pages/ConciergePage.js b/src/pages/ConciergePage.js
index 1793f651d..fc8fc8712 100644
--- a/src/pages/ConciergePage.js
+++ b/src/pages/ConciergePage.js
@@ -24,10 +24,12 @@ const propTypes = {
  *     - Else re-route to the login page
  */
 const ConciergePage = (props) => {
-    if (_.has(props.session, 'authToken')) {
-        Report.navigateToConciergeChat();
-    } else {
-        Navigation.navigate();
+    if (props.isLoadingReportData === false) {
+        if (_.has(props.session, 'authToken')) {
+            Report.navigateToConciergeChat();
+        } else {
+            Navigation.navigate();
+        }
     }
 
     return <FullScreenLoadingIndicator />;
@@ -40,4 +42,7 @@ export default withOnyx({
     session: {
         key: ONYXKEYS.SESSION,
     },
+    isLoadingReportData: {
+        key: ONYXKEYS.IS_LOADING_REPORT_DATA,
+    },
 })(ConciergePage);

Details

Before explaining my proposal, I will explain why it was working. It was working because we were always making a call to the API CreateChatReport and before that we call OpenApp. The navigation to the concierge chat was done in the callback of CreateChatReport and since API calls are sequential we were always navigation to the concierge chat after OpenApp is done. Now with the recent changes we are no longer calling CreateChatReport and we navigate immediately even before the OpenApp call is done.

With that being said, the fix is rather simple by just checking the value of isLoadingReportData if it's false (OpenApp is done with success) then we can proceed with the navigation.

I have also set initWithStoredValues to false so we don't get the value initially set to false (the result from the last session).

PS: I won't say this is a regression, it's more like a hidden bug

[rushatgabhane]

Now with the recent changes we are no longer calling CreateChatReport and we navigate immediately even before the OpenApp call is done

@s77rt That's a great analysis. Could you please add links to the PR / code so that everyone has visibility?

initWithStoredValues: false

Won't that slow down the launch time of the app? Because all the data will be loaded using a network call vs. loading it from device storage.

[bernhardoj]

Applied. I will open the PR today or tomorrow.

[bernhardoj]

I have opened the PR.

There is 1 checklist left, which is to test on Desktop. Do we have any way to open a deep link on desktop?

@rushatgabhane @stitesExpensify

[Ollyws]

@bernhardoj You can open the console and enter this.document.location = "http://localhost:8080/concierge" like I did in this PR

[bernhardoj]

It works. Thanks a lot! @Ollyws

Updated my PR.

[dylanexpensify]

@rushatgabhane where are we with reviewing the PR? Remember, within 3 days = 50% bonus!

[stitesExpensify]

The PR is already merged 😄 We're just waiting for the deploy now

[dylanexpensify]

ah hahah nice! Best answer

[rushatgabhane]

the bonus really helps with urgency! kudos to whoever's idea it was

[melvin-bot]

@rushatgabhane, @stitesExpensify, @bernhardoj, @dylanexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[dylanexpensify]

Not overdue - should this have populated the payment date header @stitesExpensify?

[stitesExpensify]

Yes, it looks like this was deployed to prod 3 days ago, so payment should be Dec. 9

[dylanexpensify]

payment tomorrow!

[dylanexpensify]

@rushatgabhane please apply! 🙇‍♂️

[dylanexpensify]

@bernhardoj offer sent!

[bernhardoj]

@dylanexpensify I see the offer amount hasn't include the bonus. Should I accept the offer first? Sorry, this is my first time receiving an offer with bonus 😅

[dylanexpensify]

no worries @bernhardoj! accept the offer and I'll include the bonus when paying out! 😄

[bernhardoj]

Accepted. Thanks for the answer! @dylanexpensify

[dylanexpensify]

@bernhardoj payment (w/bonus) sent!
@rushatgabhane sent offer!

[rushatgabhane]

@dylanexpensify accepted, thanks!

[dylanexpensify]

paid out