[HOLD #31097 ] Chat - User taken to concierge chat page when start chat with same user from 2nd time onwards

[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.3.98-1
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: Applause - Internal Team
Slack conversation:

Action Performed:

1. Navigate to staging.new.expensify.com
2. Tap '+' icon >> Start chat option
3. Enter email address & tap on the result displayed (Note: User taken to correct chat history page)
4. Tap back button to go back to LHN
5. Tap '+' icon >> Start chat option
6. Enter same email address used in step 3 & tap on the result displayed

Expected Result:

User should be taken to correct user's conversation history page

Actual Result:

User taken to concierge chat page when start chat with same user from 2nd time onwards
Note : Issue happens when start chat with a new non-existing account & users with previous chat history (2 different videos attached)

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

Bug6271854_1699652358400.Issuewithnewnonexistingaccount.mp4

Bug6271854_1699652656426.Issuewithexistingchathistory.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01ec45a3e6b8c6cadc
• Upwork Job ID: 1723101218860847104
• Last Price Increase: 2023-11-10
• Automatic offers:
• s77rt | Contributor | 28105216

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[melvin-bot]

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

[mallenexpensify]

Was able to reproduce, bizarre

[Tony-MK]

While solving this bug, I checked for duplicates and found #30062. I submitted a proposal on the other issue because I tested my prospal on it and it worked. Can please someone check if it is a duplicate?

[Tony-MK]

Proposal

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

Secondary login chat after redirecting to primary login -redirects user to Concierge chat

What is the root cause of that problem?

The ReportScreen has to re-render a minimum of two times because of firstRenderRef. Read the two snippets below from the NavigationRoot.js and ReportScreen.js files to further illustrate the effects of the width of the screen.

App/src/libs/Navigation/NavigationRoot.js

Lines 62 to 74 in fcee207

	useEffect(() => {
	if (firstRenderRef.current) {
	// we don't want to make the report back button go back to LHN if the user
	// started on the small screen so we don't set it on the first render
	// making it only work on consecutive changes of the screen size
	firstRenderRef.current = false;
	return;
	}
	if (!isSmallScreenWidth) {
	return;
	}
	Navigation.setShouldPopAllStateOnUP();
	}, [isSmallScreenWidth]);

App/src/pages/home/ReportScreen.js

Lines 301 to 305 in fcee207

	// We don't want this effect to run on the first render.
	if (firstRenderRef.current) {
	firstRenderRef.current = false;
	return;
	}

Devices with large screen widths re-render one additional time, which allows report.reportID to become present, thus being able to produce the expected result. Here is a snippet to explain the possibilities of report.reportID being unavailable.

App/src/pages/home/ReportScreen.js

Lines 238 to 241 in fcee207

	// It possible that we may not have the report object yet in Onyx yet e.g. we navigated to a URL for an accessible report that
	// is not stored locally yet. If report.reportID exists, then the report has been stored locally and nothing more needs to be done.
	// If it doesn't exist, then we fetch the report from the API.
	if (report.reportID && report.reportID === getReportID(route) && !isLoadingInitialReportActions) {

Finally, the root cause of the problem is that, in this specific case, devices with small screen widths only re-render twice because report.reportID is not present. This will make the logic below navigate the Concierge chat because, in this situation, prevReport.parentReportID doesn't have a value.

App/src/pages/home/ReportScreen.js

Lines 307 to 331 in fcee207

	const onyxReportID = report.reportID;
	const prevOnyxReportID = prevReport.reportID;
	const routeReportID = getReportID(route);
	// Navigate to the Concierge chat if the room was removed from another device (e.g. user leaving a room or removed from a room)
	if (
	// non-optimistic case
	(!prevUserLeavingStatus && userLeavingStatus) ||
	// optimistic case
	(prevOnyxReportID &&
	prevOnyxReportID === routeReportID &&
	!onyxReportID &&
	prevReport.statusNum === CONST.REPORT.STATUS.OPEN &&
	(report.statusNum === CONST.REPORT.STATUS.CLOSED || (!report.statusNum && !prevReport.parentReportID)))
	) {
	Navigation.dismissModal();
	if (Navigation.getTopmostReportId() === prevOnyxReportID) {
	Navigation.setShouldPopAllStateOnUP();
	Navigation.goBack(ROUTES.HOME, false, true);
	}
	if (prevReport.parentReportID) {
	Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(prevReport.parentReportID));
	return;
	}
	Report.navigateToConciergeChat();

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

To produce the expected result accurately, we should try to re-render the page for small devices so they mimic the control flow of the larger devices. Therefore, adding an additional condition in this useEffect hook to force the function to run one more time for small devices should solve the problem. This will ensure that report.reportID is present to allow the fragile logic in the hook to perform as intended without changing it.

[tienifr]

Proposal

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

User taken to concierge chat page when start chat with same user from 2nd time onwards

What is the root cause of that problem?

The first time the user creates a new chat with another user, it will succeed normally.

The second time, it will return failure with a preexistingReportID, since the report was already created between the 2 users and preexistingReportID will be for navigating to the correct report.

We have that navigation logic here. The problem is that, in here, we remove the duplicate optimistic report before the redirection completes. At the time, the report screen with the duplicate optimistic report is still mounted and this logic kicks in, causing the navigation to Concierge, because it basically looks the same as the report was removed from another device.

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

We need to make sure we finish navigating to the correct report first before removing the duplicate optimistic report. We need to move this line to after this line, and run it inside InteractionManager.runAfterInteractions so that it will delete the report from Onyx after the navigation completes.

InteractionManager.runAfterInteractions(() => {
    Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, null);
});

What alternative solutions did you explore? (Optional)

Use a more sophisticated way to listen to the navigation completes, for example something similar to the useWaitForNavigation hook, or a timeout. But I find InteractionManager.runAfterInteractions to be the simplest and it works well.

[getusha]

@robertKozik I think we should hold this for #31097 since we're likely to change some parts that relate to this issue.

[robertKozik]

Thanks for heads up @getusha. @mallenexpensify can we put this issue on hold until #31097 would be merged, per @getusha comment

[mallenexpensify]

Thanks @getusha , put on hold pending the below, removed Help Wanted label too

• Feat: drop user to last chat after leaving a room #31097

[tienifr]

@mallenexpensify shouldn't we keep this open while it's on hold?

[mallenexpensify]

Yup! Thanks @tienifr , not sure how that happened (it's wayyy more likely I forget to close an issue vs prematurely doing so)

[melvin-bot]

@mallenexpensify, @robertKozik Huh... This is 4 days overdue. Who can take care of this?

[mallenexpensify]

The issue this is held on is held on another issue, so it might be a bit, bumping to Weekly
#30868

[mallenexpensify]

Still double held!

[mallenexpensify]

Making a monthly cuz it's double held, this is the one that needs fixing first

• Fix/30868: Incorrect group displayed #31424
  then
• Feat: drop user to last chat after leaving a room #31097

[s77rt]

Will take this as C+
@mallenexpensify Please assign me here

[melvin-bot]

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

[mallenexpensify]

Done @s77rt

[s77rt]

The linked PRs are merged. The issue is no longer reproducible. We can close this cc @mallenexpensify

[mallenexpensify]

Thanks @s77rt !