[HOLD - for payment on July 14] Chat - Loading spinner appears after log in with different account

[kavimuru]

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

---

Action Performed:

1. Lunch the app
2. Log in with account A
3. Send some messages and attachments
4. Log out from Account A
5. Log in with Account B

Expected Result:

Able to log in with Account B

Actual Result:

Loading spinner displayed when trying to log in with another account

Workaround:

Unknown

Platform:

Where is this issue occurring?

Web ✔️
iOS
Android ✔️
Desktop App
Mobile Web

Version Number:
1.0.75-0
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Bug5133999_Screen_Recording_20210630-092336_Expensifycash.mp4

Expensify/Expensify Issue URL:

View all open jobs on Upwork

[MelvinBot]

Triggered auto assignment to @Beamanator (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[isagoico]

This is not reproducible every time. I was able to reproduce 2 out of 5 times on my device.

[Beamanator]

Hmm when I follow the steps in the OP, I even get a blank white screen in web (staging), with this error message in the console:

[Beamanator]

Further investigation shows that the report draft comment becomes null in Onyx, and lodashGet only uses the default param ('') when the key is undefined.

The below console logs came from this code that I added:

console.log(draftComments, `${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${report.reportID}`)

[Beamanator]

Updated title & OP to show this issue isn't specific to the Android platform

[Beamanator]

@isagoico I can reproduce this issue even in production web & it seems to be something that can be fixed externally so I'm going to make this external

[MelvinBot]

Triggered auto assignment to @SofiedeVreese (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[rdjuric]

Proposal

We can modify our saveReportComment to avoid merging null draftComments to Onyx by checking if (comment === null) before doing the merge.

[SofiedeVreese]

Posted on Upwork: https://www.upwork.com/jobs/~010baaee07448e0abb

[MelvinBot]

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

[dklymenk]

Proposal

Use optional chaining operator instead of trying to access length property directly in these 2 occurrences of lodashGet into .length:

--- a/src/libs/OptionsListUtils.js
+++ b/src/libs/OptionsListUtils.js
@@ -171,7 +171,7 @@ function createOption(personalDetailList, report, draftComments, {
     const personalDetail = personalDetailList[0];
     const hasDraftComment = report
         && draftComments
-        && lodashGet(draftComments, `${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${report.reportID}`, '').length > 0;
+        && lodashGet(draftComments, `${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${report.reportID}`, '')?.length > 0;

     const hasOutstandingIOU = lodashGet(report, 'hasOutstandingIOU', false);
     const lastActorDetails = report ? _.find(personalDetailList, {login: report.lastActorEmail}) : null;
@@ -299,7 +299,7 @@ function getOptions(reports, personalDetails, draftComments, activeReportID, {

         const hasDraftComment = report
             && draftComments
-            && lodashGet(draftComments, `${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${report.reportID}`, '').length > 0;
+            && lodashGet(draftComments, `${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${report.reportID}`, '')?.length > 0;

         const shouldFilterReportIfEmpty = !showReportsWithNoComments && report.lastMessageTimestamp === 0;
         const shouldFilterReportIfRead = hideReadReports && report.unreadActionCount === 0;

Reasoning

1. The fix must handle the case with an already corrupted local storage to avoid asking users to clear their storage for the site. nvm, the issue is fixed by reloading the page, so this argument makes no sense because local storage doesn't get permanently corrupted
2. In my testing adding optional chaining operator on line 302 wasn't enough to resolve the issue because the exact same error is happening on line 174. Thus, the fix should be applied to both.

[personal-xpert]

Use useState to manipulate de spinner, so it will reset once you go back

[aliabbasmalik8]

PROPOSAL:

Need to add a validation check like this.

    const hasDraftComment = report
        && draftComments
        && lodashGet(draftComments, `${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${report.reportID}`, '')
        && lodashGet(draftComments, `${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${report.reportID}`, '').length > 0;

https://github.com/Expensify/Expensify.cash/blob/870c3db1675bb294c8961a8db88bc1ffa276dea9/src/libs/OptionsListUtils.js#L172-L174

https://github.com/Expensify/Expensify.cash/blob/870c3db1675bb294c8961a8db88bc1ffa276dea9/src/libs/OptionsListUtils.js#L300-L302

Thanks

[parasharrajat]

This seems like a regression from my PR. Fix will be applied shortly. #3793

[parasharrajat]

@Beamanator While fixing the regression does not solve this problem as I was already facing the same problem before I pushed the PR. But this time I found the root cause of it. This is little involved than it looks.

Explanation

1. We have a network Queue that process all the request for us. And out of all the requests, there are a few that don't require auth token but others need Auth token.

2. Usually, the authentication request is the one that does not need auth token.

3. When user login to the chat who has a long list of unread chat or let's say few IOU reports which is not yet synced. After login, we call APIs to sync this history. But if we immediately log out that user, there is a case where a few requests will continue to send and thus our queue will detect that and pause the Queue processing as the user is not logined.

4. When we enter an email to the login form, we send a request to get the Account status to get some important information. Due to the queue is currently paused, this request will start looping and will never complete.

5. As a result, the loader is shown on the button.

Fix

1. WE have to force send the request to Get_AccountStatus even if the queue is paused. for this, we just need to append a param in data forceNetworkRequest: true at https://github.com/Expensify/Expensify.cash/blob/bfc14ceddf75fa5eb024fdc2ce24430cd37c5d49/src/libs/actions/Session.js#L108.

2. I tried to find all references to this request to make sure that we do not break the app. So as result, I found that this request is only used at the time of login when we have to get the account info. So this makes a good point to bypass the token check as we will never have the token on the Login page and this request does not need the token.

This Fixes the original issue.

Video explaining the issue in nutshell.

loop-login.mp4

[chiragsalian]

Yeah, your proposal LGTM @parasharrajat. Feel free to push out a PR and suggest the same proposal in upwork so that you can be paid for your work 👍

[SofiedeVreese]

Ok I accepted your proposal in Upwork @parasharrajat !

[isagoico]

Issue reproducible during KI retests.

[SofiedeVreese]

Payment scheduled for July 14.

[SofiedeVreese]

@parasharrajat has been paid, the contract has been completed, and the Upwork post has been closed.

Closing issue.