[$500] Chat-Infinite loading occurs if conversation ID is edited to 0

[izarutskaya]

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.9-0
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. Login to ND Expensify
2. On the URL change the conversation ID to 0 (https://staging.new.expensify.com/r/0)
3. Verify you briefly see the skeleton loader
4. Verify you see a "You don't have access to this chat" message in the conversation history

Expected Result:

"You don't have access to this chat" message in the conversation history is shown

Actual Result:

Infinite skeleton loader occurs if conversation ID is edited to 0 in the URL
Note: The issue reproduces for only "0" value

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

Bug6304320_1701946093585.2023-12-07_11-02-22.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~015ee46823d25664a9
• Upwork Job ID: 1732725262675222528
• Last Price Increase: 2023-12-07
• Automatic offers:
• alitoshmatov | Reviewer | 28066316
• bernhardoj | Contributor | 28066317

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~015ee46823d25664a9

[melvin-bot]

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

[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 - @alitoshmatov (External)

[dukenv0307]

Proposal

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

Infinite loading occurs if conversation ID is edited to 0

What is the root cause of that problem?

When the reportID is 0, we don't call openReport API and isLoadingInitialReportActions is true by default prop. So infinite loading occurs

App/src/pages/home/ReportScreen.js

Line 243 in b0268fa

if (!ReportUtils.isValidReportIDFromPath(reportIDFromPath)) {

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

We should check if isLoadingInitialReportActions but reportID is invalid we will not show loading page. Instead, we will show not found page by create a variable like isValidReportIDFromPath

isValidReportIDFromPath = ReportUtils.isValidReportIDFromPath(reportIDFromPath)
// If has no reportID in param, it means we are setting reportID in `ReportScreenIDSetter`
const hasReportIDFromParam = !!route.params.reportID;

And then update the condition to show not found page with this variable.

const shouldShowNotFoundPage = useMemo(
    () => (!firstRenderRef.current && ((!report.reportID && !isOptimisticDelete && !reportMetadata.isLoadingInitialReportActions && !isLoading && !userLeavingStatus) || (!isValidReportIDFromPath && hasReportIDFromParam))) || shouldHideReport,
    [report, reportMetadata, isLoading, shouldHideReport, isOptimisticDelete, userLeavingStatus],
);

What alternative solutions did you explore? (Optional)

NA

Result

Screen.Recording.2023-12-14.at.12.47.00.mov

Screen.Recording.2023-12-14.at.12.47.48.mov

[mkhutornyi]

Proposal

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

Infinite loading occurs occurs if reportID = 0

What is the root cause of that problem?

We validate reportID here and early return if invalid before calling OpenReport which sets isLoadingInitialReportActions to false optimistically.
So reportMetadata_0 is not set in Onyx and it fallback to this default object:

App/src/pages/home/ReportScreen.js

Lines 105 to 109 in b0268fa

	reportMetadata: {
	isLoadingInitialReportActions: true,
	isLoadingOlderReportActions: false,
	isLoadingNewerReportActions: false,
	},

So isLoadingInitialReportActions is true and it doesn't meet shouldShowNotFoundPage condition

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

1. define new flag. Let's say X!
   X = ReportUtils.isValidReportIDFromPath(reportID) && reportMetadata.isLoadingInitialReportActions
   And replace all reportMetadata.isLoadingInitialReportActions occurrences with X in ReportScreen.js component

2. (optional) For existing users who already have stale reportMetadata_reportID in Onyx:
   clear reportMetadata_reportID before early return here:
   

   App/src/pages/home/ReportScreen.js

   Lines 243 to 245 in b0268fa

   	if (!ReportUtils.isValidReportIDFromPath(reportIDFromPath)) {
   	return;
   	}

so call this function:

function clearReportMetadata(reportID: string) {
    Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`, null);
}

[yh-0218]

Proposal

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

Infinite loading occurs occurs if reportID = 0

What is the root cause of that problem?

We didn't consider isValidReportIDFromPath to get shouldShowNotFoundPage

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

We need to consider isValidReportIDFromPath to get shouldShowNotFoundPage

const shouldShowNotFoundPage = useMemo(
        () => {
            const reportIDFromPath = getReportID(route);
            if (['0'].includes(reportIDFromPath)) {
                return true;
            }
            return (!firstRenderRef.current && !report.reportID && !isOptimisticDelete && !reportMetadata.isLoadingInitialReportActions && !isLoading && !userLeavingStatus) || shouldHideReport;
        },
        [route, report.reportID, isOptimisticDelete, reportMetadata.isLoadingInitialReportActions, isLoading, userLeavingStatus, shouldHideReport],
    );

What alternative solutions did you explore? (Optional)

Screen.Recording.2023-12-07.at.2.59.55.PM.mov

[bernhardoj]

Proposal

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

When visiting a report with id of 0, the loading shows infinitely.

What is the root cause of that problem?

We currently prevent the open report API if the report id is 0, empty, or "null" string. This makes the report screen always show the skeleton loader.

App/src/pages/home/ReportScreen.js

Lines 241 to 245 in 8fa65ec

	// Report ID will be empty when the reports collection is empty.
	// This could happen when we are loading the collection for the first time after logging in.
	if (!ReportUtils.isValidReportIDFromPath(reportIDFromPath)) {
	return;
	}

App/src/libs/ReportUtils.ts

Lines 3915 to 3917 in 8fa65ec

	function isValidReportIDFromPath(reportIDFromPath: string): boolean {
	return !['', 'null', '0'].includes(reportIDFromPath);
	}

This is added to fix this issue where the app crashes if we send a "null" string as the report ID to the API, but I found it weird why it only happens with a "null" string and not other string such as "a". So, I'm trying to reproduce the issue without the fix above and it works fine (it's probably a temp BE issue).

So, currently, if we visit a report with an ID of 0 or null, the loading shows indefinitely, but visiting a report with an ID of other characters such as "a", the not found page will show, so it's a bit inconsistent with how we handle the report ID.

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

I would suggest removing isValidReportIDFromPath. Let the BE handles any report ID we sent and the not found page will be shown if it's not found in the BE.

if (!reportIDFromPath) {
    return;
}

What alternative solutions did you explore? (Optional)

Show the not found page if the report id is not empty and is invalid.

// add to shouldShowNotFoundPage condition
const reportIDParams = lodashGet(route.params, 'reportID');
|| (reportIDParams && !ReportUtils.isValidReportIDFromPath(reportIDParams))