Convert Role detail dialog to redux-form

* As parameters are configured in multiple places this change makes ParametersForm reusable by moving functions for gathering initial values and preparing values for submitting into ParametersForm. To achieve that, ParametersForm has to be connected to Redux * ParametersForm can now be reused and the set of parameters it operates on is defined by passing fields as children prop * ParametersForm can eventually be further parameterized as it currently only works for ModalPanels * RoleDetail dialog has been converted to use ParametersForm and therefore it got converted to redux-form * RoleDetail dialog now has Save and close submit button * RoleDetail dialog now persists data when switching between individual dialog tabs Closes-Bug: 1745408 Implements: blueprint ui-redux-form-migration Change-Id: I302c24b4cb4e94230c213305a536fa9e6ad09296
2018-02-13 09:54:51 +01:00 · 2018-02-13 09:54:51 +01:00 · d87861e059
parent 4b44e0cca9
commit d87861e059
7 changed files with 195 additions and 286 deletions
--- a/releasenotes/notes/role_detail_redux_form-f0724ec49562639e.yaml
+++ b/releasenotes/notes/role_detail_redux_form-f0724ec49562639e.yaml
@ -0,0 +1,13 @@
+---
+features:
+  - |
+    Improved perfomance in 'Role detail' dialog, a general error is shown when
+    sub-section contains validation error
+  - |
+    Save and Close button has been added to 'Role detail' dialog
+  - |
+    'Role detail' dialog now persists changes when switching between individual
+    tabs
+fixes:
+  - Fixes `bug 1745408 <https://bugs.launchpad.net/tripleo/+bug/1745408>`__
+    Parameters form does not load for large amount of parameters
--- a/src/js/components/parameters/Parameters.js
+++ b/src/js/components/parameters/Parameters.js
@ -16,7 +16,6 @@

 import { connect } from 'react-redux';
 import ImmutablePropTypes from 'react-immutable-proptypes';
-import { pickBy, isEqual, mapValues } from 'lodash';
 import PropTypes from 'prop-types';
 import React from 'react';

@ -81,42 +80,8 @@ class Parameters extends React.Component {
    }
  }

-  /**
-   * Filter out non updated parameters, so only parameters which have been actually changed
-   * get sent to updateparameters
-   */
-  _filterFormData(values, initialValues) {
-    return pickBy(values, (value, key) => !isEqual(value, initialValues[key]));
-  }
-
-  /**
-   * Json parameter values are sent as string, this function parses them and checks if they're object
-   * or array. Also, parameters with undefined value are set to null
-   */
-  _parseJsonTypeValues(values, parameters) {
-    return mapValues(values, (value, key) => {
-      if (parameters.get(key).type.toLowerCase() === 'json') {
-        try {
-          return JSON.parse(value);
-        } catch (e) {
-          return value === undefined ? null : value;
-        }
-      }
-      return value === undefined ? null : value;
-    });
-  }
-
-  handleSubmit = ({ saveAndClose, ...values }, dispatch, { initialValues }) => {
-    const {
-      currentPlanName,
-      history,
-      parameters,
-      updateParameters
-    } = this.props;
-    const updatedValues = this._parseJsonTypeValues(
-      this._filterFormData(values, initialValues),
-      parameters
-    );
+  handleUpdateParameters = (updatedValues, saveAndClose) => {
+    const { currentPlanName, history, updateParameters } = this.props;
    updateParameters(
      currentPlanName,
      updatedValues,
@ -124,31 +89,8 @@ class Parameters extends React.Component {
    );
  };

-  _convertJsonTypeParameterValueToString(value) {
-    // Heat defaults empty values to empty string also some JSON type parameters
-    // accept empty string as valid value
-    return ['', undefined].includes(value) ? '' : JSON.stringify(value);
-  }
-
-  getFormInitialValues() {
-    return this.props.parameters
-      .map(p => {
-        const value = p.value === undefined ? p.default : p.value;
-        if (p.type.toLowerCase() === 'json') {
-          return this._convertJsonTypeParameterValueToString(value);
-        } else {
-          return value;
-        }
-      })
-      .toJS();
-  }
-
  render() {
-    const {
-      enabledEnvironments,
-      parameters,
-      isFetchingParameters
-    } = this.props;
+    const { enabledEnvironments, isFetchingParameters } = this.props;
    return (
      <Loader
        height={120}
@ -156,19 +98,16 @@ class Parameters extends React.Component {
        loaded={!isFetchingParameters}
        componentProps={{ className: 'flex-container' }}
      >
-        <ParametersForm
-          onSubmit={this.handleSubmit}
-          parameters={parameters}
-          initialValues={this.getFormInitialValues()}
-          className="flex-container"
-        >
-          <ParametersSidebar
-            activateTab={tabName => this.setState({ activeTab: tabName })}
-            enabledEnvironments={enabledEnvironments.toList()}
-            isTabActive={this.isTabActive}
-          />
-          <div className="col-sm-8 flex-column">
-            <div className="tab-content">{this.renderTabPanes()}</div>
+        <ParametersForm updateParameters={this.handleUpdateParameters}>
+          <div className="flex-row">
+            <ParametersSidebar
+              activateTab={tabName => this.setState({ activeTab: tabName })}
+              enabledEnvironments={enabledEnvironments.toList()}
+              isTabActive={this.isTabActive}
+            />
+            <div className="col-sm-8 flex-column">
+              <div className="tab-content">{this.renderTabPanes()}</div>
+            </div>
          </div>
        </ParametersForm>
      </Loader>
@ -180,8 +119,6 @@ Parameters.propTypes = {
  enabledEnvironments: ImmutablePropTypes.map.isRequired,
  fetchEnvironmentConfiguration: PropTypes.func.isRequired,
  fetchParameters: PropTypes.func.isRequired,
-  formErrors: ImmutablePropTypes.list,
-  formFieldErrors: ImmutablePropTypes.map,
  history: PropTypes.object,
  intl: PropTypes.object,
  isFetchingParameters: PropTypes.bool.isRequired,
@ -196,8 +133,6 @@ function mapStateToProps(state, ownProps) {
    parameters: state.parameters.parameters,
    enabledEnvironments: getEnabledEnvironments(state),
    form: state.parameters.form,
-    formErrors: state.parameters.form.get('formErrors'),
-    formFieldErrors: state.parameters.form.get('formFieldErrors'),
    currentPlanName: getCurrentPlanName(state),
    isFetchingParameters: state.parameters.isFetching,
    mistralParameters: state.parameters.mistralParameters,
--- a/src/js/components/parameters/ParametersForm.js
+++ b/src/js/components/parameters/ParametersForm.js
@ -15,9 +15,10 @@
 */

 import { Button, Form, ModalFooter } from 'react-bootstrap';
+import { connect } from 'react-redux';
 import { defineMessages, FormattedMessage, injectIntl } from 'react-intl';
 import ImmutablePropTypes from 'react-immutable-proptypes';
-import { isEqual, pickBy } from 'lodash';
+import { isEqual, pickBy, mapValues } from 'lodash';
 import PropTypes from 'prop-types';
 import React from 'react';
 import { reduxForm } from 'redux-form';
@ -59,51 +60,51 @@ const messages = defineMessages({
  }
 });

-const ParametersForm = ({
-  dispatch,
-  error,
-  children,
-  onSubmit,
-  handleSubmit,
-  intl: { formatMessage },
-  invalid,
-  pristine,
-  submitting,
-  initialValues
-}) => (
-  <Form className="flex-container" onSubmit={handleSubmit} horizontal>
-    <OverlayLoader
-      loaded={!submitting}
-      content={formatMessage(messages.updatingParameters)}
-      containerClassName="flex-container"
-    >
-      <ModalFormErrorList errors={error ? [error] : []} />
-      <div className="flex-row">{children}</div>
-    </OverlayLoader>
-    <ModalFooter>
-      <CloseModalButton>
-        <FormattedMessage {...messages.cancel} />
-      </CloseModalButton>
-      <Button
-        disabled={invalid || pristine || submitting}
-        onClick={handleSubmit(values =>
-          onSubmit({ ...values, saveAndClose: true }, dispatch, {
-            initialValues
-          })
-        )}
+const ParametersForm = props => {
+  const {
+    dispatch,
+    error,
+    children,
+    onSubmit,
+    handleSubmit,
+    intl: { formatMessage },
+    invalid,
+    pristine,
+    submitting
+  } = props;
+  return (
+    <Form className="flex-container" onSubmit={handleSubmit} horizontal>
+      <OverlayLoader
+        loaded={!submitting}
+        content={formatMessage(messages.updatingParameters)}
+        containerClassName="flex-container"
      >
-        <FormattedMessage {...messages.saveAndClose} />
-      </Button>
-      <Button
-        type="submit"
-        disabled={invalid || pristine || submitting}
-        bsStyle="primary"
-      >
-        <FormattedMessage {...messages.saveChanges} />
-      </Button>
-    </ModalFooter>
-  </Form>
-);
+        <ModalFormErrorList errors={error ? [error] : []} />
+        {children}
+      </OverlayLoader>
+      <ModalFooter>
+        <CloseModalButton>
+          <FormattedMessage {...messages.cancel} />
+        </CloseModalButton>
+        <Button
+          disabled={invalid || pristine || submitting}
+          onClick={handleSubmit(values =>
+            onSubmit({ ...values, saveAndClose: true }, dispatch, props)
+          )}
+        >
+          <FormattedMessage {...messages.saveAndClose} />
+        </Button>
+        <Button
+          type="submit"
+          disabled={invalid || pristine || submitting}
+          bsStyle="primary"
+        >
+          <FormattedMessage {...messages.saveChanges} />
+        </Button>
+      </ModalFooter>
+    </Form>
+  );
+};
 ParametersForm.propTypes = {
  children: PropTypes.node,
  dispatch: PropTypes.func.isRequired,
@ -115,7 +116,8 @@ ParametersForm.propTypes = {
  onSubmit: PropTypes.func.isRequired,
  parameters: ImmutablePropTypes.map.isRequired,
  pristine: PropTypes.bool.isRequired,
-  submitting: PropTypes.bool.isRequired
+  submitting: PropTypes.bool.isRequired,
+  updateParameters: PropTypes.func.isRequired
 };

 const isJSON = value => {
@ -159,9 +161,74 @@ const validate = (
  return errors;
 };

+const convertJsonTypeParameterValueToString = value =>
+  // Heat defaults empty values to empty string also some JSON type parameters
+  // accept empty string as valid value
+  ['', undefined].includes(value) ? '' : JSON.stringify(value);
+
+const getFormInitialValues = parameters => {
+  return parameters
+    .map(p => {
+      const value = p.value === undefined ? p.default : p.value;
+      if (p.type.toLowerCase() === 'json') {
+        return convertJsonTypeParameterValueToString(value);
+      } else {
+        return value;
+      }
+    })
+    .toJS();
+};
+
+/**
+ * Filter out non updated parameters, so only parameters which have been actually changed
+ * get sent to updateparameters
+ */
+const filterFormData = (values, initialValues) => {
+  return pickBy(values, (value, key) => !isEqual(value, initialValues[key]));
+};
+
+/**
+ * Json parameter values are sent as string, this function parses them and checks if they're object
+ * or array. Also, parameters with undefined value are set to null
+ */
+const parseJsonTypeValues = (values, parameters) => {
+  return mapValues(values, (value, key) => {
+    if (parameters.get(key).type.toLowerCase() === 'json') {
+      try {
+        return JSON.parse(value);
+      } catch (e) {
+        return value === undefined ? null : value;
+      }
+    }
+    return value === undefined ? null : value;
+  });
+};
+
+const handleSubmit = (
+  { saveAndClose, ...values },
+  dispatch,
+  { initialValues, parameters, updateParameters }
+) => {
+  const updatedValues = parseJsonTypeValues(
+    filterFormData(values, initialValues),
+    parameters
+  );
+  updateParameters(updatedValues, saveAndClose);
+};
+
+const mapStateToProps = state => {
+  const { parameters } = state.parameters;
+  return {
+    initialValues: getFormInitialValues(parameters),
+    parameters: parameters
+  };
+};
+
 const form = reduxForm({
  form: 'parametersForm',
+  enableReinitialize: true,
+  onSubmit: handleSubmit,
  validate
 });

-export default injectIntl(form(ParametersForm));
+export default injectIntl(connect(mapStateToProps, null)(form(ParametersForm)));
--- a/src/js/components/roles/RoleDetail.js
+++ b/src/js/components/roles/RoleDetail.js
@ -16,28 +16,20 @@

 import { connect } from 'react-redux';
 import { defineMessages, FormattedMessage, injectIntl } from 'react-intl';
-import Formsy from 'formsy-react';
-import { fromJS, is } from 'immutable';
 import ImmutablePropTypes from 'react-immutable-proptypes';
-import { isObjectLike, mapValues } from 'lodash';
 import { Redirect, Route, Switch } from 'react-router-dom';
 import PropTypes from 'prop-types';
 import React from 'react';
-import { ModalHeader, ModalTitle, ModalFooter } from 'react-bootstrap';
+import { ModalHeader, ModalTitle } from 'react-bootstrap';

 import { checkRunningDeployment } from '../utils/checkRunningDeploymentHOC';
 import { getCurrentPlanName } from '../../selectors/plans';
 import { getRole } from '../../selectors/roles';
-import { getRoleServices } from '../../selectors/parameters';
-import { Loader, OverlayLoader } from '../ui/Loader';
-import ModalFormErrorList from '../ui/forms/ModalFormErrorList';
-import {
-  CloseModalButton,
-  CloseModalXButton,
-  RoutedModalPanel
-} from '../ui/Modals';
+import { Loader } from '../ui/Loader';
+import { CloseModalXButton, RoutedModalPanel } from '../ui/Modals';
 import NavTab from '../ui/NavTab';
 import ParametersActions from '../../actions/ParametersActions';
+import ParametersForm from '../parameters/ParametersForm';
 import RoleNetworkConfig from './RoleNetworkConfig';
 import RoleParameters from './RoleParameters';
 import RoleServices from './RoleServices';
@ -78,74 +70,19 @@ const messages = defineMessages({
 });

 class RoleDetail extends React.Component {
-  constructor() {
-    super();
-    this.state = {
-      canSubmit: false,
-      show: true
-    };
-  }
-
  componentDidMount() {
    const { currentPlanName } = this.props;
    this.props.fetchParameters(currentPlanName);
  }

-  componentDidUpdate() {
-    this.invalidateForm(this.props.formFieldErrors.toJS());
-  }
-
-  enableButton() {
-    this.setState({ canSubmit: true });
-  }
-
-  disableButton() {
-    this.setState({ canSubmit: false });
-  }
-
-  /**
-   * Filter out non updated parameters, so only parameters which have been actually changed
-   * get sent to updateparameters
-   */
-  _filterFormData(formData) {
-    return fromJS(formData)
-      .filterNot((value, key) => {
-        return is(fromJS(this.props.allParameters.get(key).default), value);
-      })
-      .toJS();
-  }
-
-  /**
-   * Json parameter values are sent as string, this function parses them and checks if they're object
-   * or array. Also, parameters with undefined value are set to null
-   */
-  _jsonParseFormData(formData) {
-    return mapValues(formData, value => {
-      try {
-        const parsedValue = JSON.parse(value);
-        if (isObjectLike(parsedValue)) {
-          return parsedValue;
-        }
-        return value;
-      } catch (e) {
-        return value === undefined ? null : value;
-      }
-    });
-  }
-
-  invalidateForm(formFieldErrors) {
-    this.refs.roleParametersForm.updateInputsWithError(formFieldErrors);
-  }
-
-  handleSubmit(formData, resetForm, invalidateForm) {
-    this.disableButton();
-
-    this.props.updateParameters(
-      this.props.currentPlanName,
-      this._filterFormData(this._jsonParseFormData(formData)),
-      Object.keys(this.refs.roleParametersForm.inputs)
+  handleUpdateParameters = (updatedValues, saveAndClose) => {
+    const { currentPlanName, history, updateParameters } = this.props;
+    updateParameters(
+      currentPlanName,
+      updatedValues,
+      saveAndClose && history.push.bind(this, `/plans/${currentPlanName}`)
    );
-  }
+  };

  renderRoleTabs() {
    const {
@ -186,10 +123,8 @@ class RoleDetail extends React.Component {
  render() {
    const {
      currentPlanName,
-      formErrors,
      intl: { formatMessage },
      isFetchingParameters,
-      isUpdatingParameters,
      location,
      match: { params: urlParams },
      role,
@ -199,87 +134,56 @@ class RoleDetail extends React.Component {
    const roleName = role ? role.name : null;
    return (
      <RoutedModalPanel redirectPath={`/plans/${currentPlanName}`}>
-        <Formsy
-          ref="roleParametersForm"
-          role="form"
-          className="form form-horizontal flex-container"
-          onSubmit={this.handleSubmit.bind(this)}
-          onValid={this.enableButton.bind(this)}
-          onInvalid={this.disableButton.bind(this)}
+        <ModalHeader>
+          <CloseModalXButton />
+          <ModalTitle>
+            <FormattedMessage
+              {...messages.role}
+              values={{ roleName: roleName }}
+            />
+          </ModalTitle>
+        </ModalHeader>
+        <Loader
+          height={60}
+          content={formatMessage(messages.loadingParameters)}
+          component="div"
+          componentProps={{ className: 'flex-container' }}
+          loaded={dataLoaded}
        >
-          <ModalHeader>
-            <CloseModalXButton />
-            <ModalTitle>
-              <FormattedMessage
-                {...messages.role}
-                values={{ roleName: roleName }}
+          <ParametersForm updateParameters={this.handleUpdateParameters}>
+            {this.renderRoleTabs()}
+            <Switch location={location}>
+              <Route
+                path="/plans/:planName/roles/:roleName/parameters"
+                component={RoleParameters}
              />
-            </ModalTitle>
-          </ModalHeader>
-          {this.renderRoleTabs()}
-          <ModalFormErrorList errors={formErrors.toJS()} />
-          <Loader
-            height={60}
-            content={formatMessage(messages.loadingParameters)}
-            component="div"
-            componentProps={{ className: 'flex-container' }}
-            loaded={dataLoaded}
-          >
-            <OverlayLoader
-              containerClassName="flex-container"
-              loaded={!isUpdatingParameters}
-              content={formatMessage(messages.updatingParameters)}
-            >
-              <Switch location={location}>
-                <Route
-                  path="/plans/:planName/roles/:roleName/parameters"
-                  component={RoleParameters}
-                />
-                <Route
-                  path="/plans/:planName/roles/:roleName/services"
-                  component={RoleServices}
-                />
-                <Route
-                  path="/plans/:planName/roles/:roleName/network-configuration"
-                  component={RoleNetworkConfig}
-                />
-                <Redirect
-                  from="/plans/:planName/roles/:roleName"
-                  to={`/plans/${currentPlanName}/roles/${
-                    urlParams.roleName
-                  }/parameters`}
-                />
-              </Switch>
-            </OverlayLoader>
-          </Loader>
-          {dataLoaded ? (
-            <ModalFooter>
-              <CloseModalButton>
-                <FormattedMessage {...messages.cancel} />
-              </CloseModalButton>
-              <button
-                type="submit"
-                disabled={!this.state.canSubmit}
-                className="btn btn-primary"
-              >
-                <FormattedMessage {...messages.saveChanges} />
-              </button>
-            </ModalFooter>
-          ) : null}
-        </Formsy>
+              <Route
+                path="/plans/:planName/roles/:roleName/services"
+                component={RoleServices}
+              />
+              <Route
+                path="/plans/:planName/roles/:roleName/network-configuration"
+                component={RoleNetworkConfig}
+              />
+              <Redirect
+                from="/plans/:planName/roles/:roleName"
+                to={`/plans/${currentPlanName}/roles/${
+                  urlParams.roleName
+                }/parameters`}
+              />
+            </Switch>
+          </ParametersForm>
+        </Loader>
      </RoutedModalPanel>
    );
  }
 }
 RoleDetail.propTypes = {
-  allParameters: ImmutablePropTypes.map.isRequired,
  currentPlanName: PropTypes.string,
  fetchParameters: PropTypes.func,
-  formErrors: ImmutablePropTypes.list,
-  formFieldErrors: ImmutablePropTypes.map,
+  history: PropTypes.object,
  intl: PropTypes.object,
  isFetchingParameters: PropTypes.bool.isRequired,
-  isUpdatingParameters: PropTypes.bool.isRequired,
  location: PropTypes.object.isRequired,
  match: PropTypes.object.isRequired,
  role: ImmutablePropTypes.record,
@ -290,13 +194,8 @@ RoleDetail.propTypes = {
 function mapStateToProps(state, props) {
  return {
    currentPlanName: getCurrentPlanName(state),
-    formErrors: state.parameters.form.get('formErrors'),
-    formFieldErrors: state.parameters.form.get('formFieldErrors'),
-    allParameters: state.parameters.parameters,
    isFetchingParameters: state.parameters.isFetching,
-    isUpdatingParameters: state.parameters.isUpdating,
    role: getRole(state, props.match.params.roleName),
-    roleServices: getRoleServices(state, props.match.params.roleName),
    rolesLoaded: state.roles.get('loaded')
  };
 }
@ -310,14 +209,9 @@ function mapDispatchToProps(dispatch, ownProps) {
        )
      );
    },
-    updateParameters: (currentPlanName, data, inputFields, redirectPath) => {
+    updateParameters: (currentPlanName, data, redirectPath) => {
      dispatch(
-        ParametersActions.updateRoleParameters(
-          currentPlanName,
-          data,
-          inputFields,
-          redirectPath
-        )
+        ParametersActions.updateParameters(currentPlanName, data, redirectPath)
      );
    }
  };
--- a/src/js/components/roles/RoleNetworkConfig.js
+++ b/src/js/components/roles/RoleNetworkConfig.js
@ -19,7 +19,7 @@ import ImmutablePropTypes from 'react-immutable-proptypes';
 import PropTypes from 'prop-types';
 import React from 'react';

-import ParameterInputList from '../parameters/ParameterInputList';
+import ParameterInputList from '../parameters/NewParameterInputList';
 import { getRoleNetworkConfig } from '../../selectors/parameters';
 import { getRole } from '../../selectors/roles';

--- a/src/js/components/roles/RoleParameters.js
+++ b/src/js/components/roles/RoleParameters.js
@ -19,7 +19,7 @@ import ImmutablePropTypes from 'react-immutable-proptypes';
 import PropTypes from 'prop-types';
 import React from 'react';

-import ParameterInputList from '../parameters/ParameterInputList';
+import ParameterInputList from '../parameters/NewParameterInputList';
 import { getRoleParameters } from '../../selectors/parameters';
 import { getRole } from '../../selectors/roles';

--- a/src/js/components/roles/RoleServices.js
+++ b/src/js/components/roles/RoleServices.js
@ -23,7 +23,7 @@ import React from 'react';

 import { getRoleServices } from '../../selectors/parameters';
 import { getRole } from '../../selectors/roles';
-import ParameterInputList from '../parameters/ParameterInputList';
+import ParameterInputList from '../parameters/NewParameterInputList';
 import Tab from '../ui/Tab';

 const messages = defineMessages({