[HOLD for payment 2024-11-29] [$250] Track expense - Options are still present in thread details after the expense is deleted

[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.63-1
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause - Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Go to self DM
3. Track an expense
4. Go to transaction thread
5. Send a message
6. Click on the report header
7. Click Delete expense
8. Delete the expense
9. Click on the thread in the main chat to return to the thread
10. Click report header

Expected Result:

Track expense options should not be displayed in thread after the expense is deleted

Actual Result:

Track expense options is still displayed in thread after the expense is deleted.
The options lead to not here page

Workaround:

Unknown

Platforms:

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

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6666489_1731686292257.20241115_235512.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021858671461676585752
• Upwork Job ID: 1858671461676585752
• Last Price Increase: 2024-11-19
• Automatic offers:
• ikevin127 | Reviewer | 104973306
• twilight2294 | Contributor | 104973308

Issue Owner

Current Issue Owner: @alexpensify

[melvin-bot]

Triggered auto assignment to @alexpensify (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.

[twilight2294]

Proposal

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

Options are still present in thread details after the expense is deleted

What is the root cause of that problem?

We do not check if !isDeletedParentAction hence it is shown even when it is deleted

App/src/pages/ReportDetailsPage.tsx

Line 378 in 88713ab

if (isTrackExpenseReport) {

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

Add isTrackExpenseReport && !isDeletedParentAction check to fix this

What alternative solutions did you explore? (Optional)

[nkdengineer]

Proposal

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

Track expense - Options are still present in thread details after the expense is deleted

What is the root cause of that problem?

The track expense option is displayed if a report is a track expense report.

App/src/pages/ReportDetailsPage.tsx

Line 378 in 88713ab

if (isTrackExpenseReport) {

isTrackExpenseReport is true if the parent report action is track expense action and the parent report is the selfDM.

App/src/libs/ReportUtils.ts

Line 1651 in 88713ab

return !isEmptyObject(parentReportAction) && selfDMReportID === report.parentReportID && ReportActionsUtils.isTrackExpenseAction(parentReportAction);

After we delete a track expense, the parent report action still has type as track in originalMessage then the track expense option still appears in the report detail page

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

Return false in isTrackExpenseReport function if the parent report action is deleted action

return (
    !isEmptyObject(parentReportAction) &&
    selfDMReportID === report.parentReportID &&
    ReportActionsUtils.isTrackExpenseAction(parentReportAction) &&
    !ReportActionsUtils.isDeletedAction(parentReportAction)
);

App/src/libs/ReportUtils.ts

Line 1651 in 88713ab

return !isEmptyObject(parentReportAction) && selfDMReportID === report.parentReportID && ReportActionsUtils.isTrackExpenseAction(parentReportAction);

Or we can do this in isTrackExpenseAction function

What alternative solutions did you explore? (Optional)

Or after we delete the track expense, we should remove the type in originalMessage of parent report action or change type to the according type.

[melvin-bot]

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

[melvin-bot]

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

[alexpensify]

@ikevin127 when you get a chance, please review these proposals and identify whether one will fix the issue. Thank you!

[ikevin127]

@twilight2294's proposal looks good to me. The RCA is correct and the proposed solution fixes the issue.

The reason I'm for the first solution and not for the 2nd proposal's solution is because ReportUtils.isTrackExpenseReport(report) is used in other parts of the app to determine whether an expense report is of type track.

Adding a check which determines whether the track expense's parent report action was deleted would bring about regressions in those instances where the extra check is not needed.

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[nkdengineer]

@ikevin127 I don't think we should consider a report as a track expense report if the parent report action is deleted.

1. All instances that we're using isTrackExpenseReport always require the parent report is not deleted

2. It should be the same when we delete an expense/iou report. When we delete an expense/iou report, even the report preview action changes to add comment action and the type of expense/iou report also changes to chat.

3. For example isSingleTransactionView shouldn't be true if the track expense is deleted. isSingleTransactionView is used to display the header as MoneyRequestHeader which doesn't make sense in this case the track expense is deleted.

App/src/pages/home/ReportScreen.tsx

Line 261 in f77087f

const isSingleTransactionView = ReportUtils.isMoneyRequest(report) || ReportUtils.isTrackExpenseReport(report);

[twilight2294]

thanks @ikevin127 for choosing my proposal, I agree that checking isTrackExpenseReport && !isDeletedParentAction is the right approach here given the nature of this bug.

[ikevin127]

@nkdengineer I understand where your proposal is coming from but in this case, based on the issue and its expected result I think adding the check where the first proposal suggested is a simple fix that targets the expected result directly and keeps things within the scope of the issue.

My goal with this review is to keep it within the scope of the issue and not change code which would affect other parts of the app in unknown ways.

If @carlosmiceli considers we should extend the scope of the issue, go with your solution and extend the testing because of all other places where the check is used - then I don't have anything against that.

@twilight2294 There's no need to reinforce what the reviewer (me, in this case) already decided on, or thumbs down the other contributor's comments as that doesn't bring about anything constructive and the discussion can go down some paths which might not be ideal.

Speaking from experience, most of the times it doesn't help when a contributor becomes hostile or defensive towards another.

Let's keep things cordial and await CME's final decision 🙏

[melvin-bot]

📣 @ikevin127 🎉 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]

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

[carlosmiceli]

I followed your recommendations, @ikevin127

@mallenexpensify, just fyi.

[twilight2294]

PR ready for review c.c. @ikevin127

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

• Fix: Track Expense Options are still present in thread details after the expense is deleted #52786

If no regressions arise, payment will be issued on 2024-11-29. 🎊

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

• @ikevin127 requires payment automatic offer (Reviewer)
• @twilight2294 requires payment automatic offer (Contributor)

[melvin-bot]

@ikevin127 @alexpensify @ikevin127 The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

[ikevin127]

BugZero Checklist:

• [Contributor] Classify the bug:

Bug classification

Source of bug:

• 1a. Result of the original design (eg. a case wasn't considered)
• 1b. Mistake during implementation
• 1c. Backend bug
• 1z. Other:

Where bug was reported:

• 2a. Reported on production
• 2b. Reported on staging (deploy blocker)
• 2c. Reported on both staging and production
• 2d. Reported on a PR
• 2z. Other:

Who reported the bug:

• 3a. Expensify user
• 3b. Expensify employee
• 3c. Contributor
• 3d. QA
• 3z. Other:

• [Contributor] 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: https://github.com/Expensify/App/pull/51951/files#r1854855831

• [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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.

• [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

• [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

  Link to issue:

Regression Test Proposal

1. Go to self DM > Track an expense.
2. Go to transaction thread > Send a message.
3. Click on the report header > Click Delete expense > Delete the expense.
4. Click on the thread in the main chat to return to the thread > Click report header.
5. Verify that the Track expense options in the RHP are not displayed anymore after the expense is deleted.

Do we agree 👍 or 👎.

[ikevin127]

cc @alexpensify for visibility

[alexpensify]

Payouts due: 2024-11-29

• Contributor: $250 @twilight2294
• Reviewer: $250 @ikevin127

Upwork job is here. The payment process has been completed here via Upwork.

[mallenexpensify]

@alexpensify for payment summaries can you use the below format that makes it clear if an issue's is due or paid (and where they're paid). Thx

Contributor: @ paid $ via Upwork
Contributor+: @ owed $ via NewDot.

[alexpensify]

@mallenexpensify - I followed up in the B0 Slack room since the template is off in TE, not sure if there are others.