From 4263359e8b6646a87c4272b54af21fb4b04f7935 Mon Sep 17 00:00:00 2001 From: krishna2323 Date: Fri, 11 Sep 2026 15:05:49 +0530 Subject: [PATCH] Gate the "Create a rule" offer on the expense it would create a rule for Signed-off-by: krishna2323 --- .../ReportActionItem/MoneyRequestView.tsx | 5 ++ src/libs/actions/IOU/UpdateMoneyRequest.ts | 79 +++++++++++++++++-- src/libs/actions/MerchantRuleSuggestion.ts | 74 ++++++++++++++--- src/libs/actions/TransactionInlineEdit.ts | 3 + .../step/DynamicIOURequestStepCategory.tsx | 1 + .../DynamicIOURequestStepCategoryCreate.tsx | 1 + .../step/DynamicIOURequestStepDescription.tsx | 1 + .../request/step/DynamicIOURequestStepTag.tsx | 1 + .../step/DynamicIOURequestStepTaxRatePage.tsx | 1 + 9 files changed, 151 insertions(+), 15 deletions(-) diff --git a/src/components/ReportActionItem/MoneyRequestView.tsx b/src/components/ReportActionItem/MoneyRequestView.tsx index 271a0c2cdda3..953fdbf8394f 100644 --- a/src/components/ReportActionItem/MoneyRequestView.tsx +++ b/src/components/ReportActionItem/MoneyRequestView.tsx @@ -700,6 +700,7 @@ function MoneyRequestView({ } updateMoneyRequestBillable({ transactionID: transaction.transactionID, + transaction, transactionThreadReport, parentReport, iouReportOwnerLogin, @@ -727,6 +728,7 @@ function MoneyRequestView({ } updateMoneyRequestReimbursable({ transactionID: transaction.transactionID, + transaction, transactionThreadReport, parentReport, iouReportOwnerLogin, @@ -868,6 +870,7 @@ function MoneyRequestView({ updateMoneyRequestTaxRate({ transactionID: transaction?.transactionID, + transaction, transactionThreadReport, parentReport, iouReportOwnerLogin, @@ -908,6 +911,7 @@ function MoneyRequestView({ updateMoneyRequestCategory({ transactionID, + transaction, transactionThreadReport, parentReport, iouReportOwnerLogin, @@ -949,6 +953,7 @@ function MoneyRequestView({ const updatedTag = insertTagIntoTransactionTagsString(transactionTag ?? '', '', tagListIndex, policy?.hasMultipleTagLists ?? false); updateMoneyRequestTag({ transactionID, + transaction, transactionThreadReport, parentReport, iouReportOwnerLogin, diff --git a/src/libs/actions/IOU/UpdateMoneyRequest.ts b/src/libs/actions/IOU/UpdateMoneyRequest.ts index 79ccc5421c94..af6a4861c771 100644 --- a/src/libs/actions/IOU/UpdateMoneyRequest.ts +++ b/src/libs/actions/IOU/UpdateMoneyRequest.ts @@ -373,6 +373,7 @@ function addMerchantRuleSuggestionRollback( /** Updates the billable field of an expense */ function updateMoneyRequestBillable({ transactionID, + transaction, transactionThreadReport, parentReport, iouReportOwnerLogin, @@ -392,6 +393,7 @@ function updateMoneyRequestBillable({ rules, }: { transactionID: string | undefined; + transaction?: OnyxEntry; transactionThreadReport: OnyxEntry; parentReport: OnyxEntry; iouReportOwnerLogin: string | undefined; @@ -438,11 +440,20 @@ function updateMoneyRequestBillable({ }); addMerchantRuleSuggestionRollback(onyxData, transactionID, CONST.MERCHANT_RULE_SUGGESTION_FIELDS.BILLABLE); API.write(WRITE_COMMANDS.UPDATE_MONEY_REQUEST_BILLABLE, params, onyxData); - trackMerchantRuleSuggestion(transactionID, CONST.MERCHANT_RULE_SUGGESTION_FIELDS.BILLABLE, transactionThreadReport.reportID, policy, policyCategories); + trackMerchantRuleSuggestion({ + transactionID, + field: CONST.MERCHANT_RULE_SUGGESTION_FIELDS.BILLABLE, + reportID: transactionThreadReport.reportID, + policy, + policyCategories, + transaction, + parentReport, + }); } function updateMoneyRequestReimbursable({ transactionID, + transaction, transactionThreadReport, parentReport, iouReportOwnerLogin, @@ -463,6 +474,7 @@ function updateMoneyRequestReimbursable({ rules, }: { transactionID: string | undefined; + transaction?: OnyxEntry; transactionThreadReport: OnyxEntry; parentReport: OnyxEntry; iouReportOwnerLogin: string | undefined; @@ -511,7 +523,15 @@ function updateMoneyRequestReimbursable({ }); addMerchantRuleSuggestionRollback(onyxData, transactionID, CONST.MERCHANT_RULE_SUGGESTION_FIELDS.REIMBURSABLE); API.write(WRITE_COMMANDS.UPDATE_MONEY_REQUEST_REIMBURSABLE, params, onyxData); - trackMerchantRuleSuggestion(transactionID, CONST.MERCHANT_RULE_SUGGESTION_FIELDS.REIMBURSABLE, transactionThreadReport.reportID, policy, policyCategories); + trackMerchantRuleSuggestion({ + transactionID, + field: CONST.MERCHANT_RULE_SUGGESTION_FIELDS.REIMBURSABLE, + reportID: transactionThreadReport.reportID, + policy, + policyCategories, + transaction, + parentReport, + }); } /** Updates the merchant field of an expense */ @@ -869,6 +889,8 @@ type UpdateMoneyRequestTagParams = { tag: string; /** Which level of a multi-level tag was edited, so the "Create a rule" callout can seed that level alone */ tagListIndex?: number; + /** Whether the edit came from a list of expenses, where the "Create a rule" callout has nowhere to appear */ + isEditedFromExpenseList?: boolean; policy: OnyxEntry; policyTagList: OnyxEntry; policyRecentlyUsedTags: OnyxEntry; @@ -896,6 +918,7 @@ function updateMoneyRequestTag({ iouReportOwnerLogin, tag, tagListIndex, + isEditedFromExpenseList, policy, policyTagList, policyRecentlyUsedTags, @@ -951,7 +974,17 @@ function updateMoneyRequestTag({ } addMerchantRuleSuggestionRollback(onyxData, transactionID, CONST.MERCHANT_RULE_SUGGESTION_FIELDS.TAG, editedTagLevels); API.write(WRITE_COMMANDS.UPDATE_MONEY_REQUEST_TAG, params, onyxData); - trackMerchantRuleSuggestion(transactionID, CONST.MERCHANT_RULE_SUGGESTION_FIELDS.TAG, transactionThreadReport?.reportID, policy, policyCategories, editedTagLevels); + trackMerchantRuleSuggestion({ + transactionID, + field: CONST.MERCHANT_RULE_SUGGESTION_FIELDS.TAG, + reportID: transactionThreadReport?.reportID, + policy, + policyCategories, + transaction, + parentReport, + editedTagLevels, + isEditedFromExpenseList, + }); } /** Updates the created tax amount of an expense */ @@ -1019,6 +1052,7 @@ function updateMoneyRequestTaxAmount({ type UpdateMoneyRequestTaxRateParams = { transactionID: string | undefined; + transaction?: OnyxEntry; transactionThreadReport: OnyxEntry; parentReport: OnyxEntry; iouReportOwnerLogin: string | undefined; @@ -1043,6 +1077,7 @@ type UpdateMoneyRequestTaxRateParams = { /** Updates the created tax rate of an expense */ function updateMoneyRequestTaxRate({ transactionID, + transaction, transactionThreadReport, parentReport, iouReportOwnerLogin, @@ -1091,7 +1126,15 @@ function updateMoneyRequestTaxRate({ addMerchantRuleSuggestionRollback(onyxData, transactionID, CONST.MERCHANT_RULE_SUGGESTION_FIELDS.TAX); API.write(WRITE_COMMANDS.UPDATE_MONEY_REQUEST_TAX_RATE, params, onyxData); - trackMerchantRuleSuggestion(transactionID, CONST.MERCHANT_RULE_SUGGESTION_FIELDS.TAX, transactionThreadReport?.reportID, policy, policyCategories); + trackMerchantRuleSuggestion({ + transactionID, + field: CONST.MERCHANT_RULE_SUGGESTION_FIELDS.TAX, + reportID: transactionThreadReport?.reportID, + policy, + policyCategories, + transaction, + parentReport, + }); } type UpdateMoneyRequestDistanceParams = { @@ -1273,6 +1316,7 @@ function updateMoneyRequestCategory({ parentReport, iouReportOwnerLogin, category, + isEditedFromExpenseList, policy, policyTagList, policyCategories, @@ -1295,6 +1339,8 @@ function updateMoneyRequestCategory({ parentReport: OnyxEntry; iouReportOwnerLogin: string | undefined; category: string; + /** Whether the edit came from a list of expenses, where the "Create a rule" callout has nowhere to appear */ + isEditedFromExpenseList?: boolean; policy: OnyxEntry; policyTagList: OnyxEntry; policyCategories: OnyxEntry; @@ -1340,7 +1386,16 @@ function updateMoneyRequestCategory({ }); addMerchantRuleSuggestionRollback(onyxData, transactionID, CONST.MERCHANT_RULE_SUGGESTION_FIELDS.CATEGORY); API.write(WRITE_COMMANDS.UPDATE_MONEY_REQUEST_CATEGORY, params, onyxData); - trackMerchantRuleSuggestion(transactionID, CONST.MERCHANT_RULE_SUGGESTION_FIELDS.CATEGORY, transactionThreadReport?.reportID, policy, policyCategories); + trackMerchantRuleSuggestion({ + transactionID, + field: CONST.MERCHANT_RULE_SUGGESTION_FIELDS.CATEGORY, + reportID: transactionThreadReport?.reportID, + policy, + policyCategories, + transaction, + parentReport, + isEditedFromExpenseList, + }); } /** Updates the description of an expense */ @@ -1351,6 +1406,7 @@ function updateMoneyRequestDescription({ parentReport, iouReportOwnerLogin, comment, + isEditedFromExpenseList, policy, policyTagList, policyCategories, @@ -1372,6 +1428,8 @@ function updateMoneyRequestDescription({ parentReport: OnyxEntry; iouReportOwnerLogin: string | undefined; comment: string; + /** Whether the edit came from a list of expenses, where the "Create a rule" callout has nowhere to appear */ + isEditedFromExpenseList?: boolean; policy: OnyxEntry; policyTagList: OnyxEntry; policyCategories: OnyxEntry; @@ -1432,7 +1490,16 @@ function updateMoneyRequestDescription({ params.description = parsedComment; addMerchantRuleSuggestionRollback(onyxData, transactionID, CONST.MERCHANT_RULE_SUGGESTION_FIELDS.DESCRIPTION); API.write(WRITE_COMMANDS.UPDATE_MONEY_REQUEST_DESCRIPTION, params, onyxData); - trackMerchantRuleSuggestion(transactionID, CONST.MERCHANT_RULE_SUGGESTION_FIELDS.DESCRIPTION, transactionThreadReport?.reportID, policy, policyCategories); + trackMerchantRuleSuggestion({ + transactionID, + field: CONST.MERCHANT_RULE_SUGGESTION_FIELDS.DESCRIPTION, + reportID: transactionThreadReport?.reportID, + policy, + policyCategories, + transaction, + parentReport, + isEditedFromExpenseList, + }); } /** Updates the distance rate of an expense */ diff --git a/src/libs/actions/MerchantRuleSuggestion.ts b/src/libs/actions/MerchantRuleSuggestion.ts index 47133762aa7c..b13079f442cc 100644 --- a/src/libs/actions/MerchantRuleSuggestion.ts +++ b/src/libs/actions/MerchantRuleSuggestion.ts @@ -1,13 +1,47 @@ import {arePolicyRulesEnabled, isControlPolicy} from '@libs/PolicyUtils'; +import {isInvoiceReport} from '@libs/ReportUtils'; +import {isDistanceRequest, isMerchantMissing, isPerDiemRequest} from '@libs/TransactionUtils'; import ONYXKEYS from '@src/ONYXKEYS'; -import type {MerchantRuleSuggestion, Policy, PolicyCategories} from '@src/types/onyx'; +import type {MerchantRuleSuggestion, Policy, PolicyCategories, Report, Transaction} from '@src/types/onyx'; import type {MerchantRuleSuggestionField} from '@src/types/onyx/MerchantRuleSuggestion'; import type {OnyxEntry, OnyxUpdate} from 'react-native-onyx'; import Onyx from 'react-native-onyx'; +type TrackMerchantRuleSuggestionParams = { + /** The edited expense */ + transactionID: string | undefined; + + /** The field that was edited */ + field: MerchantRuleSuggestionField; + + /** The edited expense's transaction thread, where the callout can show */ + reportID: string | undefined; + + /** The workspace that would own the rule */ + policy: OnyxEntry; + + /** That workspace's categories, needed to tell whether Rules are reachable at all */ + policyCategories: OnyxEntry; + + /** The edited expense itself, which decides whether a merchant rule could ever match it */ + transaction: OnyxEntry; + + /** The report holding the expense, which is what says the expense is really on this workspace */ + parentReport: OnyxEntry; + + /** Which levels of a multi-level tag were edited */ + editedTagLevels?: number[]; + + /** + * Whether the edit was made straight from a list of expenses, rather than from the expense itself. Such an edit + * records no offer, because the callout has nowhere to appear at the moment it is made. + */ + isEditedFromExpenseList?: boolean; +}; + /** * Records an edit that could become a merchant rule, so the expense can offer to create one. * @@ -19,14 +53,17 @@ import Onyx from 'react-native-onyx'; * the most recently edited expense offers. Recorded for anyone on the workspace; `useMerchantRuleSuggestion` decides * who actually sees the callout. */ -function trackMerchantRuleSuggestion( - transactionID: string | undefined, - field: MerchantRuleSuggestionField, - reportID: string | undefined, - policy: OnyxEntry, - policyCategories: OnyxEntry, - editedTagLevels?: number[], -) { +function trackMerchantRuleSuggestion({ + transactionID, + field, + reportID, + policy, + policyCategories, + transaction, + parentReport, + editedTagLevels, + isEditedFromExpenseList = false, +}: TrackMerchantRuleSuggestionParams) { // Skip workspaces that could not hold a merchant rule, otherwise an edit made with Rules off would surface the // moment somebody turned Rules on. Control only, matching the rule page the callout leads to, so an edit on a // Collect workspace does not pay for a write that could never be shown. @@ -34,6 +71,25 @@ function trackMerchantRuleSuggestion( return; } + // An offer nothing can show is an offer nobody asked for. Editing from a list of expenses leaves no expense detail + // on screen, so the record would sit there unseen and fire on whatever expense the user opened next. + if (isEditedFromExpenseList) { + return; + } + + // The policy handed in is the one whose fields the editor offered, which is not always the one that owns the + // expense. An expense held in a self DM borrows the workspace it would move to, so its edits must not be recorded + // against a workspace it has not reached. Invoices are excluded outright, since merchant rules govern expenses. + if (parentReport?.policyID !== policy?.id || isInvoiceReport(parentReport)) { + return; + } + + // A rule matches on merchant, so an expense that has none, or whose merchant is not the user's to set, can never + // be matched by the rule this offer would create. Distance and per diem expenses derive their merchant. + if (isMerchantMissing(transaction) || isDistanceRequest(transaction) || isPerDiemRequest(transaction)) { + return; + } + // Merged rather than set, so dismissals survive and `editedFields` accumulates. `isRetired` belongs to the offer // being replaced, so it is cleared: a new edit is a new offer. Onyx.merge(ONYXKEYS.RAM_ONLY_MERCHANT_RULE_SUGGESTION, { diff --git a/src/libs/actions/TransactionInlineEdit.ts b/src/libs/actions/TransactionInlineEdit.ts index 4c8f8fcc4afe..91a6b6d5b591 100644 --- a/src/libs/actions/TransactionInlineEdit.ts +++ b/src/libs/actions/TransactionInlineEdit.ts @@ -298,6 +298,7 @@ function editTransactionDescriptionInline(params: TransactionInlineEditParams, n ...iouParams, comment: newDescription, hash: params.hash, + isEditedFromExpenseList: true, }); } @@ -308,6 +309,7 @@ function editTransactionCategoryInline(params: TransactionInlineEditParams, newC ...iouParams, category: newCategory, hash: params.hash, + isEditedFromExpenseList: true, }); } @@ -357,6 +359,7 @@ function editTransactionTagInline(params: TransactionInlineEditParams, newTag: s policyRecentlyUsedTags: iouParams.policyRecentlyUsedTags, hash: params.hash, isOffline: params.isOffline, + isEditedFromExpenseList: true, }); } diff --git a/src/pages/iou/request/step/DynamicIOURequestStepCategory.tsx b/src/pages/iou/request/step/DynamicIOURequestStepCategory.tsx index 0f341b1be006..12365d518e7b 100644 --- a/src/pages/iou/request/step/DynamicIOURequestStepCategory.tsx +++ b/src/pages/iou/request/step/DynamicIOURequestStepCategory.tsx @@ -190,6 +190,7 @@ function DynamicIOURequestStepCategory({ if (isEditing && report) { updateMoneyRequestCategory({ transactionID: transaction.transactionID, + transaction, transactionThreadReport: report, parentReport, iouReportOwnerLogin, diff --git a/src/pages/iou/request/step/DynamicIOURequestStepCategoryCreate.tsx b/src/pages/iou/request/step/DynamicIOURequestStepCategoryCreate.tsx index d558ef2bf836..2f1f0e4f996b 100644 --- a/src/pages/iou/request/step/DynamicIOURequestStepCategoryCreate.tsx +++ b/src/pages/iou/request/step/DynamicIOURequestStepCategoryCreate.tsx @@ -163,6 +163,7 @@ function DynamicIOURequestStepCategoryCreate({ } else if (isEditing && report) { updateMoneyRequestCategory({ transactionID: transaction?.transactionID ?? transactionID, + transaction, transactionThreadReport: report, parentReport, iouReportOwnerLogin, diff --git a/src/pages/iou/request/step/DynamicIOURequestStepDescription.tsx b/src/pages/iou/request/step/DynamicIOURequestStepDescription.tsx index 64b011729667..fdbf480bd77f 100644 --- a/src/pages/iou/request/step/DynamicIOURequestStepDescription.tsx +++ b/src/pages/iou/request/step/DynamicIOURequestStepDescription.tsx @@ -154,6 +154,7 @@ function DynamicIOURequestStepDescription({ if (action === CONST.IOU.ACTION.EDIT) { updateMoneyRequestDescription({ transactionID: transaction?.transactionID, + transaction, transactionThreadReport: report, parentReport, iouReportOwnerLogin, diff --git a/src/pages/iou/request/step/DynamicIOURequestStepTag.tsx b/src/pages/iou/request/step/DynamicIOURequestStepTag.tsx index 4aaf1674848a..df30782005ac 100644 --- a/src/pages/iou/request/step/DynamicIOURequestStepTag.tsx +++ b/src/pages/iou/request/step/DynamicIOURequestStepTag.tsx @@ -172,6 +172,7 @@ function DynamicIOURequestStepTag({ if (isEditing) { updateMoneyRequestTag({ transactionID, + transaction, transactionThreadReport: report, parentReport, iouReportOwnerLogin, diff --git a/src/pages/iou/request/step/DynamicIOURequestStepTaxRatePage.tsx b/src/pages/iou/request/step/DynamicIOURequestStepTaxRatePage.tsx index 89a79a7fdeca..30e4442e4478 100644 --- a/src/pages/iou/request/step/DynamicIOURequestStepTaxRatePage.tsx +++ b/src/pages/iou/request/step/DynamicIOURequestStepTaxRatePage.tsx @@ -118,6 +118,7 @@ function DynamicIOURequestStepTaxRatePage({ taxCode: '', taxValue: '', taxAmount: 0, + transaction: currentTransaction, policy, policyTagList: policyTags, policyCategories,