[$500] IOU - User is redirected to not here page after deleting IOU request with message

[kbecciv]

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.1.14.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. Go to 1:1 DM.
2. Click + > Request money.
3. Create a manual request.
4. On request details page, edit the amount.
5. Click 3-dot menu > Delete request.
6. Delete the request.
7. Click on browser back button.

Expected Result:

In Step 6, after deleting the request, user is redirected to the main chat or Concierge since the report is now not unavailable.

Actual Result:

In Step 6, after deleting the request, user is redirected to not here page.

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

Bug6319777_1702997876666.20231219_225506.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01b7ea0b939fdc648e
• Upwork Job ID: 1737130492537094144
• Last Price Increase: 2024-01-02
• Automatic offers:
• akinwale | Reviewer | 28081714
• dukenv0307 | Contributor | 28081715

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @anmurali (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 - @akinwale (External)

[namhihi237]

Proposal

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

The user is redirected to not here page after deleting IOU request with a message edit amou

What is the root cause of that problem?

When we edit the amount we have a reportAction change amount, so shouldDeleteTransactionThread here returns the value false because the last visible action here include MODIFIEDEXPENSE action.

App/src/libs/actions/IOU.js

Line 2277 in b3ecf32

const shouldDeleteTransactionThread = transactionThreadID ? ReportActionsUtils.getLastVisibleMessage(transactionThreadID).lastMessageText.length === 0 : false;

Leading to not entering this condition

App/src/libs/actions/IOU.js

Lines 2503 to 2509 in b3ecf32

	// STEP 7: Navigate the user depending on which page they are on and which resources were deleted
	if (isSingleTransactionView && shouldDeleteTransactionThread && !shouldDeleteIOUReport) {
	// Pop the deleted report screen before navigating. This prevents navigating to the Concierge chat due to the missing report.
	Navigation.goBack(ROUTES.REPORT_WITH_ID.getRoute(iouReport.reportID));
	return;
	}

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

We should change the condition here to return true if all of the reportActions in the report are not VisibleAction and other than MODIFIEDEXPENSE.

We can add the param isExceptModifiedExpense boolean getLastVisibleMessage and getLastVisibleAction
And update filter visibleReportActions

 const visibleReportActions = Object.values(reportActions ?? {}).filter(
        (action) => shouldReportActionBeVisibleAsLastAction(action) && (isExceptModifiedExpense ? action.actionName !== CONST.REPORT.ACTIONS.TYPE.MODIFIEDEXPENSE : true),
    );

What alternative solutions did you explore? (Optional)

Here we can use childVisibleActionCount but need to fix more because if in the thread we have only a task, the childVisibleActionCount is still 0. because we don't update childVisibleActionCount reportActions of parent report when we create and cancel the task So we redirect without the thread being deleted. We still access the request money thread.

App/src/libs/actions/Task.js

Lines 87 to 90 in 52ecad4

	// We're only setting onyx data for the task report here because it's possible for the parent report to not exist yet (if you're assigning a task to someone you haven't chatted with before)
	// So we don't want to set the parent report data until we've successfully created that chat report
	// FOR TASK REPORT
	const optimisticData = [

So what we should todo if use childVisibleActionCount here:

• Update condition use childVisibleActionCount to check shouldDeleteTransactionThread
• Add the logic updated childVisibleActionCount if the existing parent report when create a task and remove a task

[dukenv0307]

Proposal

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

In Step 6, after deleting the request, user is redirected to not here page.

What is the root cause of that problem?

The condition here is not correct, it's checking the last visible message, which will include MODIFIEDEXPENSE report actions, which are not considered visible comments in the thread that we should keep after deleting the money request.

So shouldDeleteTransactionThread is false and it will not redirect to IOU report page after deleting money request, but it's still deleted in back-end because there's no visible report action left, causing the Not found page.

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

Use reportAction.childVisibleActionCount instead, if it's 0, that means there's no visible action in that money request thread and we're good to delete the money request.

We can do it by updating this line to

const shouldDeleteTransactionThread = transactionThreadID ? !reportAction.childVisibleActionCount : false;

What alternative solutions did you explore? (Optional)

NA

[melvin-bot]

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

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

@akinwale, @anmurali 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[melvin-bot]

@akinwale, @anmurali Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!

[melvin-bot]

@akinwale, @anmurali 12 days overdue now... This issue's end is nigh!

[akinwale]

After reviewing both proposals, the proposed solution in @dukenv0307's proposal is more elegant and I suggest that we move forward with this.

🎀👀🎀 C+ reviewed.

[melvin-bot]

Triggered auto assignment to @iwiznia, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[melvin-bot]

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

[namhihi237]

@akinwale I think that solution is not enough, as I mentioned in the alternative solution, if we forward that way we need to update childVisibleActionCount when the task is created and removed, otherwise it will cause regression

[melvin-bot]

@iwiznia @akinwale @anmurali this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[abzokhattab]

Hey folks,

I am not able to reproduce on staging which step is wrong here @iwiznia @akinwale :

Screen.Recording.2024-01-03.at.12.01.04.AM.mov

[iwiznia]

it's checking the last visible message, which will include MODIFIEDEXPENSE report actions, which are not considered visible comments in the thread that we should keep after deleting the money request

So MODIFIEDEXPENSE action is considered invisible in a non thread chat but considered visible in a thread?

[dukenv0307]

So MODIFIEDEXPENSE action is considered invisible in a non thread chat but considered visible in a thread?

@iwiznia the MODIFIEDEXPENSE only exists in the money request thread. But it's not considered a "visible action" (it will not increase childVisibleActionCount and will not display as "x replies" in the parent report). So if there's only MODIFIEDEXPENSE report actions in a money request, childVisibleActionCount will be 0 and if the money request is deleted, the money request will be gone for good.

Meanwhile if there're visible actions (which increases the childVisibleActionCount) like comment/attachment, the money request will be kept (thus the user can still see [Deleted request] and the "x replies")

[abzokhattab]

Are you still able to reproduce? cannot reproduce it on staging as mentioned here @iwiznia

[iwiznia]

Oh sorry, missed that comment... maybe this is fixed then? @kbecciv can you confirm if the bug is gone?

[akinwale]

Oh sorry, missed that comment... maybe this is fixed then? @kbecciv can you confirm if the bug is gone?

It is still reproducible. The final step is to click on the browser back button.

[iwiznia]

Ah got it. OK proposal looks good then, just nitpick I think it's clearer to make it a regular condition instead of a ternary:

const shouldDeleteTransactionThread = transactionThreadID && !reportAction.childVisibleActionCount;

[melvin-bot]

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

[akinwale]

@iwiznia I just ran another test on the latest main and it appears this has been fixed. It looks like I did not have my local branch updated when I initially checked.

We can close the issue.

[iwiznia]

Oh lol great