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 ( { 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)); diff --git a/src/pages/workspace/rules/RequireFieldsRules/AddRequireFieldsRulePage.tsx b/src/pages/workspace/rules/RequireFieldsRules/AddRequireFieldsRulePage.tsx index b9d686221d22..90e3ecbf2996 100644 --- a/src/pages/workspace/rules/RequireFieldsRules/AddRequireFieldsRulePage.tsx +++ b/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 ( { 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([]); + }); });