Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions src/hooks/useOtherFeedsForFeedSelector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Comment thread
lakchote marked this conversation as resolved.
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.
Expand All @@ -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),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<CompanyCardFeedWithDomainID>();
Expand Down Expand Up @@ -163,25 +168,30 @@ 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),
});
});
};

// 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 = () => {
Expand Down Expand Up @@ -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 (
Expand Down
94 changes: 87 additions & 7 deletions tests/ui/WorkspaceCompanyCardFeedSelectorPageTest.tsx
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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';
Expand All @@ -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',
Expand All @@ -32,6 +38,8 @@ let mockIsBlockedToAddNewFeeds = false;
let mockIsOffline = false;
let mockOtherFeeds: CardFeedListItem[] = [];
let mockCommittedFeedName: CompanyCardFeedWithDomainID | undefined;
let mockCompanyCardFeeds: Partial<Record<CompanyCardFeedWithDomainID, CombinedCardFeed>> = {};
let mockPrimaryContactMethod = 'admin@example.com';
let mockCapturedOnResume: ((payload?: () => void) => void) | undefined;
const mockVerifyAccountAndResume = jest.fn<void, [payload?: () => void]>();

Expand All @@ -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',
Expand Down Expand Up @@ -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', () => ({
Expand Down Expand Up @@ -123,7 +143,7 @@ jest.mock('@hooks/usePolicyFeatureWriteAccess', () => ({

jest.mock('@hooks/usePrimaryContactMethod', () => ({
__esModule: true,
default: () => 'admin@example.com',
default: () => mockPrimaryContactMethod,
}));

jest.mock('@hooks/useThemeIllustrations', () => ({
Expand Down Expand Up @@ -189,6 +209,8 @@ describe('WorkspaceCompanyCardFeedSelectorPage', () => {
mockIsOffline = false;
mockOtherFeeds = [];
mockCommittedFeedName = undefined;
mockCompanyCardFeeds = {};
mockPrimaryContactMethod = 'admin@example.com';
mockCapturedOnResume = undefined;
});

Expand All @@ -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();
Expand All @@ -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
Expand Down Expand Up @@ -242,6 +264,64 @@ describe('WorkspaceCompanyCardFeedSelectorPage', () => {
// Then Save is disabled, because the link request never settles offline
expect(getSaveButton()).toBeDisabled();
});

const rerenderFeedSelectorPage = (rerender: ReturnType<typeof renderFeedSelectorPage>['rerender']) =>
rerender(
<WorkspaceCompanyCardFeedSelectorPage
route={route}
navigation={navigation}
/>,
);

// 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', () => {
Expand Down
22 changes: 22 additions & 0 deletions tests/unit/hooks/useOtherFeedsForFeedSelector.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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: {}});
Expand Down
Loading