[HOLD for payment 2024-04-25] [$500] Android - Chat - Starting chat with specific number, user directed to hmm it's not here page

[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.56
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4447645
Issue reported by: Applause - Internal Team

Action Performed:

1. Launch app
2. Tap fab --- start chat
3. Enter +79585702746 and select the contact

Expected Result:

Starting chat with specific number, user must be directed to specific contact page

Actual Result:

Starting chat with specific number, user directed to hmm it's not here page

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

Bug6423987_1711162774974.err.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~014f0c32c8537d50a1
• Upwork Job ID: 1772777450410250240
• Last Price Increase: 2024-04-03
• Automatic offers:
• jayeshmangwani | Reviewer | 0

[melvin-bot]

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

[lanitochka17]

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

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~014f0c32c8537d50a1

[melvin-bot]

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

[suneox]

Proposal

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

Android - Chat - Starting chat with specific number, user directed to hmm it's not here page

What is the root cause of that problem?

When start chat with new phone number violate rule backend return error There was a problem signing up with this phone number. Please contact Concierge for assistance.

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

We should show an error message to the user when they try to start a chat with a new phone number that violates the rule.
when navigating to report screen while report is opening from search page and define an error rule do not trigger fetch report then we shouldn't trigger fetch report
update this code at this line to

    const fetchReport = useCallback(() => {
        if (!reportMetadata?.isLoadingInitialReportActions && !hasError) {
            Report.openReport(reportIDFromRoute, reportActionIDFromRoute);
        }
    }, [reportIDFromRoute, reportActionIDFromRoute, reportMetadata?.isLoadingInitialReportActions]);

and display an error message from BE

POC

What alternative solutions did you explore? (Optional)

[nkdengineer]

Proposal

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

Starting chat with specific number, user sees infinite loading skeletons

What is the root cause of that problem?

• Currently when starting a chat with a new email/phone number, we're calling openReport a few times due to the many effects in ReportScreen. The first openReport will return an error There was a problem signing up with this phone number in createChat, this is because the phone number is likely to be related to fraud, so it's rejected by our back-end. However, subsequent calls to openReport will return null as the report data, leading to the user seeing infinite loading skeleton. The expectation here should be that the user will see the report opened, but with an error above the Composer.

• When the openReport returns with an error, we're clearing the optimistic personal details of the new contacts (in here). This leads to the title here being empty and there's infinite loading in the report header

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

• Here, add a check for && !isLoadingInitialReportActions (the isLoadingInitialReportActions boolean params need to be added), this will make sure that if an openReport is in progress, another openReport will not be triggered (since it's duplicate). Then we have to pass reportMetadata?.isLoadingInitialReportActions in the places that shouldFetchReport is used, like here and here. Alternatively, we can add the shouldFetchReport to openReport itself here so that it will apply to all usage of openReport, wherever it's called.
• Here, we should not reset the optimistic personal details to null in failureData of openReport, because we still need those personal details in order to display the UI properly in failure case.

We can either remove the above block, or define a new failurePersonalDetails (that does not reset optimistic personal details to null) and use it for the failureData instead of using settledPersonalDetails.

if (allPersonalDetails?.[accountID]) {
    failurePersonalDetails[accountID] = allPersonalDetails?.[accountID];
}

Then when the user dismiss the error on the failed-to-be-created report (image attached in Result), we should clear the optimistic personal details associated with that report (the participants)

What alternative solutions did you explore? (Optional)

None

Result

[jayeshmangwani]

@suneox In Android and iOS , If I start chat with the +79585702746 , i am not able to see not here page instead I can see loading skeletons only, I have tried your proposal , and adding reportMetadata?.isLoadingInitialReportActions to fetchReport useCallback changed nothing issue still reproducing

[jayeshmangwani]

@nkdengineer Thanks for the proposal, your proposal makes sense, I have few questions :

1. In web its working fine and issue is only in mWeb and native devices, so whats the RCA that issue in only in mobile devices ?
2. It looks like that shouldFetchReport being called one more extra time in mobile devices then web, that causing this issue, so rather then adding the isLoadingInitialReportActions to shouldFetchReport function, can we prevent the shouldFetchReport function's extra rendering in mobile devices ?

[nkdengineer]

In web its working fine and issue is only in mWeb and native devices, so whats the RCA that issue in only in mobile devices ?

@jayeshmangwani I can reproduce this issue on desktop web too, what behavior do you observe there?

[jayeshmangwani]

invalid-chat.mov

@nkdengineer This is the behaviour I am getting in the latest code, let me know if you have different outcome

[suneox]

@suneox In Android and iOS , If I start chat with the +79585702746 , i am not able to see not here page instead I can see loading skeletons only, I have tried your proposal , and adding reportMetadata?.isLoadingInitialReportActions to fetchReport useCallback changed nothing issue still reproducing

This issue happens on all devices based on your config language and country you can try another phone number on diffirent country exam +8498730055 or +6677665544
tested branch

@nkdengineer Thanks for the proposal, your proposal makes sense, I have few questions :

Hi @jayeshmangwani
For this proposal the RCA and solution the same with my proposal, it also prevent openReport by check isLoadingInitialReportActions condition, a diffirent is only put condition place and the function fetchReport also call at function navigateToEndOfReport. My proposal also cover this issue and the condition place is also update by code review if we don't need to cover case navigateToEndOfReport

[suneox]

I just checked on the latest main this issue doesn't happen

[jayeshmangwani]

@jayeshmangwani I can reproduce this issue on desktop web too, what behavior do you observe there?

@nkdengineer Would you upload a video of your desktop web outcome ?

[jayeshmangwani]

I just checked on the latest main this issue doesn't happen

On which platforms ?

[nkdengineer]

@nkdengineer Would you upload a video of your desktop web outcome ?

@jayeshmangwani Here it is:

Screen.Recording.2024-03-28.at.11.01.38.PM.mp4

I think the reason why it's sometimes reproducible, sometimes not is because when network is fast, the openReport returns with createChat error before the next openReport runs, and we already have the check to avoid openReport again if there's createChat error, inside shouldFetchReport

[nkdengineer]

For #38879 (comment) RCA and solution the same with my proposal

@suneox To be fair, your proposal at most only fixes 1/2 of the issue. The chat header is skeleton loading indefinitely in your own attached POC screenshot.

cc @jayeshmangwani

[jayeshmangwani]

@lanitochka17 Can we please test this issue again again , does QA team is still able to reproduce this issue ?

[nkdengineer]

@lanitochka17 Can we please test this issue again again , does QA team is still able to reproduce this issue ?

@jayeshmangwani Are you not able to reproduce the issue now?

Please note there's the infinite loading chat header as well, that's definitely still reproducible in all platforms.

[jayeshmangwani]

No I am not able to reproduce the mentioned issue @nkdengineer

[melvin-bot]

📣 @nkdengineer You have been assigned to this job!
Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

[nkdengineer]

@jayeshmangwani The PR is here.

[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 1.4.62-17 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix header displays skeleton when creating report with invalid user #39585

If no regressions arise, payment will be issued on 2024-04-25. 🎊

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

• @jayeshmangwani requires payment automatic offer (Reviewer)
• @nkdengineer requires payment (Needs manual offer from BZ)

[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:

• [@jayeshmangwani] The PR that introduced the bug has been identified. Link to the PR:
• [@jayeshmangwani] 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:
• [@jayeshmangwani] 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:
• [@jayeshmangwani] Determine if we should create a regression test for this bug.
• [@jayeshmangwani] 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.
• [@bfitzexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[bfitzexpensify]

Switching this to daily to action on Thursday.

@nkdengineer I sent an offer to you via Upwork.

@jayeshmangwani please complete the BZ checklist - thank you!

[jayeshmangwani]

@bfitzexpensify I will complete the checklist tomorrow

[jayeshmangwani]

• [@jayeshmangwani] The PR that introduced the bug has been identified. Link to the PR: Remove invalid user from contact list #22612
• [@jayeshmangwani] 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: https://github.com/Expensify/App/pull/22612/files#r1578208405
• [@jayeshmangwani] 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: The discussion is not needed here. This issue can be caught in testing when we start chatting with the invalid user.
• [@jayeshmangwani] Determine if we should create a regression test for this bug.
• [@jayeshmangwani] 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. Open App > Press FAB > Start chat
2. Enter invalid phone number, i.e +79585702746
3. After opening the report, verify that the error appears and the header doesn't display skeleton
4. Clear the error
5. Press FAB > Start chat again
6. Verify that the phone number that we entered above doesn't appears as recent contact

Do we agree 👍 or 👎

[bfitzexpensify]

Thanks @jayeshmangwani - proposed those steps be added via https://github.com/Expensify/Expensify/issues/390645.

Payment will be completed tomorrow.

@nkdengineer a reminder to accept the Upwork job offer.

[bfitzexpensify]

Payment to @jayeshmangwani complete. Bumping @nkdengineer in Slack again so we can close this out.

[nkdengineer]

@bfitzexpensify Thanks for bumping, I accepted the offer 🙏

[bfitzexpensify]

Thanks! Payments complete. Closing this out