[HOLD for payment 2024-04-05] Sign up - Hmmm its not here page is displayed for a moment on sign up

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 1.4.56-0
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause - Internal Team

Action Performed:

1. Navigate to sing up page
2. Sign up with a new account

Expected Result:

Hmmm its not here page shouldn't be displayed momentarily

Actual Result:

Hmmm its not here page is displayed momentarily

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

Bug6423417_1711127371542.Screen_Recording_2024-03-22_at_8.06.33_in_the_evening.mp4

View all open jobs on GitHub

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[melvin-bot]

Triggered auto assignment to @deetergp (Engineering), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[lanitochka17]

@deetergp 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 #wave-collect - Release 1

[nkdengineer]

Proposal

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

Members added and deleted offline are grayed out but strikethrough is not applied

What is the root cause of that problem?

Regression from #30269

After we sign in, reportIDFromRoute is 0 string and then the not found page display with this condition.

App/src/pages/home/ReportScreen.tsx

Line 538 in 08fca15

(!!reportIDFromRoute && !ReportUtils.isValidReportIDFromPath(reportIDFromRoute)),

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

To correctly, we should get the current reportID from params to check, if it's undefined that mean we're getting the reportID to display and then skeleton view can be displayed.

const reportIDFromParams =route.params?.reportID;

// eslint-disable-next-line rulesdir/no-negated-variables
const shouldShowNotFoundPage = useMemo(
    (): boolean =>
        (!wasReportAccessibleRef.current &&
            !firstRenderRef.current &&
            !report.reportID &&
            !isOptimisticDelete &&
            !reportMetadata?.isLoadingInitialReportActions &&
            !isLoading &&
            !userLeavingStatus) ||
        shouldHideReport ||
        (!!reportIDFromParams && !ReportUtils.isValidReportIDFromPath(reportIDFromParams)),
    [report, reportMetadata, isLoading, shouldHideReport, isOptimisticDelete, userLeavingStatus, reportIDFromParams],
);

What alternative solutions did you explore? (Optional)

NA

[dukenv0307]

I reported this bug in Slack here https://expensify.slack.com/archives/C049HHMV9SM/p1711123380240439. I can take over this as C+ if it's necessary.

[ishpaul777]

I think we just need to seperate !isloading to a seperate required conditional

App/src/pages/home/ReportScreen.tsx

Lines 528 to 540 in 08fca15

	const shouldShowNotFoundPage = useMemo(
	(): boolean =>
	(!wasReportAccessibleRef.current &&
	!firstRenderRef.current &&
	!report.reportID &&
	!isOptimisticDelete &&
	!reportMetadata?.isLoadingInitialReportActions &&
	!isLoading &&
	!userLeavingStatus) ||
	shouldHideReport ||
	(!!reportIDFromRoute && !ReportUtils.isValidReportIDFromPath(reportIDFromRoute)),
	[report, reportMetadata, isLoading, shouldHideReport, isOptimisticDelete, userLeavingStatus, reportIDFromRoute],
	);

                !isLoading && (!wasReportAccessibleRef.current &&
                !firstRenderRef.current &&
                !report.reportID &&
                !isOptimisticDelete &&
                !reportMetadata?.isLoadingInitialReportActions &&
                !userLeavingStatus) ||
                shouldHideReport ||
                (!!reportIDFromRoute && !ReportUtils.isValidReportIDFromPath(reportIDFromRoute)),

cc @perunt @roryabraham

[roryabraham]

I think we just need to seperate !isloading to a seperate required conditional

Tried this, doesn't appear to work.

edit: it works, I just had to move the parens around a bit

[ishpaul777]

It should work with a slight variation !isLoading && (all other conditonal)

Screen.Recording.2024-03-23.at.2.41.48.PM.mov

[roryabraham]

exactly, got it. Putting up a PR now. @ishpaul777 you can review as C+ since this was a regression from Comment Linking.

[ishpaul777]

Sure!

[roryabraham]

PR is ready: #38856

[nkdengineer]

@roryabraham #38856 a regression of this PR

Infinite loading if we enter a reportID that is one in this array ['', 'null', '0']. for invalid reportID, OpenReport API isn't called and then isLoading is always true,

const isLoading = !ReportUtils.isValidReportIDFromPath(reportIDFromRoute) ||

Screen.Recording.2024-03-25.at.16.49.40.mov

I think each wrap logic in shouldShowNotFoundPage is added for some reason and we shouldn't wrap this with shouldShowSkeleton

As I mentioned in my proposal, the previous logic is added from this PR #33394, and we should bring this logic back. It can fix the current RCA and can prevent from causing other regressions.

[roryabraham]

Verified this is fixed on staging, closing this out and checking it off on the checklist. @nkdengineer do you have a link to this new issue / regression you're alluding to?

[nkdengineer]

@roryabraham There is no new issue for that but it's a known issue here #32651 that is fixed. But after the PR here, it is back.

[bernhardoj]

It also causes this infinite loading issue #39090. We should use the old approach as suggested by @nkdengineer

[garrettmknight]

@roryabraham shall we reopen this one to fix the regression?

[ishpaul777]

regression should be fixed by #38955 merged recently

Screen.Recording.2024-03-29.at.1.36.15.AM-1.mov

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

• Don't show 404 on ReportScreen if isLoading is true #38856

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

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

• @ishpaul777 requires payment (Needs manual offer from BZ)