fix: dimension dialog update validation (#486)

* fix: new pattern for centralising the validation logic for update button and add to layout button
dhis2 · Nov 25, 2019 · 429c51e · 429c51e
1 parent 888bf95
commit 429c51e
Show file tree

Hide file tree

Showing 7 changed files with 130 additions and 126 deletions.
diff --git a/packages/app/src/components/DimensionsPanel/Dialogs/AddToLayoutButton/AddToLayoutButton.js b/packages/app/src/components/DimensionsPanel/Dialogs/AddToLayoutButton/AddToLayoutButton.js
@@ -6,18 +6,9 @@ import MenuItem from '@material-ui/core/Button';
 import { withStyles } from '@material-ui/core/styles';
 import { getAvailableAxes } from '@dhis2/analytics';
 
-import UpdateButton from '../../../UpdateButton/UpdateButton';
 import Menu from './Menu';
-import {
-    sGetUiActiveModalDialog,
-    sGetDimensionIdsFromLayout,
-    sGetUiType,
-} from '../../../../reducers/ui';
-import {
-    acSetUiActiveModalDialog,
-    acAddUiLayoutDimensions,
-} from '../../../../actions/ui';
-import { tSetCurrentFromUi } from '../../../../actions/current';
+import { sGetUiActiveModalDialog, sGetUiType } from '../../../../reducers/ui';
+import { acAddUiLayoutDimensions } from '../../../../actions/ui';
 
 import { ADD_TO_LAYOUT_OPTIONS } from '../../../../modules/layout';
 import styles from './styles/AddToLayoutButton.style';
@@ -42,9 +33,7 @@ export class AddToLayoutButton extends Component {
             [this.props.dialogId]: axisId,
         });
 
-        this.props.onUpdate();
-
-        this.props.closeDialog(null);
+        this.props.onClick();
     };
 
     getAxisMeta = axisIdArray =>
@@ -68,7 +57,7 @@ export class AddToLayoutButton extends Component {
                 </MenuItem>
             ));
 
-    renderAddToLayoutButton = () => {
+    render() {
         const availableAxisMeta = this.getAxisMeta(
             getAvailableAxes(this.props.visType)
         );
@@ -96,48 +85,25 @@ export class AddToLayoutButton extends Component {
                 ) : null}
             </div>
         );
-    };
-
-    renderUpdateButton = () => (
-        <UpdateButton
-            className={this.props.className}
-            onClick={() => this.props.closeDialog(null)}
-        />
-    );
-
-    layoutHasDimension = dimensionId =>
-        this.props.dimensionIdsInLayout.includes(dimensionId);
-
-    render() {
-        const displayButton = this.layoutHasDimension(this.props.dialogId)
-            ? this.renderUpdateButton()
-            : this.renderAddToLayoutButton();
-
-        return displayButton;
     }
 }
 
 AddToLayoutButton.propTypes = {
     classes: PropTypes.object.isRequired,
     visType: PropTypes.string.isRequired,
     dialogId: PropTypes.string.isRequired,
-    dimensionIdsInLayout: PropTypes.array.isRequired,
     onAddDimension: PropTypes.func.isRequired,
-    onUpdate: PropTypes.func.isRequired,
-    closeDialog: PropTypes.func.isRequired,
+    onClick: PropTypes.func.isRequired,
 };
 
 const mapStateToProps = state => ({
     visType: sGetUiType(state),
     dialogId: sGetUiActiveModalDialog(state),
-    dimensionIdsInLayout: sGetDimensionIdsFromLayout(state),
 });
 
 export default connect(
     mapStateToProps,
     {
         onAddDimension: acAddUiLayoutDimensions,
-        onUpdate: tSetCurrentFromUi,
-        closeDialog: acSetUiActiveModalDialog,
     }
 )(withStyles(styles)(AddToLayoutButton));
diff --git a/packages/app/src/components/DimensionsPanel/Dialogs/DialogManager.js b/packages/app/src/components/DimensionsPanel/Dialogs/DialogManager.js
@@ -23,6 +23,7 @@ import {
 
 import HideButton from './HideButton';
 import AddToLayoutButton from './AddToLayoutButton/AddToLayoutButton';
+import UpdateVisualizationContainer from '../../UpdateButton/UpdateVisualizationContainer';
 
 import {
     acSetUiActiveModalDialog,
@@ -41,12 +42,14 @@ import {
     sGetUiParentGraphMap,
     sGetUiType,
     getAxisIdByDimensionId,
+    sGetDimensionIdsFromLayout,
 } from '../../../reducers/ui';
 import { sGetDimensions } from '../../../reducers/dimensions';
 import { sGetMetadata } from '../../../reducers/metadata';
 import { sGetSettingsDisplayNameProperty } from '../../../reducers/settings';
 import { apiFetchRecommendedIds } from '../../../api/dimensions';
 import { removeLastPathSegment, getOuPath } from '../../../modules/orgUnit';
+import UpdateButton from '../../UpdateButton/UpdateButton';
 
 export class DialogManager extends Component {
     state = {
@@ -296,6 +299,29 @@ export class DialogManager extends Component {
         );
     };
 
+    renderPrimaryButton = dialogId => (
+        <UpdateVisualizationContainer
+            renderComponent={handler =>
+                this.props.dimensionIdsInLayout.includes(dialogId) ? (
+                    <UpdateButton
+                        flat
+                        size="small"
+                        onClick={this.getPrimaryOnClick(handler)}
+                    />
+                ) : (
+                    <AddToLayoutButton
+                        onClick={this.getPrimaryOnClick(handler)}
+                    />
+                )
+            }
+        />
+    );
+
+    getPrimaryOnClick = handler => () => {
+        handler();
+        this.props.closeDialog(null);
+    };
+
     render() {
         const { dialogId, dimensions } = this.props;
         const keepMounted = !dialogId || dialogId === DIMENSION_ID_ORGUNIT;
@@ -312,7 +338,7 @@ export class DialogManager extends Component {
                 {this.renderDialogContent()}
                 <DialogActions>
                     <HideButton />
-                    {dialogId && <AddToLayoutButton dialogId={dialogId} />}
+                    {dialogId && this.renderPrimaryButton(dialogId)}
                 </DialogActions>
             </Dialog>
         );
@@ -333,6 +359,7 @@ DialogManager.propTypes = {
     metadata: PropTypes.object,
     selectedItems: PropTypes.object,
     type: PropTypes.string,
+    dimensionIdsInLayout: PropTypes.array.isRequired,
 };
 
 DialogManager.defaultProps = {
@@ -352,6 +379,7 @@ const mapStateToProps = state => ({
     type: sGetUiType(state),
     getAxisIdByDimensionId: dimensionId =>
         getAxisIdByDimensionId(state, dimensionId),
+    dimensionIdsInLayout: sGetDimensionIdsFromLayout(state),
 });
 
 export default connect(

diff --git a/packages/app/src/components/DimensionsPanel/Dialogs/__tests__/DialogManager.spec.js b/packages/app/src/components/DimensionsPanel/Dialogs/__tests__/DialogManager.spec.js
@@ -77,6 +77,7 @@ describe('The DialogManager component', () => {
             closeDialog: jest.fn(),
             setRecommendedIds: jest.fn(),
             getAxisIdByDimensionId: () => {},
+            dimensionIdsInLayout: [],
         };
         shallowDialog = undefined;
     });

diff --git a/...src/components/DimensionsPanel/Dialogs/__tests__/__snapshots__/DialogManager.spec.js.snap b/...src/components/DimensionsPanel/Dialogs/__tests__/__snapshots__/DialogManager.spec.js.snap
@@ -24,8 +24,8 @@ exports[`The DialogManager component renders OUDimension content with display:no
   </div>
   <Component>
     <Connect(HideButton) />
-    <Connect(WithStyles(AddToLayoutButton))
-      dialogId="ou"
+    <Connect(UpdateVisualizationContainer)
+      renderComponent={[Function]}
     />
   </Component>
 </Component>
@@ -87,8 +87,8 @@ exports[`The DialogManager component renders OUDimension content with display:no
   />
   <Component>
     <Connect(HideButton) />
-    <Connect(WithStyles(AddToLayoutButton))
-      dialogId="dx"
+    <Connect(UpdateVisualizationContainer)
+      renderComponent={[Function]}
     />
   </Component>
 </Component>
@@ -124,8 +124,8 @@ exports[`The DialogManager component renders the DataDimension content in dialog
   />
   <Component>
     <Connect(HideButton) />
-    <Connect(WithStyles(AddToLayoutButton))
-      dialogId="dx"
+    <Connect(UpdateVisualizationContainer)
+      renderComponent={[Function]}
     />
   </Component>
 </Component>
@@ -155,8 +155,8 @@ exports[`The DialogManager component renders the OrgUnitDimension content in dia
   </div>
   <Component>
     <Connect(HideButton) />
-    <Connect(WithStyles(AddToLayoutButton))
-      dialogId="ou"
+    <Connect(UpdateVisualizationContainer)
+      renderComponent={[Function]}
     />
   </Component>
 </Component>
@@ -177,8 +177,8 @@ exports[`The DialogManager component renders the PeriodDimension content in dial
   />
   <Component>
     <Connect(HideButton) />
-    <Connect(WithStyles(AddToLayoutButton))
-      dialogId="pe"
+    <Connect(UpdateVisualizationContainer)
+      renderComponent={[Function]}
     />
   </Component>
 </Component>

diff --git a/packages/app/src/components/MenuBar/MenuBar.js b/packages/app/src/components/MenuBar/MenuBar.js
@@ -8,6 +8,7 @@ import UpdateButton from '../UpdateButton/UpdateButton';
 import DownloadMenu from '../DownloadMenu/DownloadMenu';
 import InterpretationsButton from '../Interpretations/InterpretationsButton';
 import VisualizationOptionsManager from '../VisualizationOptions/VisualizationOptionsManager';
+import UpdateVisualizationContainer from '../UpdateButton/UpdateVisualizationContainer';
 import * as fromActions from '../../actions';
 import { sGetCurrent } from '../../reducers/current';
 import history from '../../modules/history';
@@ -33,7 +34,16 @@ const getOnError = props => error => props.onError(error);
 
 export const MenuBar = ({ classes, ...props }, context) => (
     <div className={classes.menuBar}>
-        <UpdateButton flat size="small" className={classes.updateButton} />
+        <UpdateVisualizationContainer
+            renderComponent={handler => (
+                <UpdateButton
+                    flat
+                    size="small"
+                    className={classes.updateButton}
+                    onClick={handler}
+                />
+            )}
+        />
         <FileMenu
             d2={context.d2}
             fileId={props.id || null}

diff --git a/packages/app/src/components/UpdateButton/UpdateButton.js b/packages/app/src/components/UpdateButton/UpdateButton.js
@@ -1,64 +1,12 @@
 import React from 'react';
 import PropTypes from 'prop-types';
-import { connect } from 'react-redux';
-import { withStyles } from '@material-ui/core/styles';
 import Button from '@material-ui/core/Button';
+import { withStyles } from '@material-ui/core/styles';
 import i18n from '@dhis2/d2-i18n';
 
-import { sGetUi } from '../../reducers/ui';
-import { sGetCurrent, sGetCurrentFromUi } from '../../reducers/current';
-import * as fromActions from '../../actions';
-import history from '../../modules/history';
 import styles from './styles/UpdateButton.style';
-import { CURRENT_AO_KEY } from '../../api/userDataStore';
-
-import { acSetLoadError, acClearLoadError } from '../../actions/loader';
-import { validateLayout } from '../../modules/layoutValidation';
-
-const UpdateButton = ({
-    classes,
-    acClearLoadError,
-    acSetLoadError,
-    onUpdate,
-    ui,
-    current,
-    currentFromUi,
-    onClick,
-    flat,
-    ...props
-}) => {
-    const wrappedOnClick = () => {
-        // validate layout on update
-        // validation error message will be shown without loading the plugin first
-        try {
-            validateLayout(currentFromUi);
-
-            acClearLoadError();
-        } catch (err) {
-            acSetLoadError(
-                err && err.message
-                    ? err.message
-                    : i18n.t('Error validating layout')
-            );
-        }
-        onUpdate(ui);
-
-        const urlContainsCurrentAOKey =
-            history.location.pathname === '/' + CURRENT_AO_KEY;
-
-        const pathWithoutInterpretation =
-            current && current.id ? `/${current.id}` : '/';
-
-        if (
-            !urlContainsCurrentAOKey &&
-            history.location.pathname !== pathWithoutInterpretation
-        ) {
-            history.push(pathWithoutInterpretation);
-        }
-
-        onClick();
-    };
 
+const UpdateButton = ({ classes, flat, onClick, ...props }) => {
     return (
         <Button
             data-test="update-button"
@@ -68,7 +16,7 @@ const UpdateButton = ({
             }
             variant="contained"
             color="primary"
-            onClick={wrappedOnClick}
+            onClick={onClick}
             disableRipple={true}
             disableFocusRipple={true}
         >
@@ -77,32 +25,15 @@ const UpdateButton = ({
     );
 };
 
-const mapStateToProps = state => ({
-    ui: sGetUi(state),
-    current: sGetCurrent(state),
-    currentFromUi: sGetCurrentFromUi(state),
-});
-
-const mapDispatchToProps = {
-    onUpdate: fromActions.fromCurrent.acSetCurrentFromUi,
-    acSetLoadError,
-    acClearLoadError,
-};
-
 UpdateButton.propTypes = {
     classes: PropTypes.object.isRequired,
-    ui: PropTypes.object.isRequired,
-    onUpdate: PropTypes.func.isRequired,
-    onClick: PropTypes.func,
     flat: PropTypes.bool,
+    onClick: PropTypes.func,
 };
 
 UpdateButton.defaultProps = {
-    onClick: Function.prototype,
     flat: false,
+    onClick: Function.prototype,
 };
 
-export default connect(
-    mapStateToProps,
-    mapDispatchToProps
-)(withStyles(styles)(UpdateButton));
+export default withStyles(styles)(UpdateButton);