Skip to content

Commit 52dc521

Browse files
adhityamamallanadhityamamallan
committed
Remove useSuspenseQuery
1 parent f26d165 commit 52dc521

File tree

11 files changed

+207
-152
lines changed

11 files changed

+207
-152
lines changed

‎src/views/domain-page/config/domain-page-tabs-error.config.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
1-
import getDomainWorkflowsErrorConfig from '@/views/domain-workflows/helpers/get-domain-workflows-error-config';
2-
31
import { type DomainPageTabsErrorConfig } from '../domain-page-tabs-error/domain-page-tabs-error.types';
42

53
const domainPageTabsErrorConfig: DomainPageTabsErrorConfig = {
6-
workflows: getDomainWorkflowsErrorConfig,
4+
workflows: () => ({
5+
message: 'Failed to load workflows',
6+
actions: [{ kind: 'retry', label: 'Retry' }],
7+
}),
78
metadata: () => ({
89
message: 'Failed to load metadata',
910
actions: [{ kind: 'retry', label: 'Retry' }],

‎src/views/domain-page/domain-page-content/domain-page-content.tsx‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,6 @@ export default function DomainPageContent(props: Props) {
2222
}
2323

2424
return (
25-
<section>
26-
<TabContent
27-
domain={decodedParams.domain}
28-
cluster={decodedParams.cluster}
29-
/>
30-
</section>
25+
<TabContent domain={decodedParams.domain} cluster={decodedParams.cluster} />
3126
);
3227
}

‎src/views/domain-workflows/domain-workflows-table/__tests__/domain-workflows-table.test.tsx‎

Lines changed: 79 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,24 @@
1-
import { Suspense } from 'react';
1+
import { HttpResponse } from 'msw';
22

3-
import { render, screen, act, fireEvent } from '@/test-utils/rtl';
3+
import { render, screen, userEvent } from '@/test-utils/rtl';
44

55
import { type ListWorkflowsResponse } from '@/route-handlers/list-workflows/list-workflows.types';
6-
import * as requestModule from '@/utils/request';
76

7+
import type { Props as MSWMocksHandlersProps } from '../../../../test-utils/msw-mock-handlers/msw-mock-handlers.types';
88
import { mockDomainWorkflowsQueryParamsValues } from '../../__fixtures__/domain-workflows-query-params';
99
import { type Props as EndMessageProps } from '../../domain-workflows-table-end-message/domain-workflows-table-end-message.types';
1010
import DomainWorkflowsTable from '../domain-workflows-table';
1111

12+
jest.mock('@/components/error-panel/error-panel', () =>
13+
jest.fn(({ message }: { message: string }) => <div>{message}</div>)
14+
);
15+
16+
jest.mock('../helpers/get-workflows-error-panel-props', () =>
17+
jest.fn().mockImplementation(({ error }) => ({
18+
message: error ? 'Error loading workflows' : 'No workflows found',
19+
}))
20+
);
21+
1222
jest.mock(
1323
'../../domain-workflows-table-end-message/domain-workflows-table-end-message',
1424
() =>
@@ -20,19 +30,19 @@ jest.mock(
2030
);
2131

2232
jest.mock('query-string', () => ({
23-
stringifyUrl: jest.fn(() => 'mock-stringified-api-url'),
33+
stringifyUrl: jest.fn(
34+
() => '/api/domains/mock-domain/mock-cluster/workflows'
35+
),
2436
}));
2537

26-
jest.mock('@/utils/request');
27-
2838
const mockSetQueryParams = jest.fn();
2939
jest.mock('@/hooks/use-page-query-params/use-page-query-params', () =>
3040
jest.fn(() => [mockDomainWorkflowsQueryParamsValues, mockSetQueryParams])
3141
);
3242

3343
describe(DomainWorkflowsTable.name, () => {
3444
it('renders workflows without error', async () => {
35-
await setup({});
45+
const { user } = setup({});
3646

3747
expect(await screen.findByText('Mock end message: OK')).toBeInTheDocument();
3848
Array(10).forEach((_, index) => {
@@ -41,9 +51,7 @@ describe(DomainWorkflowsTable.name, () => {
4151
).toBeInTheDocument();
4252
});
4353

44-
act(() => {
45-
fireEvent.click(screen.getByTestId('mock-end-message'));
46-
});
54+
await user.click(screen.getByTestId('mock-end-message'));
4755

4856
expect(await screen.findByText('Mock end message: OK')).toBeInTheDocument();
4957
Array(10).forEach((_, index) => {
@@ -53,23 +61,22 @@ describe(DomainWorkflowsTable.name, () => {
5361
});
5462
});
5563

56-
it('does not render if the initial call fails', async () => {
57-
let renderErrorMessage;
58-
try {
59-
await act(async () => {
60-
await setup({ errorCase: 'initial-fetch-error' });
61-
});
62-
} catch (error) {
63-
if (error instanceof Error) {
64-
renderErrorMessage = error.message;
65-
}
66-
}
67-
68-
expect(renderErrorMessage).toEqual('Request failed');
64+
it('renders error panel if the initial call fails', async () => {
65+
setup({ errorCase: 'initial-fetch-error' });
66+
67+
expect(
68+
await screen.findByText('Error loading workflows')
69+
).toBeInTheDocument();
70+
});
71+
72+
it('renders error panel if no workflows are found', async () => {
73+
setup({ errorCase: 'no-workflows' });
74+
75+
expect(await screen.findByText('No workflows found')).toBeInTheDocument();
6976
});
7077

7178
it('renders workflows and allows the user to try again if there is an error', async () => {
72-
await setup({ errorCase: 'subsequent-fetch-error' });
79+
const { user } = setup({ errorCase: 'subsequent-fetch-error' });
7380

7481
expect(await screen.findByText('Mock end message: OK')).toBeInTheDocument();
7582
Array(10).forEach((_, index) => {
@@ -78,17 +85,13 @@ describe(DomainWorkflowsTable.name, () => {
7885
).toBeInTheDocument();
7986
});
8087

81-
act(() => {
82-
fireEvent.click(screen.getByTestId('mock-end-message'));
83-
});
88+
await user.click(screen.getByTestId('mock-end-message'));
8489

8590
expect(
8691
await screen.findByText('Mock end message: Error')
8792
).toBeInTheDocument();
8893

89-
act(() => {
90-
fireEvent.click(screen.getByTestId('mock-end-message'));
91-
});
94+
await user.click(screen.getByTestId('mock-end-message'));
9295

9396
expect(await screen.findByText('Mock end message: OK')).toBeInTheDocument();
9497
Array(10).forEach((_, index) => {
@@ -99,41 +102,57 @@ describe(DomainWorkflowsTable.name, () => {
99102
});
100103
});
101104

102-
async function setup({
105+
function setup({
103106
errorCase,
104107
}: {
105-
errorCase?: 'initial-fetch-error' | 'subsequent-fetch-error';
108+
errorCase?: 'initial-fetch-error' | 'subsequent-fetch-error' | 'no-workflows';
106109
}) {
107-
// TODO: @adhitya.mamallan - This is not type-safe, explore using a library such as nock or msw
108-
const requestMock = jest.spyOn(requestModule, 'default') as jest.Mock;
109110
const pages = generateWorkflowPages(2);
111+
let currentEventIndex = 0;
112+
const user = userEvent.setup();
113+
114+
render(<DomainWorkflowsTable domain="mock-domain" cluster="mock-cluster" />, {
115+
endpointsMocks: [
116+
{
117+
path: '/api/domains/:domain/:cluster/workflows',
118+
httpMethod: 'GET',
119+
mockOnce: false,
120+
httpResolver: async () => {
121+
const index = currentEventIndex;
122+
currentEventIndex++;
123+
124+
switch (errorCase) {
125+
case 'no-workflows':
126+
return HttpResponse.json({ workflows: [], nextPage: undefined });
127+
case 'initial-fetch-error':
128+
return HttpResponse.json(
129+
{ message: 'Request failed' },
130+
{ status: 500 }
131+
);
132+
case 'subsequent-fetch-error':
133+
if (index === 0) {
134+
return HttpResponse.json(pages[0]);
135+
} else if (index === 1) {
136+
return HttpResponse.json(
137+
{ message: 'Request failed' },
138+
{ status: 500 }
139+
);
140+
} else {
141+
return HttpResponse.json(pages[1]);
142+
}
143+
default:
144+
if (index === 0) {
145+
return HttpResponse.json(pages[0]);
146+
} else {
147+
return HttpResponse.json(pages[1]);
148+
}
149+
}
150+
},
151+
},
152+
] as MSWMocksHandlersProps['endpointsMocks'],
153+
});
110154

111-
if (errorCase === 'subsequent-fetch-error') {
112-
requestMock
113-
.mockResolvedValueOnce({
114-
json: () => Promise.resolve(pages[0]),
115-
})
116-
.mockRejectedValueOnce(new Error('Request failed'))
117-
.mockResolvedValueOnce({
118-
json: () => Promise.resolve(pages[1]),
119-
});
120-
} else if (errorCase === 'initial-fetch-error') {
121-
requestMock.mockRejectedValueOnce(new Error('Request failed'));
122-
} else {
123-
requestMock
124-
.mockResolvedValueOnce({
125-
json: () => Promise.resolve(pages[0]),
126-
})
127-
.mockResolvedValueOnce({
128-
json: () => Promise.resolve(pages[1]),
129-
});
130-
}
131-
132-
render(
133-
<Suspense>
134-
<DomainWorkflowsTable domain="mock-domain" cluster="mock-cluster" />
135-
</Suspense>
136-
);
155+
return { user };
137156
}
138157

139158
// TODO @adhitya.mamallan - Explore using fakerjs.dev for cases like this

‎src/views/domain-workflows/domain-workflows-table/domain-workflows-table.constants.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1 @@
11
export const PAGE_SIZE = 10;
2-
3-
export const NO_WORKFLOWS_ERROR_MESSAGE = 'Domain has no workflows';

‎src/views/domain-workflows/domain-workflows-table/domain-workflows-table.styles.ts‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,20 @@
1-
import { styled as createStyled } from 'baseui';
1+
import { styled as createStyled, withStyle } from 'baseui';
2+
3+
import PageSection from '@/components/page-section/page-section';
24

35
export const styled = {
46
TableContainer: createStyled('div', {
57
overflowX: 'auto',
68
}),
9+
PageSection: withStyle(PageSection, () => ({
10+
display: 'flex',
11+
flexDirection: 'column',
12+
flex: 1,
13+
})),
14+
ErrorPanelContainer: createStyled('div', ({ $theme }) => ({
15+
padding: `${$theme.sizing.scale1200} 0px`,
16+
display: 'flex',
17+
alignItems: 'center',
18+
justifyContent: 'center',
19+
})),
720
};

‎src/views/domain-workflows/domain-workflows-table/domain-workflows-table.tsx‎

Lines changed: 32 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
'use client';
2-
import React, { useMemo } from 'react';
2+
import React from 'react';
33

4-
import { useSuspenseInfiniteQuery } from '@tanstack/react-query';
4+
import { useInfiniteQuery } from '@tanstack/react-query';
55
import queryString from 'query-string';
66

7-
import PageSection from '@/components/page-section/page-section';
7+
import ErrorPanel from '@/components/error-panel/error-panel';
8+
import SectionLoadingIndicator from '@/components/section-loading-indicator/section-loading-indicator';
89
import Table from '@/components/table/table';
910
import usePageQueryParams from '@/hooks/use-page-query-params/use-page-query-params';
1011
import {
@@ -18,12 +19,10 @@ import domainWorkflowsTableConfig from '../config/domain-workflows-table.config'
1819
import DomainWorkflowsTableEndMessage from '../domain-workflows-table-end-message/domain-workflows-table-end-message';
1920
import getNextSortOrder from '../helpers/get-next-sort-order';
2021

21-
import {
22-
NO_WORKFLOWS_ERROR_MESSAGE,
23-
PAGE_SIZE,
24-
} from './domain-workflows-table.constants';
22+
import { PAGE_SIZE } from './domain-workflows-table.constants';
2523
import { styled } from './domain-workflows-table.styles';
2624
import { type Props } from './domain-workflows-table.types';
25+
import getWorkflowsErrorPanelProps from './helpers/get-workflows-error-panel-props';
2726

2827
export default function DomainWorkflowsTable(props: Props) {
2928
const [queryParams, setQueryParams] = usePageQueryParams(
@@ -47,7 +46,8 @@ export default function DomainWorkflowsTable(props: Props) {
4746
hasNextPage,
4847
fetchNextPage,
4948
isFetchingNextPage,
50-
} = useSuspenseInfiniteQuery<ListWorkflowsResponse>({
49+
refetch,
50+
} = useInfiniteQuery<ListWorkflowsResponse>({
5151
queryKey: ['listWorkflows', { ...props, ...requestQueryParams }],
5252
queryFn: async ({ pageParam }) =>
5353
request(
@@ -66,23 +66,33 @@ export default function DomainWorkflowsTable(props: Props) {
6666
},
6767
});
6868

69-
const workflows = useMemo(
70-
() => data.pages.flatMap((page) => page.workflows ?? []),
71-
[data]
72-
);
69+
if (isLoading) {
70+
return <SectionLoadingIndicator />;
71+
}
72+
73+
const workflows = data?.pages.flatMap((page) => page.workflows ?? []) ?? [];
74+
75+
if (workflows.length === 0) {
76+
const errorPanelProps = getWorkflowsErrorPanelProps({
77+
error,
78+
areSearchParamsAbsent:
79+
!queryParams.search &&
80+
!queryParams.status &&
81+
!queryParams.timeRangeStart &&
82+
!queryParams.timeRangeEnd,
83+
});
7384

74-
if (
75-
!queryParams.search &&
76-
!queryParams.status &&
77-
!queryParams.timeRangeStart &&
78-
!queryParams.timeRangeEnd &&
79-
workflows.length === 0
80-
) {
81-
throw new Error(NO_WORKFLOWS_ERROR_MESSAGE);
85+
if (errorPanelProps) {
86+
return (
87+
<styled.PageSection>
88+
<ErrorPanel {...errorPanelProps} reset={refetch} />
89+
</styled.PageSection>
90+
);
91+
}
8292
}
8393

8494
return (
85-
<PageSection>
95+
<styled.PageSection>
8696
<styled.TableContainer>
8797
<Table
8898
data={workflows}
@@ -111,6 +121,6 @@ export default function DomainWorkflowsTable(props: Props) {
111121
}
112122
/>
113123
</styled.TableContainer>
114-
</PageSection>
124+
</styled.PageSection>
115125
);
116126
}

‎src/views/domain-workflows/domain-workflows-table/helpers/__tests__/get-workflows-error-panel-props.test.ts‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
import getWorkflowsErrorPanelProps from '../get-workflows-error-panel-props';
2+
3+
describe(getWorkflowsErrorPanelProps.name, () => {
4+
it('returns default error panel props for regular error', () => {
5+
expect(
6+
getWorkflowsErrorPanelProps({
7+
error: new Error('Test error'),
8+
areSearchParamsAbsent: false,
9+
})
10+
).toEqual({
11+
message: 'Failed to fetch workflows',
12+
actions: [{ kind: 'retry', label: 'Retry' }],
13+
});
14+
});
15+
16+
it('returns "not found" error panel props when search params are absent', () => {
17+
expect(
18+
getWorkflowsErrorPanelProps({
19+
error: null,
20+
areSearchParamsAbsent: true,
21+
})
22+
).toEqual({
23+
message: 'No workflows found for this domain',
24+
omitLogging: true,
25+
actions: [
26+
{
27+
kind: 'link-external',
28+
label: 'Get started on workflows',
29+
link: 'https://cadenceworkflow.io/docs/concepts/workflows',
30+
},
31+
],
32+
});
33+
});
34+
35+
it('returns undefined in all other cases', () => {
36+
expect(
37+
getWorkflowsErrorPanelProps({
38+
error: null,
39+
areSearchParamsAbsent: false,
40+
})
41+
).toBeUndefined();
42+
});
43+
});

0 commit comments

Comments
 (0)