diff --git a/src/CONST/index.ts b/src/CONST/index.ts index 225dce983496..908031dd127b 100644 --- a/src/CONST/index.ts +++ b/src/CONST/index.ts @@ -8502,6 +8502,7 @@ const CONST = { RULES: { SCOPE: { POLICY: 'policy', + ACCOUNT: 'account', }, APPROVAL_WORKFLOW: { TRIGGER: { @@ -8513,6 +8514,27 @@ const CONST = { APPROVE_REPORT: 'ApproveReport', }, }, + EXPENSE_DEFAULT: { + TRIGGER: { + CREATE_TRANSACTION: 'CreateTransaction', + }, + ACTION: { + SET: 'Set', + }, + /** Expense fields a `Set` action can write to */ + FIELD: { + BILLABLE: 'billable', + CATEGORY: 'category', + COMMENT: 'comment', + MERCHANT: 'merchant', + REIMBURSABLE: 'reimbursable', + TAG: 'tag', + TAX: 'tax', + VENDOR_ID: 'vendorID', + }, + /** Every expense default rule is created with the same priority, per the rules engine spec */ + PRIORITY: 10000, + }, }, BOOT_SPLASH_STATE: { diff --git a/src/ONYXKEYS.ts b/src/ONYXKEYS.ts index ffa163f75107..5ba2d68fa16c 100755 --- a/src/ONYXKEYS.ts +++ b/src/ONYXKEYS.ts @@ -492,6 +492,9 @@ const ONYXKEYS = { /** Set whether the search filters category data has loaded */ IS_SEARCH_FILTERS_CATEGORY_DATA_LOADED: 'isSearchFiltersCategoryDataLoaded', + /** Set once `GetRules` has answered, so screens that only consume the rules collection fetch it once */ + HAS_RULES_DATA_BEEN_FETCHED: 'hasRulesDataBeenFetched', + /** Set while search filter category data is loading */ RAM_ONLY_IS_LOADING_SEARCH_FILTERS_CATEGORY_DATA: 'isLoadingSearchFiltersCategoryData', @@ -1734,6 +1737,7 @@ type OnyxValuesMapping = { [ONYXKEYS.IS_LOADING_REPORT_DATA]: boolean; [ONYXKEYS.IS_SEARCH_FILTERS_CARD_DATA_LOADED]: boolean; [ONYXKEYS.IS_SEARCH_FILTERS_CATEGORY_DATA_LOADED]: boolean; + [ONYXKEYS.HAS_RULES_DATA_BEEN_FETCHED]: boolean; [ONYXKEYS.RAM_ONLY_IS_LOADING_SEARCH_FILTERS_CATEGORY_DATA]: boolean; [ONYXKEYS.IS_LOADING_SUBSCRIPTION_DATA]: boolean; [ONYXKEYS.IS_PENDING_UPDATE_PERSONAL_KARMA]: boolean; diff --git a/src/components/Tables/WorkspaceExpenseDefaultsTable/WorkspaceExpenseDefaultsTableRow.tsx b/src/components/Tables/WorkspaceExpenseDefaultsTable/WorkspaceExpenseDefaultsTableRow.tsx index de0d69719442..7e602700a3a2 100644 --- a/src/components/Tables/WorkspaceExpenseDefaultsTable/WorkspaceExpenseDefaultsTableRow.tsx +++ b/src/components/Tables/WorkspaceExpenseDefaultsTable/WorkspaceExpenseDefaultsTableRow.tsx @@ -106,7 +106,7 @@ function WorkspaceExpenseDefaultsTableRow({item, rowIndex, shouldUseNarrowTableL { + if (!isFetchNeeded) { + return; + } + getRules(); + }, [isFetchNeeded]); +} + +export default useRulesPrefetch; diff --git a/src/libs/API/parameters/DeleteRuleParams.ts b/src/libs/API/parameters/DeleteRuleParams.ts new file mode 100644 index 000000000000..eca99e59ccdb --- /dev/null +++ b/src/libs/API/parameters/DeleteRuleParams.ts @@ -0,0 +1,6 @@ +type DeleteRuleParams = { + /** The ID of the rule to delete */ + ruleID: string; +}; + +export default DeleteRuleParams; diff --git a/src/libs/API/parameters/SetRuleParams.ts b/src/libs/API/parameters/SetRuleParams.ts new file mode 100644 index 000000000000..4a37391c4ec9 --- /dev/null +++ b/src/libs/API/parameters/SetRuleParams.ts @@ -0,0 +1,21 @@ +type SetRuleParams = { + /** What kind of entity the rule is scoped to. Merchant rules are always scoped to a policy */ + scope: string; + + /** The ID of the scoped entity, i.e. the policyID for policy-scoped rules */ + scopeID: string; + + /** The ID of the rule being written. A new rule uses an optimistic `rand64()` value */ + ruleID: string; + + /** Determines the order rules are applied in when more than one matches */ + priority: number; + + /** The `{filters, triggers, actions}` body of the rule, stringified */ + value: string; + + /** Whether to apply the rule to the transactions that already match it */ + shouldUpdateMatchingTransactions: boolean; +}; + +export default SetRuleParams; diff --git a/src/libs/API/parameters/index.ts b/src/libs/API/parameters/index.ts index 839171906e8a..7f43a989f344 100644 --- a/src/libs/API/parameters/index.ts +++ b/src/libs/API/parameters/index.ts @@ -608,6 +608,8 @@ export type {default as DeleteDomainParams} from './DeleteDomainParams'; export type {default as GetDuplicateTransactionDetailsParams} from './GetDuplicateTransactionDetailsParams'; export type {default as SetPolicyCategoryReceiptsAndItemizedReceiptRequiredParams} from './SetPolicyCategoryReceiptsAndItemizedReceiptRequiredParams'; export type {default as SetPolicyCodingRuleParams} from './SetPolicyCodingRuleParams'; +export type {default as SetRuleParams} from './SetRuleParams'; +export type {default as DeleteRuleParams} from './DeleteRuleParams'; export type {default as SetApprovalWorkflowParams} from './SetApprovalWorkflowParams'; export type {default as RegisterAuthenticationKeyParams} from './RegisterAuthenticationKeyParams'; export type {default as RevokeMultifactorAuthenticationCredentialsParams} from './RevokeMultifactorAuthenticationCredentialsParams'; diff --git a/src/libs/API/types.ts b/src/libs/API/types.ts index 381e45f39f2b..e983b9bbd894 100644 --- a/src/libs/API/types.ts +++ b/src/libs/API/types.ts @@ -285,6 +285,8 @@ const WRITE_COMMANDS = { SET_POLICY_TIME_TRACKING_DEFAULT_RATE: 'SetPolicyTimeTrackingDefaultRate', SET_POLICY_RULES_ENABLED: 'SetPolicyRulesEnabled', SET_POLICY_CODING_RULE: 'SetPolicyCodingRule', + SET_RULE: 'SetRule', + DELETE_RULE: 'DeleteRule', SET_APPROVAL_WORKFLOW: 'SetApprovalWorkflow', SET_POLICY_EXPENSE_MAX_AMOUNT_NO_RECEIPT: 'SetPolicyExpenseMaxAmountNoReceipt', SET_POLICY_EXPENSE_MAX_AMOUNT_NO_ITEMIZED_RECEIPT: 'SetPolicyExpenseMaxAmountNoItemizedReceipt', @@ -998,6 +1000,8 @@ type WriteCommandParameters = { [WRITE_COMMANDS.ENABLE_POLICY_TIME_TRACKING]: Parameters.EnablePolicyTimeTrackingParams; [WRITE_COMMANDS.SET_POLICY_RULES_ENABLED]: Parameters.SetPolicyRulesEnabledParams; [WRITE_COMMANDS.SET_POLICY_CODING_RULE]: Parameters.SetPolicyCodingRuleParams; + [WRITE_COMMANDS.SET_RULE]: Parameters.SetRuleParams; + [WRITE_COMMANDS.DELETE_RULE]: Parameters.DeleteRuleParams; [WRITE_COMMANDS.SET_APPROVAL_WORKFLOW]: Parameters.SetApprovalWorkflowParams; [WRITE_COMMANDS.SET_POLICY_REQUIRE_COMPANY_CARDS_ENABLED]: Parameters.SetPolicyRequireCompanyCardsEnabledParams; [WRITE_COMMANDS.SET_POLICY_CATEGORY_DESCRIPTION_REQUIRED]: Parameters.SetPolicyCategoryDescriptionRequiredParams; @@ -1514,6 +1518,7 @@ const READ_COMMANDS = { OPEN_POLICY_REPORT_FIELDS_PAGE: 'OpenPolicyReportFieldsPage', OPEN_POLICY_INVOICES_PAGE: 'OpenPolicyInvoicesPage', OPEN_POLICY_RULES_PAGE: 'OpenPolicyRulesPage', + GET_RULES: 'GetRules', OPEN_POLICY_EXPENSIFY_CARDS_PAGE: 'OpenPolicyExpensifyCardsPage', OPEN_POLICY_TRAVEL_PAGE: 'OpenPolicyTravelPage', GET_TRAVEL_BILLING_STATEMENT_PDF: 'GetTravelBillingStatementPDF', @@ -1628,6 +1633,7 @@ type ReadCommandParameters = { [READ_COMMANDS.OPEN_POLICY_REPORT_FIELDS_PAGE]: Parameters.OpenPolicyReportFieldsPageParams; [READ_COMMANDS.OPEN_POLICY_INVOICES_PAGE]: Parameters.OpenPolicyReportFieldsPageParams; [READ_COMMANDS.OPEN_POLICY_RULES_PAGE]: Parameters.OpenPolicyRulesPageParams; + [READ_COMMANDS.GET_RULES]: EmptyObject; [READ_COMMANDS.OPEN_WORKSPACE_INVITE_PAGE]: Parameters.OpenWorkspaceInvitePageParams; [READ_COMMANDS.OPEN_DRAFT_WORKSPACE_REQUEST]: Parameters.OpenDraftWorkspaceRequestParams; [READ_COMMANDS.OPEN_DRAFT_PER_DIEM_EXPENSE]: Parameters.OpenDraftPerDiemExpenseParams; diff --git a/src/libs/ExpenseDefaultRuleUtils.ts b/src/libs/ExpenseDefaultRuleUtils.ts new file mode 100644 index 000000000000..ae8af2bbd1c3 --- /dev/null +++ b/src/libs/ExpenseDefaultRuleUtils.ts @@ -0,0 +1,476 @@ +/** + * Helpers for the expense default rules stored in the `rules_` collection, which is what the merchant rule + * editor reads and writes. Converts between the rules engine's filter tree and the flat form the editor + * uses, and reports the rules the form can't represent so they stay read-only instead of losing data on save. + */ +import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; +import type {Policy, Rule} from '@src/types/onyx'; +import type {ApprovalWorkflowAction} from '@src/types/onyx/ApprovalWorkflowRules'; +import type { + ExpenseDefaultAction, + ExpenseDefaultActionField, + ExpenseDefaultActions, + ExpenseDefaultRule, + ExpenseDefaultTaxValue, + ExpenseDefaultTriggers, +} from '@src/types/onyx/ExpenseDefaultRules'; +import type {RuleFilterComparison, RuleFilterNode} from '@src/types/onyx/RuleFilters'; + +import type {OnyxCollection} from 'react-native-onyx'; +import type {ValueOf} from 'type-fest'; + +import {rand64} from './NumberUtils'; +import Parser from './Parser'; +import {toIndexMap} from './WorkflowUtils'; + +/** The form shape the merchant rule editor round-trips a rule through. */ +type MerchantRuleFormValues = { + /** The merchant string an expense has to match */ + merchantToMatch: string; + + /** Whether the merchant has to match exactly (`eq`) or partially (`contains`) */ + matchType: ValueOf; + + /** The merchant the expense is renamed to */ + merchant?: string; + + /** The category set on the expense */ + category?: string; + + /** The tag set on the expense */ + tag?: string; + + /** The external ID of the tax rate set on the expense */ + tax?: string; + + /** The external ID of the vendor set on the expense */ + vendorID?: string; + + /** The description set on the expense, as markdown */ + comment?: string; + + /** Whether the expense is reimbursable */ + reimbursable?: boolean; + + /** Whether the expense is billable */ + billable?: boolean; +}; + +/** A rule from the `rules_` collection together with the ID parsed out of its Onyx key. */ +type RuleWithID = { + /** The rule's ID, i.e. the `rules_` key suffix */ + ruleID: string; + + /** The rule itself, narrowed to the expense default shape by `getPolicyExpenseDefaultRules` */ + rule: Rule & ExpenseDefaultRule; +}; + +const {FIELD, TRIGGER, ACTION} = CONST.RULES.EXPENSE_DEFAULT; +const {EQUAL_TO, CONTAINS} = CONST.SEARCH.SYNTAX_OPERATORS; + +/** The order actions are written in, which fixes the numeric keys a built rule uses. */ +const ACTION_FIELD_ORDER = [FIELD.MERCHANT, FIELD.CATEGORY, FIELD.TAG, FIELD.TAX, FIELD.VENDOR_ID, FIELD.COMMENT, FIELD.REIMBURSABLE, FIELD.BILLABLE] as const; + +/** Merchant match types the editor can represent. Any other operator on the merchant node makes a rule read-only. */ +const SUPPORTED_MERCHANT_MATCH_TYPES = new Set>([EQUAL_TO, CONTAINS]); + +const STRING_ACTION_FIELDS = new Set([FIELD.MERCHANT, FIELD.CATEGORY, FIELD.TAG, FIELD.VENDOR_ID, FIELD.COMMENT]); +const BOOLEAN_ACTION_FIELDS = new Set([FIELD.REIMBURSABLE, FIELD.BILLABLE]); + +/** The rule format has no notion of an empty value: a field the admin cleared is simply not set. */ +function emptyToUndefined(value: string | undefined): string | undefined { + return value?.trim() ? value : undefined; +} + +/** Lists a rule's actions keyed by their stringified index. Rules of other kinds (approval workflows) carry actions of a different shape. */ +function getRuleActionEntries(rule: Rule | ExpenseDefaultRule | undefined): Array<[string, ExpenseDefaultAction | ApprovalWorkflowAction]> { + if (!rule?.actions) { + return []; + } + const actions: Record = rule.actions; + return Object.entries(actions); +} + +/** Lists a rule's actions. See `getRuleActionEntries`. */ +function getRuleActions(rule: Rule | ExpenseDefaultRule | undefined): Array { + return getRuleActionEntries(rule).map(([, action]) => action); +} + +function isRuleFilterNode(value: unknown): value is RuleFilterNode { + return !!value && typeof value === 'object' && 'left' in value && 'operator' in value && 'right' in value; +} + +/** A leaf node compares a single field: its `left` is a field name rather than another node. */ +function isRuleFilterComparison(node: RuleFilterNode): node is RuleFilterComparison { + return typeof node.left === 'string'; +} + +/** + * A rule is an expense default rule when it runs on transaction creation and changes at least one field. + * This is what decides whether a rule shows up in the workspace's expense defaults list. + */ +function isExpenseDefaultRule(rule: Rule | undefined): rule is Rule & ExpenseDefaultRule { + if (!rule) { + return false; + } + + const hasCreateTransactionTrigger = Object.values(rule.triggers ?? {}).some((trigger) => trigger === TRIGGER.CREATE_TRANSACTION); + const hasSetAction = getRuleActions(rule).some((action) => action?.name === ACTION.SET); + + return hasCreateTransactionTrigger && hasSetAction; +} + +/** `GetRules` returns every rule the user can see, so callers have to narrow the collection to one policy themselves. */ +function isPolicyScopedRule(rule: Rule | undefined, policyID: string | undefined): boolean { + return !!rule && !!policyID && rule.scope === CONST.RULES.SCOPE.POLICY && rule.scopeID === policyID; +} + +/** Returns the policy's expense default rules, with each rule's ID parsed out of its Onyx key. */ +function getPolicyExpenseDefaultRules(rulesCollection: OnyxCollection | undefined, policyID: string | undefined): RuleWithID[] { + if (!policyID) { + return []; + } + + const rules: RuleWithID[] = []; + + for (const [onyxKey, rule] of Object.entries(rulesCollection ?? {})) { + if (!rule || !isPolicyScopedRule(rule, policyID) || !isExpenseDefaultRule(rule)) { + continue; + } + rules.push({ruleID: onyxKey.slice(ONYXKEYS.COLLECTION.RULE.length), rule}); + } + + return rules; +} + +/** Builds the `tax` action value, which carries the rate's name and value alongside its external ID for display. */ +function buildTaxActionValue(taxKey: string | undefined, policy: Policy | undefined): ExpenseDefaultTaxValue | undefined { + if (!taxKey) { + return undefined; + } + + const tax = policy?.taxRates?.taxes?.[taxKey]; + + return { + // field_id_TAX is the name the rules engine gives this key, so it can't follow our casing convention + // eslint-disable-next-line @typescript-eslint/naming-convention + field_id_TAX: { + externalID: taxKey, + ...(tax ? {value: tax.value, name: tax.name} : {}), + }, + }; +} + +/** Builds the filter tree for a merchant rule: a single `merchant eq|contains ` comparison. */ +function buildMerchantRuleFilters(formValues: Partial): RuleFilterComparison | undefined { + const merchantToMatch = formValues.merchantToMatch?.trim(); + if (!merchantToMatch) { + return undefined; + } + + return { + left: FIELD.MERCHANT, + operator: formValues.matchType ?? CONTAINS, + right: merchantToMatch, + }; +} + +/** Builds the `Set` actions for a merchant rule, keyed by a stringified index in a fixed field order. */ +function buildMerchantRuleActions(formValues: Partial, policy: Policy | undefined): ExpenseDefaultActions { + const comment = emptyToUndefined(formValues.comment); + const valuesByField: Partial> = { + [FIELD.MERCHANT]: emptyToUndefined(formValues.merchant), + [FIELD.CATEGORY]: emptyToUndefined(formValues.category), + [FIELD.TAG]: emptyToUndefined(formValues.tag), + [FIELD.TAX]: buildTaxActionValue(formValues.tax, policy), + [FIELD.VENDOR_ID]: emptyToUndefined(formValues.vendorID), + [FIELD.COMMENT]: comment ? Parser.replace(comment) : undefined, + [FIELD.REIMBURSABLE]: formValues.reimbursable, + [FIELD.BILLABLE]: formValues.billable, + }; + + const actions: ExpenseDefaultAction[] = []; + + for (const field of ACTION_FIELD_ORDER) { + const value = valuesByField[field]; + if (value === undefined) { + continue; + } + actions.push({name: ACTION.SET, field, value}); + } + + return toIndexMap(actions); +} + +/** + * Builds the rule body sent as the `value` param of `SetRule`. + * Returns undefined when the form has nothing to match on or nothing to set, which the API rejects. + */ +function buildMerchantRule(formValues: Partial, policy: Policy | undefined): ExpenseDefaultRule | undefined { + const filters = buildMerchantRuleFilters(formValues); + const actions = buildMerchantRuleActions(formValues, policy); + + if (!filters || Object.keys(actions).length === 0) { + return undefined; + } + + const triggers: ExpenseDefaultTriggers = toIndexMap([TRIGGER.CREATE_TRANSACTION]); + + return {triggers, filters, actions}; +} + +/** A tax action value is the only non-primitive value a `Set` action can carry. */ +function isExpenseDefaultTaxValue(value: unknown): value is ExpenseDefaultTaxValue { + if (!value || typeof value !== 'object' || !('field_id_TAX' in value)) { + return false; + } + + const taxField: unknown = value.field_id_TAX; + return !!taxField && typeof taxField === 'object' && 'externalID' in taxField && typeof taxField.externalID === 'string'; +} + +/** Every trigger has to be one the editor knows about, otherwise saving the form would drop the rest. */ +function areTriggersEditable(triggers: Record | undefined): boolean { + const triggerValues = Object.values(triggers ?? {}); + return triggerValues.length > 0 && triggerValues.every((trigger) => trigger === TRIGGER.CREATE_TRANSACTION); +} + +/** The editor matches on exactly one merchant condition, so anything else (a tree, another field, a list) can't be shown in the form. */ +function getEditableMerchantMatch(filters: RuleFilterNode | undefined): Pick | undefined { + if (!filters || !isRuleFilterNode(filters) || !isRuleFilterComparison(filters)) { + return undefined; + } + + if (filters.left !== FIELD.MERCHANT || !SUPPORTED_MERCHANT_MATCH_TYPES.has(filters.operator)) { + return undefined; + } + + // The backend ORs a list of values together. The form only has one merchant input, so only a single value round-trips. + const rightValues = [filters.right].flat(); + const merchantToMatch = rightValues.at(0); + if (rightValues.length !== 1 || typeof merchantToMatch !== 'string' || !merchantToMatch) { + return undefined; + } + + return {merchantToMatch, matchType: filters.operator}; +} + +/** + * Converts a stored rule back into the values the merchant rule editor renders. + * + * Returns undefined when the rule can't be represented by the form, such as a nested filter tree, a filter on a + * field the form has no input for, an unknown trigger or action, or two actions writing the same field. + * Callers MUST treat undefined as "show this rule read-only": rendering a partial form and saving it back + * would silently drop everything the form couldn't represent. + */ +function getMerchantRuleFormValues(rule: Rule | ExpenseDefaultRule | undefined): MerchantRuleFormValues | undefined { + if (!rule || !areTriggersEditable(rule.triggers)) { + return undefined; + } + + const merchantMatch = getEditableMerchantMatch(rule.filters); + if (!merchantMatch) { + return undefined; + } + + const formValues: MerchantRuleFormValues = {...merchantMatch}; + + const actions = getRuleActions(rule); + if (actions.length === 0) { + return undefined; + } + + const seenFields = new Set(); + + for (const action of actions) { + // An action the form can't produce - a non-`Set` action, an unknown field, or a second action on a + // field the form has a single input for - means saving the form would drop it. + if (!action || action.name !== ACTION.SET || !('field' in action) || !('value' in action) || seenFields.has(action.field)) { + return undefined; + } + seenFields.add(action.field); + + const {field, value} = action; + + if (STRING_ACTION_FIELDS.has(field)) { + if (typeof value !== 'string') { + return undefined; + } + if (field === FIELD.COMMENT) { + formValues.comment = Parser.htmlToMarkdown(value); + } else if (field === FIELD.MERCHANT) { + formValues.merchant = value; + } else if (field === FIELD.CATEGORY) { + formValues.category = value; + } else if (field === FIELD.TAG) { + formValues.tag = value; + } else { + formValues.vendorID = value; + } + continue; + } + + if (BOOLEAN_ACTION_FIELDS.has(field)) { + if (typeof value !== 'boolean') { + return undefined; + } + if (field === FIELD.REIMBURSABLE) { + formValues.reimbursable = value; + } else { + formValues.billable = value; + } + continue; + } + + if (field === FIELD.TAX) { + if (!isExpenseDefaultTaxValue(value)) { + return undefined; + } + formValues.tax = value.field_id_TAX.externalID; + continue; + } + + return undefined; + } + + return formValues; +} + +/** + * Flattens a filter tree into its leaf comparisons, left to right. Used to summarize rules the editor + * can't open, which still have to render a readable condition in the rules list. + */ +function getRuleFilterLeaves(filters: RuleFilterNode | undefined): RuleFilterComparison[] { + if (!filters || !isRuleFilterNode(filters)) { + return []; + } + + if (isRuleFilterComparison(filters)) { + return [filters]; + } + + return [...getRuleFilterLeaves(filters.left), ...getRuleFilterLeaves(filters.right)]; +} + +/** + * Summarizes the merchants a rule matches on, for the condition text and the search index. + * + * Derived from the filter tree rather than from `getMerchantRuleFormValues`, so a rule the editor can't + * represent still shows what it matches and can still be found by merchant while staying read-only. + */ +function getRuleMerchantMatchSummary(filters: RuleFilterNode | undefined): {merchants: string; isExactMatch: boolean} { + const merchantLeaves = getRuleFilterLeaves(filters).filter((leaf) => leaf.left === FIELD.MERCHANT); + + return { + merchants: merchantLeaves.flatMap((leaf) => [leaf.right].flat()).join(', '), + isExactMatch: merchantLeaves.length > 0 && merchantLeaves.every((leaf) => leaf.operator === EQUAL_TO), + }; +} + +/** A single field a rule sets, normalized for display. */ +type ExpenseDefaultRuleSummaryField = { + /** The expense field being set */ + field: ExpenseDefaultActionField; + + /** The value the field is set to. `comment` is converted back to markdown; `tax` keeps its object shape */ + value: ExpenseDefaultAction['value']; +}; + +/** + * Lists the fields a rule sets, in action-key order. Works for any rule, including ones the editor + * can't open, so the rules list can summarize them without going through the form. + */ +function getExpenseDefaultRuleSummaryFields(rule: Rule | ExpenseDefaultRule | undefined): ExpenseDefaultRuleSummaryField[] { + // Keys are stringified indexes, so "10" has to sort after "2" rather than before it. + const sortedEntries = getRuleActionEntries(rule).sort(([leftKey], [rightKey]) => Number(leftKey) - Number(rightKey)); + + const summaryFields: ExpenseDefaultRuleSummaryField[] = []; + + for (const [, action] of sortedEntries) { + if (!action || action.name !== ACTION.SET || !('field' in action) || !('value' in action)) { + continue; + } + summaryFields.push({ + field: action.field, + value: action.field === FIELD.COMMENT && typeof action.value === 'string' ? Parser.htmlToMarkdown(action.value) : action.value, + }); + } + + return summaryFields; +} + +/** + * Builds copies of a policy's expense default rules for another policy, as `ruleID -> rule`. + * + * Rules live in their own collection and carry the ID of the policy they belong to, so copying a workspace + * can't reuse the source's rules: each copy is a new rule, with a new ID, scoped to the target policy. + */ +function buildCopiedExpenseDefaultRules(rules: OnyxCollection | undefined, sourcePolicyID: string | undefined, targetPolicyID: string): Record { + const copiedRules: Record = {}; + const created = new Date().toISOString(); + + for (const {rule} of getPolicyExpenseDefaultRules(rules, sourcePolicyID)) { + if (rule.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE) { + continue; + } + + copiedRules[rand64()] = { + triggers: rule.triggers, + filters: rule.filters, + actions: rule.actions, + scope: CONST.RULES.SCOPE.POLICY, + scopeID: targetPolicyID, + priority: rule.priority ?? CONST.RULES.EXPENSE_DEFAULT.PRIORITY, + created, + pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD, + }; + } + + return copiedRules; +} + +/** How many expense default rules the policy has, ignoring ones being deleted. Used by the copy/duplicate feature lists. */ +function getExpenseDefaultRuleCount(rules: OnyxCollection | undefined, policyID: string | undefined): number { + return getPolicyExpenseDefaultRules(rules, policyID).filter(({rule}) => rule.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE).length; +} + +/** Whether any of the policy's expense default rules failed to save. */ +function hasExpenseDefaultRuleErrors(rules: OnyxCollection | undefined, policyID: string | undefined): boolean { + return getPolicyExpenseDefaultRules(rules, policyID).some(({rule}) => Object.keys(rule.errors ?? {}).length > 0); +} + +/** Whether the merchant rule editor can safely open this rule. See `getMerchantRuleFormValues`. */ +function isEditableMerchantRule(rule: Rule | ExpenseDefaultRule | undefined): boolean { + return !!getMerchantRuleFormValues(rule); +} + +/** + * Whether the merchant rule editor is allowed to open this rule for the given policy. + * + * The `rules_` collection holds every kind of rule for every workspace the user can see, so a ruleID from a + * stale link or a bookmark can resolve to another policy's rule or to an approval workflow. Saving from the + * editor writes a freshly built merchant rule over that same ruleID, so anything it can't represent has to be + * refused rather than opened. A rule the form can edit is already an expense default rule, because the form only + * accepts `CreateTransaction` triggers and `Set` actions. + */ +function canEditMerchantRule(rule: Rule | undefined, policyID: string | undefined): boolean { + return isPolicyScopedRule(rule, policyID) && isEditableMerchantRule(rule); +} + +export type {MerchantRuleFormValues}; +export { + buildCopiedExpenseDefaultRules, + canEditMerchantRule, + buildMerchantRule, + getExpenseDefaultRuleCount, + getExpenseDefaultRuleSummaryFields, + getMerchantRuleFormValues, + getPolicyExpenseDefaultRules, + getRuleFilterLeaves, + getRuleMerchantMatchSummary, + hasExpenseDefaultRuleErrors, + isEditableMerchantRule, + isExpenseDefaultRule, + isExpenseDefaultTaxValue, +}; diff --git a/src/libs/ExportOnyxState/common.ts b/src/libs/ExportOnyxState/common.ts index 4971d9f0f545..e8cbc1711e34 100644 --- a/src/libs/ExportOnyxState/common.ts +++ b/src/libs/ExportOnyxState/common.ts @@ -227,6 +227,7 @@ const safeOnyxKeys = new Set([ ONYXKEYS.HAS_LOADED_APP, ONYXKEYS.HAS_MORE_UNREPORTED_TRANSACTIONS_RESULTS, ONYXKEYS.HAS_NON_PERSONAL_POLICY, + ONYXKEYS.HAS_RULES_DATA_BEEN_FETCHED, ONYXKEYS.IMPORTED_SPREADSHEET_MEMBER_ROLE, ONYXKEYS.INPUT_FOCUSED, ONYXKEYS.IS_CHANGING_TO_NEW_BANK_ACCOUNT, diff --git a/src/libs/MerchantTypeRulesUtils.ts b/src/libs/MerchantTypeRulesUtils.ts index 9cc18a0e8f47..ce4457ad12ec 100644 --- a/src/libs/MerchantTypeRulesUtils.ts +++ b/src/libs/MerchantTypeRulesUtils.ts @@ -6,15 +6,16 @@ import ROUTES from '@src/ROUTES'; import type {Route} from '@src/ROUTES'; import INPUT_IDS from '@src/types/form/MerchantTypeRuleForm'; import type {MerchantTypeRuleForm} from '@src/types/form/MerchantTypeRuleForm'; -import type {Policy, PolicyCategories} from '@src/types/onyx'; -import type {CodingRule} from '@src/types/onyx/Policy'; +import type {Policy, PolicyCategories, Rule} from '@src/types/onyx'; + +import type {OnyxCollection} from 'react-native-onyx'; import {DEFAULT_MCC_GROUP, isDefaultMccGroupID} from './actions/Policy/Category'; import {setWorkspaceDefaultSpendCategory} from './actions/Policy/Policy'; -import {clearPolicyCodingRuleErrors} from './actions/Policy/Rules'; +import {clearMerchantRuleErrors} from './actions/Policy/Rules'; import {getCategoryTaxRulesTableData, getTaxRateDisplayName} from './CategoryTaxRulesUtils'; import {getDecodedCategoryName} from './CategoryUtils'; -import Parser from './Parser'; +import {getExpenseDefaultRuleSummaryFields, getPolicyExpenseDefaultRules, getRuleMerchantMatchSummary, isEditableMerchantRule, isExpenseDefaultTaxValue} from './ExpenseDefaultRuleUtils'; import {getMccGroupDisplayName} from './PolicyRulesUtils'; import {getCommaSeparatedTagNameWithSanitizedColons, getVendorRuleDisplayValue, isXeroActiveMatchingSource} from './PolicyUtils'; @@ -97,22 +98,24 @@ function getMerchantTypeRulesTableData({ }); } -function getMerchantCodingRulesTableData({ +function getMerchantRulesTableData({ policy, policyID, + rules, translate, isOffline, onNavigate, }: { policy: Policy | undefined; policyID: string; + rules: OnyxCollection | undefined; translate: LocaleContextProps['translate']; isOffline: boolean; onNavigate: (route: Route) => void; }): ExpenseDefaultTableItem[] { - const codingRules = policy?.rules?.codingRules; + const policyRules = getPolicyExpenseDefaultRules(rules, policyID); - if (!codingRules) { + if (policyRules.length === 0) { return []; } @@ -124,76 +127,69 @@ function getMerchantCodingRulesTableData({ tax: translate('common.tax').toLowerCase(), vendor: translate(isOnXero ? 'common.supplier' : 'common.vendor').toLowerCase(), }; + const {FIELD} = CONST.RULES.EXPENSE_DEFAULT; + + return policyRules + .filter(({rule}) => isOffline || rule.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE) + .sort((first, second) => ((second.rule.created ?? '') < (first.rule.created ?? '') ? -1 : 1)) + .map(({ruleID, rule}) => { + const summaryFields = getExpenseDefaultRuleSummaryFields(rule); + const {merchants: merchantName} = getRuleMerchantMatchSummary(rule.filters); - // A merchant rule outlives the category or tax rate it sets — the backend keeps the rule and only drops that one - // default — so no pending state is borrowed here. Only the rule's own delete counts, and it reads the same way as - // a category rule's: online the delete resolves in a moment, so the row goes rather than flashing greyed, while - // offline it stays and is styled as deleting since there is nothing to wait for. - return Object.entries(codingRules) - .filter(([, rule]) => !!rule && (isOffline || rule.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE)) - .map(([ruleID, rule]: [string, CodingRule]) => { - const merchantName = rule.filters?.right ?? ''; - const hasOnlyMerchantRename = - !!rule.merchant && - !rule.category && - !rule.tag && - !rule.comment && - !rule.tax?.field_id_TAX?.value && - !rule.vendorID && - rule.reimbursable === undefined && - rule.billable === undefined; + const hasOnlyMerchantRename = summaryFields.length === 1 && summaryFields.at(0)?.field === FIELD.MERCHANT; const typeLabel = hasOnlyMerchantRename ? translate('workspace.rules.expenseDefaultsTable.rename') : translate('workspace.rules.expenseDefaultsTable.update'); const actions: string[] = []; - if (rule.merchant) { - actions.push(translate('workspace.rules.merchantRules.ruleSummarySubtitleMerchant', rule.merchant)); - } - if (rule.category) { - actions.push(translate('workspace.rules.merchantRules.ruleSummarySubtitleUpdateField', fieldLabels.category, getDecodedCategoryName(rule.category))); - } - if (rule.tag) { - actions.push(translate('workspace.rules.merchantRules.ruleSummarySubtitleUpdateField', fieldLabels.tag, getCommaSeparatedTagNameWithSanitizedColons(rule.tag))); - } - if (rule.comment) { - const commentMarkdown = Parser.htmlToMarkdown(rule.comment); - actions.push(translate('workspace.rules.merchantRules.ruleSummarySubtitleUpdateField', fieldLabels.description, commentMarkdown)); - } - if (rule.tax?.field_id_TAX?.value) { - actions.push( - translate( - 'workspace.rules.merchantRules.ruleSummarySubtitleUpdateField', - fieldLabels.tax, - getTaxRateDisplayName(policy, rule.tax.field_id_TAX.externalID, rule.tax.field_id_TAX), - ), - ); - } - if (rule.vendorID) { - const unavailableLabel = translate(isOnXero ? 'workspace.rules.merchantRules.supplierUnavailable' : 'workspace.rules.merchantRules.vendorUnavailable'); - const vendorValue = getVendorRuleDisplayValue(policy, rule.vendorID, unavailableLabel); - actions.push(translate('workspace.rules.merchantRules.ruleSummarySubtitleUpdateField', fieldLabels.vendor, vendorValue)); - } - if (rule.reimbursable !== undefined) { - actions.push(translate('workspace.rules.merchantRules.ruleSummarySubtitleReimbursable', rule.reimbursable)); - } - if (rule.billable !== undefined) { - actions.push(translate('workspace.rules.merchantRules.ruleSummarySubtitleBillable', rule.billable)); + for (const {field, value} of summaryFields) { + if (field === FIELD.MERCHANT && typeof value === 'string') { + actions.push(translate('workspace.rules.merchantRules.ruleSummarySubtitleMerchant', value)); + } else if (field === FIELD.CATEGORY && typeof value === 'string') { + actions.push(translate('workspace.rules.merchantRules.ruleSummarySubtitleUpdateField', fieldLabels.category, getDecodedCategoryName(value))); + } else if (field === FIELD.TAG && typeof value === 'string') { + actions.push(translate('workspace.rules.merchantRules.ruleSummarySubtitleUpdateField', fieldLabels.tag, getCommaSeparatedTagNameWithSanitizedColons(value))); + } else if (field === FIELD.COMMENT && typeof value === 'string') { + actions.push(translate('workspace.rules.merchantRules.ruleSummarySubtitleUpdateField', fieldLabels.description, value)); + } else if (field === FIELD.TAX && isExpenseDefaultTaxValue(value) && !!value.field_id_TAX.externalID) { + // The rate saved on the rule is a snapshot, so resolve the live one first and keep the snapshot + // as a fallback. Without this a renamed rate reads stale, and a rule saved before the rates + // loaded has no snapshot at all and its tax default disappears from the summary. + actions.push( + translate( + 'workspace.rules.merchantRules.ruleSummarySubtitleUpdateField', + fieldLabels.tax, + getTaxRateDisplayName(policy, value.field_id_TAX.externalID, value.field_id_TAX), + ), + ); + } else if (field === FIELD.VENDOR_ID && typeof value === 'string') { + const unavailableLabel = translate(isOnXero ? 'workspace.rules.merchantRules.supplierUnavailable' : 'workspace.rules.merchantRules.vendorUnavailable'); + actions.push(translate('workspace.rules.merchantRules.ruleSummarySubtitleUpdateField', fieldLabels.vendor, getVendorRuleDisplayValue(policy, value, unavailableLabel))); + } else if (field === FIELD.REIMBURSABLE && typeof value === 'boolean') { + actions.push(translate('workspace.rules.merchantRules.ruleSummarySubtitleReimbursable', value)); + } else if (field === FIELD.BILLABLE && typeof value === 'boolean') { + actions.push(translate('workspace.rules.merchantRules.ruleSummarySubtitleBillable', value)); + } } const ruleDescription = actions.map((action, index) => (index === 0 ? action : action.charAt(0).toLowerCase() + action.slice(1))).join(', '); const pendingAction = rule.pendingAction; + // A rule the editor can't represent would lose whatever the form can't show if it were saved back, + // so the row summarizes it but doesn't open it. See `getMerchantRuleFormValues`. + const isEditable = isEditableMerchantRule(rule); + return { keyForList: ruleID, ruleID, section: CONST.POLICY.EXPENSE_DEFAULTS_SECTION.MERCHANTS, isRename: hasOnlyMerchantRename, + isSelectionDisabled: !isEditable, typeLabel, conditionText: translate('workspace.rules.expenseDefaultsTable.merchantIs', merchantName), ruleDescription, searchTokens: [merchantName, ruleDescription], pendingAction, errors: rule.errors, - onCloseError: () => clearPolicyCodingRuleErrors(policyID, ruleID, rule), - disabled: pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE, + onCloseError: () => clearMerchantRuleErrors(ruleID, rule), + disabled: !isEditable || rule.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE, action: () => onNavigate(ROUTES.RULES_MERCHANT_EDIT.getRoute(policyID, ruleID)), }; }); @@ -202,6 +198,7 @@ function getMerchantCodingRulesTableData({ function getExpenseDefaultsTableData({ policy, policyID, + rules, policyCategories, translate, isOffline, @@ -209,6 +206,7 @@ function getExpenseDefaultsTableData({ }: { policy: Policy | undefined; policyID: string; + rules: OnyxCollection | undefined; /** Read for the pending state of a category a rule depends on, so the rule shows as deleting alongside it. */ policyCategories: PolicyCategories | undefined; translate: LocaleContextProps['translate']; @@ -216,7 +214,7 @@ function getExpenseDefaultsTableData({ onNavigate: (route: Route) => void; }): ExpenseDefaultTableItem[] { const categoryTaxRules = getCategoryTaxRulesTableData({policy, policyCategories, translate, isOffline, onNavigate}); - const merchantRules = getMerchantCodingRulesTableData({policy, policyID, translate, isOffline, onNavigate}); + const merchantRules = getMerchantRulesTableData({policy, policyID, rules, translate, isOffline, onNavigate}); const merchantTypeRules = getMerchantTypeRulesTableData({policy, translate, onNavigate}); return [...categoryTaxRules, ...merchantRules, ...merchantTypeRules]; @@ -225,7 +223,7 @@ function getExpenseDefaultsTableData({ export { getDefaultMccGroupCategory, getExpenseDefaultsTableData, - getMerchantCodingRulesTableData, + getMerchantRulesTableData, getMerchantTypeRuleFormFromMccGroup, isDefaultMccGroupID, isMerchantTypeRuleKey, diff --git a/src/libs/PolicyUtils.ts b/src/libs/PolicyUtils.ts index e55940290221..85137ecdba98 100644 --- a/src/libs/PolicyUtils.ts +++ b/src/libs/PolicyUtils.ts @@ -19,7 +19,7 @@ import type { Transaction, TravelSettings, } from '@src/types/onyx'; -import type {ApprovalWorkflowFilter, ApprovalWorkflowFilterComparison, ApprovalWorkflowRule} from '@src/types/onyx/ApprovalWorkflowRules'; +import type {ApprovalWorkflowRule} from '@src/types/onyx/ApprovalWorkflowRules'; import type {ErrorFields, PendingAction, PendingFields} from '@src/types/onyx/OnyxCommon'; import type { ApprovalRule, @@ -40,6 +40,7 @@ import type { } from '@src/types/onyx/Policy'; import type PolicyEmployee from '@src/types/onyx/PolicyEmployee'; import type Rule from '@src/types/onyx/Rule'; +import type {RuleFilterComparison, RuleFilterNode} from '@src/types/onyx/RuleFilters'; import type {WorkspaceTravelSettings} from '@src/types/onyx/TravelSettings'; import {isEmptyObject} from '@src/types/utils/EmptyObject'; @@ -317,11 +318,10 @@ function hasPolicyCategoriesError(policyCategories: OnyxEntry) /** * Check if the policy has any errors within the rules. */ -function hasPolicyRulesError(policy: OnyxEntry): boolean { - const codingRules = Object.values(policy?.rules?.codingRules ?? {}); +function hasPolicyRulesError(policy: OnyxEntry, hasMerchantRuleErrors = false): boolean { const agentRules = Object.values(policy?.rules?.agentRules ?? {}); - return codingRules.some((rule) => rule && Object.keys(rule.errors ?? {}).length > 0) || agentRules.some((rule) => rule && Object.keys(rule.errors ?? {}).length > 0); + return hasMerchantRuleErrors || agentRules.some((rule) => rule && Object.keys(rule.errors ?? {}).length > 0); } /** @@ -1202,26 +1202,26 @@ function isMaxExpenseAmountSet(value: number | undefined): value is number { /** * Checks if a policy has any rules configured (structured rules, individual expense limits, or prohibited expenses). */ -function hasConfiguredRules(policy: OnyxEntry, policyCategories?: PolicyCategories | null): boolean { +function hasConfiguredRules(policy: OnyxEntry, policyCategories?: PolicyCategories | null, hasExpenseDefaultRules = false): boolean { if (!policy) { return false; } - if (!!policy.customRules && policy.customRules.trim().length > 0) { + if (hasExpenseDefaultRules) { return true; } - const {rules} = policy; - if (!!rules?.approvalRules && rules.approvalRules.length > 0) { + if (!!policy.customRules && policy.customRules.trim().length > 0) { return true; } - if (!!rules?.expenseRules && rules.expenseRules.length > 0) { + + const {rules: policyRules} = policy; + if (!!policyRules?.approvalRules && policyRules.approvalRules.length > 0) { return true; } - if (!!rules?.codingRules && Object.keys(rules.codingRules).length > 0) { + if (!!policyRules?.expenseRules && policyRules.expenseRules.length > 0) { return true; } - if (!!policy.maxExpenseAmount && policy.maxExpenseAmount !== CONST.DISABLED_MAX_EXPENSE_VALUE && policy.maxExpenseAmount !== CONST.POLICY.DEFAULT_MAX_EXPENSE_AMOUNT) { return true; } @@ -1946,17 +1946,17 @@ function getFirstRuleApprover(approvalRules: ApprovalRule[], expenseReport: Onyx /** * True when this node is a single comparison instead of a combination of two children. */ -function isApprovalWorkflowComparison(node: ApprovalWorkflowFilter | ApprovalWorkflowFilterComparison): node is ApprovalWorkflowFilterComparison { +function isApprovalWorkflowComparison(node: RuleFilterNode): node is RuleFilterComparison { return typeof node.left === 'string'; } -function matchesApprovalWorkflowEmailComparison(node: ApprovalWorkflowFilterComparison, email: string | undefined): boolean { +function matchesApprovalWorkflowEmailComparison(node: RuleFilterComparison, email: string | undefined): boolean { const expectedEmails = (Array.isArray(node.right) ? node.right : [node.right]).map((value) => String(value).toLowerCase()); const isMatch = !!email && expectedEmails.includes(email.toLowerCase()); return node.operator === CONST.SEARCH.SYNTAX_OPERATORS.NOT_EQUAL_TO ? !isMatch : isMatch; } -function matchesApprovalWorkflowAmountComparison(node: ApprovalWorkflowFilterComparison, amount: number): boolean { +function matchesApprovalWorkflowAmountComparison(node: RuleFilterComparison, amount: number): boolean { const expectedAmount = typeof node.right === 'number' ? node.right : Number(node.right); if (Number.isNaN(expectedAmount)) { return false; @@ -1980,7 +1980,7 @@ function matchesApprovalWorkflowAmountComparison(node: ApprovalWorkflowFilterCom } } -function evaluateApprovalWorkflowFilter(node: ApprovalWorkflowFilter | ApprovalWorkflowFilterComparison, context: ApprovalWorkflowContext): boolean { +function evaluateApprovalWorkflowFilter(node: RuleFilterNode, context: ApprovalWorkflowContext): boolean { if (!isApprovalWorkflowComparison(node)) { const left = evaluateApprovalWorkflowFilter(node.left, context); const right = evaluateApprovalWorkflowFilter(node.right, context); @@ -2010,6 +2010,17 @@ function evaluateApprovalWorkflowRule(rule: ApprovalWorkflowRule, context: Appro return evaluateApprovalWorkflowFilter(rule.filters, context); } +/** + * The `rules_` collection holds both approval workflow rules and expense default (merchant) rules under one + * shape, distinguished only by which triggers they carry. Narrows to the approval-workflow variant so its + * `filters`/`actions` can be read with the right shape instead of the expense-default one. + */ +function isApprovalWorkflowRule(rule: Rule): rule is Rule & ApprovalWorkflowRule { + const approvalWorkflowTriggers: string[] = Object.values(CONST.RULES.APPROVAL_WORKFLOW.TRIGGER); + const triggers = Object.values(rule.triggers ?? {}); + return triggers.length > 0 && triggers.every((trigger) => approvalWorkflowTriggers.includes(trigger)); +} + /** * Check the policy's approval workflow rules to determine where the report goes next. */ @@ -2027,7 +2038,7 @@ function getForwardsToFromRules(policy: OnyxEntry, context: ApprovalWork if (!rule || rule.scope !== CONST.RULES.SCOPE.POLICY || rule.scopeID !== policy.id || rule.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE) { continue; } - if (!Object.values(rule.triggers ?? {}).includes(trigger)) { + if (!isApprovalWorkflowRule(rule) || !Object.values(rule.triggers ?? {}).includes(trigger)) { continue; } if (!evaluateApprovalWorkflowRule(rule, context)) { diff --git a/src/libs/WorkflowUtils.ts b/src/libs/WorkflowUtils.ts index 98d411f468c7..8fda2112b7cf 100644 --- a/src/libs/WorkflowUtils.ts +++ b/src/libs/WorkflowUtils.ts @@ -7,20 +7,14 @@ import ONYXKEYS from '@src/ONYXKEYS'; import type {BankAccountList} from '@src/types/onyx'; import type {ApprovalWorkflowOnyx, Approver, Member} from '@src/types/onyx/ApprovalWorkflow'; import type ApprovalWorkflow from '@src/types/onyx/ApprovalWorkflow'; -import type { - ApprovalWorkflowAction, - ApprovalWorkflowActions, - ApprovalWorkflowFilter, - ApprovalWorkflowFilterComparison, - ApprovalWorkflowRule, - ApprovalWorkflowTriggers, -} from '@src/types/onyx/ApprovalWorkflowRules'; +import type {ApprovalWorkflowAction, ApprovalWorkflowActions, ApprovalWorkflowRule, ApprovalWorkflowTriggers} from '@src/types/onyx/ApprovalWorkflowRules'; import type {PersonalDetailsList} from '@src/types/onyx/PersonalDetails'; import type PersonalDetails from '@src/types/onyx/PersonalDetails'; import type Policy from '@src/types/onyx/Policy'; import type PolicyEmployee from '@src/types/onyx/PolicyEmployee'; import type {PolicyEmployeeList} from '@src/types/onyx/PolicyEmployee'; import type Rule from '@src/types/onyx/Rule'; +import type {RuleFilter, RuleFilterComparison, RuleFilterNode} from '@src/types/onyx/RuleFilters'; import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; import type {ValueOf} from 'type-fest'; @@ -745,23 +739,19 @@ function mergeWorkflowMembersWithAvailableMembers(workflowMembers: Member[], all type ApprovalWorkflowRulesDiff = Record; -function buildComparison( - left: ApprovalWorkflowFilterComparison['left'], - operator: ValueOf, - right: ApprovalWorkflowFilterComparison['right'], -): ApprovalWorkflowFilterComparison { +function buildComparison(left: RuleFilterComparison['left'], operator: ValueOf, right: RuleFilterComparison['right']): RuleFilterComparison { return {operator, left, right}; } -function buildAnd(left: ApprovalWorkflowFilter['left'], right: ApprovalWorkflowFilter['right']): ApprovalWorkflowFilter { +function buildAnd(left: RuleFilter['left'], right: RuleFilter['right']): RuleFilter { return {operator: CONST.SEARCH.SYNTAX_OPERATORS.AND, left, right}; } -function buildSubmitterFilter(memberEmails: string[]): ApprovalWorkflowFilterComparison { +function buildSubmitterFilter(memberEmails: string[]): RuleFilterComparison { return buildComparison(CONST.SEARCH.SYNTAX_FILTER_KEYS.FROM, CONST.SEARCH.SYNTAX_OPERATORS.EQUAL_TO, [...memberEmails]); } -function buildToComparison(email: string): ApprovalWorkflowFilterComparison { +function buildToComparison(email: string): RuleFilterComparison { return buildComparison(CONST.SEARCH.SYNTAX_FILTER_KEYS.TO, CONST.SEARCH.SYNTAX_OPERATORS.EQUAL_TO, email); } @@ -903,17 +893,17 @@ function buildApprovalWorkflowRules(approvalWorkflow: ApprovalWorkflow): Approva * Both look the same (`{operator, left, right}`), so the giveaway is `left`: a comparison points at a field * name, an `AND` points at another node. */ -function isComparisonLeaf(node: ApprovalWorkflowFilter | ApprovalWorkflowFilterComparison | undefined): node is ApprovalWorkflowFilterComparison { +function isComparisonLeaf(node: RuleFilterNode | undefined): node is RuleFilterComparison { return !!node && typeof node.left === 'string'; } /** True when a comparison node targets the `from` field with an equality operator. */ -function isSubmitterFilter(node: ApprovalWorkflowFilter | ApprovalWorkflowFilterComparison): boolean { +function isSubmitterFilter(node: RuleFilterNode): boolean { return isComparisonLeaf(node) && node.operator === CONST.SEARCH.SYNTAX_OPERATORS.EQUAL_TO && node.left === CONST.SEARCH.SYNTAX_FILTER_KEYS.FROM; } /** Return the first comparison leaf in the filter tree whose `left` field matches. */ -function getFilter(node: ApprovalWorkflowFilter | ApprovalWorkflowFilterComparison | undefined, leftKey: string): ApprovalWorkflowFilterComparison | undefined { +function getFilter(node: RuleFilterNode | undefined, leftKey: string): RuleFilterComparison | undefined { if (!node) { return undefined; } @@ -924,10 +914,7 @@ function getFilter(node: ApprovalWorkflowFilter | ApprovalWorkflowFilterComparis } /** Rebuild a filter tree, replacing every comparison leaf with the result of `mapLeaf`. */ -function mapFilters( - node: ApprovalWorkflowFilter | ApprovalWorkflowFilterComparison, - mapLeaf: (leaf: ApprovalWorkflowFilterComparison) => ApprovalWorkflowFilterComparison, -): ApprovalWorkflowFilter | ApprovalWorkflowFilterComparison { +function mapFilters(node: RuleFilterNode, mapLeaf: (leaf: RuleFilterComparison) => RuleFilterComparison): RuleFilterNode { if (isComparisonLeaf(node)) { return mapLeaf(node); } @@ -974,7 +961,7 @@ function sortObjectKeysDeep(value: unknown): unknown { * which is what we look for when deciding whether to merge two workflows into a shared rule. */ function getRuleShape(rule: ApprovalWorkflowRule): string { - const stripFromValues = (node: ApprovalWorkflowFilter | ApprovalWorkflowFilterComparison | undefined): unknown => { + const stripFromValues = (node: RuleFilterNode | undefined): unknown => { if (!node) { return node; } @@ -1465,6 +1452,20 @@ function filterRulesForPolicy(rulesCollection: OnyxCollection, policyID: s return result; } +/** + * The `rules_` collection holds every kind of rule (approval workflows, expense defaults, ...), so anything + * reading it has to narrow to the kind it handles. An approval workflow rule is one that fires on a report event. + */ +function isApprovalWorkflowRule(rule: Rule): rule is Rule & ApprovalWorkflowRule { + const approvalWorkflowTriggers: string[] = Object.values(CONST.RULES.APPROVAL_WORKFLOW.TRIGGER); + const triggers = Object.values(rule.triggers ?? {}); + + // Every trigger has to be a report event, not just one of them. A rule that also fires on transaction + // creation is an expense default and is listed as one, so treating it as a workflow here would delete it + // along with the workflows when approvals are turned off. + return triggers.length > 0 && triggers.every((trigger) => approvalWorkflowTriggers.includes(trigger)); +} + /** * Convert the `ONYXKEYS.COLLECTION.RULE` collection into the `ruleID -> rule body` map used by the * builder, reconcilers and converters, keeping only non-deleted rules scoped to `policyID`. @@ -1473,7 +1474,7 @@ function getApprovalWorkflowRulesForPolicy(rulesCollection: OnyxCollection const result: Record = {}; for (const [onyxKey, rule] of Object.entries(filterRulesForPolicy(rulesCollection, policyID))) { - if (!rule || rule.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE) { + if (!rule || rule.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE || !isApprovalWorkflowRule(rule)) { continue; } const ruleID = onyxKey.slice(ONYXKEYS.COLLECTION.RULE.length); @@ -1735,6 +1736,7 @@ export { getRulesSubmitterToWorkflowKey, getWorkflowMemberEmails, hasRuleBasedDefaultWorkflow, + isApprovalWorkflowRule, getEligibleExistingBusinessBankAccounts, getOpenConnectedToPolicyBusinessBankAccounts, getOverLimitForwardsToDisplayName, @@ -1744,6 +1746,7 @@ export { reconcileApprovalWorkflowRulesForEdit, reconcileApprovalWorkflowRulesForMembersChange, reconcileApprovalWorkflowRulesForRemove, + toIndexMap, updateWorkflowDataOnApproverRemoval, }; export type {ApprovalWorkflowRulesDiff}; diff --git a/src/libs/actions/Policy/CopyPolicySettings.ts b/src/libs/actions/Policy/CopyPolicySettings.ts index 57ce88875666..68045058e7ea 100644 --- a/src/libs/actions/Policy/CopyPolicySettings.ts +++ b/src/libs/actions/Policy/CopyPolicySettings.ts @@ -2,12 +2,13 @@ import {write} from '@libs/API'; import type {CopyPolicySettingsParams} from '@libs/API/parameters'; import {WRITE_COMMANDS} from '@libs/API/types'; import {getMicroSecondOnyxErrorWithTranslationKey} from '@libs/ErrorUtils'; +import {buildCopiedExpenseDefaultRules} from '@libs/ExpenseDefaultRuleUtils'; import {hasExplicitFlagAmount} from '@libs/FlagForReviewRulesUtils'; import {categoryHasAnyRequireFieldsRule} from '@libs/RequireFieldsRulesUtils'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; -import type {CopyPolicySettings as CopyPolicySettingsState, Policy, PolicyCategories, PolicyTagLists, PolicyCategory} from '@src/types/onyx'; +import type {CopyPolicySettings as CopyPolicySettingsState, Policy, PolicyCategories, PolicyTagLists, PolicyCategory, Rule} from '@src/types/onyx'; import type {CustomUnit, PolicyFeatureName, Rate} from '@src/types/onyx/Policy'; import type {OnyxCollection, OnyxUpdate} from 'react-native-onyx'; @@ -358,6 +359,7 @@ type CopyPolicySettingsOnyxKeys = | typeof ONYXKEYS.COLLECTION.POLICY | typeof ONYXKEYS.COLLECTION.POLICY_CATEGORIES | typeof ONYXKEYS.COLLECTION.POLICY_TAGS + | typeof ONYXKEYS.COLLECTION.RULE | typeof ONYXKEYS.COPY_POLICY_SETTINGS | typeof ONYXKEYS.NVP_BULK_POLICY_COPY_SETTINGS; @@ -367,6 +369,7 @@ function buildCopyPolicySettingsData( parts: Part[], allPolicyCategories: OnyxCollection, allPolicyTags: OnyxCollection, + allRules: OnyxCollection, ): { optimisticData: Array>; successData: Array>; @@ -407,18 +410,6 @@ function buildCopyPolicySettingsData( const sourceTagsKey = `${ONYXKEYS.COLLECTION.POLICY_TAGS}${sourcePolicy.id}` as const; const sourceCategories = allPolicyCategories?.[sourceCategoriesKey] ?? {}; const sourceTags = allPolicyTags?.[sourceTagsKey] ?? {}; - const filterPendingDeleteData = (data?: Record): Record | undefined => - data - ? (Object.fromEntries( - Object.entries(data).filter(([, value]) => { - if (!value || typeof value !== 'object' || !('pendingAction' in value)) { - return true; - } - return value.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE; - }), - ) as Record) - : undefined; - const codingRulesWithoutPendingDelete = filterPendingDeleteData(sourcePolicy.rules?.codingRules); for (const targetPolicy of targetPolicies) { const policyKey = `${ONYXKEYS.COLLECTION.POLICY}${targetPolicy.id}` as const; @@ -427,14 +418,15 @@ function buildCopyPolicySettingsData( const timeTrackingPatch = isTimeTrackingSelected ? buildTimeTrackingPatch(sourcePolicy) : undefined; const travelSettingsPatch = isTravelSelected ? buildTravelSettingsPatch(sourcePolicy, targetPolicy) : undefined; const receiptPartnersPatch = isReceiptPartnersSelected ? buildReceiptPartnersPatch(sourcePolicy) : undefined; - const codingRulesPatch = isCodingRulesSelected - ? { - rules: { - ...targetPolicy.rules, - codingRules: codingRulesWithoutPendingDelete, - }, - } - : {}; + // Merchant rules live in their own collection, so each target policy gets its own copies rather than + // a shared blob on the policy object. The server mints its own IDs, so the optimistic copies are dropped on success. + const copiedRules = isCodingRulesSelected ? buildCopiedExpenseDefaultRules(allRules, sourcePolicy.id, targetPolicy.id) : {}; + for (const [ruleID, rule] of Object.entries(copiedRules)) { + const ruleKey = `${ONYXKEYS.COLLECTION.RULE}${ruleID}` as const; + optimisticData.push({onyxMethod: Onyx.METHOD.SET, key: ruleKey, value: rule}); + successData.push({onyxMethod: Onyx.METHOD.SET, key: ruleKey, value: null}); + failureData.push({onyxMethod: Onyx.METHOD.SET, key: ruleKey, value: null}); + } // Step 1+2: SET the full policy with patched fields overlaid. // We use SET (not MERGE) because Onyx.merge deep-merges nested objects — source @@ -456,7 +448,6 @@ function buildCopyPolicySettingsData( : {}), ...(travelSettingsPatch ?? {}), ...(receiptPartnersPatch ? {receiptPartners: receiptPartnersPatch.receiptPartners} : {}), - ...codingRulesPatch, pendingFields: {...targetPolicy.pendingFields, ...pendingFields, ...timeTrackingPendingFields, ...receiptPartnersPendingFields}, }, }); @@ -583,8 +574,9 @@ function copyPolicySettings( parts: Part[], allPolicyCategories: OnyxCollection, allPolicyTags: OnyxCollection, + allRules: OnyxCollection, ): void { - const {optimisticData, successData, failureData} = buildCopyPolicySettingsData(sourcePolicy, targetPolicies, parts, allPolicyCategories, allPolicyTags); + const {optimisticData, successData, failureData} = buildCopyPolicySettingsData(sourcePolicy, targetPolicies, parts, allPolicyCategories, allPolicyTags, allRules); const params: CopyPolicySettingsParams = { policyID: sourcePolicy.id, diff --git a/src/libs/actions/Policy/Policy.ts b/src/libs/actions/Policy/Policy.ts index 8c349023e6ff..09866ae41bc8 100644 --- a/src/libs/actions/Policy/Policy.ts +++ b/src/libs/actions/Policy/Policy.ts @@ -82,6 +82,7 @@ import type {CustomRNImageManipulatorResult} from '@libs/cropOrRotateImage/types import * as CurrencyUtils from '@libs/CurrencyUtils'; import DateUtils from '@libs/DateUtils'; import * as ErrorUtils from '@libs/ErrorUtils'; +import {buildCopiedExpenseDefaultRules} from '@libs/ExpenseDefaultRuleUtils'; import {createFile, splitExtensionFromFileName} from '@libs/fileDownload/FileUtils'; import getIsNarrowLayout from '@libs/getIsNarrowLayout'; import getWorkspaceCreatedAnalyticsEvent from '@libs/getWorkspaceCreatedAnalyticsEvent'; @@ -99,6 +100,7 @@ import {getCustomUnitsForDuplication, getMemberAccountIDsForWorkspace, goBackWhe import * as ReportUtils from '@libs/ReportUtils'; import {getNegatedAmountTransaction} from '@libs/TransactionUtils'; import type {AvatarSource} from '@libs/UserAvatarUtils'; +import {isApprovalWorkflowRule} from '@libs/WorkflowUtils'; import type {Feature} from '@pages/OnboardingInterestedFeatures/types'; @@ -306,6 +308,7 @@ type DuplicatePolicyDataOptions = { file?: File | CustomRNImageManipulatorResult; policyCategories?: PolicyCategories; localCurrency: string; + rules?: OnyxCollection; }; type SetWorkspaceReimbursementActionParams = { @@ -1109,7 +1112,9 @@ function setWorkspaceApprovalMode( if (approvalMode === CONST.POLICY.APPROVAL_MODE.OPTIONAL && rules) { for (const [ruleKey, rule] of Object.entries(rules)) { - if (!rule || rule.scope !== CONST.RULES.SCOPE.POLICY || rule.scopeID !== policyID) { + // The rules collection holds every kind of rule, so only the approval workflow ones are removed here. + // Expense default rules on the same policy have nothing to do with approvals and have to survive. + if (!rule || rule.scope !== CONST.RULES.SCOPE.POLICY || rule.scopeID !== policyID || !isApprovalWorkflowRule(rule)) { continue; } const ruleID = ruleKey.slice(ONYXKEYS.COLLECTION.RULE.length); @@ -3476,7 +3481,6 @@ function buildOptimisticDuplicatePolicy( const isPerDiemFeatureSelected = duplicatedParts?.perDiem; const isOverviewFeatureSelected = duplicatedParts?.overview; const isTravelFeatureSelected = duplicatedParts?.travel; - const isCodingRulesFeatureSelected = duplicatedParts?.codingRules; const duplicatedOutputCurrency = isOverviewFeatureSelected ? sourcePolicy?.outputCurrency : duplicatedLocalCurrency; const filterPendingDeleteData = (data?: Record): Record | undefined => @@ -3491,7 +3495,6 @@ function buildOptimisticDuplicatePolicy( ) as Record) : undefined; - const codingRulesWithoutPendingDelete = filterPendingDeleteData(sourcePolicy?.rules?.codingRules); const willCopyRulesDocument = isOverviewFeatureSelected && !!sourcePolicy?.rulesDocumentURL; const employeeListWithoutPendingDelete = filterPendingDeleteData(sourcePolicy?.employeeList); const fieldListWithoutPendingDelete = filterPendingDeleteData(sourcePolicy?.fieldList); @@ -3539,7 +3542,7 @@ function buildOptimisticDuplicatePolicy( customUnitRateID: duplicatedCustomUnitRateID, }), taxRates: isTaxesFeatureSelected ? taxRatesWithoutPendingDelete : undefined, - rules: isCodingRulesFeatureSelected ? {codingRules: codingRulesWithoutPendingDelete} : undefined, + rules: undefined, pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD, pendingFields: { autoReporting: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD, @@ -3577,6 +3580,7 @@ function buildDuplicatePolicyData(policy: Policy, options: DuplicatePolicyDataOp localCurrency, currentUserAccountID, currentUserEmail, + rules, } = options; const { @@ -3620,6 +3624,7 @@ function buildDuplicatePolicyData(policy: Policy, options: DuplicatePolicyDataOp | typeof ONYXKEYS.COLLECTION.REPORT_DRAFT | typeof ONYXKEYS.COLLECTION.POLICY_CATEGORIES | typeof ONYXKEYS.COLLECTION.POLICY_CATEGORIES_DRAFT + | typeof ONYXKEYS.COLLECTION.RULE > > = [ { @@ -3697,6 +3702,7 @@ function buildDuplicatePolicyData(policy: Policy, options: DuplicatePolicyDataOp | typeof ONYXKEYS.COLLECTION.REPORT_ACTIONS | typeof ONYXKEYS.COLLECTION.POLICY_CATEGORIES | typeof ONYXKEYS.COLLECTION.POLICY_CATEGORIES_DRAFT + | typeof ONYXKEYS.COLLECTION.RULE > > = [ { @@ -3783,6 +3789,7 @@ function buildDuplicatePolicyData(policy: Policy, options: DuplicatePolicyDataOp | typeof ONYXKEYS.COLLECTION.REPORT_ACTIONS | typeof ONYXKEYS.COLLECTION.POLICY_CATEGORIES | typeof ONYXKEYS.COLLECTION.POLICY_CATEGORIES_DRAFT + | typeof ONYXKEYS.COLLECTION.RULE > > = [ { @@ -3825,6 +3832,17 @@ function buildDuplicatePolicyData(policy: Policy, options: DuplicatePolicyDataOp successData.push(...optimisticCategoriesData.successData); } + // Merchant rules are their own collection keyed per rule, and each rule names the policy it belongs to, + // so the duplicate gets fresh rules rather than a copy of the source's. The server mints its own IDs, + // which is why the optimistic copies are dropped once it responds. + const copiedRules = parts?.codingRules ? buildCopiedExpenseDefaultRules(rules, policy?.id, targetPolicyID) : {}; + for (const [ruleID, rule] of Object.entries(copiedRules)) { + const ruleKey = `${ONYXKEYS.COLLECTION.RULE}${ruleID}` as const; + optimisticData.push({onyxMethod: Onyx.METHOD.SET, key: ruleKey, value: rule}); + successData.push({onyxMethod: Onyx.METHOD.SET, key: ruleKey, value: null}); + failureData.push({onyxMethod: Onyx.METHOD.SET, key: ruleKey, value: null}); + } + // We need to clone the file to prevent non-indexable errors. const clonedFile = file ? createFile(file as File) : undefined; diff --git a/src/libs/actions/Policy/Rules.ts b/src/libs/actions/Policy/Rules.ts index 25498d54b9f5..582d870824e7 100644 --- a/src/libs/actions/Policy/Rules.ts +++ b/src/libs/actions/Policy/Rules.ts @@ -11,17 +11,19 @@ import type OpenPolicyRulesPageParams from '@libs/API/parameters/OpenPolicyRules import type SetPolicyCodingRuleParams from '@libs/API/parameters/SetPolicyCodingRuleParams'; import {READ_COMMANDS, SIDE_EFFECT_REQUEST_COMMANDS, WRITE_COMMANDS} from '@libs/API/types'; import * as ErrorUtils from '@libs/ErrorUtils'; +import {buildMerchantRule} from '@libs/ExpenseDefaultRuleUtils'; +import type {MerchantRuleFormValues} from '@libs/ExpenseDefaultRuleUtils'; import Log from '@libs/Log'; import * as NumberUtils from '@libs/NumberUtils'; import Parser from '@libs/Parser'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; -import type {MerchantRuleForm} from '@src/types/form'; import type {ImportFinalModal} from '@src/types/onyx/ImportedSpreadsheet'; import type Policy from '@src/types/onyx/Policy'; import type {AgentRule, CodingRule, CodingRuleFilter, CodingRuleTax} from '@src/types/onyx/Policy'; import type {OnyxData} from '@src/types/onyx/Request'; +import type Rule from '@src/types/onyx/Rule'; import type {OnyxUpdate} from 'react-native-onyx'; @@ -30,16 +32,10 @@ import Onyx from 'react-native-onyx'; /** A coding rule parsed from an imported spreadsheet row, keyed by a client-generated ruleID */ type ImportedMerchantRule = Omit; -/** - * Builds the tax object from a tax key and policy - */ -function buildTaxObject(taxKey: string | undefined, policy: Policy | undefined): CodingRuleTax | undefined { - if (!taxKey || !policy?.taxRates?.taxes) { - return undefined; - } - - const tax = policy.taxRates.taxes[taxKey]; - if (!tax) { +/** Builds the tax object `SetPolicyCodingRule` expects, in the legacy flat shape rather than the rules engine's action value. */ +function buildLegacyCodingRuleTax(taxKey: string | undefined, policy: Policy | undefined): CodingRuleTax | undefined { + const tax = taxKey ? policy?.taxRates?.taxes?.[taxKey] : undefined; + if (!taxKey || !tax) { return undefined; } @@ -54,68 +50,56 @@ function buildTaxObject(taxKey: string | undefined, policy: Policy | undefined): } /** - * Converts a markdown comment to HTML using Parser.replace(). - * Returns null if the comment is empty or undefined. + * Builds the `codingRuleValue` sent to `SetPolicyCodingRule`. We still write through the legacy command rather than + * `SetRule`, because `SetPolicyCodingRule` dual-writes into both `policy.rules.codingRules` and the `rules_` + * collection, while `SetRule` only writes the new collection, so older clients reading `codingRules` would silently + * stop seeing rules created or edited on a newer client. */ -function convertCommentToHTML(comment: string | undefined): string | null { - if (!comment) { - return null; - } - return Parser.replace(comment); -} +function buildLegacyCodingRule(formValues: Partial, policy: Policy | undefined, ruleID: string, created: string): Partial { + const tax = buildLegacyCodingRuleTax(formValues.tax, policy); -/** - * Maps form fields to rule properties with null for empty values. - * Used for Onyx to properly remove cleared fields during merge. - */ -function mapFormFieldsToRuleForOnyx(form: MerchantRuleForm, policy: Policy | undefined) { return { - merchant: form.merchant || null, - category: form.category || null, - tag: form.tag || null, - tax: buildTaxObject(form.tax, policy) ?? null, - vendorID: form.vendorID || null, - comment: convertCommentToHTML(form.comment), - reimbursable: form.reimbursable ?? null, - billable: form.billable ?? null, + ruleID, + filters: { + left: 'merchant', + operator: formValues.matchType ?? CONST.SEARCH.SYNTAX_OPERATORS.CONTAINS, + right: formValues.merchantToMatch ?? '', + }, + ...(formValues.merchant && {merchant: formValues.merchant}), + ...(formValues.category && {category: formValues.category}), + ...(formValues.tag && {tag: formValues.tag}), + ...(tax && {tax}), + ...(formValues.vendorID && {vendorID: formValues.vendorID}), + ...(formValues.comment && {comment: Parser.replace(formValues.comment)}), + ...(formValues.reimbursable !== undefined && {reimbursable: formValues.reimbursable}), + ...(formValues.billable !== undefined && {billable: formValues.billable}), + created, }; } /** - * Maps form fields to rule properties, omitting empty values. - * Used for API to avoid sending null values. + * Fetches every rule the user has access to. The response SETs the whole `rules_` collection. + * + * The flag lets screens that only consume the collection fetch it once rather than on every mount. It + * lives in Onyx rather than in this module so it is cleared along with the rest of the data on sign out. */ -function mapFormFieldsToRuleForAPI(form: MerchantRuleForm, policy: Policy | undefined): Partial { - const rule: Partial = {}; - - if (form.merchant) { - rule.merchant = form.merchant; - } - if (form.category) { - rule.category = form.category; - } - if (form.tag) { - rule.tag = form.tag; - } - const tax = buildTaxObject(form.tax, policy); - if (tax) { - rule.tax = tax; - } - if (form.vendorID) { - rule.vendorID = form.vendorID; - } - const commentHTML = convertCommentToHTML(form.comment); - if (commentHTML) { - rule.comment = commentHTML; - } - if (form.reimbursable !== undefined) { - rule.reimbursable = form.reimbursable; - } - if (form.billable !== undefined) { - rule.billable = form.billable; - } +function getRules() { + const successData: Array> = [ + { + onyxMethod: Onyx.METHOD.MERGE, + key: ONYXKEYS.HAS_RULES_DATA_BEEN_FETCHED, + value: true, + }, + ]; + const failureData: Array> = [ + { + onyxMethod: Onyx.METHOD.MERGE, + key: ONYXKEYS.HAS_RULES_DATA_BEEN_FETCHED, + value: false, + }, + ]; - return rule; + API.read(READ_COMMANDS.GET_RULES, {}, {successData, failureData}); } /** @@ -175,97 +159,57 @@ function getAgentRuleSuggestions(policyID: string | undefined) { * @param ruleID - Optional existing rule ID for updates * @param shouldUpdateMatchingTransactions - Whether to update transactions that match the rule */ -function setPolicyCodingRule(policyID: string, form: MerchantRuleForm, policy: Policy | undefined, ruleID?: string, shouldUpdateMatchingTransactions = false) { - if (!policyID || !form.merchantToMatch) { - Log.warn('Invalid params for setPolicyCodingRule', {policyID, merchantToMatch: form.merchantToMatch}); +/** + * Creates or updates a merchant rule. Editing a rule reuses its `ruleID`, since the rules engine has no separate update command. + * @param policyID - The ID of the policy the rule belongs to + * @param formValues - The merchant rule editor's values + * @param policy - Used to resolve the selected tax rate + * @param ruleID - The ID of the rule being edited, or undefined to create one + * @param existingRule - The rule being edited, restored on failure + * @param shouldUpdateMatchingTransactions - Whether to apply the rule to transactions that already match it + */ +function setMerchantRule( + policyID: string, + formValues: Partial, + policy: Policy | undefined, + ruleID?: string, + existingRule?: Rule, + shouldUpdateMatchingTransactions = false, +) { + const ruleValue = buildMerchantRule(formValues, policy); + + if (!policyID || !ruleValue) { + Log.warn('Invalid params for setMerchantRule', {policyID, merchantToMatch: formValues.merchantToMatch}); return; } const isEditing = !!ruleID; - const existingRule = isEditing ? policy?.rules?.codingRules?.[ruleID] : undefined; - - // Build rule with nulls for Onyx (to remove cleared fields) and without nulls for API - const ruleFieldsForOnyx = mapFormFieldsToRuleForOnyx(form, policy); - const ruleFieldsForAPI = mapFormFieldsToRuleForAPI(form, policy); - const targetRuleID = ruleID ?? NumberUtils.rand64(); - const operator = form.matchType ?? CONST.SEARCH.SYNTAX_OPERATORS.CONTAINS; + const ruleKey = `${ONYXKEYS.COLLECTION.RULE}${targetRuleID}` as const; const created = existingRule?.created ?? new Date().toISOString(); - const pendingAction = isEditing ? CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE : CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD; - const ruleForOnyx = { - ruleID: targetRuleID, - filters: { - left: 'merchant', - operator, - right: form.merchantToMatch, - }, - ...ruleFieldsForOnyx, + const optimisticRule: Rule = { + ...ruleValue, + scope: CONST.RULES.SCOPE.POLICY, + scopeID: policyID, + priority: CONST.RULES.EXPENSE_DEFAULT.PRIORITY, created, - pendingAction, + pendingAction: isEditing ? CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE : CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD, }; - // Rule for API (excludes null values) - const ruleForAPI: Partial = { - ruleID: targetRuleID, - filters: { - left: 'merchant', - operator, - right: form.merchantToMatch, - }, - ...ruleFieldsForAPI, - created, - }; - - const policyKey = `${ONYXKEYS.COLLECTION.POLICY}${policyID}` as const; - - // On failure: for new rules, remove the optimistic rule; for edits, restore the original rule - const failureRuleValue = isEditing ? existingRule : null; - - const onyxData = { - optimisticData: [ - { - onyxMethod: Onyx.METHOD.MERGE, - key: policyKey, - value: { - rules: { - codingRules: { - [targetRuleID]: ruleForOnyx, - }, - }, - }, - }, - ], - successData: [ - { - onyxMethod: Onyx.METHOD.MERGE, - key: policyKey, - value: { - rules: { - codingRules: { - [targetRuleID]: { - pendingAction: null, - errors: null, - }, - }, - }, - }, - }, - ], + const onyxData: OnyxData = { + // SET rather than MERGE: clearing a field removes its action, and a merge would leave the stale one behind. + optimisticData: [{onyxMethod: Onyx.METHOD.SET, key: ruleKey, value: {...optimisticRule, errors: null}}], + successData: [{onyxMethod: Onyx.METHOD.MERGE, key: ruleKey, value: {pendingAction: null, errors: null}}], failureData: [ { - onyxMethod: Onyx.METHOD.MERGE, - key: policyKey, + onyxMethod: Onyx.METHOD.SET, + key: ruleKey, + // Keep the rule visible with its error so the admin can retry or dismiss it, restoring the pre-edit value. value: { - rules: { - codingRules: { - [targetRuleID]: { - ...failureRuleValue, - pendingAction: isEditing ? null : CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD, - errors: ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('common.genericErrorMessage'), - }, - }, - }, + ...(isEditing && existingRule ? existingRule : optimisticRule), + pendingAction: isEditing ? null : CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD, + errors: ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('common.genericErrorMessage'), }, }, ], @@ -274,7 +218,7 @@ function setPolicyCodingRule(policyID: string, form: MerchantRuleForm, policy: P const parameters: SetPolicyCodingRuleParams = { policyID, codingRuleID: targetRuleID, - codingRuleValue: JSON.stringify(ruleForAPI), + codingRuleValue: JSON.stringify(buildLegacyCodingRule(formValues, policy, targetRuleID, created)), shouldUpdateMatchingTransactions, }; @@ -346,69 +290,34 @@ function getTransactionsMatchingCodingRule(policyID: string, filters: CodingRule } /** - * Deletes a coding rule from the given policy - * @param policyID - The ID of the policy to delete the rule from + * Deletes a merchant rule + * @param policyID - The ID of the policy the rule belongs to * @param ruleID - The ID of the rule to delete + * @param rule - The rule being deleted, restored on failure */ -function deletePolicyCodingRule(policy: Policy, ruleID: string) { - if (!policy.id || !ruleID) { - Log.warn('Invalid params for deletePolicyCodingRule'); +function deleteMerchantRule(policyID: string, ruleID: string, rule: Rule | undefined) { + if (!policyID || !ruleID) { + Log.warn('Invalid params for deleteMerchantRule', {policyID, ruleID}); return; } - const policyKey = `${ONYXKEYS.COLLECTION.POLICY}${policy.id}` as const; - const existingRule = policy.rules?.codingRules?.[ruleID]; + const ruleKey = `${ONYXKEYS.COLLECTION.RULE}${ruleID}` as const; - const onyxData = { - optimisticData: [ - { - onyxMethod: Onyx.METHOD.MERGE, - key: policyKey, - value: { - rules: { - codingRules: { - [ruleID]: { - pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE, - }, - }, - }, - }, - }, - ], - successData: [ - { - onyxMethod: Onyx.METHOD.MERGE, - key: policyKey, - value: { - rules: { - codingRules: { - [ruleID]: null, - }, - }, - }, - }, - ], + const onyxData: OnyxData = { + optimisticData: [{onyxMethod: Onyx.METHOD.MERGE, key: ruleKey, value: {pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE, errors: null}}], + successData: [{onyxMethod: Onyx.METHOD.SET, key: ruleKey, value: null}], failureData: [ { - onyxMethod: Onyx.METHOD.MERGE, - key: policyKey, - value: { - rules: { - codingRules: { - [ruleID]: { - ...existingRule, - pendingAction: null, - errors: ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('common.genericErrorMessage'), - }, - }, - }, - }, + onyxMethod: Onyx.METHOD.SET, + key: ruleKey, + value: rule ? {...rule, pendingAction: null, errors: ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('common.genericErrorMessage')} : null, }, ], }; + // An empty codingRuleValue tells SetPolicyCodingRule to delete rather than upsert the rule. const parameters: SetPolicyCodingRuleParams = { - policyID: policy.id, + policyID, codingRuleID: ruleID, codingRuleValue: '', shouldUpdateMatchingTransactions: false, @@ -625,33 +534,20 @@ function deletePolicyAgentRule(policy: Policy, agentRuleID: string) { API.write(WRITE_COMMANDS.DELETE_POLICY_AGENT_RULE, parameters, onyxData); } -function clearPolicyCodingRuleErrors(policyID: string, ruleID: string, rule: CodingRule | undefined) { +function clearMerchantRuleErrors(ruleID: string, rule: Rule | undefined) { if (!rule) { return; } - const policyKey = `${ONYXKEYS.COLLECTION.POLICY}${policyID}` as const; + const ruleKey = `${ONYXKEYS.COLLECTION.RULE}${ruleID}` as const; + // A rule that never made it to the server has nothing to keep once its error is dismissed. if (rule.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD) { - Onyx.merge(policyKey, { - rules: { - codingRules: { - [ruleID]: null, - }, - }, - }); + Onyx.set(ruleKey, null); return; } - Onyx.merge(policyKey, { - rules: { - codingRules: { - [ruleID]: { - errors: null, - }, - }, - }, - }); + Onyx.merge(ruleKey, {errors: null}); } function clearPolicyAgentRuleErrors(policyID: string, agentRuleID: string, agentRule: AgentRule | undefined) { @@ -686,16 +582,15 @@ function clearPolicyAgentRuleErrors(policyID: string, agentRuleID: string, agent export { openPolicyRulesPage, getAgentRuleSuggestions, - mapFormFieldsToRuleForOnyx, - mapFormFieldsToRuleForAPI, - setPolicyCodingRule, + getRules, + setMerchantRule, importMerchantRulesSpreadsheet, - deletePolicyCodingRule, + deleteMerchantRule, getTransactionsMatchingCodingRule, addPolicyAgentRule, updatePolicyAgentRule, deletePolicyAgentRule, - clearPolicyCodingRuleErrors, + clearMerchantRuleErrors, clearPolicyAgentRuleErrors, }; export type {ImportedMerchantRule}; diff --git a/src/pages/home/GettingStartedSection/hooks/useGettingStartedItems.ts b/src/pages/home/GettingStartedSection/hooks/useGettingStartedItems.ts index 0ff73a1d9efd..f184bece7699 100644 --- a/src/pages/home/GettingStartedSection/hooks/useGettingStartedItems.ts +++ b/src/pages/home/GettingStartedSection/hooks/useGettingStartedItems.ts @@ -4,6 +4,7 @@ import useLocalize from '@hooks/useLocalize'; import useOnboardingIntent from '@hooks/useOnboardingIntent'; import useOnyx from '@hooks/useOnyx'; import useResponsiveLayout from '@hooks/useResponsiveLayout'; +import useRulesPrefetch from '@hooks/useRulesPrefetch'; import useWorkspaceAccountID from '@hooks/useWorkspaceAccountID'; import {startMoneyRequest} from '@libs/actions/IOU/MoneyRequest'; @@ -35,6 +36,7 @@ import type {Route} from '@src/ROUTES'; import ROUTES, {DYNAMIC_ROUTES} from '@src/ROUTES'; import {hasIssuedExpensifyCardSelector} from '@selectors/Card'; +import {createHasExpenseDefaultRulesSelector} from '@selectors/Rule'; import {accountIDSelector} from '@selectors/Session'; import {validTransactionDraftIDsSelector} from '@selectors/TransactionDraft'; @@ -68,11 +70,15 @@ function useGettingStartedItems(): UseGettingStartedItemsResult { const {shouldUseNarrowLayout} = useResponsiveLayout(); const intent = useOnboardingIntent(); const [activePolicyID] = useOnyx(ONYXKEYS.NVP_ACTIVE_POLICY_ID); + const [hasMerchantRules] = useOnyx(ONYXKEYS.COLLECTION.RULE, {selector: createHasExpenseDefaultRulesSelector(activePolicyID)}); const [currentUserAccountID] = useOnyx(ONYXKEYS.SESSION, {selector: accountIDSelector}); const [firstDayFreeTrial] = useOnyx(ONYXKEYS.NVP_FIRST_DAY_FREE_TRIAL); const [reportedIntegration] = useOnyx(ONYXKEYS.ONBOARDING_USER_REPORTED_INTEGRATION); const [policy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${activePolicyID}`); const [policyCategories] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${activePolicyID}`); + + // The checklist reads the rules collection, and nothing on Home opens a workspace to populate it. + useRulesPrefetch(arePolicyRulesEnabled(policy, policyCategories)); const [allCardFeeds] = useCardFeeds(activePolicyID); const workspaceAccountID = useWorkspaceAccountID(activePolicyID); @@ -344,7 +350,7 @@ function useGettingStartedItems(): UseGettingStartedItemsResult { key: 'setupRules', label: translate('homePage.gettingStartedSection.setupRules'), subText: translate('homePage.gettingStartedSection.setupRulesSubText'), - isComplete: hasConfiguredRules(policy, policyCategories), + isComplete: hasConfiguredRules(policy, policyCategories, hasMerchantRules), route: ROUTES.WORKSPACE_RULES.getRoute(activePolicyID), }); } diff --git a/src/pages/workspace/WorkspaceInitialPage.tsx b/src/pages/workspace/WorkspaceInitialPage.tsx index 49f7720d21c8..3eba6d082a72 100644 --- a/src/pages/workspace/WorkspaceInitialPage.tsx +++ b/src/pages/workspace/WorkspaceInitialPage.tsx @@ -26,6 +26,7 @@ import useWorkspaceAccountID from '@hooks/useWorkspaceAccountID'; import {isConnectionInProgress} from '@libs/actions/connections'; import {clearErrors, openPolicyInitialPage, removeWorkspace} from '@libs/actions/Policy/Policy'; +import {getRules} from '@libs/actions/Policy/Rules'; import goBackFromWorkspaceSettingPages from '@libs/Navigation/helpers/goBackFromWorkspaceSettingPages'; import WorkspaceCreationReveal from '@libs/Navigation/helpers/WorkspaceCreationReveal'; import Navigation from '@libs/Navigation/Navigation'; @@ -44,8 +45,9 @@ import {isEmptyObject} from '@src/types/utils/EmptyObject'; import type {LayoutChangeEvent} from 'react-native'; import {findFocusedRoute, useFocusEffect, useIsFocused, useNavigationState} from '@react-navigation/native'; +import {createHasExpenseDefaultRuleErrorsSelector} from '@selectors/Rule'; import {emailSelector} from '@selectors/Session'; -import React, {useCallback, useEffect, useRef} from 'react'; +import React, {useCallback, useEffect, useMemo, useRef} from 'react'; import {View} from 'react-native'; import type {WithPolicyAndFullscreenLoadingProps} from './withPolicyAndFullscreenLoading'; @@ -87,6 +89,8 @@ function WorkspaceInitialPage({policyDraft, policy: policyProp, route}: Workspac const [connectionSyncProgress] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_CONNECTION_SYNC_PROGRESS}${policyID}`); const [policyCategories] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${routePolicyID}`); + const hasMerchantRuleErrorsSelector = useMemo(() => createHasExpenseDefaultRuleErrorsSelector(policyID), [policyID]); + const [hasMerchantRuleErrors] = useOnyx(ONYXKEYS.COLLECTION.RULE, {selector: hasMerchantRuleErrorsSelector}); const workspaceAccountID = useWorkspaceAccountID(policyID); const {shouldShowEnterCredentialsError} = useGetReceiptPartnersIntegrationData(policyID); const {shouldShowRbrForWorkspaceAccountID} = useCardFeedErrors(); @@ -147,6 +151,8 @@ function WorkspaceInitialPage({policyDraft, policy: policyProp, route}: Workspac return; } openPolicyInitialPage(routePolicyID); + // The rules collection is keyed per rule rather than per policy, so it is fetched whole whenever a workspace is opened. + getRules(); }; useNetwork({onReconnect: fetchPolicyData}); useFocusEffect( @@ -179,6 +185,7 @@ function WorkspaceInitialPage({policyDraft, policy: policyProp, route}: Workspac icons: expensifyIcons, isConnectionInProgress: isConnectionInProgress(connectionSyncProgress, policy), policyCategories, + hasMerchantRuleErrors, previousPendingFields: prevPendingFields, shouldShowEnterCredentialsError, shouldShowRBR, diff --git a/src/pages/workspace/copyPolicySettings/CopyPolicySettingsConfirmPage.tsx b/src/pages/workspace/copyPolicySettings/CopyPolicySettingsConfirmPage.tsx index e152ed1de359..a68f3f0122c4 100644 --- a/src/pages/workspace/copyPolicySettings/CopyPolicySettingsConfirmPage.tsx +++ b/src/pages/workspace/copyPolicySettings/CopyPolicySettingsConfirmPage.tsx @@ -42,6 +42,7 @@ function CopyPolicySettingsConfirmPage() { const [copyPolicySettingsState, copyPolicySettingsMetadata] = useOnyx(ONYXKEYS.COPY_POLICY_SETTINGS); const [allPolicyCategories] = useOnyx(ONYXKEYS.COLLECTION.POLICY_CATEGORIES); const [allPolicyTags] = useOnyx(ONYXKEYS.COLLECTION.POLICY_TAGS); + const [allRules] = useOnyx(ONYXKEYS.COLLECTION.RULE); const sourcePolicy = sourcePolicyID ? policies?.[`${ONYXKEYS.COLLECTION.POLICY}${sourcePolicyID}`] : undefined; const targetPolicyIDs = copyPolicySettingsState?.targetPolicyIDs ?? []; @@ -91,7 +92,7 @@ function CopyPolicySettingsConfirmPage() { Navigation.navigate(ROUTES.POLICY_COPY_SETTINGS_UPGRADE.getRoute(sourcePolicyID)); return; } - copyPolicySettings(sourcePolicy, targetPolicies, parts, allPolicyCategories, allPolicyTags); + copyPolicySettings(sourcePolicy, targetPolicies, parts, allPolicyCategories, allPolicyTags, allRules); Navigation.dismissModal(); }; diff --git a/src/pages/workspace/copyPolicySettings/CopyPolicySettingsSelectFeaturesPage.tsx b/src/pages/workspace/copyPolicySettings/CopyPolicySettingsSelectFeaturesPage.tsx index 2e66b78373e6..625fb075d8be 100644 --- a/src/pages/workspace/copyPolicySettings/CopyPolicySettingsSelectFeaturesPage.tsx +++ b/src/pages/workspace/copyPolicySettings/CopyPolicySettingsSelectFeaturesPage.tsx @@ -12,6 +12,7 @@ import useConfirmModal from '@hooks/useConfirmModal'; import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails'; import useLocalize from '@hooks/useLocalize'; import useOnyx from '@hooks/useOnyx'; +import useRulesPrefetch from '@hooks/useRulesPrefetch'; import useThemeStyles from '@hooks/useThemeStyles'; import {setCopyPolicySettingsData} from '@libs/actions/Policy/CopyPolicySettings'; @@ -46,6 +47,7 @@ import type SCREENS from '@src/SCREENS'; import {isEmptyObject} from '@src/types/utils/EmptyObject'; import {useRoute} from '@react-navigation/native'; +import {createExpenseDefaultRuleCountSelector} from '@selectors/Rule'; import React, {useEffect, useState} from 'react'; import {View} from 'react-native'; @@ -73,6 +75,8 @@ function CopyPolicySettingsSelectFeaturesPage() { const [copyPolicySettings] = useOnyx(ONYXKEYS.COPY_POLICY_SETTINGS); const [policyTags] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${sourcePolicyID}`); const [policyCategories] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${sourcePolicyID}`); + const [codingRulesCount = 0] = useOnyx(ONYXKEYS.COLLECTION.RULE, {selector: createExpenseDefaultRuleCountSelector(sourcePolicyID)}); + useRulesPrefetch(); const currentUserPersonalDetails = useCurrentUserPersonalDetails(); const sourcePolicy = sourcePolicyID ? policies?.[`${ONYXKEYS.COLLECTION.POLICY}${sourcePolicyID}`] : undefined; @@ -107,7 +111,6 @@ function CopyPolicySettingsSelectFeaturesPage() { const policyFields = Object.values(getReportFieldsByPolicyID(sourcePolicy) ?? {}).filter((field) => field.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE); const reportFieldsCount = policyFields.filter((field) => field.target !== CONST.REPORT_FIELD_TARGETS.INVOICE).length; const invoiceFieldsCount = policyFields.filter((field) => field.target === CONST.REPORT_FIELD_TARGETS.INVOICE).length; - const codingRulesCount = Object.values(sourcePolicy?.rules?.codingRules ?? {}).filter((rule) => rule.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE).length; const connectedIntegration = getAllValidConnectedIntegration(sourcePolicy, CONST.POLICY.CONNECTIONS.ACCOUNTING_CONNECTION_NAMES); const distanceRatesCount = Object.values(getDistanceRateCustomUnit(sourcePolicy)?.rates ?? {}).filter((rate) => rate.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE).length; const perDiemRates = getPerDiemCustomUnit(sourcePolicy)?.rates ?? {}; diff --git a/src/pages/workspace/duplicate/WorkspaceDuplicateSelectFeaturesForm.tsx b/src/pages/workspace/duplicate/WorkspaceDuplicateSelectFeaturesForm.tsx index 087838071f51..ba61f045de70 100644 --- a/src/pages/workspace/duplicate/WorkspaceDuplicateSelectFeaturesForm.tsx +++ b/src/pages/workspace/duplicate/WorkspaceDuplicateSelectFeaturesForm.tsx @@ -12,8 +12,10 @@ import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails' import useLocalize from '@hooks/useLocalize'; import useOnyx from '@hooks/useOnyx'; import usePolicy from '@hooks/usePolicy'; +import useRulesPrefetch from '@hooks/useRulesPrefetch'; import useThemeStyles from '@hooks/useThemeStyles'; +import {getExpenseDefaultRuleCount} from '@libs/ExpenseDefaultRuleUtils'; import {readFileAsync} from '@libs/fileDownload/FileUtils'; import {createFilteredMemberCountSelector, createInvoiceConfigurationTextSelector, getDistanceRateCustomUnit, getPerDiemCustomUnit, isCollectPolicy} from '@libs/PolicyUtils'; import {formatAddressToString} from '@libs/ReportActionsUtils'; @@ -45,12 +47,14 @@ function WorkspaceDuplicateSelectFeaturesForm({policyID}: WorkspaceDuplicateForm const isCollect = isCollectPolicy(policy); const {showConfirmModal} = useConfirmModal(); const [duplicateWorkspace] = useOnyx(ONYXKEYS.DUPLICATE_WORKSPACE); + const [allRules] = useOnyx(ONYXKEYS.COLLECTION.RULE); + useRulesPrefetch(); const [duplicatedWorkspaceAvatar, setDuplicatedWorkspaceAvatar] = useState(); const [policyTags] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`); const taxesLength = Object.values(policy?.taxRates?.taxes ?? {}).filter((tax) => tax.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE).length ?? 0; const [policyCategories] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${policyID}`); const categoriesCount = Object.values(policyCategories ?? {}).filter((category) => category.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE).length; - const codingRulesCount = Object.values(policy?.rules?.codingRules ?? {}).filter((rule) => rule.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE).length; + const codingRulesCount = getExpenseDefaultRuleCount(allRules, policy?.id); const [selectedItems, setSelectedItems] = useState([]); const policyFields = Object.values(getReportFieldsByPolicyID(policy) ?? {}).filter((field) => field.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE); const reportFields = policyFields.filter((field) => field.target !== CONST.REPORT_FIELD_TARGETS.INVOICE).length; @@ -266,9 +270,11 @@ function WorkspaceDuplicateSelectFeaturesForm({policyID}: WorkspaceDuplicateForm }, file: duplicatedWorkspaceAvatar, localCurrency: currentUserPersonalDetails?.localCurrencyCode ?? CONST.CURRENCY.USD, + rules: allRules, }); Navigation.closeRHPFlow(); }, [ + allRules, duplicateWorkspace?.name, duplicateWorkspace?.policyID, policy, diff --git a/src/pages/workspace/getWorkspaceMenuItems.ts b/src/pages/workspace/getWorkspaceMenuItems.ts index 320fae3fd8bc..352849927b2f 100644 --- a/src/pages/workspace/getWorkspaceMenuItems.ts +++ b/src/pages/workspace/getWorkspaceMenuItems.ts @@ -98,6 +98,8 @@ type GetWorkspaceMenuItemsParams = { isConnectionInProgress?: boolean; /** Categories used to determine category-related errors. */ policyCategories?: OnyxTypes.PolicyCategories; + /** Whether any of the policy's merchant rules failed to save, used to surface a red dot on the Rules row. */ + hasMerchantRuleErrors?: boolean; /** Previous pending fields used to identify the most recently enabled feature. */ previousPendingFields?: OnyxTypes.Policy['pendingFields']; /** Whether receipt partner credentials require attention. */ @@ -117,6 +119,7 @@ function getWorkspaceMenuItems({ icons, isConnectionInProgress = false, policyCategories, + hasMerchantRuleErrors, previousPendingFields, shouldShowEnterCredentialsError = false, shouldShowRBR = false, @@ -326,7 +329,7 @@ function getWorkspaceMenuItems({ translationKey: 'workspace.common.rules', icon: icons.Bolt, getRoute: () => ROUTES.WORKSPACE_RULES.getRoute(policyID), - brickRoadIndicator: hasPolicyRulesError(policy) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined, + brickRoadIndicator: hasPolicyRulesError(policy, hasMerchantRuleErrors) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined, screenName: SCREENS.WORKSPACE.RULES, sentryLabel: CONST.SENTRY_LABEL.WORKSPACE.INITIAL.RULES, highlighted: highlightedPolicyFeature === CONST.POLICY.MORE_FEATURES.ARE_RULES_ENABLED, diff --git a/src/pages/workspace/rules/MerchantRules/ImportedMerchantRulesPage.tsx b/src/pages/workspace/rules/MerchantRules/ImportedMerchantRulesPage.tsx index 3665414663ba..45be622dea1b 100644 --- a/src/pages/workspace/rules/MerchantRules/ImportedMerchantRulesPage.tsx +++ b/src/pages/workspace/rules/MerchantRules/ImportedMerchantRulesPage.tsx @@ -15,6 +15,8 @@ import type {ImportedMerchantRule} from '@libs/actions/Policy/Rules'; import {importMerchantRulesSpreadsheet} from '@libs/actions/Policy/Rules'; import Tab from '@libs/actions/Tab'; import {getDecodedCategoryName} from '@libs/CategoryUtils'; +import {getMerchantRuleFormValues, getPolicyExpenseDefaultRules} from '@libs/ExpenseDefaultRuleUtils'; +import type {MerchantRuleFormValues} from '@libs/ExpenseDefaultRuleUtils'; import {findDuplicate, generateColumnNames} from '@libs/importSpreadsheetUtils'; import Navigation from '@libs/Navigation/Navigation'; import type {PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types'; @@ -32,13 +34,12 @@ import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import ROUTES from '@src/ROUTES'; import type SCREENS from '@src/SCREENS'; -import type {ImportedSpreadsheet, Policy, PolicyCategories} from '@src/types/onyx'; +import type {ImportedSpreadsheet, Policy, PolicyCategories, Rule} from '@src/types/onyx'; import type {ImportFinalModal} from '@src/types/onyx/ImportedSpreadsheet'; import type {Errors} from '@src/types/onyx/OnyxCommon'; -import type {CodingRule} from '@src/types/onyx/Policy'; import isLoadingOnyxValue from '@src/types/utils/isLoadingOnyxValue'; -import type {OnyxEntry} from 'react-native-onyx'; +import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; import {useFocusEffect} from '@react-navigation/native'; import React, {useCallback, useMemo, useState} from 'react'; @@ -58,16 +59,16 @@ const ACTION_COLUMNS: string[] = [ * spreadsheet rows that would recreate a rule the policy already has (e.g. the same spreadsheet * imported twice) as well as duplicate rows within the same spreadsheet. */ -function getRuleContentKey(rule: Pick): string { +function getRuleContentKey(formValues: Partial): string { return JSON.stringify([ - rule.filters.operator, - rule.filters.right.toLowerCase(), - rule.merchant ?? '', - rule.category ?? '', - rule.tag ?? '', - rule.comment ?? '', - rule.reimbursable ?? null, - rule.billable ?? null, + formValues.matchType ?? CONST.SEARCH.SYNTAX_OPERATORS.CONTAINS, + formValues.merchantToMatch?.toLowerCase() ?? '', + formValues.merchant ?? '', + formValues.category ?? '', + formValues.tag ?? '', + formValues.comment ?? '', + formValues.reimbursable ?? null, + formValues.billable ?? null, ]); } @@ -146,6 +147,7 @@ function parseSpreadsheetRules( containsHeader: boolean, policy: OnyxEntry, policyCategories: OnyxEntry, + existingRules: OnyxCollection, ): ParsedSpreadsheetRules { const columns = Object.values(spreadsheet?.columns ?? {}); const merchantIsColumn = columns.findIndex((column) => column === CONST.CSV_IMPORT_COLUMNS.MERCHANT_IS); @@ -167,9 +169,13 @@ function parseSpreadsheetRules( }; // Seed the duplicate check with the policy's current rules so re-importing a spreadsheet doesn't recreate them + // Rules the editor can't represent are skipped here: their content key can't be computed, so they can't be + // matched against a spreadsheet row anyway. const seenRuleKeys = new Set( - Object.values(policy?.rules?.codingRules ?? {}) - .filter((rule) => rule.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE && rule.filters?.right) + getPolicyExpenseDefaultRules(existingRules, policy?.id) + .filter(({rule}) => rule.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE) + .map(({rule}) => getMerchantRuleFormValues(rule)) + .filter((formValues) => !!formValues) .map(getRuleContentKey), ); let skippedDuplicateCount = 0; @@ -205,10 +211,21 @@ function parseSpreadsheetRules( continue; } + const formValues: Partial = { + merchantToMatch, + matchType: merchantIsValue ? CONST.SEARCH.SYNTAX_OPERATORS.EQUAL_TO : CONST.SEARCH.SYNTAX_OPERATORS.CONTAINS, + ...(updatedMerchant && {merchant: updatedMerchant}), + ...(category && {category}), + ...(tag && {tag}), + ...(comment && {comment}), + ...(reimbursable !== undefined && {reimbursable}), + ...(billable !== undefined && {billable}), + }; + const rule: ImportedMerchantRule = { filters: { left: 'merchant', - operator: merchantIsValue ? CONST.SEARCH.SYNTAX_OPERATORS.EQUAL_TO : CONST.SEARCH.SYNTAX_OPERATORS.CONTAINS, + operator: formValues.matchType ?? CONST.SEARCH.SYNTAX_OPERATORS.CONTAINS, right: merchantToMatch, }, ...(updatedMerchant && {merchant: updatedMerchant}), @@ -220,7 +237,7 @@ function parseSpreadsheetRules( created: new Date().toISOString(), }; - const ruleKey = getRuleContentKey(rule); + const ruleKey = getRuleContentKey(formValues); if (seenRuleKeys.has(ruleKey)) { skippedDuplicateCount++; continue; @@ -253,6 +270,7 @@ function ImportedMerchantRulesPage({route}: ImportedMerchantRulesPageProps) { const policyID = route.params.policyID; const policy = usePolicy(policyID); const [policyCategories] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${policyID}`); + const [rules] = useOnyx(ONYXKEYS.COLLECTION.RULE); // Fetch categories if they're not loaded (e.g. after a cache clear) so imported category cells are // validated against the policy's real category list instead of an empty one @@ -320,7 +338,7 @@ function ImportedMerchantRulesPage({route}: ImportedMerchantRulesPageProps) { // Parse once and reuse the result for both the offline button-enablement check and the import itself, so the // button can never be enabled offline for an import that actually needs the (non-retryable) API call - const parsedRules = useMemo(() => parseSpreadsheetRules(spreadsheet, containsHeader, policy, policyCategories), [spreadsheet, containsHeader, policy, policyCategories]); + const parsedRules = useMemo(() => parseSpreadsheetRules(spreadsheet, containsHeader, policy, policyCategories, rules), [spreadsheet, containsHeader, policy, policyCategories, rules]); // When categories are enabled but not yet cached (e.g. after a cache clear, before the on-focus fetch), the // category lookup is empty so every category is wrongly flagged invalid. Invalid-category counts are only @@ -338,7 +356,7 @@ function ImportedMerchantRulesPage({route}: ImportedMerchantRulesPageProps) { return; } - const {rules, skippedDuplicateCount, invalidCategoryNames} = parsedRules; + const {rules: parsedMerchantRules, skippedDuplicateCount, invalidCategoryNames} = parsedRules; setIsImportingRules(true); // When every row was skipped (duplicate rules and/or unknown categories), skip the API call and confirm that nothing was added @@ -352,7 +370,7 @@ function ImportedMerchantRulesPage({route}: ImportedMerchantRulesPageProps) { pendingMessageKeyParams: {count: invalidCategoryNames.size}, }), } - : await importMerchantRulesSpreadsheet(policyID, rules, invalidCategoryNames.size); + : await importMerchantRulesSpreadsheet(policyID, parsedMerchantRules, invalidCategoryNames.size); const didShowImportFinalModal = await showImportSpreadsheetConfirmModal(importFinalModal, {shouldHandleNavigationBack: false}); if (!didShowImportFinalModal) { setIsImportingRules(false); diff --git a/src/pages/workspace/rules/MerchantRules/MerchantRulePageBase.tsx b/src/pages/workspace/rules/MerchantRules/MerchantRulePageBase.tsx index 510fa32d8415..80e4babf43d4 100644 --- a/src/pages/workspace/rules/MerchantRules/MerchantRulePageBase.tsx +++ b/src/pages/workspace/rules/MerchantRules/MerchantRulePageBase.tsx @@ -24,12 +24,13 @@ import usePressLoading from '@hooks/usePressLoading'; import useThemeStyles from '@hooks/useThemeStyles'; import {deletePolicyCategoryTax, movePolicyCategoryTax, openPolicyCategoriesPage, setPolicyCategoryTaxes} from '@libs/actions/Policy/Category'; -import {deletePolicyCodingRule, setPolicyCodingRule} from '@libs/actions/Policy/Rules'; +import {deleteMerchantRule, setMerchantRule} from '@libs/actions/Policy/Rules'; import {openPolicyTagsPage} from '@libs/actions/Policy/Tag'; import Tab from '@libs/actions/Tab'; import {clearDraftMerchantRule, setDraftMerchantRule} from '@libs/actions/User'; import {getCategoryTaxRuleTaxID, getTaxRateDisplayName, hasUsableTaxRates, isCategoryRuleDraft} from '@libs/CategoryTaxRulesUtils'; import {getDecodedCategoryName} from '@libs/CategoryUtils'; +import {canEditMerchantRule, getMerchantRuleFormValues, getPolicyExpenseDefaultRules} from '@libs/ExpenseDefaultRuleUtils'; import Navigation from '@libs/Navigation/Navigation'; import {hasEnabledOptions} from '@libs/OptionsListUtils'; import Parser from '@libs/Parser'; @@ -50,7 +51,6 @@ import type {MerchantRuleForm} from '@src/types/form'; import MERCHANT_RULE_INPUT_IDS from '@src/types/form/MerchantRuleForm'; import type {ExpenseDefaultRuleType} from '@src/types/form/MerchantRuleForm'; import type {PolicyTagLists} from '@src/types/onyx'; -import type {CodingRule} from '@src/types/onyx/Policy'; import getEmptyArray from '@src/types/utils/getEmptyArray'; import type IconAsset from '@src/types/utils/IconAsset'; @@ -179,8 +179,9 @@ function MerchantRulePageBase({policyID, ruleID, initialCategoryName, editCatego // the workspace has no accounting connection, and when the data has already been fetched. usePolicyConnectionsPrefetch(policy, true); - // Get the existing rule from the policy (for edit mode) - const existingRule = ruleID ? policy?.rules?.codingRules?.[ruleID] : undefined; + const [rules] = useOnyx(ONYXKEYS.COLLECTION.RULE); + // Get the existing rule from the rules collection (for edit mode) + const existingRule = ruleID ? rules?.[`${ONYXKEYS.COLLECTION.RULE}${ruleID}`] : undefined; const existingCategoryTaxID = editCategoryTaxRuleFor ? getCategoryTaxRuleTaxID(policy?.rules?.expenseRules, editCategoryTaxRuleFor) : undefined; // Initialize the form with existing rule data (for edit mode) @@ -205,27 +206,13 @@ function MerchantRulePageBase({policyID, ruleID, initialCategoryName, editCatego return; } - if (!existingRule) { + // An undefined result means the rule uses parts of the format this form can't show. Saving it back would + // drop them, so the editor stays empty and the rules list keeps such rules read-only. + const formValues = getMerchantRuleFormValues(existingRule); + if (!formValues) { return; } - - // Convert the operator to matchType for the form - // 'eq' = exact match, 'contains' = contains match - const matchType = existingRule.filters?.operator; - // Convert HTML comment back to markdown for editing - const commentMarkdown = existingRule.comment ? Parser.htmlToMarkdown(existingRule.comment) : undefined; - setDraftMerchantRule({ - merchantToMatch: existingRule.filters?.right, - matchType, - merchant: existingRule.merchant, - category: existingRule.category, - tag: existingRule.tag, - tax: existingRule.tax?.field_id_TAX?.externalID, - vendorID: existingRule.vendorID, - comment: commentMarkdown, - reimbursable: existingRule.reimbursable, - billable: existingRule.billable, - }); + setDraftMerchantRule(formValues); }, [isEditing, existingRule, isEditingCategoryTaxRule, editCategoryTaxRuleFor, existingCategoryTaxID, initialCategoryName]); // Clear the form on unmount @@ -331,29 +318,27 @@ function MerchantRulePageBase({policyID, ruleID, initialCategoryName, editCatego * A duplicate is a rule that has the same merchant to match AND the same match type (contains/exact). * When editing, we exclude the current rule from the comparison. */ - const checkForDuplicateRule = (codingRules: Record | undefined, merchantToMatch: string | undefined, matchType: string | undefined): boolean => { - if (!codingRules || !merchantToMatch) { + const checkForDuplicateRule = (merchantToMatch: string | undefined, matchType: string | undefined): boolean => { + if (!merchantToMatch) { return false; } const normalizedMerchant = merchantToMatch.toLowerCase(); const currentMatchType = matchType ?? CONST.SEARCH.SYNTAX_OPERATORS.CONTAINS; - const defaultMatchType = CONST.SEARCH.SYNTAX_OPERATORS.CONTAINS; - return Object.entries(codingRules).some(([existingRuleID, rule]) => { + return getPolicyExpenseDefaultRules(rules, policyID).some(({ruleID: existingRuleID, rule}) => { // Skip the rule being edited if (isEditing && existingRuleID === ruleID) { return false; } - if (!rule?.filters?.right) { + // A rule this form can't represent can't be a duplicate of what this form is about to save. + const existingFormValues = getMerchantRuleFormValues(rule); + if (!existingFormValues) { return false; } - const existingMerchant = rule.filters.right.toLowerCase(); - const existingMatchType = rule.filters.operator ?? defaultMatchType; - - if (existingMerchant !== normalizedMerchant || existingMatchType !== currentMatchType) { + if (existingFormValues.merchantToMatch.toLowerCase() !== normalizedMerchant || existingFormValues.matchType !== currentMatchType) { return false; } @@ -410,7 +395,7 @@ function MerchantRulePageBase({policyID, ruleID, initialCategoryName, editCatego return; } - setPolicyCodingRule(policyID, form, policy, ruleID, shouldUpdateMatchingTransactions); + setMerchantRule(policyID, form, policy, ruleID, existingRule, shouldUpdateMatchingTransactions); if (isCreatedFromExpense) { // Opened from the callout, so this page is a suffix on the expense's path. Dropping it returns to the // expense instead of the workspace Rules page. @@ -442,7 +427,7 @@ function MerchantRulePageBase({policyID, ruleID, initialCategoryName, editCatego } // Check for duplicate rules - const hasDuplicate = checkForDuplicateRule(policy?.rules?.codingRules, form.merchantToMatch, form.matchType); + const hasDuplicate = checkForDuplicateRule(form.merchantToMatch, form.matchType); if (hasDuplicate) { showConfirmModal({ title: translate('workspace.rules.merchantRules.duplicateRuleTitle'), @@ -469,7 +454,7 @@ function MerchantRulePageBase({policyID, ruleID, initialCategoryName, editCatego if (editCategoryTaxRuleFor) { deletePolicyCategoryTax(policy, editCategoryTaxRuleFor); } else if (ruleID) { - deletePolicyCodingRule(policy, ruleID); + deleteMerchantRule(policy.id, ruleID, existingRule); } return true; }; @@ -625,6 +610,12 @@ function MerchantRulePageBase({policyID, ruleID, initialCategoryName, editCatego return ; } + // The rules collection is shared across workspaces and rule kinds, so a stale link can resolve a ruleID that + // this editor must not write to. Saving would replace it with a merchant rule and drop whatever it holds. + if (ruleID && !!existingRule && !isClosing && !canEditMerchantRule(existingRule, policyID)) { + return ; + } + if (isEditingCategoryTaxRule && !existingCategoryTaxID && !isClosing) { return ; } diff --git a/src/pages/workspace/rules/PolicyRulesPageRevamp.tsx b/src/pages/workspace/rules/PolicyRulesPageRevamp.tsx index de9dfe0f741b..96f8fd550e1b 100644 --- a/src/pages/workspace/rules/PolicyRulesPageRevamp.tsx +++ b/src/pages/workspace/rules/PolicyRulesPageRevamp.tsx @@ -20,7 +20,7 @@ import useThemeStyles from '@hooks/useThemeStyles'; import useWorkspaceDocumentTitle from '@hooks/useWorkspaceDocumentTitle'; import {turnOffMobileSelectionMode} from '@libs/actions/MobileSelectionMode'; -import {openPolicyRulesPage} from '@libs/actions/Policy/Rules'; +import {getRules, openPolicyRulesPage} from '@libs/actions/Policy/Rules'; import Tab from '@libs/actions/Tab'; import Navigation from '@libs/Navigation/Navigation'; import type {PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types'; @@ -95,8 +95,11 @@ function PolicyRulesPageRevamp({route}: PolicyRulesPageRevampProps) { const {showConfirmModal} = useConfirmModal(); useEffect(() => { - // Fetch once on mount (and when policyID changes). setPolicyCodingRule already updates Onyx — refetching after saves can overwrite a newly added rule with stale data. + // Fetch once on mount and whenever policyID changes. setMerchantRule already updates Onyx, so refetching + // after a save can overwrite a newly added rule with stale data. openPolicyRulesPage(policyID); + // The rules collection is keyed per rule rather than per policy, so it is fetched whole whenever the Rules page is opened. + getRules(); }, [policyID]); useEffect(() => { diff --git a/src/pages/workspace/rules/tabs/RulesExpenseDefaultsTab.tsx b/src/pages/workspace/rules/tabs/RulesExpenseDefaultsTab.tsx index 979f0bfd8e50..09eed50e8c65 100644 --- a/src/pages/workspace/rules/tabs/RulesExpenseDefaultsTab.tsx +++ b/src/pages/workspace/rules/tabs/RulesExpenseDefaultsTab.tsx @@ -24,12 +24,14 @@ function RulesExpenseDefaultsTab({policyID, canWriteRules, selectedKeys, onSelec const {translate} = useLocalize(); const {isOffline} = useNetwork(); const policy = usePolicy(policyID); + const [rules] = useOnyx(ONYXKEYS.COLLECTION.RULE); const [policyCategories] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${policyID}`); const expenseDefaultsTableData = getExpenseDefaultsTableData({ policy, policyID, + rules, policyCategories, translate, isOffline, diff --git a/src/pages/workspace/rules/tabs/useRulesTableBulkActions.ts b/src/pages/workspace/rules/tabs/useRulesTableBulkActions.ts index 1b246fbac368..76d8f9e3b2e2 100644 --- a/src/pages/workspace/rules/tabs/useRulesTableBulkActions.ts +++ b/src/pages/workspace/rules/tabs/useRulesTableBulkActions.ts @@ -18,7 +18,7 @@ import useThemeStyles from '@hooks/useThemeStyles'; import {deleteExpensifyCardRule} from '@libs/actions/Card'; import {deletePolicyCategoryTaxes, openPolicyCategoriesPage} from '@libs/actions/Policy/Category'; import {openPolicyExpensifyCardsPage} from '@libs/actions/Policy/Policy'; -import {deletePolicyCodingRule} from '@libs/actions/Policy/Rules'; +import {deleteMerchantRule} from '@libs/actions/Policy/Rules'; import {getCategoryNameFromTaxRuleKey, isCategoryTaxRuleKey} from '@libs/CategoryTaxRulesUtils'; import {deleteFlagForReviewRule, getFlagForReviewTableData} from '@libs/FlagForReviewRulesUtils'; import {getExpenseDefaultsTableData, isMerchantTypeRuleKey} from '@libs/MerchantTypeRulesUtils'; @@ -68,6 +68,7 @@ function useRulesTableBulkActions({policyID, activeTab, selectedRuleKeysByTab, c const [expensifyCardSettings] = useOnyx(`${ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS}${defaultFundID}`); const {cardRules} = useExpensifyCardRules(policyID); const [policyCategoriesOnyx] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${policyID}`); + const [rules] = useOnyx(ONYXKEYS.COLLECTION.RULE); const arePolicyCategoriesLoading = !!policy?.areCategoriesEnabled && policyCategoriesOnyx === undefined; const areCardsEnabled = !!policy?.areExpensifyCardsEnabled; const attemptedCardSettingsFetchRef = useRef>(new Set()); @@ -152,6 +153,7 @@ function useRulesTableBulkActions({policyID, activeTab, selectedRuleKeysByTab, c const expenseDefaultsTableData: ExpenseDefaultTableItem[] = getExpenseDefaultsTableData({ policy, policyID, + rules, // Unlike the tables below, the raw value: a category pending deletion is exactly what marks its rule deleting. policyCategories: policyCategoriesOnyx, translate, @@ -261,7 +263,7 @@ function useRulesTableBulkActions({policyID, activeTab, selectedRuleKeysByTab, c continue; } - deletePolicyCodingRule(policy, ruleID); + deleteMerchantRule(policyID, ruleID, rules?.[`${ONYXKEYS.COLLECTION.RULE}${ruleID}`]); } if (selectedCategoryNames.length > 0) { @@ -276,6 +278,7 @@ function useRulesTableBulkActions({policyID, activeTab, selectedRuleKeysByTab, c filteredSelectedExpenseDefaultKeys, filteredSelectedFlagForReviewRuleKeys, filteredSelectedRequireFieldsRuleKeys, + rules, filteredSelectedSpendRuleKeys, policy, policyData, diff --git a/src/selectors/Rule.ts b/src/selectors/Rule.ts new file mode 100644 index 000000000000..137663e4efc4 --- /dev/null +++ b/src/selectors/Rule.ts @@ -0,0 +1,23 @@ +/** + * Selectors for the `rules_` collection. + * + * `GetRules` SETs every rule the user can see across every workspace, so the collection reference changes + * on any rule write anywhere. A consumer that only needs a count or a flag reduces the collection to that + * value here, which keeps `useOnyx` from deep-comparing a filtered copy of it on each of those writes. + */ +import {getExpenseDefaultRuleCount, hasExpenseDefaultRuleErrors} from '@libs/ExpenseDefaultRuleUtils'; + +import type {Rule} from '@src/types/onyx'; + +import type {OnyxCollection} from 'react-native-onyx'; + +/** How many expense default rules the policy has, ignoring ones being deleted. */ +const createExpenseDefaultRuleCountSelector = (policyID: string | undefined) => (rules: OnyxCollection) => getExpenseDefaultRuleCount(rules, policyID); + +/** Whether the policy has at least one expense default rule. */ +const createHasExpenseDefaultRulesSelector = (policyID: string | undefined) => (rules: OnyxCollection) => getExpenseDefaultRuleCount(rules, policyID) > 0; + +/** Whether any of the policy's expense default rules failed to save. */ +const createHasExpenseDefaultRuleErrorsSelector = (policyID: string | undefined) => (rules: OnyxCollection) => hasExpenseDefaultRuleErrors(rules, policyID); + +export {createExpenseDefaultRuleCountSelector, createHasExpenseDefaultRuleErrorsSelector, createHasExpenseDefaultRulesSelector}; diff --git a/src/types/onyx/ApprovalWorkflowRules.ts b/src/types/onyx/ApprovalWorkflowRules.ts index dcf57b643cb9..d8060907ef55 100644 --- a/src/types/onyx/ApprovalWorkflowRules.ts +++ b/src/types/onyx/ApprovalWorkflowRules.ts @@ -2,6 +2,8 @@ import type CONST from '@src/CONST'; import type {ValueOf} from 'type-fest'; +import type {RuleFilterNode} from './RuleFilters'; + /** * A report lifecycle event that can fire an approval-workflow rule (`ReportSubmit` or `ReportApprove`). */ @@ -34,41 +36,6 @@ type ApprovalWorkflowAction = { */ type ApprovalWorkflowActions = Record; -/** - * The value a comparison's field is compared against. For email-typed fields like `from`, this is a list - * of emails to match against. - */ -type ApprovalWorkflowFilterValue = string | number | string[]; - -/** - * A single comparison node: ` `. Both `left` and `right` are always present. - */ -type ApprovalWorkflowFilterComparison = { - /** The comparison operator. */ - operator: ValueOf; - - /** The field identifier being compared — one of the search-syntax filter keys (`from`, `to`, `amount`). */ - left: string; - - /** The literal value being compared against. */ - right: ApprovalWorkflowFilterValue; -}; - -/** - * A boolean filter that combines two child nodes. `left` / `right` may each be either a leaf comparison - * or a nested boolean filter. Both children are always present. - */ -type ApprovalWorkflowFilter = { - /** Boolean combinator (`AND` in practice). */ - operator: ValueOf; - - /** Left-hand child: leaf comparison or nested boolean filter. */ - left: ApprovalWorkflowFilterComparison | ApprovalWorkflowFilter; - - /** Right-hand child: leaf comparison or nested boolean filter. */ - right: ApprovalWorkflowFilterComparison | ApprovalWorkflowFilter; -}; - /** * The body of a single approval-workflow rule. When the report event matches one of the `triggers` and * the `filters` match the report, the rule's `actions` are performed @@ -78,7 +45,7 @@ type ApprovalWorkflowRule = { triggers: ApprovalWorkflowTriggers; /** Conditions that must match the report for the rule to fire. */ - filters: ApprovalWorkflowFilter | ApprovalWorkflowFilterComparison; + filters: RuleFilterNode; /** What happens when the rule matches. */ actions: ApprovalWorkflowActions; @@ -87,4 +54,4 @@ type ApprovalWorkflowRule = { isDefaultApprovalWorkflow?: boolean; }; -export type {ApprovalWorkflowAction, ApprovalWorkflowActions, ApprovalWorkflowFilter, ApprovalWorkflowFilterComparison, ApprovalWorkflowRule, ApprovalWorkflowTriggers}; +export type {ApprovalWorkflowAction, ApprovalWorkflowActions, ApprovalWorkflowRule, ApprovalWorkflowTriggers}; diff --git a/src/types/onyx/ExpenseDefaultRules.ts b/src/types/onyx/ExpenseDefaultRules.ts new file mode 100644 index 000000000000..429ca6d3d3ae --- /dev/null +++ b/src/types/onyx/ExpenseDefaultRules.ts @@ -0,0 +1,86 @@ +import type CONST from '@src/CONST'; + +import type {ValueOf} from 'type-fest'; + +import type {RuleFilterNode} from './RuleFilters'; + +/** + * A transaction lifecycle event that can fire an expense-default rule (currently only `CreateTransaction`). + */ +type ExpenseDefaultTrigger = ValueOf; + +/** + * The triggers of a rule, keyed by a string index (e.g. `{"0": "CreateTransaction"}`). A rule fires when + * any of its triggers matches the transaction event. + */ +type ExpenseDefaultTriggers = Record; + +/** + * The name of the action a rule performs when it matches (currently only `Set`). + */ +type ExpenseDefaultActionName = ValueOf; + +/** + * The expense field a `Set` action writes to. + */ +type ExpenseDefaultActionField = ValueOf; + +/** + * The value of a `tax` action, wrapping the tax rate in the backend API format. + */ +type ExpenseDefaultTaxValue = { + /** Object wrapping the tax field - field_id_TAX matches the backend API format */ + // field_id_TAX is the name the rules engine gives this key, so it can't follow our casing convention + // eslint-disable-next-line @typescript-eslint/naming-convention + field_id_TAX: { + /** The external ID of the tax rate */ + externalID: string; + + /** The tax rate value (e.g. "8.5%") */ + value?: string; + + /** The name of the tax rate */ + name?: string; + }; +}; + +/** + * The value a `Set` action writes. Which of these is valid depends on the action's `field`. + */ +type ExpenseDefaultActionValue = string | boolean | ExpenseDefaultTaxValue; + +/** + * A single change applied to expenses that match the rule's filters. + */ +type ExpenseDefaultAction = { + /** What the rule does when it matches. */ + name: ExpenseDefaultActionName; + + /** The expense field the action writes to. */ + field: ExpenseDefaultActionField; + + /** The value written to `field`. */ + value: ExpenseDefaultActionValue; +}; + +/** + * The actions of a rule, keyed by a string index (e.g. `{"0": {"name": "Set", "field": "category", "value": "Travel"}}`). + */ +type ExpenseDefaultActions = Record; + +/** + * The body of a single expense-default rule (what merchant rules are stored as). When a transaction event + * matches one of the `triggers` and the `filters` match the transaction, the rule's `actions` are applied. + */ +type ExpenseDefaultRule = { + /** Transaction lifecycle events that fire this rule. */ + triggers: ExpenseDefaultTriggers; + + /** Conditions that must match the transaction for the rule to fire. */ + filters: RuleFilterNode; + + /** What gets applied to matching expenses. */ + actions: ExpenseDefaultActions; +}; + +export type {ExpenseDefaultAction, ExpenseDefaultActionField, ExpenseDefaultActions, ExpenseDefaultRule, ExpenseDefaultTaxValue, ExpenseDefaultTriggers}; diff --git a/src/types/onyx/Rule.ts b/src/types/onyx/Rule.ts index a48f1defc989..861e1831d070 100644 --- a/src/types/onyx/Rule.ts +++ b/src/types/onyx/Rule.ts @@ -3,22 +3,32 @@ import type CONST from '@src/CONST'; import type {ValueOf} from 'type-fest'; import type {ApprovalWorkflowRule} from './ApprovalWorkflowRules'; +import type {ExpenseDefaultRule} from './ExpenseDefaultRules'; import type {Errors, OnyxValueWithOfflineFeedback} from './OnyxCommon'; -/** The kind of entity a rule is scoped to (currently only workspace policies). */ +/** The kind of entity a rule is scoped to. */ type RuleScope = ValueOf; +/** + * The body of a rule, i.e. the `value` sent to `SetRule`. Which body a rule has is determined by its + * triggers and actions rather than by a discriminator field. + */ +type RuleBody = ApprovalWorkflowRule | ExpenseDefaultRule; + /** * A rule as stored in the `ONYXKEYS.COLLECTION.RULE` collection under `rules_`. */ type Rule = OnyxValueWithOfflineFeedback< - ApprovalWorkflowRule & { + RuleBody & { /** What kind of entity this rule is scoped to. */ scope: RuleScope; /** ID of the scoped entity (the policyID for `policy`-scoped rules). */ scopeID: string; + /** Determines the order rules are applied in when more than one matches. */ + priority?: number; + /** ISO timestamp of when the rule was created. */ created?: string; diff --git a/src/types/onyx/RuleFilters.ts b/src/types/onyx/RuleFilters.ts new file mode 100644 index 000000000000..72957d91c552 --- /dev/null +++ b/src/types/onyx/RuleFilters.ts @@ -0,0 +1,42 @@ +import type CONST from '@src/CONST'; + +import type {ValueOf} from 'type-fest'; + +/** + * The value a comparison's field is compared against. An array is matched as an OR of its entries. + */ +type RuleFilterValue = string | number | string[]; + +/** + * A single comparison node: ` `. Both `left` and `right` are always present. + */ +type RuleFilterComparison = { + /** The comparison operator. */ + operator: ValueOf; + + /** The field identifier being compared, one of the search-syntax filter keys such as `from`, `merchant` or `amount`. */ + left: string; + + /** The literal value being compared against. */ + right: RuleFilterValue; +}; + +/** + * A boolean filter that combines two child nodes. `left` / `right` may each be either a leaf comparison + * or a nested boolean filter. Both children are always present. + */ +type RuleFilter = { + /** Boolean combinator (`and` / `or`). */ + operator: ValueOf; + + /** Left-hand child: leaf comparison or nested boolean filter. */ + left: RuleFilterComparison | RuleFilter; + + /** Right-hand child: leaf comparison or nested boolean filter. */ + right: RuleFilterComparison | RuleFilter; +}; + +/** The root of a rule's filter tree: either a single comparison or a boolean combination of nodes. */ +type RuleFilterNode = RuleFilter | RuleFilterComparison; + +export type {RuleFilter, RuleFilterComparison, RuleFilterNode}; diff --git a/tests/actions/CopyPolicySettingsTest.ts b/tests/actions/CopyPolicySettingsTest.ts index 75a4c0d7e09b..2873642e4c80 100644 --- a/tests/actions/CopyPolicySettingsTest.ts +++ b/tests/actions/CopyPolicySettingsTest.ts @@ -181,7 +181,7 @@ describe('actions/Policy/CopyPolicySettings', () => { const sourcePolicy = makeSourcePolicy(); const targetPolicy = makeTargetPolicy(); - const {optimisticData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], [part], {}, {}); + const {optimisticData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], [part], {}, {}, {}); const policy = getOptimisticPolicy(optimisticData); expect(policy).toBeDefined(); @@ -200,7 +200,7 @@ describe('actions/Policy/CopyPolicySettings', () => { const sourcePolicy = makeSourcePolicy(); const targetPolicy = makeTargetPolicy(); - const {optimisticData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], ['overview'], {}, {}); + const {optimisticData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], ['overview'], {}, {}, {}); const policy = getOptimisticPolicy(optimisticData); expect(policy?.areCategoriesEnabled).toEqual(targetPolicy.areCategoriesEnabled); @@ -211,7 +211,7 @@ describe('actions/Policy/CopyPolicySettings', () => { const sourcePolicy = makeSourcePolicy({glCodes: true, showTagGLCodes: true}); const targetPolicy = makeTargetPolicy({glCodes: false, showTagGLCodes: false}); - const {optimisticData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], ['rules'], {}, {}); + const {optimisticData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], ['rules'], {}, {}, {}); const policy = getOptimisticPolicy(optimisticData); expect(policy?.glCodes).toBe(true); @@ -238,7 +238,7 @@ describe('actions/Policy/CopyPolicySettings', () => { }, }); - const {optimisticData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], ['travel'], {}, {}); + const {optimisticData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], ['travel'], {}, {}, {}); const policy = getOptimisticPolicy(optimisticData); // The travel toggle copies. @@ -270,7 +270,7 @@ describe('actions/Policy/CopyPolicySettings', () => { }; const targetPolicy = makeTargetPolicy({travelSettings: targetTravelSettings}); - const {optimisticData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], ['travel'], {}, {}); + const {optimisticData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], ['travel'], {}, {}, {}); const policy = getOptimisticPolicy(optimisticData); expect(policy?.travelSettings).toEqual(targetTravelSettings); @@ -288,7 +288,7 @@ describe('actions/Policy/CopyPolicySettings', () => { // Target has never been provisioned for travel, so it has no travelSettings. const targetPolicy = makeTargetPolicy(); - const {optimisticData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], ['travel'], {}, {}); + const {optimisticData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], ['travel'], {}, {}, {}); const policy = getOptimisticPolicy(optimisticData); // The source's autoAddTripName=false is reflected so the UI does not show the opposite preference. @@ -311,7 +311,7 @@ describe('actions/Policy/CopyPolicySettings', () => { [TARGET_CATEGORIES_KEY]: targetCategories, }; - const {optimisticData, failureData} = buildCopyPolicySettingsData(makeSourcePolicy(), [makeTargetPolicy()], ['categories'], allPolicyCategories, {}); + const {optimisticData, failureData} = buildCopyPolicySettingsData(makeSourcePolicy(), [makeTargetPolicy()], ['categories'], allPolicyCategories, {}, {}); const optimisticSet = optimisticData.find((u) => u.key === TARGET_CATEGORIES_KEY && u.onyxMethod === Onyx.METHOD.SET); const failureSet = failureData.find((u) => u.key === TARGET_CATEGORIES_KEY && u.onyxMethod === Onyx.METHOD.SET); @@ -329,7 +329,7 @@ describe('actions/Policy/CopyPolicySettings', () => { [TARGET_TAGS_KEY]: targetTags, }; - const {optimisticData, failureData} = buildCopyPolicySettingsData(makeSourcePolicy(), [makeTargetPolicy()], ['tags'], {}, allPolicyTags); + const {optimisticData, failureData} = buildCopyPolicySettingsData(makeSourcePolicy(), [makeTargetPolicy()], ['tags'], {}, allPolicyTags, {}); const optimisticSet = optimisticData.find((u) => u.key === TARGET_TAGS_KEY && u.onyxMethod === Onyx.METHOD.SET); const failureSet = failureData.find((u) => u.key === TARGET_TAGS_KEY && u.onyxMethod === Onyx.METHOD.SET); @@ -339,7 +339,7 @@ describe('actions/Policy/CopyPolicySettings', () => { }); it('does not emit POLICY_CATEGORIES updates when categories not selected', () => { - const {optimisticData, failureData} = buildCopyPolicySettingsData(makeSourcePolicy(), [makeTargetPolicy()], ['overview'], {}, {}); + const {optimisticData, failureData} = buildCopyPolicySettingsData(makeSourcePolicy(), [makeTargetPolicy()], ['overview'], {}, {}, {}); expect(optimisticData.some((u) => u.key === TARGET_CATEGORIES_KEY)).toBe(false); expect(failureData.some((u) => u.key === TARGET_CATEGORIES_KEY)).toBe(false); }); @@ -366,7 +366,7 @@ describe('actions/Policy/CopyPolicySettings', () => { [TARGET_CATEGORIES_KEY]: targetCategories, }; - const {optimisticData, failureData} = buildCopyPolicySettingsData(makeSourcePolicy(), [makeTargetPolicy()], ['rules'], allPolicyCategories, {}); + const {optimisticData, failureData} = buildCopyPolicySettingsData(makeSourcePolicy(), [makeTargetPolicy()], ['rules'], allPolicyCategories, {}, {}); const optimisticMerge = optimisticData.find((u) => u.key === TARGET_CATEGORIES_KEY && u.onyxMethod === Onyx.METHOD.MERGE); expect(optimisticMerge?.value).toEqual({ @@ -397,7 +397,7 @@ describe('actions/Policy/CopyPolicySettings', () => { [TARGET_CATEGORIES_KEY]: targetCategories, }; - const {optimisticData} = buildCopyPolicySettingsData(makeSourcePolicy(), [makeTargetPolicy()], ['rules', 'categories'], allPolicyCategories, {}); + const {optimisticData} = buildCopyPolicySettingsData(makeSourcePolicy(), [makeTargetPolicy()], ['rules', 'categories'], allPolicyCategories, {}, {}); expect(optimisticData.some((u) => u.key === TARGET_CATEGORIES_KEY && u.onyxMethod === Onyx.METHOD.MERGE)).toBe(false); expect(optimisticData.find((u) => u.key === TARGET_CATEGORIES_KEY && u.onyxMethod === Onyx.METHOD.SET)?.value).toEqual(sourceCategories); @@ -409,7 +409,7 @@ describe('actions/Policy/CopyPolicySettings', () => { [TARGET_CATEGORIES_KEY]: {Food: {name: 'Food', enabled: true}}, }; - const {optimisticData, failureData} = buildCopyPolicySettingsData(makeSourcePolicy(), [makeTargetPolicy()], ['rules'], allPolicyCategories, {}); + const {optimisticData, failureData} = buildCopyPolicySettingsData(makeSourcePolicy(), [makeTargetPolicy()], ['rules'], allPolicyCategories, {}, {}); expect(optimisticData.some((u) => u.key === TARGET_CATEGORIES_KEY)).toBe(false); expect(failureData.some((u) => u.key === TARGET_CATEGORIES_KEY)).toBe(false); @@ -422,7 +422,7 @@ describe('actions/Policy/CopyPolicySettings', () => { [TARGET_CATEGORIES_KEY]: {Food: {name: 'Food', enabled: false}}, }; - const {optimisticData, failureData} = buildCopyPolicySettingsData(makeSourcePolicy(), [makeTargetPolicy()], ['rules'], allPolicyCategories, {}); + const {optimisticData, failureData} = buildCopyPolicySettingsData(makeSourcePolicy(), [makeTargetPolicy()], ['rules'], allPolicyCategories, {}, {}); expect(optimisticData.some((u) => u.key === TARGET_CATEGORIES_KEY)).toBe(false); expect(failureData.some((u) => u.key === TARGET_CATEGORIES_KEY)).toBe(false); @@ -436,7 +436,7 @@ describe('actions/Policy/CopyPolicySettings', () => { [TARGET_CATEGORIES_KEY]: {Food: {name: 'Food', enabled: false}}, }; - const {optimisticData, failureData} = buildCopyPolicySettingsData(makeSourcePolicy(), [makeTargetPolicy()], ['rules'], allPolicyCategories, {}); + const {optimisticData, failureData} = buildCopyPolicySettingsData(makeSourcePolicy(), [makeTargetPolicy()], ['rules'], allPolicyCategories, {}, {}); expect(optimisticData.some((u) => u.key === TARGET_CATEGORIES_KEY)).toBe(false); expect(failureData.some((u) => u.key === TARGET_CATEGORIES_KEY)).toBe(false); @@ -448,7 +448,7 @@ describe('actions/Policy/CopyPolicySettings', () => { [TARGET_CATEGORIES_KEY]: {Food: {name: 'Food', enabled: false}}, }; - const {optimisticData} = buildCopyPolicySettingsData(makeSourcePolicy(), [makeTargetPolicy()], ['rules'], allPolicyCategories, {}); + const {optimisticData} = buildCopyPolicySettingsData(makeSourcePolicy(), [makeTargetPolicy()], ['rules'], allPolicyCategories, {}, {}); const optimisticMerge = optimisticData.find((u) => u.key === TARGET_CATEGORIES_KEY && u.onyxMethod === Onyx.METHOD.MERGE); expect(optimisticMerge?.value).toEqual({Food: {name: 'Food', enabled: true, commentHint: 'Add the attendee list'}}); @@ -468,21 +468,21 @@ describe('actions/Policy/CopyPolicySettings', () => { [TARGET_CATEGORIES_KEY]: {Food: {name: 'Food', enabled: true}}, }; - const {optimisticData} = buildCopyPolicySettingsData(makeSourcePolicy(), [makeTargetPolicy()], ['rules'], allPolicyCategories, {}); + const {optimisticData} = buildCopyPolicySettingsData(makeSourcePolicy(), [makeTargetPolicy()], ['rules'], allPolicyCategories, {}, {}); const optimisticMerge = optimisticData.find((u) => u.key === TARGET_CATEGORIES_KEY && u.onyxMethod === Onyx.METHOD.MERGE); expect(optimisticMerge?.value).toEqual({Food: {name: 'Food', enabled: true, maxExpenseAmount: 5000}}); }); it('falls back to empty object when source has no categories', () => { - const {optimisticData} = buildCopyPolicySettingsData(makeSourcePolicy(), [makeTargetPolicy()], ['categories'], {}, {}); + const {optimisticData} = buildCopyPolicySettingsData(makeSourcePolicy(), [makeTargetPolicy()], ['categories'], {}, {}, {}); const optimisticSet = optimisticData.find((u) => u.key === TARGET_CATEGORIES_KEY && u.onyxMethod === Onyx.METHOD.SET); expect(optimisticSet?.value).toEqual({}); }); it('falls back to empty object when source has no tags', () => { - const {optimisticData} = buildCopyPolicySettingsData(makeSourcePolicy(), [makeTargetPolicy()], ['tags'], {}, {}); + const {optimisticData} = buildCopyPolicySettingsData(makeSourcePolicy(), [makeTargetPolicy()], ['tags'], {}, {}, {}); const optimisticSet = optimisticData.find((u) => u.key === TARGET_TAGS_KEY && u.onyxMethod === Onyx.METHOD.SET); expect(optimisticSet?.value).toEqual({}); @@ -494,7 +494,7 @@ describe('actions/Policy/CopyPolicySettings', () => { const sourcePolicy = makeSourcePolicy({outputCurrency: 'USD', maxExpenseAmount: 50000}); const targetPolicy = makeTargetPolicy({outputCurrency: 'EUR', maxExpenseAmount: 1000}); - const {failureData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], ['overview', 'rules'], {}, {}); + const {failureData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], ['overview', 'rules'], {}, {}, {}); const policy = getFailurePolicy(failureData); @@ -508,7 +508,7 @@ describe('actions/Policy/CopyPolicySettings', () => { const targetPolicy = makeTargetPolicy(); const sourcePolicyKey = `${ONYXKEYS.COLLECTION.POLICY}${SOURCE_POLICY_ID}` as const; - const {failureData, successData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], ['overview'], {}, {}); + const {failureData, successData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], ['overview'], {}, {}, {}); const sourceFailure = failureData.find((entry) => entry.key === sourcePolicyKey && entry.onyxMethod === Onyx.METHOD.MERGE); expect(sourceFailure).toBeDefined(); @@ -551,7 +551,7 @@ describe('actions/Policy/CopyPolicySettings', () => { }, }); - const {optimisticData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], ['distanceRates'], {}, {}); + const {optimisticData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], ['distanceRates'], {}, {}, {}); const policy = getOptimisticPolicy(optimisticData); expect(policy?.customUnits).toBeDefined(); @@ -591,7 +591,7 @@ describe('actions/Policy/CopyPolicySettings', () => { }, }); - const {optimisticData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], ['distanceRates'], {}, {}); + const {optimisticData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], ['distanceRates'], {}, {}, {}); const policy = getOptimisticPolicy(optimisticData); expect(Object.keys(policy?.customUnits?.[targetExistingDistanceID]?.rates ?? {})).toEqual(['TGT_DEFAULT']); @@ -601,7 +601,7 @@ describe('actions/Policy/CopyPolicySettings', () => { const sourcePolicy = makeSourcePolicy({customUnits: {[sourceDistanceUnit.customUnitID]: sourceDistanceUnit}}); const targetPolicy = makeTargetPolicy({customUnits: {}}); - const {optimisticData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], ['distanceRates'], {}, {}); + const {optimisticData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], ['distanceRates'], {}, {}, {}); const policy = getOptimisticPolicy(optimisticData); expect(policy?.customUnits).toEqual({}); @@ -630,7 +630,7 @@ describe('actions/Policy/CopyPolicySettings', () => { }, }); - const {optimisticData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], ['distanceRates', 'perDiem'], {}, {}); + const {optimisticData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], ['distanceRates', 'perDiem'], {}, {}, {}); const policy = getOptimisticPolicy(optimisticData); expect(Object.keys(policy?.customUnits ?? {}).sort()).toEqual([targetExistingDistanceID, targetExistingPerDiemID].sort()); @@ -646,7 +646,7 @@ describe('actions/Policy/CopyPolicySettings', () => { const policyKeyA = `${ONYXKEYS.COLLECTION.POLICY}TARGET_A` as const; const policyKeyB = `${ONYXKEYS.COLLECTION.POLICY}TARGET_B` as const; - const {optimisticData, failureData, successData} = buildCopyPolicySettingsData(makeSourcePolicy(), [targetA, targetB], ['overview'], {}, {}); + const {optimisticData, failureData, successData} = buildCopyPolicySettingsData(makeSourcePolicy(), [targetA, targetB], ['overview'], {}, {}, {}); const optimisticSets = optimisticData.filter((u) => u.onyxMethod === Onyx.METHOD.SET && (u.key === policyKeyA || u.key === policyKeyB)); expect(optimisticSets).toHaveLength(2); @@ -665,7 +665,7 @@ describe('actions/Policy/CopyPolicySettings', () => { const catKeyB = `${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}TARGET_B` as const; const sourceCategories: PolicyCategories = {Food: {name: 'Food', enabled: true, areCommentsRequired: false}}; - const {optimisticData} = buildCopyPolicySettingsData(makeSourcePolicy(), [targetA, targetB], ['categories'], {[SOURCE_CATEGORIES_KEY]: sourceCategories}, {}); + const {optimisticData} = buildCopyPolicySettingsData(makeSourcePolicy(), [targetA, targetB], ['categories'], {[SOURCE_CATEGORIES_KEY]: sourceCategories}, {}, {}); expect(optimisticData.find((u) => u.key === catKeyA && u.onyxMethod === Onyx.METHOD.SET)?.value).toEqual(sourceCategories); expect(optimisticData.find((u) => u.key === catKeyB && u.onyxMethod === Onyx.METHOD.SET)?.value).toEqual(sourceCategories); @@ -674,7 +674,7 @@ describe('actions/Policy/CopyPolicySettings', () => { describe('COPY_POLICY_SETTINGS lifecycle key', () => { it("sets currentStep='loading' optimistically and nulls it on failure", () => { - const {optimisticData, failureData, successData} = buildCopyPolicySettingsData(makeSourcePolicy(), [makeTargetPolicy()], ['overview'], {}, {}); + const {optimisticData, failureData, successData} = buildCopyPolicySettingsData(makeSourcePolicy(), [makeTargetPolicy()], ['overview'], {}, {}, {}); const optLifecycle = optimisticData.find((u) => u.key === ONYXKEYS.COPY_POLICY_SETTINGS); const failLifecycle = failureData.find((u) => u.key === ONYXKEYS.COPY_POLICY_SETTINGS); @@ -692,7 +692,7 @@ describe('actions/Policy/CopyPolicySettings', () => { const sourcePolicy = makeSourcePolicy({address: {addressStreet: '1 Src St', city: 'NYC', country: 'US', state: 'NY', zipCode: '10001'}}); const targetPolicy = makeTargetPolicy({address: {addressStreet: '2 Tgt Ave', city: 'Berlin', country: 'DE', state: 'BE', zipCode: '10115'}}); - const {optimisticData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], ['overview'], {}, {}); + const {optimisticData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], ['overview'], {}, {}, {}); const policy = getOptimisticPolicy(optimisticData); expect(policy?.address).toEqual(sourcePolicy.address); @@ -719,7 +719,7 @@ describe('actions/Policy/CopyPolicySettings', () => { const sourcePolicy = makeSourcePolicy({customUnits: {[sourceDistanceUnit.customUnitID]: sourceDistanceUnit}}); const targetPolicy = makeTargetPolicy({customUnits: {[targetDistanceUnit.customUnitID]: targetDistanceUnit}}); - const {optimisticData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], ['distanceRates'], {}, {}); + const {optimisticData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], ['distanceRates'], {}, {}, {}); const policy = getOptimisticPolicy(optimisticData); // The optimistic unit is keyed by target's existing ID and carries only the name-matched rate, @@ -740,7 +740,7 @@ describe('actions/Policy/CopyPolicySettings', () => { const sourcePolicy = makeSourcePolicy({customUnits: {[sourceDistanceUnit.customUnitID]: sourceDistanceUnit}}); const targetPolicy = makeTargetPolicy({customUnits: {}}); - const {failureData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], ['distanceRates'], {}, {}); + const {failureData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], ['distanceRates'], {}, {}, {}); const policy = getFailurePolicy(failureData); // Failure restores the full original target — which had no customUnits @@ -755,7 +755,7 @@ describe('actions/Policy/CopyPolicySettings', () => { units: {time: {enabled: false, rate: 10}}, }); - const {optimisticData, successData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], ['timeTracking'], {}, {}); + const {optimisticData, successData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], ['timeTracking'], {}, {}, {}); const policy = getOptimisticPolicy(optimisticData); expect(policy?.units?.time).toEqual({enabled: true, rate: 75}); @@ -775,7 +775,7 @@ describe('actions/Policy/CopyPolicySettings', () => { tax: {trackingEnabled: false}, }); - const {optimisticData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], ['taxes'], {}, {}); + const {optimisticData} = buildCopyPolicySettingsData(sourcePolicy, [targetPolicy], ['taxes'], {}, {}, {}); const policy = getOptimisticPolicy(optimisticData); expect(policy?.tax).toEqual(sourcePolicy.tax); @@ -783,7 +783,7 @@ describe('actions/Policy/CopyPolicySettings', () => { it('successData clears errors on target policies after retry-success', () => { const targetPolicy = makeTargetPolicy(); - const {successData} = buildCopyPolicySettingsData(makeSourcePolicy(), [targetPolicy], ['overview', 'currency'], {}, {}); + const {successData} = buildCopyPolicySettingsData(makeSourcePolicy(), [targetPolicy], ['overview', 'currency'], {}, {}, {}); const targetSuccess = successData.find((entry) => entry.key === POLICY_KEY && entry.onyxMethod === Onyx.METHOD.MERGE); const successPatch = getMergedPolicyPatch(targetSuccess); diff --git a/tests/actions/PolicyRulesTest.ts b/tests/actions/PolicyRulesTest.ts index 7e0ab4552a10..90465eb3c98b 100644 --- a/tests/actions/PolicyRulesTest.ts +++ b/tests/actions/PolicyRulesTest.ts @@ -1,5 +1,13 @@ import OnyxUpdateManager from '@libs/actions/OnyxUpdateManager'; -import {addPolicyAgentRule, clearPolicyAgentRuleErrors, clearPolicyCodingRuleErrors, deletePolicyAgentRule, updatePolicyAgentRule} from '@libs/actions/Policy/Rules'; +import { + addPolicyAgentRule, + clearMerchantRuleErrors, + clearPolicyAgentRuleErrors, + deleteMerchantRule, + deletePolicyAgentRule, + setMerchantRule, + updatePolicyAgentRule, +} from '@libs/actions/Policy/Rules'; import {WRITE_COMMANDS} from '@libs/API/types'; import {flush as flushSequentialQueue} from '@libs/Network/SequentialQueue'; @@ -7,8 +15,11 @@ import {getAll as getAllPersistedRequests, getOngoingRequest as getOngoingPersis import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; -import type {Policy} from '@src/types/onyx'; -import type {AgentRule, CodingRule} from '@src/types/onyx/Policy'; +import type {Policy, Rule} from '@src/types/onyx'; +import type {ExpenseDefaultAction} from '@src/types/onyx/ExpenseDefaultRules'; +import type {AgentRule} from '@src/types/onyx/Policy'; + +import type {OnyxCollection} from 'react-native-onyx'; import Onyx from 'react-native-onyx'; @@ -34,6 +45,34 @@ function getPolicy(policyID: string): Promise { }); } +async function getRules(): Promise> { + let collection: OnyxCollection = {}; + await TestHelper.getOnyxData({ + key: ONYXKEYS.COLLECTION.RULE, + callback: (value) => { + collection = value ?? {}; + }, + }); + return collection; +} + +/** Mirrors the way the rules engine keys `triggers` and `actions` by a stringified index. */ +function toIndexMap(values: T[]): Record { + return Object.fromEntries(values.map((value, index) => [String(index), value])); +} + +/** A minimal merchant rule as stored in the rules collection. */ +function buildMerchantRuleForPolicy(policyID: string): Rule { + return { + scope: CONST.RULES.SCOPE.POLICY, + scopeID: policyID, + priority: CONST.RULES.EXPENSE_DEFAULT.PRIORITY, + triggers: toIndexMap([CONST.RULES.EXPENSE_DEFAULT.TRIGGER.CREATE_TRANSACTION]), + filters: {left: CONST.RULES.EXPENSE_DEFAULT.FIELD.MERCHANT, operator: CONST.SEARCH.SYNTAX_OPERATORS.EQUAL_TO, right: 'Starbucks'}, + actions: toIndexMap([{name: CONST.RULES.EXPENSE_DEFAULT.ACTION.SET, field: CONST.RULES.EXPENSE_DEFAULT.FIELD.CATEGORY, value: 'Coffee'}]), + }; +} + describe('actions/PolicyRules', () => { beforeAll(() => { Onyx.init({keys: ONYXKEYS}); @@ -427,60 +466,122 @@ describe('actions/PolicyRules', () => { }); }); - describe('clearPolicyCodingRuleErrors', () => { - it('removes the coding rule entirely when its pendingAction was ADD', async () => { + describe('setMerchantRule', () => { + it('writes the rule under its own key, scoped to the policy', async () => { const fakePolicy = createRandomPolicy(0); - const ruleID = 'codingRule1'; - const rule: CodingRule = { - ruleID, - filters: {left: 'merchant', operator: CONST.SEARCH.SYNTAX_OPERATORS.EQUAL_TO, right: 'Starbucks'}, + mockFetch?.pause?.(); + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`, fakePolicy); + + setMerchantRule(fakePolicy.id, {merchantToMatch: 'Starbucks', matchType: CONST.SEARCH.SYNTAX_OPERATORS.EQUAL_TO, category: 'Coffee'}, fakePolicy); + await waitForBatchedUpdates(); + + const rules = await getRules(); + const [ruleKey, rule] = Object.entries(rules ?? {}).at(0) ?? []; + expect(ruleKey?.startsWith(ONYXKEYS.COLLECTION.RULE)).toBe(true); + expect(rule).toMatchObject({ + scope: CONST.RULES.SCOPE.POLICY, + scopeID: fakePolicy.id, + priority: CONST.RULES.EXPENSE_DEFAULT.PRIORITY, pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD, - errors: {[ERROR_KEY]: 'boom'}, - }; - await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`, { - ...fakePolicy, - rules: {codingRules: {[ruleID]: rule}}, + triggers: toIndexMap([CONST.RULES.EXPENSE_DEFAULT.TRIGGER.CREATE_TRANSACTION]), + filters: {left: CONST.RULES.EXPENSE_DEFAULT.FIELD.MERCHANT, operator: CONST.SEARCH.SYNTAX_OPERATORS.EQUAL_TO, right: 'Starbucks'}, + actions: toIndexMap([{name: CONST.RULES.EXPENSE_DEFAULT.ACTION.SET, field: CONST.RULES.EXPENSE_DEFAULT.FIELD.CATEGORY, value: 'Coffee'}]), }); - clearPolicyCodingRuleErrors(fakePolicy.id, ruleID, rule); + await mockFetch?.resume?.(); await waitForBatchedUpdates(); - const policy = await getPolicy(fakePolicy.id); - expect(policy?.rules?.codingRules?.[ruleID]).toBeFalsy(); + const savedRule = Object.values((await getRules()) ?? {}).at(0); + expect(savedRule?.pendingAction).toBeFalsy(); + }); + + it('drops an action for a field the edit cleared, rather than merging it with the previous value', async () => { + const fakePolicy = createRandomPolicy(0); + const ruleID = 'merchantRule1'; + mockFetch?.pause?.(); + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`, fakePolicy); + + setMerchantRule(fakePolicy.id, {merchantToMatch: 'Starbucks', category: 'Coffee', tag: 'Team A'}, fakePolicy, ruleID); + await waitForBatchedUpdates(); + + const existingRule = (await getRules())?.[`${ONYXKEYS.COLLECTION.RULE}${ruleID}`]; + setMerchantRule(fakePolicy.id, {merchantToMatch: 'Starbucks', category: 'Coffee'}, fakePolicy, ruleID, existingRule); + await waitForBatchedUpdates(); + + const updatedRule = (await getRules())?.[`${ONYXKEYS.COLLECTION.RULE}${ruleID}`]; + const updatedActions: ExpenseDefaultAction[] = Object.values(updatedRule?.actions ?? {}).filter((action): action is ExpenseDefaultAction => 'field' in action); + const fields = updatedActions.map((action) => action.field); + expect(fields).toEqual([CONST.RULES.EXPENSE_DEFAULT.FIELD.CATEGORY]); + expect(updatedRule?.pendingAction).toBe(CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE); }); - it('clears only the errors when the coding rule has a non-ADD pending action', async () => { + it('does nothing when the form has no merchant to match', async () => { const fakePolicy = createRandomPolicy(0); - const ruleID = 'codingRule1'; - const rule: CodingRule = { - ruleID, - filters: {left: 'merchant', operator: CONST.SEARCH.SYNTAX_OPERATORS.EQUAL_TO, right: 'Starbucks'}, + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`, fakePolicy); + + setMerchantRule(fakePolicy.id, {merchantToMatch: '', category: 'Coffee'}, fakePolicy); + await waitForBatchedUpdates(); + + expect(Object.keys((await getRules()) ?? {})).toHaveLength(0); + }); + }); + + describe('deleteMerchantRule', () => { + it('marks the rule as pending delete', async () => { + const fakePolicy = createRandomPolicy(0); + const ruleID = 'merchantRule1'; + const rule = buildMerchantRuleForPolicy(fakePolicy.id); + mockFetch?.pause?.(); + await Onyx.set(`${ONYXKEYS.COLLECTION.RULE}${ruleID}`, rule); + + deleteMerchantRule(fakePolicy.id, ruleID, rule); + await waitForBatchedUpdates(); + + expect((await getRules())?.[`${ONYXKEYS.COLLECTION.RULE}${ruleID}`]?.pendingAction).toBe(CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE); + + await mockFetch?.resume?.(); + await waitForBatchedUpdates(); + }); + }); + + describe('clearMerchantRuleErrors', () => { + it('removes the rule entirely when its pendingAction was ADD', async () => { + const ruleID = 'merchantRule1'; + const rule: Rule = { + ...buildMerchantRuleForPolicy('policy1'), + pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD, + errors: {[ERROR_KEY]: 'boom'}, + }; + await Onyx.set(`${ONYXKEYS.COLLECTION.RULE}${ruleID}`, rule); + + clearMerchantRuleErrors(ruleID, rule); + await waitForBatchedUpdates(); + + expect((await getRules())?.[`${ONYXKEYS.COLLECTION.RULE}${ruleID}`]).toBeFalsy(); + }); + + it('clears only the errors when the rule has a non-ADD pending action', async () => { + const ruleID = 'merchantRule1'; + const rule: Rule = { + ...buildMerchantRuleForPolicy('policy1'), pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE, errors: {[ERROR_KEY]: 'boom'}, }; - await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`, { - ...fakePolicy, - rules: {codingRules: {[ruleID]: rule}}, - }); + await Onyx.set(`${ONYXKEYS.COLLECTION.RULE}${ruleID}`, rule); - clearPolicyCodingRuleErrors(fakePolicy.id, ruleID, rule); + clearMerchantRuleErrors(ruleID, rule); await waitForBatchedUpdates(); - const policy = await getPolicy(fakePolicy.id); - const cleared = policy?.rules?.codingRules?.[ruleID]; + const cleared = (await getRules())?.[`${ONYXKEYS.COLLECTION.RULE}${ruleID}`]; expect(cleared?.errors).toBeFalsy(); expect(cleared?.pendingAction).toBe(CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE); }); it('does nothing when no rule is passed', async () => { - const fakePolicy = createRandomPolicy(0); - await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`, fakePolicy); - - clearPolicyCodingRuleErrors(fakePolicy.id, 'missing', undefined); + clearMerchantRuleErrors('missing', undefined); await waitForBatchedUpdates(); - const policy = await getPolicy(fakePolicy.id); - expect(policy?.rules?.codingRules).toBeFalsy(); + expect(Object.keys((await getRules()) ?? {})).toHaveLength(0); }); }); }); diff --git a/tests/actions/PolicyTest.ts b/tests/actions/PolicyTest.ts index c48e587eab38..a243f431c415 100644 --- a/tests/actions/PolicyTest.ts +++ b/tests/actions/PolicyTest.ts @@ -14,6 +14,7 @@ import * as Policy from '@src/libs/actions/Policy/Policy'; import ONYXKEYS from '@src/ONYXKEYS'; import type {Onboarding, PolicyJoinMember, PolicyReportField, Policy as PolicyType, Report, ReportAction, ReportActions, Transaction, TransactionViolations} from '@src/types/onyx'; import type {Participant, ReportNextStep} from '@src/types/onyx/Report'; +import type Rule from '@src/types/onyx/Rule'; import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; @@ -62,6 +63,22 @@ function requireCallArgument(call: unknown, index: number): unknown { jest.mock('@libs/GoogleTagManager'); OnyxUpdateManager(); +/** Build the index-keyed object shape the rules API uses for lists */ +function indexMap(...values: T[]): Record { + return Object.fromEntries(values.map((value, index) => [String(index), value])); +} + +async function getRulesCollection(): Promise> { + let collection: OnyxCollection = {}; + await TestHelper.getOnyxData({ + key: ONYXKEYS.COLLECTION.RULE, + callback: (value) => { + collection = value ?? {}; + }, + }); + return collection; +} + describe('actions/Policy', () => { beforeAll(() => { Onyx.init({ @@ -628,8 +645,17 @@ describe('actions/Policy', () => { address: {addressStreet: '1 Main Street', city: 'Paris', country: 'FR', state: '', zipCode: '75001'}, isTravelEnabled: true, tax: {trackingEnabled: true}, - rules: {codingRules: {rule1: {filters: {left: 'merchant', operator: 'eq', right: 'Acme'}, category: 'Travel'}}}, }; + const sourceRule: Rule = { + scope: CONST.RULES.SCOPE.POLICY, + scopeID: fakePolicy.id, + triggers: indexMap(CONST.RULES.EXPENSE_DEFAULT.TRIGGER.CREATE_TRANSACTION), + filters: {left: CONST.RULES.EXPENSE_DEFAULT.FIELD.MERCHANT, operator: CONST.SEARCH.SYNTAX_OPERATORS.EQUAL_TO, right: 'Acme'}, + actions: indexMap({name: CONST.RULES.EXPENSE_DEFAULT.ACTION.SET, field: CONST.RULES.EXPENSE_DEFAULT.FIELD.CATEGORY, value: 'Travel'}), + }; + // The copies are optimistic only - they are dropped once the server responds with its own rule IDs, + // so the request stays paused while they are asserted. + mockFetch?.pause?.(); await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`, fakePolicy); await waitForBatchedUpdates(); @@ -660,6 +686,7 @@ describe('actions/Policy', () => { codingRules: true, }, localCurrency: 'USD', + rules: {[`${ONYXKEYS.COLLECTION.RULE}sourceRule`]: sourceRule}, }; Policy.duplicateWorkspace(fakePolicy, options); @@ -679,7 +706,22 @@ describe('actions/Policy', () => { expect(policy?.address).toEqual(fakePolicy.address); expect(policy?.isTravelEnabled).toBe(true); expect(policy?.tax).toEqual(fakePolicy.tax); - expect(policy?.rules).toEqual({codingRules: fakePolicy.rules?.codingRules}); + + // Merchant rules are copied into the rules collection as new rules scoped to the duplicate, + // rather than onto the duplicated policy object. + const duplicatedRules = Object.values((await getRulesCollection()) ?? {}).filter((rule) => rule?.scopeID === policyID); + expect(duplicatedRules).toHaveLength(1); + expect(duplicatedRules.at(0)).toMatchObject({ + scope: CONST.RULES.SCOPE.POLICY, + scopeID: policyID, + triggers: sourceRule.triggers, + filters: sourceRule.filters, + actions: sourceRule.actions, + pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD, + }); + + await mockFetch?.resume?.(); + await waitForBatchedUpdates(); }); it('duplicate workspace with 3+ members creates optimistic announce chat using currentUserAccountID', async () => { @@ -3408,6 +3450,58 @@ describe('actions/Policy', () => { }); describe('setWorkspaceApprovalMode', () => { + it('should delete the policy approval workflow rules but keep its expense default rules when disabling approvals', async () => { + const apiWriteSpy = jest.spyOn(APIModule, 'write').mockImplementation(() => Promise.resolve()); + await Onyx.set(ONYXKEYS.SESSION, {email: ESH_EMAIL, accountID: ESH_ACCOUNT_ID}); + + const policyID = Policy.generatePolicyID(); + const fakePolicy: PolicyType = { + ...createRandomPolicy(0, CONST.POLICY.TYPE.TEAM), + id: policyID, + approvalMode: CONST.POLICY.APPROVAL_MODE.BASIC, + approver: ESH_EMAIL, + owner: ESH_EMAIL, + }; + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, fakePolicy); + await waitForBatchedUpdates(); + + const approvalRuleKey = `${ONYXKEYS.COLLECTION.RULE}approval1` as const; + const expenseDefaultRuleKey = `${ONYXKEYS.COLLECTION.RULE}merchant1` as const; + const rules: OnyxCollection = { + [approvalRuleKey]: { + scope: CONST.RULES.SCOPE.POLICY, + scopeID: policyID, + triggers: indexMap(CONST.RULES.APPROVAL_WORKFLOW.TRIGGER.REPORT_SUBMIT), + filters: {operator: CONST.SEARCH.SYNTAX_OPERATORS.EQUAL_TO, left: CONST.SEARCH.SYNTAX_FILTER_KEYS.FROM, right: [EMPLOYEE_EMAIL]}, + actions: indexMap({name: CONST.RULES.APPROVAL_WORKFLOW.ACTION.FORWARD_TO, approver: ESH_EMAIL}), + }, + [expenseDefaultRuleKey]: { + scope: CONST.RULES.SCOPE.POLICY, + scopeID: policyID, + triggers: indexMap(CONST.RULES.EXPENSE_DEFAULT.TRIGGER.CREATE_TRANSACTION), + filters: {operator: CONST.SEARCH.SYNTAX_OPERATORS.CONTAINS, left: CONST.RULES.EXPENSE_DEFAULT.FIELD.MERCHANT, right: 'Starbucks'}, + actions: indexMap({name: CONST.RULES.EXPENSE_DEFAULT.ACTION.SET, field: CONST.RULES.EXPENSE_DEFAULT.FIELD.CATEGORY, value: 'Coffee'}), + }, + }; + + Policy.setWorkspaceApprovalMode(fakePolicy, ESH_EMAIL, CONST.POLICY.APPROVAL_MODE.OPTIONAL, ESH_ACCOUNT_ID, ESH_EMAIL, false, rules); + await waitForBatchedUpdates(); + + // The approval rule is removed with the workflow, the merchant rule on the same policy is left alone. + expect(apiWriteSpy).toHaveBeenCalledWith( + WRITE_COMMANDS.DISABLE_POLICY_APPROVALS, + expect.anything(), + expect.objectContaining({optimisticData: expect.arrayContaining([expect.objectContaining({key: approvalRuleKey, value: null})])}), + ); + expect(apiWriteSpy).not.toHaveBeenCalledWith( + WRITE_COMMANDS.DISABLE_POLICY_APPROVALS, + expect.anything(), + expect.objectContaining({optimisticData: expect.arrayContaining([expect.objectContaining({key: expenseDefaultRuleKey})])}), + ); + + apiWriteSpy.mockRestore(); + }); + it('should not change employee list when disabling approval', async () => { mockFetch?.pause?.(); await Onyx.set(ONYXKEYS.SESSION, {email: ESH_EMAIL, accountID: ESH_ACCOUNT_ID}); diff --git a/tests/actions/WorkflowTest.ts b/tests/actions/WorkflowTest.ts index 31ea2894256a..87b93e6c68b0 100644 --- a/tests/actions/WorkflowTest.ts +++ b/tests/actions/WorkflowTest.ts @@ -14,10 +14,11 @@ import { updateApprovalWorkflow, updateApprovalWorkflowRules, } from '@src/libs/actions/Workflow'; -import {calculateApprovers, convertApprovalWorkflowRulesToWorkflows, extractSubmitterEmails, getApprovalWorkflowRulesForPolicy} from '@src/libs/WorkflowUtils'; +import {calculateApprovers, convertApprovalWorkflowRulesToWorkflows, extractSubmitterEmails, getApprovalWorkflowRulesForPolicy, isApprovalWorkflowRule} from '@src/libs/WorkflowUtils'; import ONYXKEYS from '@src/ONYXKEYS'; import type {ApprovalWorkflowOnyx, PersonalDetailsList, Policy, Policy as PolicyType, Report} from '@src/types/onyx'; import type {Approver} from '@src/types/onyx/ApprovalWorkflow'; +import type {ApprovalWorkflowRule} from '@src/types/onyx/ApprovalWorkflowRules'; import type Rule from '@src/types/onyx/Rule'; import type {OnyxCollection} from 'react-native-onyx'; @@ -71,10 +72,16 @@ async function getRulesCollection(): Promise> { return collection; } -async function getActivePolicyRules(policyID: string): Promise { +/** The rules collection also holds rules of other kinds, so these tests narrow it to approval workflow rules. */ +async function getActivePolicyRules(policyID: string): Promise> { const collection = await getRulesCollection(); return Object.values(collection ?? {}).filter( - (rule): rule is Rule => !!rule && rule.scope === CONST.RULES.SCOPE.POLICY && rule.scopeID === policyID && rule.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE, + (rule): rule is Rule & ApprovalWorkflowRule => + !!rule && + rule.scope === CONST.RULES.SCOPE.POLICY && + rule.scopeID === policyID && + rule.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE && + isApprovalWorkflowRule(rule), ); } diff --git a/tests/ui/EditMerchantRulePageGuardTest.tsx b/tests/ui/EditMerchantRulePageGuardTest.tsx new file mode 100644 index 000000000000..54664104f23e --- /dev/null +++ b/tests/ui/EditMerchantRulePageGuardTest.tsx @@ -0,0 +1,148 @@ +import {act, render, screen} from '@testing-library/react-native'; + +import ComposeProviders from '@components/ComposeProviders'; +import {CurrentUserPersonalDetailsProvider} from '@components/CurrentUserPersonalDetailsProvider'; +import {LocaleContextProvider} from '@components/LocaleContextProvider'; +import OnyxListItemProvider from '@components/OnyxListItemProvider'; + +import {navigationRef} from '@libs/Navigation/Navigation'; +import createPlatformStackNavigator from '@libs/Navigation/PlatformStackNavigation/createPlatformStackNavigator'; +import type {SettingsNavigatorParamList} from '@libs/Navigation/types'; + +import EditMerchantRulePage from '@pages/workspace/rules/MerchantRules/EditMerchantRulePage'; + +import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; +import SCREENS from '@src/SCREENS'; +import type {Policy, Rule} from '@src/types/onyx'; + +import {PortalProvider} from '@gorhom/portal'; +import {NavigationContainer} from '@react-navigation/native'; +import React from 'react'; +import Onyx from 'react-native-onyx'; + +import createRandomPolicy from '../utils/collections/policies'; +import {buildPersonalDetails} from '../utils/TestHelper'; +import waitForBatchedUpdatesWithAct from '../utils/waitForBatchedUpdatesWithAct'; + +const POLICY_ID = 'policy1'; +const OTHER_POLICY_ID = 'policy2'; +const RULE_ID = 'merchantRule1'; +const ADMIN_EMAIL = 'admin@example.com'; +const ADMIN_ACCOUNT_ID = 1; + +const {FIELD, TRIGGER, ACTION} = CONST.RULES.EXPENSE_DEFAULT; + +const Stack = createPlatformStackNavigator(); + +/** Mirrors the way the rules engine keys `triggers` and `actions` by a stringified index. */ +function toIndexMap(values: T[]): Record { + return Object.fromEntries(values.map((value, index) => [String(index), value])); +} + +function buildRulesEnabledControlPolicy(): Policy { + return { + ...createRandomPolicy(0), + id: POLICY_ID, + type: CONST.POLICY.TYPE.CORPORATE, + role: CONST.POLICY.ROLE.ADMIN, + areRulesEnabled: true, + pendingAction: undefined, + }; +} + +function buildEditableRule(): Rule { + return { + scope: CONST.RULES.SCOPE.POLICY, + scopeID: POLICY_ID, + triggers: toIndexMap([TRIGGER.CREATE_TRANSACTION]), + filters: {left: FIELD.MERCHANT, operator: CONST.SEARCH.SYNTAX_OPERATORS.CONTAINS, right: 'Starbucks'}, + actions: toIndexMap([{name: ACTION.SET, field: FIELD.CATEGORY, value: 'Coffee'}]), + }; +} + +async function seedOnyx(rule: Rule) { + await act(async () => { + await Onyx.clear(); + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${POLICY_ID}`, buildRulesEnabledControlPolicy()); + await Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, {[ADMIN_ACCOUNT_ID]: buildPersonalDetails(ADMIN_EMAIL, ADMIN_ACCOUNT_ID, 'admin')}); + await Onyx.merge(ONYXKEYS.SESSION, {email: ADMIN_EMAIL, accountID: ADMIN_ACCOUNT_ID}); + await Onyx.set(`${ONYXKEYS.COLLECTION.RULE}${RULE_ID}`, rule); + await waitForBatchedUpdatesWithAct(); + }); +} + +/** The page reads its params off the navigator, and `usePressLoading` needs a real navigation context. */ +function renderEditMerchantRulePage() { + return render( + + + + + + + + + , + ); +} + +/** + * The rules collection is shared across workspaces and rule kinds, so a ruleID reached by a bookmark, a + * deeplink or browser history can point at something this editor must not write to. Saving replaces that + * ruleID with a freshly built merchant rule, so the page has to refuse rather than render an empty form. + */ +describe('EditMerchantRulePage route guard', () => { + afterEach(async () => { + await act(async () => { + await Onyx.clear(); + }); + }); + + it('opens the editor for a rule this policy can edit', async () => { + await seedOnyx(buildEditableRule()); + renderEditMerchantRulePage(); + await waitForBatchedUpdatesWithAct(); + + expect(screen.getByTestId('EditMerchantRulePage')).toBeOnTheScreen(); + }); + + it('refuses a rule scoped to another policy', async () => { + await seedOnyx({...buildEditableRule(), scopeID: OTHER_POLICY_ID}); + renderEditMerchantRulePage(); + await waitForBatchedUpdatesWithAct(); + + expect(screen.queryByTestId('EditMerchantRulePage')).toBeNull(); + }); + + it('refuses an approval workflow rule that shares the ruleID', async () => { + await seedOnyx({ + ...buildEditableRule(), + triggers: toIndexMap([CONST.RULES.APPROVAL_WORKFLOW.TRIGGER.REPORT_SUBMIT]), + actions: toIndexMap([{name: CONST.RULES.APPROVAL_WORKFLOW.ACTION.FORWARD_TO, approver: 'approver@example.com'}]), + }); + renderEditMerchantRulePage(); + await waitForBatchedUpdatesWithAct(); + + expect(screen.queryByTestId('EditMerchantRulePage')).toBeNull(); + }); + + it('refuses a nested filter tree the form cannot represent', async () => { + await seedOnyx({ + ...buildEditableRule(), + filters: { + left: {left: FIELD.MERCHANT, operator: CONST.SEARCH.SYNTAX_OPERATORS.CONTAINS, right: 'Starbucks'}, + operator: CONST.SEARCH.SYNTAX_OPERATORS.AND, + right: {left: FIELD.MERCHANT, operator: CONST.SEARCH.SYNTAX_OPERATORS.CONTAINS, right: 'Costa'}, + }, + }); + renderEditMerchantRulePage(); + await waitForBatchedUpdatesWithAct(); + + expect(screen.queryByTestId('EditMerchantRulePage')).toBeNull(); + }); +}); diff --git a/tests/unit/ExpenseDefaultRuleUtilsTest.ts b/tests/unit/ExpenseDefaultRuleUtilsTest.ts new file mode 100644 index 000000000000..fd245a21e60b --- /dev/null +++ b/tests/unit/ExpenseDefaultRuleUtilsTest.ts @@ -0,0 +1,422 @@ +import { + buildMerchantRule, + canEditMerchantRule, + getExpenseDefaultRuleCount, + getExpenseDefaultRuleSummaryFields, + getMerchantRuleFormValues, + getPolicyExpenseDefaultRules, + getRuleFilterLeaves, + getRuleMerchantMatchSummary, + hasExpenseDefaultRuleErrors, + isExpenseDefaultRule, +} from '@libs/ExpenseDefaultRuleUtils'; +import type {MerchantRuleFormValues} from '@libs/ExpenseDefaultRuleUtils'; +import Parser from '@libs/Parser'; + +import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; +import type {Policy, Rule} from '@src/types/onyx'; +import type {ExpenseDefaultAction, ExpenseDefaultRule} from '@src/types/onyx/ExpenseDefaultRules'; +import type {RuleFilterComparison, RuleFilterNode} from '@src/types/onyx/RuleFilters'; + +import createRandomPolicy from '../utils/collections/policies'; + +const {FIELD, TRIGGER, ACTION} = CONST.RULES.EXPENSE_DEFAULT; +const {EQUAL_TO, CONTAINS, AND, OR, GREATER_THAN} = CONST.SEARCH.SYNTAX_OPERATORS; + +const POLICY_ID = 'ABC123'; +const OTHER_POLICY_ID = 'DEF456'; +const TAX_KEY = 'id_TAX_RATE_1'; + +/** Mirrors the way the rules engine keys `triggers` and `actions` by a stringified index. */ +function toIndexMap(values: T[]): Record { + return Object.fromEntries(values.map((value, index) => [String(index), value])); +} + +const policy: Policy = { + ...createRandomPolicy(1), + id: POLICY_ID, + taxRates: { + name: 'Tax', + defaultExternalID: TAX_KEY, + defaultValue: '10%', + foreignTaxDefault: TAX_KEY, + taxes: { + [TAX_KEY]: {name: 'GST', value: '10%'}, + }, + }, +}; + +const merchantFilter: RuleFilterComparison = {left: FIELD.MERCHANT, operator: CONTAINS, right: 'Starbucks'}; +const setCategoryAction: ExpenseDefaultAction = {name: ACTION.SET, field: FIELD.CATEGORY, value: 'Coffee'}; + +const merchantRuleBody: ExpenseDefaultRule = { + triggers: toIndexMap([TRIGGER.CREATE_TRANSACTION]), + filters: merchantFilter, + actions: toIndexMap([setCategoryAction]), +}; + +/** Wraps a rule body into a collection-shaped rule so the collection helpers can be tested. */ +function asStoredRule(body: ExpenseDefaultRule, scopeID = POLICY_ID, scope: Rule['scope'] = CONST.RULES.SCOPE.POLICY): Rule { + return {...body, scope, scopeID}; +} + +/** Builds a rule body the merchant rule form can't represent, by overriding one part of a valid one. */ +function buildRuleWithOverrides(overrides: Record): ExpenseDefaultRule { + return {...merchantRuleBody, ...overrides} as ExpenseDefaultRule; +} + +describe('ExpenseDefaultRuleUtils', () => { + describe('buildMerchantRule', () => { + it('builds the CreateTransaction trigger, the merchant filter and one Set action per field', () => { + const rule = buildMerchantRule( + { + merchantToMatch: 'Starbucks', + matchType: EQUAL_TO, + merchant: 'Starbucks Coffee', + category: 'Coffee', + tag: 'Team A', + tax: TAX_KEY, + vendorID: 'vendor-1', + comment: 'A description', + reimbursable: true, + billable: false, + }, + policy, + ); + + expect(rule).toEqual({ + triggers: toIndexMap([TRIGGER.CREATE_TRANSACTION]), + filters: {left: FIELD.MERCHANT, operator: EQUAL_TO, right: 'Starbucks'}, + actions: toIndexMap([ + {name: ACTION.SET, field: FIELD.MERCHANT, value: 'Starbucks Coffee'}, + {name: ACTION.SET, field: FIELD.CATEGORY, value: 'Coffee'}, + {name: ACTION.SET, field: FIELD.TAG, value: 'Team A'}, + // eslint-disable-next-line @typescript-eslint/naming-convention + {name: ACTION.SET, field: FIELD.TAX, value: {field_id_TAX: {externalID: TAX_KEY, value: '10%', name: 'GST'}}}, + {name: ACTION.SET, field: FIELD.VENDOR_ID, value: 'vendor-1'}, + {name: ACTION.SET, field: FIELD.COMMENT, value: Parser.replace('A description')}, + {name: ACTION.SET, field: FIELD.REIMBURSABLE, value: true}, + {name: ACTION.SET, field: FIELD.BILLABLE, value: false}, + ]), + }); + }); + + it('defaults the match type to contains and trims the matched merchant', () => { + const rule = buildMerchantRule({merchantToMatch: ' Uber ', category: 'Travel'}, policy); + + expect(rule?.filters).toEqual({left: FIELD.MERCHANT, operator: CONTAINS, right: 'Uber'}); + }); + + it('keeps false booleans, which are meaningful values rather than empty ones', () => { + const rule = buildMerchantRule({merchantToMatch: 'Uber', billable: false, reimbursable: false}, policy); + + expect(Object.values(rule?.actions ?? {})).toEqual([ + {name: ACTION.SET, field: FIELD.REIMBURSABLE, value: false}, + {name: ACTION.SET, field: FIELD.BILLABLE, value: false}, + ]); + }); + + it('returns undefined when there is nothing to match on', () => { + expect(buildMerchantRule({merchantToMatch: ' ', category: 'Coffee'}, policy)).toBeUndefined(); + }); + + it('returns undefined when there is nothing to set', () => { + expect(buildMerchantRule({merchantToMatch: 'Starbucks'}, policy)).toBeUndefined(); + }); + + it('treats a whitespace-only description as nothing to set', () => { + expect(buildMerchantRule({merchantToMatch: 'Starbucks', comment: ' '}, policy)).toBeUndefined(); + }); + + it('still records the tax external ID when the rate is missing from the policy', () => { + const rule = buildMerchantRule({merchantToMatch: 'Starbucks', tax: 'id_UNKNOWN'}, policy); + + // eslint-disable-next-line @typescript-eslint/naming-convention + expect(Object.values(rule?.actions ?? {}).at(0)?.value).toEqual({field_id_TAX: {externalID: 'id_UNKNOWN'}}); + }); + }); + + describe('getMerchantRuleFormValues round trip', () => { + it('returns the values it was built from', () => { + const formValues: MerchantRuleFormValues = { + merchantToMatch: 'Starbucks', + matchType: EQUAL_TO, + merchant: 'Starbucks Coffee', + category: 'Coffee', + tag: 'Team A', + tax: TAX_KEY, + vendorID: 'vendor-1', + comment: 'A description', + reimbursable: true, + billable: false, + }; + + expect(getMerchantRuleFormValues(buildMerchantRule(formValues, policy))).toEqual(formValues); + }); + + it('round trips a rule that only renames the merchant', () => { + const formValues: MerchantRuleFormValues = {merchantToMatch: 'STARBUCKS #123', matchType: CONTAINS, merchant: 'Starbucks'}; + + expect(getMerchantRuleFormValues(buildMerchantRule(formValues, policy))).toEqual(formValues); + }); + + it('converts the description back to markdown', () => { + const rule = buildMerchantRule({merchantToMatch: 'Starbucks', comment: 'A description'}, policy); + + expect(getMerchantRuleFormValues(rule)?.comment).toBe('A description'); + }); + + it('accepts a single-value list on the right of the merchant filter', () => { + const rule = buildRuleWithOverrides({filters: {left: FIELD.MERCHANT, operator: CONTAINS, right: ['Starbucks']}}); + + expect(getMerchantRuleFormValues(rule)).toEqual({merchantToMatch: 'Starbucks', matchType: CONTAINS, category: 'Coffee'}); + }); + }); + + describe('getMerchantRuleFormValues rejects rules the form cannot represent', () => { + it.each([ + [ + 'a nested filter tree', + { + filters: { + left: merchantFilter, + operator: AND, + right: {left: FIELD.CATEGORY, operator: EQUAL_TO, right: 'Coffee'}, + }, + }, + ], + ['a filter on a field the form has no input for', {filters: {left: FIELD.CATEGORY, operator: EQUAL_TO, right: 'Coffee'}}], + ['an operator the form has no control for', {filters: {left: FIELD.MERCHANT, operator: GREATER_THAN, right: 'Starbucks'}}], + ['a list of merchants, which the form has one input for', {filters: {left: FIELD.MERCHANT, operator: OR, right: ['Starbucks', 'Costa']}}], + ['a trigger the form does not set', {triggers: toIndexMap([TRIGGER.CREATE_TRANSACTION, CONST.RULES.APPROVAL_WORKFLOW.TRIGGER.REPORT_SUBMIT])}], + ['no triggers at all', {triggers: {}}], + ['no actions at all', {actions: {}}], + ['an action the form cannot produce', {actions: toIndexMap([{name: CONST.RULES.APPROVAL_WORKFLOW.ACTION.FORWARD_TO, approver: 'a@b.com'}])}], + ['an unknown field', {actions: toIndexMap([{name: ACTION.SET, field: 'attendees', value: 'someone'}])}], + [ + 'two actions writing the same field', + { + actions: toIndexMap([setCategoryAction, {name: ACTION.SET, field: FIELD.CATEGORY, value: 'Travel'}]), + }, + ], + ['a value of the wrong type', {actions: toIndexMap([{name: ACTION.SET, field: FIELD.BILLABLE, value: 'true'}])}], + ['a malformed tax value', {actions: toIndexMap([{name: ACTION.SET, field: FIELD.TAX, value: TAX_KEY}])}], + ])('returns undefined for %s', (_description, overrides) => { + expect(getMerchantRuleFormValues(buildRuleWithOverrides(overrides))).toBeUndefined(); + }); + + it('returns undefined for an undefined rule', () => { + expect(getMerchantRuleFormValues(undefined)).toBeUndefined(); + }); + }); + + describe('isExpenseDefaultRule', () => { + it('is true for a rule that runs on transaction creation and sets a field', () => { + expect(isExpenseDefaultRule(asStoredRule(merchantRuleBody))).toBe(true); + }); + + it('is false for a rule with no Set action', () => { + const rule = asStoredRule(buildRuleWithOverrides({actions: toIndexMap([{name: CONST.RULES.APPROVAL_WORKFLOW.ACTION.FORWARD_TO, approver: 'a@b.com'}])})); + + expect(isExpenseDefaultRule(rule)).toBe(false); + }); + + it('is false for a rule that does not run on transaction creation', () => { + const rule = asStoredRule(buildRuleWithOverrides({triggers: toIndexMap([CONST.RULES.APPROVAL_WORKFLOW.TRIGGER.REPORT_SUBMIT])})); + + expect(isExpenseDefaultRule(rule)).toBe(false); + }); + }); + + describe('getPolicyExpenseDefaultRules', () => { + it('keeps only the expense default rules scoped to the given policy', () => { + const collection = { + [`${ONYXKEYS.COLLECTION.RULE}1`]: asStoredRule(merchantRuleBody), + [`${ONYXKEYS.COLLECTION.RULE}2`]: asStoredRule(merchantRuleBody, OTHER_POLICY_ID), + [`${ONYXKEYS.COLLECTION.RULE}3`]: asStoredRule(merchantRuleBody, '5555', CONST.RULES.SCOPE.ACCOUNT), + [`${ONYXKEYS.COLLECTION.RULE}4`]: asStoredRule(buildRuleWithOverrides({triggers: toIndexMap([CONST.RULES.APPROVAL_WORKFLOW.TRIGGER.REPORT_SUBMIT])})), + }; + + expect(getPolicyExpenseDefaultRules(collection, POLICY_ID)).toEqual([{ruleID: '1', rule: collection[`${ONYXKEYS.COLLECTION.RULE}1`]}]); + }); + + it('returns an empty list without a policy ID', () => { + expect(getPolicyExpenseDefaultRules({[`${ONYXKEYS.COLLECTION.RULE}1`]: asStoredRule(merchantRuleBody)}, undefined)).toEqual([]); + }); + }); + + describe('getExpenseDefaultRuleCount', () => { + it('counts only the rules scoped to the given policy', () => { + const collection = { + [`${ONYXKEYS.COLLECTION.RULE}1`]: asStoredRule(merchantRuleBody), + [`${ONYXKEYS.COLLECTION.RULE}2`]: asStoredRule(merchantRuleBody, OTHER_POLICY_ID), + }; + + expect(getExpenseDefaultRuleCount(collection, POLICY_ID)).toBe(1); + }); + + it('ignores rules being deleted', () => { + const collection = { + [`${ONYXKEYS.COLLECTION.RULE}1`]: {...asStoredRule(merchantRuleBody), pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE}, + }; + + expect(getExpenseDefaultRuleCount(collection, POLICY_ID)).toBe(0); + }); + + it('is zero for an empty collection', () => { + expect(getExpenseDefaultRuleCount({}, POLICY_ID)).toBe(0); + }); + }); + + describe('hasExpenseDefaultRuleErrors', () => { + const failedRule = {...asStoredRule(merchantRuleBody), errors: {error: 'Whoops'}}; + + it("is true when one of the policy's rules failed to save", () => { + expect(hasExpenseDefaultRuleErrors({[`${ONYXKEYS.COLLECTION.RULE}1`]: failedRule}, POLICY_ID)).toBe(true); + }); + + it('is false when the failed rule belongs to another policy', () => { + expect(hasExpenseDefaultRuleErrors({[`${ONYXKEYS.COLLECTION.RULE}1`]: {...failedRule, scopeID: OTHER_POLICY_ID}}, POLICY_ID)).toBe(false); + }); + + it('is false when no rule carries an error', () => { + expect(hasExpenseDefaultRuleErrors({[`${ONYXKEYS.COLLECTION.RULE}1`]: asStoredRule(merchantRuleBody)}, POLICY_ID)).toBe(false); + }); + }); + + describe('canEditMerchantRule', () => { + it('allows a representable rule scoped to the policy', () => { + expect(canEditMerchantRule(asStoredRule(merchantRuleBody), POLICY_ID)).toBe(true); + }); + + it('refuses a rule belonging to another policy', () => { + expect(canEditMerchantRule(asStoredRule(merchantRuleBody, OTHER_POLICY_ID), POLICY_ID)).toBe(false); + }); + + it('refuses an account scoped rule', () => { + expect(canEditMerchantRule(asStoredRule(merchantRuleBody, POLICY_ID, CONST.RULES.SCOPE.ACCOUNT), POLICY_ID)).toBe(false); + }); + + it('refuses an approval workflow rule that happens to share the ruleID', () => { + const approvalWorkflowRule = asStoredRule( + buildRuleWithOverrides({ + triggers: toIndexMap([CONST.RULES.APPROVAL_WORKFLOW.TRIGGER.REPORT_SUBMIT]), + actions: toIndexMap([{name: CONST.RULES.APPROVAL_WORKFLOW.ACTION.FORWARD_TO, approver: 'a@b.com'}]), + }), + ); + + expect(canEditMerchantRule(approvalWorkflowRule, POLICY_ID)).toBe(false); + }); + + it('refuses a rule the form cannot represent, so saving cannot overwrite it', () => { + const nested = asStoredRule( + buildRuleWithOverrides({ + filters: {left: merchantFilter, operator: AND, right: {left: FIELD.MERCHANT, operator: CONTAINS, right: 'Costa'}}, + }), + ); + + expect(canEditMerchantRule(nested, POLICY_ID)).toBe(false); + }); + + it('refuses without a rule or without a policy', () => { + expect(canEditMerchantRule(undefined, POLICY_ID)).toBe(false); + expect(canEditMerchantRule(asStoredRule(merchantRuleBody), undefined)).toBe(false); + }); + }); + + describe('getRuleMerchantMatchSummary', () => { + it('reads a single merchant leaf', () => { + expect(getRuleMerchantMatchSummary(merchantFilter)).toEqual({merchants: 'Starbucks', isExactMatch: false}); + }); + + it('flags an exact match', () => { + expect(getRuleMerchantMatchSummary({left: FIELD.MERCHANT, operator: EQUAL_TO, right: 'Starbucks'})).toEqual({merchants: 'Starbucks', isExactMatch: true}); + }); + + it('joins the values of a merchant leaf holding a list, which the editor cannot represent', () => { + const filters: RuleFilterComparison = {left: FIELD.MERCHANT, operator: EQUAL_TO, right: ['Starbucks', 'Costa']}; + + expect(getRuleMerchantMatchSummary(filters)).toEqual({merchants: 'Starbucks, Costa', isExactMatch: true}); + }); + + it('joins the merchants of a nested tree, which the editor cannot represent', () => { + const filters: RuleFilterNode = { + left: merchantFilter, + operator: AND, + right: {left: FIELD.MERCHANT, operator: CONTAINS, right: 'Costa'}, + }; + + expect(getRuleMerchantMatchSummary(filters)).toEqual({merchants: 'Starbucks, Costa', isExactMatch: false}); + // The same rule stays read-only, so the summary is the only thing the row can show. + expect(getMerchantRuleFormValues(asStoredRule({...merchantRuleBody, filters}))).toBeUndefined(); + }); + + it('is not an exact match when the merchant leaves disagree on the operator', () => { + const filters: RuleFilterNode = { + left: {left: FIELD.MERCHANT, operator: EQUAL_TO, right: 'Starbucks'}, + operator: OR, + right: {left: FIELD.MERCHANT, operator: CONTAINS, right: 'Costa'}, + }; + + expect(getRuleMerchantMatchSummary(filters).isExactMatch).toBe(false); + }); + + it('ignores leaves on other fields', () => { + const filters: RuleFilterNode = { + left: merchantFilter, + operator: AND, + right: {left: FIELD.CATEGORY, operator: EQUAL_TO, right: 'Coffee'}, + }; + + expect(getRuleMerchantMatchSummary(filters)).toEqual({merchants: 'Starbucks', isExactMatch: false}); + }); + + it('is empty without a merchant leaf', () => { + expect(getRuleMerchantMatchSummary({left: FIELD.CATEGORY, operator: EQUAL_TO, right: 'Coffee'})).toEqual({merchants: '', isExactMatch: false}); + }); + + it('is empty without filters', () => { + expect(getRuleMerchantMatchSummary(undefined)).toEqual({merchants: '', isExactMatch: false}); + }); + }); + + describe('getRuleFilterLeaves', () => { + it('flattens a nested tree left to right', () => { + const middleLeaf: RuleFilterComparison = {left: FIELD.CATEGORY, operator: EQUAL_TO, right: 'Coffee'}; + const rightLeaf: RuleFilterComparison = {left: FIELD.TAG, operator: EQUAL_TO, right: 'Team A'}; + const tree: RuleFilterNode = {left: {left: merchantFilter, operator: OR, right: middleLeaf}, operator: AND, right: rightLeaf}; + + expect(getRuleFilterLeaves(tree)).toEqual([merchantFilter, middleLeaf, rightLeaf]); + }); + + it('returns a single leaf unchanged', () => { + expect(getRuleFilterLeaves(merchantFilter)).toEqual([merchantFilter]); + }); + }); + + describe('getExpenseDefaultRuleSummaryFields', () => { + it('lists the fields a rule sets in action-key order, including rules the form cannot open', () => { + // Keys are deliberately out of numeric order, and "10" would sort before "2" as a string. + const rule = buildRuleWithOverrides({ + actions: Object.fromEntries([ + ['0', setCategoryAction], + ['10', {name: ACTION.SET, field: FIELD.BILLABLE, value: true}], + ['2', {name: ACTION.SET, field: FIELD.COMMENT, value: Parser.replace('A description')}], + ]), + }); + + expect(getExpenseDefaultRuleSummaryFields(rule)).toEqual([ + {field: FIELD.CATEGORY, value: 'Coffee'}, + {field: FIELD.COMMENT, value: 'A description'}, + {field: FIELD.BILLABLE, value: true}, + ]); + }); + + it('skips actions that do not set a field', () => { + const rule = buildRuleWithOverrides({actions: toIndexMap([{name: CONST.RULES.APPROVAL_WORKFLOW.ACTION.FORWARD_TO, approver: 'a@b.com'}])}); + + expect(getExpenseDefaultRuleSummaryFields(rule)).toEqual([]); + }); + }); +}); diff --git a/tests/unit/ImportedMerchantRulesPageTest.tsx b/tests/unit/ImportedMerchantRulesPageTest.tsx index 758b28682a41..a0960cfd7507 100644 --- a/tests/unit/ImportedMerchantRulesPageTest.tsx +++ b/tests/unit/ImportedMerchantRulesPageTest.tsx @@ -15,7 +15,7 @@ import ImportedMerchantRulesPage, { import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; -import type {ImportedSpreadsheet, Policy, PolicyCategories} from '@src/types/onyx'; +import type {ImportedSpreadsheet, Policy, PolicyCategories, Rule} from '@src/types/onyx'; import React from 'react'; import Onyx from 'react-native-onyx'; @@ -23,6 +23,11 @@ import Onyx from 'react-native-onyx'; import {buildPersonalDetails} from '../utils/TestHelper'; import waitForBatchedUpdatesWithAct from '../utils/waitForBatchedUpdatesWithAct'; +/** Mirrors the way the rules engine keys `triggers` and `actions` by a stringified index. */ +function toIndexMap(values: T[]): Record { + return Object.fromEntries(values.map((value, index) => [String(index), value])); +} + const POLICY_ID = 'imported-merchant-rules-test-policy'; const ADMIN_EMAIL = 'admin@example.com'; const ADMIN_ACCOUNT_ID = 1; @@ -268,7 +273,7 @@ describe('ImportedMerchantRulesPage', () => { describe('parseSpreadsheetRules', () => { it('builds a net-new rule from a mapped row', () => { - const result = parseSpreadsheetRules(buildSpreadsheet(), true, buildRulesEnabledControlPolicy(), undefined); + const result = parseSpreadsheetRules(buildSpreadsheet(), true, buildRulesEnabledControlPolicy(), undefined, {}); expect(Object.keys(result.rules)).toHaveLength(1); expect(Object.values(result.rules).at(0)).toMatchObject({ @@ -279,22 +284,40 @@ describe('ImportedMerchantRulesPage', () => { expect(result.invalidCategoryNames.size).toBe(0); }); - it('skips a row that duplicates an existing coding rule', () => { + it('skips a row that duplicates an existing merchant rule', () => { const policy = buildRulesEnabledControlPolicy(); - policy.rules = { - codingRules: { - existing: {filters: {left: 'merchant', operator: CONST.SEARCH.SYNTAX_OPERATORS.EQUAL_TO, right: 'Starbucks'}, merchant: 'SBUX'}, - }, + const existingRule: Rule = { + scope: CONST.RULES.SCOPE.POLICY, + scopeID: policy.id, + triggers: toIndexMap([CONST.RULES.EXPENSE_DEFAULT.TRIGGER.CREATE_TRANSACTION]), + filters: {left: CONST.RULES.EXPENSE_DEFAULT.FIELD.MERCHANT, operator: CONST.SEARCH.SYNTAX_OPERATORS.EQUAL_TO, right: 'Starbucks'}, + actions: toIndexMap([{name: CONST.RULES.EXPENSE_DEFAULT.ACTION.SET, field: CONST.RULES.EXPENSE_DEFAULT.FIELD.MERCHANT, value: 'SBUX'}]), }; - const result = parseSpreadsheetRules(buildSpreadsheet(), true, policy, undefined); + const result = parseSpreadsheetRules(buildSpreadsheet(), true, policy, undefined, {[`${ONYXKEYS.COLLECTION.RULE}existing`]: existingRule}); expect(Object.keys(result.rules)).toHaveLength(0); expect(result.skippedDuplicateCount).toBe(1); }); + it('does not treat a rule from another policy as a duplicate', () => { + const policy = buildRulesEnabledControlPolicy(); + const otherPolicyRule: Rule = { + scope: CONST.RULES.SCOPE.POLICY, + scopeID: 'another-policy', + triggers: toIndexMap([CONST.RULES.EXPENSE_DEFAULT.TRIGGER.CREATE_TRANSACTION]), + filters: {left: CONST.RULES.EXPENSE_DEFAULT.FIELD.MERCHANT, operator: CONST.SEARCH.SYNTAX_OPERATORS.EQUAL_TO, right: 'Starbucks'}, + actions: toIndexMap([{name: CONST.RULES.EXPENSE_DEFAULT.ACTION.SET, field: CONST.RULES.EXPENSE_DEFAULT.FIELD.MERCHANT, value: 'SBUX'}]), + }; + + const result = parseSpreadsheetRules(buildSpreadsheet(), true, policy, undefined, {[`${ONYXKEYS.COLLECTION.RULE}other`]: otherPolicyRule}); + + expect(Object.keys(result.rules)).toHaveLength(1); + expect(result.skippedDuplicateCount).toBe(0); + }); + it('drops a row whose category cell does not match a workspace category', () => { - const result = parseSpreadsheetRules(buildInvalidCategorySpreadsheet(), true, buildRulesEnabledControlPolicy(), undefined); + const result = parseSpreadsheetRules(buildInvalidCategorySpreadsheet(), true, buildRulesEnabledControlPolicy(), undefined, {}); expect(Object.keys(result.rules)).toHaveLength(0); expect([...result.invalidCategoryNames]).toEqual(['nonexistent category']); diff --git a/tests/unit/MerchantRuleTaxSummaryTest.ts b/tests/unit/MerchantRuleTaxSummaryTest.ts new file mode 100644 index 000000000000..5ab44d9f7b46 --- /dev/null +++ b/tests/unit/MerchantRuleTaxSummaryTest.ts @@ -0,0 +1,82 @@ +import {getMerchantRulesTableData} from '@libs/MerchantTypeRulesUtils'; + +import CONST from '@src/CONST'; +import IntlStore from '@src/languages/IntlStore'; +import ONYXKEYS from '@src/ONYXKEYS'; +import type {Policy, Rule} from '@src/types/onyx'; +import type {ExpenseDefaultTaxValue} from '@src/types/onyx/ExpenseDefaultRules'; + +import createRandomPolicy from '../utils/collections/policies'; +import {translateLocal} from '../utils/TestHelper'; +import waitForBatchedUpdates from '../utils/waitForBatchedUpdates'; + +const {FIELD, TRIGGER, ACTION} = CONST.RULES.EXPENSE_DEFAULT; +const TAX_KEY = 'id_TAX_RATE_1'; + +/** Mirrors the way the rules engine keys `triggers` and `actions` by a stringified index. */ +function toIndexMap(values: T[]): Record { + return Object.fromEntries(values.map((value, index) => [String(index), value])); +} + +/** A policy whose tax list either holds the rate the rule points at, or has not loaded it. */ +const buildPolicy = (taxes?: Record): Policy => ({ + ...createRandomPolicy(0), + id: 'policy1', + taxRates: { + name: 'Tax', + defaultExternalID: TAX_KEY, + defaultValue: '10%', + foreignTaxDefault: TAX_KEY, + taxes: taxes ?? {}, + }, +}); + +/** + * The rule stores whatever the rate was called when it was saved. `savedTaxRate` omitted models a rule + * saved before the policy's tax rates had loaded, which is when `buildTaxActionValue` writes no snapshot. + */ +const buildTaxRule = (savedTaxRate?: {name: string; value: string}): Rule => { + // eslint-disable-next-line @typescript-eslint/naming-convention + const taxValue: ExpenseDefaultTaxValue = {field_id_TAX: {externalID: TAX_KEY, ...(savedTaxRate ?? {})}}; + + return { + scope: CONST.RULES.SCOPE.POLICY, + scopeID: 'policy1', + triggers: toIndexMap([TRIGGER.CREATE_TRANSACTION]), + filters: {left: FIELD.MERCHANT, operator: CONST.SEARCH.SYNTAX_OPERATORS.EQUAL_TO, right: 'Coffee Shop'}, + actions: toIndexMap([{name: ACTION.SET, field: FIELD.TAX, value: taxValue}]), + }; +}; + +describe('Merchant rule tax summary', () => { + beforeEach(() => { + IntlStore.load(CONST.LOCALES.EN); + return waitForBatchedUpdates(); + }); + + const describeRule = (policy: Policy, rule: Rule) => + getMerchantRulesTableData({ + policy, + policyID: policy.id, + rules: {[`${ONYXKEYS.COLLECTION.RULE}rule1`]: rule}, + translate: translateLocal, + isOffline: false, + onNavigate: () => {}, + }).at(0)?.ruleDescription; + + describe('Expense defaults table', () => { + it('prefers the live rate over the one captured when the rule was saved', () => { + const policy = buildPolicy({[TAX_KEY]: {name: 'GST', value: '15%'}}); + + expect(describeRule(policy, buildTaxRule({name: 'Old GST', value: '10%'}))).toContain('Update tax to "GST (15%)"'); + }); + + it('falls back to the saved rate when the policy no longer lists it', () => { + expect(describeRule(buildPolicy(), buildTaxRule({name: 'GST', value: '10%'}))).toContain('Update tax to "GST (10%)"'); + }); + + it('falls back to the tax ID rather than dropping the default when there is no saved rate', () => { + expect(describeRule(buildPolicy(), buildTaxRule())).toContain(`Update tax to "${TAX_KEY}"`); + }); + }); +}); diff --git a/tests/unit/PolicyUtilsTest.ts b/tests/unit/PolicyUtilsTest.ts index c670928ba8ef..253698d9824c 100644 --- a/tests/unit/PolicyUtilsTest.ts +++ b/tests/unit/PolicyUtilsTest.ts @@ -3803,23 +3803,17 @@ describe('PolicyUtils', () => { }); }); - describe('rules.codingRules', () => { - it('returns true when codingRules has entries', () => { - const policy = createMock({ - rules: { - codingRules: { - rule1: { - ruleID: 'rule1', - filters: {left: 'merchant', operator: CONST.SEARCH.SYNTAX_OPERATORS.EQUAL_TO, right: 'Starbucks'}, - }, - }, - }, - }); - expect(hasConfiguredRules(policy)).toBe(true); + describe('merchant rules', () => { + // Which rules count towards this is decided by the caller's selector and covered in + // ExpenseDefaultRuleUtilsTest, so only the pass-through is checked here. + const policy = createMock({id: 'policy1', rules: {}}); + + it('returns true when the policy has a merchant rule', () => { + expect(hasConfiguredRules(policy, undefined, true)).toBe(true); }); - it('returns false when codingRules is empty', () => { - expect(hasConfiguredRules(createMock({rules: {codingRules: {}}}))).toBe(false); + it('returns false when it has none', () => { + expect(hasConfiguredRules(policy, undefined, false)).toBe(false); }); }); @@ -4884,39 +4878,37 @@ describe('PolicyUtils', () => { }); describe('hasPolicyRulesError', () => { + // Whether a merchant rule failed is reduced by the caller's selector and covered in + // ExpenseDefaultRuleUtilsTest, so only the agent rules and the pass-through are checked here. + const POLICY_ID = 'policy-with-rules'; + it('returns false for an undefined policy', () => { expect(hasPolicyRulesError(undefined)).toBe(false); }); - it('returns false when no coding or agent rules exist', () => { - const policy: Policy = {...createRandomPolicy(0), rules: {}}; + it('returns false when no merchant or agent rules exist', () => { + const policy: Policy = {...createRandomPolicy(0), id: POLICY_ID, rules: {}}; expect(hasPolicyRulesError(policy)).toBe(false); }); - it('returns false when rules exist but none have errors', () => { + it('returns false when agent rules exist but none have errors', () => { const policy: Policy = { ...createRandomPolicy(0), - rules: { - codingRules: {rule1: {ruleID: 'rule1', filters: {left: 'merchant', operator: CONST.SEARCH.SYNTAX_OPERATORS.EQUAL_TO, right: 'Starbucks'}}}, - agentRules: {ai1: {ruleID: 'ai1', prompt: 'p', created: '2026-06-08'}}, - }, + id: POLICY_ID, + rules: {agentRules: {ai1: {ruleID: 'ai1', prompt: 'p', created: '2026-06-08'}}}, }; expect(hasPolicyRulesError(policy)).toBe(false); }); - it('returns true when a coding rule has errors', () => { - const policy: Policy = { - ...createRandomPolicy(0), - rules: { - codingRules: {rule1: {ruleID: 'rule1', filters: {left: 'merchant', operator: CONST.SEARCH.SYNTAX_OPERATORS.EQUAL_TO, right: 'Starbucks'}, errors: {123: 'boom'}}}, - }, - }; - expect(hasPolicyRulesError(policy)).toBe(true); + it('returns true when a merchant rule has errors', () => { + const policy: Policy = {...createRandomPolicy(0), id: POLICY_ID, rules: {}}; + expect(hasPolicyRulesError(policy, true)).toBe(true); }); it('returns true when an agent rule has errors', () => { const policy: Policy = { ...createRandomPolicy(0), + id: POLICY_ID, rules: { agentRules: {ai1: {ruleID: 'ai1', prompt: 'p', created: '2026-06-08', errors: {123: 'boom'}}}, }, diff --git a/tests/unit/VendorMatchingMerchantRulesTest.ts b/tests/unit/VendorMatchingMerchantRulesTest.ts index 302520102bb4..9f148e45d0f1 100644 --- a/tests/unit/VendorMatchingMerchantRulesTest.ts +++ b/tests/unit/VendorMatchingMerchantRulesTest.ts @@ -1,18 +1,24 @@ -import {mapFormFieldsToRuleForAPI, mapFormFieldsToRuleForOnyx} from '@libs/actions/Policy/Rules'; -import {getMerchantCodingRulesTableData} from '@libs/MerchantTypeRulesUtils'; +import {buildMerchantRule} from '@libs/ExpenseDefaultRuleUtils'; +import {getMerchantRulesTableData} from '@libs/MerchantTypeRulesUtils'; import {hasVendorFeature} from '@libs/PolicyUtils'; import CONST from '@src/CONST'; import IntlStore from '@src/languages/IntlStore'; +import ONYXKEYS from '@src/ONYXKEYS'; import type {MerchantRuleForm} from '@src/types/form/MerchantRuleForm'; -import type {Policy} from '@src/types/onyx'; -import type {CodingRule, Connections} from '@src/types/onyx/Policy'; +import type {Policy, Rule} from '@src/types/onyx'; +import type {Connections} from '@src/types/onyx/Policy'; import createRandomPolicy from '../utils/collections/policies'; import createMock from '../utils/createMock'; import {translateLocal} from '../utils/TestHelper'; import waitForBatchedUpdates from '../utils/waitForBatchedUpdates'; +/** Mirrors the way the rules engine keys `triggers` and `actions` by a stringified index. */ +function toIndexMap(values: T[]): Record { + return Object.fromEntries(values.map((value, index) => [String(index), value])); +} + /** * A minimal merchant rule form. Individual tests override only the fields they exercise, so the * mappers are validated against a realistic full form rather than a hand-picked subset. @@ -92,68 +98,61 @@ const buildQBOWithStaleXeroPolicy = (qboVendors: Array<{id: string; name: string }), }); -const withCodingRules = (policy: Policy, codingRules: Record): Policy => ({...policy, rules: {...policy.rules, codingRules}}); - -const buildVendorRule = (vendorID: string): CodingRule => ({ - filters: {left: 'merchant', operator: CONST.SEARCH.SYNTAX_OPERATORS.EQUAL_TO, right: 'Coffee Shop'}, - vendorID, +const buildVendorRule = (policy: Policy, vendorID: string): Rule => ({ + ...buildMerchantRule({merchantToMatch: 'Coffee Shop', matchType: CONST.SEARCH.SYNTAX_OPERATORS.EQUAL_TO, vendorID}, policy), + scope: CONST.RULES.SCOPE.POLICY, + scopeID: policy.id, + triggers: toIndexMap([CONST.RULES.EXPENSE_DEFAULT.TRIGGER.CREATE_TRANSACTION]), + filters: {left: CONST.RULES.EXPENSE_DEFAULT.FIELD.MERCHANT, operator: CONST.SEARCH.SYNTAX_OPERATORS.EQUAL_TO, right: 'Coffee Shop'}, + actions: toIndexMap([{name: CONST.RULES.EXPENSE_DEFAULT.ACTION.SET, field: CONST.RULES.EXPENSE_DEFAULT.FIELD.VENDOR_ID, value: vendorID}]), }); -describe('Vendor matching on merchant rules', () => { - describe('mapFormFieldsToRuleForOnyx', () => { - it('serializes a set vendorID', () => { - expect(mapFormFieldsToRuleForOnyx(buildForm({vendorID: 'v-1'}), undefined).vendorID).toBe('v-1'); - }); +const withVendorRule = (policy: Policy, vendorID: string) => ({[`${ONYXKEYS.COLLECTION.RULE}rule1`]: buildVendorRule(policy, vendorID)}); - it('serializes an unset vendorID to null so Onyx merge clears it', () => { - expect(mapFormFieldsToRuleForOnyx(buildForm({vendorID: ''}), undefined).vendorID).toBeNull(); - }); - }); - - describe('mapFormFieldsToRuleForAPI', () => { - it('includes vendorID when set', () => { - expect(mapFormFieldsToRuleForAPI(buildForm({vendorID: 'v-1'}), undefined).vendorID).toBe('v-1'); +describe('Vendor matching on merchant rules', () => { + describe('buildMerchantRule vendor action', () => { + it('writes a Set action for a vendorID', () => { + const actions = Object.values(buildMerchantRule(buildForm({vendorID: 'v-1'}), undefined)?.actions ?? {}); + expect(actions).toContainEqual({name: CONST.RULES.EXPENSE_DEFAULT.ACTION.SET, field: CONST.RULES.EXPENSE_DEFAULT.FIELD.VENDOR_ID, value: 'v-1'}); }); - it('omits vendorID entirely when unset (never sends null)', () => { - const rule = mapFormFieldsToRuleForAPI(buildForm({vendorID: ''}), undefined); - expect('vendorID' in rule).toBe(false); + it('writes no vendor action when the vendorID is unset, so the rule stops setting it', () => { + const actions = Object.values(buildMerchantRule(buildForm({vendorID: '', category: 'Coffee'}), undefined)?.actions ?? {}); + expect(actions.some((action) => action.field === CONST.RULES.EXPENSE_DEFAULT.FIELD.VENDOR_ID)).toBe(false); }); }); - describe('getMerchantCodingRulesTableData vendor summary', () => { + describe('getMerchantRulesTableData vendor summary', () => { beforeEach(() => { IntlStore.load(CONST.LOCALES.EN); return waitForBatchedUpdates(); }); - const buildTableData = (policy: Policy) => - getMerchantCodingRulesTableData({ + const buildTableData = (policy: Policy, vendorID: string) => + getMerchantRulesTableData({ policy, policyID: policy.id, + rules: withVendorRule(policy, vendorID), translate: translateLocal, isOffline: false, onNavigate: () => {}, }); it('resolves the vendor name when the vendor is in the loaded list', () => { - const policy = withCodingRules(buildQBOPolicy([{id: 'v-1', name: 'Acme Co', currency: 'USD'}]), {rule1: buildVendorRule('v-1')}); - expect(buildTableData(policy).at(0)?.ruleDescription).toContain('Update vendor to "Acme Co"'); + const policy = buildQBOPolicy([{id: 'v-1', name: 'Acme Co', currency: 'USD'}]); + expect(buildTableData(policy, 'v-1').at(0)?.ruleDescription).toContain('Update vendor to "Acme Co"'); }); it('shows "Vendor unavailable" when the list is loaded but the vendor is missing', () => { - const policy = withCodingRules(buildQBOPolicy([]), {rule1: buildVendorRule('v-1')}); - expect(buildTableData(policy).at(0)?.ruleDescription).toContain('Update vendor to "Vendor unavailable"'); + expect(buildTableData(buildQBOPolicy([]), 'v-1').at(0)?.ruleDescription).toContain('Update vendor to "Vendor unavailable"'); }); it('preserves the raw external ID while the active vendor list is not hydrated', () => { - const policy = withCodingRules(buildQBOPolicy(undefined), {rule1: buildVendorRule('v-1')}); - expect(buildTableData(policy).at(0)?.ruleDescription).toContain('Update vendor to "v-1"'); + expect(buildTableData(buildQBOPolicy(undefined), 'v-1').at(0)?.ruleDescription).toContain('Update vendor to "v-1"'); }); it('renders "Vendor unavailable" when no matching integration remains', () => { - const policy = withCodingRules(createRandomPolicy(0), {rule1: buildVendorRule('v-1')}); - const description = buildTableData(policy).at(0)?.ruleDescription; + const description = buildTableData(createRandomPolicy(0), 'v-1').at(0)?.ruleDescription; expect(description).toContain('Update vendor to "Vendor unavailable"'); expect(description).not.toContain('"v-1"'); }); @@ -162,10 +161,8 @@ describe('Vendor matching on merchant rules', () => { // Active source is QBO (empty vendor list, so loaded). The rule's vendorID matches only the stale Xero // connection, which the active picker and violation logic ignore. The summary must not render the Xero // name as if the vendor were valid — it should surface the active-scoped "unavailable" copy instead. - const policy = withCodingRules(buildQBOWithStaleXeroPolicy([], {xeroVendor: {id: 'xeroVendor', name: 'Stale Xero Vendor', email: 'stale@example.com'}}), { - rule1: buildVendorRule('xeroVendor'), - }); - const description = buildTableData(policy).at(0)?.ruleDescription; + const policy = buildQBOWithStaleXeroPolicy([], {xeroVendor: {id: 'xeroVendor', name: 'Stale Xero Vendor', email: 'stale@example.com'}}); + const description = buildTableData(policy, 'xeroVendor').at(0)?.ruleDescription; expect(description).toContain('Update vendor to "Vendor unavailable"'); expect(description).not.toContain('Stale Xero Vendor'); }); @@ -175,19 +172,17 @@ describe('Vendor matching on merchant rules', () => { // (vendor-matching active). Admin later switches to Vendor Bill, so QBO is no longer the active vendor-matching // source. The rule summary must still render the vendor's name — not the raw external ID — because the vendor // list is still known via the connection data. - const policy = withCodingRules(buildQBOWithVendorBillExportPolicy([{id: 'v-1', name: 'Acme Co', currency: 'USD'}]), {rule1: buildVendorRule('v-1')}); - expect(buildTableData(policy).at(0)?.ruleDescription).toContain('Update vendor to "Acme Co"'); + const policy = buildQBOWithVendorBillExportPolicy([{id: 'v-1', name: 'Acme Co', currency: 'USD'}]); + expect(buildTableData(policy, 'v-1').at(0)?.ruleDescription).toContain('Update vendor to "Acme Co"'); }); it('uses "supplier" wording and "Supplier unavailable" on Xero workspaces', () => { - const resolved = withCodingRules(buildXeroPolicy({xc1: {id: 'xc1', name: 'Acme Xero', email: 'acme@example.com'}}), {rule1: buildVendorRule('xc1')}); - expect(buildTableData(resolved).at(0)?.ruleDescription).toContain('Update supplier to "Acme Xero"'); + const resolved = buildXeroPolicy({xc1: {id: 'xc1', name: 'Acme Xero', email: 'acme@example.com'}}); + expect(buildTableData(resolved, 'xc1').at(0)?.ruleDescription).toContain('Update supplier to "Acme Xero"'); - const missing = withCodingRules(buildXeroPolicy({}), {rule1: buildVendorRule('xc1')}); - expect(buildTableData(missing).at(0)?.ruleDescription).toContain('Update supplier to "Supplier unavailable"'); + expect(buildTableData(buildXeroPolicy({}), 'xc1').at(0)?.ruleDescription).toContain('Update supplier to "Supplier unavailable"'); - const pendingHydration = withCodingRules(buildXeroPolicy(undefined), {rule1: buildVendorRule('xc1')}); - expect(buildTableData(pendingHydration).at(0)?.ruleDescription).toContain('Update supplier to "xc1"'); + expect(buildTableData(buildXeroPolicy(undefined), 'xc1').at(0)?.ruleDescription).toContain('Update supplier to "xc1"'); }); }); diff --git a/tests/unit/WorkflowUtilsTest.ts b/tests/unit/WorkflowUtilsTest.ts index c0138d1fec10..76923856f8ba 100644 --- a/tests/unit/WorkflowUtilsTest.ts +++ b/tests/unit/WorkflowUtilsTest.ts @@ -16,6 +16,7 @@ import { getOverLimitForwardsToDisplayName, getRulesSubmitterToFirstApprover, getRulesSubmitterToWorkflowKey, + isApprovalWorkflowRule, mergeWorkflowMembersWithAvailableMembers, reconcileApprovalWorkflowRulesForCreate, reconcileApprovalWorkflowRulesForEdit, @@ -26,12 +27,13 @@ import { import type {Policy} from '@src/types/onyx'; import type {Approver, Member} from '@src/types/onyx/ApprovalWorkflow'; import type ApprovalWorkflow from '@src/types/onyx/ApprovalWorkflow'; -import type {ApprovalWorkflowFilter, ApprovalWorkflowFilterComparison, ApprovalWorkflowRule} from '@src/types/onyx/ApprovalWorkflowRules'; +import type {ApprovalWorkflowRule} from '@src/types/onyx/ApprovalWorkflowRules'; import type {BankAccountList} from '@src/types/onyx/BankAccount'; import type {PersonalDetailsList} from '@src/types/onyx/PersonalDetails'; import type {PolicyEmployeeList} from '@src/types/onyx/PolicyEmployee'; import type PolicyEmployee from '@src/types/onyx/PolicyEmployee'; import type Rule from '@src/types/onyx/Rule'; +import type {RuleFilter, RuleFilterComparison} from '@src/types/onyx/RuleFilters'; import createRandomPolicy from '../utils/collections/policies'; import createMock from '../utils/createMock'; @@ -1655,7 +1657,7 @@ describe('WorkflowUtils', () => { const approveActions = {'0': {name: CONST.RULES.APPROVAL_WORKFLOW.ACTION.APPROVE_REPORT}}; const buildFromFilter = (emails: string[]) => ({operator: CONST.SEARCH.SYNTAX_OPERATORS.EQUAL_TO, left: CONST.SEARCH.SYNTAX_FILTER_KEYS.FROM, right: emails}); const buildToFilter = (email: string) => ({operator: CONST.SEARCH.SYNTAX_OPERATORS.EQUAL_TO, left: CONST.SEARCH.SYNTAX_FILTER_KEYS.TO, right: email}); - const and = (left: ApprovalWorkflowFilter | ApprovalWorkflowFilterComparison, right: ApprovalWorkflowFilter | ApprovalWorkflowFilterComparison): ApprovalWorkflowFilter => ({ + const and = (left: RuleFilter | RuleFilterComparison, right: RuleFilter | RuleFilterComparison): RuleFilter => ({ operator: CONST.SEARCH.SYNTAX_OPERATORS.AND, left, right, @@ -2110,7 +2112,7 @@ describe('WorkflowUtils', () => { }); describe('filterRulesForPolicy', () => { - const ruleForPolicy = (scopeID: string, extra: Partial = {}): Rule => ({ + const ruleForPolicy = (scopeID: string, extra: Partial> = {}): Rule => ({ scope: CONST.RULES.SCOPE.POLICY, scopeID, triggers: {'0': CONST.RULES.APPROVAL_WORKFLOW.TRIGGER.REPORT_SUBMIT}, @@ -2138,4 +2140,40 @@ describe('WorkflowUtils', () => { expect(filterRulesForPolicy(undefined, 'policy1')).toEqual({}); }); }); + + describe('isApprovalWorkflowRule', () => { + // `Rule` types triggers as one kind or the other, so a rule mixing them can only arrive from the + // server. Building the collection untyped and asserting once is the only way to model that payload. + const ruleWithTriggers = (...triggers: string[]) => { + const rule = { + scope: CONST.RULES.SCOPE.POLICY, + scopeID: 'policy1', + triggers: Object.fromEntries(triggers.map((trigger, index) => [String(index), trigger])), + filters: {operator: CONST.SEARCH.SYNTAX_OPERATORS.EQUAL_TO, left: CONST.SEARCH.SYNTAX_FILTER_KEYS.FROM, right: 'a@example.com'}, + actions: {'0': {name: CONST.RULES.APPROVAL_WORKFLOW.ACTION.FORWARD_TO, approver: 'b@example.com'}}, + }; + + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + return rule as unknown as Rule; + }; + + it('is true for a rule that only fires on report events', () => { + expect(isApprovalWorkflowRule(ruleWithTriggers(CONST.RULES.APPROVAL_WORKFLOW.TRIGGER.REPORT_SUBMIT))).toBe(true); + expect(isApprovalWorkflowRule(ruleWithTriggers(CONST.RULES.APPROVAL_WORKFLOW.TRIGGER.REPORT_SUBMIT, CONST.RULES.APPROVAL_WORKFLOW.TRIGGER.REPORT_APPROVE))).toBe(true); + }); + + it('is false for an expense default rule', () => { + expect(isApprovalWorkflowRule(ruleWithTriggers(CONST.RULES.EXPENSE_DEFAULT.TRIGGER.CREATE_TRANSACTION))).toBe(false); + }); + + it('is false for a rule that also fires on transaction creation, so disabling approvals cannot delete it', () => { + const mixed = ruleWithTriggers(CONST.RULES.APPROVAL_WORKFLOW.TRIGGER.REPORT_SUBMIT, CONST.RULES.EXPENSE_DEFAULT.TRIGGER.CREATE_TRANSACTION); + + expect(isApprovalWorkflowRule(mixed)).toBe(false); + }); + + it('is false for a rule with no triggers at all', () => { + expect(isApprovalWorkflowRule(ruleWithTriggers())).toBe(false); + }); + }); }); diff --git a/tests/unit/getWorkspaceMenuItemsTest.ts b/tests/unit/getWorkspaceMenuItemsTest.ts index ef2afd1fbf39..3119fa781e1a 100644 --- a/tests/unit/getWorkspaceMenuItemsTest.ts +++ b/tests/unit/getWorkspaceMenuItemsTest.ts @@ -266,32 +266,36 @@ describe('getWorkspaceMenuItems', () => { expect(items.find((item) => item.translationKey === 'workspace.common.workflows')?.brickRoadIndicator).toBe(CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR); }); - it('shows an error indicator when rules have errors', () => { - const policy = createMock({ - ...buildPolicy(CONST.POLICY.ROLE.ADMIN), - areRulesEnabled: true, - rules: { - codingRules: { - rule: { - ruleID: 'rule', - filters: {left: 'merchant', operator: CONST.SEARCH.SYNTAX_OPERATORS.EQUAL_TO, right: 'Acme'}, - errors: {error: 'Whoops'}, - }, - }, - }, - }); + it('shows an error indicator when a merchant rule failed to save', () => { + const policy = createMock({...buildPolicy(CONST.POLICY.ROLE.ADMIN), areRulesEnabled: true}); const items = getWorkspaceMenuItems({ policy, policyID: policy.id, currentUserLogin, icons, + hasMerchantRuleErrors: true, convertToDisplayString: () => '', }); expect(items.find((item) => item.translationKey === 'workspace.common.rules')?.brickRoadIndicator).toBe(CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR); }); + it('shows no error indicator when no merchant rule failed', () => { + const policy = createMock({...buildPolicy(CONST.POLICY.ROLE.ADMIN), areRulesEnabled: true}); + + const items = getWorkspaceMenuItems({ + policy, + policyID: policy.id, + currentUserLogin, + icons, + hasMerchantRuleErrors: false, + convertToDisplayString: () => '', + }); + + expect(items.find((item) => item.translationKey === 'workspace.common.rules')?.brickRoadIndicator).toBeUndefined(); + }); + it('shows an information indicator when Merge HR setup is incomplete', () => { const policy = createMock({ ...buildPolicy(CONST.POLICY.ROLE.ADMIN),