Skip to content

Commit 71c3aca

Browse files
authored
Improve loading for workflow page (#873)
improve loading for workflow page
1 parent 71d6574 commit 71c3aca

File tree

13 files changed

+156
-117
lines changed

13 files changed

+156
-117
lines changed
Lines changed: 51 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React, { Suspense } from 'react';
1+
import React from 'react';
22

33
import { HttpResponse } from 'msw';
44

@@ -48,7 +48,7 @@ describe(WorkflowActions.name, () => {
4848
});
4949

5050
it('renders the button with the correct text', async () => {
51-
await setup({});
51+
setup({});
5252

5353
const actionsButton = await screen.findByRole('button');
5454
expect(actionsButton).toHaveAttribute(
@@ -67,7 +67,7 @@ describe(WorkflowActions.name, () => {
6767
});
6868

6969
it('renders the menu when the button is clicked', async () => {
70-
const { user } = await setup({});
70+
const { user } = setup({});
7171

7272
const actionsButton = await screen.findByRole('button');
7373
await user.click(actionsButton);
@@ -76,7 +76,7 @@ describe(WorkflowActions.name, () => {
7676
});
7777

7878
it('renders the button with disabled configs if config fetching fails', async () => {
79-
const { user } = await setup({ isConfigError: true });
79+
const { user } = setup({ isConfigError: true });
8080

8181
const actionsButton = await screen.findByRole('button');
8282
await user.click(actionsButton);
@@ -87,7 +87,7 @@ describe(WorkflowActions.name, () => {
8787
});
8888

8989
it('shows the modal when a menu option is clicked', async () => {
90-
const { user } = await setup({});
90+
const { user } = setup({});
9191

9292
const actionsButton = await screen.findByRole('button');
9393
await user.click(actionsButton);
@@ -97,22 +97,15 @@ describe(WorkflowActions.name, () => {
9797
});
9898

9999
it('renders nothing if describeWorkflow fails', async () => {
100-
let renderErrorMessage;
101-
try {
102-
await act(async () => {
103-
await setup({ isError: true });
104-
});
105-
} catch (error) {
106-
if (error instanceof Error) {
107-
renderErrorMessage = error.message;
108-
}
109-
}
110-
111-
expect(renderErrorMessage).toEqual('Failed to fetch workflow summary');
100+
setup({ isError: true });
101+
102+
await waitFor(() => {
103+
expect(screen.queryByRole('button')).not.toBeInTheDocument();
104+
});
112105
});
113106
});
114107

115-
async function setup({
108+
function setup({
116109
isError,
117110
isConfigError,
118111
}: {
@@ -121,53 +114,48 @@ async function setup({
121114
}) {
122115
const user = userEvent.setup();
123116

124-
const renderResult = render(
125-
<Suspense>
126-
<WorkflowActions />
127-
</Suspense>,
128-
{
129-
endpointsMocks: [
130-
{
131-
path: '/api/domains/:domain/:cluster/workflows/:workflowId/:runId',
132-
httpMethod: 'GET',
133-
httpResolver: () => {
134-
if (isError) {
135-
return HttpResponse.json(
136-
{ message: 'Failed to fetch workflow summary' },
137-
{ status: 500 }
138-
);
139-
} else {
140-
return HttpResponse.json(describeWorkflowResponse, {
141-
status: 200,
142-
});
143-
}
144-
},
117+
const renderResult = render(<WorkflowActions />, {
118+
endpointsMocks: [
119+
{
120+
path: '/api/domains/:domain/:cluster/workflows/:workflowId/:runId',
121+
httpMethod: 'GET',
122+
httpResolver: () => {
123+
if (isError) {
124+
return HttpResponse.json(
125+
{ message: 'Failed to fetch workflow summary' },
126+
{ status: 500 }
127+
);
128+
} else {
129+
return HttpResponse.json(describeWorkflowResponse, {
130+
status: 200,
131+
});
132+
}
145133
},
146-
{
147-
path: '/api/config',
148-
httpMethod: 'GET',
149-
httpResolver: () => {
150-
if (isConfigError) {
151-
return HttpResponse.json(
152-
{ message: 'Failed to fetch config' },
153-
{ status: 500 }
154-
);
155-
} else {
156-
return HttpResponse.json(
157-
{
158-
terminate: true,
159-
cancel: true,
160-
},
161-
{
162-
status: 200,
163-
}
164-
);
165-
}
166-
},
134+
},
135+
{
136+
path: '/api/config',
137+
httpMethod: 'GET',
138+
httpResolver: () => {
139+
if (isConfigError) {
140+
return HttpResponse.json(
141+
{ message: 'Failed to fetch config' },
142+
{ status: 500 }
143+
);
144+
} else {
145+
return HttpResponse.json(
146+
{
147+
terminate: true,
148+
cancel: true,
149+
},
150+
{
151+
status: 200,
152+
}
153+
);
154+
}
167155
},
168-
],
169-
}
170-
);
156+
},
157+
],
158+
});
171159

172160
return { user, renderResult };
173161
}

src/views/workflow-actions/workflow-actions-menu/helpers/__tests__/get-action-disabled-reason.test.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,15 @@ describe(getActionDisabledReason.name, () => {
1212
expect(result).toBeUndefined();
1313
});
1414

15+
it('returns undefined when workflow status is not loaded', () => {
16+
const result = getActionDisabledReason({
17+
actionEnabledConfig: 'ENABLED',
18+
actionRunnableStatus: undefined,
19+
});
20+
21+
expect(result).toBeUndefined();
22+
});
23+
1524
it('returns disabled message when config is not loaded', () => {
1625
const result = getActionDisabledReason({
1726
actionEnabledConfig: undefined,

src/views/workflow-actions/workflow-actions-menu/helpers/get-action-disabled-reason.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,12 @@ export default function getActionDisabledReason({
99
actionRunnableStatus,
1010
}: {
1111
actionEnabledConfig?: WorkflowActionEnabledConfigValue;
12-
actionRunnableStatus: WorkflowActionRunnableStatus;
12+
actionRunnableStatus?: WorkflowActionRunnableStatus;
1313
}): string | undefined {
14+
if (!actionRunnableStatus) {
15+
return undefined;
16+
}
17+
1418
if (actionEnabledConfig !== 'ENABLED') {
1519
return WORKFLOW_ACTIONS_DISABLED_REASONS_CONFIG[
1620
actionEnabledConfig ?? 'DISABLED_DEFAULT'

src/views/workflow-actions/workflow-actions-menu/workflow-actions-menu.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@ export default function WorkflowActionsMenu({
1717
{workflowActionsConfig.map((action) => {
1818
const actionDisabledReason = getActionDisabledReason({
1919
actionEnabledConfig: actionsEnabledConfig?.[action.id],
20-
actionRunnableStatus: action.getRunnableStatus(workflow),
20+
actionRunnableStatus: workflow
21+
? action.getRunnableStatus(workflow)
22+
: undefined,
2123
});
2224

2325
return (

src/views/workflow-actions/workflow-actions-menu/workflow-actions-menu.types.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { type DescribeWorkflowResponse } from '@/route-handlers/describe-workflo
44
import { type WorkflowAction } from '../workflow-actions.types';
55

66
export type Props = {
7-
workflow: DescribeWorkflowResponse;
7+
workflow?: DescribeWorkflowResponse;
88
actionsEnabledConfig?: WorkflowActionsEnabledConfig;
99
onActionSelect: (action: WorkflowAction<any, any, any>) => void;
1010
};

src/views/workflow-actions/workflow-actions.tsx

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import { useParams } from 'next/navigation';
1616
import { MdArrowDropDown } from 'react-icons/md';
1717

1818
import useConfigValue from '@/hooks/use-config-value/use-config-value';
19-
import useDescribeWorkflow from '@/views/workflow-page/hooks/use-describe-workflow';
19+
import { useDescribeWorkflow } from '@/views/workflow-page/hooks/use-describe-workflow';
2020
import { type WorkflowPageParams } from '@/views/workflow-page/workflow-page.types';
2121

2222
import WorkflowActionsMenu from './workflow-actions-menu/workflow-actions-menu';
@@ -34,22 +34,28 @@ export default function WorkflowActions() {
3434
'domain'
3535
);
3636

37-
const { data: workflow } = useDescribeWorkflow({
37+
const {
38+
data: workflow,
39+
isLoading: isWorkflowLoading,
40+
error: workflowError,
41+
} = useDescribeWorkflow({
3842
...workflowDetailsParams,
3943
});
4044

41-
const { data: actionsEnabledConfig, isLoading } = useConfigValue(
42-
'WORKFLOW_ACTIONS_ENABLED',
43-
{
45+
const { data: actionsEnabledConfig, isLoading: isActionsEnabledLoading } =
46+
useConfigValue('WORKFLOW_ACTIONS_ENABLED', {
4447
domain: params.domain,
4548
cluster: params.cluster,
46-
}
47-
);
49+
});
4850

4951
const [selectedAction, setSelectedAction] = useState<
5052
WorkflowAction<any, any, any> | undefined
5153
>(undefined);
5254

55+
if (workflowError) {
56+
return null;
57+
}
58+
5359
return (
5460
<SnackbarProvider
5561
placement={SNACKBAR_PLACEMENT.bottom}
@@ -77,7 +83,7 @@ export default function WorkflowActions() {
7783
kind={KIND.secondary}
7884
overrides={overrides.button}
7985
endEnhancer={<MdArrowDropDown size={20} />}
80-
isLoading={isLoading}
86+
isLoading={isWorkflowLoading || isActionsEnabledLoading}
8187
>
8288
Workflow Actions
8389
</Button>

src/views/workflow-history/workflow-history.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import { type RequestError } from '@/utils/request/request-error';
2525
import sortBy from '@/utils/sort-by';
2626

2727
import workflowPageQueryParamsConfig from '../workflow-page/config/workflow-page-query-params.config';
28-
import useDescribeWorkflow from '../workflow-page/hooks/use-describe-workflow';
28+
import { useSuspenseDescribeWorkflow } from '../workflow-page/hooks/use-describe-workflow';
2929

3030
import workflowHistoryFiltersConfig from './config/workflow-history-filters.config';
3131
import getVisibleGroupsHasMissingEvents from './helpers/get-visible-groups-has-missing-events';
@@ -69,7 +69,9 @@ export default function WorkflowHistory({ params }: Props) {
6969
pageFiltersConfig: workflowHistoryFiltersConfig,
7070
});
7171

72-
const { data: wfExecutionDescription } = useDescribeWorkflow({ ...params });
72+
const { data: wfExecutionDescription } = useSuspenseDescribeWorkflow({
73+
...params,
74+
});
7375
const { workflowExecutionInfo } = wfExecutionDescription;
7476
const {
7577
data: result,
Lines changed: 42 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,45 +1,59 @@
11
'use client';
22

3-
import { useSuspenseQuery } from '@tanstack/react-query';
3+
import { useQuery, useSuspenseQuery } from '@tanstack/react-query';
44

55
import { type DescribeWorkflowResponse } from '@/route-handlers/describe-workflow/describe-workflow.types';
66
import request from '@/utils/request';
77
import { type RequestError } from '@/utils/request/request-error';
88

99
import WORKFLOW_PAGE_STATUS_REFETCH_INTERVAL from '../config/workflow-page-status-refetch-interval.config';
1010

11-
type Props = {
12-
domain: string;
13-
cluster: string;
14-
workflowId: string;
15-
runId: string;
16-
refetchInterval?: number;
17-
};
11+
import {
12+
type DescribeWorkflowQueryKey,
13+
type UseDescribeWorkflowParams,
14+
} from './use-describe-workflow.types';
1815

19-
export default function useDescribeWorkflow({
16+
const getDefaultConfigurations = ({
2017
refetchInterval = WORKFLOW_PAGE_STATUS_REFETCH_INTERVAL,
2118
...params
22-
}: Props) {
19+
}: UseDescribeWorkflowParams) => ({
20+
queryKey: ['describe_workflow', params] as DescribeWorkflowQueryKey,
21+
queryFn: ({ queryKey: [_, p] }: { queryKey: DescribeWorkflowQueryKey }) =>
22+
request(
23+
`/api/domains/${p.domain}/${p.cluster}/workflows/${p.workflowId}/${p.runId}`
24+
).then((res) => res.json()),
25+
refetchInterval: (query: { state: { data?: DescribeWorkflowResponse } }) => {
26+
const { closeStatus } = query.state.data?.workflowExecutionInfo || {};
27+
if (
28+
!closeStatus ||
29+
closeStatus === 'WORKFLOW_EXECUTION_CLOSE_STATUS_INVALID'
30+
)
31+
return refetchInterval;
32+
33+
return false;
34+
},
35+
});
36+
37+
export function useDescribeWorkflow({
38+
refetchInterval = WORKFLOW_PAGE_STATUS_REFETCH_INTERVAL,
39+
...params
40+
}: UseDescribeWorkflowParams) {
41+
return useQuery<
42+
DescribeWorkflowResponse,
43+
RequestError,
44+
DescribeWorkflowResponse,
45+
DescribeWorkflowQueryKey
46+
>(getDefaultConfigurations({ refetchInterval, ...params }));
47+
}
48+
49+
export function useSuspenseDescribeWorkflow({
50+
refetchInterval = WORKFLOW_PAGE_STATUS_REFETCH_INTERVAL,
51+
...params
52+
}: UseDescribeWorkflowParams) {
2353
return useSuspenseQuery<
2454
DescribeWorkflowResponse,
2555
RequestError,
2656
DescribeWorkflowResponse,
27-
[string, typeof params]
28-
>({
29-
queryKey: ['describe_workflow', params] as const,
30-
queryFn: ({ queryKey: [_, p] }) =>
31-
request(
32-
`/api/domains/${p.domain}/${p.cluster}/workflows/${p.workflowId}/${p.runId}`
33-
).then((res) => res.json()),
34-
refetchInterval: (query) => {
35-
const { closeStatus } = query.state.data?.workflowExecutionInfo || {};
36-
if (
37-
!closeStatus ||
38-
closeStatus === 'WORKFLOW_EXECUTION_CLOSE_STATUS_INVALID'
39-
)
40-
return refetchInterval;
41-
42-
return false;
43-
},
44-
});
57+
DescribeWorkflowQueryKey
58+
>(getDefaultConfigurations({ refetchInterval, ...params }));
4559
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
export type UseDescribeWorkflowParams = {
2+
domain: string;
3+
cluster: string;
4+
workflowId: string;
5+
runId: string;
6+
refetchInterval?: number;
7+
};
8+
9+
export type DescribeWorkflowQueryKey = [
10+
'describe_workflow',
11+
Omit<UseDescribeWorkflowParams, 'refetchInterval'>,
12+
];

0 commit comments

Comments
 (0)