Merge pull request #915 from mzazrivec/add_validation_to_migration_th…

…rottling_inputs

[#911] Add validation to migration throttling inputs and add CPU throttling field

app/javascript/react/screens/App/Settings/__tests__/__snapshots__/SettingsActions.test.js.snap

@@ -77,6 +77,7 @@ Array [
         },
         "transformation": Object {
           "limits": Object {
+            "cpu_limit_per_host": 10,
             "max_concurrent_tasks_per_ems": 10,
             "max_concurrent_tasks_per_host": 10,
           },
@@ -114,6 +115,7 @@ Array [
         },
         "transformation": Object {
           "limits": Object {
+            "cpu_limit_per_host": 10,
             "max_concurrent_tasks_per_ems": 10,
             "max_concurrent_tasks_per_host": 10,
           },

app/javascript/react/screens/App/Settings/__tests__/__snapshots__/SettingsReducer.test.js.snap

@@ -311,6 +311,7 @@ Object {
   "isSavingSettings": false,
   "postConversionHostsResults": Array [],
   "savedSettings": Object {
+    "cpu_limit_per_host": 10,
     "max_concurrent_tasks_per_ems": 10,
     "max_concurrent_tasks_per_host": 10,
   },
@@ -431,6 +432,7 @@ Object {
   "isSavingSettings": false,
   "postConversionHostsResults": Array [],
   "savedSettings": Object {
+    "cpu_limit_per_host": 10,
     "max_concurrent_tasks_per_ems": 10,
     "max_concurrent_tasks_per_host": 10,
   },

app/javascript/react/screens/App/Settings/helpers.js

@@ -3,22 +3,16 @@ import { RHV, OPENSTACK } from '../../../../common/constants';
 
 export const getFormValuesFromApiSettings = payload => ({
   max_concurrent_tasks_per_host: payload.transformation.limits.max_concurrent_tasks_per_host,
-  max_concurrent_tasks_per_ems: payload.transformation.limits.max_concurrent_tasks_per_ems
-  /* FIXME: remove the comment once backend is ready
-  cpu_limit_per_host: payload.transformation.limits.cpu_limit_per_host,
-  network_limit_per_host: payload.transformation.limits.network_limit_per_host
-  */
+  max_concurrent_tasks_per_ems: payload.transformation.limits.max_concurrent_tasks_per_ems,
+  cpu_limit_per_host: payload.transformation.limits.cpu_limit_per_host
 });
 
 export const getApiSettingsFromFormValues = values => ({
   transformation: {
     limits: {
       max_concurrent_tasks_per_host: values.max_concurrent_tasks_per_host,
-      max_concurrent_tasks_per_ems: values.max_concurrent_tasks_per_ems
-      /* FIXME: remove the comment once backend is ready
-      cpu_limit_per_host: values.cpu_limit_per_host,
-      network_limit_per_host: values.network_limit_per_host
-      */
+      max_concurrent_tasks_per_ems: values.max_concurrent_tasks_per_ems,
+      cpu_limit_per_host: values.cpu_limit_per_host
     }
   }
 });

app/javascript/react/screens/App/Settings/screens/GeneralSettings/GeneralSettings.js

@@ -3,7 +3,7 @@ import PropTypes from 'prop-types';
 import { reduxForm, Field } from 'redux-form';
 import { Form, Button, Icon, OverlayTrigger, Popover, Spinner } from 'patternfly-react';
 import NumberInput from '../../../common/forms/NumberInput';
-// import TextInputWithCheckbox from '../../../common/forms/TextInputWithCheckbox'; // FIXME: uncomment once backend is ready
+import TextInputWithCheckbox from '../../../common/forms/TextInputWithCheckbox';
 
 const FORM_NAME = 'settings';
 
@@ -34,14 +34,30 @@ export class GeneralSettings extends React.Component {
   };
 
   render() {
-    const { isFetchingServers, isFetchingSettings, isSavingSettings, savedSettings, settingsForm } = this.props;
+    const {
+      isFetchingServers,
+      isFetchingSettings,
+      isSavingSettings,
+      savedSettings,
+      settingsForm,
+      invalid,
+      pristine
+    } = this.props;
 
     const hasUnsavedChanges =
       settingsForm &&
       settingsForm.values &&
       Object.keys(savedSettings).some(key => savedSettings[key] !== settingsForm.values[key]);
 
-    // const inputEnabledFunction = value => value !== 'unlimited'; // FIXME: uncomment once backend is ready
+    const inputEnabledFunction = value => value !== 'unlimited';
+
+    const validatePercentInput = value => {
+      const numberRegex = /^\d+$/;
+      if ((inputEnabledFunction(value) && !numberRegex.test(value)) || value < 0 || value > 100) {
+        return __('The entered value must be between 0 and 100');
+      }
+      return null;
+    };
 
     return (
       <Spinner loading={isFetchingServers || isFetchingSettings} style={{ marginTop: 15 }}>
@@ -96,7 +112,6 @@ export class GeneralSettings extends React.Component {
                 />
               </div>
             </Form.FormGroup>
-            {/* FIXME: uncomment once backend is ready
             <Form.FormGroup />
             <div>
               <h3>{__('Resource Utilization Limits for Migrations')}</h3>
@@ -105,23 +120,18 @@ export class GeneralSettings extends React.Component {
               id="cpu_limit_per_host"
               name="cpu_limit_per_host"
               component={TextInputWithCheckbox}
-              normalize={TextInputWithCheckbox.normalizeStringToInt}
+              validate={validatePercentInput}
               label={__('Max CPU utilization per conversion host')}
               postfix="％"
               inputEnabledFunction={inputEnabledFunction}
+              initialUncheckedValue="unlimited"
             />
-            <Field
-              id="network_limit_per_host"
-              name="network_limit_per_host"
-              component={TextInputWithCheckbox}
-              normalize={TextInputWithCheckbox.normalizeStringToInt}
-              label={__('Total network throughput')}
-              postfix={__('MB/s')}
-              inputEnabledFunction={inputEnabledFunction}
-            />
-*/}
             <Form.FormGroup className="col-md-1 pull-left" style={{ marginTop: '40px' }}>
-              <Button bsStyle="primary" onClick={this.onApplyClick} disabled={!hasUnsavedChanges || isSavingSettings}>
+              <Button
+                bsStyle="primary"
+                onClick={this.onApplyClick}
+                disabled={!hasUnsavedChanges || isSavingSettings || invalid || pristine}
+              >
                 {__('Apply')}
               </Button>
               {isSavingSettings && (
@@ -150,7 +160,9 @@ GeneralSettings.propTypes = {
   settingsForm: PropTypes.object,
   fetchServersUrl: PropTypes.string,
   fetchSettingsUrl: PropTypes.string,
-  formChangeAction: PropTypes.func
+  formChangeAction: PropTypes.func,
+  invalid: PropTypes.bool,
+  pristine: PropTypes.bool
 };
 
 GeneralSettings.defaultProps = {

app/javascript/react/screens/App/Settings/screens/GeneralSettings/__tests__/__snapshots__/GeneralSettings.test.js.snap

@@ -115,6 +115,24 @@ exports[`GeneralSettings component renders the general settings page 1`] = `
           />
         </div>
       </FormGroup>
+      <FormGroup
+        bsClass="form-group"
+      />
+      <div>
+        <h3>
+          Resource Utilization Limits for Migrations
+        </h3>
+      </div>
+      <Field
+        component={[Function]}
+        id="cpu_limit_per_host"
+        initialUncheckedValue="unlimited"
+        inputEnabledFunction={[Function]}
+        label="Max CPU utilization per conversion host"
+        name="cpu_limit_per_host"
+        postfix="％"
+        validate={[Function]}
+      />
       <FormGroup
         bsClass="form-group"
         className="col-md-1 pull-left"

app/javascript/react/screens/App/Settings/settings.fixures.js

@@ -19,7 +19,8 @@ export const settings = Immutable({
   transformation: {
     limits: {
       max_concurrent_tasks_per_host: 10,
-      max_concurrent_tasks_per_ems: 10
+      max_concurrent_tasks_per_ems: 10,
+      cpu_limit_per_host: 10
     }
   },
   otherSettings: {

app/javascript/react/screens/App/common/forms/TextInputWithCheckbox.js

@@ -1,6 +1,6 @@
 import React from 'react';
 import PropTypes from 'prop-types';
-import { Form } from 'patternfly-react';
+import { Form, FormControl, HelpBlock, InputGroup } from 'patternfly-react';
 
 class TextInputWithCheckbox extends React.Component {
   constructor(props) {
@@ -11,18 +11,24 @@ class TextInputWithCheckbox extends React.Component {
   }
 
   render() {
-    const {
-      id,
-      label,
-      input: { value, onChange },
-      postfix
-    } = this.props;
+    const { id, label, input, postfix, initialUncheckedValue, meta } = this.props;
 
-    const onCheckboxChange = event => this.setState({ inputEnabled: event.target.checked });
+    const onCheckboxChange = event => {
+      if (event.target.checked) {
+        if (meta.initial === initialUncheckedValue) {
+          input.onChange(0);
+        } else {
+          input.onChange(meta.initial);
+        }
+      } else {
+        input.onChange(initialUncheckedValue);
+      }
+      this.setState({ inputEnabled: event.target.checked });
+    };
 
     return (
-      <Form.FormGroup>
-        <Form.ControlLabel className="col-md-4">
+      <Form.FormGroup validationState={meta.error ? 'error' : undefined}>
+        <Form.ControlLabel className="col-md-5">
           <div className="checkbox-inline pull-left">
             <label>
               <input
@@ -37,18 +43,11 @@ class TextInputWithCheckbox extends React.Component {
           </div>
         </Form.ControlLabel>
         <div className="col-md-2">
-          <div className="input-group">
-            <input
-              type="text"
-              className="form-control"
-              name={id}
-              id={id}
-              defaultValue={value}
-              readOnly={!this.state.inputEnabled}
-              onChange={event => onChange(event.target.value)}
-            />
-            <div className="input-group-addon postfix-label">{postfix}</div>
-          </div>
+          <InputGroup>
+            <FormControl type="text" name={id} id={id} readOnly={!this.state.inputEnabled} {...input} />
+            <InputGroup.Addon>{postfix}</InputGroup.Addon>
+          </InputGroup>
+          {this.state.inputEnabled && meta.error && <HelpBlock>{meta.error}</HelpBlock>}
         </div>
       </Form.FormGroup>
     );
@@ -63,9 +62,9 @@ TextInputWithCheckbox.propTypes = {
   }),
   label: PropTypes.string.isRequired,
   postfix: PropTypes.string,
-  inputEnabledFunction: PropTypes.func.isRequired
+  inputEnabledFunction: PropTypes.func.isRequired,
+  initialUncheckedValue: PropTypes.string,
+  meta: PropTypes.object
 };
 
-TextInputWithCheckbox.normalizeStringToInt = str => (str && parseInt(str.replace(/\D/g, ''), 10)) || 0;
-
 export default TextInputWithCheckbox;