Skip to content

Commit

Permalink
fix(KFLUXUI-198): component relationship remove button
Browse files Browse the repository at this point in the history
  • Loading branch information
testcara committed Dec 3, 2024
1 parent 06ab0bb commit 40abe88
Show file tree
Hide file tree
Showing 9 changed files with 191 additions and 17 deletions.
18 changes: 17 additions & 1 deletion src/components/ComponentRelation/ComponentRelationForm.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as React from 'react';
import { Bullseye, Flex, FlexItem, Grid, GridItem, Radio } from '@patternfly/react-core';
import { Bullseye, Button, Flex, FlexItem, Grid, GridItem, Radio } from '@patternfly/react-core';
import { MinusCircleIcon } from '@patternfly/react-icons/dist/esm/icons/minus-circle-icon';
import { useField } from 'formik';
import { HelpTooltipIcon } from '../../shared';
import {
Expand All @@ -12,12 +13,17 @@ type ComponentRelationProps = {
componentNames: string[];
groupedComponents: { [application: string]: string[] };
index?: number;
removeProps: {
disableRemove: boolean;
onRemove: () => void;
};
};

export const ComponentRelation: React.FC<ComponentRelationProps> = ({
index,
componentNames,
groupedComponents,
removeProps: { disableRemove, onRemove },
}) => {
const sourceName = `relations.${index.toString()}.source`;
const nudgeName = `relations.${index.toString()}.nudgeType`;
Expand Down Expand Up @@ -87,6 +93,16 @@ export const ComponentRelation: React.FC<ComponentRelationProps> = ({
groupedComponents={groupedComponents}
/>
</GridItem>
<GridItem span={1}>
<Button
id={`remove-relation-${index}`}
variant="plain"
onClick={onRemove}
isDisabled={disableRemove}
>
<MinusCircleIcon />
</Button>
</GridItem>
</Grid>
);
};
20 changes: 11 additions & 9 deletions src/components/ComponentRelation/ComponentRelationModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,14 @@ export const ComponentRelationModal: React.FC<ComponentRelationModalProps> = ({
setShowCancelModal(true);
}, [application, track, workspace]);

const initialValues: ComponentRelationFormikValue = {
relations:
loaded && !error && nudgeData.length > 0
? nudgeData
: [{ source: '', nudgeType: ComponentRelationNudgeType.NUDGES, target: [] }],
};
const initialValues: ComponentRelationFormikValue = React.useMemo(() => {
return {
relations:
loaded && !error && nudgeData.length > 0
? nudgeData
: [{ source: '', nudgeType: ComponentRelationNudgeType.NUDGES, target: [] }],
};
}, [error, loaded, nudgeData]);

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

helpers.setSubmitting(false);
helpers.setStatus({ submitError: e?.message });
}
return updateNudgeDependencies(values.relations, namespace)
return updateNudgeDependencies(values.relations, initialValues.relations, namespace)

Check warning on line 91 in src/components/ComponentRelation/ComponentRelationModal.tsx

View check run for this annotation

Codecov / codecov/patch

src/components/ComponentRelation/ComponentRelationModal.tsx#L91

Added line #L91 was not covered by tests
.then((compResults: ComponentKind[]) => {
compResults.forEach((c) => {
track('Component relationship updated', {
Expand All @@ -105,7 +107,7 @@ export const ComponentRelationModal: React.FC<ComponentRelationModalProps> = ({
helpers.setStatus({ submitError: e?.message });
});
},
[application, namespace, onSaveRelationships, track, workspace],
[application, namespace, onSaveRelationships, track, workspace, initialValues],
);

if (showSubmissionModal && !showCancelModal) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { configure, screen } from '@testing-library/react';
import { act, configure, fireEvent, screen } from '@testing-library/react';
import { formikRenderer } from '../../../utils/test-utils';
import { ComponentRelation } from '../ComponentRelationForm';
import { ComponentRelationNudgeType } from '../type';
Expand All @@ -12,6 +12,10 @@ describe('ComponentRelationForm', () => {
index={0}
componentNames={['asdf', 'asd']}
groupedComponents={{ app: ['asdf', 'asd'] }}
removeProps={{
disableRemove: true,
onRemove: jest.fn(),
}}
/>,
{
relations: [
Expand All @@ -22,5 +26,11 @@ describe('ComponentRelationForm', () => {
expect(screen.getAllByTestId('toggle-component-menu')).toHaveLength(2);
expect(screen.getAllByTestId('nudges-0')).toHaveLength(1);
expect(screen.getAllByTestId('nudged-by-0')).toHaveLength(1);
// mouseOver help icon
const nodgeSvg = screen.getAllByRole('img', { hidden: true })[1];
act(() => {
fireEvent.mouseEnter(nodgeSvg);
});
expect(nodgeSvg.getAttributeNames().includes('aria-describedby'));
});
});
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { configure, fireEvent, render, screen, waitFor } from '@testing-library/react';
import { componentCRMocks } from '../../Components/__data__/mock-data';
import { ComponentRelationModal } from '../ComponentRelationModal';
import { ComponentRelationNudgeType, ComponentRelationValue } from '../type';
import { useNudgeData } from '../useNudgeData';
import { updateNudgeDependencies } from '../utils';

configure({ testIdAttribute: 'id' });

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

jest.mock('../utils', () => ({
...jest.requireActual('../utils'),
updateNudgeDependencies: jest.fn(() => Promise.resolve([])),
updateNudgeDependencies: jest.fn(),
}));

jest.mock('../useNudgeData', () => ({
useNudgeData: jest.fn(),
}));

class MockResizeObserver {
Expand All @@ -37,9 +44,29 @@ class MockResizeObserver {
}
}

const mockComponentRelations: ComponentRelationValue[] = [
{
source: 'a',
nudgeType: ComponentRelationNudgeType.NUDGES,
target: ['b'],
},
{
source: 'c',
nudgeType: ComponentRelationNudgeType.NUDGES,
target: ['d'],
},
];

window.ResizeObserver = MockResizeObserver;
const useNudgeDataMock = useNudgeData as jest.Mock;
const updateNudgeDependenciesMock = updateNudgeDependencies as jest.Mock;

describe('ComponentRelationModal', () => {
beforeEach(() => {
useNudgeDataMock.mockReturnValue([[], true, null]);
updateNudgeDependenciesMock.mockResolvedValue(componentCRMocks);
});

it('should render modal', () => {
render(<ComponentRelationModal modalProps={{ isOpen: true }} application="apps" />);
screen.getByText('Component relationships');
Expand All @@ -50,6 +77,15 @@ describe('ComponentRelationModal', () => {
expect(screen.getAllByTestId('toggle-component-menu')).toHaveLength(2);
});

it('should remove a relation', () => {
render(<ComponentRelationModal modalProps={{ isOpen: true }} application="apps" />);
expect(screen.queryAllByTestId(/remove-relation-\d+/)).toHaveLength(1);
fireEvent.click(screen.getByText(`Add another component relationship`));
expect(screen.getAllByTestId(/remove-relation-\d+/)).toHaveLength(2);
fireEvent.click(screen.getByTestId('remove-relation-0'));
expect(screen.queryAllByTestId(/remove-relation-\d+/)).toHaveLength(1);
});

it('should show cancelation modal when clicked on cancel', () => {
let isOpen = true;
const onClose = () => {
Expand Down Expand Up @@ -79,17 +115,47 @@ describe('ComponentRelationModal', () => {
});

it('should show confirmation modal on relationship save', async () => {
useNudgeDataMock.mockReturnValue([mockComponentRelations, true, null]);
updateNudgeDependenciesMock.mockResolvedValue(componentCRMocks);
let isOpen = true;
const onClose = () => {
isOpen = false;
};
render(<ComponentRelationModal modalProps={{ isOpen, onClose }} application="apps" />);
const { rerender } = render(
<ComponentRelationModal modalProps={{ isOpen, onClose }} application="apps" />,
);
expect(screen.queryByText('Component relationships')).toBeInTheDocument();
fireEvent.click(screen.getByTestId('nudged-by-0'));
const saveButton = screen.getByText('Save relationships');
expect(saveButton.getAttribute('class')).not.toContain('pf-m-disabled');
fireEvent.click(saveButton);
expect(saveButton.getAttribute('class')).toContain('pf-m-in-progress');
await waitFor(() => expect(saveButton.getAttribute('class')).not.toContain('pf-m-in-progress'));
await waitFor(() => {
expect(saveButton.getAttribute('class')).toContain('pf-m-in-progress');
});

rerender(<ComponentRelationModal modalProps={{ isOpen, onClose }} application="apps" />);
await waitFor(() => {
expect(screen.queryByText('Relationships updated!')).toBeInTheDocument();
});

const doneButton = screen.getByText('Done');
fireEvent.click(doneButton);
});

it('should display an error on failure', async () => {
useNudgeDataMock.mockReturnValue([mockComponentRelations, true, null]);
updateNudgeDependenciesMock.mockRejectedValue(new Error('error'));
let isOpen = true;
const onClose = () => {
isOpen = false;
};
render(<ComponentRelationModal modalProps={{ isOpen, onClose }} application="apps" />);
expect(screen.queryByText('Component relationships')).toBeInTheDocument();
fireEvent.click(screen.getByTestId('nudged-by-0'));
const saveButton = screen.getByText('Save relationships');
expect(saveButton.getAttribute('class')).not.toContain('pf-m-disabled');
fireEvent.click(saveButton);
await waitFor(() => expect(screen.queryByText('Danger alert:')).toBeInTheDocument());
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,24 @@ describe('MultiSelectComponentDropdown', () => {
expect(button.querySelector('.pf-m-read').innerHTML).toEqual('2');
});

it('should select/unselect all item from menu', () => {
formikRenderer(
<MultiSelectComponentsDropdown groupedComponents={{ c: ['a', 'b'] }} name="multiSelect" />,
{ multiSelect: '' },
);
screen.getByText('Choose components to nudge');
const button = screen.getByTestId('toggle-component-menu');
fireEvent.click(button);
expect(button.querySelector('.pf-m-read')).not.toBeInTheDocument();
const menu = screen.getAllByRole('menuitem');
const selectAllButton = menu[0].querySelector('input');
fireEvent.click(selectAllButton);
expect(button.querySelector('.pf-m-read')).toBeInTheDocument();
expect(button.querySelector('.pf-m-read').innerHTML).toEqual('2');
fireEvent.click(selectAllButton);
expect(button.querySelector('.pf-m-read')).not.toBeInTheDocument();
});

it('should not select disabled menu item', () => {
formikRenderer(
<MultiSelectComponentsDropdown
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,13 @@ describe('useComponentRelationAction', () => {
`You don't have access to define component relationships`,
);
});

it('should disable action when there is one component in the app', () => {
jest.clearAllMocks();
// mock one component for the application
mockUseComponents.mockReturnValue([[{}], true, undefined]);
mockUseAccessReviewModel.mockReturnValue([true]);
const { result } = renderHook(() => useComponentRelationAction('application'));
expect(result.current().isDisabled).toEqual(true);
});
});
37 changes: 36 additions & 1 deletion src/components/ComponentRelation/__tests__/utils.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,40 @@
import { ComponentRelationNudgeType } from '../type';
import { componentRelationValidationSchema, transformNudgeData } from '../utils';
import {
componentRelationValidationSchema,
computeNudgeDataChanges,
transformNudgeData,
} from '../utils';

describe('computeNudgeDataChanges', () => {
it('should compute data changes', () => {
expect(
computeNudgeDataChanges(
[{ source: 'a', nudgeType: ComponentRelationNudgeType.NUDGES, target: ['b', 'c'] }],
[],
),
).toEqual([{ source: 'a', nudgeType: ComponentRelationNudgeType.NUDGES, target: [] }]);
expect(
computeNudgeDataChanges(
[],
[{ source: 'a', nudgeType: ComponentRelationNudgeType.NUDGES, target: ['b', 'c'] }],
),
).toEqual([{ source: 'a', nudgeType: ComponentRelationNudgeType.NUDGES, target: ['b', 'c'] }]);
expect(
computeNudgeDataChanges(
[
{ source: 'a', nudgeType: ComponentRelationNudgeType.NUDGES, target: ['b'] },
{ source: 'b', nudgeType: ComponentRelationNudgeType.NUDGES, target: ['c'] },
{ source: 'c', nudgeType: ComponentRelationNudgeType.NUDGES, target: ['d'] },
],
[{ source: 'b', nudgeType: ComponentRelationNudgeType.NUDGES, target: ['c'] }],
),
).toEqual([
{ source: 'b', nudgeType: ComponentRelationNudgeType.NUDGES, target: ['c'] },
{ source: 'a', nudgeType: ComponentRelationNudgeType.NUDGES, target: [] },
{ source: 'c', nudgeType: ComponentRelationNudgeType.NUDGES, target: [] },
]);
});
});

describe('transformNudgeData', () => {
it('should transform data', () => {
Expand Down
4 changes: 4 additions & 0 deletions src/components/ComponentRelation/cr-modals.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ export const DefineComponentRelationModal: React.FC<DefineComponentRelationModal
componentNames={componentNames}
groupedComponents={groupedComponents}
index={index}
removeProps={{
disableRemove: values.relations.length <= 1,
onRemove: () => arrayHelpers.remove(index),
}}
/>
{index !== values.relations.length - 1 ? <Divider /> : null}
</>
Expand Down
18 changes: 16 additions & 2 deletions src/components/ComponentRelation/utils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { union } from 'lodash-es';
import { differenceBy, union } from 'lodash-es';
import * as yup from 'yup';
import { K8sQueryPatchResource } from '../../k8s';
import { ComponentModel } from '../../models';
Expand Down Expand Up @@ -55,12 +55,26 @@ export const transformNudgeData = (data: ComponentRelationValue[]): { [key: stri
}, {});
};

export const computeNudgeDataChanges = (
initialValues: ComponentRelationValue[],

Check warning on line 59 in src/components/ComponentRelation/utils.ts

View check run for this annotation

Codecov / codecov/patch

src/components/ComponentRelation/utils.ts#L59

Added line #L59 was not covered by tests
values: ComponentRelationValue[],
): ComponentRelationValue[] => {
// Find the missing relation and set the target to [] to remove it
const removedSources = differenceBy(initialValues, values, 'source').map((val) => ({
...val,
target: [],

Check warning on line 65 in src/components/ComponentRelation/utils.ts

View check run for this annotation

Codecov / codecov/patch

src/components/ComponentRelation/utils.ts#L63-L65

Added lines #L63 - L65 were not covered by tests
}));
return [...values, ...removedSources];
};

Check warning on line 69 in src/components/ComponentRelation/utils.ts

View check run for this annotation

Codecov / codecov/patch

src/components/ComponentRelation/utils.ts#L69

Added line #L69 was not covered by tests
export const updateNudgeDependencies = async (
values: ComponentRelationValue[],
initialValues: ComponentRelationValue[],
namespace: string,
dryRun?: boolean,
) => {
const transformedData = transformNudgeData(values);
const valueChanges = computeNudgeDataChanges(initialValues, values);
const transformedData = transformNudgeData(valueChanges);
const data = [];
for (const [componentName, nudgeData] of Object.entries(transformedData)) {
const result = await K8sQueryPatchResource({
Expand Down

0 comments on commit 40abe88

Please sign in to comment.