Skip to content

Commit adc7e77

Browse files
sahil143sahil143
authored
Merge pull request #45 from testcara/KFLUXUI-198
fix(KFLUXUI-198): component relationship remove button
2 parents f80051c + 094af03 commit adc7e77

9 files changed

+205
-18
lines changed

‎src/components/ComponentRelation/ComponentRelationForm.tsx

+17-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import * as React from 'react';
2-
import { Bullseye, Flex, FlexItem, Grid, GridItem, Radio } from '@patternfly/react-core';
2+
import { Bullseye, Button, Flex, FlexItem, Grid, GridItem, Radio } from '@patternfly/react-core';
3+
import { MinusCircleIcon } from '@patternfly/react-icons/dist/esm/icons/minus-circle-icon';
34
import { useField } from 'formik';
45
import { HelpTooltipIcon } from '../../shared';
56
import {
@@ -12,12 +13,17 @@ type ComponentRelationProps = {
1213
componentNames: string[];
1314
groupedComponents: { [application: string]: string[] };
1415
index?: number;
16+
removeProps: {
17+
disableRemove: boolean;
18+
onRemove: () => void;
19+
};
1520
};
1621

1722
export const ComponentRelation: React.FC<ComponentRelationProps> = ({
1823
index,
1924
componentNames,
2025
groupedComponents,
26+
removeProps: { disableRemove, onRemove },
2127
}) => {
2228
const sourceName = `relations.${index.toString()}.source`;
2329
const nudgeName = `relations.${index.toString()}.nudgeType`;
@@ -87,6 +93,16 @@ export const ComponentRelation: React.FC<ComponentRelationProps> = ({
8793
groupedComponents={groupedComponents}
8894
/>
8995
</GridItem>
96+
<GridItem span={1}>
97+
<Button
98+
id={`remove-relation-${index}`}
99+
variant="plain"
100+
onClick={onRemove}
101+
isDisabled={disableRemove}
102+
>
103+
<MinusCircleIcon />
104+
</Button>
105+
</GridItem>
90106
</Grid>
91107
);
92108
};

‎src/components/ComponentRelation/ComponentRelationModal.tsx

+11-9
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,14 @@ export const ComponentRelationModal: React.FC<ComponentRelationModalProps> = ({
6060
setShowCancelModal(true);
6161
}, [application, track, workspace]);
6262

63-
const initialValues: ComponentRelationFormikValue = {
64-
relations:
65-
loaded && !error && nudgeData.length > 0
66-
? nudgeData
67-
: [{ source: '', nudgeType: ComponentRelationNudgeType.NUDGES, target: [] }],
68-
};
63+
const initialValues: ComponentRelationFormikValue = React.useMemo(() => {
64+
return {
65+
relations:
66+
loaded && !error && nudgeData.length > 0
67+
? nudgeData
68+
: [{ source: '', nudgeType: ComponentRelationNudgeType.NUDGES, target: [] }],
69+
};
70+
}, [error, loaded, nudgeData]);
6971

7072
const handleSubmit = React.useCallback(
7173
async (
@@ -78,15 +80,15 @@ export const ComponentRelationModal: React.FC<ComponentRelationModalProps> = ({
7880
workspace,
7981
});
8082
try {
81-
await updateNudgeDependencies(values.relations, namespace, true);
83+
await updateNudgeDependencies(values.relations, initialValues.relations, namespace, true);
8284
} catch (e) {
8385
// eslint-disable-next-line no-console
8486
console.error(`Error while updating dependency data for component`, e);
8587

8688
helpers.setSubmitting(false);
8789
helpers.setStatus({ submitError: e?.message });
8890
}
89-
return updateNudgeDependencies(values.relations, namespace)
91+
return updateNudgeDependencies(values.relations, initialValues.relations, namespace)
9092
.then((compResults: ComponentKind[]) => {
9193
compResults.forEach((c) => {
9294
track('Component relationship updated', {
@@ -105,7 +107,7 @@ export const ComponentRelationModal: React.FC<ComponentRelationModalProps> = ({
105107
helpers.setStatus({ submitError: e?.message });
106108
});
107109
},
108-
[application, namespace, onSaveRelationships, track, workspace],
110+
[application, namespace, onSaveRelationships, track, workspace, initialValues],
109111
);
110112

111113
if (showSubmissionModal && !showCancelModal) {

‎src/components/ComponentRelation/__tests__/ComponentRelationForm.spec.tsx

+11-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { configure, screen } from '@testing-library/react';
1+
import { act, configure, fireEvent, screen } from '@testing-library/react';
22
import { formikRenderer } from '../../../utils/test-utils';
33
import { ComponentRelation } from '../ComponentRelationForm';
44
import { ComponentRelationNudgeType } from '../type';
@@ -12,6 +12,10 @@ describe('ComponentRelationForm', () => {
1212
index={0}
1313
componentNames={['asdf', 'asd']}
1414
groupedComponents={{ app: ['asdf', 'asd'] }}
15+
removeProps={{
16+
disableRemove: true,
17+
onRemove: jest.fn(),
18+
}}
1519
/>,
1620
{
1721
relations: [
@@ -22,5 +26,11 @@ describe('ComponentRelationForm', () => {
2226
expect(screen.getAllByTestId('toggle-component-menu')).toHaveLength(2);
2327
expect(screen.getAllByTestId('nudges-0')).toHaveLength(1);
2428
expect(screen.getAllByTestId('nudged-by-0')).toHaveLength(1);
29+
// mouseOver help icon
30+
const nodgeSvg = screen.getAllByRole('img', { hidden: true })[1];
31+
act(() => {
32+
fireEvent.mouseEnter(nodgeSvg);
33+
});
34+
expect(nodgeSvg.getAttributeNames().includes('aria-describedby'));
2535
});
2636
});

‎src/components/ComponentRelation/__tests__/ComponentRelationModal.spec.tsx

+69-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
import { configure, fireEvent, render, screen, waitFor } from '@testing-library/react';
22
import { componentCRMocks } from '../../Components/__data__/mock-data';
33
import { ComponentRelationModal } from '../ComponentRelationModal';
4+
import { ComponentRelationNudgeType, ComponentRelationValue } from '../type';
5+
import { useNudgeData } from '../useNudgeData';
6+
import { updateNudgeDependencies } from '../utils';
47

58
configure({ testIdAttribute: 'id' });
69

@@ -20,7 +23,11 @@ jest.mock('../../../utils/analytics', () => ({
2023

2124
jest.mock('../utils', () => ({
2225
...jest.requireActual('../utils'),
23-
updateNudgeDependencies: jest.fn(() => Promise.resolve([])),
26+
updateNudgeDependencies: jest.fn(),
27+
}));
28+
29+
jest.mock('../useNudgeData', () => ({
30+
useNudgeData: jest.fn(),
2431
}));
2532

2633
class MockResizeObserver {
@@ -37,9 +44,29 @@ class MockResizeObserver {
3744
}
3845
}
3946

47+
const mockComponentRelations: ComponentRelationValue[] = [
48+
{
49+
source: 'a',
50+
nudgeType: ComponentRelationNudgeType.NUDGES,
51+
target: ['b'],
52+
},
53+
{
54+
source: 'c',
55+
nudgeType: ComponentRelationNudgeType.NUDGES,
56+
target: ['d'],
57+
},
58+
];
59+
4060
window.ResizeObserver = MockResizeObserver;
61+
const useNudgeDataMock = useNudgeData as jest.Mock;
62+
const updateNudgeDependenciesMock = updateNudgeDependencies as jest.Mock;
4163

4264
describe('ComponentRelationModal', () => {
65+
beforeEach(() => {
66+
useNudgeDataMock.mockReturnValue([[], true, null]);
67+
updateNudgeDependenciesMock.mockResolvedValue(componentCRMocks);
68+
});
69+
4370
it('should render modal', () => {
4471
render(<ComponentRelationModal modalProps={{ isOpen: true }} application="apps" />);
4572
screen.getByText('Component relationships');
@@ -50,6 +77,15 @@ describe('ComponentRelationModal', () => {
5077
expect(screen.getAllByTestId('toggle-component-menu')).toHaveLength(2);
5178
});
5279

80+
it('should remove a relation', () => {
81+
render(<ComponentRelationModal modalProps={{ isOpen: true }} application="apps" />);
82+
expect(screen.queryAllByTestId(/remove-relation-\d+/)).toHaveLength(1);
83+
fireEvent.click(screen.getByText(`Add another component relationship`));
84+
expect(screen.getAllByTestId(/remove-relation-\d+/)).toHaveLength(2);
85+
fireEvent.click(screen.getByTestId('remove-relation-0'));
86+
expect(screen.queryAllByTestId(/remove-relation-\d+/)).toHaveLength(1);
87+
});
88+
5389
it('should show cancelation modal when clicked on cancel', () => {
5490
let isOpen = true;
5591
const onClose = () => {
@@ -79,17 +115,47 @@ describe('ComponentRelationModal', () => {
79115
});
80116

81117
it('should show confirmation modal on relationship save', async () => {
118+
useNudgeDataMock.mockReturnValue([mockComponentRelations, true, null]);
119+
updateNudgeDependenciesMock.mockResolvedValue(componentCRMocks);
82120
let isOpen = true;
83121
const onClose = () => {
84122
isOpen = false;
85123
};
86-
render(<ComponentRelationModal modalProps={{ isOpen, onClose }} application="apps" />);
124+
const { rerender } = render(
125+
<ComponentRelationModal modalProps={{ isOpen, onClose }} application="apps" />,
126+
);
87127
expect(screen.queryByText('Component relationships')).toBeInTheDocument();
88128
fireEvent.click(screen.getByTestId('nudged-by-0'));
89129
const saveButton = screen.getByText('Save relationships');
90130
expect(saveButton.getAttribute('class')).not.toContain('pf-m-disabled');
91131
fireEvent.click(saveButton);
92132
expect(saveButton.getAttribute('class')).toContain('pf-m-in-progress');
93-
await waitFor(() => expect(saveButton.getAttribute('class')).not.toContain('pf-m-in-progress'));
133+
await waitFor(() => {
134+
expect(saveButton.getAttribute('class')).toContain('pf-m-in-progress');
135+
});
136+
137+
rerender(<ComponentRelationModal modalProps={{ isOpen, onClose }} application="apps" />);
138+
await waitFor(() => {
139+
expect(screen.queryByText('Relationships updated!')).toBeInTheDocument();
140+
});
141+
142+
const doneButton = screen.getByText('Done');
143+
fireEvent.click(doneButton);
144+
});
145+
146+
it('should display an error on failure', async () => {
147+
useNudgeDataMock.mockReturnValue([mockComponentRelations, true, null]);
148+
updateNudgeDependenciesMock.mockRejectedValue(new Error('error'));
149+
let isOpen = true;
150+
const onClose = () => {
151+
isOpen = false;
152+
};
153+
render(<ComponentRelationModal modalProps={{ isOpen, onClose }} application="apps" />);
154+
expect(screen.queryByText('Component relationships')).toBeInTheDocument();
155+
fireEvent.click(screen.getByTestId('nudged-by-0'));
156+
const saveButton = screen.getByText('Save relationships');
157+
expect(saveButton.getAttribute('class')).not.toContain('pf-m-disabled');
158+
fireEvent.click(saveButton);
159+
await waitFor(() => expect(screen.queryByText('Danger alert:')).toBeInTheDocument());
94160
});
95161
});

‎src/components/ComponentRelation/__tests__/ComponentRelationshipDropdowns.spec.tsx

+18
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,24 @@ describe('MultiSelectComponentDropdown', () => {
7979
expect(button.querySelector('.pf-m-read').innerHTML).toEqual('2');
8080
});
8181

82+
it('should select/unselect all item from menu', () => {
83+
formikRenderer(
84+
<MultiSelectComponentsDropdown groupedComponents={{ c: ['a', 'b'] }} name="multiSelect" />,
85+
{ multiSelect: '' },
86+
);
87+
screen.getByText('Choose components to nudge');
88+
const button = screen.getByTestId('toggle-component-menu');
89+
fireEvent.click(button);
90+
expect(button.querySelector('.pf-m-read')).not.toBeInTheDocument();
91+
const menu = screen.getAllByRole('menuitem');
92+
const selectAllButton = menu[0].querySelector('input');
93+
fireEvent.click(selectAllButton);
94+
expect(button.querySelector('.pf-m-read')).toBeInTheDocument();
95+
expect(button.querySelector('.pf-m-read').innerHTML).toEqual('2');
96+
fireEvent.click(selectAllButton);
97+
expect(button.querySelector('.pf-m-read')).not.toBeInTheDocument();
98+
});
99+
82100
it('should not select disabled menu item', () => {
83101
formikRenderer(
84102
<MultiSelectComponentsDropdown

‎src/components/ComponentRelation/__tests__/useComponentRelationAction.spec.ts

+9
Original file line numberDiff line numberDiff line change
@@ -34,4 +34,13 @@ describe('useComponentRelationAction', () => {
3434
`You don't have access to define component relationships`,
3535
);
3636
});
37+
38+
it('should disable action when there is one component in the app', () => {
39+
jest.clearAllMocks();
40+
// mock one component for the application
41+
mockUseComponents.mockReturnValue([[{}], true, undefined]);
42+
mockUseAccessReviewModel.mockReturnValue([true]);
43+
const { result } = renderHook(() => useComponentRelationAction('application'));
44+
expect(result.current().isDisabled).toEqual(true);
45+
});
3746
});

‎src/components/ComponentRelation/__tests__/utils.spec.ts

+50-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,40 @@
11
import { ComponentRelationNudgeType } from '../type';
2-
import { componentRelationValidationSchema, transformNudgeData } from '../utils';
2+
import {
3+
componentRelationValidationSchema,
4+
computeNudgeDataChanges,
5+
transformNudgeData,
6+
} from '../utils';
7+
8+
describe('computeNudgeDataChanges', () => {
9+
it('should compute data changes', () => {
10+
expect(
11+
computeNudgeDataChanges(
12+
[{ source: 'a', nudgeType: ComponentRelationNudgeType.NUDGES, target: ['b', 'c'] }],
13+
[],
14+
),
15+
).toEqual([{ source: 'a', nudgeType: ComponentRelationNudgeType.NUDGES, target: [] }]);
16+
expect(
17+
computeNudgeDataChanges(
18+
[],
19+
[{ source: 'a', nudgeType: ComponentRelationNudgeType.NUDGES, target: ['b', 'c'] }],
20+
),
21+
).toEqual([{ source: 'a', nudgeType: ComponentRelationNudgeType.NUDGES, target: ['b', 'c'] }]);
22+
expect(
23+
computeNudgeDataChanges(
24+
[
25+
{ source: 'a', nudgeType: ComponentRelationNudgeType.NUDGES, target: ['b'] },
26+
{ source: 'b', nudgeType: ComponentRelationNudgeType.NUDGES, target: ['c'] },
27+
{ source: 'c', nudgeType: ComponentRelationNudgeType.NUDGES, target: ['d'] },
28+
],
29+
[{ source: 'b', nudgeType: ComponentRelationNudgeType.NUDGES, target: ['c'] }],
30+
),
31+
).toEqual([
32+
{ source: 'b', nudgeType: ComponentRelationNudgeType.NUDGES, target: ['c'] },
33+
{ source: 'a', nudgeType: ComponentRelationNudgeType.NUDGES, target: [] },
34+
{ source: 'c', nudgeType: ComponentRelationNudgeType.NUDGES, target: [] },
35+
]);
36+
});
37+
});
338

439
describe('transformNudgeData', () => {
540
it('should transform data', () => {
@@ -35,7 +70,10 @@ describe('transformNudgeData', () => {
3570
describe('componentRelationValidationSchema', () => {
3671
it('should validate yup schema', async () => {
3772
const values = {
38-
relations: [{ source: 'adf', nudgeType: ComponentRelationNudgeType.NUDGES, target: [] }],
73+
relations: [
74+
{ source: 'adf', nudgeType: ComponentRelationNudgeType.NUDGES, target: [] },
75+
{ source: 'adf', nudgeType: ComponentRelationNudgeType.NUDGED_BY, target: ['b', 'c'] },
76+
],
3977
};
4078
await expect(componentRelationValidationSchema.validate(values)).resolves.toBe(values);
4179
await expect(
@@ -51,5 +89,15 @@ describe('componentRelationValidationSchema', () => {
5189
],
5290
}),
5391
).rejects.toThrow('2 errors occurred');
92+
await expect(
93+
componentRelationValidationSchema.validate({
94+
relations: [],
95+
}),
96+
).rejects.toThrowError();
97+
await expect(
98+
componentRelationValidationSchema.validate({
99+
relations: [{ source: 'adf', target: ['a'] }],
100+
}),
101+
).rejects.toThrowError();
54102
});
55103
});

‎src/components/ComponentRelation/cr-modals.tsx

+4
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,10 @@ export const DefineComponentRelationModal: React.FC<DefineComponentRelationModal
8484
componentNames={componentNames}
8585
groupedComponents={groupedComponents}
8686
index={index}
87+
removeProps={{
88+
disableRemove: values.relations.length <= 1,
89+
onRemove: () => arrayHelpers.remove(index),
90+
}}
8791
/>
8892
{index !== values.relations.length - 1 ? <Divider /> : null}
8993
</>

‎src/components/ComponentRelation/utils.ts

+16-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { union } from 'lodash-es';
1+
import { differenceBy, union } from 'lodash-es';
22
import * as yup from 'yup';
33
import { K8sQueryPatchResource } from '../../k8s';
44
import { ComponentModel } from '../../models';
@@ -55,12 +55,26 @@ export const transformNudgeData = (data: ComponentRelationValue[]): { [key: stri
5555
}, {});
5656
};
5757

58+
export const computeNudgeDataChanges = (
59+
initialValues: ComponentRelationValue[],
60+
values: ComponentRelationValue[],
61+
): ComponentRelationValue[] => {
62+
// Find the missing relation and set the target to [] to remove it
63+
const removedSources = differenceBy(initialValues, values, 'source').map((val) => ({
64+
...val,
65+
target: [],
66+
}));
67+
return [...values, ...removedSources];
68+
};
69+
5870
export const updateNudgeDependencies = async (
5971
values: ComponentRelationValue[],
72+
initialValues: ComponentRelationValue[],
6073
namespace: string,
6174
dryRun?: boolean,
6275
) => {
63-
const transformedData = transformNudgeData(values);
76+
const valueChanges = computeNudgeDataChanges(initialValues, values);
77+
const transformedData = transformNudgeData(valueChanges);
6478
const data = [];
6579
for (const [componentName, nudgeData] of Object.entries(transformedData)) {
6680
const result = await K8sQueryPatchResource({

0 commit comments

Comments
 (0)