[HOLD for payment 2023-12-29] [$1000] IOU - Hmm, not there screen briefly displayed when deleting one IOU out of several

[lanitochka17]

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

---

Action Performed:

1. Open the app
2. Navigate to a chat without previous IOUs
3. Send 3 money requests
4. Open money request detail screen
5. Tap three dots and delete a money request

Expected Result:

User is navigated to the money request preview

Actual Result:

"Hmm, not there" briefly is shown while navigating to the IOU preview

Workaround:

Unknown

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.78-3

Reproducible in staging?: Yes

Reproducible in production?: Yes

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

Bug6226595_1696547640453.video_2023-10-05_19-07-07.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0146ac43978a445e18
• Upwork Job ID: 1710217485696090112
• Last Price Increase: 2023-12-08
• Automatic offers:
• s77rt | Reviewer | 28053787
• dukenv0307 | Contributor | 28053789

[melvin-bot]

Triggered auto assignment to @flaviadefaria (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]

Job added to Upwork: https://www.upwork.com/jobs/~0146ac43978a445e18

[melvin-bot]

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

[ZhenjaHorbach]

Proposal

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

After deleting an IOU, we can replace not there screen while closing the screen

What is the root cause of that problem?

Since the deletion occurs before the end of the animation screen change
We can see this not there screen

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

To fix this bug we can use isFocused to be sure that we want to show not there screen only when screen is focused

we can update shouldShow value for FullPageNotFoundView in ReportScreen

isFocused we can get from @react-navigation/native library

const isFocused = useNavigation().isFocused()
const isFocused = useIsFocused()

And then pass here

shouldShow={shouldShowNotFoundPage && isFocused }

App/src/pages/home/ReportScreen.js

Lines 377 to 384 in 4b0752a

	<FullPageNotFoundView
	shouldShow={shouldShowNotFoundPage}
	subtitleKey="notFound.noAccess"
	shouldShowCloseButton={false}
	shouldShowBackButton={isSmallScreenWidth}
	onBackButtonPress={Navigation.goBack}
	shouldShowLink={false}
	>

What alternative solutions did you explore? (Optional)

NA

[dukenv0307]

Proposal

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

"Hmm, not there" briefly is shown while navigating to the IOU preview

What is the root cause of that problem?

There're 2 broad scenarios where we're currently showing the Not found page in the report screen:

1. When the user actively accessed a report with incorrect id, or they access a report they don't have permissions for.

This is unexpected behavior from the user and so it makes sense to show the not found page

2. When the user does something in the app that leads to the report being deleted (delete money request, leave thread, the report is removed from another device).

This is normal scenario and we should not show the not found page in this case, but should make the UX seamless. This issue belongs to this case.

It happens because briefly after the report is removed, this condition becomes true and the not found page shows briefly before we navigate them to another screen

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

We should check for the 2nd scenario and do not show the not found page in this case. For that scenario, the report was always available and accessible before it becomes empty, we can make use of this characteristics.

• Add a wasReportAccessible ref to indicate whether the report was ever accessible to the user

const wasReportAccessibleRef = useRef(false);

• When report.reportID is not empty and shouldHideReport is false, this means the report is accessible and we'll set wasReportAccessibleRef.current to true

useEffect(() => {
    if (report && report.reportID && !shouldHideReport) {
        wasReportAccessibleRef.current = true;
    }
}, [shouldHideReport, report]);

• In shouldShowNotFoundPage here, we'll not return true if the report was accessible, we'll instead let our navigation (when deleting request, leave thread, ...) navigate the user to the correct report, and not show the brief not found page as in this case

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

Update:
After this change, it will surface an issue when deleting money request from another device as highlighted here. This is actually an existing unrelated bug, in this case the user should be navigated back to the parent report as per this code block (currently it shows not found page which is not good UX) which works for many other cases like user being removed from room in another device, ...

But that condition currently doesn't work for deleting money request from another device. We should add a check to this line: If the prevReport is a money request report or an iou report and report (current report) is the default props (means it's not available), then also goes to this block and do the navigation to parent IOU report).

What alternative solutions did you explore? (Optional)

Using isFocused as suggested above will cause several regressions when we open the search bar, the report page will turn from not found page to loading page.

[s77rt]

@ZhenjaHorbach Thanks for the proposal. Your RCA is correct. But the solution looks like a workaround. The not found page should not be coupled with the focus state as it's unrelated.

[s77rt]

@dukenv0307 Thanks for the proposal. Your RCA makes sense. However the solution won't work correctly in many cases. If the page is not found then we should show the not found view, even if it was accessible before. Otherwise we would have regressions e.g. if you are viewing a report of a workspace then the admin remove you from the workspace then you will still be looking at the same report instead of the not found page because the delete action was done remotely and our navigation will do nothing about it.

[dukenv0307]

Otherwise we would have regressions e.g. if you are viewing a report of a workspace then the admin remove you from the workspace then you will still be looking at the same report instead of the not found page because the delete action was done remotely and our navigation will do nothing about it.

@s77rt In this case we'll be navigating to the Concierge page as handled here. So I don't think there'll be any regression with that. IMO all cases of scenario 2, we're already navigating users to another report (and we should, it's weird for the users to be viewing the report and it becomes not found page some moments later)

[s77rt]

@dukenv0307 Testing this out. We are getting an infinite skeleton view

Before	After
Screen.Recording.2023-10-07.at.5.06.18.PM.mov	Screen.Recording.2023-10-07.at.5.05.26.PM.mov

[s77rt]

Not overdue. Still looking for proposals

[melvin-bot]

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

[flaviadefaria]

@lschurr I'm officially no longer in the BugZero team so reassigning this as it will not close until tomorrow.

[melvin-bot]

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

[lschurr]

Looks like we're still waiting for proposals.

[melvin-bot]

📣 @s77rt 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

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

[dukenv0307]

@s77rt The PR is ready for review

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

• Fix: the user is navigated without showing the Not Found Page when deleting a money request or IOU report #33041

If no regressions arise, payment will be issued on 2023-12-29. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

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

• @s77rt requires payment automatic offer (Reviewer)
• @dukenv0307 requires payment automatic offer (Contributor)

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

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

[lschurr]

Could you work on the checklist for this one @s77rt

[s77rt]

• The PR that introduced the bug has been identified: Migrate policy API to remove policy members #10452
• The offending PR has been commented on: Migrate policy API to remove policy members #10452 (comment)
• A discussion in #expensify-bugs has been started: Not needed
• Determine if we should create a regression test for this bug: I don't think this qualify for a regression test

[s77rt]

@lschurr Done!

[lschurr]

Payment summary:

• Bug reporter: Applause (no payment)
• Contributor: @dukenv0307 $1000
• Contributor+: @s77rt $1000

[lschurr]

Payments issued in Upwork. Closing this one out :)

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.