Repository: provectus/kafka-ui
Issue: Topics: Number of partitions field validation should not allow `.` and `,`
Labels: type/bug
good first issue
scope/frontend
status/accepted
status/confirmed


**Describe the bug** (Actual behavior)
It's possible to fill "," within Number of partitions field

**Expected behavior**
Let's not allow to add "," as there can be only integers from 1  

**Set up**
https://www.kafka-ui.provectus.io/


**Steps to Reproduce**
<!-- We'd like you to provide an example setup (via docker-compose, helm, etc.) 
to reproduce the problem, especially with a complex setups. -->

1. Navigate to Topics
2. Add a Topic
3. Fill "Number of Partitions" with copy pasting ",77" or add "1,5"

**Screenshots**

https://user-images.githubusercontent.com/104780608/217461981-bc94e309-7bd1-4386-b8ba-0b5a413ab9e4.mov







Issue comments:
I guess that not only `.`, but also `,` should not be allowed and not only for partitions, but for in sync replicas, time to retain data and maximum message size. They all should not accept decimal values.

Pull request review comments:
I would ask you to create helper function and cover it with unit tests
i think this function can be located outside of the component , and transformed into a pure function since it returns a boolean it does not have any side effects.

That way this function will not be created every-time `Input` component re-renders.
Thanks for your suggestion. I forgot to take this into consideration, and I've fixed this in the new commit.

Patch that resolved the issue:
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 ae9d429236a..e8d7f1b844a 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
@@ -116,6 +116,8 @@ const TopicForm: React.FC<Props> = ({
                   placeholder="Number of partitions"
                   min="1"
                   name="partitions"
+                  positiveOnly
+                  integerOnly
                 />
                 <FormError>
                   <ErrorMessage errors={errors} name="partitions" />
@@ -161,6 +163,8 @@ const TopicForm: React.FC<Props> = ({
               placeholder="Min In Sync Replicas"
               min="1"
               name="minInSyncReplicas"
+              positiveOnly
+              integerOnly
             />
             <FormError>
               <ErrorMessage errors={errors} name="minInSyncReplicas" />
@@ -177,6 +181,8 @@ const TopicForm: React.FC<Props> = ({
                 placeholder="Replication Factor"
                 min="1"
                 name="replicationFactor"
+                positiveOnly
+                integerOnly
               />
               <FormError>
                 <ErrorMessage errors={errors} name="replicationFactor" />
@@ -227,6 +233,8 @@ const TopicForm: React.FC<Props> = ({
               placeholder="Maximum message size"
               min="1"
               name="maxMessageBytes"
+              positiveOnly
+              integerOnly
             />
             <FormError>
               <ErrorMessage errors={errors} name="maxMessageBytes" />
diff --git a/kafka-ui-react-app/src/components/common/Input/Input.tsx b/kafka-ui-react-app/src/components/common/Input/Input.tsx
index 932f954aa5c..f5049e69618 100644
--- a/kafka-ui-react-app/src/components/common/Input/Input.tsx
+++ b/kafka-ui-react-app/src/components/common/Input/Input.tsx
@@ -11,6 +11,87 @@ export interface InputProps
   hookFormOptions?: RegisterOptions;
   search?: boolean;
   positiveOnly?: boolean;
+
+  // Some may only accept integer, like `Number of Partitions`
+  // some may accept decimal
+  integerOnly?: boolean;
+}
+
+function inputNumberCheck(
+  key: string,
+  positiveOnly: boolean,
+  integerOnly: boolean,
+  getValues: (name: string) => string,
+  componentName: string
+) {
+  let isValid = true;
+  if (!((key >= '0' && key <= '9') || key === '-' || key === '.')) {
+    // If not a valid digit char.
+    isValid = false;
+  } else {
+    // If there is any restriction.
+    if (positiveOnly) {
+      isValid = !(key === '-');
+    }
+    if (isValid && integerOnly) {
+      isValid = !(key === '.');
+    }
+
+    // Check invalid format
+    const value = getValues(componentName);
+
+    if (isValid && (key === '-' || key === '.')) {
+      if (!positiveOnly) {
+        if (key === '-') {
+          if (value !== '') {
+            // '-' should not appear anywhere except the start of the string
+            isValid = false;
+          }
+        }
+      }
+      if (!integerOnly) {
+        if (key === '.') {
+          if (value === '' || value.indexOf('.') !== -1) {
+            // '.' should not appear at the start of the string or appear twice
+            isValid = false;
+          }
+        }
+      }
+    }
+  }
+  return isValid;
+}
+
+function pasteNumberCheck(
+  text: string,
+  positiveOnly: boolean,
+  integerOnly: boolean
+) {
+  let value: string;
+  value = text;
+  let sign = '';
+  if (!positiveOnly) {
+    if (value.charAt(0) === '-') {
+      sign = '-';
+    }
+  }
+  if (integerOnly) {
+    value = value.replace(/\D/g, '');
+  } else {
+    value = value.replace(/[^\d.]/g, '');
+    if (value.indexOf('.') !== value.lastIndexOf('.')) {
+      const strs = value.split('.');
+      value = '';
+      for (let i = 0; i < strs.length; i += 1) {
+        value += strs[i];
+        if (i === 0) {
+          value += '.';
+        }
+      }
+    }
+  }
+  value = sign + value;
+  return value;
 }
 
 const Input: React.FC<InputProps> = ({
@@ -20,17 +101,27 @@ const Input: React.FC<InputProps> = ({
   inputSize = 'L',
   type,
   positiveOnly,
+  integerOnly,
   ...rest
 }) => {
   const methods = useFormContext();
+
   const keyPressEventHandler = (
     event: React.KeyboardEvent<HTMLInputElement>
   ) => {
-    const { key, code } = event;
+    const { key } = event;
     if (type === 'number') {
-      // Manualy prevent input of 'e' character for all number inputs
+      // Manually prevent input of non-digit and non-minus for all number inputs
       // and prevent input of negative numbers for positiveOnly inputs
-      if (key === 'e' || (positiveOnly && (key === '-' || code === 'Minus'))) {
+      if (
+        !inputNumberCheck(
+          key,
+          typeof positiveOnly === 'boolean' ? positiveOnly : false,
+          typeof integerOnly === 'boolean' ? integerOnly : false,
+          methods.getValues,
+          typeof name === 'string' ? name : ''
+        )
+      ) {
         event.preventDefault();
       }
     }
@@ -38,17 +129,14 @@ const Input: React.FC<InputProps> = ({
   const pasteEventHandler = (event: React.ClipboardEvent<HTMLInputElement>) => {
     if (type === 'number') {
       const { clipboardData } = event;
-      const text = clipboardData.getData('Text');
-      // replace all non-digit characters with empty string
-      let value = text.replace(/[^\d.]/g, '');
-      if (positiveOnly) {
-        // check if value is negative
-        const parsedData = parseFloat(value);
-        if (parsedData < 0) {
-          // remove minus sign
-          value = String(Math.abs(parsedData));
-        }
-      }
+      // The 'clipboardData' does not have key 'Text', but has key 'text' instead.
+      const text = clipboardData.getData('text');
+      // Check the format of pasted text.
+      const value = pasteNumberCheck(
+        text,
+        typeof positiveOnly === 'boolean' ? positiveOnly : false,
+        typeof integerOnly === 'boolean' ? integerOnly : false
+      );
       // if paste value contains non-numeric characters or
       // negative for positiveOnly fields then prevent paste
       if (value !== text) {
diff --git a/kafka-ui-react-app/src/components/common/Input/__tests__/Input.spec.tsx b/kafka-ui-react-app/src/components/common/Input/__tests__/Input.spec.tsx
index 3082f6f610a..0254196965d 100644
--- a/kafka-ui-react-app/src/components/common/Input/__tests__/Input.spec.tsx
+++ b/kafka-ui-react-app/src/components/common/Input/__tests__/Input.spec.tsx
@@ -4,12 +4,23 @@ import { screen } from '@testing-library/react';
 import { render } from 'lib/testHelpers';
 import userEvent from '@testing-library/user-event';
 
+// Mock useFormContext
+let component: HTMLInputElement;
+
 const setupWrapper = (props?: Partial<InputProps>) => (
   <Input name="test" {...props} />
 );
 jest.mock('react-hook-form', () => ({
   useFormContext: () => ({
     register: jest.fn(),
+
+    // Mock methods.getValues and methods.setValue
+    getValues: jest.fn(() => {
+      return component.value;
+    }),
+    setValue: jest.fn((key, val) => {
+      component.value = val;
+    }),
   }),
 }));
 
@@ -23,20 +34,146 @@ describe('Custom Input', () => {
     });
   });
   describe('number', () => {
-    const getInput = () => screen.getByRole('spinbutton');
+    const getInput = () => screen.getByRole<HTMLInputElement>('spinbutton');
+
+    describe('input', () => {
+      it('allows user to type numbers only', async () => {
+        render(setupWrapper({ type: 'number' }));
+        const input = getInput();
+        component = input;
+        await userEvent.type(input, 'abc131');
+        expect(input).toHaveValue(131);
+      });
+
+      it('allows user to type negative values', async () => {
+        render(setupWrapper({ type: 'number' }));
+        const input = getInput();
+        component = input;
+        await userEvent.type(input, '-2');
+        expect(input).toHaveValue(-2);
+      });
+
+      it('allows user to type positive values only', async () => {
+        render(setupWrapper({ type: 'number', positiveOnly: true }));
+        const input = getInput();
+        component = input;
+        await userEvent.type(input, '-2');
+        expect(input).toHaveValue(2);
+      });
+
+      it('allows user to type decimal', async () => {
+        render(setupWrapper({ type: 'number' }));
+        const input = getInput();
+        component = input;
+        await userEvent.type(input, '2.3');
+        expect(input).toHaveValue(2.3);
+      });
+
+      it('allows user to type integer only', async () => {
+        render(setupWrapper({ type: 'number', integerOnly: true }));
+        const input = getInput();
+        component = input;
+        await userEvent.type(input, '2.3');
+        expect(input).toHaveValue(23);
+      });
+
+      it("not allow '-' appear at any position of the string except the start", async () => {
+        render(setupWrapper({ type: 'number' }));
+        const input = getInput();
+        component = input;
+        await userEvent.type(input, '2-3');
+        expect(input).toHaveValue(23);
+      });
 
-    it('allows user to type only numbers', async () => {
-      render(setupWrapper({ type: 'number' }));
-      const input = getInput();
-      await userEvent.type(input, 'abc131');
-      expect(input).toHaveValue(131);
+      it("not allow '.' appear at the start of the string", async () => {
+        render(setupWrapper({ type: 'number' }));
+        const input = getInput();
+        component = input;
+        await userEvent.type(input, '.33');
+        expect(input).toHaveValue(33);
+      });
+
+      it("not allow '.' appear twice in the string", async () => {
+        render(setupWrapper({ type: 'number' }));
+        const input = getInput();
+        component = input;
+        await userEvent.type(input, '3.3.3');
+        expect(input).toHaveValue(3.33);
+      });
     });
 
-    it('allows negative values', async () => {
-      render(setupWrapper({ type: 'number' }));
-      const input = getInput();
-      await userEvent.type(input, '-2');
-      expect(input).toHaveValue(-2);
+    describe('paste', () => {
+      it('allows user to paste numbers only', async () => {
+        render(setupWrapper({ type: 'number' }));
+        const input = getInput();
+        component = input;
+        await userEvent.click(input);
+        await userEvent.paste('abc131');
+        expect(input).toHaveValue(131);
+      });
+
+      it('allows user to paste negative values', async () => {
+        render(setupWrapper({ type: 'number' }));
+        const input = getInput();
+        component = input;
+        await userEvent.click(input);
+        await userEvent.paste('-2');
+        expect(input).toHaveValue(-2);
+      });
+
+      it('allows user to paste positive values only', async () => {
+        render(setupWrapper({ type: 'number', positiveOnly: true }));
+        const input = getInput();
+        component = input;
+        await userEvent.click(input);
+        await userEvent.paste('-2');
+        expect(input).toHaveValue(2);
+      });
+
+      it('allows user to paste decimal', async () => {
+        render(setupWrapper({ type: 'number' }));
+        const input = getInput();
+        component = input;
+        await userEvent.click(input);
+        await userEvent.paste('2.3');
+        expect(input).toHaveValue(2.3);
+      });
+
+      it('allows user to paste integer only', async () => {
+        render(setupWrapper({ type: 'number', integerOnly: true }));
+        const input = getInput();
+        component = input;
+        await userEvent.click(input);
+        await userEvent.paste('2.3');
+        expect(input).toHaveValue(23);
+      });
+
+      it("not allow '-' appear at any position of the pasted string except the start", async () => {
+        render(setupWrapper({ type: 'number' }));
+        const input = getInput();
+        component = input;
+        await userEvent.click(input);
+        await userEvent.paste('2-3');
+        expect(input).toHaveValue(23);
+      });
+
+      it("not allow '.' appear at the start of the pasted string", async () => {
+        render(setupWrapper({ type: 'number' }));
+        const input = getInput();
+        component = input;
+        await userEvent.click(input);
+        await userEvent.paste('.33');
+        expect(input).toHaveValue(0.33);
+      });
+
+      it("not allow '.' appear twice in the pasted string", async () => {
+        render(setupWrapper({ type: 'number' }));
+        const input = getInput();
+        component = input;
+        await userEvent.click(input);
+        await userEvent.paste('3.3.3');
+        expect(input).toHaveValue(3.33);
+      });
     });
   });
 });
diff --git a/kafka-ui-react-app/src/components/common/NewTable/__test__/Table.spec.tsx b/kafka-ui-react-app/src/components/common/NewTable/__test__/Table.spec.tsx
index 43dd7b8dbf7..c60cccdabea 100644
--- a/kafka-ui-react-app/src/components/common/NewTable/__test__/Table.spec.tsx
+++ b/kafka-ui-react-app/src/components/common/NewTable/__test__/Table.spec.tsx
@@ -20,6 +20,17 @@ jest.mock('react-router-dom', () => ({
   useNavigate: () => mockedUsedNavigate,
 }));
 
+// This is needed by ESLint.
+jest.mock('react-hook-form', () => ({
+  useFormContext: () => ({
+    register: jest.fn(),
+
+    // Mock methods.getValues and methods.setValue
+    getValues: jest.fn(),
+    setValue: jest.fn(),
+  }),
+}));
+
 type Datum = typeof data[0];
 
 const data = [


Test patch: