Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fixes #1213] Topic Creation: Inconsistency in Deleted parameters #1395

Merged
merged 9 commits into from
Jan 19, 2022
2 changes: 1 addition & 1 deletion kafka-ui-react-app/src/components/Topics/New/New.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ const New: React.FC = () => {

history.push(clusterTopicPath(clusterName, data.name));
} catch (error) {
const response = await getResponse(error);
const response = await getResponse(error as Response);
const alert: FailurePayload = {
subject: ['schema', data.name].join('-'),
title: `Schema ${data.name}`,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import React from 'react';
import React, { useRef } from 'react';
import { ErrorMessage } from '@hookform/error-message';
import { TOPIC_CUSTOM_PARAMS } from 'lib/constants';
import { FieldArrayWithId, useFormContext } from 'react-hook-form';
import { remove as _remove } from 'lodash';
import { TopicFormData } from 'redux/interfaces';
import { InputLabel } from 'components/common/Input/InputLabel.styled';
import { FormError } from 'components/common/Input/Input.styled';
Expand All @@ -14,7 +13,7 @@ import * as C from 'components/Topics/shared/Form/TopicForm.styled';

import * as S from './CustomParams.styled';

interface Props {
export interface Props {
isDisabled: boolean;
index: number;
existingFields: string[];
Expand All @@ -37,19 +36,17 @@ const CustomParamField: React.FC<Props> = ({
watch,
} = useFormContext<TopicFormData>();
const nameValue = watch(`customParams.${index}.name`);
let prevName = '';
const prevName = useRef(nameValue);

React.useEffect(() => {
prevName = nameValue;
}, []);

React.useEffect(() => {
if (nameValue !== prevName) {
if (nameValue !== prevName.current) {
let newExistingFields = [...existingFields];
if (prevName) {
newExistingFields = _remove(newExistingFields, (el) => el === prevName);
if (prevName.current) {
newExistingFields = newExistingFields.filter(
(name) => name !== prevName.current
);
}
prevName = nameValue;
prevName.current = nameValue;
newExistingFields.push(nameValue);
setExistingFields(newExistingFields);
setValue(`customParams.${index}.value`, TOPIC_CUSTOM_PARAMS[nameValue]);
Expand Down Expand Up @@ -83,7 +80,10 @@ const CustomParamField: React.FC<Props> = ({
))}
</Select>
<FormError>
<ErrorMessage errors={errors} name={`customParams.${index}.name`} />
<ErrorMessage
errors={errors}
name={`customParams.${index}.name` as const}
/>
</FormError>
</div>
</>
Expand All @@ -101,13 +101,22 @@ const CustomParamField: React.FC<Props> = ({
disabled={isDisabled}
/>
<FormError>
<ErrorMessage errors={errors} name={`customParams.${index}.value`} />
<ErrorMessage
errors={errors}
name={`customParams.${index}.value` as const}
/>
</FormError>
</div>

<S.DeleteButtonWrapper>
<IconButtonWrapper onClick={() => remove(index)} aria-hidden>
<CloseIcon />
<IconButtonWrapper
onClick={() => remove(index)}
onKeyDown={(e: React.KeyboardEvent) =>
e.code === 'Space' && remove(index)
}
title={`Delete customParam field ${index}`}
>
<CloseIcon aria-hidden />
</IconButtonWrapper>
</S.DeleteButtonWrapper>
</C.Column>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,10 @@ export const ParamsWrapper = styled.div`
`;

export const DeleteButtonWrapper = styled.div`
height: 32px;
min-height: 32px;
display: flex;
flex-direction: column;
justify-content: center;
align-items: center;
align-self: flex-end;
flex-grow: 0.25 !important;
justify-self: flex-start;
margin-top: 32px;
`;
Original file line number Diff line number Diff line change
@@ -1,35 +1,48 @@
import React from 'react';
import { TopicConfigByName, TopicFormData } from 'redux/interfaces';
import { useFieldArray, useFormContext } from 'react-hook-form';
import { useFieldArray, useFormContext, useWatch } from 'react-hook-form';
import { Button } from 'components/common/Button/Button';

import CustomParamField from './CustomParamField';
import * as S from './CustomParams.styled';

export const INDEX_PREFIX = 'customParams';

interface Props {
export interface CustomParamsProps {
isSubmitting: boolean;
config?: TopicConfigByName;
}

const CustomParams: React.FC<Props> = ({ isSubmitting }) => {
const CustomParams: React.FC<CustomParamsProps> = ({ isSubmitting }) => {
const { control } = useFormContext<TopicFormData>();
const { fields, append, remove } = useFieldArray({
control,
name: INDEX_PREFIX,
});
const watchFieldArray = useWatch({
control,
name: INDEX_PREFIX,
defaultValue: fields,
});
const controlledFields = fields.map((field, index) => {
return {
...field,
...watchFieldArray[index],
};
});

const [existingFields, setExistingFields] = React.useState<string[]>([]);

const removeField = (index: number): void => {
setExistingFields(
existingFields.filter((field) => field === fields[index].name)
existingFields.filter((field) => field !== controlledFields[index].name)
);
remove(index);
};

return (
<S.ParamsWrapper>
{fields.map((field, idx) => (
{controlledFields.map((field, idx) => (
<CustomParamField
key={field.id}
field={field}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
import React from 'react';
import { screen, within } from '@testing-library/react';
import { render } from 'lib/testHelpers';
import CustomParamsField, {
Props,
} from 'components/Topics/shared/Form/CustomParams/CustomParamField';
import { FormProvider, useForm } from 'react-hook-form';
import userEvent from '@testing-library/user-event';
import { TOPIC_CUSTOM_PARAMS } from 'lib/constants';

const isDisabled = false;
const index = 0;
const existingFields: string[] = [];
const field = { name: 'name', value: 'value', id: 'id' };
const remove = jest.fn();
const setExistingFields = jest.fn();

const SPACE_KEY = ' ';

describe('CustomParamsField', () => {
const setupComponent = (props: Props) => {
const Wrapper: React.FC = ({ children }) => {
const methods = useForm();
return <FormProvider {...methods}>{children}</FormProvider>;
};

return render(
<Wrapper>
<CustomParamsField {...props} />
</Wrapper>
);
};

it('renders with props', () => {
setupComponent({
field,
isDisabled,
index,
remove,
existingFields,
setExistingFields,
});
expect(screen.getByRole('listbox')).toBeInTheDocument();
expect(screen.getByRole('textbox')).toBeInTheDocument();
expect(screen.getByRole('button')).toBeInTheDocument();
});

describe('core functionality works', () => {
it('click on button triggers remove', () => {
setupComponent({
field,
isDisabled,
index,
remove,
existingFields,
setExistingFields,
});
userEvent.click(screen.getByRole('button'));
expect(remove.mock.calls.length).toBe(1);
});

it('pressing space on button triggers remove', () => {
setupComponent({
field,
isDisabled,
index,
remove,
existingFields,
setExistingFields,
});
userEvent.type(screen.getByRole('button'), SPACE_KEY);
// userEvent.type triggers remove two times as at first it clicks on element and then presses space
expect(remove.mock.calls.length).toBe(2);
});

it('can select option', () => {
setupComponent({
field,
isDisabled,
index,
remove,
existingFields,
setExistingFields,
});
const listbox = screen.getByRole('listbox');
userEvent.selectOptions(listbox, ['compression.type']);

const option = within(listbox).getByRole('option', { selected: true });
expect(option).toHaveValue('compression.type');
});

it('selecting option updates textbox value', () => {
setupComponent({
field,
isDisabled,
index,
remove,
existingFields,
setExistingFields,
});
const listbox = screen.getByRole('listbox');
userEvent.selectOptions(listbox, ['compression.type']);

const textbox = screen.getByRole('textbox');
expect(textbox).toHaveValue(TOPIC_CUSTOM_PARAMS['compression.type']);
});

it('selecting option updates triggers setExistingFields', () => {
setupComponent({
field,
isDisabled,
index,
remove,
existingFields,
setExistingFields,
});
const listbox = screen.getByRole('listbox');
userEvent.selectOptions(listbox, ['compression.type']);

expect(setExistingFields.mock.calls.length).toBe(1);
});
});
});
Loading