[REVIEW TESTS] [Due for payment 2025-04-14] [$500] Update duplicate detection to be treated as any other violation (i.e. on hold applied)

[pecanoro]

Part of the project

Main issue: https://github.com/Expensify/Expensify/issues/463946

Feature Description

Duplicate detection is more in line with our RTER Violation pattern (like the pending receipt match violation) than our full Hold pattern. Let's update duplicate detections to be treated more like the warning so that when the approver approves a report with potential duplicates, we won't show the hold modal. To do so, we'll:

• Stop putting the expenses on HOLD and showing that modal.
• Update potential duplicates to be RTER Violations (or any other warning violation) instead, simply stopping the expenses from being harvested, but not stopping them from being manually submitted or approved.
• Show a different message/next step at the top of the expense/report:
• Update the report's next steps, mirroring the pending receipt pattern:
  "Potential duplicate expenses identified. Review duplicates to enable submission."
• No change for the expense next steps
• No change to the RBR, it'll still show as the RTER Violation

Manual Test Steps

Once these updates are implemented, you should be able to run these steps and verify the expected outcomes:

1. As the submitter, create an expense for $10 with the date of today and ‘Home Depot’ as the merchant
2. As the submitter, create another expense for $10 with the date of today and ‘Home Depot’ as the merchant

• Verify that you see RBR on the workspace chat
• Verify that you see RBR on the report preview in the workspace chat
• Verify that you see RBR on each expense in the report
• Verify the next steps in the report header read: “Potential duplicate expenses identified. Review duplicates to enable submission.”

3. As the submitter, create another expense for $5.
4. As the submitter, submit the report manually
5. As the approver, access the report and click ‘Approve’

• Verify that you’re presented with a modal asking whether you want to Approve only compliant expenses, or approve the entire report.
• Verify that the options show for the entire report ($25) and one for only the expense without violations ($5).

cc/ @garrettmknight

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021898088991592917027
• Upwork Job ID: 1898088991592917027
• Last Price Increase: 2025-03-10
• Automatic offers:
• DylanDylann | Reviewer | 106480966
• nkdengineer | Contributor | 106480968

Issue Owner

Current Issue Owner: @garrettmknight

[pecanoro]

Chatting with @garrettmknight about some details and some tests before opening this up with completed details

[melvin-bot]

Current assignee @garrettmknight is eligible for the Bug assigner, not assigning anyone new.

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

Upwork job price has been updated to $500

[garrettmknight]

I think this is a bit more complicated than most, so let's up the bounty.

[nkdengineer]

Proposal

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

Update duplicate detection to be treated as any other violation (i.e. on hold applied)

What is the root cause of that problem?

This is an improvement

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

Stop putting the expenses on HOLD and showing that modal.

1. We need to remove the isDuplicate from isOnHold function, then it will not show the hold modal

App/src/libs/TransactionUtils/index.ts

Line 986 in 3013e7b

return !!transaction.comment?.hold || isDuplicate(transaction.transactionID, true);

Update the report's next steps, mirroring the pending receipt pattern:
"Potential duplicate expenses identified. Review duplicates to enable submission."

2. Defined a hasDuplicateTransaction function.

function hasDuplicateTransactions(iouReportID?: string, allReportTransactions?: SearchTransaction[]): boolean {
    const transactionsByIouReportID = getReportTransactions(iouReportID);
    const reportTransactions = allReportTransactions ?? transactionsByIouReportID;

    return reportTransactions.length > 0 && reportTransactions.some((transaction) => isDuplicate(transaction.transactionID, true));
}

And then update in MoneyReportHeader to show another message text

const hasDuplicate = hasDuplicateTransactions(moneyRequestReport?.reportID);
const shouldShowStatusBar =
        hasAllPendingRTERViolations || shouldShowBrokenConnectionViolation || hasOnlyHeldExpenses || hasScanningReceipt || isPayAtEndExpense || hasOnlyPendingTransactions || hasDuplicate;

const getStatusBarProps: () => MoneyRequestHeaderStatusBarProps | undefined = () => {
   ...
   if (hasDuplicate) {
        return {icon: getStatusIcon(Expensicons.Hourglass), description: 'Potential duplicate expenses identified. Review duplicates to enable submission.'};
    }
}

Update potential duplicates to be RTER Violations (or any other warning violation) instead, simply stopping the expenses from being harvested, but not stopping them from being manually submitted or approved.

3. I'm not sure what we need to do in this part

Verify that you’re presented with a modal asking whether you want to Approve only compliant expenses, or approve the entire report.
Verify that the options show for the entire report ($25) and one for only the expense without violations ($5).

4. We need to update the flow to display the approval confirm modal to contain the duplicate report check

• Update the condition here to also display the confirm modal if we have duplicate transaction

App/src/components/MoneyReportHeader.tsx

Line 246 in 3013e7b

} else if (isAnyTransactionOnHold) {

• Update the condition here to get the hold/duplicate transactions/reportActions for Onyx data when we only pay/approve partially

App/src/libs/actions/IOU.ts

Line 7648 in 3013e7b

if (transaction?.comment?.hold) {

5. To display the correct nonHeld amount, we need to make a backend change to update the unheldTotal to not include the amount of the duplicate transaction like we did for the hold transaction. For example, in the test case above the unheldTotal of the expense report should be -500

App/src/libs/ReportUtils.ts

Line 8449 in 3013e7b

nonHeldAmount: convertToDisplayString(unheldTotal * coefficient, iouReport?.currency),

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

None

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

[melvin-bot]

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

[pecanoro]

@DylanDylann What do you think? I think we can go with @nkdengineer but I would love for you to double-check everything

[DylanDylann]

On my list today. I will review it asap

[DylanDylann]

I just read the description and checked @nkdengineer's proposal. Overall, he gave a correct direction to implement this feature. But some things aren't clear. Anyways, this seems to require lots of change. Thus, I think we can bring these discussions on the PR phase to speed it up.

Given that, let's go with @nkdengineer

🎀 👀 🎀 C+ Reviewed

[melvin-bot]

Current assignee @pecanoro is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[pecanoro]

Assigning @nkdengineer to the issue

[garrettmknight]

@nkdengineer / @DylanDylann can we update the icon from an exclamation point to a flag before paying this one out? #58387 (comment)

[nkdengineer]

@garrettmknight Sure. Will open a follow-up PR soon.

[nkdengineer]

@garrettmknight Here is the flag icon we want right.

[garrettmknight]

Yep, that's the one. Thanks!

[nkdengineer]

@DylanDylann The follow-up PR is ready for review. cc @garrettmknight

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

• update duplicate icon #59520

If no regressions arise, payment will be issued on 2025-04-14. 🎊

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

• @DylanDylann requires payment automatic offer (Reviewer)
• @nkdengineer requires payment automatic offer (Contributor)

[melvin-bot]

@DylanDylann @garrettmknight @DylanDylann 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]

[garrettmknight]

Thanks! I don't think we need the checklist for this one since it's a new feature. I'll pay out, but I'll leave this open to check out testrails for hold to make sure we aren't testing for the old functionality.

[garrettmknight]

Paid! Thanks for the quick work.

[melvin-bot]

@garrettmknight Whoops! This issue is 2 days overdue. Let's get this updated quick!

[garrettmknight]

Still need to review and update test rails.

[garrettmknight]

Alright, found the test case and submitted and issue to update it. Closing!