From 365f838f8b10a85574233cdd8e584ff05084e9b0 Mon Sep 17 00:00:00 2001 From: "truph01 (via MelvinBot)" Date: Wed, 14 Oct 2026 13:52:59 +0000 Subject: [PATCH 1/3] Keep the staged other-workspace card feed checked after linking Stage other-workspace feeds by the feed#fundID key they get in the available feeds list, computed once in useOtherFeedsForFeedSelector as selectionValue (undefined when the fundID is missing). The checkmark now follows the feed when linking moves it into the available list before the RHP closes. Co-authored-by: truph01 --- src/hooks/useOtherFeedsForFeedSelector.tsx | 17 +++--- .../WorkspaceCompanyCardFeedSelectorPage.tsx | 9 ++- ...rkspaceCompanyCardFeedSelectorPageTest.tsx | 59 ++++++++++++++++--- .../useOtherFeedsForFeedSelector.test.tsx | 22 +++++++ 4 files changed, 90 insertions(+), 17 deletions(-) diff --git a/src/hooks/useOtherFeedsForFeedSelector.tsx b/src/hooks/useOtherFeedsForFeedSelector.tsx index 77296a08b224..c08334ea67c4 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,14 @@ 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 +84,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..2f59147e48a8 100644 --- a/src/pages/workspace/companyCards/WorkspaceCompanyCardFeedSelectorPage.tsx +++ b/src/pages/workspace/companyCards/WorkspaceCompanyCardFeedSelectorPage.tsx @@ -176,12 +176,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 +231,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..3a9dd59be668 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,29 @@ describe('WorkspaceCompanyCardFeedSelectorPage', () => { // Then Save is disabled, because the link request never settles offline expect(getSaveButton()).toBeDisabled(); }); + + it('keeps the staged feed checked after linking moves it into the available feeds list', () => { + // 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, so the feed leaves "From other workspaces" and shows up in the available feeds list + mockOtherFeeds = []; + mockCompanyCardFeeds = {[OTHER_WORKSPACE_FEED_KEY]: {feed: CONST.COMPANY_CARD.FEED_BANK_NAME.VISA, domainID: 123}}; + rerender( + , + ); + + // Then the same feed is still checked in its new row, so the checkmark does not disappear while the RHP closes + expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}${OTHER_WORKSPACE_FEED_KEY}`).props.accessibilityState).toMatchObject({selected: true}); + }); }); 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: {}}); From 49411907cf18ea96ba6cce1576b7d393d86f56ef Mon Sep 17 00:00:00 2001 From: "truph01 (via MelvinBot)" Date: Thu, 15 Oct 2026 03:27:02 +0000 Subject: [PATCH 2/3] Freeze the feed selector lists while linking an other-workspace feed Co-authored-by: truph01 --- .../WorkspaceCompanyCardFeedSelectorPage.tsx | 11 +++- ...rkspaceCompanyCardFeedSelectorPageTest.tsx | 57 +++++++++++++++---- 2 files changed, 55 insertions(+), 13 deletions(-) diff --git a/src/pages/workspace/companyCards/WorkspaceCompanyCardFeedSelectorPage.tsx b/src/pages/workspace/companyCards/WorkspaceCompanyCardFeedSelectorPage.tsx index 2f59147e48a8..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), diff --git a/tests/ui/WorkspaceCompanyCardFeedSelectorPageTest.tsx b/tests/ui/WorkspaceCompanyCardFeedSelectorPageTest.tsx index 3a9dd59be668..8f51d8fe3414 100644 --- a/tests/ui/WorkspaceCompanyCardFeedSelectorPageTest.tsx +++ b/tests/ui/WorkspaceCompanyCardFeedSelectorPageTest.tsx @@ -265,7 +265,21 @@ describe('WorkspaceCompanyCardFeedSelectorPage', () => { expect(getSaveButton()).toBeDisabled(); }); - it('keeps the staged feed checked after linking moves it into the available feeds list', () => { + 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(() => {})); @@ -274,18 +288,39 @@ describe('WorkspaceCompanyCardFeedSelectorPage', () => { fireEvent.press(getSaveButton()); expect(linkCardFeedToPolicy).toHaveBeenCalledTimes(1); - // When the link response reaches Onyx before the page closes, so the feed leaves "From other workspaces" and shows up in the available feeds list - mockOtherFeeds = []; - mockCompanyCardFeeds = {[OTHER_WORKSPACE_FEED_KEY]: {feed: CONST.COMPANY_CARD.FEED_BANK_NAME.VISA, domainID: 123}}; - rerender( - , + // 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 same feed is still checked in its new row, so the checkmark does not disappear while the RHP closes - expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}${OTHER_WORKSPACE_FEED_KEY}`).props.accessibilityState).toMatchObject({selected: true}); + // 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(); }); }); From e774731e7967dcf0b2216ade91bbdcda04f3ba0a Mon Sep 17 00:00:00 2001 From: "truph01 (via MelvinBot)" Date: Sat, 17 Oct 2026 03:59:19 +0000 Subject: [PATCH 3/3] Add blank lines before comments in useOtherFeedsForFeedSelector Co-authored-by: truph01 --- src/hooks/useOtherFeedsForFeedSelector.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/hooks/useOtherFeedsForFeedSelector.tsx b/src/hooks/useOtherFeedsForFeedSelector.tsx index c08334ea67c4..10516a5d2a31 100644 --- a/src/hooks/useOtherFeedsForFeedSelector.tsx +++ b/src/hooks/useOtherFeedsForFeedSelector.tsx @@ -66,8 +66,10 @@ function useOtherFeedsForFeedSelector(policyID: string): CardFeedListItem[] { } 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. if (feedValueForActivePolicy && companyCardFeeds && feedValueForActivePolicy in companyCardFeeds) { continue;