From 85f095657cbb93f61e4f4745ae9dcce0aa0847b4 Mon Sep 17 00:00:00 2001
From: Arsen Simonyan <103444767+simonyandev@users.noreply.github.com>
Date: Fri, 22 Apr 2022 11:39:07 +0400
Subject: [PATCH] Fix topic custom parameter duplication overriding (#1859)

* fix topic custom parameter multiple items of the same field bug

* move condition into the updateSelectedOption function
---
 .../Form/CustomParams/CustomParamField.tsx    | 16 +++--
 .../components/common/Select/Select.styled.ts |  5 +-
 .../src/components/common/Select/Select.tsx   | 13 ++--
 .../common/Select/__tests__/Select.spec.tsx   | 65 +++++++++++++------
 4 files changed, 66 insertions(+), 33 deletions(-)

diff --git a/kafka-ui-react-app/src/components/Topics/shared/Form/CustomParams/CustomParamField.tsx b/kafka-ui-react-app/src/components/Topics/shared/Form/CustomParams/CustomParamField.tsx
index 5984247a75..d56bd50d54 100644
--- a/kafka-ui-react-app/src/components/Topics/shared/Form/CustomParams/CustomParamField.tsx
+++ b/kafka-ui-react-app/src/components/Topics/shared/Form/CustomParams/CustomParamField.tsx
@@ -39,6 +39,14 @@ const CustomParamField: React.FC<Props> = ({
   const nameValue = watch(`customParams.${index}.name`);
   const prevName = useRef(nameValue);
 
+  const options = Object.keys(TOPIC_CUSTOM_PARAMS)
+    .sort()
+    .map((option) => ({
+      value: option,
+      label: option,
+      disabled: existingFields.includes(option),
+    }));
+
   React.useEffect(() => {
     if (nameValue !== prevName.current) {
       let newExistingFields = [...existingFields];
@@ -72,13 +80,7 @@ const CustomParamField: React.FC<Props> = ({
               minWidth="270px"
               onChange={onChange}
               value={value}
-              options={Object.keys(TOPIC_CUSTOM_PARAMS)
-                .sort()
-                .map((opt) => ({
-                  value: opt,
-                  label: opt,
-                  disabled: existingFields.includes(opt),
-                }))}
+              options={options}
             />
           )}
         />
diff --git a/kafka-ui-react-app/src/components/common/Select/Select.styled.ts b/kafka-ui-react-app/src/components/common/Select/Select.styled.ts
index 936a4786e3..cf70423295 100644
--- a/kafka-ui-react-app/src/components/common/Select/Select.styled.ts
+++ b/kafka-ui-react-app/src/components/common/Select/Select.styled.ts
@@ -94,9 +94,12 @@ export const Option = styled.li<OptionProps>`
   transition: all 0.2s ease-in-out;
   cursor: ${({ disabled }) => (disabled ? 'not-allowed' : 'pointer')};
   gap: 5px;
+  color: ${(props) =>
+    props.theme.select.color[props.disabled ? 'disabled' : 'normal']};
 
   &:hover {
-    background-color: ${(props) => props.theme.select.backgroundColor.hover};
+    background-color: ${(props) =>
+      props.theme.select.backgroundColor[props.disabled ? 'normal' : 'hover']};
   }
 
   &:active {
diff --git a/kafka-ui-react-app/src/components/common/Select/Select.tsx b/kafka-ui-react-app/src/components/common/Select/Select.tsx
index ae9824e291..d9c56ebdab 100644
--- a/kafka-ui-react-app/src/components/common/Select/Select.tsx
+++ b/kafka-ui-react-app/src/components/common/Select/Select.tsx
@@ -48,12 +48,17 @@ const Select: React.FC<SelectProps> = ({
   useClickOutside(selectContainerRef, clickOutsideHandler);
 
   const updateSelectedOption = (option: SelectOption) => {
-    if (disabled) return;
+    if (!option.disabled) {
+      setSelectedOption(option.value);
 
-    setSelectedOption(option.value);
-    if (onChange) onChange(option.value);
-    setShowOptions(false);
+      if (onChange) {
+        onChange(option.value);
+      }
+
+      setShowOptions(false);
+    }
   };
+
   React.useEffect(() => {
     setSelectedOption(value);
   }, [isLive, value]);
diff --git a/kafka-ui-react-app/src/components/common/Select/__tests__/Select.spec.tsx b/kafka-ui-react-app/src/components/common/Select/__tests__/Select.spec.tsx
index 5ace7a2846..dd433b991c 100644
--- a/kafka-ui-react-app/src/components/common/Select/__tests__/Select.spec.tsx
+++ b/kafka-ui-react-app/src/components/common/Select/__tests__/Select.spec.tsx
@@ -1,4 +1,7 @@
-import Select, { SelectProps } from 'components/common/Select/Select';
+import Select, {
+  SelectOption,
+  SelectProps,
+} from 'components/common/Select/Select';
 import React from 'react';
 import { render } from 'lib/testHelpers';
 import { screen } from '@testing-library/react';
@@ -10,34 +13,54 @@ jest.mock('react-hook-form', () => ({
   }),
 }));
 
-const options = [
-  { label: 'test-label1', value: 'test-value1' },
-  { label: 'test-label2', value: 'test-value2' },
+const options: Array<SelectOption> = [
+  { label: 'test-label1', value: 'test-value1', disabled: false },
+  { label: 'test-label2', value: 'test-value2', disabled: true },
 ];
 
 const renderComponent = (props?: Partial<SelectProps>) =>
   render(<Select name="test" {...props} />);
 
 describe('Custom Select', () => {
-  it('renders component', () => {
-    renderComponent();
-    expect(screen.getByRole('listbox')).toBeInTheDocument();
-  });
-  it('show select options when select is being clicked', () => {
-    renderComponent({
-      options,
+  describe('when isLive is not specified', () => {
+    beforeEach(() => {
+      renderComponent({
+        options,
+      });
     });
-    expect(screen.getByRole('option')).toBeInTheDocument();
-    userEvent.click(screen.getByRole('listbox'));
-    expect(screen.getAllByRole('option')).toHaveLength(3);
-  });
-  it('checking select option change', () => {
-    renderComponent({
-      options,
+
+    it('renders component', () => {
+      expect(screen.getByRole('listbox')).toBeInTheDocument();
+    });
+
+    it('show select options when select is being clicked', () => {
+      expect(screen.getByRole('option')).toBeInTheDocument();
+      userEvent.click(screen.getByRole('listbox'));
+      expect(screen.getAllByRole('option')).toHaveLength(3);
+    });
+
+    it('checking select option change', () => {
+      const listbox = screen.getByRole('listbox');
+      const optionLabel = 'test-label1';
+
+      userEvent.click(listbox);
+      userEvent.selectOptions(listbox, [optionLabel]);
+
+      expect(screen.getByRole('option')).toHaveTextContent(optionLabel);
+    });
+
+    it('trying to select disabled option does not trigger change', () => {
+      const listbox = screen.getByRole('listbox');
+      const normalOptionLabel = 'test-label1';
+      const disabledOptionLabel = 'test-label2';
+
+      userEvent.click(listbox);
+      userEvent.selectOptions(listbox, [normalOptionLabel]);
+      userEvent.click(listbox);
+      userEvent.selectOptions(listbox, [disabledOptionLabel]);
+
+      expect(screen.getByRole('option')).toHaveTextContent(normalOptionLabel);
     });
-    userEvent.click(screen.getByRole('listbox'));
-    userEvent.selectOptions(screen.getByRole('listbox'), ['test-label1']);
-    expect(screen.getByRole('option')).toHaveTextContent('test-label1');
   });
 
   describe('when non-live', () => {