From 8b1f329d57912528b97c8a5dd7e3406dc8a20532 Mon Sep 17 00:00:00 2001 From: krishna2323 Date: Tue, 4 Aug 2026 02:30:25 +0530 Subject: [PATCH 1/3] Fix category rules regressions from the contextual Category rules section Signed-off-by: krishna2323 --- src/libs/CategoryContextualRulesUtils.ts | 25 +++++++++++++++---- .../categories/CategorySettingsPage.tsx | 9 +++++-- .../AddFlagForReviewRulePage.tsx | 14 +++++++++++ .../FlagForReviewRulePageBase.tsx | 4 ++- .../AddRequireFieldsRulePage.tsx | 14 +++++++++++ .../RequireFieldsRulePageBase.tsx | 6 +++-- 6 files changed, 62 insertions(+), 10 deletions(-) diff --git a/src/libs/CategoryContextualRulesUtils.ts b/src/libs/CategoryContextualRulesUtils.ts index 4a2470fe2cf0..499cc0d09514 100644 --- a/src/libs/CategoryContextualRulesUtils.ts +++ b/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, }); } } diff --git a/src/pages/workspace/categories/CategorySettingsPage.tsx b/src/pages/workspace/categories/CategorySettingsPage.tsx index 32dcc7cf5225..46ec252b4533 100644 --- a/src/pages/workspace/categories/CategorySettingsPage.tsx +++ b/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} /> ))} diff --git a/src/pages/workspace/rules/FlagForReviewRules/AddFlagForReviewRulePage.tsx b/src/pages/workspace/rules/FlagForReviewRules/AddFlagForReviewRulePage.tsx index 87c178956424..58b8ba4822cf 100644 --- a/src/pages/workspace/rules/FlagForReviewRules/AddFlagForReviewRulePage.tsx +++ b/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 ( Date: Tue, 4 Aug 2026 02:57:06 +0530 Subject: [PATCH 2/3] Seed the flag rule draft once categories load and fix the contextual rules test Signed-off-by: krishna2323 --- .../FlagForReviewRulePageBase.tsx | 9 ++- .../unit/CategoryContextualRulesUtilsTest.ts | 58 +++++++++++++++++++ 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/src/pages/workspace/rules/FlagForReviewRules/FlagForReviewRulePageBase.tsx b/src/pages/workspace/rules/FlagForReviewRules/FlagForReviewRulePageBase.tsx index 460a6bd76c86..fbd182e83c31 100644 --- a/src/pages/workspace/rules/FlagForReviewRules/FlagForReviewRulePageBase.tsx +++ b/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 recognised 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; diff --git a/tests/unit/CategoryContextualRulesUtilsTest.ts b/tests/unit/CategoryContextualRulesUtilsTest.ts index 97b455ccbaec..ddb67f4ef576 100644 --- a/tests/unit/CategoryContextualRulesUtilsTest.ts +++ b/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([]); + }); }); From c524e8a49651965877d1bb7e3819c311d1ede9a2 Mon Sep 17 00:00:00 2001 From: krishna2323 Date: Tue, 4 Aug 2026 04:17:04 +0530 Subject: [PATCH 3/3] fix spell check. Signed-off-by: krishna2323 --- .../rules/FlagForReviewRules/FlagForReviewRulePageBase.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/workspace/rules/FlagForReviewRules/FlagForReviewRulePageBase.tsx b/src/pages/workspace/rules/FlagForReviewRules/FlagForReviewRulePageBase.tsx index fbd182e83c31..f3971e850e69 100644 --- a/src/pages/workspace/rules/FlagForReviewRules/FlagForReviewRulePageBase.tsx +++ b/src/pages/workspace/rules/FlagForReviewRules/FlagForReviewRulePageBase.tsx @@ -117,7 +117,7 @@ function FlagForReviewRulePageBase({ // 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 recognised once the + // 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;