diff --git a/src/hooks/useOtherFeedsForFeedSelector.tsx b/src/hooks/useOtherFeedsForFeedSelector.tsx index 77296a08b224..10516a5d2a31 100644 --- a/src/hooks/useOtherFeedsForFeedSelector.tsx +++ b/src/hooks/useOtherFeedsForFeedSelector.tsx @@ -27,6 +27,8 @@ import useThemeStyles from './useThemeStyles'; type CardFeedListItem = ListItem & { /** Combined feed key */ value: CompanyCardFeedWithDomainID; + /** Key the feed has in the active policy's feed list (`${feed}${separator}${fundID}`), set for feeds from other workspaces. Undefined when the fundID is missing. */ + selectionValue?: CompanyCardFeedWithDomainID; /** Card feed value */ feed: CompanyCardFeedWithNumber; fundID?: number; @@ -62,14 +64,16 @@ function useOtherFeedsForFeedSelector(policyID: string): CardFeedListItem[] { if (feed?.linkedPolicyIDs?.filter(Boolean).some((linkedPolicyID) => linkedPolicyID.toUpperCase() === upperPolicyID)) { continue; } + const feedName = feed.feed as CompanyCardFeedWithNumber; + const fundID = Number(feed.fundID); + + // The key this feed gets in the active policy's available list (`${feed}${separator}${fundID}`). Without a valid fundID there is no such key. + const feedValueForActivePolicy = fundID ? getCardFeedWithDomainID(feedName, fundID) : undefined; + // Skip feeds already present in the active policy's available list to avoid duplicate rows across the two lists. - // `companyCardFeeds` is keyed at runtime by the domain-ID-suffixed feed value (`${feed}${separator}${fundID}`), - // so a membership check on that value avoids an unsafe assertion to the narrower `CompanyCardFeedWithNumber` key type. - const feedValueForActivePolicy = getCardFeedWithDomainID(feed.feed, Number(feed.fundID)); - if (companyCardFeeds && feedValueForActivePolicy in companyCardFeeds) { + if (feedValueForActivePolicy && companyCardFeeds && feedValueForActivePolicy in companyCardFeeds) { continue; } - const feedName = feed.feed; const plaidUrl = getPlaidInstitutionIconUrl(feedName); // Use the same fundID-aware domain lookup as the visibility logic (`getVisibleCompanyCardFeedsForSelector`) // so the domain email resolves even when the domains collection isn't keyed by fundID. @@ -82,8 +86,9 @@ function useOtherFeedsForFeedSelector(policyID: string): CardFeedListItem[] { otherPolicyFeeds.push({ value: feed.id as CompanyCardFeedWithDomainID, - feed: feedName as CompanyCardFeedWithNumber, - fundID: Number(feed.fundID), + selectionValue: feedValueForActivePolicy, + feed: feedName, + fundID, country: feed?.country, alternateText: domainName ?? linkedPolicy?.name ?? firstLinkedPolicyID, text: getCustomOrFormattedFeedName(translate, feedName, feed.name), diff --git a/src/pages/workspace/companyCards/WorkspaceCompanyCardFeedSelectorPage.tsx b/src/pages/workspace/companyCards/WorkspaceCompanyCardFeedSelectorPage.tsx index 9b99c9beb8b7..aab6c17dded6 100644 --- a/src/pages/workspace/companyCards/WorkspaceCompanyCardFeedSelectorPage.tsx +++ b/src/pages/workspace/companyCards/WorkspaceCompanyCardFeedSelectorPage.tsx @@ -77,9 +77,14 @@ function WorkspaceCompanyCardFeedSelectorPage({route}: WorkspaceCompanyCardFeedS const icons = useMemoizedLazyExpensifyIcons(['Plus']); const [feedWithError, setFeedWithError] = useState<{feed?: string; error?: Errors} | undefined>(undefined); - const {companyCardFeeds, feedName: selectedFeedName} = useCompanyCards({policyID}); + const {companyCardFeeds: liveCompanyCardFeeds, feedName: selectedFeedName} = useCompanyCards({policyID}); const {shouldShowRbrForFeedNameWithDomainID} = useCardFeedErrors(); - const otherFeeds = useOtherFeedsForFeedSelector(policyID); + const liveOtherFeeds = useOtherFeedsForFeedSelector(policyID); + + // The link response reaches Onyx before the RHP starts closing, so the lists are frozen while linking to keep the feed from jumping between sections. + const [frozenFeeds, setFrozenFeeds] = useState<{companyCardFeeds: typeof liveCompanyCardFeeds; otherFeeds: CardFeedListItem[]}>(); + const companyCardFeeds = frozenFeeds ? frozenFeeds.companyCardFeeds : liveCompanyCardFeeds; + const otherFeeds = frozenFeeds?.otherFeeds ?? liveOtherFeeds; const primaryContactMethod = usePrimaryContactMethod(); const [draftFeed, setDraftFeed] = useState(); @@ -163,12 +168,14 @@ function WorkspaceCompanyCardFeedSelectorPage({route}: WorkspaceCompanyCardFeedS return; } const feedValue = getCardFeedWithDomainID(feed.feed, feed.fundID); + setFrozenFeeds({companyCardFeeds: liveCompanyCardFeeds, otherFeeds: liveOtherFeeds}); linkCardFeedToPolicy(feed.fundID, policyID, CONST.COMPANY_CARD.LINK_FEED_TYPE.COMPANY_CARD, feed?.country, feed.feed) .then(() => { updateSelectedFeed(feedValue, policyID); goBack(); }) .catch((error: TranslationPaths) => { + setFrozenFeeds(undefined); setFeedWithError({ feed: feed.value, error: getMicroSecondOnyxErrorWithTranslationKey(error), @@ -176,12 +183,15 @@ function WorkspaceCompanyCardFeedSelectorPage({route}: WorkspaceCompanyCardFeedS }); }; + // Other-workspace feeds are selected by the key they get in the main list once linked, so the checkmark follows the feed when it moves there on Save. + const getSelectionValue = (feed: CardFeedListItem) => feed.selectionValue ?? feed.value; + const selectFeed = (feed: CardFeedListItem) => { setFeedWithError(undefined); - setDraftFeed(feed.value); + setDraftFeed(getSelectionValue(feed)); }; - const stagedOtherWorkspaceFeed = otherFeeds.find((feed) => feed.value === currentSelectedFeed); + const stagedOtherWorkspaceFeed = otherFeeds.find((feed) => getSelectionValue(feed) === currentSelectedFeed); const isStagedFeedOnPage = !!stagedOtherWorkspaceFeed || feeds.some((feed) => feed.value === currentSelectedFeed); const saveFeed = () => { @@ -228,7 +238,7 @@ function WorkspaceCompanyCardFeedSelectorPage({route}: WorkspaceCompanyCardFeedS const item = { ...feed, // The hook marks the committed feed as selected, but the checkmark has to follow the staged one. - isSelected: feed.value === currentSelectedFeed, + isSelected: getSelectionValue(feed) === currentSelectedFeed, errors: feedWithError?.feed === feed.value ? feedWithError.error : undefined, }; return ( diff --git a/tests/ui/WorkspaceCompanyCardFeedSelectorPageTest.tsx b/tests/ui/WorkspaceCompanyCardFeedSelectorPageTest.tsx index 7eb686ae37b6..8f51d8fe3414 100644 --- a/tests/ui/WorkspaceCompanyCardFeedSelectorPageTest.tsx +++ b/tests/ui/WorkspaceCompanyCardFeedSelectorPageTest.tsx @@ -1,6 +1,6 @@ import {act, fireEvent, render, screen} from '@testing-library/react-native'; -import type {CompanyCardFeedWithDomainID} from '@hooks/useCardFeeds'; +import type {CombinedCardFeed, CompanyCardFeedWithDomainID} from '@hooks/useCardFeeds'; import type {CardFeedListItem} from '@hooks/useOtherFeedsForFeedSelector'; import Navigation from '@libs/Navigation/Navigation'; @@ -9,7 +9,7 @@ import type {SettingsNavigatorParamList} from '@libs/Navigation/types'; import WorkspaceCompanyCardFeedSelectorPage from '@pages/workspace/companyCards/WorkspaceCompanyCardFeedSelectorPage'; -import {clearAddNewCardFlow} from '@userActions/CompanyCards'; +import {clearAddNewCardFlow, linkCardFeedToPolicy} from '@userActions/CompanyCards'; import CONST from '@src/CONST'; import ROUTES, {DYNAMIC_ROUTES} from '@src/ROUTES'; @@ -19,8 +19,14 @@ import React from 'react'; const POLICY_ID = 'policy123'; +const OTHER_WORKSPACE_FEED_KEY: CompanyCardFeedWithDomainID = `${CONST.COMPANY_CARD.FEED_BANK_NAME.VISA}${CONST.COMPANY_CARD.FEED_KEY_SEPARATOR}123`; + +// Matches what useOtherFeedsForFeedSelector builds: the row value is `${fundID}_${feed}`, while the selection uses the main list key `${feed}#${fundID}`. const OTHER_WORKSPACE_FEED: CardFeedListItem = { - value: `${CONST.COMPANY_CARD.FEED_BANK_NAME.VISA}${CONST.COMPANY_CARD.FEED_KEY_SEPARATOR}123`, + // The hook casts its `${fundID}_${feed}` row id to this type the same way. + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + value: `123_${CONST.COMPANY_CARD.FEED_BANK_NAME.VISA}` as CompanyCardFeedWithDomainID, + selectionValue: OTHER_WORKSPACE_FEED_KEY, feed: CONST.COMPANY_CARD.FEED_BANK_NAME.VISA, fundID: 123, text: 'Other workspace feed', @@ -32,6 +38,8 @@ let mockIsBlockedToAddNewFeeds = false; let mockIsOffline = false; let mockOtherFeeds: CardFeedListItem[] = []; let mockCommittedFeedName: CompanyCardFeedWithDomainID | undefined; +let mockCompanyCardFeeds: Partial> = {}; +let mockPrimaryContactMethod = 'admin@example.com'; let mockCapturedOnResume: ((payload?: () => void) => void) | undefined; const mockVerifyAccountAndResume = jest.fn void]>(); @@ -43,6 +51,18 @@ jest.mock('@hooks/useVerifyAccountAndResume', () => ({ }, })); +// The available feeds render in a real SelectionList, which reads focus from navigation. +jest.mock('@react-navigation/native', () => { + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + const actualNav = jest.requireActual('@react-navigation/native'); + // eslint-disable-next-line @typescript-eslint/no-unsafe-return + return { + ...actualNav, + useIsFocused: () => true, + useFocusEffect: jest.fn(), + }; +}); + jest.mock('@components/HeaderWithBackButton', () => () => null); jest.mock( '@components/ScreenWrapper', @@ -79,7 +99,7 @@ jest.mock('@hooks/useCompanyCardIcons', () => ({ jest.mock('@hooks/useCompanyCards', () => ({ __esModule: true, - default: () => ({companyCardFeeds: {}, feedName: mockCommittedFeedName}), + default: () => ({companyCardFeeds: mockCompanyCardFeeds, feedName: mockCommittedFeedName}), })); jest.mock('@hooks/useIsBlockedToAddFeed', () => ({ @@ -123,7 +143,7 @@ jest.mock('@hooks/usePolicyFeatureWriteAccess', () => ({ jest.mock('@hooks/usePrimaryContactMethod', () => ({ __esModule: true, - default: () => 'admin@example.com', + default: () => mockPrimaryContactMethod, })); jest.mock('@hooks/useThemeIllustrations', () => ({ @@ -189,6 +209,8 @@ describe('WorkspaceCompanyCardFeedSelectorPage', () => { mockIsOffline = false; mockOtherFeeds = []; mockCommittedFeedName = undefined; + mockCompanyCardFeeds = {}; + mockPrimaryContactMethod = 'admin@example.com'; mockCapturedOnResume = undefined; }); @@ -201,7 +223,7 @@ describe('WorkspaceCompanyCardFeedSelectorPage', () => { it('keeps Save disabled when the committed feed is an other-workspace feed the user has not picked', () => { // Given the committed feed is the other-workspace feed, so it is staged without the user picking anything - mockCommittedFeedName = OTHER_WORKSPACE_FEED.value; + mockCommittedFeedName = OTHER_WORKSPACE_FEED_KEY; // When the page opens renderFeedSelectorPage(); @@ -212,7 +234,7 @@ describe('WorkspaceCompanyCardFeedSelectorPage', () => { it('starts the link flow once the user picks the other-workspace feed', () => { // Given the committed feed is the other-workspace feed - mockCommittedFeedName = OTHER_WORKSPACE_FEED.value; + mockCommittedFeedName = OTHER_WORKSPACE_FEED_KEY; renderFeedSelectorPage(); // When the user picks it and presses Save @@ -242,6 +264,64 @@ describe('WorkspaceCompanyCardFeedSelectorPage', () => { // Then Save is disabled, because the link request never settles offline expect(getSaveButton()).toBeDisabled(); }); + + const rerenderFeedSelectorPage = (rerender: ReturnType['rerender']) => + rerender( + , + ); + + // Mirrors what Onyx holds once the link response lands: the feed leaves "From other workspaces" and shows up in the available feeds list. + const moveFeedIntoAvailableList = () => { + mockOtherFeeds = []; + mockCompanyCardFeeds = {[OTHER_WORKSPACE_FEED_KEY]: {feed: CONST.COMPANY_CARD.FEED_BANK_NAME.VISA, domainID: 123}}; + }; + + it('keeps the staged feed checked in place while the link response lands before the page closes', () => { + // Given a user with a validated work email stages the other-workspace feed and presses Save + mockPrimaryContactMethod = ''; + jest.mocked(linkCardFeedToPolicy).mockReturnValue(new Promise(() => {})); + const {rerender} = renderFeedSelectorPage(); + fireEvent.press(screen.getByText(OTHER_WORKSPACE_FEED.text ?? '')); + fireEvent.press(getSaveButton()); + expect(linkCardFeedToPolicy).toHaveBeenCalledTimes(1); + + // When the link response reaches Onyx before the page closes + moveFeedIntoAvailableList(); + rerenderFeedSelectorPage(rerender); + + // Then the feed stays checked under "From other workspaces" and no available feed row appears, so nothing moves while the RHP closes + expect(screen.getByText('workspace.companyCards.fromOtherWorkspaces')).toBeOnTheScreen(); + expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}${OTHER_WORKSPACE_FEED.keyForList}`).props.accessibilityState).toMatchObject({selected: true}); + expect(screen.queryByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}${OTHER_WORKSPACE_FEED_KEY}`)).not.toBeOnTheScreen(); + }); + + it('shows the latest feeds again when linking fails', async () => { + // Given a user with a validated work email stages the other-workspace feed, presses Save, and the feeds change while the link request is pending + mockPrimaryContactMethod = ''; + let rejectLink: (error: string) => void = () => {}; + jest.mocked(linkCardFeedToPolicy).mockReturnValue( + new Promise((resolve, reject) => { + rejectLink = reject; + }), + ); + const {rerender} = renderFeedSelectorPage(); + fireEvent.press(screen.getByText(OTHER_WORKSPACE_FEED.text ?? '')); + fireEvent.press(getSaveButton()); + moveFeedIntoAvailableList(); + rerenderFeedSelectorPage(rerender); + + // When the link request fails + await act(async () => { + rejectLink('common.genericErrorMessage'); + }); + + // Then the page stops freezing the lists, because the page stays open and has to reflect the current feeds + expect(screen.queryByText('workspace.companyCards.fromOtherWorkspaces')).not.toBeOnTheScreen(); + expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}${OTHER_WORKSPACE_FEED_KEY}`)).toBeOnTheScreen(); + }); }); it('navigates a validated user on a plan blocked from new feeds straight to the upgrade screen', () => { diff --git a/tests/unit/hooks/useOtherFeedsForFeedSelector.test.tsx b/tests/unit/hooks/useOtherFeedsForFeedSelector.test.tsx index d86f0c6fef5c..c00667bc8367 100644 --- a/tests/unit/hooks/useOtherFeedsForFeedSelector.test.tsx +++ b/tests/unit/hooks/useOtherFeedsForFeedSelector.test.tsx @@ -213,6 +213,7 @@ describe('useOtherFeedsForFeedSelector', () => { // Use toMatchObject so Jest does not deep-compare leftElement (React nodes use private fields and break toEqual). expect(result.current.at(0)).toMatchObject({ value: '999_oauth.chase.com', + selectionValue: `oauth.chase.com${CONST.COMPANY_CARD.FEED_KEY_SEPARATOR}999`, feed: 'oauth.chase.com', fundID: 999, country: 'US', @@ -223,6 +224,27 @@ describe('useOtherFeedsForFeedSelector', () => { expect(React.isValidElement(result.current.at(0)?.leftElement)).toBe(true); }); + it('should leave selectionValue undefined when the feed has no valid fundID', () => { + // Given a feed whose fundID cannot be parsed, so there is no `${feed}${separator}${fundID}` key it could match in the available list + mockVisibleFeeds([ + { + id: '_oauth.chase.com', + feed: 'oauth.chase.com', + fundID: '', + name: 'Chase feed', + linkedPolicyIDs: ['policy_unrelated'], + }, + ]); + + // When the hook builds the other-workspace rows + const {result} = renderHook(() => useOtherFeedsForFeedSelector(currentPolicyID)); + + // Then the row is still listed, but without a selection key, so the page falls back to the row value instead of a bogus `feed#NaN` key + expect(result.current).toHaveLength(1); + expect(result.current.at(0)?.selectionValue).toBeUndefined(); + expect(mockGetCardFeedWithDomainID).not.toHaveBeenCalled(); + }); + it('should set isSelected when feed id matches the selected feed from useCompanyCards', () => { const feedId = '999_oauth.chase.com'; mockUseCompanyCards.mockReturnValue({feedName: feedId, companyCardFeeds: {}});