Merge pull request Expensify#81363 from Expensify/vit-fixShippingCard

[Bug] Fix RHP not closing after shipping Expensify Card

Author: MariaHCD

src/pages/MissingPersonalDetails/MissingPersonalDetailsMagicCodePage.tsx

@@ -1,12 +1,11 @@
+import {areAllExpensifyCardsShipped} from '@selectors/Card';
 import React, {useCallback, useEffect, useMemo} from 'react';
-import type {OnyxEntry} from 'react-native-onyx';
 import ValidateCodeActionContent from '@components/ValidateCodeActionModal/ValidateCodeActionContent';
 import useLocalize from '@hooks/useLocalize';
 import useOnyx from '@hooks/useOnyx';
 import {clearDraftValues} from '@libs/actions/FormActions';
 import {clearPersonalDetailsErrors, updatePersonalDetailsAndShipExpensifyCards} from '@libs/actions/PersonalDetails';
 import {requestValidateCodeAction, resetValidateActionCodeSent} from '@libs/actions/User';
-import {isPersonalCard} from '@libs/CardUtils';
 import {normalizeCountryCode} from '@libs/CountryUtils';
 import {getLatestError} from '@libs/ErrorUtils';
 import Navigation from '@libs/Navigation/Navigation';
@@ -16,20 +15,16 @@ import ONYXKEYS from '@src/ONYXKEYS';
 import ROUTES from '@src/ROUTES';
 import {primaryLoginSelector} from '@src/selectors/Account';
 import type {PersonalDetailsForm} from '@src/types/form';
-import type {CardList} from '@src/types/onyx';
 import {isEmptyObject} from '@src/types/utils/EmptyObject';
 import {getSubPageValues} from './utils';
 
-const areAllCardsShippedSelector = (cardList: OnyxEntry<CardList>) =>
-    Object.values(cardList ?? {})?.every((card) => card?.state !== CONST.EXPENSIFY_CARD.STATE.STATE_NOT_ISSUED && !isPersonalCard(card));
-
 function MissingPersonalDetailsMagicCodePage() {
     const {translate} = useLocalize();
     const [privatePersonalDetails] = useOnyx(ONYXKEYS.PRIVATE_PERSONAL_DETAILS, {canBeMissing: false});
     const [draftValues] = useOnyx(ONYXKEYS.FORMS.PERSONAL_DETAILS_FORM_DRAFT, {canBeMissing: false});
     const [countryCode = CONST.DEFAULT_COUNTRY_CODE] = useOnyx(ONYXKEYS.COUNTRY_CODE, {canBeMissing: false});
 
-    const [areAllCardsShipped] = useOnyx(ONYXKEYS.CARD_LIST, {selector: areAllCardsShippedSelector, canBeMissing: true});
+    const [areAllCardsShipped] = useOnyx(ONYXKEYS.CARD_LIST, {selector: areAllExpensifyCardsShipped, canBeMissing: true});
     const [primaryLogin] = useOnyx(ONYXKEYS.ACCOUNT, {selector: primaryLoginSelector, canBeMissing: true});
 
     const [validateCodeAction] = useOnyx(ONYXKEYS.VALIDATE_ACTION_CODE, {canBeMissing: true});

src/selectors/Card.ts

@@ -1,8 +1,9 @@
 import type {OnyxEntry} from 'react-native-onyx';
 import type {LocalizedTranslate} from '@components/LocaleContextProvider';
 import {getCardFeedsForDisplay} from '@libs/CardFeedUtils';
-import {isCard, isCardHiddenFromSearch, isPersonalCard} from '@libs/CardUtils';
+import {isCard, isCardHiddenFromSearch, isExpensifyCard, isPersonalCard} from '@libs/CardUtils';
 import {filterObject} from '@libs/ObjectUtils';
+import CONST from '@src/CONST';
 import type {CardList, NonPersonalAndWorkspaceCardListDerivedValue} from '@src/types/onyx';
 
 /**
@@ -42,4 +43,14 @@ const defaultExpensifyCardSelector = (allCards: OnyxEntry<NonPersonalAndWorkspac
  */
 const cardByIdSelector = (cardID: string) => (cardList: OnyxEntry<CardList>) => cardList?.[cardID];
 
-export {filterCardsHiddenFromSearch, filterOutPersonalCards, defaultExpensifyCardSelector, cardByIdSelector};
+/**
+ * Checks if all Expensify cards have been shipped (state is not STATE_NOT_ISSUED).
+ * Only considers valid Expensify cards - ignores personal cards, company cards, and invalid entries.
+ * Returns true if there are no Expensify cards pending issue, or if there are no Expensify cards at all.
+ */
+const areAllExpensifyCardsShipped = (cardList: OnyxEntry<CardList>): boolean =>
+    Object.values(cardList ?? {})
+        .filter((card) => isCard(card) && isExpensifyCard(card))
+        .every((card) => card.state !== CONST.EXPENSIFY_CARD.STATE.STATE_NOT_ISSUED);
+
+export {filterCardsHiddenFromSearch, filterOutPersonalCards, defaultExpensifyCardSelector, cardByIdSelector, areAllExpensifyCardsShipped};

tests/unit/selectors/CardTest.ts

@@ -1,5 +1,5 @@
 /* eslint-disable @typescript-eslint/naming-convention */
-import {defaultExpensifyCardSelector, filterCardsHiddenFromSearch, filterOutPersonalCards} from '@selectors/Card';
+import {areAllExpensifyCardsShipped, defaultExpensifyCardSelector, filterCardsHiddenFromSearch, filterOutPersonalCards} from '@selectors/Card';
 import type {ValueOf} from 'type-fest';
 import CONST from '@src/CONST';
 import type {Card, CardList} from '@src/types/onyx';
@@ -189,6 +189,85 @@ describe('defaultExpensifyCardSelector', () => {
     });
 });
 
+describe('areAllExpensifyCardsShipped', () => {
+    it('returns true when cardList is undefined or empty', () => {
+        expect(areAllExpensifyCardsShipped(undefined)).toBe(true);
+        expect(areAllExpensifyCardsShipped({})).toBe(true);
+    });
+
+    it('returns true when all Expensify cards are shipped (not STATE_NOT_ISSUED)', () => {
+        const cardList: CardList = {
+            '1': createRandomExpensifyCard(1, {state: CONST.EXPENSIFY_CARD.STATE.NOT_ACTIVATED}),
+            '2': createRandomExpensifyCard(2, {state: CONST.EXPENSIFY_CARD.STATE.OPEN}),
+        };
+        expect(areAllExpensifyCardsShipped(cardList)).toBe(true);
+    });
+
+    it('returns false when any Expensify card is in STATE_NOT_ISSUED', () => {
+        const cardList: CardList = {
+            '1': createRandomExpensifyCard(1, {state: CONST.EXPENSIFY_CARD.STATE.OPEN}),
+            '2': createRandomExpensifyCard(2, {state: CONST.EXPENSIFY_CARD.STATE.STATE_NOT_ISSUED}),
+        };
+        expect(areAllExpensifyCardsShipped(cardList)).toBe(false);
+    });
+
+    // CRITICAL: This test ensures the personal cards should not affect the result
+    it('returns true when Expensify cards are shipped even if user has personal cards', () => {
+        const personalCard = createRandomCard(1, {bank: CONST.PERSONAL_CARD.BANK_NAME.CSV});
+        const expensifyCard = createRandomExpensifyCard(2, {state: CONST.EXPENSIFY_CARD.STATE.NOT_ACTIVATED});
+
+        const cardList: CardList = {
+            '1': personalCard,
+            '2': expensifyCard,
+        };
+        expect(areAllExpensifyCardsShipped(cardList)).toBe(true);
+    });
+
+    // CRITICAL: This test ensures the company cards should not affect the result
+    it('returns true when Expensify cards are shipped even if user has company cards', () => {
+        const companyCard = createRandomCompanyCard(1, {bank: 'vcf'});
+        const expensifyCard = createRandomExpensifyCard(2, {state: CONST.EXPENSIFY_CARD.STATE.OPEN});
+
+        const cardList: CardList = {
+            '1': companyCard,
+            '2': expensifyCard,
+        };
+        expect(areAllExpensifyCardsShipped(cardList)).toBe(true);
+    });
+
+    it('ignores invalid card entries (missing cardID or bank)', () => {
+        const validCard = createRandomExpensifyCard(1, {state: CONST.EXPENSIFY_CARD.STATE.OPEN});
+        const invalidCard = {cardID: 2} as Card; // Missing bank
+
+        const cardList: CardList = {
+            '1': validCard,
+            '2': invalidCard,
+        };
+        expect(areAllExpensifyCardsShipped(cardList)).toBe(true);
+    });
+
+    it('returns true when only non-Expensify cards exist', () => {
+        const cardList: CardList = {
+            '1': createRandomCompanyCard(1, {bank: 'vcf'}),
+            '2': createRandomCard(2, {bank: CONST.PERSONAL_CARD.BANK_NAME.CSV}),
+        };
+        expect(areAllExpensifyCardsShipped(cardList)).toBe(true);
+    });
+
+    it('returns false when mixing shipped and unshipped Expensify cards with other card types', () => {
+        const companyCard = createRandomCompanyCard(1, {bank: 'vcf'});
+        const shippedExpensifyCard = createRandomExpensifyCard(2, {state: CONST.EXPENSIFY_CARD.STATE.OPEN});
+        const unshippedExpensifyCard = createRandomExpensifyCard(3, {state: CONST.EXPENSIFY_CARD.STATE.STATE_NOT_ISSUED});
+
+        const cardList: CardList = {
+            '1': companyCard,
+            '2': shippedExpensifyCard,
+            '3': unshippedExpensifyCard,
+        };
+        expect(areAllExpensifyCardsShipped(cardList)).toBe(false);
+    });
+});
+
 describe('filterOutPersonalCards', () => {
     it('should return only cards with a valid fundID', () => {
         const cardList: CardList = {