diff --git a/src/libs/PolicyUtils.ts b/src/libs/PolicyUtils.ts index 47276dc4c220..e2dd6a7aca94 100644 --- a/src/libs/PolicyUtils.ts +++ b/src/libs/PolicyUtils.ts @@ -2432,21 +2432,25 @@ function isXeroActiveMatchingSource(policy: OnyxEntry): boolean { } /** - * Vendor matching feature gate. Returns true when the workspace has the `vendorMatching` beta - * enabled AND a supported accounting integration is connected with a non-reimbursable export type - * that scopes the vendor field. Mirrors the per-integration `hasVendorFeature` checks on the PHP - * side so the App and backend agree on which workspaces see the field. + * Vendor matching feature gate. Returns true when a supported accounting integration is connected + * with a non-reimbursable export type that scopes the vendor field. Mirrors the per-integration + * `hasVendorFeature` checks on the PHP side so the App and backend agree on which workspaces see + * the field. * - * Supported integrations: - * - QBO with non-reimbursable export = Credit Card or Debit Card (R1) - * - Sage Intacct with non-reimbursable export = Credit Card Charge (R2) - * - Xero (R4) — no export-destination enum; connection present is sufficient + * The `vendorMatching` beta only gates the integrations that haven't reached GA yet, so + * `isVendorMatchingBetaEnabled` is consulted on the Intacct and Xero branches but not on QBO: + * - QBO (R1) with non-reimbursable export = Credit Card or Debit Card. GA, so no beta required + * - Sage Intacct (R2) with non-reimbursable export = Credit Card Charge. Beta required + * - Xero (R3) has no export destination enum, so a present connection is enough. Beta required */ function hasVendorFeature(policy: OnyxEntry, isVendorMatchingBetaEnabled: boolean): boolean { - if (!isVendorMatchingBetaEnabled || !policy) { + if (!policy) { return false; } - return isQBOVendorMatchingActive(policy) || isIntacctVendorMatchingActive(policy) || isXeroVendorMatchingActive(policy); + if (isQBOVendorMatchingActive(policy)) { + return true; + } + return isVendorMatchingBetaEnabled && (isIntacctVendorMatchingActive(policy) || isXeroVendorMatchingActive(policy)); } /** diff --git a/src/pages/workspace/WorkspaceInitialPage.tsx b/src/pages/workspace/WorkspaceInitialPage.tsx index abc7fce0096d..576970144512 100644 --- a/src/pages/workspace/WorkspaceInitialPage.tsx +++ b/src/pages/workspace/WorkspaceInitialPage.tsx @@ -261,10 +261,12 @@ function WorkspaceInitialPage({policyDraft, policy: policyProp, route}: Workspac // The Vendors row gate below reads policy.connections (via hasVendorFeature and // isMatchingVendorListLoaded), which is empty on a non-active workspace until a page - // requiring connections is opened. Prefetch it here, gated on read-access + beta so this - // doesn't fire an accounting-page read on every workspace visit. + // requiring connections is opened. Prefetch it here, gated on read-access. It can't be + // narrowed to vendor-capable workspaces because that answer lives in the very data being + // fetched. The hook already skips the fetch when the app is offline, when the workspace has + // no accounting connection, and when the data has already been fetched. const canReadVendors = canReadPolicyFeature(CONST.POLICY.POLICY_FEATURE.VENDORS); - usePolicyConnectionsPrefetch(policy, canReadVendors && isBetaEnabled(CONST.BETAS.VENDOR_MATCHING)); + usePolicyConnectionsPrefetch(policy, canReadVendors); const workspaceMenuItems: WorkspaceMenuItem[] = [ { diff --git a/src/pages/workspace/WorkspaceMoreFeaturesPage/index.tsx b/src/pages/workspace/WorkspaceMoreFeaturesPage/index.tsx index 9517b73cbcee..b23847fe0002 100644 --- a/src/pages/workspace/WorkspaceMoreFeaturesPage/index.tsx +++ b/src/pages/workspace/WorkspaceMoreFeaturesPage/index.tsx @@ -149,11 +149,18 @@ function WorkspaceMoreFeaturesPage({policy, route}: WorkspaceMoreFeaturesPagePro const isTravelInvoicingEnabled = getIsTravelInvoicingEnabled(getCardSettings(travelCardSettings, CONST.TRAVEL.PROGRAM_TRAVEL_US)); const {canWrite: canWriteMoreFeatures, withReadOnlyFallback} = usePolicyFeatureWriteAccess(policy, CONST.POLICY.POLICY_FEATURE.MORE_FEATURES); - // The Vendors toggle's active state reads policy.connections (via hasVendorFeature), which is - // empty on a non-active workspace until a connections-aware read runs. OpenPolicyMoreFeaturesPage - // doesn't hydrate the detailed connection config, so prefetch it here (gated on the beta) to keep - // the toggle from showing off/non-navigable when the workspace actually supports vendors. - usePolicyConnectionsPrefetch(policy, isVendorMatchingEnabled); + // The Vendors toggle reads policy.connections (via hasVendorFeature), which is empty on a + // non-active workspace until a connections-aware read runs. OpenPolicyMoreFeaturesPage doesn't + // hydrate the detailed connection config, so prefetch it here to keep the toggle from showing + // off and non-navigable when the workspace actually supports vendors. It can't be narrowed to + // vendor-capable workspaces because that answer lives in the very data being fetched. The hook + // already skips the fetch when the app is offline, when the workspace has no accounting + // connection, and when the data has already been fetched. + usePolicyConnectionsPrefetch(policy, true); + + // Beta members see the row on any workspace so they can tell the feature exists. Everyone else + // only sees it once a connection actually scopes the vendor field, which post-GA means QBO. + const shouldShowVendorsFeature = isVendorMatchingEnabled || hasVendorFeature(policy, isVendorMatchingEnabled); const warnAccountingManagesOrganizeFeature = async () => { if (!hasAccountingConnection || !policyID) { @@ -458,7 +465,7 @@ function WorkspaceMoreFeaturesPage({policy, route}: WorkspaceMoreFeaturesPagePro Navigation.navigate(ROUTES.WORKSPACE_TAXES.getRoute(policyID)); }} /> - {isVendorMatchingEnabled && ( + {shouldShowVendorsFeature && ( vendor.id === qboConfig?.nonReimbursableCreditCardDefaultVendor); // This page is the QBO-only default-vendor editor: gate the row on QBO's own non-reimbursable export mode rather than the cross-integration `hasVendorFeature`, so an Intacct workspace whose QBO connection is in Vendor Bill mode does not get a QBO default-vendor row whose target setting isn't active. const qboNonReimbursableDestination = qboConfig?.nonReimbursableExpensesExportDestination; - const isQBOVendorMatchingActive = + const isVendorFeatureAvailable = qboNonReimbursableDestination === CONST.QUICKBOOKS_NON_REIMBURSABLE_EXPORT_ACCOUNT_TYPE.CREDIT_CARD || qboNonReimbursableDestination === CONST.QUICKBOOKS_NON_REIMBURSABLE_EXPORT_ACCOUNT_TYPE.DEBIT_CARD; - const isVendorFeatureAvailable = isBetaEnabled(CONST.BETAS.VENDOR_MATCHING) && isQBOVendorMatchingActive; const backPath = useDynamicBackPath(DYNAMIC_ROUTES.POLICY_ACCOUNTING_QUICKBOOKS_ONLINE_COMPANY_CARD_EXPENSE_ACCOUNT.path); const {isAccordionExpanded, shouldAnimateAccordionSection} = useAccordionAnimation(!!qboConfig?.autoCreateVendor); let nonReimbursableExportDescription; diff --git a/src/pages/workspace/rules/MerchantRules/AddVendorPage.tsx b/src/pages/workspace/rules/MerchantRules/AddVendorPage.tsx index cc715721eda2..166d9e0b3fba 100644 --- a/src/pages/workspace/rules/MerchantRules/AddVendorPage.tsx +++ b/src/pages/workspace/rules/MerchantRules/AddVendorPage.tsx @@ -51,10 +51,12 @@ function AddVendorPage({route}: AddVendorPageProps) { // This picker can be deep-linked directly, and its gate below reads policy.connections (via // hasVendorFeature and getMatchingVendorByID), which is empty on a non-active workspace until a page - // requiring connections is opened. Prefetch it here, gated on the beta alone (not hasVendorFeature, - // which itself depends on the connection data — a chicken-and-egg) so the picker becomes available and - // resolves the selected vendor once connections hydrate. - const {isFetchNeeded, isLoadingFetchedFlag} = usePolicyConnectionsPrefetch(policy, isBetaEnabled(CONST.BETAS.VENDOR_MATCHING)); + // requiring connections is opened. Prefetch it here unconditionally so the picker becomes available + // and resolves the selected vendor once connections hydrate. It can't be narrowed by + // hasVendorFeature, because that itself depends on the connection data being fetched. The hook + // already skips the fetch when the app is offline, when the workspace has no accounting + // connection, and when the data has already been fetched. + const {isFetchNeeded, isLoadingFetchedFlag} = usePolicyConnectionsPrefetch(policy, true); const selectedVendorItem = getSelectedVendorItem(policy, form?.vendorID); diff --git a/src/pages/workspace/rules/MerchantRules/MerchantRulePageBase.tsx b/src/pages/workspace/rules/MerchantRules/MerchantRulePageBase.tsx index accdae73401c..a4ee32094e91 100644 --- a/src/pages/workspace/rules/MerchantRules/MerchantRulePageBase.tsx +++ b/src/pages/workspace/rules/MerchantRules/MerchantRulePageBase.tsx @@ -136,10 +136,12 @@ function MerchantRulePageBase({policyID, ruleID, initialCategoryName, titleKey, // The "Set vendor to" row gate below reads policy.connections (via hasVendorFeature and // isMatchingVendorListLoaded), which is empty on a non-active workspace until a page requiring - // connections is opened. This editor only fetches categories/tags, so prefetch connections here, - // gated on the beta alone (not hasVendorFeature, which itself depends on the connection data — a - // chicken-and-egg) so the row appears and resolves the stored vendor once connections hydrate. - usePolicyConnectionsPrefetch(policy, isBetaEnabled(CONST.BETAS.VENDOR_MATCHING)); + // connections is opened. This editor only fetches categories and tags, so prefetch connections + // here unconditionally so the row appears and resolves the stored vendor once connections + // hydrate. It can't be narrowed by hasVendorFeature, because that itself depends on the + // connection data being fetched. The hook already skips the fetch when the app is offline, when + // the workspace has no accounting connection, and when the data has already been fetched. + usePolicyConnectionsPrefetch(policy, true); // Get the existing rule from the policy (for edit mode) const existingRule = ruleID ? policy?.rules?.codingRules?.[ruleID] : undefined; diff --git a/tests/ui/MoneyRequestViewTest.tsx b/tests/ui/MoneyRequestViewTest.tsx index e36db3677c5f..85263ffd5664 100644 --- a/tests/ui/MoneyRequestViewTest.tsx +++ b/tests/ui/MoneyRequestViewTest.tsx @@ -520,6 +520,69 @@ describe('MoneyRequestView edit fields', () => { }); }); + it('shows the vendor row on QBO without the vendorMatching beta because QBO (R1) is generally available', async () => { + const threadReport = { + ...LHNTestUtils.getFakeReport(), + parentReportID: expenseReportID, + parentReportActionID, + }; + + await setupTestData(); + await act(async () => { + await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, { + reimbursable: false, + comment: {vendor: {externalID: 'v-1', isManuallySet: false}}, + }); + }); + await waitForBatchedUpdatesWithAct(); + + renderMoneyRequestView(threadReport, { + connections: { + [CONST.POLICY.CONNECTIONS.NAME.QBO]: { + config: {nonReimbursableExpensesExportDestination: CONST.QUICKBOOKS_NON_REIMBURSABLE_EXPORT_ACCOUNT_TYPE.CREDIT_CARD}, + data: {vendors: [{id: 'v-1', name: 'Acme Co', currency: CONST.CURRENCY.USD}]}, + }, + }, + }); + await waitForBatchedUpdatesWithAct(); + + await waitFor(() => { + expect(screen.getByTestId('menu-item-title-common.vendor')).toHaveTextContent('Acme Co'); + }); + }); + + it('hides the vendor row on Xero without the vendorMatching beta because Xero (R3) is still pre-GA', async () => { + const threadReport = { + ...LHNTestUtils.getFakeReport(), + parentReportID: expenseReportID, + parentReportActionID, + }; + + await setupTestData(); + await act(async () => { + await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, { + reimbursable: false, + comment: {vendor: {externalID: 'xc1', isManuallySet: false}}, + }); + }); + await waitForBatchedUpdatesWithAct(); + + renderMoneyRequestView(threadReport, { + connections: { + [CONST.POLICY.CONNECTIONS.NAME.XERO]: { + config: {isConfigured: true}, + data: {contacts: {xc1: {id: 'xc1', name: 'Acme Xero', email: 'acme@example.com'}}}, + }, + }, + }); + await waitForBatchedUpdatesWithAct(); + + await waitFor(() => { + expect(screen.queryByTestId('menu-item-common.supplier')).not.toBeOnTheScreen(); + expect(screen.queryByTestId('menu-item-common.vendor')).not.toBeOnTheScreen(); + }); + }); + it('falls back to the vendor externalID when the assigned vendor is missing from every connection', async () => { const threadReport = { ...LHNTestUtils.getFakeReport(), diff --git a/tests/unit/AddVendorPageTest.ts b/tests/unit/AddVendorPageTest.ts index eec2120de6c5..f1bca4c34ba7 100644 --- a/tests/unit/AddVendorPageTest.ts +++ b/tests/unit/AddVendorPageTest.ts @@ -115,8 +115,12 @@ describe('AddVendorPage', () => { const qboPolicy = buildQBOPolicy([{id: 'v-1', name: 'Acme Co', currency: 'USD'}]); const xeroPolicy = buildXeroPolicy({xc1: {id: 'xc1', name: 'Acme Xero', email: 'acme@example.com'}}); - it('hides the row when the beta is off even with a vendor integration connected', () => { - expect(hasVendorFeature(qboPolicy, false)).toBe(false); + it('shows the row on QBO with the beta off because QBO vendor matching is generally available', () => { + expect(hasVendorFeature(qboPolicy, false)).toBe(true); + }); + + it('hides the row on Xero when the beta is off because Xero vendor matching is not generally available yet', () => { + expect(hasVendorFeature(xeroPolicy, false)).toBe(false); }); it('hides the row when no vendor integration is connected', () => { diff --git a/tests/unit/PolicyUtilsTest.ts b/tests/unit/PolicyUtilsTest.ts index 694be3688800..92413431f44a 100644 --- a/tests/unit/PolicyUtilsTest.ts +++ b/tests/unit/PolicyUtilsTest.ts @@ -3550,15 +3550,23 @@ describe('PolicyUtils', () => { expect(hasVendorFeature(buildXeroPolicy(undefined, {isConfigured: false}), true)).toBe(false); }); - it('returns false when beta is disabled, even with Credit Card export configured', () => { - expect(hasVendorFeature(buildQBOPolicy(CONST.QUICKBOOKS_NON_REIMBURSABLE_EXPORT_ACCOUNT_TYPE.CREDIT_CARD), false)).toBe(false); + it('returns true when beta is disabled and QBO non-reimbursable export is Credit Card because QBO (R1) is generally available', () => { + expect(hasVendorFeature(buildQBOPolicy(CONST.QUICKBOOKS_NON_REIMBURSABLE_EXPORT_ACCOUNT_TYPE.CREDIT_CARD), false)).toBe(true); }); - it('returns false when beta is disabled, even with Intacct CC Charge export configured', () => { + it('returns true when beta is disabled and QBO non-reimbursable export is Debit Card because QBO (R1) is generally available', () => { + expect(hasVendorFeature(buildQBOPolicy(CONST.QUICKBOOKS_NON_REIMBURSABLE_EXPORT_ACCOUNT_TYPE.DEBIT_CARD), false)).toBe(true); + }); + + it('returns false when beta is disabled and QBO non-reimbursable export is Vendor Bill because GA did not widen the export mode gate', () => { + expect(hasVendorFeature(buildQBOPolicy(CONST.QUICKBOOKS_NON_REIMBURSABLE_EXPORT_ACCOUNT_TYPE.VENDOR_BILL), false)).toBe(false); + }); + + it('returns false when beta is disabled and Intacct CC Charge export is configured because Intacct (R2) is still pre-GA', () => { expect(hasVendorFeature(buildIntacctPolicy(CONST.SAGE_INTACCT_NON_REIMBURSABLE_EXPENSE_TYPE.CREDIT_CARD_CHARGE), false)).toBe(false); }); - it('returns false when beta is disabled, even with Xero connected', () => { + it('returns false when beta is disabled and Xero is connected because Xero (R3) is still pre-GA', () => { expect(hasVendorFeature(buildXeroPolicy(), false)).toBe(false); }); diff --git a/tests/unit/VendorMatchingMerchantRulesTest.ts b/tests/unit/VendorMatchingMerchantRulesTest.ts index 0dcae227eaab..fe57d05b7541 100644 --- a/tests/unit/VendorMatchingMerchantRulesTest.ts +++ b/tests/unit/VendorMatchingMerchantRulesTest.ts @@ -237,8 +237,12 @@ describe('Vendor matching on merchant rules', () => { expect(hasVendorFeature(buildQBOPolicy([{id: 'v-1', name: 'Acme Co', currency: 'USD'}]), true)).toBe(true); }); - it('is hidden when the beta is off', () => { - expect(hasVendorFeature(buildQBOPolicy([{id: 'v-1', name: 'Acme Co', currency: 'USD'}]), false)).toBe(false); + it('is visible on QBO when the beta is off because QBO (R1) is generally available', () => { + expect(hasVendorFeature(buildQBOPolicy([{id: 'v-1', name: 'Acme Co', currency: 'USD'}]), false)).toBe(true); + }); + + it('is hidden on Xero when the beta is off because Xero (R3) is still pre-GA', () => { + expect(hasVendorFeature(buildXeroPolicy({xc1: {id: 'xc1', name: 'Acme Xero', email: 'acme@example.com'}}), false)).toBe(false); }); it('is hidden when no vendor integration is connected', () => { diff --git a/tests/unit/ViolationUtilsTest.ts b/tests/unit/ViolationUtilsTest.ts index 1e7cf440f632..04d76672997e 100644 --- a/tests/unit/ViolationUtilsTest.ts +++ b/tests/unit/ViolationUtilsTest.ts @@ -2892,7 +2892,7 @@ describe('getViolationsOnyxData', () => { expect(result.value).not.toContainEqual(inactiveVendorViolation); }); - it('does not add the violation when the vendorMatching beta is disabled, even with QBO configured', () => { + it('adds the violation when the vendorMatching beta is disabled but QBO is configured, because QBO (R1) is generally available', () => { isBetaEnabledSpy.mockImplementation(() => false); policy = policyWithQBOVendorFeature(); transaction.comment = {...transaction.comment, vendor: {externalID: 'v-missing', isManuallySet: true}}; @@ -2906,7 +2906,7 @@ describe('getViolationsOnyxData', () => { hasDependentTags: false, isInvoiceTransaction: false, }); - expect(result.value).not.toContainEqual(inactiveVendorViolation); + expect(result.value).toEqual(expect.arrayContaining([inactiveVendorViolation])); }); it('does not add the violation while the QBO vendor list is still hydrating (vendors undefined)', () => {