From 36eec715efdbfd55eae392928cdad91c2acf8a29 Mon Sep 17 00:00:00 2001
From: Jen Jones Arnesen <jennifer@dhis2.org>
Date: Wed, 17 Mar 2021 15:14:05 +0100
Subject: [PATCH] fix: only request dimensions when needed (#1654)

This fix decreases the initial api requests being made when the dashboard first loads. There's no need to request dimensions unless they are needed for filtering the dashboard or setting filter settings while editing a dashboard.
---
 src/components/App.js                         | 28 ------------
 src/components/ControlBar/EditBar.js          |  4 --
 .../ControlBar/__tests__/EditBar.spec.js      |  8 ++++
 .../__snapshots__/EditBar.spec.js.snap        |  3 ++
 src/components/ItemFilter/FilterSelector.js   | 15 +++----
 .../ItemFilter/FilterSettingsDialog.js        |  4 +-
 .../__tests__/FilterSettingsDialog.spec.js    | 18 +++++---
 src/modules/useDimensions.js                  | 44 +++++++++++++++++++
 src/reducers/dimensions.js                    |  8 ++--
 9 files changed, 79 insertions(+), 53 deletions(-)
 create mode 100644 src/modules/useDimensions.js

diff --git a/src/components/App.js b/src/components/App.js
index 782af3d68..536ddcc86 100644
--- a/src/components/App.js
+++ b/src/components/App.js
@@ -4,19 +4,14 @@ import { connect } from 'react-redux'
 import PropTypes from 'prop-types'
 import { CssVariables } from '@dhis2/ui'
 import { useD2 } from '@dhis2/app-runtime-adapter-d2'
-import { apiFetchDimensions } from '@dhis2/analytics'
-import { useDataEngine } from '@dhis2/app-runtime'
 
 import Dashboard from './Dashboard/Dashboard'
 import AlertBar from './AlertBar/AlertBar'
-import { useUserSettings } from './UserSettingsProvider'
 
 import { acReceivedUser } from '../actions/user'
 import { tFetchDashboards } from '../actions/dashboards'
 import { tSetControlBarRows } from '../actions/controlBar'
 import { tSetShowDescription } from '../actions/selected'
-import { acSetDimensions } from '../actions/dimensions'
-import getFilteredDimensions from '../modules/getFilteredDimensions'
 
 import {
     EDIT,
@@ -30,8 +25,6 @@ import './App.css'
 
 const App = props => {
     const { d2 } = useD2()
-    const dataEngine = useDataEngine()
-    const { userSettings } = useUserSettings()
 
     useEffect(() => {
         props.setCurrentUser(d2.currentUser)
@@ -50,25 +43,6 @@ const App = props => {
         )
     }, [])
 
-    useEffect(() => {
-        const fetchDimensions = async () => {
-            try {
-                const dimensions = await apiFetchDimensions(
-                    dataEngine,
-                    userSettings.keyAnalysisDisplayProperty
-                )
-
-                props.setDimensions(getFilteredDimensions(dimensions))
-            } catch (e) {
-                console.error(e)
-            }
-        }
-
-        if (userSettings.keyAnalysisDisplayProperty) {
-            fetchDimensions()
-        }
-    }, [userSettings])
-
     return (
         <>
             <CssVariables colors spacers />
@@ -117,7 +91,6 @@ App.propTypes = {
     fetchDashboards: PropTypes.func.isRequired,
     setControlBarRows: PropTypes.func.isRequired,
     setCurrentUser: PropTypes.func.isRequired,
-    setDimensions: PropTypes.func.isRequired,
     setShowDescription: PropTypes.func.isRequired,
 }
 
@@ -125,7 +98,6 @@ const mapDispatchToProps = {
     fetchDashboards: tFetchDashboards,
     setControlBarRows: tSetControlBarRows,
     setCurrentUser: acReceivedUser,
-    setDimensions: acSetDimensions,
     setShowDescription: tSetShowDescription,
 }
 
diff --git a/src/components/ControlBar/EditBar.js b/src/components/ControlBar/EditBar.js
index a46782497..55fb81077 100644
--- a/src/components/ControlBar/EditBar.js
+++ b/src/components/ControlBar/EditBar.js
@@ -19,7 +19,6 @@ import {
 } from '../../actions/editDashboard'
 import { acClearPrintDashboard } from '../../actions/printDashboard'
 import { tDeleteDashboard } from '../../actions/dashboards'
-import { sGetDimensions } from '../../reducers/dimensions'
 import {
     sGetEditDashboardRoot,
     sGetIsNewDashboard,
@@ -152,7 +151,6 @@ const EditBar = props => {
     const filterSettingsDialog = () =>
         dashboard || props.newDashboard ? (
             <FilterSettingsDialog
-                dimensions={props.dimensions}
                 restrictFilters={props.restrictFilters}
                 initiallySelectedItems={props.allowedFilters}
                 onClose={toggleFilterSettingsDialog}
@@ -222,7 +220,6 @@ EditBar.propTypes = {
     dashboardId: PropTypes.string,
     dashboardName: PropTypes.string,
     deleteAccess: PropTypes.bool,
-    dimensions: PropTypes.array,
     isPrintPreviewView: PropTypes.bool,
     newDashboard: PropTypes.bool,
     restrictFilters: PropTypes.bool,
@@ -254,7 +251,6 @@ const mapStateToProps = state => {
         dashboardId: dashboard.id,
         dashboardName: dashboard.name,
         deleteAccess,
-        dimensions: sGetDimensions(state),
         newDashboard,
         restrictFilters: dashboard.restrictFilters,
         isPrintPreviewView: sGetIsPrintPreviewView(state),
diff --git a/src/components/ControlBar/__tests__/EditBar.spec.js b/src/components/ControlBar/__tests__/EditBar.spec.js
index 1a9a7e9af..69c1a01a7 100644
--- a/src/components/ControlBar/__tests__/EditBar.spec.js
+++ b/src/components/ControlBar/__tests__/EditBar.spec.js
@@ -18,6 +18,14 @@ jest.mock('@dhis2/app-runtime-adapter-d2')
 jest.mock('@dhis2/app-runtime')
 jest.mock('../../../api/fetchDashboard')
 
+jest.mock(
+    '../../ItemFilter/FilterSettingsDialog',
+    () =>
+        function MockFilterSettingsDialog() {
+            return <div className="mock-filter-settings-dialog" />
+        }
+)
+
 jest.mock(
     '@dhis2/d2-ui-translation-dialog',
     () =>
diff --git a/src/components/ControlBar/__tests__/__snapshots__/EditBar.spec.js.snap b/src/components/ControlBar/__tests__/__snapshots__/EditBar.spec.js.snap
index 222544ed3..962b3e7a0 100644
--- a/src/components/ControlBar/__tests__/__snapshots__/EditBar.spec.js.snap
+++ b/src/components/ControlBar/__tests__/__snapshots__/EditBar.spec.js.snap
@@ -62,6 +62,9 @@ exports[`renders Save and Discard buttons but no dialogs when no dashboard id 1`
       </button>
     </div>
   </div>
+  <div
+    class="mock-filter-settings-dialog"
+  />
 </div>
 `;
 
diff --git a/src/components/ItemFilter/FilterSelector.js b/src/components/ItemFilter/FilterSelector.js
index 0f44a7a02..55264cd41 100644
--- a/src/components/ItemFilter/FilterSelector.js
+++ b/src/components/ItemFilter/FilterSelector.js
@@ -3,25 +3,24 @@ import PropTypes from 'prop-types'
 import { connect } from 'react-redux'
 import ArrowDropDownIcon from '@material-ui/icons/ArrowDropDown'
 import isEmpty from 'lodash/isEmpty'
-
 import i18n from '@dhis2/d2-i18n'
 import { DimensionsPanel } from '@dhis2/analytics'
-
 import { Button, Popover } from '@dhis2/ui'
 import FilterDialog from './FilterDialog'
 
 import { sGetActiveModalDimension } from '../../reducers/activeModalDimension'
-import { sGetDimensions } from '../../reducers/dimensions'
 import { sGetItemFiltersRoot } from '../../reducers/itemFilters'
 import {
     acClearActiveModalDimension,
     acSetActiveModalDimension,
 } from '../../actions/activeModalDimension'
+import useDimensions from '../../modules/useDimensions'
 
 import classes from './styles/FilterSelector.module.css'
 
 const FilterSelector = props => {
     const [showPopover, setShowPopover] = useState(false)
+    const dimensions = useDimensions(showPopover)
 
     const ref = useRef(null)
 
@@ -33,21 +32,21 @@ const FilterSelector = props => {
 
     const selectDimension = id => {
         props.setActiveModalDimension(
-            props.dimensions.find(dimension => dimension.id === id)
+            dimensions.find(dimension => dimension.id === id)
         )
     }
 
     const filterDimensions = () => {
         if (!props.restrictFilters) {
-            return props.dimensions
+            return dimensions
         } else {
-            return props.dimensions.filter(d =>
+            return dimensions.filter(d =>
                 [...props.allowedFilters].includes(d.id)
             )
         }
     }
 
-    return isEmpty(filterDimensions()) ? null : (
+    return props.restrictFilters && !props.allowedFilters ? null : (
         <>
             <span className={classes.buttonContainer} ref={ref}>
                 <Button onClick={() => setShowPopover(true)}>
@@ -87,7 +86,6 @@ const FilterSelector = props => {
 
 const mapStateToProps = state => ({
     dimension: sGetActiveModalDimension(state),
-    dimensions: sGetDimensions(state),
     initiallySelectedItems: sGetItemFiltersRoot(state),
 })
 
@@ -95,7 +93,6 @@ FilterSelector.propTypes = {
     allowedFilters: PropTypes.array,
     clearActiveModalDimension: PropTypes.func,
     dimension: PropTypes.object,
-    dimensions: PropTypes.array,
     initiallySelectedItems: PropTypes.object,
     restrictFilters: PropTypes.bool,
     setActiveModalDimension: PropTypes.func,
diff --git a/src/components/ItemFilter/FilterSettingsDialog.js b/src/components/ItemFilter/FilterSettingsDialog.js
index 3fdd2463f..87afe5b49 100644
--- a/src/components/ItemFilter/FilterSettingsDialog.js
+++ b/src/components/ItemFilter/FilterSettingsDialog.js
@@ -12,6 +12,7 @@ import {
     Radio,
     Transfer,
 } from '@dhis2/ui'
+import useDimensions from '../../modules/useDimensions'
 
 import classes from './styles/FilterSettingsDialog.module.css'
 
@@ -57,7 +58,6 @@ RadioOptions.propTypes = {
 }
 
 const FilterSettingsDialog = ({
-    dimensions,
     restrictFilters,
     initiallySelectedItems,
     onClose,
@@ -66,6 +66,7 @@ const FilterSettingsDialog = ({
 }) => {
     const [selected, setSelected] = useState(initiallySelectedItems)
     const [filtersSelectable, setFiltersSelectable] = useState(restrictFilters)
+    const dimensions = useDimensions(open)
 
     const updateFilterDimensionRestrictability = val => {
         if (val) {
@@ -186,7 +187,6 @@ const FilterSettingsDialog = ({
 }
 
 FilterSettingsDialog.propTypes = {
-    dimensions: PropTypes.array,
     initiallySelectedItems: PropTypes.array,
     open: PropTypes.bool,
     restrictFilters: PropTypes.bool,
diff --git a/src/components/ItemFilter/__tests__/FilterSettingsDialog.spec.js b/src/components/ItemFilter/__tests__/FilterSettingsDialog.spec.js
index e988810bc..0e9e47d1d 100644
--- a/src/components/ItemFilter/__tests__/FilterSettingsDialog.spec.js
+++ b/src/components/ItemFilter/__tests__/FilterSettingsDialog.spec.js
@@ -3,17 +3,22 @@ import { shallow } from 'enzyme'
 import toJson from 'enzyme-to-json'
 import FilterSettingsDialog from '../FilterSettingsDialog'
 
-const mockDimensions = [
-    { id: 'kl', name: 'kvikk lunsj' },
-    { id: 'ss', name: 'salt sild' },
-    { id: 'sm', name: 'seigmenn' },
-]
+jest.mock('../../../modules/useDimensions', () => ({
+    __esModule: true,
+    default: () => {
+        return [
+            { id: 'kl', name: 'kvikk lunsj' },
+            { id: 'ss', name: 'salt sild' },
+            { id: 'sm', name: 'seigmenn' },
+        ]
+    },
+}))
+
 const mockSelectedItems = ['kl']
 
 it('renders correctly when filters are not restricted', () => {
     const container = shallow(
         <FilterSettingsDialog
-            dimensions={mockDimensions}
             restrictFilters={false}
             initiallySelectedItems={[]}
             open={true}
@@ -25,7 +30,6 @@ it('renders correctly when filters are not restricted', () => {
 it('renders correctly when filters are restricted', () => {
     const container = shallow(
         <FilterSettingsDialog
-            dimensions={mockDimensions}
             restrictFilters={true}
             initiallySelectedItems={mockSelectedItems}
             open={true}
diff --git a/src/modules/useDimensions.js b/src/modules/useDimensions.js
new file mode 100644
index 000000000..1a7b90a38
--- /dev/null
+++ b/src/modules/useDimensions.js
@@ -0,0 +1,44 @@
+import { useEffect } from 'react'
+import { useSelector, useDispatch } from 'react-redux'
+import { acSetDimensions } from '../actions/dimensions'
+import { apiFetchDimensions } from '@dhis2/analytics'
+import { useDataEngine } from '@dhis2/app-runtime'
+
+import getFilteredDimensions from './getFilteredDimensions'
+import { useUserSettings } from '../components/UserSettingsProvider'
+
+const useDimensions = doFetch => {
+    const dataEngine = useDataEngine()
+    const { userSettings } = useUserSettings()
+    const dimensions = useSelector(state => state.dimensions)
+    const dispatch = useDispatch()
+
+    useEffect(() => {
+        const fetchDimensions = async () => {
+            try {
+                const unfilteredDimensions = await apiFetchDimensions(
+                    dataEngine,
+                    userSettings.keyAnalysisDisplayProperty
+                )
+
+                dispatch(
+                    acSetDimensions(getFilteredDimensions(unfilteredDimensions))
+                )
+            } catch (e) {
+                console.error(e)
+            }
+        }
+
+        if (
+            !dimensions.length &&
+            doFetch &&
+            userSettings.keyAnalysisDisplayProperty
+        ) {
+            fetchDimensions()
+        }
+    }, [dimensions, doFetch, userSettings.keyAnalysisDisplayProperty])
+
+    return dimensions
+}
+
+export default useDimensions
diff --git a/src/reducers/dimensions.js b/src/reducers/dimensions.js
index 6bff94f44..71b7b0ccb 100644
--- a/src/reducers/dimensions.js
+++ b/src/reducers/dimensions.js
@@ -5,15 +5,17 @@ import {
 } from '@dhis2/analytics'
 
 export const SET_DIMENSIONS = 'SET_DIMENSIONS'
-export const DEFAULT_DIMENSIONS = [
+const PE_OU_DIMENSIONS = [
     getDimensionById(DIMENSION_ID_PERIOD),
     getDimensionById(DIMENSION_ID_ORGUNIT),
 ]
 
-export default (state = DEFAULT_DIMENSIONS, action) => {
+const INITIAL_DIMENSIONS = []
+
+export default (state = INITIAL_DIMENSIONS, action) => {
     switch (action.type) {
         case SET_DIMENSIONS: {
-            return [...DEFAULT_DIMENSIONS, ...action.value]
+            return [...PE_OU_DIMENSIONS, ...action.value]
         }
         default:
             return state