Fix category rules regressions from the contextual Category rules section

src/libs/CategoryContextualRulesUtils.ts

@@ -26,6 +26,8 @@ type CategoryContextualRule = {
     /** Dynamic route suffix under category settings (keeps Categories underlay on refresh). */
     dynamicRoutePath: DynamicRouteSuffix;
     pendingAction?: PendingAction;
+    /** Optimistically deleted rules stay listed (struck through) but must not be openable. */
+    isDisabled?: boolean;
 };
 
 function getFlagForReviewContextualSummary(
@@ -58,12 +60,15 @@ function getCategoryContextualRules({
     categoryName,
     translate,
     convertToDisplayString,
+    isOffline,
 }: {
     policy: Policy | undefined;
     category: PolicyCategory | undefined;
     categoryName: string;
     translate: LocaleContextProps['translate'];
     convertToDisplayString: CurrencyListActionsContextType['convertToDisplayString'];
+    /** Offline keeps optimistically deleted rules listed so the pending delete is visible. */
+    isOffline: boolean;
 }): CategoryContextualRule[] {
     if (!policy?.id || !category) {
         return [];
@@ -72,25 +77,35 @@ function getCategoryContextualRules({
     const policyCurrency = policy.outputCurrency ?? CONST.CURRENCY.USD;
     const rules: CategoryContextualRule[] = [];
 
+    const flagPendingAction = category.pendingFields?.maxExpenseAmount;
+    const isFlagPendingDelete = flagPendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE;
     const flagSummary = getFlagForReviewContextualSummary(category, translate, convertToDisplayString, policyCurrency);
-    if (flagSummary) {
+    if (flagSummary && (isOffline || !isFlagPendingDelete)) {
         rules.push({
             key: `flag-for-review-${categoryName}`,
             summary: flagSummary,
             dynamicRoutePath: DYNAMIC_ROUTES.WORKSPACE_CATEGORY_RULES_FLAG_FOR_REVIEW_EDIT.path,
-            pendingAction: category.pendingFields?.maxExpenseAmount,
+            pendingAction: flagPendingAction,
+            isDisabled: isFlagPendingDelete,
         });
     }
 
-    if (categoryHasAnyRequireFieldsRule(category)) {
-        const descriptions = getRequireFieldsRuleDescriptionsForCategory(category, translate, convertToDisplayString, policyCurrency);
+    // Mirrors getRequireFieldsTableData: a pending delete keeps the rule listed, and its description has to
+    // include the fields being removed, otherwise the summary comes back empty and the row disappears.
+    const requireFieldsPendingAction = getRequireFieldsPendingActionForCategory(category);
+    const isRequireFieldsPendingDelete = requireFieldsPendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE;
+    const hasRequireFieldsRule = categoryHasAnyRequireFieldsRule(category) || isRequireFieldsPendingDelete;
+
+    if (hasRequireFieldsRule && (isOffline || !isRequireFieldsPendingDelete)) {
+        const descriptions = getRequireFieldsRuleDescriptionsForCategory(category, translate, convertToDisplayString, policyCurrency, isRequireFieldsPendingDelete);
         const summary = formatRequireFieldsRuleDescriptions(descriptions);
         if (summary) {
             rules.push({
                 key: `require-fields-${categoryName}`,
                 summary,
                 dynamicRoutePath: DYNAMIC_ROUTES.WORKSPACE_CATEGORY_RULES_REQUIRE_FIELDS_EDIT.path,
-                pendingAction: getRequireFieldsPendingActionForCategory(category),
+                pendingAction: requireFieldsPendingAction,
+                isDisabled: isRequireFieldsPendingDelete,
             });
         }
     }

src/pages/workspace/categories/CategorySettingsPage.tsx

@@ -15,6 +15,7 @@ import useDynamicBackPath from '@hooks/useDynamicBackPath';
 import useEnvironment from '@hooks/useEnvironment';
 import {useMemoizedLazyExpensifyIcons} from '@hooks/useLazyAsset';
 import useLocalize from '@hooks/useLocalize';
+import useNetwork from '@hooks/useNetwork';
 import useOnboardingTaskInformation from '@hooks/useOnboardingTaskInformation';
 import useOnyx from '@hooks/useOnyx';
 import usePermissions from '@hooks/usePermissions';
@@ -76,6 +77,7 @@ function CategorySettingsPage({route: {params, name}, navigation}: CategorySetti
     const isRulesRevampEnabled = isBetaEnabled(CONST.BETAS.RULES_REVAMP);
     const {environmentURL} = useEnvironment();
     const currentUserPersonalDetails = useCurrentUserPersonalDetails();
+    const {isOffline} = useNetwork();
 
     const policyCategory = policyCategories?.[categoryName] ?? Object.values(policyCategories).find((category) => category.previousCategoryName === categoryName);
     const policyCurrency = policy?.outputCurrency ?? CONST.CURRENCY.USD;
@@ -94,8 +96,9 @@ function CategorySettingsPage({route: {params, name}, navigation}: CategorySetti
             categoryName: policyCategory.name,
             translate,
             convertToDisplayString,
+            isOffline,
         });
-    }, [convertToDisplayString, isRulesRevampEnabled, policy, policyCategory, translate]);
+    }, [convertToDisplayString, isOffline, isRulesRevampEnabled, policy, policyCategory, translate]);
 
     const shouldPreventDisableOrDelete = isDisablingOrDeletingLastEnabledCategory(policy, policyData.categories, [policyCategory]);
     const isQuickSettingsFlow = name === SCREENS.SETTINGS_CATEGORIES.DYNAMIC_SETTINGS_CATEGORY_SETTINGS;
@@ -559,7 +562,9 @@ function CategorySettingsPage({route: {params, name}, navigation}: CategorySetti
                                         numberOfLinesTitle={3}
                                         shouldShowBasicTitle
                                         onPress={() => Navigation.navigate(createDynamicRoute(rule.dynamicRoutePath))}
-                                        shouldShowRightIcon
+                                        shouldShowRightIcon={!rule.isDisabled}
+                                        interactive={!rule.isDisabled}
+                                        disabled={rule.isDisabled}
                                     />
                                 </OfflineWithFeedback>
                             ))}

src/pages/workspace/rules/FlagForReviewRules/AddFlagForReviewRulePage.tsx

@@ -1,6 +1,10 @@
+import useOnyx from '@hooks/useOnyx';
+
+import {hasExplicitFlagAmount} from '@libs/FlagForReviewRulesUtils';
 import type {PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types';
 import type {SettingsNavigatorParamList} from '@libs/Navigation/types';
 
+import ONYXKEYS from '@src/ONYXKEYS';
 import SCREENS from '@src/SCREENS';
 
 import React from 'react';
@@ -15,9 +19,19 @@ function AddFlagForReviewRulePage({route}: AddFlagForReviewRulePageProps) {
     const {policyID, categoryName} = route.params;
     const isCategoryScopedFlow = route.name === SCREENS.WORKSPACE.DYNAMIC_CATEGORY_FLAG_FOR_REVIEW_RULE_NEW;
 
+    const [policyCategories] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${policyID}`);
+    const scopedCategory = categoryName ? policyCategories?.[categoryName] : undefined;
+
+    // Flag for review is one per category, so reaching Create new rule for a category that already has one is
+    // really an edit. Passing categoryName seeds the amount from the category instead of opening it empty, which
+    // would otherwise overwrite the existing rule on save. initialCategoryName stays set so saving still returns
+    // to the category details page rather than the New rule hub we came from.
+    const hasExistingRule = !!scopedCategory && hasExplicitFlagAmount(scopedCategory.maxExpenseAmount);
+
     return (
         <FlagForReviewRulePageBase
             policyID={policyID}
+            categoryName={hasExistingRule ? categoryName : undefined}
             initialCategoryName={categoryName}
             isCategoryLocked={isCategoryScopedFlow ? true : undefined}
             isCategoryScopedFlow={isCategoryScopedFlow}

src/pages/workspace/rules/FlagForReviewRules/FlagForReviewRulePageBase.tsx

@@ -92,6 +92,7 @@ function FlagForReviewRulePageBase({
     const selectedCategoryName = form?.[INPUT_IDS.CATEGORY];
     const categoryDisplayName = selectedCategoryName ? getDecodedCategoryName(selectedCategoryName) : undefined;
 
+    const draftMaxExpenseAmount = form?.[INPUT_IDS.MAX_EXPENSE_AMOUNT];
     const parsedMaxAmount = Number.parseFloat(form?.[INPUT_IDS.MAX_EXPENSE_AMOUNT] ?? '');
     const maxAmountMenuTitle = Number.isFinite(parsedMaxAmount) ? convertToDisplayString(convertToBackendAmount(parsedMaxAmount), policyCurrency) : '';
 
@@ -114,14 +115,18 @@ function FlagForReviewRulePageBase({
             return;
         }
 
-        if (selectedCategoryName === categoryName) {
+        // A draft holding the category *and* an amount is either already seeded or the user's in-progress edit, so
+        // leave it alone. A draft with only the category came from the create flow's initial seed, which is what
+        // happens when this page opens before policyCategories has loaded: the rule is only recognized once the
+        // category arrives, and the amount still has to be seeded then.
+        if (selectedCategoryName === categoryName && draftMaxExpenseAmount !== undefined) {
             initializedDraftForRuleKeyRef.current = categoryName;
             return;
         }
 
         initializedDraftForRuleKeyRef.current = categoryName;
         setDraftFlagForReviewRule(getFlagForReviewFormFromCategory(category, getCurrencyDecimals, policyCurrency));
-    }, [category, categoryName, getCurrencyDecimals, initialCategoryName, isEditing, policyCurrency, selectedCategoryName]);
+    }, [category, categoryName, draftMaxExpenseAmount, getCurrencyDecimals, initialCategoryName, isEditing, policyCurrency, selectedCategoryName]);
     const fetchPolicyData = useCallback(() => {
         if (!policy?.areCategoriesEnabled || policyCategories) {
             return;
@@ -146,7 +151,9 @@ function FlagForReviewRulePageBase({
 
         saveFlagForReviewRule(policyID, policyData.categories, form, isEditing ? categoryName : undefined);
 
-        if (!isEditing && isRulesRevampEnabled) {
+        // initialCategoryName is also set when the create screen is editing a category's existing rule, and in that
+        // case going back one step would land on the New rule hub instead of the category we came from.
+        if ((!isEditing || !!initialCategoryName) && isRulesRevampEnabled) {
             const savedCategoryName = form[INPUT_IDS.CATEGORY] ?? initialCategoryName;
             if (initialCategoryName && savedCategoryName) {
                 Navigation.goBack(categorySettingsBackPath ?? getWorkspaceCategorySettingsRoute(policyID, savedCategoryName));

src/pages/workspace/rules/RequireFieldsRules/AddRequireFieldsRulePage.tsx

@@ -1,6 +1,10 @@
+import useOnyx from '@hooks/useOnyx';
+
 import type {PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types';
 import type {SettingsNavigatorParamList} from '@libs/Navigation/types';
+import {categoryHasAnyRequireFieldsRule} from '@libs/RequireFieldsRulesUtils';
 
+import ONYXKEYS from '@src/ONYXKEYS';
 import SCREENS from '@src/SCREENS';
 
 import React from 'react';
@@ -15,9 +19,19 @@ function AddRequireFieldsRulePage({route}: AddRequireFieldsRulePageProps) {
     const {policyID, categoryName} = route.params;
     const isCategoryScopedFlow = route.name === SCREENS.WORKSPACE.DYNAMIC_CATEGORY_REQUIRE_FIELDS_RULE_NEW;
 
+    const [policyCategories] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${policyID}`);
+    const scopedCategory = categoryName ? policyCategories?.[categoryName] : undefined;
+
+    // Field requirements are one per category, so reaching Create new rule for a category that already has them
+    // is really an edit. Passing categoryName seeds the form from the category instead of opening it empty, and
+    // stops an unchanged save from failing validation. initialCategoryName stays set so saving still returns to
+    // the category details page rather than the New rule hub we came from.
+    const hasExistingRule = !!scopedCategory && categoryHasAnyRequireFieldsRule(scopedCategory);
+
     return (
         <RequireFieldsRulePageBase
             policyID={policyID}
+            categoryName={hasExistingRule ? categoryName : undefined}
             initialCategoryName={categoryName}
             isCategoryLocked={isCategoryScopedFlow ? true : undefined}
             testID="AddRequireFieldsRulePage"

src/pages/workspace/rules/RequireFieldsRules/RequireFieldsRulePageBase.tsx

@@ -350,7 +350,7 @@ function RequireFieldsRulePageBase({policyID, categoryName, initialCategoryName,
 
         if (isEditing && !didChangeCategory && !hasRequireFieldsRuleChanges(selectedCategory ?? category, formToSave, touchedFields, clearedFields)) {
             clearDraftRequireFieldsRule();
-            Navigation.goBack();
+            Navigation.goBack(initialCategoryName ? (categorySettingsBackPath ?? getWorkspaceCategorySettingsRoute(policyID, initialCategoryName)) : undefined);
             return;
         }
 
@@ -364,7 +364,9 @@ function RequireFieldsRulePageBase({policyID, categoryName, initialCategoryName,
 
         clearDraftRequireFieldsRule();
 
-        if (!isEditing && isRulesRevampEnabled) {
+        // initialCategoryName is also set when the create screen is editing a category's existing rule, and in that
+        // case going back one step would land on the New rule hub instead of the category we came from.
+        if ((!isEditing || !!initialCategoryName) && isRulesRevampEnabled) {
             const savedCategoryName = savedCategory ?? initialCategoryName;
             if (initialCategoryName && savedCategoryName) {
                 Navigation.goBack(categorySettingsBackPath ?? getWorkspaceCategorySettingsRoute(policyID, savedCategoryName));

tests/unit/CategoryContextualRulesUtilsTest.ts

@@ -32,6 +32,7 @@ describe('getCategoryContextualRules', () => {
                 categoryName: 'Travel',
                 translate: translateLocal,
                 convertToDisplayString,
+                isOffline: false,
             }),
         ).toEqual([]);
     });
@@ -51,6 +52,7 @@ describe('getCategoryContextualRules', () => {
             categoryName: 'Travel',
             translate: translateLocal,
             convertToDisplayString,
+            isOffline: false,
         });
 
         expect(rules).toHaveLength(2);
@@ -60,4 +62,60 @@ describe('getCategoryContextualRules', () => {
         expect(rules.at(1)?.summary).toContain('Require description');
         expect(rules.at(1)?.dynamicRoutePath).toBe(DYNAMIC_ROUTES.WORKSPACE_CATEGORY_RULES_REQUIRE_FIELDS_EDIT.path);
     });
+
+    it('keeps optimistically deleted rules listed and disabled while offline', () => {
+        const category: PolicyCategory = {
+            name: 'Travel',
+            enabled: true,
+            maxExpenseAmount: 20000,
+            expenseLimitType: CONST.POLICY.EXPENSE_LIMIT_TYPES.EXPENSE,
+            areCommentsRequired: true,
+            pendingFields: {
+                maxExpenseAmount: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE,
+                areCommentsRequired: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE,
+            },
+        };
+
+        const rules = getCategoryContextualRules({
+            policy,
+            category,
+            categoryName: 'Travel',
+            translate: translateLocal,
+            convertToDisplayString,
+            isOffline: true,
+        });
+
+        expect(rules).toHaveLength(2);
+        expect(rules.at(0)?.pendingAction).toBe(CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE);
+        expect(rules.at(0)?.isDisabled).toBe(true);
+        // The require-fields summary is built from fields that are pending delete, so it has to opt into them.
+        expect(rules.at(1)?.summary).toContain('Require description');
+        expect(rules.at(1)?.pendingAction).toBe(CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE);
+        expect(rules.at(1)?.isDisabled).toBe(true);
+    });
+
+    it('drops optimistically deleted rules once back online', () => {
+        const category: PolicyCategory = {
+            name: 'Travel',
+            enabled: true,
+            maxExpenseAmount: 20000,
+            expenseLimitType: CONST.POLICY.EXPENSE_LIMIT_TYPES.EXPENSE,
+            areCommentsRequired: true,
+            pendingFields: {
+                maxExpenseAmount: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE,
+                areCommentsRequired: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE,
+            },
+        };
+
+        expect(
+            getCategoryContextualRules({
+                policy,
+                category,
+                categoryName: 'Travel',
+                translate: translateLocal,
+                convertToDisplayString,
+                isOffline: false,
+            }),
+        ).toEqual([]);
+    });
 });