[Rules Engine Release 1] Use the new SetRule and DeleteRule commands

src/libs/API/parameters/DeleteRuleParams.ts

@@ -0,0 +1,6 @@
+type DeleteRuleParams = {
+    /** The ID of the rule to delete */
+    ruleID: string;
+};
+
+export default DeleteRuleParams;

src/libs/API/parameters/SetRuleParams.ts

@@ -0,0 +1,28 @@
+import type CONST from '@src/CONST';
+
+import type {ValueOf} from 'type-fest';
+
+type SetRuleParams = {
+    /** What kind of entity the rule is scoped to. Merchant rules are always scoped to a policy */
+    scope: ValueOf<typeof CONST.RULES.SCOPE>;
+
+    /** 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 rule body, as a stringified `RuleValue`. It stays a string because the request is sent as
+     * `FormData`, which can only carry strings.
+     */
+    value: string;
+
+    /** Whether to apply the rule to the transactions that already match it */
+    shouldUpdateMatchingTransactions: boolean;
+};
+
+export default SetRuleParams;

src/libs/API/parameters/index.ts

@@ -644,7 +644,8 @@ export type {default as ChangeDomainSecurityGroupParams} from './ChangeDomainSec
 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';

src/libs/API/types.ts

@@ -289,7 +289,8 @@ const WRITE_COMMANDS = {
     ENABLE_POLICY_TIME_TRACKING: 'EnablePolicyTimeTracking',
     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',
@@ -1045,7 +1046,8 @@ type WriteCommandParameters = {
     [WRITE_COMMANDS.ENABLE_POLICY_INVOICING]: Parameters.EnablePolicyInvoicingParams;
     [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;

src/libs/ExpenseDefaultRuleUtils.ts

@@ -222,8 +222,8 @@ function buildMerchantRuleActions(formValues: Partial<MerchantRuleFormValues>, p
 }
 
 /**
- * Builds the rules engine body for a merchant rule. It is what the `rules_` collection stores, and what the
- * legacy `codingRuleValue` is derived from until writes move to `SetRule`.
+ * Builds the rules engine body for a merchant rule. It is what the `rules_` collection stores and what is 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<MerchantRuleFormValues>, policy: Policy | undefined): BuiltMerchantRule | undefined {
@@ -451,7 +451,7 @@ function canEditMerchantRule(rule: Rule | undefined, policyID: string | undefine
     return isPolicyScopedRule(rule, policyID) && isEditableMerchantRule(rule);
 }
 
-export type {BuiltMerchantRule, MerchantRuleFormValues};
+export type {MerchantRuleFormValues};
 export {
     buildCopiedExpenseDefaultRules,
     buildMerchantRule,

src/libs/actions/Policy/Rules.ts

@@ -5,28 +5,27 @@ import * as API from '@libs/API';
 import type {
     AddPolicyAgentRuleParams,
     DeletePolicyAgentRuleParams,
+    DeleteRuleParams,
     GetAgentRuleSuggestionsParams,
     ImportMerchantRulesSpreadsheetParams,
+    SetRuleParams,
     UpdatePolicyAgentRuleParams,
 } from '@libs/API/parameters';
 import type OpenPolicyRulesPageParams from '@libs/API/parameters/OpenPolicyRulesPageParams';
-import type SetPolicyCodingRuleParams from '@libs/API/parameters/SetPolicyCodingRuleParams';
 import {READ_COMMANDS, SIDE_EFFECT_REQUEST_COMMANDS, WRITE_COMMANDS} from '@libs/API/types';
 import {getMicroSecondOnyxErrorWithTranslationKey} from '@libs/ErrorUtils';
-import {buildMerchantRule, isExpenseDefaultTaxValue} from '@libs/ExpenseDefaultRuleUtils';
-import type {BuiltMerchantRule, MerchantRuleFormValues} from '@libs/ExpenseDefaultRuleUtils';
 import Log from '@libs/Log';
 import {getIsOffline} from '@libs/NetworkState';
 import {rand64} from '@libs/NumberUtils';
 
 import CONST from '@src/CONST';
 import ONYXKEYS from '@src/ONYXKEYS';
-import type {ExpenseDefaultAction} from '@src/types/onyx/ExpenseDefaultRules';
 import type {ImportFinalModal} from '@src/types/onyx/ImportedSpreadsheet';
 import type Policy from '@src/types/onyx/Policy';
 import type {AgentRule, CodingRule, CodingRuleFilter} from '@src/types/onyx/Policy';
 import type {OnyxData} from '@src/types/onyx/Request';
 import type Rule from '@src/types/onyx/Rule';
+import type {RuleBody} from '@src/types/onyx/Rule';
 
 import type {OnyxUpdate} from 'react-native-onyx';
 
@@ -35,69 +34,6 @@ import Onyx from 'react-native-onyx';
 /** A coding rule parsed from an imported spreadsheet row, keyed by a client-generated ruleID */
 type ImportedMerchantRule = Omit<CodingRule, 'ruleID' | 'pendingAction' | 'errors'>;
 
-/**
- * Builds the `codingRuleValue` sent to `SetPolicyCodingRule`. Writes go through the legacy command rather than
- * `SetRule` because it dual-writes into both `policy.rules.codingRules` and the `rules_` collection, while
- * `SetRule` only writes the new collection. Older clients read `codingRules`, so writing only the new collection
- * would silently stop them seeing rules created or edited on a newer client. This whole path goes away once the
- * minimum supported version reads the `rules_` collection.
- *
- * This reads the built rule rather than the form so the optimistic Onyx value and the saved rule share one
- * definition of what an empty field is. Reading the form separately let a padded merchant, a whitespace-only
- * description, or a tax selected before the rates loaded reach the server after the rule had already dropped it.
- */
-function buildLegacyCodingRule(ruleValue: BuiltMerchantRule, ruleID: string, created: string): Partial<CodingRule> {
-    const {FIELD} = CONST.RULES.EXPENSE_DEFAULT;
-    const {ACTIONS: ACTION} = CONST.RULES;
-
-    const valuesByField = new Map<string, ExpenseDefaultAction['value']>();
-    for (const action of Object.values(ruleValue.actions)) {
-        if (action.name !== ACTION.SET) {
-            continue;
-        }
-        valuesByField.set(action.field, action.value);
-    }
-
-    const getStringValue = (field: string): string | undefined => {
-        const value = valuesByField.get(field);
-        return typeof value === 'string' ? value : undefined;
-    };
-    const getBooleanValue = (field: string): boolean | undefined => {
-        const value = valuesByField.get(field);
-        return typeof value === 'boolean' ? value : undefined;
-    };
-
-    const taxValue = valuesByField.get(FIELD.TAX);
-    const tax = isExpenseDefaultTaxValue(taxValue) ? taxValue : undefined;
-    const merchant = getStringValue(FIELD.MERCHANT);
-    const category = getStringValue(FIELD.CATEGORY);
-    const tag = getStringValue(FIELD.TAG);
-    const vendorID = getStringValue(FIELD.VENDOR_ID);
-    const comment = getStringValue(FIELD.COMMENT);
-    const reimbursable = getBooleanValue(FIELD.REIMBURSABLE);
-    const billable = getBooleanValue(FIELD.BILLABLE);
-
-    const {left, operator, right} = ruleValue.filters;
-
-    return {
-        ruleID,
-        filters: {
-            left,
-            operator,
-            right: typeof right === 'string' ? right : String(right),
-        },
-        ...(merchant && {merchant}),
-        ...(category && {category}),
-        ...(tag && {tag}),
-        ...(tax && {tax}),
-        ...(vendorID && {vendorID}),
-        ...(comment && {comment}),
-        ...(reimbursable !== undefined && {reimbursable}),
-        ...(billable !== undefined && {billable}),
-        created,
-    };
-}
-
 /** How long a `GetRules` read may stay marked in flight before the flag is treated as stale. */
 const RULES_FETCH_TIMEOUT_MS = 30 * 1000;
 
@@ -228,26 +164,17 @@ function getAgentRuleSuggestions(policyID: string | undefined) {
 }
 
 /**
- * Creates or updates a merchant rule. Editing a rule reuses its `ruleID`, since the rules engine has no separate update command.
+ * Creates or updates a rule of any kind. 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 ruleBody - The triggers, filters and actions to store, built by whichever editor owns this kind of rule
+ * @param priority - Determines the order rules are applied in when more than one matches
  * @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<MerchantRuleFormValues>,
-    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});
+function setRule(policyID: string, ruleBody: RuleBody | undefined, priority: number, ruleID?: string, existingRule?: Rule, shouldUpdateMatchingTransactions = false) {
+    if (!policyID || !ruleBody) {
+        Log.warn('Invalid params for setRule', {policyID, ruleID});
         return;
     }
 
@@ -256,11 +183,12 @@ function setMerchantRule(
     const ruleKey = `${ONYXKEYS.COLLECTION.RULE}${targetRuleID}` as const;
     const created = existingRule?.created ?? new Date().toISOString();
 
+    // Shared between the optimistic rule and the request so the rule shown while offline matches what the server saves.
+    const ruleScope = {scope: CONST.RULES.SCOPE.POLICY, scopeID: policyID, priority};
+
     const optimisticRule: Rule = {
-        ...ruleValue,
-        scope: CONST.RULES.SCOPE.POLICY,
-        scopeID: policyID,
-        priority: CONST.RULES.EXPENSE_DEFAULT.PRIORITY,
+        ...ruleBody,
+        ...ruleScope,
         created,
         pendingAction: isEditing ? CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE : CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD,
     };
@@ -283,14 +211,15 @@ function setMerchantRule(
         ],
     };
 
-    const parameters: SetPolicyCodingRuleParams = {
-        policyID,
-        codingRuleID: targetRuleID,
-        codingRuleValue: JSON.stringify(buildLegacyCodingRule(ruleValue, targetRuleID, created)),
+    const parameters: SetRuleParams = {
+        ...ruleScope,
+        ruleID: targetRuleID,
+        // FormData cannot carry an object, so the rule body goes over the wire as a string.
+        value: JSON.stringify(ruleBody),
         shouldUpdateMatchingTransactions,
     };
 
-    API.write(WRITE_COMMANDS.SET_POLICY_CODING_RULE, parameters, onyxData);
+    API.write(WRITE_COMMANDS.SET_RULE, parameters, onyxData);
 }
 
 /**
@@ -363,14 +292,13 @@ function getTransactionsMatchingCodingRule(policyID: string, filters: CodingRule
 }
 
 /**
- * Deletes a merchant rule
- * @param policyID - The ID of the policy the rule belongs to
+ * Deletes a rule of any kind
  * @param ruleID - The ID of the rule to delete
  * @param rule - The rule being deleted, restored on failure
  */
-function deleteMerchantRule(policyID: string, ruleID: string, rule: Rule | undefined) {
-    if (!policyID || !ruleID) {
-        Log.warn('Invalid params for deleteMerchantRule', {policyID, ruleID});
+function deleteRule(ruleID: string, rule: Rule | undefined) {
+    if (!ruleID) {
+        Log.warn('Invalid params for deleteRule', {ruleID});
         return;
     }
 
@@ -388,15 +316,9 @@ function deleteMerchantRule(policyID: string, ruleID: string, rule: Rule | undef
         ],
     };
 
-    // An empty codingRuleValue tells SetPolicyCodingRule to delete rather than upsert the rule.
-    const parameters: SetPolicyCodingRuleParams = {
-        policyID,
-        codingRuleID: ruleID,
-        codingRuleValue: '',
-        shouldUpdateMatchingTransactions: false,
-    };
+    const parameters: DeleteRuleParams = {ruleID};
 
-    API.write(WRITE_COMMANDS.SET_POLICY_CODING_RULE, parameters, onyxData);
+    API.write(WRITE_COMMANDS.DELETE_RULE, parameters, onyxData);
 }
 
 function addPolicyAgentRule(policyID: string, agentRuleID: string, prompt: string) {
@@ -657,9 +579,9 @@ export {
     getAgentRuleSuggestions,
     getRules,
     resetRulesFetchState,
-    setMerchantRule,
+    setRule,
     importMerchantRulesSpreadsheet,
-    deleteMerchantRule,
+    deleteRule,
     getTransactionsMatchingCodingRule,
     addPolicyAgentRule,
     updatePolicyAgentRule,

src/pages/workspace/rules/MerchantRules/MerchantRulePageBase.tsx

@@ -27,13 +27,13 @@ import useRulesPrefetch from '@hooks/useRulesPrefetch';
 import useThemeStyles from '@hooks/useThemeStyles';
 
 import {deletePolicyCategoryTax, movePolicyCategoryTax, openPolicyCategoriesPage, setPolicyCategoryTaxes} from '@libs/actions/Policy/Category';
-import {deleteMerchantRule, setMerchantRule} from '@libs/actions/Policy/Rules';
+import {deleteRule, setRule} 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 {buildMerchantRule, canEditMerchantRule, getMerchantRuleFormValues, getPolicyExpenseDefaultRules} from '@libs/ExpenseDefaultRuleUtils';
 import Navigation from '@libs/Navigation/Navigation';
 import {hasEnabledOptions} from '@libs/OptionsListUtils';
 import Parser from '@libs/Parser';
@@ -431,7 +431,7 @@ function MerchantRulePageBase({
             return;
         }
 
-        setMerchantRule(policyID, form, policy, ruleID, existingRule, shouldUpdateMatchingTransactions);
+        setRule(policyID, buildMerchantRule(form, policy), CONST.RULES.EXPENSE_DEFAULT.PRIORITY, 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.
@@ -482,15 +482,15 @@ function MerchantRulePageBase({
         startWithLoading(() => saveRule());
     };
 
-    const deleteRule = () => {
+    const handleDeleteRule = () => {
         if (!policy) {
             return false;
         }
         setIsClosing(true);
         if (editCategoryTaxRuleFor) {
             deletePolicyCategoryTax(policy, editCategoryTaxRuleFor);
         } else if (ruleID) {
-            deleteMerchantRule(policy.id, ruleID, existingRule);
+            deleteRule(ruleID, existingRule);
         }
         return true;
     };
@@ -502,7 +502,7 @@ function MerchantRulePageBase({
 
     const {deleteIconButtonProps} = useRuleDeleteHeaderProps({
         canDelete: canDeleteRule,
-        onDelete: deleteRule,
+        onDelete: handleDeleteRule,
         sentryLabel: CONST.SENTRY_LABEL.WORKSPACE.RULES.MERCHANT_RULE_DELETE,
         backTo: categorySettingsBackPath,
     });

src/pages/workspace/rules/tabs/useRulesTableBulkActions.ts

@@ -19,7 +19,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 {deleteMerchantRule} from '@libs/actions/Policy/Rules';
+import {deleteRule} from '@libs/actions/Policy/Rules';
 import {getCategoryNameFromTaxRuleKey, isCategoryTaxRuleKey} from '@libs/CategoryTaxRulesUtils';
 import {deleteFlagForReviewRule, getFlagForReviewTableData} from '@libs/FlagForReviewRulesUtils';
 import {getExpenseDefaultsTableData, isMerchantTypeRuleKey} from '@libs/MerchantTypeRulesUtils';
@@ -266,7 +266,7 @@ function useRulesTableBulkActions({policyID, activeTab, selectedRuleKeysByTab, c
                 continue;
             }
 
-            deleteMerchantRule(policyID, ruleID, rules?.[`${ONYXKEYS.COLLECTION.RULE}${ruleID}`]);
+            deleteRule(ruleID, rules?.[`${ONYXKEYS.COLLECTION.RULE}${ruleID}`]);
         }
 
         if (selectedCategoryNames.length > 0) {

src/types/onyx/Rule.ts

@@ -38,3 +38,4 @@ type Rule = OnyxValueWithOfflineFeedback<
 >;
 
 export default Rule;
+export type {RuleBody};