Skip to content

Commit 4d1f6f7

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request openshift#16077 from cajieh/refactor-console-shared-package-firehose-usek8swatchresource
CONSOLE-5087: Refactor console-shared-package from Firehose to useK8sWatchResource
2 parents 42e0cce + 070c80d commit 4d1f6f7

23 files changed

Lines changed: 871 additions & 253 deletions

File tree

‎frontend/packages/console-shared/src/components/formik-fields/ResourceDropdownField.tsx‎

Lines changed: 37 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
import type { FC } from 'react';
2+
import { useMemo } from 'react';
23
import { FormGroup, FormHelperText, HelperText, HelperTextItem } from '@patternfly/react-core';
34
import type { FormikValues } from 'formik';
45
import { useField, useFormikContext } from 'formik';
5-
import { Firehose } from '@console/internal/components/utils/firehose';
6-
import type { FirehoseResource } from '@console/internal/components/utils/types';
6+
import type { FirehoseResult } from '@console/internal/components/utils/types';
77
import type { K8sResourceKind } from '@console/internal/module/k8s';
88
import { useFormikValidationFix } from '../../hooks/useFormikValidationFix';
99
import type { ResourceDropdownItems } from '../dropdown/ResourceDropdown';
@@ -13,7 +13,7 @@ import { getFieldId } from './field-utils';
1313

1414
export interface ResourceDropdownFieldProps extends DropdownFieldProps {
1515
dataSelector: string[] | number[] | symbol[];
16-
resources: FirehoseResource[];
16+
resources: FirehoseResult[];
1717
showBadge?: boolean;
1818
onLoad?: (items: ResourceDropdownItems) => void;
1919
onChange?: (key: string, name?: string | object, resource?: K8sResourceKind) => void;
@@ -50,24 +50,42 @@ const ResourceDropdownField: FC<ResourceDropdownFieldProps> = ({
5050

5151
useFormikValidationFix(field.value);
5252

53+
// Derive loaded and loadError from resources array for ResourceDropdown
54+
// ResourceDropdown expects these as top-level props to manage loading state
55+
const { loaded, loadError } = useMemo(() => {
56+
if (!resources || resources.length === 0) {
57+
return { loaded: true, loadError: undefined };
58+
}
59+
type ResourceWithOptional = { loaded: boolean; loadError?: unknown; optional?: boolean };
60+
const requiredResources = (resources as ResourceWithOptional[]).filter((r) => !r.optional);
61+
const target = requiredResources.length ? requiredResources : resources;
62+
const allLoaded = target.every((r) => r.loaded);
63+
const resourceWithLoadError = target.find((r) => r.loadError);
64+
return {
65+
loaded: allLoaded,
66+
loadError: resourceWithLoadError?.loadError,
67+
};
68+
}, [resources]);
69+
5370
return (
5471
<FormGroup fieldId={fieldId} label={label} isRequired={required} data-test={dataTest}>
55-
<Firehose resources={resources}>
56-
<ResourceDropdown
57-
{...props}
58-
id={fieldId}
59-
dataSelector={dataSelector}
60-
selectedKey={field.value}
61-
isFullWidth={fullWidth}
62-
onLoad={onLoad}
63-
resourceFilter={resourceFilter}
64-
onChange={(value: string, name: string | object, resource: K8sResourceKind) => {
65-
props.onChange && props.onChange(value, name, resource);
66-
setFieldValue(props.name, value);
67-
setFieldTouched(props.name, true);
68-
}}
69-
/>
70-
</Firehose>
72+
<ResourceDropdown
73+
{...props}
74+
id={fieldId}
75+
dataSelector={dataSelector}
76+
selectedKey={field.value}
77+
isFullWidth={fullWidth}
78+
onLoad={onLoad}
79+
resourceFilter={resourceFilter}
80+
resources={resources}
81+
loaded={loaded}
82+
loadError={loadError}
83+
onChange={(value: string, name: string | object, resource: K8sResourceKind) => {
84+
props.onChange && props.onChange(value, name, resource);
85+
setFieldValue(props.name, value);
86+
setFieldTouched(props.name, true);
87+
}}
88+
/>
7189

7290
<FormHelperText>
7391
<HelperText>

‎frontend/packages/dev-console/src/components/buildconfig/sections/SecretsSection.tsx‎

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
import type { FC } from 'react';
2+
import { useMemo } from 'react';
23
import { TextInputTypes } from '@patternfly/react-core';
34
import * as fuzzy from 'fuzzysearch';
45
import { useTranslation } from 'react-i18next';
5-
import type { FirehoseResource } from '@console/internal/components/utils';
6+
import { useK8sWatchResources } from '@console/internal/components/utils/k8s-watch-hook';
67
import { SecretModel } from '@console/internal/models';
8+
import type { SecretKind } from '@console/internal/module/k8s';
9+
import { referenceForModel } from '@console/internal/module/k8s';
710
import { InputField, MultiColumnField, ResourceDropdownField } from '@console/shared';
811
import FormSection from '../../import/section/FormSection';
912

@@ -20,14 +23,30 @@ const SecretsSection: FC<{ namespace: string }> = ({ namespace }) => {
2023
const { t } = useTranslation();
2124

2225
const autocompleteFilter = (text: string, item: any): boolean => fuzzy(text, item?.props?.name);
23-
const resources: FirehoseResource[] = [
24-
{
26+
27+
const watchedResources = useK8sWatchResources<{ secrets: SecretKind[] }>({
28+
secrets: {
2529
isList: true,
26-
kind: SecretModel.kind,
27-
prop: SecretModel.id,
30+
kind: referenceForModel(SecretModel),
2831
namespace,
2932
},
30-
];
33+
});
34+
35+
const resources = useMemo(
36+
() => [
37+
{
38+
data: watchedResources.secrets.data,
39+
loaded: watchedResources.secrets.loaded,
40+
loadError: watchedResources.secrets.loadError,
41+
kind: SecretModel.kind,
42+
},
43+
],
44+
[
45+
watchedResources.secrets.data,
46+
watchedResources.secrets.loaded,
47+
watchedResources.secrets.loadError,
48+
],
49+
);
3150

3251
const mountPointLabel = t('devconsole~Mount point');
3352

‎frontend/packages/dev-console/src/components/buildconfig/sections/TriggersSection.tsx‎

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
import type { FC } from 'react';
2+
import { useMemo } from 'react';
23
import { FormGroup } from '@patternfly/react-core';
34
import { useField } from 'formik';
45
import * as fuzzy from 'fuzzysearch';
56
import { useTranslation } from 'react-i18next';
6-
import type { FirehoseResource } from '@console/internal/components/utils';
7+
import { useK8sWatchResources } from '@console/internal/components/utils/k8s-watch-hook';
78
import { SecretModel } from '@console/internal/models';
9+
import type { SecretKind } from '@console/internal/module/k8s';
10+
import { referenceForModel } from '@console/internal/module/k8s';
811
import {
912
DropdownField,
1013
MultiColumnField,
@@ -42,14 +45,30 @@ const TriggersSection: FC<{ namespace: string }> = ({ namespace }) => {
4245
};
4346

4447
const autocompleteFilter = (text: string, item: any): boolean => fuzzy(text, item?.props?.name);
45-
const resources: FirehoseResource[] = [
46-
{
48+
49+
const watchedResources = useK8sWatchResources<{ secrets: SecretKind[] }>({
50+
secrets: {
4751
isList: true,
48-
kind: SecretModel.kind,
49-
prop: SecretModel.id,
52+
kind: referenceForModel(SecretModel),
5053
namespace,
5154
},
52-
];
55+
});
56+
57+
const resources = useMemo(
58+
() => [
59+
{
60+
data: watchedResources.secrets.data,
61+
loaded: watchedResources.secrets.loaded,
62+
loadError: watchedResources.secrets.loadError,
63+
kind: SecretModel.kind,
64+
},
65+
],
66+
[
67+
watchedResources.secrets.data,
68+
watchedResources.secrets.loaded,
69+
watchedResources.secrets.loadError,
70+
],
71+
);
5372

5473
return (
5574
<FormSection title={t('devconsole~Triggers')} dataTest="section triggers">

‎frontend/packages/dev-console/src/components/buildconfig/sections/__tests__/TriggersSection.spec.tsx‎

Lines changed: 85 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,38 @@
11
import type { FC, ReactNode } from 'react';
2-
import { render, waitFor } from '@testing-library/react';
2+
import { screen, waitFor } from '@testing-library/react';
33
import userEvent from '@testing-library/user-event';
4-
import type { FormikConfig } from 'formik';
54
import { Formik } from 'formik';
5+
import { useK8sWatchResources } from '@console/internal/components/utils/k8s-watch-hook';
6+
import { renderWithProviders } from '@console/shared/src/test-utils/unit-test-utils';
67
import type { TriggersSectionFormData } from '../TriggersSection';
78
import TriggersSection from '../TriggersSection';
89

9-
interface WrapperProps extends FormikConfig<TriggersSectionFormData> {
10-
children?: ReactNode;
10+
// Mock PatternFly topology to prevent console warnings during tests
11+
jest.mock('@patternfly/react-topology', () => ({}));
12+
13+
jest.mock('@console/internal/components/utils/k8s-watch-hook', () => ({
14+
useK8sWatchResources: jest.fn(),
15+
}));
16+
17+
const mockUseK8sWatchResources = useK8sWatchResources as jest.Mock;
18+
19+
const initialValues: TriggersSectionFormData = {
20+
formData: {
21+
triggers: {
22+
configChange: false,
23+
imageChange: false,
24+
otherTriggers: [],
25+
},
26+
},
27+
};
28+
29+
interface FormikWrapperProps {
30+
children: ReactNode;
31+
onSubmit: jest.Mock;
1132
}
1233

13-
const Wrapper: FC<WrapperProps> = ({ children, ...formikConfig }) => (
14-
<Formik {...formikConfig}>
34+
const FormikWrapper: FC<FormikWrapperProps> = ({ children, onSubmit }) => (
35+
<Formik initialValues={initialValues} onSubmit={onSubmit}>
1536
{(formikProps) => (
1637
<form onSubmit={formikProps.handleSubmit}>
1738
{children}
@@ -21,100 +42,93 @@ const Wrapper: FC<WrapperProps> = ({ children, ...formikConfig }) => (
2142
</Formik>
2243
);
2344

24-
const initialValues: TriggersSectionFormData = {
25-
formData: {
26-
triggers: {
27-
configChange: false,
28-
imageChange: false,
29-
otherTriggers: [],
30-
},
31-
},
45+
const renderTriggersSection = (onSubmit: jest.Mock) => {
46+
renderWithProviders(
47+
<FormikWrapper onSubmit={onSubmit}>
48+
<TriggersSection namespace="a-namespace" />
49+
</FormikWrapper>,
50+
);
3251
};
3352

3453
describe('TriggersSection', () => {
35-
it('should render empty form', () => {
54+
beforeEach(() => {
55+
mockUseK8sWatchResources.mockReturnValue({
56+
secrets: { data: [], loaded: true, loadError: null },
57+
});
58+
});
59+
60+
afterEach(() => {
61+
jest.restoreAllMocks();
62+
});
63+
64+
it('should render all trigger configuration options', () => {
3665
const onSubmit = jest.fn();
3766

38-
const renderResult = render(
39-
<Wrapper initialValues={initialValues} onSubmit={onSubmit}>
40-
<TriggersSection namespace="a-namespace" />
41-
</Wrapper>,
42-
);
67+
renderTriggersSection(onSubmit);
4368

44-
renderResult.getByTestId('section triggers');
45-
renderResult.getByText('Triggers');
46-
renderResult.getByText('Automatically build a new image when config changes');
47-
renderResult.getByText('Automatically build a new image when image changes');
48-
renderResult.getByText('Add trigger');
69+
expect(screen.getByTestId('section triggers')).toBeInTheDocument();
70+
expect(screen.getByText('Triggers')).toBeVisible();
71+
expect(
72+
screen.getByText('Automatically build a new image when config changes'),
73+
).toBeInTheDocument();
74+
expect(screen.getByText('Automatically build a new image when image changes')).toBeVisible();
75+
expect(screen.getByRole('button', { name: 'Add trigger' })).toBeVisible();
4976

50-
expect((renderResult.getByTestId('image-change checkbox') as HTMLInputElement).checked).toBe(
51-
false,
52-
);
77+
const imageChangeCheckbox = screen.getByTestId('image-change checkbox') as HTMLInputElement;
78+
expect(imageChangeCheckbox).not.toBeChecked();
5379

54-
expect(onSubmit).toHaveBeenCalledTimes(0);
80+
expect(onSubmit).not.toHaveBeenCalled();
5581
});
5682

57-
it('should allow user to change config change checkbox trigger and save this data', async () => {
83+
it('should toggle config change checkbox and submit form with updated value', async () => {
5884
const user = userEvent.setup();
5985
const onSubmit = jest.fn();
86+
renderTriggersSection(onSubmit);
6087

61-
const renderResult = render(
62-
<Wrapper initialValues={initialValues} onSubmit={onSubmit}>
63-
<TriggersSection namespace="a-namespace" />
64-
</Wrapper>,
65-
);
66-
67-
// Change form
68-
await user.click(renderResult.getByTestId('config-change checkbox'));
88+
const configChangeCheckbox = screen.getByTestId('config-change checkbox');
89+
await user.click(configChangeCheckbox);
6990

70-
// Submit
71-
const submitButton = renderResult.getByRole('button', { name: 'Submit' });
91+
const submitButton = screen.getByRole('button', { name: 'Submit' });
7292
await user.click(submitButton);
93+
7394
await waitFor(() => {
7495
expect(onSubmit).toHaveBeenCalledTimes(1);
7596
});
7697

77-
const expectedFormData: TriggersSectionFormData = {
78-
formData: {
79-
triggers: {
80-
configChange: true,
81-
imageChange: false,
82-
otherTriggers: [],
83-
},
84-
},
85-
};
86-
expect(onSubmit).toHaveBeenLastCalledWith(expectedFormData, expect.anything());
98+
expect(onSubmit.mock.calls[0][0]).toEqual(
99+
expect.objectContaining({
100+
formData: expect.objectContaining({
101+
triggers: expect.objectContaining({
102+
configChange: true,
103+
}),
104+
}),
105+
}),
106+
);
87107
});
88108

89-
it('should allow user to change image change checkbox trigger and save this data', async () => {
109+
it('should toggle image change checkbox and submit form with updated value', async () => {
90110
const user = userEvent.setup();
91111
const onSubmit = jest.fn();
112+
renderTriggersSection(onSubmit);
92113

93-
const renderResult = render(
94-
<Wrapper initialValues={initialValues} onSubmit={onSubmit}>
95-
<TriggersSection namespace="a-namespace" />
96-
</Wrapper>,
97-
);
98-
99-
// Change form
100-
await user.click(renderResult.getByTestId('image-change checkbox'));
114+
const imageChangeCheckbox = screen.getByTestId('image-change checkbox');
115+
await user.click(imageChangeCheckbox);
101116

102-
// Submit
103-
const submitButton = renderResult.getByRole('button', { name: 'Submit' });
117+
const submitButton = screen.getByRole('button', { name: 'Submit' });
104118
await user.click(submitButton);
119+
105120
await waitFor(() => {
106121
expect(onSubmit).toHaveBeenCalledTimes(1);
107122
});
108123

109-
const expectedFormData: TriggersSectionFormData = {
110-
formData: {
111-
triggers: {
112-
configChange: false,
113-
imageChange: true,
114-
otherTriggers: [],
115-
},
116-
},
117-
};
118-
expect(onSubmit).toHaveBeenLastCalledWith(expectedFormData, expect.anything());
124+
expect(onSubmit.mock.calls[0][0]).toEqual(
125+
expect.objectContaining({
126+
formData: expect.objectContaining({
127+
triggers: expect.objectContaining({
128+
imageChange: true,
129+
}),
130+
}),
131+
}),
132+
);
119133
});
120134
});

0 commit comments

Comments
 (0)