Преглед на файлове

[FE] Fix number of partitions field validation issue (#3400)

* Now user can only input valid digits, and '-' is not allowed in positive-only inputs.

* Fix ISSUE#3319

Now user can only input valid digits, and '-' is not allowed in positive-only inputs.

* Revert "Fix ISSUE#3319"

This reverts commit a4e34f5af3b4f049bc19089cfa53b7a99aafb0cf.

* Fix ISSUE#3319
Created a helper function, and added a unit test to cover it.

* Fix ISSUE#3319
Located the helper function outside the component, and renamed some unit tests to make their meaning more clear.

* Fix ISSUE#3319
- Added an attribute 'integerOnly' to component 'Input', to represent whether this input component instance will accept decimal.
- Improved input-check function and paste-check function, to avoid invalid number format (like '3-3', '3.3.3').
- Added new unit tests to test new input-check and paste-check functions.
- Added attribute 'integerOnly' to Input instances in the TopicForm component.
Snowfox0618 преди 2 години
родител
ревизия
18c046af5b

+ 8 - 0
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" />

+ 102 - 14
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) {

+ 148 - 11
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);
+      });
     });
   });
 });

+ 11 - 0
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 = [