From 5abc01a47a81509afb58912191cf4e411e842ec5 Mon Sep 17 00:00:00 2001 From: Agata Kosior Date: Tue, 14 Jan 2025 17:23:19 +0100 Subject: [PATCH 1/6] fix: always send feedType with the order number for custom feeds --- src/libs/CardUtils.ts | 25 +++++++++++- src/libs/actions/CompanyCards.ts | 39 ++++++++++++++++--- .../companyCards/addNew/DetailsStep.tsx | 8 +++- src/types/onyx/CardFeeds.ts | 4 ++ 4 files changed, 69 insertions(+), 7 deletions(-) diff --git a/src/libs/CardUtils.ts b/src/libs/CardUtils.ts index d13500913c4f..c46f44e406b0 100644 --- a/src/libs/CardUtils.ts +++ b/src/libs/CardUtils.ts @@ -11,7 +11,7 @@ import type {OnyxValues} from '@src/ONYXKEYS'; import ONYXKEYS from '@src/ONYXKEYS'; import type {BankAccountList, Card, CardFeeds, CardList, CompanyCardFeed, PersonalDetailsList, WorkspaceCardsList} from '@src/types/onyx'; import type {FilteredCardList} from '@src/types/onyx/Card'; -import type {CompanyCardNicknames, CompanyFeeds, DirectCardFeedData} from '@src/types/onyx/CardFeeds'; +import type {CompanyCardFeedWithNumber, CompanyCardNicknames, CompanyFeeds, DirectCardFeedData} from '@src/types/onyx/CardFeeds'; import {isEmptyObject} from '@src/types/utils/EmptyObject'; import type IconAsset from '@src/types/utils/IconAsset'; import localeCompare from './LocaleCompare'; @@ -418,6 +418,28 @@ function getAllCardsForWorkspace(workspaceAccountID: number): CardList { return cards; } +const CUSTOM_FEEDS = [CONST.COMPANY_CARD.FEED_BANK_NAME.MASTER_CARD, CONST.COMPANY_CARD.FEED_BANK_NAME.VISA, CONST.COMPANY_CARD.FEED_BANK_NAME.AMEX]; + +function getFeedType(cardFeed: CompanyCardFeed, cardFeeds: OnyxEntry): CompanyCardFeedWithNumber { + if (CUSTOM_FEEDS.some((feed) => feed === cardFeed)) { + const filteredFeeds = Object.keys(cardFeeds?.settings?.companyCards ?? {}).filter((str) => str.includes(cardFeed)); + + const feedNumbers = filteredFeeds.map((str) => parseInt(str.replace(cardFeed, ''), 10)).filter(Boolean); + feedNumbers.sort((a, b) => a - b); + + let firstAvailableNumber = 1; + for (const num of feedNumbers) { + if (num && num !== firstAvailableNumber) { + return `${cardFeed}${firstAvailableNumber}`; + } + firstAvailableNumber++; + } + + return `${cardFeed}${firstAvailableNumber}`; + } + return cardFeed; +} + export { isExpensifyCard, isCorporateCard, @@ -448,4 +470,5 @@ export { checkIfNewFeedConnected, getDefaultCardName, getAllCardsForWorkspace, + getFeedType, }; diff --git a/src/libs/actions/CompanyCards.ts b/src/libs/actions/CompanyCards.ts index d0e19398c257..8369bf8dde25 100644 --- a/src/libs/actions/CompanyCards.ts +++ b/src/libs/actions/CompanyCards.ts @@ -1,5 +1,5 @@ +import type {OnyxEntry, OnyxUpdate} from 'react-native-onyx'; import Onyx from 'react-native-onyx'; -import type {OnyxUpdate} from 'react-native-onyx'; import * as API from '@libs/API'; import type { AssignCompanyCardParams, @@ -18,7 +18,7 @@ import * as PolicyUtils from '@libs/PolicyUtils'; import * as ReportUtils from '@libs/ReportUtils'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; -import type {Card} from '@src/types/onyx'; +import type {Card, CardFeeds} from '@src/types/onyx'; import type {AssignCard, AssignCardData} from '@src/types/onyx/AssignCard'; import type {AddNewCardFeedData, AddNewCardFeedStep, CompanyCardFeed} from '@src/types/onyx/CardFeeds'; import type {OnyxData} from '@src/types/onyx/Request'; @@ -53,18 +53,35 @@ function clearAddNewCardFlow() { }); } -function addNewCompanyCardsFeed(policyID: string, feedType: CompanyCardFeed, feedDetails: string, lastSelectedFeed?: CompanyCardFeed) { +function addNewCompanyCardsFeed(policyID: string, cardFeed: CompanyCardFeed, feedDetails: string, cardFeeds: OnyxEntry, lastSelectedFeed?: CompanyCardFeed) { const authToken = NetworkStore.getAuthToken(); + const workspaceAccountID = PolicyUtils.getWorkspaceAccountID(policyID); if (!authToken) { return; } + const feedType = CardUtils.getFeedType(cardFeed, cardFeeds); + const optimisticData: OnyxUpdate[] = [ { onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.LAST_SELECTED_FEED}${policyID}`, - value: feedType, + value: cardFeed, + }, + { + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_DOMAIN_MEMBER}${workspaceAccountID}`, + value: { + isLoading: true, + settings: { + companyCards: { + [feedType]: { + errors: null, + }, + }, + }, + }, }, ]; @@ -74,13 +91,25 @@ function addNewCompanyCardsFeed(policyID: string, feedType: CompanyCardFeed, fee key: `${ONYXKEYS.COLLECTION.LAST_SELECTED_FEED}${policyID}`, value: lastSelectedFeed ?? null, }, + { + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_DOMAIN_MEMBER}${workspaceAccountID}`, + value: { + isLoading: true, + settings: { + companyCards: { + [feedType]: null, + }, + }, + }, + }, ]; const successData: OnyxUpdate[] = [ { onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.LAST_SELECTED_FEED}${policyID}`, - value: feedType, + value: cardFeed, }, ]; diff --git a/src/pages/workspace/companyCards/addNew/DetailsStep.tsx b/src/pages/workspace/companyCards/addNew/DetailsStep.tsx index 368a1ef2a6a7..369835ac6def 100644 --- a/src/pages/workspace/companyCards/addNew/DetailsStep.tsx +++ b/src/pages/workspace/companyCards/addNew/DetailsStep.tsx @@ -15,6 +15,7 @@ import useAutoFocusInput from '@hooks/useAutoFocusInput'; import useLocalize from '@hooks/useLocalize'; import useTheme from '@hooks/useTheme'; import useThemeStyles from '@hooks/useThemeStyles'; +import * as PolicyUtils from '@libs/PolicyUtils'; import * as ValidationUtils from '@libs/ValidationUtils'; import Navigation from '@navigation/Navigation'; import variables from '@styles/variables'; @@ -34,8 +35,13 @@ function DetailsStep({policyID}: DetailsStepProps) { const theme = useTheme(); const styles = useThemeStyles(); const {inputCallbackRef} = useAutoFocusInput(); + const [addNewCard] = useOnyx(ONYXKEYS.ADD_NEW_COMPANY_CARD); const [lastSelectedFeed] = useOnyx(`${ONYXKEYS.COLLECTION.LAST_SELECTED_FEED}${policyID}`); + + const workspaceAccountID = PolicyUtils.getWorkspaceAccountID(policyID ?? '-1'); + const [cardFeeds] = useOnyx(`${ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_DOMAIN_MEMBER}${workspaceAccountID}`); + const feedProvider = addNewCard?.data?.feedType; const isStripeFeedProvider = feedProvider === CONST.COMPANY_CARD.FEED_BANK_NAME.STRIPE; const bank = addNewCard?.data?.selectedBank; @@ -53,7 +59,7 @@ function DetailsStep({policyID}: DetailsStepProps) { .map(([key, value]) => `${key}: ${value}`) .join(', '); - CompanyCards.addNewCompanyCardsFeed(policyID, addNewCard.data.feedType, feedDetails, lastSelectedFeed); + CompanyCards.addNewCompanyCardsFeed(policyID, addNewCard.data.feedType, feedDetails, cardFeeds, lastSelectedFeed); Navigation.navigate(ROUTES.WORKSPACE_COMPANY_CARDS.getRoute(policyID)); }; diff --git a/src/types/onyx/CardFeeds.ts b/src/types/onyx/CardFeeds.ts index 9ea014c5007c..fa3929ac5b6e 100644 --- a/src/types/onyx/CardFeeds.ts +++ b/src/types/onyx/CardFeeds.ts @@ -5,6 +5,9 @@ import type * as OnyxCommon from './OnyxCommon'; /** Card feed */ type CompanyCardFeed = ValueOf; +/** Custom card feed with a number */ +type CompanyCardFeedWithNumber = CompanyCardFeed | `${CompanyCardFeed}${number}`; + /** Card feed provider */ type CardFeedProvider = | typeof CONST.COMPANY_CARD.FEED_BANK_NAME.MASTER_CARD @@ -135,4 +138,5 @@ export type { CardFeedProvider, CompanyFeeds, CompanyCardNicknames, + CompanyCardFeedWithNumber, }; From 504e7a10ca16b1b21faa2a62e0bf257076302be0 Mon Sep 17 00:00:00 2001 From: Agata Kosior Date: Wed, 15 Jan 2025 10:29:13 +0100 Subject: [PATCH 2/6] fix: eslint --- .../companyCards/addNew/DetailsStep.tsx | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/pages/workspace/companyCards/addNew/DetailsStep.tsx b/src/pages/workspace/companyCards/addNew/DetailsStep.tsx index 369835ac6def..c78b15244a2a 100644 --- a/src/pages/workspace/companyCards/addNew/DetailsStep.tsx +++ b/src/pages/workspace/companyCards/addNew/DetailsStep.tsx @@ -15,11 +15,11 @@ import useAutoFocusInput from '@hooks/useAutoFocusInput'; import useLocalize from '@hooks/useLocalize'; import useTheme from '@hooks/useTheme'; import useThemeStyles from '@hooks/useThemeStyles'; -import * as PolicyUtils from '@libs/PolicyUtils'; -import * as ValidationUtils from '@libs/ValidationUtils'; +import {getWorkspaceAccountID} from '@libs/PolicyUtils'; +import {getFieldRequiredErrors} from '@libs/ValidationUtils'; import Navigation from '@navigation/Navigation'; import variables from '@styles/variables'; -import * as CompanyCards from '@userActions/CompanyCards'; +import {addNewCompanyCardsFeed, setAddNewCompanyCardStepAndData} from '@userActions/CompanyCards'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import ROUTES from '@src/ROUTES'; @@ -39,7 +39,7 @@ function DetailsStep({policyID}: DetailsStepProps) { const [addNewCard] = useOnyx(ONYXKEYS.ADD_NEW_COMPANY_CARD); const [lastSelectedFeed] = useOnyx(`${ONYXKEYS.COLLECTION.LAST_SELECTED_FEED}${policyID}`); - const workspaceAccountID = PolicyUtils.getWorkspaceAccountID(policyID ?? '-1'); + const workspaceAccountID = getWorkspaceAccountID(policyID); const [cardFeeds] = useOnyx(`${ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_DOMAIN_MEMBER}${workspaceAccountID}`); const feedProvider = addNewCard?.data?.feedType; @@ -59,21 +59,21 @@ function DetailsStep({policyID}: DetailsStepProps) { .map(([key, value]) => `${key}: ${value}`) .join(', '); - CompanyCards.addNewCompanyCardsFeed(policyID, addNewCard.data.feedType, feedDetails, cardFeeds, lastSelectedFeed); + addNewCompanyCardsFeed(policyID, addNewCard.data.feedType, feedDetails, cardFeeds, lastSelectedFeed); Navigation.navigate(ROUTES.WORKSPACE_COMPANY_CARDS.getRoute(policyID)); }; const handleBackButtonPress = () => { if (isOtherBankSelected) { - CompanyCards.setAddNewCompanyCardStepAndData({step: CONST.COMPANY_CARDS.STEP.CARD_NAME}); + setAddNewCompanyCardStepAndData({step: CONST.COMPANY_CARDS.STEP.CARD_NAME}); return; } - CompanyCards.setAddNewCompanyCardStepAndData({step: CONST.COMPANY_CARDS.STEP.CARD_INSTRUCTIONS}); + setAddNewCompanyCardStepAndData({step: CONST.COMPANY_CARDS.STEP.CARD_INSTRUCTIONS}); }; const validate = useCallback( (values: FormOnyxValues): FormInputErrors => { - const errors = ValidationUtils.getFieldRequiredErrors(values, [INPUT_IDS.BANK_ID]); + const errors = getFieldRequiredErrors(values, [INPUT_IDS.BANK_ID]); switch (feedProvider) { case CONST.COMPANY_CARD.FEED_BANK_NAME.VISA: From a6f62027bac2e15684df3ee3643a5dc31f3f7782 Mon Sep 17 00:00:00 2001 From: Agata Kosior Date: Wed, 15 Jan 2025 15:25:57 +0100 Subject: [PATCH 3/6] feat: add unit tests --- tests/unit/CardUtilsTest.ts | 56 +++++++++++++++++++++++++++++++++++-- 1 file changed, 54 insertions(+), 2 deletions(-) diff --git a/tests/unit/CardUtilsTest.ts b/tests/unit/CardUtilsTest.ts index f9b7f05dab98..451104a9147a 100644 --- a/tests/unit/CardUtilsTest.ts +++ b/tests/unit/CardUtilsTest.ts @@ -23,6 +23,22 @@ const companyCardsCustomFeedSettings = { liabilityType: 'personal', }, }; +const companyCardsCustomFeedSettingsWithNumbers = { + [`${CONST.COMPANY_CARD.FEED_BANK_NAME.MASTER_CARD}1`]: { + pending: true, + }, + [`${CONST.COMPANY_CARD.FEED_BANK_NAME.VISA}1`]: { + liabilityType: 'personal', + }, +}; +const companyCardsCustomVisaFeedSettingsWithNumbers = { + [`${CONST.COMPANY_CARD.FEED_BANK_NAME.VISA}1`]: { + pending: false, + }, + [`${CONST.COMPANY_CARD.FEED_BANK_NAME.VISA}3`]: { + pending: false, + }, +}; const companyCardsCustomFeedSettingsWithoutExpensifyBank = { [CONST.COMPANY_CARD.FEED_BANK_NAME.MASTER_CARD]: { pending: true, @@ -158,6 +174,18 @@ const cardFeedsCollection: OnyxCollection = { companyCards: companyCardsCustomFeedSettings, }, }, + // Policy with custom feeds only, feed names with numbers + FAKE_ID_4: { + settings: { + companyCards: companyCardsCustomFeedSettingsWithNumbers, + }, + }, + // Policy with several Visa feeds + FAKE_ID_5: { + settings: { + companyCards: companyCardsCustomVisaFeedSettingsWithNumbers, + }, + }, }; describe('CardUtils', () => { @@ -356,7 +384,10 @@ describe('CardUtils', () => { it('Should return filtered custom feed cards list', () => { const cardsList = CardUtils.getFilteredCardList(customFeedCardsList, undefined); // eslint-disable-next-line @typescript-eslint/naming-convention - expect(cardsList).toStrictEqual({'480801XXXXXX2111': 'ENCRYPTED_CARD_NUMBER', '480801XXXXXX2566': 'ENCRYPTED_CARD_NUMBER'}); + expect(cardsList).toStrictEqual({ + '480801XXXXXX2111': 'ENCRYPTED_CARD_NUMBER', + '480801XXXXXX2566': 'ENCRYPTED_CARD_NUMBER', + }); }); it('Should return filtered direct feed cards list with a single card', () => { @@ -368,7 +399,11 @@ describe('CardUtils', () => { it('Should return filtered direct feed cards list with multiple cards', () => { const cardsList = CardUtils.getFilteredCardList(directFeedCardsMultipleList, oAuthAccountDetails[CONST.COMPANY_CARD.FEED_BANK_NAME.CAPITAL_ONE]); // eslint-disable-next-line @typescript-eslint/naming-convention - expect(cardsList).toStrictEqual({'CREDIT CARD...1233': 'CREDIT CARD...1233', 'CREDIT CARD...3333': 'CREDIT CARD...3333', 'CREDIT CARD...7788': 'CREDIT CARD...7788'}); + expect(cardsList).toStrictEqual({ + 'CREDIT CARD...1233': 'CREDIT CARD...1233', + 'CREDIT CARD...3333': 'CREDIT CARD...3333', + 'CREDIT CARD...7788': 'CREDIT CARD...7788', + }); }); it('Should return empty object if no data was provided', () => { @@ -376,4 +411,21 @@ describe('CardUtils', () => { expect(cardsList).toStrictEqual({}); }); }); + + describe('getFeedType', () => { + it('should return the feed name with a consecutive number, if there is already a feed with a number', () => { + const feedType = CardUtils.getFeedType('vcf', cardFeedsCollection.FAKE_ID_4); + expect(feedType).toBe('vcf2'); + }); + + it('should return the feed name with 1, if there is already a feed without a number', () => { + const feedType = CardUtils.getFeedType('vcf', cardFeedsCollection.FAKE_ID_3); + expect(feedType).toBe('vcf1'); + }); + + it('should return the feed name with with the first smallest available number', () => { + const feedType = CardUtils.getFeedType('vcf', cardFeedsCollection.FAKE_ID_5); + expect(feedType).toBe('vcf2'); + }); + }); }); From 406e76d3400ce19ebc737bd8104643b999e060ab Mon Sep 17 00:00:00 2001 From: Agata Kosior Date: Wed, 15 Jan 2025 15:52:09 +0100 Subject: [PATCH 4/6] fix: apply requested changes --- src/libs/actions/CompanyCards.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libs/actions/CompanyCards.ts b/src/libs/actions/CompanyCards.ts index 8369bf8dde25..d4a905d5ef6d 100644 --- a/src/libs/actions/CompanyCards.ts +++ b/src/libs/actions/CompanyCards.ts @@ -67,7 +67,7 @@ function addNewCompanyCardsFeed(policyID: string, cardFeed: CompanyCardFeed, fee { onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.LAST_SELECTED_FEED}${policyID}`, - value: cardFeed, + value: feedType, }, { onyxMethod: Onyx.METHOD.MERGE, @@ -109,7 +109,7 @@ function addNewCompanyCardsFeed(policyID: string, cardFeed: CompanyCardFeed, fee { onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.LAST_SELECTED_FEED}${policyID}`, - value: cardFeed, + value: feedType, }, ]; From 792c76ce4cd47dc52ef1e979b5602da170199a40 Mon Sep 17 00:00:00 2001 From: Agata Kosior Date: Wed, 15 Jan 2025 15:54:47 +0100 Subject: [PATCH 5/6] fix: eslint --- tests/unit/CardUtilsTest.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/unit/CardUtilsTest.ts b/tests/unit/CardUtilsTest.ts index 451104a9147a..0ac8de13c8c8 100644 --- a/tests/unit/CardUtilsTest.ts +++ b/tests/unit/CardUtilsTest.ts @@ -383,9 +383,10 @@ describe('CardUtils', () => { describe('getFilteredCardList', () => { it('Should return filtered custom feed cards list', () => { const cardsList = CardUtils.getFilteredCardList(customFeedCardsList, undefined); - // eslint-disable-next-line @typescript-eslint/naming-convention expect(cardsList).toStrictEqual({ + // eslint-disable-next-line @typescript-eslint/naming-convention '480801XXXXXX2111': 'ENCRYPTED_CARD_NUMBER', + // eslint-disable-next-line @typescript-eslint/naming-convention '480801XXXXXX2566': 'ENCRYPTED_CARD_NUMBER', }); }); @@ -398,10 +399,12 @@ describe('CardUtils', () => { it('Should return filtered direct feed cards list with multiple cards', () => { const cardsList = CardUtils.getFilteredCardList(directFeedCardsMultipleList, oAuthAccountDetails[CONST.COMPANY_CARD.FEED_BANK_NAME.CAPITAL_ONE]); - // eslint-disable-next-line @typescript-eslint/naming-convention expect(cardsList).toStrictEqual({ + // eslint-disable-next-line @typescript-eslint/naming-convention 'CREDIT CARD...1233': 'CREDIT CARD...1233', + // eslint-disable-next-line @typescript-eslint/naming-convention 'CREDIT CARD...3333': 'CREDIT CARD...3333', + // eslint-disable-next-line @typescript-eslint/naming-convention 'CREDIT CARD...7788': 'CREDIT CARD...7788', }); }); From 8f074e74375e94ade3a26e4cd37b3bb1fd506fd7 Mon Sep 17 00:00:00 2001 From: Agata Kosior Date: Wed, 15 Jan 2025 18:26:40 +0100 Subject: [PATCH 6/6] fix: apply requested changes --- src/libs/CardUtils.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/libs/CardUtils.ts b/src/libs/CardUtils.ts index d853fb8d753d..6ef61c5cfd2d 100644 --- a/src/libs/CardUtils.ts +++ b/src/libs/CardUtils.ts @@ -453,24 +453,24 @@ const getDescriptionForPolicyDomainCard = (domainName: string): string => { const CUSTOM_FEEDS = [CONST.COMPANY_CARD.FEED_BANK_NAME.MASTER_CARD, CONST.COMPANY_CARD.FEED_BANK_NAME.VISA, CONST.COMPANY_CARD.FEED_BANK_NAME.AMEX]; -function getFeedType(cardFeed: CompanyCardFeed, cardFeeds: OnyxEntry): CompanyCardFeedWithNumber { - if (CUSTOM_FEEDS.some((feed) => feed === cardFeed)) { - const filteredFeeds = Object.keys(cardFeeds?.settings?.companyCards ?? {}).filter((str) => str.includes(cardFeed)); +function getFeedType(feedKey: CompanyCardFeed, cardFeeds: OnyxEntry): CompanyCardFeedWithNumber { + if (CUSTOM_FEEDS.some((feed) => feed === feedKey)) { + const filteredFeeds = Object.keys(cardFeeds?.settings?.companyCards ?? {}).filter((str) => str.includes(feedKey)); - const feedNumbers = filteredFeeds.map((str) => parseInt(str.replace(cardFeed, ''), 10)).filter(Boolean); + const feedNumbers = filteredFeeds.map((str) => parseInt(str.replace(feedKey, ''), 10)).filter(Boolean); feedNumbers.sort((a, b) => a - b); let firstAvailableNumber = 1; for (const num of feedNumbers) { if (num && num !== firstAvailableNumber) { - return `${cardFeed}${firstAvailableNumber}`; + return `${feedKey}${firstAvailableNumber}`; } firstAvailableNumber++; } - return `${cardFeed}${firstAvailableNumber}`; + return `${feedKey}${firstAvailableNumber}`; } - return cardFeed; + return feedKey; } export {