BE: FE: Topics: Allow custom topic params upon creation (#271)

Co-authored-by: Mgrdich <[email protected]> Co-authored-by: VladSenyuta <[email protected]>
kafbat · Apr 26, 2024 · c402f36 · c402f36
1 parent 89d03cc
commit c402f36
Show file tree

Hide file tree

Showing 12 changed files with 536 additions and 147 deletions.
diff --git a/e2e-tests/src/main/java/io/kafbat/ui/screens/schemas/SchemaCreateForm.java b/e2e-tests/src/main/java/io/kafbat/ui/screens/schemas/SchemaCreateForm.java
@@ -3,6 +3,7 @@
 import static com.codeborne.selenide.Selenide.$;
 import static com.codeborne.selenide.Selenide.$$x;
 import static com.codeborne.selenide.Selenide.$x;
+import static com.codeborne.selenide.Selenide.sleep;
 import static org.openqa.selenium.By.id;
 
 import com.codeborne.selenide.Condition;
@@ -96,6 +97,7 @@ public int getVersionsNumberFromList() {
   @Step
   public SchemaCreateForm selectVersionFromDropDown(int versionNumberDd) {
     $x(String.format(ddlElementLocator, versionNumberDd)).shouldBe(Condition.visible).click();
+    sleep(1000);
     return this;
   }
 

diff --git a/e2e-tests/src/main/java/io/kafbat/ui/screens/topics/TopicCreateEditForm.java b/e2e-tests/src/main/java/io/kafbat/ui/screens/topics/TopicCreateEditForm.java
@@ -29,11 +29,11 @@ public class TopicCreateEditForm extends BasePage {
   protected SelenideElement minInSyncReplicasField = $x("//input[@name='minInSyncReplicas']");
   protected SelenideElement cleanUpPolicyDdl = $x("//ul[@id='topicFormCleanupPolicy']");
   protected SelenideElement maxSizeOnDiscDdl = $x("//ul[@id='topicFormRetentionBytes']");
-  protected SelenideElement customParameterDdl = $x("//ul[contains(@name,'customParams')]");
-  protected SelenideElement deleteCustomParameterBtn = $x("//span[contains(@title,'Delete customParam')]");
-  protected SelenideElement addCustomParameterTypeBtn = $x("//button[contains(text(),'Add Custom Parameter')]");
+  protected SelenideElement customParameterDdl = $x("//input[contains(@name, 'customParams')][@role='listitem']");
+  protected SelenideElement deleteCustomParameterBtn = $x("//span[contains(@title, 'Delete customParam')]");
+  protected SelenideElement addCustomParameterTypeBtn = $x("//button[contains(text(), 'Add Custom Parameter')]");
   protected SelenideElement customParameterValueField = $x("//input[@placeholder='Value']");
-  protected SelenideElement validationCustomParameterValueMsg = $x("//p[contains(text(),'Value is required')]");
+  protected SelenideElement validationCustomParameterValueMsg = $x("//p[contains(text(), 'Value is required')]");
   protected String ddlElementLocator = "//li[@value='%s']";
   protected String btnTimeToRetainLocator = "//button[@class][text()='%s']";
   protected String customParamsElmCss = "ul[role=listbox][name^=customParams][name$=name]";

diff --git a/e2e-tests/src/test/java/io/kafbat/ui/smokesuite/brokers/BrokersTest.java b/e2e-tests/src/test/java/io/kafbat/ui/smokesuite/brokers/BrokersTest.java
@@ -160,7 +160,7 @@ public void brokersConfigEditCheck() {
     configItem
         .setValue(String.valueOf(newValue))
         .clickCancelBtn();
-    Assert.assertEquals(getIntegerFromString(configItem.getValue(), false), defaultValue,
+    Assert.assertEquals(getIntegerFromString(configItem.getValue(), true), defaultValue,
         "configItem.getValue()");
     configItem
         .clickEditBtn()
@@ -173,7 +173,7 @@ public void brokersConfigEditCheck() {
     softly.assertFalse(configItem.getSaveBtn().isDisplayed(), "getSaveBtn().isDisplayed()");
     softly.assertFalse(configItem.getCancelBtn().isDisplayed(), "getCancelBtn().isDisplayed()");
     softly.assertTrue(configItem.getEditBtn().isDisplayed(), "getEditBtn().isDisplayed()");
-    softly.assertEquals(getIntegerFromString(configItem.getValue(), false), newValue,
+    softly.assertEquals(getIntegerFromString(configItem.getValue(), true), newValue,
         "configItem.getValue()");
     softly.assertAll();
   }

diff --git a/e2e-tests/src/test/java/io/kafbat/ui/smokesuite/schemas/SchemasTest.java b/e2e-tests/src/test/java/io/kafbat/ui/smokesuite/schemas/SchemasTest.java
@@ -90,7 +90,7 @@ public void compareVersionsCheck() {
         .getVersionsNumberFromList();
     Assert.assertEquals(versionsNumberFromDdl, latestVersion, "Versions number is not matched");
     schemaCreateForm
-        .selectVersionFromDropDown(1);
+        .selectVersionFromDropDown(latestVersion);
     Assert.assertEquals(schemaCreateForm.getMarkedLinesNumber(), 42, "getMarkedLinesNumber()");
   }
 

diff --git a/frontend/src/components/Topics/shared/Form/CustomParams/CustomParamField.tsx b/frontend/src/components/Topics/shared/Form/CustomParams/CustomParamField.tsx
@@ -4,12 +4,12 @@ import { TOPIC_CUSTOM_PARAMS } from 'lib/constants';
 import { FieldArrayWithId, useFormContext, Controller } from 'react-hook-form';
 import { InputLabel } from 'components/common/Input/InputLabel.styled';
 import { FormError } from 'components/common/Input/Input.styled';
-import Select from 'components/common/Select/Select';
 import Input from 'components/common/Input/Input';
 import IconButtonWrapper from 'components/common/Icons/IconButtonWrapper';
 import CloseCircleIcon from 'components/common/Icons/CloseCircleIcon';
 import * as C from 'components/Topics/shared/Form/TopicForm.styled';
 import { ConfigSource } from 'generated-sources';
+import InputWithOptions from 'components/common/InputWithOptions/InputWithOptions';
 import { TopicConfigParams, TopicFormData } from 'lib/interfaces/topic';
 
 import * as S from './CustomParams.styled';
@@ -37,6 +37,7 @@ const CustomParamField: React.FC<Props> = ({
     formState: { errors },
     setValue,
     watch,
+    trigger,
     control,
   } = useFormContext<TopicFormData>();
   const nameValue = watch(`customParams.${index}.name`);
@@ -76,17 +77,18 @@ const CustomParamField: React.FC<Props> = ({
         <InputLabel>Custom Parameter *</InputLabel>
         <Controller
           control={control}
-          rules={{ required: 'Custom Parameter is required.' }}
           name={`customParams.${index}.name`}
-          render={({ field: { value, name, onChange } }) => (
-            <Select
-              name={name}
-              placeholder="Select"
-              disabled={isDisabled}
-              minWidth="270px"
-              onChange={onChange}
+          render={({ field: { name, onChange, value } }) => (
+            <InputWithOptions
               value={value}
               options={options}
+              name={name}
+              onChange={(s) => {
+                onChange(s);
+                trigger('customParams');
+              }}
+              minWidth="270px"
+              placeholder="Select"
             />
           )}
         />
@@ -101,9 +103,6 @@ const CustomParamField: React.FC<Props> = ({
         <InputLabel>Value *</InputLabel>
         <Input
           name={`customParams.${index}.value` as const}
-          hookFormOptions={{
-            required: 'Value is required.',
-          }}
           placeholder="Value"
           defaultValue={field.value}
           autoComplete="off"

diff --git a/frontend/src/components/Topics/shared/Form/CustomParams/CustomParams.tsx b/frontend/src/components/Topics/shared/Form/CustomParams/CustomParams.tsx
@@ -3,6 +3,8 @@ import { useFieldArray, useFormContext, useWatch } from 'react-hook-form';
 import { Button } from 'components/common/Button/Button';
 import { TOPIC_CUSTOM_PARAMS_PREFIX } from 'lib/constants';
 import PlusIcon from 'components/common/Icons/PlusIcon';
+import { ErrorMessage } from '@hookform/error-message';
+import { FormError } from 'components/common/Input/Input.styled';
 import { TopicConfigParams, TopicFormData } from 'lib/interfaces/topic';
 
 import CustomParamField from './CustomParamField';
@@ -18,7 +20,12 @@ const CustomParams: React.FC<CustomParamsProps> = ({
   isSubmitting,
   config,
 }) => {
-  const { control } = useFormContext<TopicFormData>();
+  const {
+    trigger,
+    control,
+    formState: { errors },
+  } = useFormContext<TopicFormData>();
+
   const { fields, append, remove } = useFieldArray({
     control,
     name: TOPIC_CUSTOM_PARAMS_PREFIX,
@@ -36,12 +43,14 @@ const CustomParams: React.FC<CustomParamsProps> = ({
   });
 
   const [existingFields, setExistingFields] = React.useState<string[]>([]);
-
   const removeField = (index: number): void => {
-    setExistingFields(
-      existingFields.filter((field) => field !== controlledFields[index].name)
-    );
-    remove(index);
+    const itemIndex = existingFields.indexOf(controlledFields[index].name);
+    if (itemIndex !== -1) {
+      existingFields.splice(itemIndex, 1);
+      setExistingFields(existingFields);
+      remove(index);
+      trigger('customParams');
+    }
   };
 
   return (
@@ -58,6 +67,9 @@ const CustomParams: React.FC<CustomParamsProps> = ({
           setExistingFields={setExistingFields}
         />
       ))}
+      <FormError>
+        <ErrorMessage errors={errors} name={`customParams` as const} />
+      </FormError>
       <div>
         <Button
           type="button"

diff --git a/frontend/src/components/Topics/shared/Form/CustomParams/__test__/CustomParamField.spec.tsx b/frontend/src/components/Topics/shared/Form/CustomParams/__test__/CustomParamField.spec.tsx
@@ -1,5 +1,5 @@
 import React, { PropsWithChildren } from 'react';
-import { act, screen, within } from '@testing-library/react';
+import { act, screen } from '@testing-library/react';
 import { render } from 'lib/testHelpers';
 import CustomParamsField, {
   Props,
@@ -42,7 +42,14 @@ describe('CustomParamsField', () => {
     setExistingFields.mockClear();
   });
 
-  it('renders the component with its view correctly', () => {
+  const getCustomParamInput = () => screen.getByRole('listitem');
+  const getCustomParamsList = () => screen.getByRole('listbox');
+  const getValueInput = () => screen.getByRole('textbox');
+  const getRemoveButton = () => screen.getByRole('button');
+
+  const topicCustomParam1 = Object.keys(TOPIC_CUSTOM_PARAMS)[0];
+
+  it('renders the component with its view correctly', async () => {
     setupComponent({
       field,
       isDisabled,
@@ -51,9 +58,11 @@ describe('CustomParamsField', () => {
       existingFields,
       setExistingFields,
     });
-    expect(screen.getByRole('listbox')).toBeInTheDocument();
-    expect(screen.getByRole('textbox')).toBeInTheDocument();
-    expect(screen.getByRole('button')).toBeInTheDocument();
+    expect(getCustomParamInput()).toBeInTheDocument();
+    expect(getValueInput()).toBeInTheDocument();
+    expect(getRemoveButton()).toBeInTheDocument();
+    await userEvent.click(getCustomParamInput());
+    expect(getCustomParamsList()).toBeInTheDocument();
   });
 
   describe('core functionality works', () => {
@@ -66,7 +75,7 @@ describe('CustomParamsField', () => {
         existingFields,
         setExistingFields,
       });
-      await userEvent.click(screen.getByRole('button'));
+      await userEvent.click(getRemoveButton());
       expect(remove).toHaveBeenCalledTimes(1);
     });
 
@@ -79,7 +88,7 @@ describe('CustomParamsField', () => {
         existingFields,
         setExistingFields,
       });
-      await userEvent.type(screen.getByRole('button'), SPACE_KEY);
+      await userEvent.type(getRemoveButton(), SPACE_KEY);
       // userEvent.type triggers remove two times as at first it clicks on element and then presses space
       expect(remove).toHaveBeenCalledTimes(2);
     });
@@ -93,12 +102,10 @@ describe('CustomParamsField', () => {
         existingFields,
         setExistingFields,
       });
-      const listbox = screen.getByRole('listbox');
-      await selectOption(listbox, 'compression.type');
-
-      const selectedOption = within(listbox).getAllByRole('option');
-      expect(selectedOption.length).toEqual(1);
-      expect(selectedOption[0]).toHaveTextContent('compression.type');
+      await userEvent.click(getCustomParamInput());
+      await selectOption(getCustomParamsList(), topicCustomParam1);
+      expect(screen.queryByRole('listbox')).not.toBeInTheDocument();
+      expect(getCustomParamInput()).toHaveValue(topicCustomParam1);
     });
 
     it('selecting option updates textbox value', async () => {
@@ -110,11 +117,12 @@ describe('CustomParamsField', () => {
         existingFields,
         setExistingFields,
       });
-      const listbox = screen.getByRole('listbox');
-      await selectOption(listbox, 'compression.type');
+      await userEvent.click(getCustomParamInput());
+      await selectOption(getCustomParamsList(), topicCustomParam1);
 
-      const textbox = screen.getByRole('textbox');
-      expect(textbox).toHaveValue(TOPIC_CUSTOM_PARAMS['compression.type']);
+      expect(getValueInput()).toHaveValue(
+        TOPIC_CUSTOM_PARAMS[topicCustomParam1]
+      );
     });
 
     it('selecting option updates triggers setExistingFields', async () => {
@@ -126,8 +134,8 @@ describe('CustomParamsField', () => {
         existingFields,
         setExistingFields,
       });
-      const listbox = screen.getByRole('listbox');
-      await selectOption(listbox, 'compression.type');
+      await userEvent.click(getCustomParamInput());
+      await selectOption(getCustomParamsList(), topicCustomParam1);
 
       expect(setExistingFields).toHaveBeenCalledTimes(1);
     });