Browse Source

Fix default properties' values propagation upon topic edit (#2205)

* fix topic  edit form

* remove unused import

* add test for topicParamsTransformer

* add tests for topicParamsTransformer

* try to fix tests topicParamsTransformer

* try to fix tests topicParamsTransformer

* try to fix tests topicParamsTransformer

* fix tests for topicParamsTransformer

* try to fix tests topicParamsTransformer

* try to fix tests topicParamsTransformer

* try to fix tests topicParamsTransformer

* try to fix tests topicParamsTransformer

* fix review comments

* fix tests name

* fix tests name

* remove default value from getValue function

* fix tests names

* fix topic create page

* remove unused import

Co-authored-by: Roman Zabaluev <rzabaluev@provectus.com>
Smbat Siradeghyan 2 years ago
parent
commit
819ae60e6b

+ 1 - 1
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>

+ 15 - 27
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)}
             />

+ 64 - 0
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',
+    },
+  ],
+};

+ 101 - 0
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);
+    });
+  });
+});

+ 43 - 0
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;

+ 13 - 2
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}

+ 2 - 2
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,