Improve error handling and edge cases of UnitInputSelection

webpack/components/ResourceQuotaForm/ResourceQuotaFormConstants.js

@@ -19,14 +19,14 @@ export const RESOURCE_NAME_DISK = 'Disk space';
 /* Resource units (order the units with increasing factor!) */
 export const RESOURCE_UNIT_CPU = [{ symbol: 'cores', factor: 1 }];
 export const RESOURCE_UNIT_MEMORY = [
-  { symbol: 'MB', factor: 1 },
-  { symbol: 'GB', factor: 1024 },
-  { symbol: 'TB', factor: 1024 * 1024 },
+  { symbol: 'MiB', factor: 1 },
+  { symbol: 'GiB', factor: 1024 },
+  { symbol: 'TiB', factor: 1024 * 1024 },
 ];
 export const RESOURCE_UNIT_DISK = [
-  { symbol: 'GB', factor: 1 },
-  { symbol: 'TB', factor: 1024 },
-  { symbol: 'PB', factor: 1024 * 1024 },
+  { symbol: 'GiB', factor: 1 },
+  { symbol: 'TiB', factor: 1024 },
+  { symbol: 'PiB', factor: 1024 * 1024 },
 ];
 
 /* Resource value bounds */

webpack/components/ResourceQuotaForm/components/Resource/UnitInputField.js

@@ -2,6 +2,7 @@ import React, { useState, useEffect, useCallback } from 'react';
 import PropTypes from 'prop-types';
 import {
   FormGroup,
+  FormHelperText,
   TextInput,
   InputGroup,
   InputGroupText,
@@ -60,17 +61,19 @@ const UnitInputField = ({
   }, [minValue, maxValue, selectedUnit]);
 
   /* text for float errors */
-  const errorTextNatural = useCallback(
-    () => __('Value must be a natural number.'),
-    []
-  );
+  const errorTextNatural = useCallback(() => __('Value must be a number.'), []);
 
-  /* text for float errors */
-  const errorTextFloating = useCallback(
-    () => __(`No floating point for smallest unit (${units[0].symbol}).`),
+  /* text for float inputs (rounding) */
+  const warningTextRounded = useCallback(
+    roundedValue => __(`Rounding to: ${roundedValue} (${units[0].symbol}).`),
     [units]
   );
 
+  /* warning text displayed beneath value input field (built-in is used for errors) */
+  const helperTextWarning = (text, isHidden) => (
+    <FormHelperText isHidden={isHidden}>{text}</FormHelperText>
+  );
+
   /* applies the selected unit and checks the bounds */
   const isValid = useCallback(
     val => {
@@ -83,20 +86,9 @@ const UnitInputField = ({
         setErrorText(errorTextBounds());
         return false;
       }
-      if (baseValue !== Math.floor(baseValue)) {
-        setErrorText(errorTextFloating());
-        return false;
-      }
       return true;
     },
-    [
-      minValue,
-      maxValue,
-      valueToBaseUnit,
-      errorTextNatural,
-      errorTextBounds,
-      errorTextFloating,
-    ]
+    [minValue, maxValue, valueToBaseUnit, errorTextNatural, errorTextBounds]
   );
 
   /* applies the selected unit and returns the base-unit value */
@@ -116,9 +108,17 @@ const UnitInputField = ({
       setValidated('default');
     } else if (isValid(inputValue)) {
       const baseValue = valueToBaseUnit(inputValue);
-      onChange(baseValue);
+      let validatedValue = baseValue;
+      if (baseValue !== Math.floor(baseValue)) {
+        validatedValue = Math.floor(baseValue);
+        setErrorText(warningTextRounded(validatedValue));
+        setValidated('warning');
+      } else {
+        // Keep baseValue as validatedValue
+        setValidated('default');
+      }
+      onChange(validatedValue);
       handleInputValidation(true);
-      setValidated('default');
     } else {
       handleInputValidation(false);
       setValidated('error');
@@ -131,6 +131,7 @@ const UnitInputField = ({
     onChange,
     isValid,
     valueToBaseUnit,
+    warningTextRounded,
   ]);
 
   /* set the selected unit */
@@ -181,6 +182,7 @@ const UnitInputField = ({
       label={__('Quota Limit')}
       validated={validated}
       helperTextInvalid={errorText}
+      helperText={helperTextWarning(errorText, validated !== 'warning')}
       fieldId="quota-limit-resource-quota-form-group"
       labelIcon={labelIcon || {}}
     >

webpack/components/ResourceQuotaForm/components/Resource/__test__/UnitInputField.test.js

@@ -0,0 +1,108 @@
+import React from 'react';
+import '@testing-library/jest-dom';
+import { render, screen, fireEvent } from '@testing-library/react';
+
+import { testComponentSnapshotsWithFixtures } from '@theforeman/test';
+import LabelIcon from 'foremanReact/components/common/LabelIcon';
+
+import UnitInputField from '../UnitInputField';
+
+const getDefaultProps = () => ({
+  initialValue: 0,
+  onChange: jest.fn(),
+  isDisabled: false,
+  handleInputValidation: jest.fn(),
+  units: [
+    { symbol: 'MiB', factor: 1 },
+    { symbol: 'GiB', factor: 1024 },
+  ],
+  labelIcon: <LabelIcon text="Descriptive title." />,
+  minValue: 0,
+  maxValue: 5,
+});
+
+const fixtureDefault = {
+  'should render default': {
+    ...getDefaultProps(),
+  },
+};
+
+const fixtureSingleUnit = {
+  'should render without dropdown (single unit)': {
+    ...getDefaultProps(),
+    units: [{ symbol: 'cores', factor: 1 }],
+  },
+};
+
+const fixtureDisabled = {
+  'should render as disabled field': {
+    ...getDefaultProps(),
+    isDisabled: true,
+  },
+};
+
+describe('UnitInputField', () => {
+  testComponentSnapshotsWithFixtures(UnitInputField, fixtureDefault);
+  testComponentSnapshotsWithFixtures(UnitInputField, fixtureSingleUnit);
+  testComponentSnapshotsWithFixtures(UnitInputField, fixtureDisabled);
+
+  it('triggers handleInputValidation on unit change', async () => {
+    const props = getDefaultProps();
+
+    render(<UnitInputField {...props} />);
+    const input = screen.getByRole('textbox');
+    fireEvent.change(input, { target: { value: 3 } });
+
+    // gets called (1.) with initialValue and (2.) the simulated change
+    expect(props.onChange).toHaveBeenCalledTimes(2);
+    expect(props.onChange).toHaveBeenCalledWith(props.initialValue);
+    expect(props.onChange).toHaveBeenLastCalledWith(3);
+    expect(props.handleInputValidation).toHaveBeenCalledTimes(2);
+    expect(props.handleInputValidation).toHaveBeenCalledWith(true);
+  });
+
+  test('triggers onChange with rounded value', () => {
+    const props = getDefaultProps();
+
+    render(<UnitInputField {...props} />);
+    const input = screen.getByRole('textbox');
+    fireEvent.change(input, { target: { value: 3.5 } });
+
+    // gets called (1.) with initialValue and (2.) the simulated change
+    expect(props.onChange).toHaveBeenCalledTimes(2);
+    expect(props.onChange).toHaveBeenCalledWith(props.initialValue);
+    expect(props.onChange).toHaveBeenLastCalledWith(3);
+    expect(props.handleInputValidation).toHaveBeenCalledTimes(2);
+    expect(props.handleInputValidation).toHaveBeenCalledWith(true);
+  });
+
+  test('does not trigger onChange when value out of bounds', () => {
+    const props = getDefaultProps();
+
+    render(<UnitInputField {...props} />);
+    const input = screen.getByRole('textbox');
+    fireEvent.change(input, { target: { value: props.maxValue + 1 } });
+
+    // onChange only called for initialValue
+    expect(props.onChange).toHaveBeenCalledTimes(1);
+    expect(props.onChange).toHaveBeenCalledWith(props.initialValue);
+    // handleInputValidation called with false => invalid
+    expect(props.handleInputValidation).toHaveBeenCalledTimes(2);
+    expect(props.handleInputValidation).toHaveBeenLastCalledWith(false);
+  });
+
+  test('does not trigger onChange when value is not a number', () => {
+    const props = getDefaultProps();
+
+    render(<UnitInputField {...props} />);
+    const input = screen.getByRole('textbox');
+    fireEvent.change(input, { target: { value: 'no number' } });
+
+    // onChange only called for initialValue
+    expect(props.onChange).toHaveBeenCalledTimes(1);
+    expect(props.onChange).toHaveBeenCalledWith(props.initialValue);
+    // handleInputValidation called with false => invalid
+    expect(props.handleInputValidation).toHaveBeenCalledTimes(2);
+    expect(props.handleInputValidation).toHaveBeenLastCalledWith(false);
+  });
+});

webpack/components/ResourceQuotaForm/components/Resource/__test__/__snapshots__/UnitInputField.test.js.snap

@@ -0,0 +1,155 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`UnitInputField should render as disabled field 1`] = `
+<FormGroup
+  fieldId="quota-limit-resource-quota-form-group"
+  helperText={
+    <FormHelperText
+      isHidden={true}
+    >
+
+    </FormHelperText>
+  }
+  helperTextInvalid=""
+  label="Quota Limit"
+  labelIcon={
+    <LabelIcon
+      text="Descriptive title."
+    />
+  }
+  validated="default"
+>
+  <InputGroup>
+    <TextInput
+      id="reg_token_life_time_input"
+      isDisabled={true}
+      max={5}
+      min={0}
+      onChange={[Function]}
+      validated="default"
+      value={0}
+    />
+    <Dropdown
+      dropdownItems={
+        Array [
+          <DropdownItem
+            id="unit-dropdownitem-mib"
+          >
+            MiB
+          </DropdownItem>,
+          <DropdownItem
+            id="unit-dropdownitem-gib"
+          >
+            GiB
+          </DropdownItem>,
+        ]
+      }
+      isOpen={false}
+      onSelect={[Function]}
+      toggle={
+        <DropdownToggle
+          isDisabled={true}
+          onToggle={[Function]}
+        >
+          MiB
+        </DropdownToggle>
+      }
+    />
+  </InputGroup>
+</FormGroup>
+`;
+
+exports[`UnitInputField should render default 1`] = `
+<FormGroup
+  fieldId="quota-limit-resource-quota-form-group"
+  helperText={
+    <FormHelperText
+      isHidden={true}
+    >
+
+    </FormHelperText>
+  }
+  helperTextInvalid=""
+  label="Quota Limit"
+  labelIcon={
+    <LabelIcon
+      text="Descriptive title."
+    />
+  }
+  validated="default"
+>
+  <InputGroup>
+    <TextInput
+      id="reg_token_life_time_input"
+      isDisabled={false}
+      max={5}
+      min={0}
+      onChange={[Function]}
+      validated="default"
+      value={0}
+    />
+    <Dropdown
+      dropdownItems={
+        Array [
+          <DropdownItem
+            id="unit-dropdownitem-mib"
+          >
+            MiB
+          </DropdownItem>,
+          <DropdownItem
+            id="unit-dropdownitem-gib"
+          >
+            GiB
+          </DropdownItem>,
+        ]
+      }
+      isOpen={false}
+      onSelect={[Function]}
+      toggle={
+        <DropdownToggle
+          isDisabled={false}
+          onToggle={[Function]}
+        >
+          MiB
+        </DropdownToggle>
+      }
+    />
+  </InputGroup>
+</FormGroup>
+`;
+
+exports[`UnitInputField should render without dropdown (single unit) 1`] = `
+<FormGroup
+  fieldId="quota-limit-resource-quota-form-group"
+  helperText={
+    <FormHelperText
+      isHidden={true}
+    >
+
+    </FormHelperText>
+  }
+  helperTextInvalid=""
+  label="Quota Limit"
+  labelIcon={
+    <LabelIcon
+      text="Descriptive title."
+    />
+  }
+  validated="default"
+>
+  <InputGroup>
+    <TextInput
+      id="reg_token_life_time_input"
+      isDisabled={false}
+      max={5}
+      min={0}
+      onChange={[Function]}
+      validated="default"
+      value={0}
+    />
+    <InputGroupText>
+      cores
+    </InputGroupText>
+  </InputGroup>
+</FormGroup>
+`;