From c45c831c3de5fbec6142ba0e66dc66735651ea1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Musia=C5=82?= Date: Thu, 17 Sep 2026 16:32:31 +0200 Subject: [PATCH 1/3] Align workspace selector logic --- src/hooks/useCreateReport.tsx | 11 ++++- .../menuItems/CreateReportMenuItem.tsx | 15 ++++-- tests/unit/CreateReportMenuItemTest.tsx | 35 +++++++++----- tests/unit/useCreateReportTest.tsx | 48 +++++++++++++++++++ 4 files changed, 92 insertions(+), 17 deletions(-) diff --git a/src/hooks/useCreateReport.tsx b/src/hooks/useCreateReport.tsx index a67a94f32316..4503d0d54dfa 100644 --- a/src/hooks/useCreateReport.tsx +++ b/src/hooks/useCreateReport.tsx @@ -18,6 +18,7 @@ import {useCallback} from 'react'; import useCreateEmptyReportConfirmation from './useCreateEmptyReportConfirmation'; import useCurrentUserPersonalDetails from './useCurrentUserPersonalDetails'; import useOnyx from './useOnyx'; +import usePreferredPolicy from './usePreferredPolicy'; import useShouldShowEmptyReportConfirmation from './useShouldShowEmptyReportConfirmation'; type UseCreateReportParams = { @@ -46,7 +47,8 @@ type UseCreateReportResult = { * * Decision flow: * 1. Navigate to upgrade path if user has no valid group policies at all - * 2. Navigate to workspace selector if default is personal AND there are at least 2 non-personal workspaces, or if the chosen default is billing-restricted and alternatives exist + * 2. Navigate to workspace selector if default is personal AND there are at least 2 non-personal workspaces, or if the chosen default is billing-restricted and alternatives exist. + * Skipped when the domain security group locks the user to a preferred workspace and a default resolved. * 3. Show empty report confirmation or create directly if workspace is valid * 4. Navigate to restricted action if billing restricts the workspace */ @@ -64,6 +66,7 @@ export default function useCreateReport({ const [userBillingGracePeriodEnds] = useOnyx(ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_USER_BILLING_GRACE_PERIOD_END); const [amountOwed] = useOnyx(ONYXKEYS.NVP_PRIVATE_AMOUNT_OWED); const {accountID} = useCurrentUserPersonalDetails(); + const {isRestrictedToPreferredPolicy} = usePreferredPolicy(); // Gate visibility and routing on policy hydration. Without this, during Onyx cold-start // groupPoliciesWithChatEnabled.length === 0 would be true even for users who actually have @@ -114,12 +117,15 @@ export default function useCreateReport({ // at least 2 non-personal workspaces to choose between. Also fall back to the selector if // the default is billing-restricted and alternatives exist, so the user isn't dead-ended // on the restricted-action page. + // When the domain security group locks the user to a preferred workspace, never offer + // alternatives: create on the default, or land on the restricted-action page if it's billing-restricted. const isDefaultPersonal = !activePolicy || activePolicy.type === CONST.POLICY.TYPE.PERSONAL || !isGroupPolicy(activePolicy); const hasMultipleNonPersonalWorkspaces = groupPoliciesWithChatEnabled.length > 1; const isDefaultBillingRestricted = !!workspaceIDForReportCreation && shouldRestrictUserBillableActions(defaultChatEnabledPolicy, ownerBillingGracePeriodEnd, userBillingGracePeriodEnds, amountOwed, accountID); + const shouldOfferAlternatives = !isRestrictedToPreferredPolicy && hasMultipleNonPersonalWorkspaces && (isDefaultPersonal || isDefaultBillingRestricted); - if (!workspaceIDForReportCreation || (isDefaultPersonal && hasMultipleNonPersonalWorkspaces) || (isDefaultBillingRestricted && hasMultipleNonPersonalWorkspaces)) { + if (!workspaceIDForReportCreation || shouldOfferAlternatives) { if (onNavigateToWorkspaceSelection) { onNavigateToWorkspaceSelection(); } else { @@ -150,6 +156,7 @@ export default function useCreateReport({ userBillingGracePeriodEnds, amountOwed, accountID, + isRestrictedToPreferredPolicy, groupPoliciesWithChatEnabled.length, onNavigateToWorkspaceSelection, shouldShowEmptyReportConfirmation, diff --git a/src/pages/inbox/sidebar/FABPopoverContent/menuItems/CreateReportMenuItem.tsx b/src/pages/inbox/sidebar/FABPopoverContent/menuItems/CreateReportMenuItem.tsx index 2b0b92f37873..1a06e8296b48 100644 --- a/src/pages/inbox/sidebar/FABPopoverContent/menuItems/CreateReportMenuItem.tsx +++ b/src/pages/inbox/sidebar/FABPopoverContent/menuItems/CreateReportMenuItem.tsx @@ -31,9 +31,16 @@ import React from 'react'; const ITEM_ID = CONST.FAB_MENU_ITEM_IDS.CREATE_REPORT; -// Returns up to 2 matching policies -const chatEnabledPaidGroupPoliciesSelector = (policies: OnyxCollection, currentUserLogin: string | undefined) => - getGroupPoliciesWhereReportCanBeCreated(policies, currentUserLogin).slice(0, 2); +// Returns up to 2 matching policies. The active policy is moved to the front so it survives the slice, +// otherwise getDefaultChatEnabledPolicy can't find it and the workspace selector opens instead of creating directly. +const chatEnabledPaidGroupPoliciesSelector = (policies: OnyxCollection, currentUserLogin: string | undefined, activePolicyID: string | undefined) => { + const eligiblePolicies = getGroupPoliciesWhereReportCanBeCreated(policies, currentUserLogin); + const activePolicyIndex = eligiblePolicies.findIndex((policy) => policy.id === activePolicyID); + if (activePolicyIndex < 1) { + return eligiblePolicies.slice(0, 2); + } + return [eligiblePolicies.at(activePolicyIndex), ...eligiblePolicies.filter((_, index) => index !== activePolicyIndex)].slice(0, 2); +}; function CreateReportMenuItem() { const [activePolicyID] = useOnyx(ONYXKEYS.NVP_ACTIVE_POLICY_ID); @@ -50,7 +57,7 @@ function CreateReportMenuItem() { const isASAPSubmitBetaEnabled = isBetaEnabled(CONST.BETAS.ASAP_SUBMIT); const hasViolations = hasViolationsReportUtils(undefined, transactionViolations, session?.accountID ?? CONST.DEFAULT_NUMBER_ID, session?.email ?? ''); const [groupPoliciesWithChatEnabled = CONST.EMPTY_ARRAY] = useOnyx(ONYXKEYS.COLLECTION.POLICY, { - selector: (policies: Parameters[0]) => chatEnabledPaidGroupPoliciesSelector(policies, session?.email), + selector: (policies: Parameters[0]) => chatEnabledPaidGroupPoliciesSelector(policies, session?.email, activePolicyID), }); const [isTrackIntentUser] = useOnyx(ONYXKEYS.NVP_INTRO_SELECTED, {selector: isTrackIntentUserSelector}); const [rules] = useOnyx(ONYXKEYS.COLLECTION.RULE); diff --git a/tests/unit/CreateReportMenuItemTest.tsx b/tests/unit/CreateReportMenuItemTest.tsx index d71c115b3581..9cd7b8ee2aa5 100644 --- a/tests/unit/CreateReportMenuItemTest.tsx +++ b/tests/unit/CreateReportMenuItemTest.tsx @@ -101,21 +101,21 @@ function makePolicy(id: string, type: Policy['type']): Policy { } as Policy; } -function setupUseOnyx() { +function setupUseOnyx(activePolicyID = 'personal-1') { const personalPolicy = makePolicy('personal-1', CONST.POLICY.TYPE.PERSONAL); const groupPolicy = makePolicy('team-1', CONST.POLICY.TYPE.TEAM); const submitPolicy = makePolicy('submit-1', CONST.POLICY.TYPE.SUBMIT); + const corporatePolicy = makePolicy('corporate-1', CONST.POLICY.TYPE.CORPORATE); + const policies = { + [`${ONYXKEYS.COLLECTION.POLICY}${personalPolicy.id}`]: personalPolicy, + [`${ONYXKEYS.COLLECTION.POLICY}${groupPolicy.id}`]: groupPolicy, + [`${ONYXKEYS.COLLECTION.POLICY}${submitPolicy.id}`]: submitPolicy, + [`${ONYXKEYS.COLLECTION.POLICY}${corporatePolicy.id}`]: corporatePolicy, + }; const values = new Map([ - [ONYXKEYS.NVP_ACTIVE_POLICY_ID, personalPolicy.id], - [`${ONYXKEYS.COLLECTION.POLICY}${personalPolicy.id}`, personalPolicy], - [ - ONYXKEYS.COLLECTION.POLICY, - { - [`${ONYXKEYS.COLLECTION.POLICY}${personalPolicy.id}`]: personalPolicy, - [`${ONYXKEYS.COLLECTION.POLICY}${groupPolicy.id}`]: groupPolicy, - [`${ONYXKEYS.COLLECTION.POLICY}${submitPolicy.id}`]: submitPolicy, - }, - ], + [ONYXKEYS.NVP_ACTIVE_POLICY_ID, activePolicyID], + [`${ONYXKEYS.COLLECTION.POLICY}${activePolicyID}`, policies[`${ONYXKEYS.COLLECTION.POLICY}${activePolicyID}`]], + [ONYXKEYS.COLLECTION.POLICY, policies], [ONYXKEYS.SESSION, {accountID: 1, email: 'user@test.com'}], [ONYXKEYS.BETAS, []], [ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS, {}], @@ -144,4 +144,17 @@ describe('CreateReportMenuItem', () => { expect.objectContaining({id: 'submit-1', type: CONST.POLICY.TYPE.SUBMIT}), ]); }); + + it('keeps the active workspace in the sliced list when it is not among the first two eligible policies', () => { + setupUseOnyx('corporate-1'); + + render(); + + const params = mockUseCreateReport.mock.calls.at(0)?.at(0); + expect(params?.groupPoliciesWithChatEnabled).toHaveLength(2); + expect(params?.groupPoliciesWithChatEnabled).toEqual([ + expect.objectContaining({id: 'corporate-1', type: CONST.POLICY.TYPE.CORPORATE}), + expect.objectContaining({id: 'team-1', type: CONST.POLICY.TYPE.TEAM}), + ]); + }); }); diff --git a/tests/unit/useCreateReportTest.tsx b/tests/unit/useCreateReportTest.tsx index 7aed632c96da..43c782d1bb3e 100644 --- a/tests/unit/useCreateReportTest.tsx +++ b/tests/unit/useCreateReportTest.tsx @@ -75,6 +75,13 @@ jest.mock('@libs/SubscriptionUtils', () => ({ shouldRestrictUserBillableActions: (...args: Parameters) => mockShouldRestrictUserBillableActions(...args), })); +const mockIsRestrictedToPreferredPolicy = jest.fn(() => false); +jest.mock('@hooks/usePreferredPolicy', () => () => ({ + isRestrictedToPreferredPolicy: mockIsRestrictedToPreferredPolicy(), + preferredPolicyID: undefined, + isRestrictedPolicyCreation: false, +})); + // ── Helpers ──────────────────────────────────────────────────────────────────── const POLICY_ID = 'policy-123'; @@ -119,10 +126,51 @@ describe('useCreateReport', () => { jest.clearAllMocks(); reportIDCounter.value = 100; mockShouldRestrictUserBillableActions.mockReturnValue(false); + mockIsRestrictedToPreferredPolicy.mockReturnValue(false); mockUseShouldShowEmptyReportConfirmation.mockReturnValue(false); setupUseCreateReportOnyx(); }); + describe('domain preferred workspace restriction', () => { + const personalPolicy: OnyxEntry = {...makePaidPolicy('personal-1'), type: CONST.POLICY.TYPE.PERSONAL}; + + it.each([ + ['creates directly on an unrestricted default with multiple workspaces', makePaidPolicy('p1'), false, 'create'], + ['shows the billing restriction page instead of the selector when the default is billing-restricted with multiple workspaces', makePaidPolicy('p1'), true, 'restricted'], + ['still shows the selector when no default workspace could be resolved', personalPolicy, false, 'selector'], + ])('%s', (_description, activePolicy, isBillingRestricted, expected) => { + setupUseCreateReportOnyx({activePolicy}); + mockIsRestrictedToPreferredPolicy.mockReturnValue(true); + mockShouldRestrictUserBillableActions.mockReturnValue(isBillingRestricted); + const onCreateReport = jest.fn(); + const policies = [makePaidPolicy('p1'), makePaidPolicy('p2'), makePaidPolicy('p3')]; + + const {result} = renderHook(() => + useCreateReport({ + onCreateReport, + groupPoliciesWithChatEnabled: policies, + }), + ); + + act(() => { + result.current.createReport(); + }); + + const selectorRoute = DYNAMIC_ROUTES.NEW_REPORT_WORKSPACE_SELECTION.getRoute(); + if (expected === 'create') { + expect(onCreateReport).toHaveBeenCalledWith(false); + expect(Navigation.navigate).not.toHaveBeenCalled(); + } else if (expected === 'restricted') { + expect(Navigation.navigate).toHaveBeenCalledWith(ROUTES.RESTRICTED_ACTION.getRoute('p1')); + expect(Navigation.navigate).not.toHaveBeenCalledWith(selectorRoute); + expect(onCreateReport).not.toHaveBeenCalled(); + } else { + expect(Navigation.navigate).toHaveBeenCalledWith(selectorRoute); + expect(onCreateReport).not.toHaveBeenCalled(); + } + }); + }); + describe('upgrade path (no policies)', () => { it('navigates to upgrade path when user has no group policies', () => { const onCreateReport = jest.fn(); From 5c6fe1b7eafe119e4beb8d470a801891a0cc0944 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Musia=C5=82?= Date: Fri, 18 Sep 2026 12:32:40 +0200 Subject: [PATCH 2/3] self review fixes --- .../useCreateNavigationSuggestions.ts | 12 ++-- src/hooks/useCreateReport.tsx | 35 ++++++----- src/libs/PolicyUtils.ts | 21 ++++--- src/pages/Search/EmptySearchView.tsx | 10 ++-- .../menuItems/CreateReportMenuItem.tsx | 21 +++---- tests/unit/CreateReportMenuItemTest.tsx | 1 - tests/unit/PolicyUtilsTest.ts | 18 ++++++ .../useCreateNavigationSuggestionsTest.ts | 10 ++-- tests/unit/useCreateReportTest.tsx | 59 ++++++++++++------- 9 files changed, 113 insertions(+), 74 deletions(-) diff --git a/src/components/Search/SearchRouter/useCreateNavigationSuggestions.ts b/src/components/Search/SearchRouter/useCreateNavigationSuggestions.ts index 1645cc01a10d..e9499bd0762a 100644 --- a/src/components/Search/SearchRouter/useCreateNavigationSuggestions.ts +++ b/src/components/Search/SearchRouter/useCreateNavigationSuggestions.ts @@ -23,7 +23,7 @@ import getCreateReportRoute, {getReportsRootRoute, navigateToCreateReportWorkspa import Navigation from '@libs/Navigation/Navigation'; import {openTravelDotLink} from '@libs/openTravelDotLink'; // eslint-disable-next-line no-restricted-imports -- TravelDot booking requires a paid workspace, matching the existing FAB behavior. -import {canSendInvoice, getDefaultChatEnabledPolicy, getGroupPoliciesWhereReportCanBeCreated, hasAcceptedTravelTerms, isPaidGroupPolicy, shouldShowPolicy} from '@libs/PolicyUtils'; +import {canSendInvoice, getGroupPoliciesWhereReportCanBeCreated, hasAcceptedTravelTerms, isPaidGroupPolicy, shouldShowPolicy} from '@libs/PolicyUtils'; import {generateReportID} from '@libs/ReportUtils'; import isOnSearchMoneyRequestReportPage from '@navigation/helpers/isOnSearchMoneyRequestReportPage'; @@ -37,8 +37,11 @@ import {primaryLoginSelector} from '@src/selectors/Account'; import {isTrackIntentUserSelector} from '@src/selectors/Onboarding'; import {emailSelector} from '@src/selectors/Session'; import {validTransactionDraftIDsSelector} from '@src/selectors/TransactionDraft'; +import type {Policy} from '@src/types/onyx'; import type IconAsset from '@src/types/utils/IconAsset'; +import type {OnyxEntry} from 'react-native-onyx'; + import {Str} from 'expensify-common'; import {useState} from 'react'; @@ -114,7 +117,6 @@ function useCreateNavigationSuggestions(query = ''): NavigationSuggestionSourceI const [isLoading = false] = useOnyx(ONYXKEYS.IS_LOADING_APP); const [rules] = useOnyx(ONYXKEYS.COLLECTION.RULE); - const defaultChatEnabledPolicy = getDefaultChatEnabledPolicy([...groupPoliciesWithChatEnabled], activePolicy); const isInvoiceVisible = canSendInvoice(allPolicies ?? null, sessionEmail); const isTravelVisible = !!activePolicy?.isTravelEnabled; const isBlockedFromSpotnanaTravel = isBetaEnabled(CONST.BETAS.PREVENT_SPOTNANA_TRAVEL); @@ -142,8 +144,8 @@ function useCreateNavigationSuggestions(query = ''): NavigationSuggestionSourceI ); const {createReport, isVisible: isCreateReportVisible} = useCreateReport({ - onCreateReport: (shouldDismissEmptyReportsConfirmation?: boolean) => { - if (!defaultChatEnabledPolicy?.id) { + onCreateReport: (policy: OnyxEntry, shouldDismissEmptyReportsConfirmation?: boolean) => { + if (!policy?.id) { return; } @@ -157,7 +159,7 @@ function useCreateNavigationSuggestions(query = ''): NavigationSuggestionSourceI currentUserPersonalDetails, false, isBetaEnabled(CONST.BETAS.ASAP_SUBMIT), - defaultChatEnabledPolicy, + policy, allBetas, isTrackIntentUser, getCurrencyDecimals, diff --git a/src/hooks/useCreateReport.tsx b/src/hooks/useCreateReport.tsx index 4503d0d54dfa..02423a447fbf 100644 --- a/src/hooks/useCreateReport.tsx +++ b/src/hooks/useCreateReport.tsx @@ -1,7 +1,8 @@ +import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID'; import interceptAnonymousUser from '@libs/interceptAnonymousUser'; import createDynamicRoute from '@libs/Navigation/helpers/dynamicRoutesUtils/createDynamicRoute'; import Navigation from '@libs/Navigation/Navigation'; -import {getDefaultChatEnabledPolicy, isGroupPolicy} from '@libs/PolicyUtils'; +import {canCreateReportOnPolicy, getDefaultChatEnabledPolicy, isGroupPolicy} from '@libs/PolicyUtils'; import {generateReportID} from '@libs/ReportUtils'; import {shouldRestrictUserBillableActions} from '@libs/SubscriptionUtils'; @@ -22,8 +23,8 @@ import usePreferredPolicy from './usePreferredPolicy'; import useShouldShowEmptyReportConfirmation from './useShouldShowEmptyReportConfirmation'; type UseCreateReportParams = { - /** Callback that creates the report and navigates after creation */ - onCreateReport: (shouldDismissEmptyReportsConfirmation?: boolean) => void; + /** Callback that creates the report on the resolved workspace and navigates after creation */ + onCreateReport: (policy: OnyxEntry, shouldDismissEmptyReportsConfirmation?: boolean) => void; /** Group paid policies with expense chat enabled */ groupPoliciesWithChatEnabled: readonly never[] | Array>; /** Optional custom navigation to the workspace selector */ @@ -48,7 +49,7 @@ type UseCreateReportResult = { * Decision flow: * 1. Navigate to upgrade path if user has no valid group policies at all * 2. Navigate to workspace selector if default is personal AND there are at least 2 non-personal workspaces, or if the chosen default is billing-restricted and alternatives exist. - * Skipped when the domain security group locks the user to a preferred workspace and a default resolved. + * When the domain security group locks the user to an eligible preferred workspace, that workspace is the default and the selector is never offered. * 3. Show empty report confirmation or create directly if workspace is valid * 4. Navigate to restricted action if billing restricts the workspace */ @@ -65,17 +66,23 @@ export default function useCreateReport({ const [ownerBillingGracePeriodEnd] = useOnyx(ONYXKEYS.NVP_PRIVATE_OWNER_BILLING_GRACE_PERIOD_END); const [userBillingGracePeriodEnds] = useOnyx(ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_USER_BILLING_GRACE_PERIOD_END); const [amountOwed] = useOnyx(ONYXKEYS.NVP_PRIVATE_AMOUNT_OWED); - const {accountID} = useCurrentUserPersonalDetails(); - const {isRestrictedToPreferredPolicy} = usePreferredPolicy(); + const {accountID, login} = useCurrentUserPersonalDetails(); + const {isRestrictedToPreferredPolicy, preferredPolicyID} = usePreferredPolicy(); + const [preferredPolicy, preferredPolicyLoadStatus] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${getNonEmptyStringOnyxID(preferredPolicyID)}`); // Gate visibility and routing on policy hydration. Without this, during Onyx cold-start // groupPoliciesWithChatEnabled.length === 0 would be true even for users who actually have // workspaces, sending them to MONEY_REQUEST_UPGRADE as if they had none. - const arePoliciesLoaded = !isLoadingOnyxValue(policiesLoadStatus); + const arePoliciesLoaded = !isLoadingOnyxValue(policiesLoadStatus, preferredPolicyLoadStatus); const isVisible = arePoliciesLoaded; const shouldNavigateToUpgradePath = groupPoliciesWithChatEnabled.length === 0; - const defaultChatEnabledPolicy = getDefaultChatEnabledPolicy(groupPoliciesWithChatEnabled as Array>, activePolicy); + // A domain security group can lock members to a preferred workspace. When that workspace can take reports it wins over + // the active policy, mirroring useDefaultExpensePolicy. An ineligible preferred workspace falls back to the normal rules. + const isLockedToPreferredPolicy = isRestrictedToPreferredPolicy && canCreateReportOnPolicy(preferredPolicy, login); + const defaultChatEnabledPolicy = isLockedToPreferredPolicy + ? preferredPolicy + : (getDefaultChatEnabledPolicy(groupPoliciesWithChatEnabled as Array>, activePolicy) ?? undefined); const defaultChatEnabledPolicyID = defaultChatEnabledPolicy?.id; const shouldShowEmptyReportConfirmation = useShouldShowEmptyReportConfirmation(defaultChatEnabledPolicyID, shouldSkipEmptyReportConfirmation); @@ -83,7 +90,7 @@ export default function useCreateReport({ const {openCreateReportConfirmation} = useCreateEmptyReportConfirmation({ policyID: defaultChatEnabledPolicyID, policyName: defaultChatEnabledPolicy?.name ?? '', - onConfirm: onCreateReport, + onConfirm: (shouldDismissEmptyReportsConfirmation: boolean) => onCreateReport(defaultChatEnabledPolicy, shouldDismissEmptyReportsConfirmation), shouldHandleNavigationBack, }); @@ -117,13 +124,13 @@ export default function useCreateReport({ // at least 2 non-personal workspaces to choose between. Also fall back to the selector if // the default is billing-restricted and alternatives exist, so the user isn't dead-ended // on the restricted-action page. - // When the domain security group locks the user to a preferred workspace, never offer - // alternatives: create on the default, or land on the restricted-action page if it's billing-restricted. + // When locked to the preferred workspace, never offer alternatives: create on it, or land on + // the restricted-action page if it's billing-restricted. const isDefaultPersonal = !activePolicy || activePolicy.type === CONST.POLICY.TYPE.PERSONAL || !isGroupPolicy(activePolicy); const hasMultipleNonPersonalWorkspaces = groupPoliciesWithChatEnabled.length > 1; const isDefaultBillingRestricted = !!workspaceIDForReportCreation && shouldRestrictUserBillableActions(defaultChatEnabledPolicy, ownerBillingGracePeriodEnd, userBillingGracePeriodEnds, amountOwed, accountID); - const shouldOfferAlternatives = !isRestrictedToPreferredPolicy && hasMultipleNonPersonalWorkspaces && (isDefaultPersonal || isDefaultBillingRestricted); + const shouldOfferAlternatives = !isLockedToPreferredPolicy && hasMultipleNonPersonalWorkspaces && (isDefaultPersonal || isDefaultBillingRestricted); if (!workspaceIDForReportCreation || shouldOfferAlternatives) { if (onNavigateToWorkspaceSelection) { @@ -139,7 +146,7 @@ export default function useCreateReport({ if (shouldShowEmptyReportConfirmation) { openCreateReportConfirmation(); } else { - onCreateReport(false); + onCreateReport(defaultChatEnabledPolicy, false); } return; } @@ -156,7 +163,7 @@ export default function useCreateReport({ userBillingGracePeriodEnds, amountOwed, accountID, - isRestrictedToPreferredPolicy, + isLockedToPreferredPolicy, groupPoliciesWithChatEnabled.length, onNavigateToWorkspaceSelection, shouldShowEmptyReportConfirmation, diff --git a/src/libs/PolicyUtils.ts b/src/libs/PolicyUtils.ts index 3a5bc72863d6..57c2104fd517 100644 --- a/src/libs/PolicyUtils.ts +++ b/src/libs/PolicyUtils.ts @@ -3138,6 +3138,17 @@ function hasAnyPaidPolicy(policies: OnyxCollection | null) { return getGroupPaidPolicies(policies).length > 0; } +/** Whether the user can create a report on this workspace; the per-policy rule behind `getGroupPoliciesWhereReportCanBeCreated`. */ +function canCreateReportOnPolicy(policy: OnyxEntry, currentUserLogin?: string): policy is Policy { + return ( + !!policy && + !policy.isJoinRequestPending && + (isPaidGroupPolicy(policy) || isSubmitPolicy(policy)) && + shouldShowPolicy(policy, false, currentUserLogin) && + !isTeachersUnitePolicyID(policy.id) + ); +} + /** * Returns the group workspaces where the user can create a report: paid (Team/Corporate) workspaces, * plus Submit workspaces. Submit workspaces are free but still support report creation, so they belong @@ -3147,14 +3158,7 @@ function getGroupPoliciesWhereReportCanBeCreated(policies: OnyxCollection - !!policy && - !policy.isJoinRequestPending && - (isPaidGroupPolicy(policy) || isSubmitPolicy(policy)) && - shouldShowPolicy(policy, false, currentUserLogin) && - !isTeachersUnitePolicyID(policy.id), - ); + return Object.values(policies).filter((policy): policy is Policy => canCreateReportOnPolicy(policy, currentUserLogin)); } /** @@ -3629,6 +3633,7 @@ export { areSettingsInErrorFields, settingsPendingAction, getGroupPaidPolicies, + canCreateReportOnPolicy, getGroupPoliciesWhereReportCanBeCreated, getDefaultChatEnabledPolicy, getDefaultChatEnabledPolicySelection, diff --git a/src/pages/Search/EmptySearchView.tsx b/src/pages/Search/EmptySearchView.tsx index 2b803f25234a..43078c46c017 100644 --- a/src/pages/Search/EmptySearchView.tsx +++ b/src/pages/Search/EmptySearchView.tsx @@ -24,7 +24,7 @@ import {startTestDrive} from '@libs/actions/Tour'; import DateUtils from '@libs/DateUtils'; import interceptAnonymousUser from '@libs/interceptAnonymousUser'; import Navigation from '@libs/Navigation/Navigation'; -import {canSendInvoice, getDefaultChatEnabledPolicy, getGroupPoliciesWhereReportCanBeCreated} from '@libs/PolicyUtils'; +import {canSendInvoice, getGroupPoliciesWhereReportCanBeCreated} from '@libs/PolicyUtils'; import {generateReportID, hasViolations as hasViolationsReportUtils} from '@libs/ReportUtils'; import {getAllPolicyValues, getFilterFromQuery, isDefaultExpenseReportsQuery, isDefaultExpensesQuery, isSearchBeforeViolationsSnapshotStarted} from '@libs/SearchQueryUtils'; import {TODO_SEARCH_KEYS} from '@libs/SearchUIUtils'; @@ -158,8 +158,6 @@ function EmptySearchViewContent({ const [isTrackIntentUser] = useOnyx(ONYXKEYS.NVP_INTRO_SELECTED, {selector: isTrackIntentUserSelector}); const [rules] = useOnyx(ONYXKEYS.COLLECTION.RULE); - const defaultChatEnabledPolicy = getDefaultChatEnabledPolicy(groupPoliciesWithChatEnabled as Array>, activePolicy); - const filteredPolicyID = getFilterFromQuery(queryJSON, CONST.SEARCH.SYNTAX_FILTER_KEYS.POLICY_ID); let isFilteredWorkspaceAccessible = true; if (filteredPolicyID.value) { @@ -168,8 +166,8 @@ function EmptySearchViewContent({ isFilteredWorkspaceAccessible = !!filteredPolicy; } - const handleCreateWorkspaceReport = (shouldDismissEmptyReportsConfirmation?: boolean) => { - if (!defaultChatEnabledPolicy?.id) { + const handleCreateWorkspaceReport = (policy: OnyxEntry, shouldDismissEmptyReportsConfirmation?: boolean) => { + if (!policy?.id) { return; } @@ -177,7 +175,7 @@ function EmptySearchViewContent({ currentUserPersonalDetails, hasViolations, isASAPSubmitBetaEnabled, - defaultChatEnabledPolicy, + policy, betas, isTrackIntentUser, getCurrencyDecimals, diff --git a/src/pages/inbox/sidebar/FABPopoverContent/menuItems/CreateReportMenuItem.tsx b/src/pages/inbox/sidebar/FABPopoverContent/menuItems/CreateReportMenuItem.tsx index 1a06e8296b48..60953815a63e 100644 --- a/src/pages/inbox/sidebar/FABPopoverContent/menuItems/CreateReportMenuItem.tsx +++ b/src/pages/inbox/sidebar/FABPopoverContent/menuItems/CreateReportMenuItem.tsx @@ -10,7 +10,7 @@ import useResponsiveLayout from '@hooks/useResponsiveLayout'; import {createNewReport} from '@libs/actions/Report'; import getCreateReportRoute, {getReportsRootRoute, navigateToCreateReportWorkspaceSelection} from '@libs/Navigation/helpers/getCreateReportRoute'; import Navigation from '@libs/Navigation/Navigation'; -import {getDefaultChatEnabledPolicy, getGroupPoliciesWhereReportCanBeCreated} from '@libs/PolicyUtils'; +import {getGroupPoliciesWhereReportCanBeCreated} from '@libs/PolicyUtils'; import {hasViolations as hasViolationsReportUtils} from '@libs/ReportUtils'; import isOnSearchMoneyRequestReportPage from '@navigation/helpers/isOnSearchMoneyRequestReportPage'; @@ -32,14 +32,12 @@ import React from 'react'; const ITEM_ID = CONST.FAB_MENU_ITEM_IDS.CREATE_REPORT; // Returns up to 2 matching policies. The active policy is moved to the front so it survives the slice, -// otherwise getDefaultChatEnabledPolicy can't find it and the workspace selector opens instead of creating directly. +// otherwise useCreateReport can't resolve it as the default and opens the workspace selector instead of creating directly. const chatEnabledPaidGroupPoliciesSelector = (policies: OnyxCollection, currentUserLogin: string | undefined, activePolicyID: string | undefined) => { const eligiblePolicies = getGroupPoliciesWhereReportCanBeCreated(policies, currentUserLogin); - const activePolicyIndex = eligiblePolicies.findIndex((policy) => policy.id === activePolicyID); - if (activePolicyIndex < 1) { - return eligiblePolicies.slice(0, 2); - } - return [eligiblePolicies.at(activePolicyIndex), ...eligiblePolicies.filter((_, index) => index !== activePolicyIndex)].slice(0, 2); + const activePolicy = eligiblePolicies.find((policy) => policy.id === activePolicyID); + const otherPolicy = eligiblePolicies.find((policy) => policy.id !== activePolicyID); + return activePolicy && otherPolicy ? [activePolicy, otherPolicy] : eligiblePolicies.slice(0, 2); }; function CreateReportMenuItem() { @@ -47,7 +45,6 @@ function CreateReportMenuItem() { const {translate} = useLocalize(); const {shouldUseNarrowLayout} = useResponsiveLayout(); const icons = useMemoizedLazyExpensifyIcons(['Document']); - const [activePolicy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${activePolicyID}`); const [session] = useOnyx(ONYXKEYS.SESSION, {selector: sessionEmailAndAccountIDSelector}); const [allBetas] = useOnyx(ONYXKEYS.BETAS); const [transactionViolations] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS); @@ -62,12 +59,10 @@ function CreateReportMenuItem() { const [isTrackIntentUser] = useOnyx(ONYXKEYS.NVP_INTRO_SELECTED, {selector: isTrackIntentUserSelector}); const [rules] = useOnyx(ONYXKEYS.COLLECTION.RULE); - const defaultChatEnabledPolicy = getDefaultChatEnabledPolicy(groupPoliciesWithChatEnabled as Array>, activePolicy); - const isReportInSearch = isOnSearchMoneyRequestReportPage(); - const handleCreateWorkspaceReport = (shouldDismissEmptyReportsConfirmation?: boolean) => { - if (!defaultChatEnabledPolicy?.id) { + const handleCreateWorkspaceReport = (policy: OnyxEntry, shouldDismissEmptyReportsConfirmation?: boolean) => { + if (!policy?.id) { return; } @@ -79,7 +74,7 @@ function CreateReportMenuItem() { currentUserPersonalDetails, hasViolations, isASAPSubmitBetaEnabled, - defaultChatEnabledPolicy, + policy, allBetas, isTrackIntentUser, getCurrencyDecimals, diff --git a/tests/unit/CreateReportMenuItemTest.tsx b/tests/unit/CreateReportMenuItemTest.tsx index 9cd7b8ee2aa5..ab3d2d10f9fa 100644 --- a/tests/unit/CreateReportMenuItemTest.tsx +++ b/tests/unit/CreateReportMenuItemTest.tsx @@ -68,7 +68,6 @@ jest.mock('@libs/PolicyUtils', () => { const CONSTANTS = jest.requireActual<{default: typeof CONST}>('@src/CONST').default; return { - getDefaultChatEnabledPolicy: jest.fn((policies: Policy[]) => policies.at(0)), getGroupPoliciesWhereReportCanBeCreated: jest.fn((policies: Record | undefined) => Object.values(policies ?? {}).filter( (policy): policy is Policy => diff --git a/tests/unit/PolicyUtilsTest.ts b/tests/unit/PolicyUtilsTest.ts index 568769de42e3..8d527a8194a7 100644 --- a/tests/unit/PolicyUtilsTest.ts +++ b/tests/unit/PolicyUtilsTest.ts @@ -15,6 +15,7 @@ import { canSendInvoiceFromWorkspace, evaluateApprovalWorkflowRule, findVendorByID, + canCreateReportOnPolicy, getActivePolicies, getActivePoliciesWithExpenseChat, getActivePoliciesWithExpenseChatAndPerDiemEnabled, @@ -5309,6 +5310,23 @@ describe('arePolicyRulesEnabled', () => { }); }); +describe('canCreateReportOnPolicy', () => { + // createRandomPolicy randomizes these, so eligibility comes out flaky + const eligibleFields = {role: CONST.POLICY.ROLE.ADMIN, isJoinRequestPending: false, pendingAction: undefined, archivedDate: undefined}; + + it.each([ + ['allows a paid Team workspace', {...createRandomPolicy(1, CONST.POLICY.TYPE.TEAM), ...eligibleFields}, true], + ['allows a paid Corporate workspace', {...createRandomPolicy(2, CONST.POLICY.TYPE.CORPORATE), ...eligibleFields}, true], + ['allows a Submit workspace', {...createRandomPolicy(3, CONST.POLICY.TYPE.SUBMIT), ...eligibleFields}, true], + ['rejects a personal workspace', {...createRandomPolicy(4, CONST.POLICY.TYPE.PERSONAL), ...eligibleFields}, false], + ['rejects a workspace with a pending join request', {...createRandomPolicy(5, CONST.POLICY.TYPE.TEAM), ...eligibleFields, isJoinRequestPending: true}, false], + ['rejects a workspace pending deletion', {...createRandomPolicy(6, CONST.POLICY.TYPE.TEAM), ...eligibleFields, pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE}, false], + ['rejects an undefined policy', undefined, false], + ])('%s', (_description, policy, expected) => { + expect(canCreateReportOnPolicy(policy)).toBe(expected); + }); +}); + describe('getDefaultChatEnabledPolicy', () => { const submitPolicy = {...createRandomPolicy(1, CONST.POLICY.TYPE.SUBMIT), id: 'submit1'}; const teamPolicy = {...createRandomPolicy(2, CONST.POLICY.TYPE.TEAM), id: 'team1'}; diff --git a/tests/unit/useCreateNavigationSuggestionsTest.ts b/tests/unit/useCreateNavigationSuggestionsTest.ts index 39bb27024e10..7ffbed7516f1 100644 --- a/tests/unit/useCreateNavigationSuggestionsTest.ts +++ b/tests/unit/useCreateNavigationSuggestionsTest.ts @@ -15,7 +15,7 @@ import ONYXKEYS from '@src/ONYXKEYS'; import ROUTES from '@src/ROUTES'; type MockUseCreateReportParams = { - onCreateReport: (shouldDismissEmptyReportsConfirmation?: boolean) => void; + onCreateReport: (policy: unknown, shouldDismissEmptyReportsConfirmation?: boolean) => void; groupPoliciesWithChatEnabled: unknown[] | readonly never[]; onNavigateToWorkspaceSelection: () => void; shouldHandleNavigationBack: boolean; @@ -37,7 +37,6 @@ const mockUseOnyx = jest.fn const isBetaEnabledByDefault = (beta: string) => beta !== CONST.BETAS.PREVENT_SPOTNANA_TRAVEL; const mockIsBetaEnabled = jest.fn(isBetaEnabledByDefault); const mockCanSendInvoice = jest.fn(() => false); -const mockGetDefaultChatEnabledPolicy = jest.fn((policies: unknown[]) => (policies.length === 1 ? policies.at(0) : undefined)); const mockGetGroupPoliciesWhereReportCanBeCreated = jest.fn(); const mockShouldShowPolicy = jest.fn(() => true); const mockHasAcceptedTravelTerms = jest.fn(() => false); @@ -165,7 +164,6 @@ jest.mock('@libs/openTravelDotLink', () => ({ jest.mock('@libs/PolicyUtils', () => ({ canSendInvoice: (...args: unknown[]) => mockCanSendInvoice(...args), - getDefaultChatEnabledPolicy: (policies: unknown[]) => mockGetDefaultChatEnabledPolicy(policies), getGroupPoliciesWhereReportCanBeCreated: (policies: unknown, currentUserLogin?: string) => mockGetGroupPoliciesWhereReportCanBeCreated(policies, currentUserLogin), hasAcceptedTravelTerms: () => mockHasAcceptedTravelTerms(), isPaidGroupPolicy: () => mockIsPaidGroupPolicy(), @@ -282,7 +280,7 @@ describe('useCreateNavigationSuggestions', () => { renderHook(() => useCreateNavigationSuggestions()); const onCreateReport = mockUseCreateReport.mock.calls.at(0)?.at(0)?.onCreateReport; - act(() => onCreateReport?.()); + act(() => onCreateReport?.(undefined)); expect(createNewReport).not.toHaveBeenCalled(); expect(Navigation.navigate).not.toHaveBeenCalled(); @@ -421,7 +419,7 @@ describe('useCreateNavigationSuggestions', () => { renderHook(() => useCreateNavigationSuggestions()); const onCreateReport = mockUseCreateReport.mock.calls.at(0)?.at(0)?.onCreateReport; - act(() => onCreateReport?.(true)); + act(() => onCreateReport?.(submitPolicy, true)); expect(createNewReport).toHaveBeenCalledWith(expect.anything(), false, true, submitPolicy, [], false, mockGetCurrencyDecimals, undefined, false, true); expect(clearLastSearchParams).not.toHaveBeenCalled(); @@ -437,7 +435,7 @@ describe('useCreateNavigationSuggestions', () => { mockIsOnSearchMoneyRequestReportPage.mockReturnValue(true); const createReportParams = mockUseCreateReport.mock.calls.at(0)?.at(0); - act(() => createReportParams?.onCreateReport()); + act(() => createReportParams?.onCreateReport(submitPolicy)); act(() => createReportParams?.onNavigateToWorkspaceSelection()); expect(clearLastSearchParams).toHaveBeenCalledTimes(1); diff --git a/tests/unit/useCreateReportTest.tsx b/tests/unit/useCreateReportTest.tsx index 43c782d1bb3e..61413d2b5e7c 100644 --- a/tests/unit/useCreateReportTest.tsx +++ b/tests/unit/useCreateReportTest.tsx @@ -55,6 +55,9 @@ jest.mock('@libs/PolicyUtils', () => { isGroupPolicy: jest.fn( (policy: OnyxEntry) => policy?.type === CONSTANTS.POLICY.TYPE.TEAM || policy?.type === CONSTANTS.POLICY.TYPE.CORPORATE || policy?.type === CONSTANTS.POLICY.TYPE.SUBMIT, ), + canCreateReportOnPolicy: jest.fn( + (policy: OnyxEntry) => policy?.type === CONSTANTS.POLICY.TYPE.TEAM || policy?.type === CONSTANTS.POLICY.TYPE.CORPORATE || policy?.type === CONSTANTS.POLICY.TYPE.SUBMIT, + ), }; }); @@ -75,12 +78,12 @@ jest.mock('@libs/SubscriptionUtils', () => ({ shouldRestrictUserBillableActions: (...args: Parameters) => mockShouldRestrictUserBillableActions(...args), })); -const mockIsRestrictedToPreferredPolicy = jest.fn(() => false); -jest.mock('@hooks/usePreferredPolicy', () => () => ({ - isRestrictedToPreferredPolicy: mockIsRestrictedToPreferredPolicy(), - preferredPolicyID: undefined, +const mockUsePreferredPolicy = jest.fn(() => ({ + isRestrictedToPreferredPolicy: false, + preferredPolicyID: undefined as string | undefined, isRestrictedPolicyCreation: false, })); +jest.mock('@hooks/usePreferredPolicy', () => () => mockUsePreferredPolicy()); // ── Helpers ──────────────────────────────────────────────────────────────────── @@ -104,14 +107,21 @@ function makeSubmitPolicy(id = POLICY_ID): Policy { return {...policy, type: CONST.POLICY.TYPE.SUBMIT}; } -function setupUseCreateReportOnyx({activePolicy, emptyReportsConfirmationDismissed}: {activePolicy?: OnyxEntry; emptyReportsConfirmationDismissed?: boolean} = {}) { +function setupUseCreateReportOnyx({ + activePolicy, + preferredPolicy, + emptyReportsConfirmationDismissed, +}: {activePolicy?: OnyxEntry; preferredPolicy?: OnyxEntry; emptyReportsConfirmationDismissed?: boolean} = {}) { mockUseOnyx.mockImplementation((key) => { if (key === ONYXKEYS.NVP_ACTIVE_POLICY_ID) { return [activePolicy?.id, {status: 'loaded'}]; } - if (key === `${ONYXKEYS.COLLECTION.POLICY}${activePolicy?.id}`) { + if (activePolicy && key === `${ONYXKEYS.COLLECTION.POLICY}${activePolicy.id}`) { return [activePolicy, {status: 'loaded'}]; } + if (preferredPolicy && key === `${ONYXKEYS.COLLECTION.POLICY}${preferredPolicy.id}`) { + return [preferredPolicy, {status: 'loaded'}]; + } if (key === ONYXKEYS.NVP_EMPTY_REPORTS_CONFIRMATION_DISMISSED) { return [emptyReportsConfirmationDismissed, {status: 'loaded'}]; } @@ -126,21 +136,28 @@ describe('useCreateReport', () => { jest.clearAllMocks(); reportIDCounter.value = 100; mockShouldRestrictUserBillableActions.mockReturnValue(false); - mockIsRestrictedToPreferredPolicy.mockReturnValue(false); + mockUsePreferredPolicy.mockReturnValue({isRestrictedToPreferredPolicy: false, preferredPolicyID: undefined, isRestrictedPolicyCreation: false}); mockUseShouldShowEmptyReportConfirmation.mockReturnValue(false); setupUseCreateReportOnyx(); }); describe('domain preferred workspace restriction', () => { const personalPolicy: OnyxEntry = {...makePaidPolicy('personal-1'), type: CONST.POLICY.TYPE.PERSONAL}; + const preferredPolicy = makePaidPolicy('preferred-1'); + const ineligiblePreferredPolicy: OnyxEntry = {...makePaidPolicy('preferred-1'), type: CONST.POLICY.TYPE.PERSONAL}; it.each([ - ['creates directly on an unrestricted default with multiple workspaces', makePaidPolicy('p1'), false, 'create'], - ['shows the billing restriction page instead of the selector when the default is billing-restricted with multiple workspaces', makePaidPolicy('p1'), true, 'restricted'], - ['still shows the selector when no default workspace could be resolved', personalPolicy, false, 'selector'], - ])('%s', (_description, activePolicy, isBillingRestricted, expected) => { - setupUseCreateReportOnyx({activePolicy}); - mockIsRestrictedToPreferredPolicy.mockReturnValue(true); + ['creates on the preferred workspace instead of the active one', makePaidPolicy('p1'), preferredPolicy, false, 'create'], + ['creates on the preferred workspace when the active one is personal and multiple workspaces exist', personalPolicy, preferredPolicy, false, 'create'], + ['shows the billing restriction page instead of the selector when the preferred workspace is billing-restricted', makePaidPolicy('p1'), preferredPolicy, true, 'restricted'], + ['falls back to the normal rules when the preferred workspace cannot take reports', personalPolicy, ineligiblePreferredPolicy, false, 'selector'], + ])('%s', (_description, activePolicy, restrictedPolicy, isBillingRestricted, expected) => { + setupUseCreateReportOnyx({activePolicy, preferredPolicy: restrictedPolicy}); + mockUsePreferredPolicy.mockReturnValue({ + isRestrictedToPreferredPolicy: true, + preferredPolicyID: restrictedPolicy?.id, + isRestrictedPolicyCreation: false, + }); mockShouldRestrictUserBillableActions.mockReturnValue(isBillingRestricted); const onCreateReport = jest.fn(); const policies = [makePaidPolicy('p1'), makePaidPolicy('p2'), makePaidPolicy('p3')]; @@ -158,10 +175,10 @@ describe('useCreateReport', () => { const selectorRoute = DYNAMIC_ROUTES.NEW_REPORT_WORKSPACE_SELECTION.getRoute(); if (expected === 'create') { - expect(onCreateReport).toHaveBeenCalledWith(false); + expect(onCreateReport).toHaveBeenCalledWith(expect.objectContaining({id: 'preferred-1'}), false); expect(Navigation.navigate).not.toHaveBeenCalled(); } else if (expected === 'restricted') { - expect(Navigation.navigate).toHaveBeenCalledWith(ROUTES.RESTRICTED_ACTION.getRoute('p1')); + expect(Navigation.navigate).toHaveBeenCalledWith(ROUTES.RESTRICTED_ACTION.getRoute('preferred-1')); expect(Navigation.navigate).not.toHaveBeenCalledWith(selectorRoute); expect(onCreateReport).not.toHaveBeenCalled(); } else { @@ -280,7 +297,7 @@ describe('useCreateReport', () => { }); expect(Navigation.navigate).not.toHaveBeenCalledWith(DYNAMIC_ROUTES.NEW_REPORT_WORKSPACE_SELECTION.getRoute()); - expect(onCreateReport).toHaveBeenCalledWith(false); + expect(onCreateReport).toHaveBeenCalledWith(expect.anything(), false); }); it('does NOT show selector when default is a Submit workspace, even with 2+ Submit workspaces', () => { @@ -302,7 +319,7 @@ describe('useCreateReport', () => { }); expect(Navigation.navigate).not.toHaveBeenCalledWith(DYNAMIC_ROUTES.NEW_REPORT_WORKSPACE_SELECTION.getRoute()); - expect(onCreateReport).toHaveBeenCalledWith(false); + expect(onCreateReport).toHaveBeenCalledWith(expect.anything(), false); }); it('does NOT show selector when default is personal but only 1 non-personal workspace exists', () => { @@ -327,7 +344,7 @@ describe('useCreateReport', () => { }); expect(Navigation.navigate).not.toHaveBeenCalledWith(DYNAMIC_ROUTES.NEW_REPORT_WORKSPACE_SELECTION.getRoute()); - expect(onCreateReport).toHaveBeenCalledWith(false); + expect(onCreateReport).toHaveBeenCalledWith(expect.anything(), false); }); }); @@ -347,7 +364,7 @@ describe('useCreateReport', () => { result.current.createReport(); }); - expect(onCreateReport).toHaveBeenCalledWith(false); + expect(onCreateReport).toHaveBeenCalledWith(expect.anything(), false); expect(Navigation.navigate).not.toHaveBeenCalled(); }); @@ -431,7 +448,7 @@ describe('useCreateReport', () => { }); // Should call onCreateReport directly, not navigate to upgrade - expect(onCreateReport).toHaveBeenCalledWith(false); + expect(onCreateReport).toHaveBeenCalledWith(expect.anything(), false); const calls = jest.mocked(Navigation.navigate).mock.calls; const navigatedToUpgrade = calls.some((call) => { const firstArg = call.at(0); @@ -460,7 +477,7 @@ describe('useCreateReport', () => { result.current.createReport(); }); - expect(onCreateReport).toHaveBeenCalledWith(false); + expect(onCreateReport).toHaveBeenCalledWith(expect.anything(), false); expect(mockOpenCreateReportConfirmation).not.toHaveBeenCalled(); }); }); From 3e97649243f4fc71b1f6428b3993cc6d5364033f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Musia=C5=82?= Date: Fri, 18 Sep 2026 13:03:36 +0200 Subject: [PATCH 3/3] simplify --- src/hooks/useCreateReport.tsx | 23 +++++--------- src/libs/PolicyUtils.ts | 21 +++++-------- .../menuItems/CreateReportMenuItem.tsx | 13 ++------ tests/unit/CreateReportMenuItemTest.tsx | 20 ++++--------- tests/unit/PolicyUtilsTest.ts | 18 ----------- tests/unit/useCreateReportTest.tsx | 30 ++++++------------- 6 files changed, 34 insertions(+), 91 deletions(-) diff --git a/src/hooks/useCreateReport.tsx b/src/hooks/useCreateReport.tsx index 02423a447fbf..d321978152ed 100644 --- a/src/hooks/useCreateReport.tsx +++ b/src/hooks/useCreateReport.tsx @@ -1,8 +1,7 @@ -import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID'; import interceptAnonymousUser from '@libs/interceptAnonymousUser'; import createDynamicRoute from '@libs/Navigation/helpers/dynamicRoutesUtils/createDynamicRoute'; import Navigation from '@libs/Navigation/Navigation'; -import {canCreateReportOnPolicy, getDefaultChatEnabledPolicy, isGroupPolicy} from '@libs/PolicyUtils'; +import {getDefaultChatEnabledPolicy, isGroupPolicy} from '@libs/PolicyUtils'; import {generateReportID} from '@libs/ReportUtils'; import {shouldRestrictUserBillableActions} from '@libs/SubscriptionUtils'; @@ -48,8 +47,7 @@ type UseCreateReportResult = { * * Decision flow: * 1. Navigate to upgrade path if user has no valid group policies at all - * 2. Navigate to workspace selector if default is personal AND there are at least 2 non-personal workspaces, or if the chosen default is billing-restricted and alternatives exist. - * When the domain security group locks the user to an eligible preferred workspace, that workspace is the default and the selector is never offered. + * 2. Navigate to workspace selector if default is personal AND there are at least 2 non-personal workspaces, or if the chosen default is billing-restricted and alternatives exist * 3. Show empty report confirmation or create directly if workspace is valid * 4. Navigate to restricted action if billing restricts the workspace */ @@ -66,23 +64,20 @@ export default function useCreateReport({ const [ownerBillingGracePeriodEnd] = useOnyx(ONYXKEYS.NVP_PRIVATE_OWNER_BILLING_GRACE_PERIOD_END); const [userBillingGracePeriodEnds] = useOnyx(ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_USER_BILLING_GRACE_PERIOD_END); const [amountOwed] = useOnyx(ONYXKEYS.NVP_PRIVATE_AMOUNT_OWED); - const {accountID, login} = useCurrentUserPersonalDetails(); + const {accountID} = useCurrentUserPersonalDetails(); const {isRestrictedToPreferredPolicy, preferredPolicyID} = usePreferredPolicy(); - const [preferredPolicy, preferredPolicyLoadStatus] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${getNonEmptyStringOnyxID(preferredPolicyID)}`); // Gate visibility and routing on policy hydration. Without this, during Onyx cold-start // groupPoliciesWithChatEnabled.length === 0 would be true even for users who actually have // workspaces, sending them to MONEY_REQUEST_UPGRADE as if they had none. - const arePoliciesLoaded = !isLoadingOnyxValue(policiesLoadStatus, preferredPolicyLoadStatus); + const arePoliciesLoaded = !isLoadingOnyxValue(policiesLoadStatus); const isVisible = arePoliciesLoaded; const shouldNavigateToUpgradePath = groupPoliciesWithChatEnabled.length === 0; - // A domain security group can lock members to a preferred workspace. When that workspace can take reports it wins over - // the active policy, mirroring useDefaultExpensePolicy. An ineligible preferred workspace falls back to the normal rules. - const isLockedToPreferredPolicy = isRestrictedToPreferredPolicy && canCreateReportOnPolicy(preferredPolicy, login); - const defaultChatEnabledPolicy = isLockedToPreferredPolicy - ? preferredPolicy - : (getDefaultChatEnabledPolicy(groupPoliciesWithChatEnabled as Array>, activePolicy) ?? undefined); + // A domain security group can lock the user to a preferred workspace. It then takes precedence over the active policy and the selector is skipped. + const lockedPreferredPolicy = isRestrictedToPreferredPolicy ? groupPoliciesWithChatEnabled.find((policy) => policy?.id === preferredPolicyID) : undefined; + const isLockedToPreferredPolicy = !!lockedPreferredPolicy; + const defaultChatEnabledPolicy = lockedPreferredPolicy ?? getDefaultChatEnabledPolicy(groupPoliciesWithChatEnabled as Array>, activePolicy) ?? undefined; const defaultChatEnabledPolicyID = defaultChatEnabledPolicy?.id; const shouldShowEmptyReportConfirmation = useShouldShowEmptyReportConfirmation(defaultChatEnabledPolicyID, shouldSkipEmptyReportConfirmation); @@ -124,8 +119,6 @@ export default function useCreateReport({ // at least 2 non-personal workspaces to choose between. Also fall back to the selector if // the default is billing-restricted and alternatives exist, so the user isn't dead-ended // on the restricted-action page. - // When locked to the preferred workspace, never offer alternatives: create on it, or land on - // the restricted-action page if it's billing-restricted. const isDefaultPersonal = !activePolicy || activePolicy.type === CONST.POLICY.TYPE.PERSONAL || !isGroupPolicy(activePolicy); const hasMultipleNonPersonalWorkspaces = groupPoliciesWithChatEnabled.length > 1; const isDefaultBillingRestricted = diff --git a/src/libs/PolicyUtils.ts b/src/libs/PolicyUtils.ts index fcb829f8de58..c477e5de5b59 100644 --- a/src/libs/PolicyUtils.ts +++ b/src/libs/PolicyUtils.ts @@ -3163,17 +3163,6 @@ function hasAnyPaidPolicy(policies: OnyxCollection | null) { return getGroupPaidPolicies(policies).length > 0; } -/** Whether the user can create a report on this workspace; the per-policy rule behind `getGroupPoliciesWhereReportCanBeCreated`. */ -function canCreateReportOnPolicy(policy: OnyxEntry, currentUserLogin?: string): policy is Policy { - return ( - !!policy && - !policy.isJoinRequestPending && - (isPaidGroupPolicy(policy) || isSubmitPolicy(policy)) && - shouldShowPolicy(policy, false, currentUserLogin) && - !isTeachersUnitePolicyID(policy.id) - ); -} - /** * Returns the group workspaces where the user can create a report: paid (Team/Corporate) workspaces, * plus Submit workspaces. Submit workspaces are free but still support report creation, so they belong @@ -3183,7 +3172,14 @@ function getGroupPoliciesWhereReportCanBeCreated(policies: OnyxCollection canCreateReportOnPolicy(policy, currentUserLogin)); + return Object.values(policies).filter( + (policy): policy is Policy => + !!policy && + !policy.isJoinRequestPending && + (isPaidGroupPolicy(policy) || isSubmitPolicy(policy)) && + shouldShowPolicy(policy, false, currentUserLogin) && + !isTeachersUnitePolicyID(policy.id), + ); } /** @@ -3660,7 +3656,6 @@ export { areSettingsInErrorFields, settingsPendingAction, getGroupPaidPolicies, - canCreateReportOnPolicy, getGroupPoliciesWhereReportCanBeCreated, getDefaultChatEnabledPolicy, getDefaultChatEnabledPolicySelection, diff --git a/src/pages/inbox/sidebar/FABPopoverContent/menuItems/CreateReportMenuItem.tsx b/src/pages/inbox/sidebar/FABPopoverContent/menuItems/CreateReportMenuItem.tsx index 7825a689abd3..b8b619c0062a 100644 --- a/src/pages/inbox/sidebar/FABPopoverContent/menuItems/CreateReportMenuItem.tsx +++ b/src/pages/inbox/sidebar/FABPopoverContent/menuItems/CreateReportMenuItem.tsx @@ -31,17 +31,10 @@ import React from 'react'; const ITEM_ID = CONST.FAB_MENU_ITEM_IDS.CREATE_REPORT; -// Returns up to 2 matching policies. The active policy is moved to the front so it survives the slice, -// otherwise useCreateReport can't resolve it as the default and opens the workspace selector instead of creating directly. -const chatEnabledPaidGroupPoliciesSelector = (policies: OnyxCollection, currentUserLogin: string | undefined, activePolicyID: string | undefined) => { - const eligiblePolicies = getGroupPoliciesWhereReportCanBeCreated(policies, currentUserLogin); - const activePolicy = eligiblePolicies.find((policy) => policy.id === activePolicyID); - const otherPolicy = eligiblePolicies.find((policy) => policy.id !== activePolicyID); - return activePolicy && otherPolicy ? [activePolicy, otherPolicy] : eligiblePolicies.slice(0, 2); -}; +const chatEnabledPaidGroupPoliciesSelector = (policies: OnyxCollection, currentUserLogin: string | undefined) => + getGroupPoliciesWhereReportCanBeCreated(policies, currentUserLogin); function CreateReportMenuItem() { - const [activePolicyID] = useOnyx(ONYXKEYS.NVP_ACTIVE_POLICY_ID); const {translate} = useLocalize(); const {shouldUseNarrowLayout} = useResponsiveLayout(); const icons = useMemoizedLazyExpensifyIcons(['Document']); @@ -53,7 +46,7 @@ function CreateReportMenuItem() { const isASAPSubmitBetaEnabled = isBetaEnabled(CONST.BETAS.ASAP_SUBMIT); const hasViolations = hasViolationsReportUtils(undefined, transactionViolations, session?.accountID ?? CONST.DEFAULT_NUMBER_ID, session?.email ?? ''); const [groupPoliciesWithChatEnabled = CONST.EMPTY_ARRAY] = useOnyx(ONYXKEYS.COLLECTION.POLICY, { - selector: (policies: Parameters[0]) => chatEnabledPaidGroupPoliciesSelector(policies, session?.email, activePolicyID), + selector: (policies: Parameters[0]) => chatEnabledPaidGroupPoliciesSelector(policies, session?.email), }); const [isTrackIntentUser] = useOnyx(ONYXKEYS.NVP_INTRO_SELECTED, {selector: isTrackIntentUserSelector}); const [rules] = useOnyx(ONYXKEYS.COLLECTION.RULE); diff --git a/tests/unit/CreateReportMenuItemTest.tsx b/tests/unit/CreateReportMenuItemTest.tsx index cf56c523d127..deb1063f2298 100644 --- a/tests/unit/CreateReportMenuItemTest.tsx +++ b/tests/unit/CreateReportMenuItemTest.tsx @@ -133,27 +133,19 @@ describe('CreateReportMenuItem', () => { setupUseOnyx(); }); - it('passes only report-creation workspaces to useCreateReport', () => { + it.each([ + ['the personal workspace is active', 'personal-1'], + ['a workspace beyond the first two eligible ones is active', 'corporate-1'], + ])('passes every report-creation workspace to useCreateReport when %s', (_description, activePolicyID) => { + setupUseOnyx(activePolicyID); + render(); const params = mockUseCreateReport.mock.calls.at(0)?.at(0); - expect(params?.groupPoliciesWithChatEnabled).toHaveLength(2); expect(params?.groupPoliciesWithChatEnabled).toEqual([ expect.objectContaining({id: 'team-1', type: CONST.POLICY.TYPE.TEAM}), expect.objectContaining({id: 'submit-1', type: CONST.POLICY.TYPE.SUBMIT}), - ]); - }); - - it('keeps the active workspace in the sliced list when it is not among the first two eligible policies', () => { - setupUseOnyx('corporate-1'); - - render(); - - const params = mockUseCreateReport.mock.calls.at(0)?.at(0); - expect(params?.groupPoliciesWithChatEnabled).toHaveLength(2); - expect(params?.groupPoliciesWithChatEnabled).toEqual([ expect.objectContaining({id: 'corporate-1', type: CONST.POLICY.TYPE.CORPORATE}), - expect.objectContaining({id: 'team-1', type: CONST.POLICY.TYPE.TEAM}), ]); }); }); diff --git a/tests/unit/PolicyUtilsTest.ts b/tests/unit/PolicyUtilsTest.ts index 54533dcd9436..c3e025ef9846 100644 --- a/tests/unit/PolicyUtilsTest.ts +++ b/tests/unit/PolicyUtilsTest.ts @@ -15,7 +15,6 @@ import { canSendInvoiceFromWorkspace, evaluateApprovalWorkflowRule, findVendorByID, - canCreateReportOnPolicy, getActivePolicies, getActivePoliciesWithExpenseChat, getActivePoliciesWithExpenseChatAndPerDiemEnabled, @@ -5373,23 +5372,6 @@ describe('arePolicyRulesEnabled', () => { }); }); -describe('canCreateReportOnPolicy', () => { - // createRandomPolicy randomizes these, so eligibility comes out flaky - const eligibleFields = {role: CONST.POLICY.ROLE.ADMIN, isJoinRequestPending: false, pendingAction: undefined, archivedDate: undefined}; - - it.each([ - ['allows a paid Team workspace', {...createRandomPolicy(1, CONST.POLICY.TYPE.TEAM), ...eligibleFields}, true], - ['allows a paid Corporate workspace', {...createRandomPolicy(2, CONST.POLICY.TYPE.CORPORATE), ...eligibleFields}, true], - ['allows a Submit workspace', {...createRandomPolicy(3, CONST.POLICY.TYPE.SUBMIT), ...eligibleFields}, true], - ['rejects a personal workspace', {...createRandomPolicy(4, CONST.POLICY.TYPE.PERSONAL), ...eligibleFields}, false], - ['rejects a workspace with a pending join request', {...createRandomPolicy(5, CONST.POLICY.TYPE.TEAM), ...eligibleFields, isJoinRequestPending: true}, false], - ['rejects a workspace pending deletion', {...createRandomPolicy(6, CONST.POLICY.TYPE.TEAM), ...eligibleFields, pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE}, false], - ['rejects an undefined policy', undefined, false], - ])('%s', (_description, policy, expected) => { - expect(canCreateReportOnPolicy(policy)).toBe(expected); - }); -}); - describe('getDefaultChatEnabledPolicy', () => { const submitPolicy = {...createRandomPolicy(1, CONST.POLICY.TYPE.SUBMIT), id: 'submit1'}; const teamPolicy = {...createRandomPolicy(2, CONST.POLICY.TYPE.TEAM), id: 'team1'}; diff --git a/tests/unit/useCreateReportTest.tsx b/tests/unit/useCreateReportTest.tsx index 61413d2b5e7c..7a7e6048ea8a 100644 --- a/tests/unit/useCreateReportTest.tsx +++ b/tests/unit/useCreateReportTest.tsx @@ -55,9 +55,6 @@ jest.mock('@libs/PolicyUtils', () => { isGroupPolicy: jest.fn( (policy: OnyxEntry) => policy?.type === CONSTANTS.POLICY.TYPE.TEAM || policy?.type === CONSTANTS.POLICY.TYPE.CORPORATE || policy?.type === CONSTANTS.POLICY.TYPE.SUBMIT, ), - canCreateReportOnPolicy: jest.fn( - (policy: OnyxEntry) => policy?.type === CONSTANTS.POLICY.TYPE.TEAM || policy?.type === CONSTANTS.POLICY.TYPE.CORPORATE || policy?.type === CONSTANTS.POLICY.TYPE.SUBMIT, - ), }; }); @@ -107,11 +104,7 @@ function makeSubmitPolicy(id = POLICY_ID): Policy { return {...policy, type: CONST.POLICY.TYPE.SUBMIT}; } -function setupUseCreateReportOnyx({ - activePolicy, - preferredPolicy, - emptyReportsConfirmationDismissed, -}: {activePolicy?: OnyxEntry; preferredPolicy?: OnyxEntry; emptyReportsConfirmationDismissed?: boolean} = {}) { +function setupUseCreateReportOnyx({activePolicy, emptyReportsConfirmationDismissed}: {activePolicy?: OnyxEntry; emptyReportsConfirmationDismissed?: boolean} = {}) { mockUseOnyx.mockImplementation((key) => { if (key === ONYXKEYS.NVP_ACTIVE_POLICY_ID) { return [activePolicy?.id, {status: 'loaded'}]; @@ -119,9 +112,6 @@ function setupUseCreateReportOnyx({ if (activePolicy && key === `${ONYXKEYS.COLLECTION.POLICY}${activePolicy.id}`) { return [activePolicy, {status: 'loaded'}]; } - if (preferredPolicy && key === `${ONYXKEYS.COLLECTION.POLICY}${preferredPolicy.id}`) { - return [preferredPolicy, {status: 'loaded'}]; - } if (key === ONYXKEYS.NVP_EMPTY_REPORTS_CONFIRMATION_DISMISSED) { return [emptyReportsConfirmationDismissed, {status: 'loaded'}]; } @@ -143,24 +133,22 @@ describe('useCreateReport', () => { describe('domain preferred workspace restriction', () => { const personalPolicy: OnyxEntry = {...makePaidPolicy('personal-1'), type: CONST.POLICY.TYPE.PERSONAL}; - const preferredPolicy = makePaidPolicy('preferred-1'); - const ineligiblePreferredPolicy: OnyxEntry = {...makePaidPolicy('preferred-1'), type: CONST.POLICY.TYPE.PERSONAL}; it.each([ - ['creates on the preferred workspace instead of the active one', makePaidPolicy('p1'), preferredPolicy, false, 'create'], - ['creates on the preferred workspace when the active one is personal and multiple workspaces exist', personalPolicy, preferredPolicy, false, 'create'], - ['shows the billing restriction page instead of the selector when the preferred workspace is billing-restricted', makePaidPolicy('p1'), preferredPolicy, true, 'restricted'], - ['falls back to the normal rules when the preferred workspace cannot take reports', personalPolicy, ineligiblePreferredPolicy, false, 'selector'], - ])('%s', (_description, activePolicy, restrictedPolicy, isBillingRestricted, expected) => { - setupUseCreateReportOnyx({activePolicy, preferredPolicy: restrictedPolicy}); + ['creates on the preferred workspace instead of the active one', makePaidPolicy('p1'), 'preferred-1', false, 'create'], + ['creates on the preferred workspace when the active one is personal and multiple workspaces exist', personalPolicy, 'preferred-1', false, 'create'], + ['shows the billing restriction page instead of the selector when the preferred workspace is billing-restricted', makePaidPolicy('p1'), 'preferred-1', true, 'restricted'], + ['falls back to the normal rules when the preferred workspace is not one the user can create reports on', personalPolicy, 'not-eligible', false, 'selector'], + ])('%s', (_description, activePolicy, preferredPolicyID, isBillingRestricted, expected) => { + setupUseCreateReportOnyx({activePolicy}); mockUsePreferredPolicy.mockReturnValue({ isRestrictedToPreferredPolicy: true, - preferredPolicyID: restrictedPolicy?.id, + preferredPolicyID, isRestrictedPolicyCreation: false, }); mockShouldRestrictUserBillableActions.mockReturnValue(isBillingRestricted); const onCreateReport = jest.fn(); - const policies = [makePaidPolicy('p1'), makePaidPolicy('p2'), makePaidPolicy('p3')]; + const policies = [makePaidPolicy('p1'), makePaidPolicy('p2'), makePaidPolicy('preferred-1')]; const {result} = renderHook(() => useCreateReport({