UI support for MSK (#2828)

* UI support for MSK

* fixed test changes

* fixed pull request comments

* Use the opposite operator (!==) instead.

* fixed update topic

* Minor code modifications in the Edit Submission logic

* minor typescript code change, in the TopicFormData

* minor CustomParamField short circuit code efficiency improveral

* Topic Form Edit , sanitization of Default non-dirty fields, while sending the dynamic ones

* fix checkTopicCreatePossibility()

* fix checkTopicCreatePossibility()

Co-authored-by: Mgrdich <[email protected]>
Co-authored-by: Mgrdich <[email protected]>
Co-authored-by: VladSenyuta <[email protected]>

Author: 3 people

kafka-ui-e2e-checks/src/main/java/com/provectus/kafka/ui/models/Topic.java

@@ -9,8 +9,8 @@
 @Data
 @Accessors(chain = true)
 public class Topic {
-    private String name, timeToRetainData, maxMessageBytes, messageKey, messageContent,
-            partitions, customParameterValue;
+    private String name, timeToRetainData, maxMessageBytes, messageKey, messageContent, customParameterValue;
+    private int numberOfPartitions;
     private CustomParameterType customParameterType;
     private CleanupPolicyValue cleanupPolicyValue;
     private MaxSizeOnDisk maxSizeOnDisk;

kafka-ui-e2e-checks/src/main/java/com/provectus/kafka/ui/pages/topic/TopicCreateEditForm.java

@@ -48,7 +48,10 @@ public boolean isDeleteCustomParameterButtonEnabled() {
 
   @Step
   public TopicCreateEditForm setTopicName(String topicName) {
-    nameField.setValue(topicName);
+    nameField.shouldBe(Condition.enabled).clear();
+    if (topicName != null) {
+      nameField.sendKeys(topicName);
+    }
     return this;
   }
 
@@ -108,8 +111,9 @@ public TopicCreateEditForm setMaxMessageBytes(String bytes) {
   }
 
   @Step
-  public TopicCreateEditForm setPartitions(String partitions) {
-    partitionsField.setValue(partitions);
+  public TopicCreateEditForm setNumberOfPartitions(int partitions) {
+    partitionsField.shouldBe(Condition.enabled).clear();
+    partitionsField.sendKeys(String.valueOf(partitions));
     return this;
   }
 

kafka-ui-e2e-checks/src/main/java/com/provectus/kafka/ui/pages/topic/TopicDetails.java

@@ -85,8 +85,8 @@ public String getCleanUpPolicy() {
   }
 
   @Step
-  public String getPartitions() {
-    return partitionsField.getText();
+  public int getPartitions() {
+    return Integer.parseInt(partitionsField.getText().trim());
   }
 
   @Step

kafka-ui-e2e-checks/src/test/java/com/provectus/kafka/ui/suite/topics/TopicsTests.java

@@ -8,6 +8,7 @@
 import static com.provectus.kafka.ui.settings.Source.CLUSTER_NAME;
 import static com.provectus.kafka.ui.utilities.FileUtils.fileToString;
 import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic;
+import static org.apache.commons.lang3.RandomUtils.nextInt;
 import static org.assertj.core.api.Assertions.assertThat;
 
 import com.codeborne.selenide.Condition;
@@ -40,7 +41,7 @@ public class TopicsTests extends BaseTest {
   private static final String SUITE_TITLE = "Topics";
   private static final Topic TOPIC_TO_CREATE = new Topic()
       .setName("new-topic-" + randomAlphabetic(5))
-      .setPartitions("1")
+      .setNumberOfPartitions(1)
       .setCustomParameterType(COMPRESSION_TYPE)
       .setCustomParameterValue("producer")
       .setCleanupPolicyValue(DELETE);
@@ -77,7 +78,7 @@ public void createTopic() {
     topicCreateEditForm
         .waitUntilScreenReady()
         .setTopicName(TOPIC_TO_CREATE.getName())
-        .setPartitions(TOPIC_TO_CREATE.getPartitions())
+        .setNumberOfPartitions(TOPIC_TO_CREATE.getNumberOfPartitions())
         .selectCleanupPolicy(TOPIC_TO_CREATE.getCleanupPolicyValue())
         .clickCreateTopicBtn();
     topicDetails
@@ -92,7 +93,8 @@ public void createTopic() {
         .isTrue();
     softly.assertThat(topicDetails.getCleanUpPolicy()).as("getCleanUpPolicy()")
         .isEqualTo(TOPIC_TO_CREATE.getCleanupPolicyValue().toString());
-    softly.assertThat(topicDetails.getPartitions()).as("getPartitions()").isEqualTo(TOPIC_TO_CREATE.getPartitions());
+    softly.assertThat(topicDetails.getPartitions()).as("getPartitions()")
+        .isEqualTo(TOPIC_TO_CREATE.getNumberOfPartitions());
     softly.assertAll();
     naviSideBar
         .openSideMenu(TOPICS);
@@ -237,11 +239,17 @@ void checkTopicCreatePossibility() {
         .waitUntilScreenReady()
         .clickAddTopicBtn();
     topicCreateEditForm
-        .waitUntilScreenReady()
-        .setTopicName("");
+        .waitUntilScreenReady();
+    assertThat(topicCreateEditForm.isCreateTopicButtonEnabled()).as("isCreateTopicButtonEnabled()").isFalse();
+    topicCreateEditForm
+        .setTopicName("testName");
+    assertThat(topicCreateEditForm.isCreateTopicButtonEnabled()).as("isCreateTopicButtonEnabled()").isFalse();
+    topicCreateEditForm
+        .setTopicName(null)
+        .setNumberOfPartitions(nextInt(1, 10));
     assertThat(topicCreateEditForm.isCreateTopicButtonEnabled()).as("isCreateTopicButtonEnabled()").isFalse();
     topicCreateEditForm
-        .setTopicName("testTopic1");
+        .setTopicName("testName");
     assertThat(topicCreateEditForm.isCreateTopicButtonEnabled()).as("isCreateTopicButtonEnabled()").isTrue();
   }
 

kafka-ui-react-app/src/components/Topics/New/__test__/New.spec.tsx

@@ -1,7 +1,7 @@
 import React from 'react';
 import New from 'components/Topics/New/New';
 import { Route, Routes } from 'react-router-dom';
-import { act, screen, waitFor } from '@testing-library/react';
+import { act, screen } from '@testing-library/react';
 import {
   clusterTopicCopyPath,
   clusterTopicNewPath,
@@ -13,18 +13,17 @@ import { useCreateTopic } from 'lib/hooks/api/topics';
 
 const clusterName = 'local';
 const topicName = 'test-topic';
+const minValue = '1';
 
 const mockNavigate = jest.fn();
 jest.mock('react-router-dom', () => ({
   ...jest.requireActual('react-router-dom'),
   useNavigate: () => mockNavigate,
 }));
-
 jest.mock('lib/hooks/api/topics', () => ({
   useCreateTopic: jest.fn(),
 }));
-
-const renderComponent = (path: string) =>
+const renderComponent = (path: string) => {
   render(
     <Routes>
       <Route path={clusterTopicNewPath()} element={<New />} />
@@ -33,9 +32,9 @@ const renderComponent = (path: string) =>
     </Routes>,
     { initialEntries: [path] }
   );
+};
 
 const createTopicMock = jest.fn();
-
 describe('New', () => {
   beforeEach(() => {
     (useCreateTopic as jest.Mock).mockImplementation(() => ({
@@ -45,7 +44,6 @@ describe('New', () => {
   afterEach(() => {
     mockNavigate.mockClear();
   });
-
   it('checks header for create new', async () => {
     await act(() => {
       renderComponent(clusterTopicNewPath(clusterName));
@@ -57,14 +55,25 @@ describe('New', () => {
     renderComponent(`${clusterTopicCopyPath(clusterName)}?name=test`);
     expect(screen.getByRole('heading', { name: 'Copy' })).toBeInTheDocument();
   });
-
   it('validates form', async () => {
-    renderComponent(clusterTopicNewPath(clusterName));
+    await renderComponent(clusterTopicNewPath(clusterName));
     await userEvent.type(screen.getByPlaceholderText('Topic Name'), topicName);
     await userEvent.clear(screen.getByPlaceholderText('Topic Name'));
-    await waitFor(() => {
-      expect(screen.getByText('name is a required field')).toBeInTheDocument();
-    });
+    await userEvent.tab();
+    await expect(
+      screen.getByText('name is a required field')
+    ).toBeInTheDocument();
+
+    await userEvent.type(
+      screen.getByLabelText('Number of partitions *'),
+      minValue
+    );
+    await userEvent.clear(screen.getByLabelText('Number of partitions *'));
+    await userEvent.tab();
+    await expect(
+      screen.getByText('Number of partitions is required and must be a number')
+    ).toBeInTheDocument();
+
     expect(createTopicMock).not.toHaveBeenCalled();
     expect(mockNavigate).not.toHaveBeenCalled();
   });
@@ -83,6 +92,10 @@ describe('New', () => {
   it('submits valid form', async () => {
     renderComponent(clusterTopicNewPath(clusterName));
     await userEvent.type(screen.getByPlaceholderText('Topic Name'), topicName);
+    await userEvent.type(
+      screen.getByLabelText('Number of partitions *'),
+      minValue
+    );
     await userEvent.click(screen.getByText('Create topic'));
     expect(createTopicMock).toHaveBeenCalledTimes(1);
     expect(mockNavigate).toHaveBeenLastCalledWith(`../${topicName}`);

kafka-ui-react-app/src/components/Topics/Topic/Edit/Edit.tsx

@@ -19,6 +19,7 @@ import {
   useUpdateTopic,
 } from 'lib/hooks/api/topics';
 import DangerZone from 'components/Topics/Topic/Edit/DangerZone/DangerZone';
+import { ConfigSource } from 'generated-sources';
 
 export const TOPIC_EDIT_FORM_DEFAULT_PROPS = {
   partitions: 1,
@@ -54,16 +55,32 @@ const Edit: React.FC = () => {
   topicConfig?.forEach((param) => {
     config.byName[param.name] = param;
   });
-
   const onSubmit = async (data: TopicFormDataRaw) => {
-    await updateTopic.mutateAsync(data);
+    const filteredDirtyDefaultEntries = Object.entries(data).filter(
+      ([key, val]) => {
+        const isDirty =
+          String(val) !==
+          String(defaultValues[key as keyof typeof defaultValues]);
+
+        const isDefaultConfig =
+          config.byName[key]?.source === ConfigSource.DEFAULT_CONFIG;
+
+        // if it is changed should be sent or if it was Dynamic
+        return isDirty || !isDefaultConfig;
+      }
+    );
+
+    const newData = Object.fromEntries(filteredDirtyDefaultEntries);
+
+    await updateTopic.mutateAsync(newData);
     navigate('../');
   };
 
   return (
     <>
       <FormProvider {...methods}>
         <TopicForm
+          config={config.byName}
           topicName={topicName}
           retentionBytes={defaultValues.retentionBytes}
           inSyncReplicas={Number(defaultValues.minInSyncReplicas)}

kafka-ui-react-app/src/components/Topics/Topic/Edit/__test__/Edit.spec.tsx

@@ -70,7 +70,7 @@ describe('Edit Component', () => {
     renderComponent();
     const btn = screen.getAllByText(/Update topic/i)[0];
     const field = screen.getByRole('spinbutton', {
-      name: 'Min In Sync Replicas * Min In Sync Replicas *',
+      name: 'Min In Sync Replicas Min In Sync Replicas',
     });
     await userEvent.type(field, '1');
     await userEvent.click(btn);

kafka-ui-react-app/src/components/Topics/Topic/Edit/topicParamsTransformer.ts

@@ -20,7 +20,7 @@ const topicParamsTransformer = (topic?: Topic, config?: TopicConfig[]) => {
 
   const customParams = config.reduce((acc, { name, value, defaultValue }) => {
     if (value === defaultValue) return acc;
-    if (!Object.keys(TOPIC_CUSTOM_PARAMS).includes(name)) return acc;
+    if (!TOPIC_CUSTOM_PARAMS[name]) return acc;
     return [...acc, { name, value }];
   }, [] as { name: string; value?: string }[]);
 

kafka-ui-react-app/src/components/Topics/shared/Form/CustomParams/CustomParamField.tsx

@@ -2,18 +2,20 @@ import React, { useRef } from 'react';
 import { ErrorMessage } from '@hookform/error-message';
 import { TOPIC_CUSTOM_PARAMS } from 'lib/constants';
 import { FieldArrayWithId, useFormContext, Controller } from 'react-hook-form';
-import { TopicFormData } from 'redux/interfaces';
+import { TopicConfigParams, TopicFormData } from 'redux/interfaces';
 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 CloseIcon from 'components/common/Icons/CloseIcon';
 import * as C from 'components/Topics/shared/Form/TopicForm.styled';
+import { ConfigSource } from 'generated-sources';
 
 import * as S from './CustomParams.styled';
 
 export interface Props {
+  config?: TopicConfigParams;
   isDisabled: boolean;
   index: number;
   existingFields: string[];
@@ -27,6 +29,7 @@ const CustomParamField: React.FC<Props> = ({
   isDisabled,
   index,
   remove,
+  config,
   existingFields,
   setExistingFields,
 }) => {
@@ -44,7 +47,10 @@ const CustomParamField: React.FC<Props> = ({
     .map((option) => ({
       value: option,
       label: option,
-      disabled: existingFields.includes(option),
+      disabled:
+        (config &&
+          config[option].source !== ConfigSource.DYNAMIC_TOPIC_CONFIG) ||
+        existingFields.includes(option),
     }));
 
   React.useEffect(() => {

kafka-ui-react-app/src/components/Topics/shared/Form/CustomParams/CustomParams.tsx

@@ -1,5 +1,5 @@
 import React from 'react';
-import { TopicFormData } from 'redux/interfaces';
+import { TopicConfigParams, TopicFormData } from 'redux/interfaces';
 import { useFieldArray, useFormContext, useWatch } from 'react-hook-form';
 import { Button } from 'components/common/Button/Button';
 import { TOPIC_CUSTOM_PARAMS_PREFIX } from 'lib/constants';
@@ -9,10 +9,15 @@ import CustomParamField from './CustomParamField';
 import * as S from './CustomParams.styled';
 
 export interface CustomParamsProps {
+  config?: TopicConfigParams;
   isSubmitting: boolean;
+  isEditing?: boolean;
 }
 
-const CustomParams: React.FC<CustomParamsProps> = ({ isSubmitting }) => {
+const CustomParams: React.FC<CustomParamsProps> = ({
+  isSubmitting,
+  config,
+}) => {
   const { control } = useFormContext<TopicFormData>();
   const { fields, append, remove } = useFieldArray({
     control,
@@ -41,9 +46,10 @@ const CustomParams: React.FC<CustomParamsProps> = ({ isSubmitting }) => {
 
   return (
     <S.ParamsWrapper>
-      {controlledFields.map((field, idx) => (
+      {controlledFields?.map((field, idx) => (
         <CustomParamField
           key={field.id}
+          config={config}
           field={field}
           remove={removeField}
           index={idx}