[HOLD for payment 2024-07-17] [$250] Track - Missing "Delete expense" in report details page when expense is tracked in self DM

[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: 9.0.1-10
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Go to workspace chat
3. Go to + > Track expense > Manual
4. Track an expense
5. Go to transaction thread
6. Click on the chat header
7. Note that "Delete expense" is present in report details page
8. Go to self DM.
9. Track an expense
10. Go to transaction thread
11. Click on the chat header

Expected Result:

"Delete expense" will be present in report details page when expense is tracked in self DM

Actual Result:

"Delete expense" is not present in report details page when expense is tracked in self DM

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

Bug6523362_1719259207236.20240625_035453.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01bb4860cbcf9ab40f
• Upwork Job ID: 1806787222801178404
• Last Price Increase: 2024-06-28
• Automatic offers:
• bernhardoj | Contributor | 102950333

Issue Owner

Current Issue Owner: @sakluger

[melvin-bot]

Triggered auto assignment to @sakluger (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[lanitochka17]

@sakluger 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 #vip-vsp

[dominictb]

Proposal

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

"Delete expense" is not present in report details page when expense is tracked in self DM

What is the root cause of that problem?

• In case of track expense report, transactionThreadReport or transactionThreadReportID is undefined because the report.type is chat. So in this:
  

  App/src/pages/ReportDetailsPage.tsx

  Lines 95 to 98 in 7fb3d9e

  	const transactionThreadReportID = useMemo(
  	() => ReportActionsUtils.getOneTransactionThreadReportID(report.reportID, reportActions ?? [], isOffline),
  	[report.reportID, reportActions, isOffline],
  	);

  
  transactionThreadReportID is undefined because of this:
  

  App/src/libs/ReportActionsUtils.ts

  Lines 958 to 960 in 7fb3d9e

  	if (report?.type !== CONST.REPORT.TYPE.IOU && report?.type !== CONST.REPORT.TYPE.EXPENSE && report?.type !== CONST.REPORT.TYPE.INVOICE) {
  	return;
  	}

• Then, we use ReportUtils.isTrackExpenseReport(transactionThreadReport) in:
  

  App/src/pages/ReportDetailsPage.tsx

  Lines 190 to 191 in 7fb3d9e

  	const canDeleteRequest =
  	isActionOwner && (ReportUtils.canAddOrDeleteTransactions(moneyRequestReport) || ReportUtils.isTrackExpenseReport(transactionThreadReport)) && !isDeletedParentAction;

  
  to check if we can delete a request and in this case, canDeleteRequest is false because ReportUtils.isTrackExpenseReport(transactionThreadReport) is false.

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

• Should use ReportUtils.isTrackExpenseReport(report) instead

What alternative solutions did you explore? (Optional)

[gijoe0295]

Proposal

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

"Delete expense" is not present in report details page when expense is tracked in self DM

What is the root cause of that problem?

Track expense report is a single transaction view. In other words, it is the thread transaction report itself.

But the condition here using transactionThreadReport to check for isTrackExpenseReport is not correct because track expense report does not have transaction thread report.

App/src/pages/ReportDetailsPage.tsx

Lines 190 to 191 in 17b10c4

	const canDeleteRequest =
	isActionOwner && (ReportUtils.canAddOrDeleteTransactions(moneyRequestReport) || ReportUtils.isTrackExpenseReport(transactionThreadReport)) && !isDeletedParentAction;

This condition is copied from MoneyReportHeader which is only used for money request report so it's not correct.

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

Use report itself as a fallback to check for isTrackExpenseReport if transactionThreadReport does not exist as already used in MoneyRequestHeader:

App/src/components/MoneyRequestHeader.tsx

Line 106 in 6873dd9

const changeMoneyRequestStatus = () => {

ReportUtils.isTrackExpenseReport(transactionThreadReport ?? report)

[dominictb]

Proposal updated

• I updated the RCA

[bernhardoj]

Proposal

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

Missing delete expense option in self DM track expense report details page.

What is the root cause of that problem?

The condition to show the delete expense is shown as below:

App/src/pages/ReportDetailsPage.tsx

Lines 190 to 191 in 621e8fc

	const canDeleteRequest =
	isActionOwner && (ReportUtils.canAddOrDeleteTransactions(moneyRequestReport) || ReportUtils.isTrackExpenseReport(transactionThreadReport)) && !isDeletedParentAction;

The condition that fails in self DM track is ReportUtils.canAddOrDeleteTransactions(moneyRequestReport) and ReportUtils.isTrackExpenseReport(transactionThreadReport). transactionThreadReport is from the one-transaction report thread.

App/src/pages/ReportDetailsPage.tsx

Lines 95 to 100 in 621e8fc

	const transactionThreadReportID = useMemo(
	() => ReportActionsUtils.getOneTransactionThreadReportID(report.reportID, reportActions ?? [], isOffline),
	[report.reportID, reportActions, isOffline],
	);
	const [transactionThreadReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${transactionThreadReportID}`);

For self DM track, transactionThreadReport is undefined because it's not a one-transaction report and moneyRequestReport (the parent report) is a self DM report, so canAddOrDeleteTransactions returns false.

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

The ReportUtils.isTrackExpenseReport condition is copied from the MoneyRequestHeader and it's added to show the delete expense option for self DM track expense, so using transactionThreadReport doesn't make sense.

Instead, we should check if it's a self DM track or not with below condition:
const isSelfDMTrack = ReportUtils.isTrackExpenseReport(report) && ReportUtils.isSelfDM(parentReport);
So, the condition will be:

const canDeleteRequest = isActionOwner && (ReportUtils.canAddOrDeleteTransactions(moneyRequestReport) || isSelfDMTrack) && !isDeletedParentAction;

This is the same solution with my proposal here but on a different component and case.

[melvin-bot]

@sakluger Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

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

[melvin-bot]

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

[jayeshmangwani]

I agree with @bernhardoj 's Proposal to check the isSelfDM condition for the canDeleteRequest check. LGTM 🚀

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[jayeshmangwani]

@dominictb I see that your comment has been marked as outdated. Is there anything I can help with regarding the selected proposal?

[dominictb]

@jayeshmangwani Thanks. I got a result why we need to check isSelfDM, so I marked my question as outdated. And the selected proposal LGTM.

[jayeshmangwani]

Thanks for the understanding!

[melvin-bot]

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

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

• Fix missing delete expense in self dm track expense details page #44717

If no regressions arise, payment will be issued on 2024-07-17. 🎊

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

• @jayeshmangwani requires payment through NewDot Manual Requests
• @bernhardoj 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:

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

[bernhardoj]

I'll request in ND once payment is due.

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.6-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix missing delete expense in self dm track expense details page #44717

If no regressions arise, payment will be issued on 2024-07-22. 🎊

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

• @jayeshmangwani requires payment through NewDot Manual Requests
• @bernhardoj 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:

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

[bernhardoj]

The payment should be due tomorrow (07-17)

[sakluger]

Thanks @bernhardoj! I'll complete payments tomorrow.

@jayeshmangwani could you please complete the BZ checklist?

[bernhardoj]

I requested in ND.

[jayeshmangwani]

• [@jayeshmangwani] The PR that introduced the bug has been identified. Link to the PR: [Details Revamp] Update ReportDetailsPage for Rooms, Groups, Money Reports and Policy Expenses #43251
• [@jayeshmangwani] 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: [Details Revamp] Update ReportDetailsPage for Rooms, Groups, Money Reports and Policy Expenses #43251 (comment)
• [@jayeshmangwani] 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: N/A
• [@jayeshmangwani] Determine if we should create a regression test for this bug. Yes
• [@jayeshmangwani] 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.

Regression Test Proposal

1. Open self DM.
2. Track a new expense.
3. Open the track expense report.
4. Press the header.
5. Verify that there is a "Delete expense" option.

Do we agree 👍 or 👎

[bernhardoj]

@sakluger hi, I have requested the payment in ND (not paid yet). Can/should I refund the upwork so I get paid in ND?

[sakluger]

@bernhardoj, I'm sorry - I missed your comment about requesting in NewDot. Let me find out if it's possible to reverse the Upwork payment.

[sakluger]

Hi @bernhardoj - I requested a refund in Upwork. Can you please confirm the refund and let me know once you have done so?

[bernhardoj]

@sakluger I have issued the refund

[sakluger]

Summarizing payment on this issue:

Contributor: @bernhardoj $250, please request on NewDot
Contributor+: @jayeshmangwani $250, please request on Newdot

[JmillsExpensify]

$250 approved for @bernhardoj

[jayeshmangwani]

Requested $250

[JmillsExpensify]

$250 approved for @jayeshmangwani