Skip to content
Open
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
12 changes: 10 additions & 2 deletions src/components/Tables/WorkspaceCompanyCardsTable/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,12 @@ type WorkspaceCompanyCardsTableProps = {
/** Policy ID */
policyID: string;

/** Whether the policy is loaded */
/** Whether the policy is fully loaded, i.e. its account ID has resolved */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is true for the offline case, when it's not fully loaded, right?
May be we clarify that in the comment

isPolicyLoaded: boolean;

/** Whether the company cards page fetch is still expected to land, i.e. no feeds are cached for the workspace yet */
isPageFetchPending: boolean;

/** Domain or workspace account ID */
domainOrWorkspaceAccountID: number;

Expand Down Expand Up @@ -85,6 +88,7 @@ function WorkspaceCompanyCardsTable({
ref,
policyID,
isPolicyLoaded,
isPageFetchPending,
domainOrWorkspaceAccountID,
companyCards,
onAssignCard,
Expand Down Expand Up @@ -130,7 +134,6 @@ function WorkspaceCompanyCardsTable({
const hasOnceLoadedSelectedFeed = !!bankName && !!companyCardsLoadingState?.feeds?.[bankName]?.hasOnceLoaded;

const hasNoAssignedCard = Object.keys(assignedCards ?? {}).length === 0;
const areWorkspaceCardFeedsLoading = !!workspaceCardFeedsStatus?.[domainOrWorkspaceAccountID]?.isLoading && !hasOnceLoadedPage;

// Synthesize error locally since Onyx discards writes to collection keys with member ID '0'.
const shouldShowWorkspaceFeedsLoadError = domainOrWorkspaceAccountID === CONST.DEFAULT_NUMBER_ID && isPolicyLoaded && !isOffline;
Expand Down Expand Up @@ -160,6 +163,11 @@ function WorkspaceCompanyCardsTable({
// If we already have fetched cards, then do not show a loading spinner (let the remaining updates refresh in the background), else show it
const hasCards = (companyCardEntries ?? []).length > 0;

// The page fetch is kicked off from an effect, so its optimistic `isLoading` flag only lands after the first render.
// Treat the window before it as loading too, otherwise the empty feed state flashes before the loading indicator shows up.
const isPageFetchAwaited = isPageFetchPending && !hasFeedErrors;
const areWorkspaceCardFeedsLoading = (!!workspaceCardFeedsStatus?.[domainOrWorkspaceAccountID]?.isLoading || isPageFetchAwaited) && !hasOnceLoadedPage;

const isLoadingOnyxCardList = !hasCards && isLoadingOnyxValue(cardListMetadata);
const isLoadingOnyxPersonalDetails = isLoadingOnyxValue(personalDetailsMetadata);
const isLoadingOnyxFeed = !isNoFeed && isLoadingOnyxValue(lastSelectedFeedMetadata) && !hasOnceLoadedSelectedFeed;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,14 @@ function WorkspaceCompanyCardsPage({route}: WorkspaceCompanyCardsPageProps) {
onReconnect: loadPolicyCompanyCardsPage,
});

const isPolicyLoaded = !!policy && (policy.policyAccountID !== undefined || isOffline);

const isLoading = !isOffline && (!allCardFeeds || (isFeedAdded && isLoadingOnyxValue(cardListMetadata)));

const hasFeedsLoaded = !!allCardFeeds && Object.keys(allCardFeeds).length > 0;

const isPageFetchPending = !hasFeedsLoaded;

useEffect(() => {
if (isOffline || hasFeedsLoaded) {
return;
Expand Down Expand Up @@ -134,7 +138,8 @@ function WorkspaceCompanyCardsPage({route}: WorkspaceCompanyCardsPageProps) {
<WorkspaceCompanyCardsTable
ref={companyCardsTableRef}
policyID={policyID}
isPolicyLoaded={!!policy}
isPolicyLoaded={isPolicyLoaded}
isPageFetchPending={isPageFetchPending}
domainOrWorkspaceAccountID={domainOrWorkspaceAccountID}
companyCards={companyCards}
onAssignCard={assignCard}
Expand Down
105 changes: 88 additions & 17 deletions tests/ui/WorkspaceCompanyCardsPageSelectionModeTest.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,13 @@ const POLICY_ID = 'policy123';
const mockClearTableSelection = jest.fn();
const mockGoBack = jest.fn();
const mockTurnOffMobileSelectionMode = jest.fn();
const mockTableProps: {current?: {isSelectionModeEnabled: boolean}} = {};
const mockTableProps: {current?: {isSelectionModeEnabled: boolean; isPolicyLoaded: boolean; isPageFetchPending: boolean}} = {};
let mockFeedName = 'feed-a';
let mockIsMobileSelectionModeEnabled = true;
let mockShouldUseNarrowLayout = true;
let mockIsOffline = false;
let mockPolicy: {name: string; policyAccountID?: number; employeeList: Record<string, unknown>} | undefined = {name: 'Acme', policyAccountID: 123, employeeList: {}};
let mockAllCardFeeds: Record<string, unknown> | undefined = {feed: {}};

jest.mock('@components/DecisionModal', () => () => null);

Expand All @@ -28,12 +31,14 @@ jest.mock('@components/Tables/WorkspaceCompanyCardsTable', () => {
const {View} = require('react-native');

// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access
const MockWorkspaceCompanyCardsTable = ReactMock.forwardRef(({isSelectionModeEnabled}: {isSelectionModeEnabled: boolean}, ref: unknown) => {
mockTableProps.current = {isSelectionModeEnabled};
// eslint-disable-next-line @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access
ReactMock.useImperativeHandle(ref, () => ({clearSelection: mockClearTableSelection}));
return <View testID="WorkspaceCompanyCardsTable" />;
});
const MockWorkspaceCompanyCardsTable = ReactMock.forwardRef(
({isSelectionModeEnabled, isPolicyLoaded, isPageFetchPending}: {isSelectionModeEnabled: boolean; isPolicyLoaded: boolean; isPageFetchPending: boolean}, ref: unknown) => {
mockTableProps.current = {isSelectionModeEnabled, isPolicyLoaded, isPageFetchPending};
// eslint-disable-next-line @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access
ReactMock.useImperativeHandle(ref, () => ({clearSelection: mockClearTableSelection}));
return <View testID="WorkspaceCompanyCardsTable" />;
},
);

return {
__esModule: true,
Expand All @@ -50,7 +55,7 @@ jest.mock('@hooks/useAssignCard', () => ({
jest.mock('@hooks/useCompanyCards', () => ({
__esModule: true,
default: () => ({
allCardFeeds: {feed: {}},
allCardFeeds: mockAllCardFeeds,
feedName: mockFeedName,
selectedFeed: undefined,
bankName: undefined,
Expand Down Expand Up @@ -81,12 +86,12 @@ jest.mock('@hooks/useMobileSelectionMode', () => ({

jest.mock('@hooks/useNetwork', () => ({
__esModule: true,
default: () => ({isOffline: false}),
default: () => ({isOffline: mockIsOffline}),
}));

jest.mock('@hooks/usePolicy', () => ({
__esModule: true,
default: () => ({name: 'Acme', policyAccountID: 123, employeeList: {}}),
default: () => mockPolicy,
}));

jest.mock('@hooks/useResponsiveLayout', () => ({
Expand Down Expand Up @@ -181,14 +186,19 @@ function getWorkspaceCompanyCardsPage() {
);
}

function resetMocks() {
jest.clearAllMocks();
mockTableProps.current = undefined;
mockFeedName = 'feed-a';
mockIsMobileSelectionModeEnabled = true;
mockShouldUseNarrowLayout = true;
mockIsOffline = false;
mockPolicy = {name: 'Acme', policyAccountID: 123, employeeList: {}};
mockAllCardFeeds = {feed: {}};
}

describe('WorkspaceCompanyCardsPage selection mode', () => {
beforeEach(() => {
jest.clearAllMocks();
mockTableProps.current = undefined;
mockFeedName = 'feed-a';
mockIsMobileSelectionModeEnabled = true;
mockShouldUseNarrowLayout = true;
});
beforeEach(resetMocks);

it('uses the focused select header and table controls in narrow-layout selection mode', () => {
render(getWorkspaceCompanyCardsPage());
Expand Down Expand Up @@ -230,3 +240,64 @@ describe('WorkspaceCompanyCardsPage selection mode', () => {
expect(mockClearTableSelection).not.toHaveBeenCalled();
});
});

describe('WorkspaceCompanyCardsPage isPolicyLoaded', () => {
beforeEach(resetMocks);

it('reports the policy as loaded once its account ID has been returned', () => {
render(getWorkspaceCompanyCardsPage());

expect(mockTableProps.current?.isPolicyLoaded).toBe(true);
});

it('reports the policy as not loaded while a freshly created workspace has no account ID yet', () => {
mockPolicy = {name: 'Acme', employeeList: {}};

render(getWorkspaceCompanyCardsPage());

expect(mockTableProps.current?.isPolicyLoaded).toBe(false);
});

it('reports the policy as loaded when its account ID resolved to 0, so the feeds load error can surface', () => {
mockPolicy = {name: 'Acme', policyAccountID: 0, employeeList: {}};

render(getWorkspaceCompanyCardsPage());

expect(mockTableProps.current?.isPolicyLoaded).toBe(true);
});

it('reports the policy as loaded offline, where the account ID can never resolve', () => {
mockPolicy = {name: 'Acme', employeeList: {}};
mockIsOffline = true;

render(getWorkspaceCompanyCardsPage());

expect(mockTableProps.current?.isPolicyLoaded).toBe(true);
});

it('reports the policy as not loaded when there is no policy at all', () => {
mockPolicy = undefined;

render(getWorkspaceCompanyCardsPage());

expect(mockTableProps.current?.isPolicyLoaded).toBe(false);
});
});

describe('WorkspaceCompanyCardsPage isPageFetchPending', () => {
beforeEach(resetMocks);

it('reports the page fetch as pending while no feeds are cached for the workspace', () => {
mockAllCardFeeds = undefined;

render(getWorkspaceCompanyCardsPage());

expect(mockTableProps.current?.isPageFetchPending).toBe(true);
});

it('reports the page fetch as settled once feeds are cached, since it is no longer re-fetched', () => {
render(getWorkspaceCompanyCardsPage());

expect(mockTableProps.current?.isPageFetchPending).toBe(false);
});
});
94 changes: 91 additions & 3 deletions tests/ui/WorkspaceCompanyCardsTableTest.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -134,13 +134,24 @@ function buildCompanyCards({
};
}

function renderTable(companyCards: UseCompanyCardsResult, isSelectionModeEnabled = false) {
type RenderTableOverrides = {
isPolicyLoaded?: boolean;
isPageFetchPending?: boolean;
domainOrWorkspaceAccountID?: number;
};

function renderTable(
companyCards: UseCompanyCardsResult,
isSelectionModeEnabled = false,
{isPolicyLoaded = true, isPageFetchPending = false, domainOrWorkspaceAccountID = DOMAIN_OR_WORKSPACE_ACCOUNT_ID}: RenderTableOverrides = {},
) {
return render(
<ComposeProviders components={[OnyxListItemProvider, LocaleContextProvider]}>
<WorkspaceCompanyCardsTable
policyID={POLICY_ID}
isPolicyLoaded
domainOrWorkspaceAccountID={DOMAIN_OR_WORKSPACE_ACCOUNT_ID}
isPolicyLoaded={isPolicyLoaded}
isPageFetchPending={isPageFetchPending}
domainOrWorkspaceAccountID={domainOrWorkspaceAccountID}
companyCards={companyCards}
onAssignCard={jest.fn()}
isAssigningCardDisabled={false}
Expand Down Expand Up @@ -215,6 +226,83 @@ describe('WorkspaceCompanyCardsTable loading suppression', () => {
});
});

describe('WorkspaceCompanyCardsTable pending page fetch', () => {
beforeEach(async () => {
await Onyx.clear();
await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, {});
await waitForBatchedUpdates();
});

it('shows the loading indicator while the page fetch is still awaited, so the empty feed state cannot flash', async () => {
renderTable(buildCompanyCards({workspaceCardFeedsStatus: {}}), false, {isPageFetchPending: true});

await waitForBatchedUpdates();

expect(screen.getByTestId('WorkspaceCompanyCardsTableLoadingIndicator')).toBeTruthy();
expect(screen.queryByTestId('WorkspaceCompanyCardPageEmptyState')).toBeNull();
});

it('shows the empty feed state once the page fetch has succeeded', async () => {
await Onyx.merge(`${ONYXKEYS.COLLECTION.RAM_ONLY_COMPANY_CARDS_LOADING_STATE}${DOMAIN_OR_WORKSPACE_ACCOUNT_ID}`, {
hasOnceLoadedPage: true,
});

renderTable(buildCompanyCards({workspaceCardFeedsStatus: {}}), false, {isPageFetchPending: true});

await waitForBatchedUpdates();

expect(screen.queryByTestId('WorkspaceCompanyCardsTableLoadingIndicator')).toBeNull();
expect(screen.getByTestId('WorkspaceCompanyCardPageEmptyState')).toBeTruthy();
});

it('shows the feeds load error instead of the loading indicator when the page fetch failed', async () => {
renderTable(
buildCompanyCards({
workspaceCardFeedsStatus: {
[DOMAIN_OR_WORKSPACE_ACCOUNT_ID]: {
errors: {
[CONST.COMPANY_CARDS.WORKSPACE_FEEDS_LOAD_ERROR]: TestHelper.translateLocal('workspace.companyCards.error.workspaceFeedsCouldNotBeLoadedMessage'),
},
},
},
}),
false,
{isPageFetchPending: true},
);

await waitForBatchedUpdates();

expect(screen.queryByTestId('WorkspaceCompanyCardsTableLoadingIndicator')).toBeNull();
expect(screen.getByText(TestHelper.translateLocal('workspace.companyCards.error.workspaceFeedsCouldNotBeLoadedTitle'))).toBeTruthy();
});
});

describe('WorkspaceCompanyCardsTable unresolved workspace account ID', () => {
beforeEach(async () => {
await Onyx.clear();
await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, {});
await waitForBatchedUpdates();
});

it('keeps the loading indicator up instead of flashing the feeds load error while the policy account ID is unresolved', async () => {
renderTable(buildCompanyCards({workspaceCardFeedsStatus: {}}), false, {isPolicyLoaded: false, domainOrWorkspaceAccountID: CONST.DEFAULT_NUMBER_ID});

await waitForBatchedUpdates();

expect(screen.getByTestId('WorkspaceCompanyCardsTableLoadingIndicator')).toBeTruthy();
expect(screen.queryByText(TestHelper.translateLocal('workspace.companyCards.error.workspaceFeedsCouldNotBeLoadedTitle'))).toBeNull();
});

it('still shows the feeds load error once the policy is loaded and the account ID is genuinely 0', async () => {
renderTable(buildCompanyCards({workspaceCardFeedsStatus: {}}), false, {domainOrWorkspaceAccountID: CONST.DEFAULT_NUMBER_ID});

await waitForBatchedUpdates();

expect(screen.queryByTestId('WorkspaceCompanyCardsTableLoadingIndicator')).toBeNull();
expect(screen.getByText(TestHelper.translateLocal('workspace.companyCards.error.workspaceFeedsCouldNotBeLoadedTitle'))).toBeTruthy();
});
});

describe('WorkspaceCompanyCardsTable selection mode', () => {
beforeEach(async () => {
await Onyx.clear();
Expand Down
Loading