[$1000] Blank page appears instead of Hmm... it's not here page

[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. from userA, create a workspace
2. invite userB to workspace
3. from userB, go to #announce
4. click on header to see details
5. from userA, remove the userB

Expected Result:

should show Hmm... it's not here in the RHP

Actual Result:

blank page appears

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.74-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
Notes/Photos/Videos: Any additional supporting documentation

Recording.1603.mp4

Expensify/Expensify Issue URL:
Issue reported by: @gadhiyamanan
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1692600279076409

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~016233e1e785234a4a
• Upwork Job ID: 1706382137664765952
• Last Price Increase: 2023-10-18

[melvin-bot]

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

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~016233e1e785234a4a

[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 @laurenreidexpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[melvin-bot]

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

[ewanmellor]

Proposal

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

If the user has the Details RHN open for a channel, and the user is removed from the workspace, then the RHN pane goes to black and does not show the "Hmm... it's not here" screen.

What is the root cause of that problem?

This is a result of an assumption made inside WithReportOrNotFound:

App/src/pages/home/report/withReportOrNotFound.js

Line 55 in f5409e7

// If the content was shown but it's not anymore that means the report was deleted and we are probably navigating out of this screen.

The comment says "we are probably navigating out of this screen" but this is not the case. We have lost access to the report, but we do not have anything to cause us to navigate away. WithReportOrNotFound returns null (so that we don't show a glitchy transition) but since we're not navigating in this case, we just render a blank panel where the ReportDetailsPage used to be. This does not happen once the page is reloaded, because if WithReportOrNotFound has never successfully rendered the report then it correctly shows the NotFoundPage. This problem only occurs if we used to be able to render the report, but then lose that ability.

This bug was introduced here:

commit b2426a23b6b721a1073752c795974108ff7799f0
Author: Adam Grzybowski <adam.grzybowski@swmansion.com>
Date:   Tue Aug 8 12:37:10 2023 +0200

   fix glitching animation of NotFoundPage on transition

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

I think it would be reasonable in this case to render the content that was previously shown. That way, if we are navigating away anyway, as the code assumes, then we would not switch to the NotFoundPage and we'd still avoid the glitchy transition. If we are not navigating then we'd stay on the current page, i.e. displaying stale content for a report that we have lost access to. The next time the user navigates, they will step to a screen that will render as NotFoundPage as normal. That seems like reasonable behavior to me, given how small a corner case this is.

My proposal is to save the props seen by WithReportOrNotFound in a ref, and in the case where we currently return null, instead return the WrappedComponent with those remembered props.

What alternative solutions did you explore? (Optional)

I considered adding a fallback route to WithReportOrNotFound, and making it so it navigates to the fallback route in this case. This would effectively mean all the /r/:reportID/* routes would navigate to /r/:reportID in this situation, and you'd see the NotFoundPage that way. The problem with this is you don't know whether some other part of the app is going to trigger a nav, so you could have two competing navigations that trigger at the same time. That seems like it could cause more problems than it solves.

[kameshwarnayak]

Proposal

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

Blank page appears instead of Hmm... it's not here page

What is the root cause of that problem?

Root cause is that we return null assuming the modal will be dismissed in withReportOrNotFound.js.

we are probably navigating out of this screen, this is the assumption made but we are not moving out of the modal. null is returned and it shows empty screen.

App/src/pages/home/report/withReportOrNotFound.js

Lines 55 to 59 in 52c3f53

	// If the content was shown but it's not anymore that means the report was deleted and we are probably navigating out of this screen.
	// Return null for this case to avoid rendering FullScreenLoadingIndicator or NotFoundPage when animating transition.
	if (shouldShowNotFoundPage && contentShown.current) {
	return null;
	}

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

As the comment says, the report is deleted and we are navigating out of the details screen. We can dismiss the modal here. Change the above code to below. Import Navigation at the top of withReportOrNotFound.js file

        // If the content was shown but it's not anymore that means the report was deleted and we are probably navigating out of this screen.
        // Return null for this case to avoid rendering FullScreenLoadingIndicator or NotFoundPage when animating transition.
        if (shouldShowNotFoundPage && contentShown.current) {
            Navigation.dismissModal();
            return null;
        }

Result :

screen-recording.mp4

What alternative solutions did you explore? (Optional)

NA

[ntdiary]

Hi, @stephanieelliott, I'm still busy reviewing other issues so I don't have enough bandwidth. Could you please reassign this to another C+? : )

[melvin-bot]

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

[stephanieelliott]

Hey @fedirjh we've got a couple proposals already, can you take a look please?

[fedirjh]

I couldn’t test @kameshwarnayak's solution on main, we have an issue with not found reports, it displays skeleton instead of not found view.

@ewanmellor I will re-test both approaches after the issue is fixed.

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot]

@stephanieelliott, @fedirjh Whoops! This issue is 2 days overdue. Let's get this updated quick!

[stephanieelliott]

#28489 was deployed to prof earlier today, can you test when you get a chance @fedirjh?

[stephanieelliott]

Hey @fedirjh bump on this, can you re-test when you get a chance?

[kameshwarnayak]

@fedirjh - Updated the proposal based on the input. Look forward to your thoughts

[yh-0218]

Proposal

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

Blank page appears instead of Hmm... it's not here page

What is the root cause of that problem?

When remove userB from userA's workspace,
userB's status: contentShown.current=true, shouldShowNotFoundPage=true
In this case, returns null

App/src/pages/home/report/withReportOrNotFound.js

Lines 57 to 59 in 562f9d8

	if (shouldShowNotFoundPage && contentShown.current) {
	return null;
	}

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

I think this is enough to fix that issue.
Need to add one more condition there. !_.isEmpty(props.report)

App/src/pages/home/report/withReportOrNotFound.js

Lines 57 to 59 in 562f9d8

	if (shouldShowNotFoundPage && contentShown.current) {
	return null;
	}

if (!_.isEmpty(props.report) && shouldShowNotFoundPage && contentShown.current) {
        return null;
 }

What alternative solutions did you explore? (Optional)

[Vadym-33]

Proposal

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

Blank page appears instead of Hmm... it's not here page

What is the root cause of that problem?

When remove userB from userA's workspace,
userB's status: contentShown.current=true, shouldShowNotFoundPage=true
In this case, returns null

App/src/pages/home/report/withReportOrNotFound.js

Lines 57 to 59 in 562f9d8

	if (shouldShowNotFoundPage && contentShown.current) {
	return null;
	}

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

Need to update condition.

App/src/pages/home/report/withReportOrNotFound.js

Lines 57 to 59 in 562f9d8

	if (shouldShowNotFoundPage && contentShown.current) {
	return null;
	}

if (_.isEmpty(props.report) && contentShown.current) {
        return null;
 }

What alternative solutions did you explore? (Optional)

[kameshwarnayak]

@fedirjh - Tested this solution with other usecases and the result is as expected. Please find the video below

screen-recording.mp4

Have mentioned the logic of solution in the proposal

[melvin-bot]

@stephanieelliott @fedirjh this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

[melvin-bot]

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

[fedirjh]

Look forward to your thoughts

@kameshwarnayak What could happen if the navigation is triggered and then the modal is dismissed given that modal dismissal will trigger a navigation? I feel this will lead to an unpredictable behavior.

---

@s-alves10 This bug affects RHN modals that are related to a given report, so it's not only the details page, we have settings, members, and notes ...

---

@yh-0218 and @Vadym-33 Can you further explain your solutions?

[s-alves10]

Proposal

Updated

[yh-0218]

I think this condition will help all case.

if (!_.isEmpty(props.report) && shouldShowNotFoundPage && contentShown.current) {
        return null;
 }

[Vadym-33]

if (_.isEmpty(props.report) && contentShown.current) {
        return null;
 }

we need to return null on NotFoundPage with this condition.

[kameshwarnayak]

@fedirjh - dismissModal will trigger a navigation to the previous open report. That is intended and basically we need to move out of the modal on null/empty. As you can see this in the below video

274492237-46d75076-b4c8-4b6b-be93-6ae732a87448.mp4

You can see in the below code, dismissModal works on RIGHT_MODAL_NAVIGATOR, NOT_FOUND and REPORT_ATTACHMENTS only. Let me test on NOT_FOUND & REPORT_ATTACHMENTS and let you know the result.

App/src/libs/Navigation/Navigation.js

Lines 164 to 168 in b494186

	switch (lastRoute.name) {
	case NAVIGATORS.RIGHT_MODAL_NAVIGATOR:
	case SCREENS.NOT_FOUND:
	case SCREENS.REPORT_ATTACHMENTS:
	// if we are not in the target report, we need to navigate to it after dismissing the modal

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot]

@stephanieelliott, @fedirjh Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

@stephanieelliott @fedirjh this issue is now 4 weeks old and preventing us from maintaining WAQ, can you:

• Decide whether any proposals currently meet our guidelines and can be approved as-is today
• If no proposals meet that standard, please take this issue internal and treat it as one of your highest priorities
• If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

[melvin-bot]

Current assignee @fedirjh is eligible for the Internal assigner, not assigning anyone new.

[s-alves10]

@fedirjh

Did you check the updated proposal?

[kameshwarnayak]

I am not able to reproduce this in the latest master.

[s-alves10]

Yeah, now it closes the RHN and navigates to the concierge page

[fedirjh]

Yeah, now it closes the RHN and navigates to the concierge page

I couldn't replicate either. It appears that it has been resolved with this PR #26149.

CleanShot.2023-10-24.at.12.12.15.mp4

[fedirjh]

@stephanieelliott Let’s close as fixed by another PR #26149.