From a2d20cf539605559b72f0cc520ff14a72c20521e Mon Sep 17 00:00:00 2001 From: Milan Zazrivec Date: Tue, 19 Mar 2019 16:42:37 +0100 Subject: [PATCH 1/3] Add validation to migration throttling inputs --- .../react/screens/App/Settings/helpers.js | 8 +-- .../GeneralSettings/GeneralSettings.js | 40 +++++++++++---- .../App/common/forms/TextInputWithCheckbox.js | 49 +++++++++---------- 3 files changed, 57 insertions(+), 40 deletions(-) diff --git a/app/javascript/react/screens/App/Settings/helpers.js b/app/javascript/react/screens/App/Settings/helpers.js index 88d0c26c4b..81a22d2b79 100644 --- a/app/javascript/react/screens/App/Settings/helpers.js +++ b/app/javascript/react/screens/App/Settings/helpers.js @@ -3,22 +3,18 @@ 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 + max_concurrent_tasks_per_ems: payload.transformation.limits.max_concurrent_tasks_per_ems, cpu_limit_per_host: payload.transformation.limits.cpu_limit_per_host, network_limit_per_host: payload.transformation.limits.network_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 + max_concurrent_tasks_per_ems: values.max_concurrent_tasks_per_ems, cpu_limit_per_host: values.cpu_limit_per_host, network_limit_per_host: values.network_limit_per_host - */ } } }); diff --git a/app/javascript/react/screens/App/Settings/screens/GeneralSettings/GeneralSettings.js b/app/javascript/react/screens/App/Settings/screens/GeneralSettings/GeneralSettings.js index 620713eb36..bdfbe4f063 100644 --- a/app/javascript/react/screens/App/Settings/screens/GeneralSettings/GeneralSettings.js +++ b/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 ( @@ -96,7 +112,6 @@ export class GeneralSettings extends React.Component { /> - {/* FIXME: uncomment once backend is ready

{__('Resource Utilization Limits for Migrations')}

@@ -105,23 +120,28 @@ 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" /> -*/} - {isSavingSettings && ( @@ -150,7 +170,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 = { diff --git a/app/javascript/react/screens/App/common/forms/TextInputWithCheckbox.js b/app/javascript/react/screens/App/common/forms/TextInputWithCheckbox.js index e5c492d0f3..76908943ec 100644 --- a/app/javascript/react/screens/App/common/forms/TextInputWithCheckbox.js +++ b/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 ( - - + +
+ +
+

+ Resource Utilization Limits for Migrations +

+
+ + Date: Thu, 18 Apr 2019 11:46:06 +0200 Subject: [PATCH 3/3] Temporarily remove network_limit_per_host This commit can be reverted later, once the backend support is in. --- .../__snapshots__/SettingsActions.test.js.snap | 2 -- .../__snapshots__/SettingsReducer.test.js.snap | 2 -- app/javascript/react/screens/App/Settings/helpers.js | 6 ++---- .../screens/GeneralSettings/GeneralSettings.js | 10 ---------- .../__snapshots__/GeneralSettings.test.js.snap | 10 ---------- .../react/screens/App/Settings/settings.fixures.js | 3 +-- 6 files changed, 3 insertions(+), 30 deletions(-) diff --git a/app/javascript/react/screens/App/Settings/__tests__/__snapshots__/SettingsActions.test.js.snap b/app/javascript/react/screens/App/Settings/__tests__/__snapshots__/SettingsActions.test.js.snap index 4546ce5c6d..c2503ab821 100644 --- a/app/javascript/react/screens/App/Settings/__tests__/__snapshots__/SettingsActions.test.js.snap +++ b/app/javascript/react/screens/App/Settings/__tests__/__snapshots__/SettingsActions.test.js.snap @@ -80,7 +80,6 @@ Array [ "cpu_limit_per_host": 10, "max_concurrent_tasks_per_ems": 10, "max_concurrent_tasks_per_host": 10, - "network_limit_per_host": 10, }, }, }, @@ -119,7 +118,6 @@ Array [ "cpu_limit_per_host": 10, "max_concurrent_tasks_per_ems": 10, "max_concurrent_tasks_per_host": 10, - "network_limit_per_host": 10, }, }, }, diff --git a/app/javascript/react/screens/App/Settings/__tests__/__snapshots__/SettingsReducer.test.js.snap b/app/javascript/react/screens/App/Settings/__tests__/__snapshots__/SettingsReducer.test.js.snap index cc4e0e5d31..2032d9a242 100644 --- a/app/javascript/react/screens/App/Settings/__tests__/__snapshots__/SettingsReducer.test.js.snap +++ b/app/javascript/react/screens/App/Settings/__tests__/__snapshots__/SettingsReducer.test.js.snap @@ -314,7 +314,6 @@ Object { "cpu_limit_per_host": 10, "max_concurrent_tasks_per_ems": 10, "max_concurrent_tasks_per_host": 10, - "network_limit_per_host": 10, }, "savingSettingsRejected": false, "servers": Array [], @@ -436,7 +435,6 @@ Object { "cpu_limit_per_host": 10, "max_concurrent_tasks_per_ems": 10, "max_concurrent_tasks_per_host": 10, - "network_limit_per_host": 10, }, "savingSettingsRejected": false, "servers": Array [], diff --git a/app/javascript/react/screens/App/Settings/helpers.js b/app/javascript/react/screens/App/Settings/helpers.js index 81a22d2b79..f91a161c91 100644 --- a/app/javascript/react/screens/App/Settings/helpers.js +++ b/app/javascript/react/screens/App/Settings/helpers.js @@ -4,8 +4,7 @@ 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, - cpu_limit_per_host: payload.transformation.limits.cpu_limit_per_host, - network_limit_per_host: payload.transformation.limits.network_limit_per_host + cpu_limit_per_host: payload.transformation.limits.cpu_limit_per_host }); export const getApiSettingsFromFormValues = values => ({ @@ -13,8 +12,7 @@ export const getApiSettingsFromFormValues = values => ({ limits: { max_concurrent_tasks_per_host: values.max_concurrent_tasks_per_host, max_concurrent_tasks_per_ems: values.max_concurrent_tasks_per_ems, - cpu_limit_per_host: values.cpu_limit_per_host, - network_limit_per_host: values.network_limit_per_host + cpu_limit_per_host: values.cpu_limit_per_host } } }); diff --git a/app/javascript/react/screens/App/Settings/screens/GeneralSettings/GeneralSettings.js b/app/javascript/react/screens/App/Settings/screens/GeneralSettings/GeneralSettings.js index bdfbe4f063..9d7c28103e 100644 --- a/app/javascript/react/screens/App/Settings/screens/GeneralSettings/GeneralSettings.js +++ b/app/javascript/react/screens/App/Settings/screens/GeneralSettings/GeneralSettings.js @@ -126,16 +126,6 @@ export class GeneralSettings extends React.Component { inputEnabledFunction={inputEnabledFunction} initialUncheckedValue="unlimited" /> -