[HOLD for payment 2024-12-05] [$250] Dupe detection - RBR is not shown when creating dupe expense with a Gmail account

[IuliiaHerets]

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: v9.0.59-0
Reproducible in staging?: Y
Reproducible in production?: Y
Issue was found when executing this PR: #48958
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team

Action Performed:

1. Login to an account.
2. Create a workspace
3. Open the Workspace chat
4. Submit an expense in the Workspace with a category,
5. Submit another expense in the Workspace with a the same amount and merchant in the above but with no category.

Expected Result:

RBR(red dot) is shown in LHN and also in the report because of the dupe expense.

Actual Result:

RBR(red dot) is not shown in LHN and in the report. RBR is only shown inside the expense report

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6658672_1731060429199.Screen_Recording_2024-11-08_at_4.20.15_at_night.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021857074546459286027
• Upwork Job ID: 1857074546459286027
• Last Price Increase: 2024-11-21
• Automatic offers:
• ahmedGaber93 | Reviewer | 105026209

Issue Owner

Current Issue Owner: @muttmuure

[melvin-bot]

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

[bernhardoj]

Edited by proposal-police: This proposal was edited at 2024-11-21 13:28:28 UTC.

Proposal

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

RBR is not shown on the report preview when money request is dupe.

What is the root cause of that problem?

The RBR doesn't show on the report preview because it only shows if the dupe detection permission is enabled. IF the account doesn't have the permission, then RBR won't show.

App/src/components/ReportActionItem/ReportPreview.tsx

Lines 156 to 160 in 5e69bfc

	const hasErrors =
	(hasMissingSmartscanFields && !iouSettled) ||
	// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
	ReportUtils.hasViolations(iouReportID, transactionViolations) ||
	ReportUtils.hasWarningTypeViolations(iouReportID, transactionViolations) ||

App/src/libs/TransactionUtils/index.ts

Lines 832 to 842 in 5e69bfc

	function hasWarningTypeViolation(transactionID: string, transactionViolations: OnyxCollection<TransactionViolation[]>): boolean {
	const warningTypeViolations = transactionViolations?.[ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS + transactionID]?.filter(
	(violation: TransactionViolation) => violation.type === CONST.VIOLATION_TYPES.WARNING,
	);
	const hasOnlyDupeDetectionViolation = warningTypeViolations?.every((violation: TransactionViolation) => violation.name === CONST.VIOLATIONS.DUPLICATED_TRANSACTION);
	if (!Permissions.canUseDupeDetection(allBetas ?? []) && hasOnlyDupeDetectionViolation) {
	return false;
	}
	return !!warningTypeViolations && warningTypeViolations.length > 0;
	}

However, on the individual expense preview, we just checks for the amount of the duplicates violation without checking whether the permission is enabled or not, which is different form hasWarningTypeViolations.

App/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx

Lines 147 to 149 in 5e69bfc

	const hasDuplicates = duplicates.length > 0;
	const shouldShowRBR = hasNoticeTypeViolations || hasViolations || hasFieldErrors || (!isFullySettled && !isFullyApproved && isOnHold) || hasDuplicates;

So, I think there are 2 issues here. In ReportPreview, we check for warning type violation. In MoneyRequestPreviewContent, we don't check for warning type violation, but we check for duplicates, ignoring the dupe detection permission.

For the RBR doesn't show for workspace chat in the LHN issue, we already have a logic to do that actually.

App/src/libs/ReportUtils.ts

Lines 6352 to 6356 in 5955bd7

	const allReports = Object.values(ReportConnection.getAllReports() ?? {}) as Report[];
	const potentialReports = allReports.filter((r) => r?.ownerAccountID === currentUserAccountID && (r?.stateNum ?? 0) <= 1 && r?.policyID === report.policyID);
	return potentialReports.some(
	(potentialReport) => hasViolations(potentialReport.reportID, transactionViolations) || hasWarningTypeViolations(potentialReport.reportID, transactionViolations),
	);

It checks all the related report and see if there are violations. However, hasWarningTypeViolations always returns false. It happens because the warningTypeViolations here is empty. Even though the dupe violation is a warning type, the showInReview is not null.

App/src/libs/TransactionUtils/index.ts

Lines 886 to 891 in 5955bd7

	function hasWarningTypeViolation(transactionID: string, transactionViolations: OnyxCollection<TransactionViolation[]>, showInReview?: boolean | null): boolean {
	const violations = transactionViolations?.[ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS + transactionID];
	const warningTypeViolations =
	violations?.filter(
	(violation: TransactionViolation) => violation.type === CONST.VIOLATION_TYPES.WARNING && (showInReview === null || showInReview === (violation.showInReview ?? false)),
	) ?? [];

It's because we pass nothing to showInReview. So, undefined !== null.

App/src/libs/ReportUtils.ts

Line 6355 in 5955bd7

(potentialReport) => hasViolations(potentialReport.reportID, transactionViolations) || hasWarningTypeViolations(potentialReport.reportID, transactionViolations),

App/src/libs/ReportUtils.ts

Lines 6370 to 6372 in 5955bd7

	function hasWarningTypeViolations(reportID: string, transactionViolations: OnyxCollection<TransactionViolation[]>, shouldShowInReview?: boolean): boolean {
	const transactions = reportsTransactions[reportID] ?? [];
	return transactions.some((transaction) => TransactionUtils.hasWarningTypeViolation(transaction.transactionID, transactionViolations, shouldShowInReview));

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

Let's use hasWarningTypeViolations in MoneyRequestPreviewContent too.

const hasWarningTypeViolations = TransactionUtils.hasWarningTypeViolation(transaction?.transactionID ?? '-1', transactionViolations);
...
const shouldShowRBR = hasNoticeTypeViolations || hasViolations || hasFieldErrors || (!isFullySettled && !isFullyApproved && isOnHold) || hasWarningTypeViolations;

(MoneyRequestPreviewContent also checks for hasNoticeTypeViolation but ReportPreview don't. We can add it to ReportPreview too if needed by adding it to ReportUtils, just like the others, but I don't know how to trigger notice type violation, so not sure.)

To fix the LHN RBR issue, we should first update the showInReview type by removing the null type. Then, check for undefined instead of null.

App/src/libs/TransactionUtils/index.ts

Lines 886 to 891 in 5955bd7

	function hasWarningTypeViolation(transactionID: string, transactionViolations: OnyxCollection<TransactionViolation[]>, showInReview?: boolean | null): boolean {
	const violations = transactionViolations?.[ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS + transactionID];
	const warningTypeViolations =
	violations?.filter(
	(violation: TransactionViolation) => violation.type === CONST.VIOLATION_TYPES.WARNING && (showInReview === null || showInReview === (violation.showInReview ?? false)),
	) ?? [];

(or we can just pass update showInReview to be a required param but keep it nullable)

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

@muttmuure Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot]

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

[melvin-bot]

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

[ahmedGaber93]

@bernhardoj Thanks for the proposal

Your proposal fix the issue on report preview but not on LHN. Can you also fix it?

20241115230948340.mp4

[bernhardoj]

I'm not sure if we should show the RBR on the workspace chat. In this PR, they mention that the RBR should show on the workspace chat, but this comment says otherwise.

NOTE: the previous logic before the PR also doesn't show expense report violations RBR on workspace chat.

[ahmedGaber93]

they mention that the RBR should show on the workspace chat, but this #51893 (comment) says otherwise.

I think they said it should show on some cases only

@iwiznia Just for confirmation, in the duplication violations case, should we show RBR on the workspace chat on LHN?

[iwiznia]

Yes, RBRs in the LHN should only show in the workspace chat

[ahmedGaber93]

Thanks for confirmation

[ahmedGaber93]

@bernhardoj Bump for this change #52243 (comment)

[bernhardoj]

Ok, I got it now. It's supposed to work but there is a small bug. Updated my proposal to fix that.

[melvin-bot]

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

[ahmedGaber93]

Updated my proposal to fix that.

Great, I will review it tomorrow

[melvin-bot]

@ahmedGaber93 @muttmuure 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!

[ahmedGaber93]

@bernhardoj's proposal LGTM!

🎀 👀 🎀 C+ reviewed

[bernhardoj]

PR is ready

cc: @ahmedGaber93

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

• Fix RBR is not shown correctly #52994

If no regressions arise, payment will be issued on 2024-12-05. 🎊

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

• @ahmedGaber93 requires payment automatic offer (Reviewer)
• @bernhardoj requires payment through NewDot Manual Requests

[melvin-bot]

@ahmedGaber93 @muttmuure @ahmedGaber93 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]

[ahmedGaber93]

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 (eg. bug slipped through the normal regression and PR testing process on staging)
• 2b. Reported on staging (eg. found during regression or PR testing)
• 2d. Reported on a PR
• 2z. Other:

Who reported the bug:

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

• [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/47056/files#r1876926777

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

There is no need for regression test because this issue related to dupe detection beta and it recently removed here #53167

[ahmedGaber93]

@muttmuure Could you please add the payment summary here? Also, please ignore the Upwork offer for me (I will request it on ND).

[bernhardoj]

Requested in ND.

[melvin-bot]

@AndrewGable, @ahmedGaber93, @muttmuure, @bernhardoj Eep! 4 days overdue now. Issues have feelings too...

[AndrewGable]

Just waiting on payment to close

[JmillsExpensify]

I need a payment summary to approve payment

[muttmuure]

@bernhardoj - $250 C+
@ahmedGaber93 - $250 C

[ahmedGaber93]

Requested in ND

[JmillsExpensify]

$250 approved for @ahmedGaber93

[JmillsExpensify]

$250 approved for @bernhardoj

[melvin-bot]

@AndrewGable, @ahmedGaber93, @muttmuure, @bernhardoj Huh... This is 4 days overdue. Who can take care of this?