Repository: provectus/kafka-ui
Issue: [Topics] There are default values when user try to edit topic
Labels: type/bug
good first issue
scope/frontend
status/accepted
status/confirmed

**Describe the bug**
When user try to edit topic after creation there are defaults values in edit topic settings view


**Set up**
Local docker.
App version: Version:v0.3.3(38c4cf7)


**Steps to Reproduce**
1. Create topic (e.g. using API)
2. Go to newly created topic
3. Go to Edit Settings
4. Edit something (e.g. change Cleanup policy)
5. Save
6. Go to Edit settings again
7. Observe the view

**Expected behavior**
At Edit settings view should be current topic's values

**Actual result**
All setting are default



Issue comments:
Hello there sbritprovectus! 👋

Thank you and congratulations 🎉 for opening your very first issue in this project! 💖

In case you want to claim this issue, please comment down below! We will try to get back to you as soon as we can. 👀
Almost all fields are not set according to the actual values but rather contain the widgets default value. I'm happy to take a look at this.

Pull request review comments:
I'm not sure why this is here. I can see you reassign it `formInit = false;` on line 79 inside useEffect. This is not a good way to do, instead use useState, or if you don't need to trigger rerenders use useRef to store this variable.
Just create a ./fixtures file somewhere near this test file to separate such huge objects from the tests.
```suggestion
const startParams: TopicWithDetailedInfo = {
```
```suggestion
      source: ConfigSource.DEFAULT_CONFIG,
```
check this file. We already have fixtures
```
src/components/Topics/Topic/Edit/__test__/fixtures.ts
```
pls cover all the cases it is not complex
it returns default values when topic is not defined
describe maxMessageBytes
 - it 
 - it convert to number
 - it returns default value when 
test case
```suggestion
    it('returns completed values', () => {
```
pls fix it for all cases
```suggestion
    it('returns transformed values', () => {
```
```suggestion
        transformedParams.partitions
```
move payload to fixtures file
I would prefer to move `|| -1` logic inside of `getValue`. So add one more `defaultValue` param
```suggestion
    it('returns transformed value', () => {
```
```suggestion
    it(`returns default value when ${name} not defined`, () => {
```
```suggestion
    it('returns number value', () => {
```
```suggestion
    it('returns value when field exists', () => {
```
```suggestion
    it('returns default value when field does not exist', () => {
```
```suggestion
    it('returns value when filed name does not exist', () => {
```
```suggestion
    it('returns transformed value', () => {
```
```suggestion
    it('returns default value when partitionCount not defined', () => {
```
```suggestion
  describe('customParams', () => {
    it('returns value when configs is empty', () => {
```
Are you sure we can use `-1` by default? ping @Haarolean 
Uuhm is that like for all the int params? Seems dangerous. How did it work before?
I added that. There was no such thing.
```suggestion
  defaultValue?: string | number
```
returns

Patch that resolved the issue:
diff --git a/kafka-ui-react-app/src/components/Topics/New/New.tsx b/kafka-ui-react-app/src/components/Topics/New/New.tsx
index 7346820afce..7faefb36c9d 100644
--- a/kafka-ui-react-app/src/components/Topics/New/New.tsx
+++ b/kafka-ui-react-app/src/components/Topics/New/New.tsx
@@ -57,7 +57,7 @@ const New: React.FC = () => {
           partitionCount={Number(partitionCount)}
           replicationFactor={Number(replicationFactor)}
           inSyncReplicas={Number(inSyncReplicas)}
-          isSubmitting={methods.formState.isSubmitting}
+          isSubmitting={false}
           onSubmit={methods.handleSubmit(onSubmit)}
         />
       </FormProvider>
diff --git a/kafka-ui-react-app/src/components/Topics/Topic/Edit/Edit.tsx b/kafka-ui-react-app/src/components/Topics/Topic/Edit/Edit.tsx
index 807c6e9c7b1..76309dc182d 100644
--- a/kafka-ui-react-app/src/components/Topics/Topic/Edit/Edit.tsx
+++ b/kafka-ui-react-app/src/components/Topics/Topic/Edit/Edit.tsx
@@ -1,10 +1,9 @@
-import React from 'react';
+import React, { useEffect } from 'react';
 import {
   ClusterName,
   TopicFormDataRaw,
   TopicName,
   TopicConfigByName,
-  TopicWithDetailedInfo,
   TopicFormData,
 } from 'redux/interfaces';
 import { useForm, FormProvider } from 'react-hook-form';
@@ -13,12 +12,13 @@ import { RouteParamsClusterTopic } from 'lib/paths';
 import { useNavigate } from 'react-router-dom';
 import { yupResolver } from '@hookform/resolvers/yup';
 import { topicFormValidationSchema } from 'lib/yupExtended';
-import { TOPIC_CUSTOM_PARAMS_PREFIX, TOPIC_CUSTOM_PARAMS } from 'lib/constants';
 import styled from 'styled-components';
 import PageHeading from 'components/common/PageHeading/PageHeading';
 import { useAppSelector } from 'lib/hooks/redux';
 import { getFullTopic } from 'redux/reducers/topics/selectors';
 import useAppParams from 'lib/hooks/useAppParams';
+import topicParamsTransformer from 'components/Topics/Topic/Edit/topicParamsTransformer';
+import { MILLISECONDS_IN_WEEK } from 'lib/constants';
 
 import DangerZoneContainer from './DangerZone/DangerZoneContainer';
 
@@ -39,6 +39,7 @@ export interface Props {
 const EditWrapperStyled = styled.div`
   display: flex;
   justify-content: center;
+
   & > * {
     width: 800px;
   }
@@ -50,29 +51,9 @@ export const DEFAULTS = {
   minInSyncReplicas: 1,
   cleanupPolicy: 'delete',
   retentionBytes: -1,
+  retentionMs: MILLISECONDS_IN_WEEK,
   maxMessageBytes: 1000012,
-};
-
-const topicParams = (topic: TopicWithDetailedInfo | undefined) => {
-  if (!topic) {
-    return DEFAULTS;
-  }
-
-  const { name, replicationFactor } = topic;
-
-  return {
-    ...DEFAULTS,
-    name,
-    partitions: topic.partitionCount || DEFAULTS.partitions,
-    replicationFactor,
-    [TOPIC_CUSTOM_PARAMS_PREFIX]: topic.config
-      ?.filter(
-        (el) =>
-          el.value !== el.defaultValue &&
-          Object.keys(TOPIC_CUSTOM_PARAMS).includes(el.name)
-      )
-      .map((el) => ({ name: el.name, value: el.value })),
-  };
+  customParams: [],
 };
 
 let formInit = false;
@@ -87,7 +68,7 @@ const Edit: React.FC<Props> = ({
 
   const topic = useAppSelector((state) => getFullTopic(state, topicName));
 
-  const defaultValues = React.useMemo(() => topicParams(topic), [topic]);
+  const defaultValues = topicParamsTransformer(topic);
 
   const methods = useForm<TopicFormData>({
     defaultValues,
@@ -95,12 +76,16 @@ const Edit: React.FC<Props> = ({
     mode: 'onChange',
   });
 
+  useEffect(() => {
+    methods.reset(defaultValues);
+  }, [!topic]);
+
   const [isSubmitting, setIsSubmitting] = React.useState<boolean>(false);
   const navigate = useNavigate();
 
   React.useEffect(() => {
     fetchTopicConfig({ clusterName, topicName });
-  }, [fetchTopicConfig, clusterName, topicName]);
+  }, [fetchTopicConfig, clusterName, topicName, isTopicUpdated]);
 
   React.useEffect(() => {
     if (isSubmitting && isTopicUpdated) {
@@ -138,7 +123,10 @@ const Edit: React.FC<Props> = ({
           <FormProvider {...methods}>
             <TopicForm
               topicName={topicName}
+              retentionBytes={defaultValues.retentionBytes}
+              inSyncReplicas={Number(defaultValues.minInSyncReplicas)}
               isSubmitting={isSubmitting}
+              cleanUpPolicy={topic.cleanUpPolicy}
               isEditing
               onSubmit={methods.handleSubmit(onSubmit)}
             />
diff --git a/kafka-ui-react-app/src/components/Topics/Topic/Edit/__test__/fixtures.ts b/kafka-ui-react-app/src/components/Topics/Topic/Edit/__test__/fixtures.ts
index 49b7f033751..5cbdebd97aa 100644
--- a/kafka-ui-react-app/src/components/Topics/Topic/Edit/__test__/fixtures.ts
+++ b/kafka-ui-react-app/src/components/Topics/Topic/Edit/__test__/fixtures.ts
@@ -551,3 +551,67 @@ export const topicWithInfo: TopicWithDetailedInfo = {
   partitions,
   config,
 };
+export const customConfigs = [
+  {
+    name: 'segment.bytes',
+    value: '1',
+    defaultValue: '1073741824',
+    source: ConfigSource.DEFAULT_CONFIG,
+    isSensitive: false,
+    isReadOnly: false,
+    synonyms: [
+      {
+        name: 'log.segment.bytes',
+        value: '1073741824',
+        source: ConfigSource.DEFAULT_CONFIG,
+      },
+    ],
+  },
+  {
+    name: 'retention.ms',
+    value: '604',
+    defaultValue: '604800000',
+    source: ConfigSource.DYNAMIC_TOPIC_CONFIG,
+    isSensitive: false,
+    isReadOnly: false,
+    synonyms: [
+      {
+        name: 'retention.ms',
+        value: '604800000',
+        source: ConfigSource.DYNAMIC_TOPIC_CONFIG,
+      },
+    ],
+  },
+  {
+    name: 'flush.messages',
+    value: '92233',
+    defaultValue: '9223372036854775807',
+    source: ConfigSource.DEFAULT_CONFIG,
+    isSensitive: false,
+    isReadOnly: false,
+    synonyms: [
+      {
+        name: 'log.flush.interval.messages',
+        value: '9223372036854775807',
+        source: ConfigSource.DEFAULT_CONFIG,
+      },
+    ],
+  },
+];
+
+export const transformedParams = {
+  partitions: 1,
+  replicationFactor: 1,
+  cleanupPolicy: 'delete',
+  retentionBytes: -1,
+  maxMessageBytes: 1000012,
+  name: topicName,
+  minInSyncReplicas: 1,
+  retentionMs: 604800000,
+  customParams: [
+    {
+      name: 'delete.retention.ms',
+      value: '86400001',
+    },
+  ],
+};
diff --git a/kafka-ui-react-app/src/components/Topics/Topic/Edit/__test__/topicParamsTransformer.spec.ts b/kafka-ui-react-app/src/components/Topics/Topic/Edit/__test__/topicParamsTransformer.spec.ts
new file mode 100644
index 00000000000..5077dc3b29e
--- /dev/null
+++ b/kafka-ui-react-app/src/components/Topics/Topic/Edit/__test__/topicParamsTransformer.spec.ts
@@ -0,0 +1,101 @@
+import topicParamsTransformer, {
+  getValue,
+} from 'components/Topics/Topic/Edit/topicParamsTransformer';
+import { DEFAULTS } from 'components/Topics/Topic/Edit/Edit';
+
+import { transformedParams, customConfigs, topicWithInfo } from './fixtures';
+
+describe('topicParamsTransformer', () => {
+  const testField = (name: keyof typeof DEFAULTS, fieldName: string) => {
+    it('returns transformed value', () => {
+      expect(topicParamsTransformer(topicWithInfo)[name]).toEqual(
+        transformedParams[name]
+      );
+    });
+    it(`returns default value when ${name} not defined`, () => {
+      expect(
+        topicParamsTransformer({
+          ...topicWithInfo,
+          config: topicWithInfo.config?.filter(
+            (config) => config.name !== fieldName
+          ),
+        })[name]
+      ).toEqual(DEFAULTS[name]);
+    });
+
+    it('returns number value', () => {
+      expect(
+        typeof topicParamsTransformer(topicWithInfo).retentionBytes
+      ).toEqual('number');
+    });
+  };
+
+  describe('getValue', () => {
+    it('returns value when field exists', () => {
+      expect(
+        getValue(topicWithInfo, 'confluent.tier.segment.hotset.roll.min.bytes')
+      ).toEqual(104857600);
+    });
+    it('returns undefined when filed name does not exist', () => {
+      expect(getValue(topicWithInfo, 'some.unsupported.fieldName')).toEqual(
+        undefined
+      );
+    });
+    it('returns default value when field does not exist', () => {
+      expect(
+        getValue(topicWithInfo, 'some.unsupported.fieldName', 100)
+      ).toEqual(100);
+    });
+  });
+  describe('Topic', () => {
+    it('returns default values when topic not defined found', () => {
+      expect(topicParamsTransformer(undefined)).toEqual(DEFAULTS);
+    });
+
+    it('returns transformed values', () => {
+      expect(topicParamsTransformer(topicWithInfo)).toEqual(transformedParams);
+    });
+  });
+
+  describe('Topic partitions', () => {
+    it('returns transformed value', () => {
+      expect(topicParamsTransformer(topicWithInfo).partitions).toEqual(
+        transformedParams.partitions
+      );
+    });
+    it('returns default value when partitionCount not defined', () => {
+      expect(
+        topicParamsTransformer({ ...topicWithInfo, partitionCount: undefined })
+          .partitions
+      ).toEqual(DEFAULTS.partitions);
+    });
+  });
+
+  describe('maxMessageBytes', () =>
+    testField('maxMessageBytes', 'max.message.bytes'));
+
+  describe('minInSyncReplicas', () =>
+    testField('minInSyncReplicas', 'min.insync.replicas'));
+
+  describe('retentionBytes', () =>
+    testField('retentionBytes', 'retention.bytes'));
+
+  describe('retentionMs', () => testField('retentionMs', 'retention.ms'));
+
+  describe('customParams', () => {
+    it('returns value when configs is empty', () => {
+      expect(
+        topicParamsTransformer({ ...topicWithInfo, config: [] }).customParams
+      ).toEqual([]);
+    });
+
+    it('returns value when had a 2 custom configs', () => {
+      expect(
+        topicParamsTransformer({
+          ...topicWithInfo,
+          config: customConfigs,
+        }).customParams?.length
+      ).toEqual(2);
+    });
+  });
+});
diff --git a/kafka-ui-react-app/src/components/Topics/Topic/Edit/topicParamsTransformer.ts b/kafka-ui-react-app/src/components/Topics/Topic/Edit/topicParamsTransformer.ts
new file mode 100644
index 00000000000..1c5b8491c69
--- /dev/null
+++ b/kafka-ui-react-app/src/components/Topics/Topic/Edit/topicParamsTransformer.ts
@@ -0,0 +1,43 @@
+import { TopicWithDetailedInfo } from 'redux/interfaces';
+import {
+  MILLISECONDS_IN_WEEK,
+  TOPIC_CUSTOM_PARAMS,
+  TOPIC_CUSTOM_PARAMS_PREFIX,
+} from 'lib/constants';
+import { DEFAULTS } from 'components/Topics/Topic/Edit/Edit';
+
+export const getValue = (
+  topic: TopicWithDetailedInfo,
+  fieldName: string,
+  defaultValue?: number
+) =>
+  Number(topic?.config?.find((config) => config.name === fieldName)?.value) ||
+  defaultValue;
+
+const topicParamsTransformer = (topic?: TopicWithDetailedInfo) => {
+  if (!topic) {
+    return DEFAULTS;
+  }
+
+  const { name, replicationFactor } = topic;
+
+  return {
+    ...DEFAULTS,
+    name,
+    replicationFactor,
+    partitions: topic.partitionCount || DEFAULTS.partitions,
+    maxMessageBytes: getValue(topic, 'max.message.bytes', 1000012),
+    minInSyncReplicas: getValue(topic, 'min.insync.replicas', 1),
+    retentionBytes: getValue(topic, 'retention.bytes', -1),
+    retentionMs: getValue(topic, 'retention.ms', MILLISECONDS_IN_WEEK),
+
+    [TOPIC_CUSTOM_PARAMS_PREFIX]: topic.config
+      ?.filter(
+        (el) =>
+          el.value !== el.defaultValue &&
+          Object.keys(TOPIC_CUSTOM_PARAMS).includes(el.name)
+      )
+      .map((el) => ({ name: el.name, value: el.value })),
+  };
+};
+export default topicParamsTransformer;
diff --git a/kafka-ui-react-app/src/components/Topics/shared/Form/TopicForm.tsx b/kafka-ui-react-app/src/components/Topics/shared/Form/TopicForm.tsx
index d2cb51fae5d..13afbe12335 100644
--- a/kafka-ui-react-app/src/components/Topics/shared/Form/TopicForm.tsx
+++ b/kafka-ui-react-app/src/components/Topics/shared/Form/TopicForm.tsx
@@ -19,6 +19,7 @@ export interface Props {
   partitionCount?: number;
   replicationFactor?: number;
   inSyncReplicas?: number;
+  retentionBytes?: number;
   cleanUpPolicy?: string;
   isEditing?: boolean;
   isSubmitting: boolean;
@@ -40,6 +41,7 @@ const RetentionBytesOptions: Array<SelectOption> = [
 ];
 
 const TopicForm: React.FC<Props> = ({
+  retentionBytes,
   topicName,
   isEditing,
   isSubmitting,
@@ -55,8 +57,17 @@ const TopicForm: React.FC<Props> = ({
   } = useFormContext();
   const getCleanUpPolicy =
     CleanupPolicyOptions.find((option: SelectOption) => {
-      return option.value === cleanUpPolicy?.toLowerCase();
+      return (
+        option.value.toString().replace(/,/g, '_') ===
+        cleanUpPolicy?.toLowerCase()
+      );
     })?.value || CleanupPolicyOptions[0].value;
+
+  const getRetentionBytes =
+    RetentionBytesOptions.find((option: SelectOption) => {
+      return option.value === retentionBytes;
+    })?.value || RetentionBytesOptions[0].value;
+
   return (
     <StyledForm onSubmit={onSubmit} aria-label="topic form">
       <fieldset disabled={isSubmitting}>
@@ -180,7 +191,7 @@ const TopicForm: React.FC<Props> = ({
                   id="topicFormRetentionBytes"
                   aria-labelledby="topicFormRetentionBytesLabel"
                   name={name}
-                  value={RetentionBytesOptions[0].value}
+                  value={getRetentionBytes}
                   onChange={onChange}
                   minWidth="100%"
                   options={RetentionBytesOptions}
diff --git a/kafka-ui-react-app/src/redux/reducers/topics/topicsSlice.ts b/kafka-ui-react-app/src/redux/reducers/topics/topicsSlice.ts
index 271cc7798a9..4a661240197 100644
--- a/kafka-ui-react-app/src/redux/reducers/topics/topicsSlice.ts
+++ b/kafka-ui-react-app/src/redux/reducers/topics/topicsSlice.ts
@@ -169,12 +169,12 @@ const formatTopicUpdate = (form: TopicFormDataRaw): TopicUpdate => {
 
   return {
     configs: {
+      ...Object.values(customParams || {}).reduce(topicReducer, {}),
       'cleanup.policy': cleanupPolicy,
       'retention.ms': retentionMs,
       'retention.bytes': retentionBytes,
       'max.message.bytes': maxMessageBytes,
       'min.insync.replicas': minInSyncReplicas,
-      ...Object.values(customParams || {}).reduce(topicReducer, {}),
     },
   };
 };
@@ -355,7 +355,7 @@ export const clearTopicsMessages = createAsyncThunk<
   }
 });
 
-const initialState: TopicsState = {
+export const initialState: TopicsState = {
   byName: {},
   allNames: [],
   totalPages: 1,


Test patch: