Merge pull request Expensify#77117 from allgandalf/allowAddingFeedForCollectCSV

MariaHCD · web-flow · commit a0bb986e6cca · 2026-01-08T10:33:11.000+04:00
[Internal QA] Don't count CSV uploads in Classic as a feed in ND
diff --git a/src/hooks/useIsBlockedToAddFeed.ts b/src/hooks/useIsBlockedToAddFeed.ts
@@ -1,5 +1,5 @@
-import {useEffect, useState} from 'react';
-import {getCompanyFeeds} from '@libs/CardUtils';
+import {useMemo} from 'react';
+import {getCompanyFeeds, isCSVFeedOrExpensifyCard} from '@libs/CardUtils';
 import {isCollectPolicy} from '@libs/PolicyUtils';
 import isLoadingOnyxValue from '@src/types/utils/isLoadingOnyxValue';
 import useCardFeeds from './useCardFeeds';
@@ -10,6 +10,7 @@ import usePolicy from './usePolicy';
  *
  * Collect plan workspaces are limited to one company card feed. This hook checks if the workspace already has
  * a feed and returns whether users should be blocked from adding more feeds.
+ * CSV uploads from Classic and Expensify Cards should not count toward this limit.
  *
  * @param policyID - The ID of the workspace/policy to check
  * @returns An object containing:
@@ -19,24 +20,27 @@ import usePolicy from './usePolicy';
 function useIsBlockedToAddFeed(policyID?: string) {
     const policy = usePolicy(policyID);
     const [cardFeeds, allFeedsResult, defaultFeed] = useCardFeeds(policyID);
-    const companyFeeds = getCompanyFeeds(cardFeeds, true);
+    // Include pending feeds in the count to prevent users from adding multiple feeds
+    // Pending feeds count toward the limit because the backend checks before adding
+    const companyFeeds = getCompanyFeeds(cardFeeds, true, false);
     const isCollect = isCollectPolicy(policy);
     const isAllFeedsResultLoading = isLoadingOnyxValue(allFeedsResult);
-    const [prevCompanyFeedsLength, setPrevCompanyFeedsLength] = useState(0);
 
     const isLoading = !cardFeeds || !!defaultFeed?.isLoading;
 
-    useEffect(() => {
+    // Count feeds excluding CSV uploads from Classic and Expensify Cards
+    // Include pending feeds in the count to enforce the limit
+    const connectedFeedsCount = useMemo(() => {
         if (isLoading) {
-            return;
+            return 0;
         }
-        const connectedFeeds = Object.entries(companyFeeds)?.length;
-        setPrevCompanyFeedsLength(connectedFeeds);
-        // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps -- we don't want this effect to run again
-    }, [isLoading]);
+        const feeds = companyFeeds ?? {};
+        const nonCSVFeeds = Object.keys(feeds).filter((feedKey) => !isCSVFeedOrExpensifyCard(feedKey));
+        return nonCSVFeeds.length;
+    }, [isLoading, companyFeeds]);
 
     return {
-        isBlockedToAddNewFeeds: isCollect && !isLoading && prevCompanyFeedsLength >= 1,
+        isBlockedToAddNewFeeds: isCollect && !isLoading && connectedFeedsCount >= 1,
         isAllFeedsResultLoading: isCollect && (isLoading || isAllFeedsResultLoading),
     };
 }
diff --git a/src/libs/CardUtils.ts b/src/libs/CardUtils.ts
@@ -397,6 +397,21 @@ function isCustomFeed(feed: CompanyCardFeedWithNumber | undefined): boolean {
     return [CONST.COMPANY_CARD.FEED_BANK_NAME.MASTER_CARD, CONST.COMPANY_CARD.FEED_BANK_NAME.VISA, CONST.COMPANY_CARD.FEED_BANK_NAME.AMEX].some((value) => feed.startsWith(value));
 }
 
+/**
+ * Checks if a feed key represents a CSV feed or Expensify Card.
+ * CSV feeds include feeds starting with "csv" or "ccupload", or containing "ccupload".
+ * Expensify Cards also don't count toward feed limits.
+ *
+ * @param feedKey - The feed key to check
+ * @returns true if the feed is a CSV feed or Expensify Card, false otherwise
+ */
+function isCSVFeedOrExpensifyCard(feedKey: string): boolean {
+    const lowerFeedKey = feedKey.toLowerCase();
+    // Exclude CSV feeds (feed types starting with "csv" or "ccupload", or containing "ccupload")
+    // Also exclude Expensify Cards which don't count toward the limit
+    return lowerFeedKey.startsWith('csv') || lowerFeedKey.startsWith('ccupload') || feedKey.includes(CONST.COMPANY_CARD.FEED_BANK_NAME.CSV) || feedKey === 'Expensify Card';
+}
+
 function getOriginalCompanyFeeds(cardFeeds: OnyxEntry<CardFeeds>): CompanyFeeds {
     return Object.fromEntries(
         Object.entries(cardFeeds?.settings?.companyCards ?? {}).filter(([key, value]) => {
@@ -922,6 +937,7 @@ export {
     isSelectedFeedExpired,
     getCompanyFeeds,
     isCustomFeed,
+    isCSVFeedOrExpensifyCard,
     getBankCardDetailsImage,
     getSelectedFeed,
     getPlaidCountry,
diff --git a/src/pages/workspace/companyCards/BankConnection/index.tsx b/src/pages/workspace/companyCards/BankConnection/index.tsx
@@ -84,13 +84,16 @@ function BankConnection({policyID: policyIDFromProps, feed, route}: BankConnecti
     }, [url]);
 
     useEffect(() => {
-        if (!policyID || !isBlockedToAddNewFeeds) {
+        // Only redirect if blocked AND we're not in the middle of adding a feed (i.e., no feed selected and not a new feed)
+        // If feed is defined, user is assigning cards to an existing feed, so don't redirect
+        // If isNewFeedConnected is true, user just successfully added a feed, so don't redirect
+        if (!policyID || !isBlockedToAddNewFeeds || feed || isNewFeedConnected) {
             return;
         }
         Navigation.navigate(ROUTES.WORKSPACE_UPGRADE.getRoute(policyID, CONST.UPGRADE_FEATURE_INTRO_MAPPING.companyCards.alias, ROUTES.WORKSPACE_COMPANY_CARDS.getRoute(policyID)), {
             forceReplace: true,
         });
-    }, [isBlockedToAddNewFeeds, policyID]);
+    }, [isBlockedToAddNewFeeds, policyID, feed, isNewFeedConnected]);
 
     const handleBackButtonPress = () => {
         customWindow?.close();
diff --git a/src/pages/workspace/companyCards/addNew/AddNewCardPage.tsx b/src/pages/workspace/companyCards/addNew/AddNewCardPage.tsx
@@ -47,13 +47,17 @@ function AddNewCardPage({policy}: WithPolicyAndFullscreenLoadingProps) {
     const isAddCardFeedLoading = isLoadingOnyxValue(addNewCardFeedMetadata);
 
     useEffect(() => {
-        if (!policyID || !isBlockedToAddNewFeeds) {
+        // Only redirect if blocked AND user is trying to start a new flow (currentStep is SELECT_BANK or undefined/initial state)
+        // Don't redirect if user is already in the middle of adding a feed (other steps)
+        // Don't redirect if user just successfully added a feed (isNewFeedConnected would be true in BankConnection)
+        const isInitialStep = !currentStep || currentStep === CONST.COMPANY_CARDS.STEP.SELECT_BANK;
+        if (!policyID || !isBlockedToAddNewFeeds || !isInitialStep) {
             return;
         }
         Navigation.navigate(ROUTES.WORKSPACE_UPGRADE.getRoute(policyID, CONST.UPGRADE_FEATURE_INTRO_MAPPING.companyCards.alias, ROUTES.WORKSPACE_COMPANY_CARDS.getRoute(policyID)), {
             forceReplace: true,
         });
-    }, [isBlockedToAddNewFeeds, policyID]);
+    }, [isBlockedToAddNewFeeds, policyID, currentStep]);
 
     useEffect(() => {
         return () => {
diff --git a/tests/unit/CardUtilsTest.ts b/tests/unit/CardUtilsTest.ts
@@ -32,6 +32,7 @@ import {
     getSelectedFeed,
     getYearFromExpirationDateString,
     hasIssuedExpensifyCard,
+    isCSVFeedOrExpensifyCard,
     isCustomFeed as isCustomFeedCardUtils,
     isExpensifyCard,
     isExpensifyCardFullySetUp,
@@ -499,6 +500,63 @@ describe('CardUtils', () => {
         });
     });
 
+    describe('isCSVFeedOrExpensifyCard', () => {
+        it('Should return true for feed key starting with "csv" (lowercase)', () => {
+            expect(isCSVFeedOrExpensifyCard('csv123')).toBe(true);
+        });
+
+        it('Should return true for feed key starting with "CSV" (uppercase)', () => {
+            expect(isCSVFeedOrExpensifyCard('CSV123')).toBe(true);
+        });
+
+        it('Should return true for feed key starting with "Csv" (mixed case)', () => {
+            expect(isCSVFeedOrExpensifyCard('Csv123')).toBe(true);
+        });
+
+        it('Should return true for feed key starting with "ccupload" (lowercase)', () => {
+            expect(isCSVFeedOrExpensifyCard('ccupload123')).toBe(true);
+        });
+
+        it('Should return true for feed key starting with "CCUPLOAD" (uppercase)', () => {
+            expect(isCSVFeedOrExpensifyCard('CCUPLOAD123')).toBe(true);
+        });
+
+        it('Should return true for feed key starting with "Ccupload" (mixed case)', () => {
+            expect(isCSVFeedOrExpensifyCard('Ccupload123')).toBe(true);
+        });
+
+        it('Should return true for feed key containing "ccupload"', () => {
+            expect(isCSVFeedOrExpensifyCard('prefix-ccupload-suffix')).toBe(true);
+        });
+
+        it('Should return true for feed key containing CONST.COMPANY_CARD.FEED_BANK_NAME.CSV', () => {
+            expect(isCSVFeedOrExpensifyCard(`prefix-${CONST.COMPANY_CARD.FEED_BANK_NAME.CSV}-suffix`)).toBe(true);
+        });
+
+        it('Should return true for "Expensify Card" feed key', () => {
+            expect(isCSVFeedOrExpensifyCard('Expensify Card')).toBe(true);
+        });
+
+        it('Should return false for regular feed keys', () => {
+            expect(isCSVFeedOrExpensifyCard(CONST.COMPANY_CARD.FEED_BANK_NAME.VISA)).toBe(false);
+            expect(isCSVFeedOrExpensifyCard(CONST.COMPANY_CARD.FEED_BANK_NAME.MASTER_CARD)).toBe(false);
+            expect(isCSVFeedOrExpensifyCard(CONST.COMPANY_CARD.FEED_BANK_NAME.AMEX)).toBe(false);
+            expect(isCSVFeedOrExpensifyCard(CONST.COMPANY_CARD.FEED_BANK_NAME.CHASE)).toBe(false);
+        });
+
+        it('Should return false for feed keys that contain "csv" but do not start with it', () => {
+            expect(isCSVFeedOrExpensifyCard('vcf-csv-feed')).toBe(false);
+        });
+
+        it('Should return true for feed keys that contain "ccupload" anywhere (not just at start)', () => {
+            expect(isCSVFeedOrExpensifyCard('prefix-ccupload-suffix')).toBe(true);
+        });
+
+        it('Should return false for empty string', () => {
+            expect(isCSVFeedOrExpensifyCard('')).toBe(false);
+        });
+    });
+
     describe('getOriginalCompanyFeeds', () => {
         it('Should return both custom and direct feeds with filtered out "Expensify Card" bank', () => {
             const companyFeeds = getOriginalCompanyFeeds(cardFeedsCollection.FAKE_ID_1);
diff --git a/tests/unit/hooks/useIsBlockedToAddFeed.test.ts b/tests/unit/hooks/useIsBlockedToAddFeed.test.ts
@@ -8,11 +8,6 @@ import createRandomPolicy from '../../utils/collections/policies';
 
 const mockPolicyID = '123456';
 
-const delay = (ms: number) =>
-    new Promise((resolve) => {
-        setTimeout(resolve, ms);
-    });
-
 const mockPolicy = {...createRandomPolicy(Number(mockPolicyID), CONST.POLICY.TYPE.TEAM, 'TestPolicy'), policyID: mockPolicyID, workspaceAccountID: Number(mockPolicyID)};
 
 const mockCardFeeds = {
@@ -41,8 +36,14 @@ jest.mock('@hooks/useCardFeeds', () => ({
 }));
 describe('useIsBlockedToAddFeed', () => {
     beforeEach(async () => {
+        await Onyx.clear();
         await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${mockPolicy?.policyID}`, mockPolicy);
     });
+
+    afterEach(async () => {
+        await Onyx.clear();
+        jest.clearAllMocks();
+    });
     it('should return true if collect policy and feed already exists', () => {
         (useCardFeeds as jest.Mock).mockReturnValue([mockCardFeeds, {status: 'loaded'}]);
         const {result} = renderHook(() => useIsBlockedToAddFeed(mockPolicyID));
@@ -56,26 +57,21 @@ describe('useIsBlockedToAddFeed', () => {
         expect(result?.current.isBlockedToAddNewFeeds).toBe(false);
     });
 
-    it('should return isBlockedToAddNewFeeds as false if collect policy and new feed added', async () => {
-        (useCardFeeds as jest.Mock).mockReturnValue([{}, {status: 'loaded'}]);
-        const {result, rerender} = renderHook(() => useIsBlockedToAddFeed(mockPolicyID));
-        expect(result.current.isBlockedToAddNewFeeds).toBe(false);
-        // Set initial empty state and wait for new connection to be established
-        await delay(2000);
+    it('should return isBlockedToAddNewFeeds as false if collect policy and CSV feed exists (allows adding another feed)', async () => {
         (useCardFeeds as jest.Mock).mockReturnValue([
             {
-                ins: {
-                    feed: 'ins',
-                    customFeedName: 'Regions Bank cards',
-                    accountList: ['Plaid Checking 0000', 'Plaid Credit Card 3333'],
+                // eslint-disable-next-line @typescript-eslint/naming-convention
+                'csv#123456': {
+                    feed: 'csv#123456',
+                    customFeedName: 'CSV Upload',
+                    accountList: ['Card 0000'],
                 },
             },
             {status: 'loaded'},
         ]);
-        // Wait to set state happened
-        await delay(2000);
+        const {result} = renderHook(() => useIsBlockedToAddFeed(mockPolicyID));
 
-        rerender(mockPolicyID);
+        // CSV feeds don't count toward the limit, so user can add another feed
         expect(result.current.isBlockedToAddNewFeeds).toBe(false);
     });
 
@@ -125,7 +121,90 @@ describe('useIsBlockedToAddFeed', () => {
         expect(result.current.isBlockedToAddNewFeeds).toBe(false);
 
         (useCardFeeds as jest.Mock).mockReturnValue([mockCardFeeds, {status: 'loaded'}, {isLoading: false}]);
-        rerender({policyID: mockPolicyID});
+        rerender(mockPolicyID);
+        expect(result.current.isBlockedToAddNewFeeds).toBe(true);
+    });
+
+    it('should return true if collect policy and pending feed exists (pending feeds count toward limit)', () => {
+        (useCardFeeds as jest.Mock).mockReturnValue([
+            {
+                // eslint-disable-next-line @typescript-eslint/naming-convention
+                'plaid.ins_19#123456': {
+                    feed: 'plaid.ins_19',
+                    domainID: 123456,
+                    pending: true,
+                    liabilityType: 'corporate',
+                },
+            },
+            {status: 'loaded'},
+        ]);
+        const {result} = renderHook(() => useIsBlockedToAddFeed(mockPolicyID));
+
+        // Pending feeds count toward the limit, so user should be blocked from adding another feed
+        expect(result.current.isBlockedToAddNewFeeds).toBe(true);
+    });
+
+    it('should return false if collect policy and pending CSV feed exists (pending CSV feeds do not count toward limit)', () => {
+        (useCardFeeds as jest.Mock).mockReturnValue([
+            {
+                // eslint-disable-next-line @typescript-eslint/naming-convention
+                'csv#123456': {
+                    feed: 'csv#123456',
+                    customFeedName: 'CSV Upload',
+                    accountList: ['Card 0000'],
+                    pending: true,
+                },
+            },
+            {status: 'loaded'},
+        ]);
+        const {result} = renderHook(() => useIsBlockedToAddFeed(mockPolicyID));
+
+        // Pending CSV feeds don't count toward the limit, so user can add another feed
+        expect(result.current.isBlockedToAddNewFeeds).toBe(false);
+    });
+
+    it('should return true if collect policy has both regular feed and pending feed', () => {
+        (useCardFeeds as jest.Mock).mockReturnValue([
+            {
+                // eslint-disable-next-line @typescript-eslint/naming-convention
+                'plaid.ins_19#123456': {
+                    feed: 'plaid.ins_19',
+                    domainID: 123456,
+                    pending: false,
+                    liabilityType: 'corporate',
+                },
+                // eslint-disable-next-line @typescript-eslint/naming-convention
+                'oauth.chase.com#123456': {
+                    feed: 'oauth.chase.com',
+                    domainID: 123456,
+                    pending: true,
+                    liabilityType: 'corporate',
+                },
+            },
+            {status: 'loaded'},
+        ]);
+        const {result} = renderHook(() => useIsBlockedToAddFeed(mockPolicyID));
+
+        // Both regular and pending feeds count, so user should be blocked
+        expect(result.current.isBlockedToAddNewFeeds).toBe(true);
+    });
+
+    it('should return true if collect policy has only pending feed (no regular feeds)', () => {
+        (useCardFeeds as jest.Mock).mockReturnValue([
+            {
+                // eslint-disable-next-line @typescript-eslint/naming-convention
+                'oauth.chase.com#123456': {
+                    feed: 'oauth.chase.com',
+                    domainID: 123456,
+                    pending: true,
+                    liabilityType: 'corporate',
+                },
+            },
+            {status: 'loaded'},
+        ]);
+        const {result} = renderHook(() => useIsBlockedToAddFeed(mockPolicyID));
+
+        // Pending feeds count toward the limit, so even with only a pending feed, user should be blocked
         expect(result.current.isBlockedToAddNewFeeds).toBe(true);
     });
 });
