Skip to content

Fix Rules revamp regressions on Collect workspaces #98236

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Krishna2323 wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Fix Rules revamp regressions on Collect workspaces #98236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
49ae910
Fix Collect rules gating, accounting guard, tab restore and landscape…
Krishna2323 Aug 11, 2026
48e65ae
Gate Collect category rules to upgrade and center the empty state
Krishna2323 Aug 11, 2026
93829fd
Clear the field requirements draft on unmount
Krishna2323 Aug 11, 2026
84d9256
update comments.
Krishna2323 Aug 11, 2026
586c403
Split tab doc comments into sentences
Krishna2323 Aug 11, 2026
6b7e863
Route Collect tag approver through the upgrade flow
Krishna2323 Aug 11, 2026
20ab5dd
update comment and remove dead ref code.
Krishna2323 Aug 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/ROUTES.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3365,11 +3365,12 @@ const ROUTES = {
},
WORKSPACE_RULES: {
route: 'workspaces/:policyID/rules',
Comment thread
Krishna2323 marked this conversation as resolved.
getRoute: (policyID: string | undefined) => {
/** @param tab preselects a Rules tab. The page otherwise restores the last one used. */
getRoute: (policyID: string | undefined, tab?: string) => {
if (!policyID) {
Log.warn('Invalid policyID is used to build the WORKSPACE_RULES route');
}
return `workspaces/${policyID}/rules` as const;
return `workspaces/${policyID}/rules${tab ? `?tab=${tab}` : ''}` as const;
},
},
WORKSPACE_DISTANCE_RATES: {
Expand Down
42 changes: 33 additions & 9 deletions src/components/Rule/RuleCategoriesDisabledEmptyState.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,21 @@
import Button from '@components/ButtonComposed';
import FixedFooter from '@components/FixedFooter';
import {ModalActions} from '@components/Modal/Global/ModalContext';
import ScrollView from '@components/ScrollView';
import WorkspaceEmptyStateSection from '@components/WorkspaceEmptyStateSection';

import useConfirmModal from '@hooks/useConfirmModal';
import {useMemoizedLazyIllustrations} from '@hooks/useLazyAsset';
import useLocalize from '@hooks/useLocalize';
import usePolicyData from '@hooks/usePolicyData';
import useThemeStyles from '@hooks/useThemeStyles';

import {enablePolicyCategories, openPolicyCategoriesPage} from '@libs/actions/Policy/Category';
import Navigation from '@libs/Navigation/Navigation';
import {hasAccountingConnections} from '@libs/PolicyUtils';

import CONST from '@src/CONST';
import ROUTES from '@src/ROUTES';

import React from 'react';
import {View} from 'react-native';
Expand All @@ -24,8 +30,25 @@ function RuleCategoriesDisabledEmptyState({policyID}: RuleCategoriesDisabledEmpt
const {translate} = useLocalize();
const illustrations = useMemoizedLazyIllustrations(['FolderOpen']);
const policyData = usePolicyData(policyID);
const {showConfirmModal} = useConfirmModal();
const isConnectedToAccounting = hasAccountingConnections(policyData.policy);

const enableCategories = async () => {
// Accounting owns Categories while a connection is active, same as the Categories toggle on More features.
if (isConnectedToAccounting) {
const {action} = await showConfirmModal({
title: translate('workspace.moreFeatures.connectionsWarningModal.featureEnabledTitle'),
prompt: translate('workspace.moreFeatures.connectionsWarningModal.featureEnabledText'),
confirmText: translate('workspace.moreFeatures.connectionsWarningModal.manageSettings'),
cancelText: translate('common.cancel'),
});
if (action !== ModalActions.CONFIRM) {
return;
}
Navigation.navigate(ROUTES.POLICY_ACCOUNTING.getRoute(policyID));
return;
}

const enableCategories = () => {
enablePolicyCategories(policyData, true, false);

// The categories collection is empty while the feature is disabled, and enabling it only merges the
Expand All @@ -35,14 +58,15 @@ function RuleCategoriesDisabledEmptyState({policyID}: RuleCategoriesDisabledEmpt

return (
<View style={[styles.flex1]}>
<WorkspaceEmptyStateSection
shouldStyleAsCard={false}
icon={illustrations.FolderOpen}
title={translate('workspace.rules.categoriesDisabledEmptyState.title')}
subtitle={translate('workspace.rules.categoriesDisabledEmptyState.subtitle')}
containerStyle={[styles.flex1, styles.justifyContentCenter]}
/>
<FixedFooter style={[styles.mtAuto, styles.pt5]}>
<ScrollView contentContainerStyle={[styles.flexGrow1, styles.justifyContentCenter, styles.alignItemsCenter]}>
<WorkspaceEmptyStateSection
shouldStyleAsCard={false}
icon={illustrations.FolderOpen}
title={translate('workspace.rules.categoriesDisabledEmptyState.title')}
subtitle={translate('workspace.rules.categoriesDisabledEmptyState.subtitle')}
/>
</ScrollView>
<FixedFooter style={[styles.pt5]}>

@JS00001 JS00001 Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no more mtAuto?

@Krishna2323 Krishna2323 Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed anymore. The content is now inside a ScrollView whose contentContainerStyle has flexGrow1

<Button
variant={CONST.BUTTON_VARIANT.SUCCESS}
size={CONST.BUTTON_SIZE.LARGE}
Expand Down
3 changes: 3 additions & 0 deletions src/libs/Navigation/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3027,6 +3027,9 @@ type WorkspaceSplitNavigatorParamList = {
};
[SCREENS.WORKSPACE.RULES]: {
policyID: string;

/** Preselects a Rules tab. The page otherwise restores the last one used. */
tab?: string;
Comment thread
Krishna2323 marked this conversation as resolved.
};
[SCREENS.WORKSPACE.TIME_TRACKING]: {
policyID: string;
Expand Down
23 changes: 16 additions & 7 deletions src/pages/workspace/categories/CategorySettingsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ import Navigation from '@libs/Navigation/Navigation';
import type {PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types';
import {isDisablingOrDeletingLastEnabledCategory} from '@libs/OptionsListUtils';
import {getPersonalDetailByEmail} from '@libs/PersonalDetailsUtils';
import {arePolicyRulesEnabled, getWorkflowApprovalsUnavailable, hasTags, isAttendeeTrackingEnabled, isControlPolicy} from '@libs/PolicyUtils';
import {arePolicyRulesEnabled, getWorkflowApprovalsUnavailable, hasTags, isAttendeeTrackingEnabled, isControlPolicy, tryNavigateToControlPolicyUpgrade} from '@libs/PolicyUtils';

import type {SettingsNavigatorParamList} from '@navigation/types';

Expand Down Expand Up @@ -83,7 +83,7 @@ function CategorySettingsPage({route: {params, name}, navigation}: CategorySetti
const policyCurrency = policy?.outputCurrency ?? CONST.CURRENCY.USD;
const policyCategoryExpenseLimitType = policyCategory?.expenseLimitType ?? CONST.POLICY.EXPENSE_LIMIT_TYPES.EXPENSE;
const decodedCategoryName = getDecodedCategoryName(policyCategory?.name ?? '');
const categoryRulesEnabled = arePolicyRulesEnabled(policy, policyCategories);
const categoryRulesEnabled = arePolicyRulesEnabled(policy, policyCategories, isRulesRevampEnabled);

const contextualRules = useMemo(() => {
if (!isRulesRevampEnabled || !policyCategory) {
Expand Down Expand Up @@ -273,6 +273,15 @@ function CategorySettingsPage({route: {params, name}, navigation}: CategorySetti
const workflowApprovalsUnavailable = getWorkflowApprovalsUnavailable(policy);
const approverDisabled = !policy?.areWorkflowsEnabled || workflowApprovalsUnavailable;

/** Collect sees this section but every destination is Control-only, so upgrade instead of hitting Not Found. */
const navigateToCategoryRule = (dynamicRouteSuffix: string) => {
const ruleRoute = createDynamicRoute(dynamicRouteSuffix);
if (tryNavigateToControlPolicyUpgrade(policy, CONST.UPGRADE_FEATURE_INTRO_MAPPING.rules.alias, ruleRoute)) {
return;
}
Navigation.navigate(ruleRoute);
};

if (!policyCategory) {
return <NotFoundPage />;
}
Expand Down Expand Up @@ -391,7 +400,7 @@ function CategorySettingsPage({route: {params, name}, navigation}: CategorySetti
title={policyCategory?.commentHint}
description={translate('workspace.rules.categoryRules.descriptionHint')}
onPress={() => {
Navigation.navigate(createDynamicRoute(DYNAMIC_ROUTES.WORKSPACE_CATEGORY_DESCRIPTION_HINT.path));
navigateToCategoryRule(DYNAMIC_ROUTES.WORKSPACE_CATEGORY_DESCRIPTION_HINT.path);
}}
interactive={canWriteCategories}
shouldShowRightIcon={canWriteCategories}
Expand All @@ -402,7 +411,7 @@ function CategorySettingsPage({route: {params, name}, navigation}: CategorySetti
title={approverText}
description={translate('workspace.rules.categoryRules.approver')}
onPress={() => {
Navigation.navigate(createDynamicRoute(DYNAMIC_ROUTES.WORKSPACE_CATEGORY_APPROVER.path));
navigateToCategoryRule(DYNAMIC_ROUTES.WORKSPACE_CATEGORY_APPROVER.path);
}}
interactive={canWriteCategories}
shouldShowRightIcon={canWriteCategories}
Expand All @@ -419,7 +428,7 @@ function CategorySettingsPage({route: {params, name}, navigation}: CategorySetti
title={defaultTaxRateText}
description={translate('workspace.rules.categoryRules.defaultTaxRate')}
onPress={() => {
Navigation.navigate(createDynamicRoute(DYNAMIC_ROUTES.WORKSPACE_CATEGORY_DEFAULT_TAX_RATE.path));
navigateToCategoryRule(DYNAMIC_ROUTES.WORKSPACE_CATEGORY_DEFAULT_TAX_RATE.path);
}}
interactive={canWriteCategories}
shouldShowRightIcon={canWriteCategories}
Expand Down Expand Up @@ -561,7 +570,7 @@ function CategorySettingsPage({route: {params, name}, navigation}: CategorySetti
title={rule.summary}
numberOfLinesTitle={3}
shouldShowBasicTitle
onPress={() => Navigation.navigate(createDynamicRoute(rule.dynamicRoutePath))}
onPress={() => navigateToCategoryRule(rule.dynamicRoutePath)}
shouldShowRightIcon={!rule.isDisabled}
interactive={!rule.isDisabled}
disabled={rule.isDisabled}
Expand All @@ -572,7 +581,7 @@ function CategorySettingsPage({route: {params, name}, navigation}: CategorySetti
<MenuItem
icon={expensifyIcons.Plus}
title={translate('workspace.rules.categoryRules.createNewRule')}
onPress={() => Navigation.navigate(createDynamicRoute(DYNAMIC_ROUTES.WORKSPACE_CATEGORY_RULES_NEW.path))}
onPress={() => navigateToCategoryRule(DYNAMIC_ROUTES.WORKSPACE_CATEGORY_RULES_NEW.path)}
/>
)}
</>
Expand Down
15 changes: 13 additions & 2 deletions src/pages/workspace/rules/PolicyRulesPageRevamp.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ const agentsRulesBannerDismissedSelector = (value: OnyxEntry<DismissedProductTra

function PolicyRulesPageRevamp({route}: PolicyRulesPageRevampProps) {
const {translate} = useLocalize();
const {policyID} = route.params;
const {policyID, tab: requestedTab} = route.params;
const policy = usePolicy(policyID);
useWorkspaceDocumentTitle(policy?.name, 'workspace.common.rules');
const styles = useThemeStyles();
Expand Down Expand Up @@ -114,6 +114,16 @@ function PolicyRulesPageRevamp({route}: PolicyRulesPageRevampProps) {
Tab.setSelectedTab(CONST.TAB.RULES_TAB_TYPE, RULES_TAB.GENERAL);
}, [activeTab, policy]);

useEffect(() => {
// The tab param is only an entry hint (deep link, post-upgrade bounce-back). The selected tab itself lives in
// Onyx so it survives leaving the page, which is why tab presses update Onyx rather than the URL.
if (!requestedTab || !isRulesTab(requestedTab)) {
return;
}

Tab.setSelectedTab(CONST.TAB.RULES_TAB_TYPE, requestedTab);
}, [requestedTab]);

const clearAllTableSelection = useCallback(() => {
setSelectedRuleKeysByTab((prev) => (Object.keys(prev).length > 0 ? {} : prev));
turnOffMobileSelectionMode();
Expand Down Expand Up @@ -260,7 +270,8 @@ function PolicyRulesPageRevamp({route}: PolicyRulesPageRevampProps) {
return;
}

if (key !== RULES_TAB.GENERAL && tryNavigateToControlPolicyUpgrade(policy, rulesUpgradeAlias, rulesUpgradeBackTo)) {
// Come back to the tab the user asked for, so upgrading lands them where they were headed.
if (key !== RULES_TAB.GENERAL && tryNavigateToControlPolicyUpgrade(policy, rulesUpgradeAlias, ROUTES.WORKSPACE_RULES.getRoute(policyID, key))) {
return;
}

Expand Down
3 changes: 3 additions & 0 deletions src/pages/workspace/rules/RequireFieldsRules/RequireFieldsRulePageBase.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,9 @@ function RequireFieldsRulePageBase({policyID, categoryName, initialCategoryName,
}
}

// Otherwise cleared only on save, so backing out left the draft for the next rule to inherit.
useEffect(() => () => clearDraftRequireFieldsRule(), []);

useEffect(() => {
if (!isEditing) {
if (initializedDraftForRuleKeyRef.current !== ROUTES.NEW) {
Expand Down
13 changes: 11 additions & 2 deletions src/pages/workspace/tags/DynamicTagSettingsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import useDynamicBackPath from '@hooks/useDynamicBackPath';
import useEnvironment from '@hooks/useEnvironment';
import {useMemoizedLazyExpensifyIcons} from '@hooks/useLazyAsset';
import useLocalize from '@hooks/useLocalize';
import usePermissions from '@hooks/usePermissions';
import usePolicyData from '@hooks/usePolicyData';
import usePolicyFeatureWriteAccess from '@hooks/usePolicyFeatureWriteAccess';
import useThemeStyles from '@hooks/useThemeStyles';
Expand All @@ -33,6 +34,7 @@ import {
hasDependentTags as hasDependentTagsPolicyUtils,
isControlPolicy,
isMultiLevelTags as isMultiLevelTagsPolicyUtils,
tryNavigateToControlPolicyUpgrade,
} from '@libs/PolicyUtils';

import type {SettingsNavigatorParamList} from '@navigation/types';
Expand Down Expand Up @@ -62,6 +64,8 @@ function DynamicTagSettingsPage({route, navigation}: DynamicTagSettingsPageProps
const policyData = usePolicyData(policyID);
const {policy, tags: policyTags} = policyData;
const {canWrite: canWriteTags, withReadOnlyFallback} = usePolicyFeatureWriteAccess(policy, CONST.POLICY.POLICY_FEATURE.TAGS);
const {isBetaEnabled} = usePermissions();
const isRulesRevampEnabled = isBetaEnabled(CONST.BETAS.RULES_REVAMP);
const policyTag = getTagListByOrderWeight(policyTags, orderWeight);
const {environmentURL} = useEnvironment();
const hasAccountingConnections = hasAccountingConnectionsPolicyUtils(policy);
Expand Down Expand Up @@ -131,7 +135,12 @@ function DynamicTagSettingsPage({route, navigation}: DynamicTagSettingsPageProps
};

const navigateToEditTagApprover = () => {
Navigation.navigate(isQuickSettingsFlow ? createDynamicRoute(DYNAMIC_ROUTES.SETTINGS_TAG_APPROVER.path) : createDynamicRoute(DYNAMIC_ROUTES.WORKSPACE_TAG_APPROVER.path));
const approverRoute = isQuickSettingsFlow ? createDynamicRoute(DYNAMIC_ROUTES.SETTINGS_TAG_APPROVER.path) : createDynamicRoute(DYNAMIC_ROUTES.WORKSPACE_TAG_APPROVER.path);
// Collect sees this section but the approver page is Control-only, so upgrade instead of hitting Not Found.
if (tryNavigateToControlPolicyUpgrade(policy, CONST.UPGRADE_FEATURE_INTRO_MAPPING.rules.alias, approverRoute)) {
return;
}
Navigation.navigate(approverRoute);
};

const isThereAnyAccountingConnection = Object.keys(policy?.connections ?? {}).length !== 0;
Expand Down Expand Up @@ -209,7 +218,7 @@ function DynamicTagSettingsPage({route, navigation}: DynamicTagSettingsPageProps
</OfflineWithFeedback>
)}

{arePolicyRulesEnabled(policy, policyData.categories) && !isMultiLevelTags && (
{arePolicyRulesEnabled(policy, policyData.categories, isRulesRevampEnabled) && !isMultiLevelTags && (
Comment thread
Krishna2323 marked this conversation as resolved.
<>
<View style={[styles.mh5, styles.mv3, styles.pt3, styles.borderTop]}>
<Text style={[styles.textNormal, styles.textStrong, styles.mv3]}>{translate('workspace.tags.tagRules')}</Text>
Expand Down
8 changes: 6 additions & 2 deletions src/pages/workspace/upgrade/WorkspaceUpgradePage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,8 @@ function WorkspaceUpgradePage({route}: WorkspaceUpgradePageProps) {
case CONST.UPGRADE_FEATURE_INTRO_MAPPING.companyCards.id:
return route.params.backTo ? Navigation.goBack(route.params.backTo) : Navigation.goBack();
case CONST.UPGRADE_FEATURE_INTRO_MAPPING.rules.id:
return Navigation.goBack(route.params.backTo ?? ROUTES.WORKSPACE_MORE_FEATURES.getRoute(policyID));
// The Rules backTo can carry a tab, and comparing params would replace the mounted route instead of popping.
return Navigation.goBack(route.params.backTo ?? ROUTES.WORKSPACE_MORE_FEATURES.getRoute(policyID), {compareParams: false});
case CONST.UPGRADE_FEATURE_INTRO_MAPPING.perDiem.id:
case CONST.UPGRADE_FEATURE_INTRO_MAPPING.invoicing.id:
case CONST.UPGRADE_FEATURE_INTRO_MAPPING.companyCardSubmit.id:
Expand Down Expand Up @@ -249,7 +250,10 @@ function WorkspaceUpgradePage({route}: WorkspaceUpgradePageProps) {
}
break;
case CONST.UPGRADE_FEATURE_INTRO_MAPPING.rules.id:
enablePolicyRules(policy, true, false, policyDataRef.current);
// Re-enabling would re-run the sidebar's "just enabled" highlight on a row that already shows.
if (!policy?.areRulesEnabled) {
enablePolicyRules(policy, true, false, policyDataRef.current);
}
break;
case CONST.UPGRADE_FEATURE_INTRO_MAPPING.publicReceiptVisibility.id:
setPolicyReceiptVisibilityPublic(policyID, true, policy?.isReceiptVisibilityPublic);
Expand Down
Loading