[$500] [HOLD for payment 2024-01-26] Request Money - User is redirected to chat then IOU report after deleting request offline

[kbecciv]

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.24-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:
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. Go offline.
2. Go to 1:1 DM.
3. Create a money request.
4. Go to request details page.
5. Tap 3-dot menu > Delete request.
6. Delete the request.

Expected Result:

User is redirected to the main chat after deleting the request.

Actual Result:

User is redirected to the main chat first, then back to IOU report.

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

Bug6337882_1704903684423.Screen_Recording_20240110_222315_New_Expensify.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01abb6552e1259c745
• Upwork Job ID: 1749405409534021632
• Last Price Increase: 2024-01-22

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

[blimpich]

Looking at this now.

[blimpich]

Borked my VM, taking awhile to fix it in order to test this out locally.

[situchan]

#34244 was deployed to staging. This can be easily tested without manually scrolling.
Maybe ask QA team to re-test?

Edit: sorry commented in wrong issue 🙏

[blimpich]

Hmmm I don't see how that PR would fix this bug but if you think its worthwhile @situchan then I see no harm in trying. @kbecciv would you mind testing again?

[kbecciv]

Issue is reproduced on build 1.4.24.0

Screen_Recording_20240110_162525_New.Expensify.mp4

[blimpich]

Alright taking a look at this again.

[blimpich]

I believe #32698 is causing this. I reproduced the error locally, reverted that PR, and then couldn't reproduce it locally again. @rojiphil @fedirjh can you confirm / any idea why this error would be happening, and how we could fix it?

[rojiphil]

Looking into this now

[rojiphil]

Proposal

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

On deletion of the last Money Request, user is redirected to Chat Report momentarily and then to the IOU report.

What is the root cause of that problem?

When we delete a money request from the transaction thread, we navigate back again to the parent Money Request report here because of the condition here. This was done to handle a specific issue (i.e. deletion of a Money Request when multiple Money Requests exist) as mentioned here for which a PR was recently raised here. Since we have introduced the offline support for Money Request deletion, we do not delete the Money Request report but maintain in Onyx as shown here until a successful API response comes back from server. However, this will make the condition true here thereby resulting in navigation back to the Money Request report. This is the root cause.

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

We can check if the Money Request report is marked for deletion as we do here. We can apply the same check here and skip the navigation back to the Money Request report. We can also create a utility function instead of duplicating the code.

What alternative solutions did you explore? (Optional)

[thienlnam]

Can you push up a PR for this @rojiphil?

Actually, I think we can remove this from blocking deploy - it's a pretty specific flow and requires you to be offline. I'll like @blimpich decide how to handle this when he gets back online

[blimpich]

I agree, I was kind on the fence for whether or not it should be deploy blocker. We can just deal with this as a normal issue.

[fedirjh]

@blimpich The proposal looks good to me, and I just tested this PR #34320 and it works as expected. Should we proceed with it?

[blimpich]

@fedirjh Yup!

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

• prevent offline navigation on money request report deletion #34320

If no regressions arise, payment will be issued on 2024-01-26. 🎊

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

• @rojiphil requires payment (Needs manual offer from BZ)
• @fedirjh requires payment (Needs manual offer from BZ)

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

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

[melvin-bot]

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

[melvin-bot]

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

[laurenreidexpensify]

@fedirjh @rojiphil pls apply to upwork job https://www.upwork.com/jobs/~01abb6552e1259c745

[blimpich]

@laurenreidexpensify sorry I forgot to comment here but payment for this issue was handled by this issue already #26511. I should have commented.

[blimpich]

See this comment: #26511 (comment)

[laurenreidexpensify]

Ah cool - if that's the case then @fedirjh please complete checklist and we can close. Thanks

[fedirjh]

@laurenreidexpensify This regression with a recent PR and the author already fixed it. It was marked as deploy blocker so I believe we have the necessary regression test in place.