fix: applying a filter to a dashboard with a Map item results in the map not rendering [DHIS2-11054]

cypress/integration/ui/dashboard_filter.feature

@@ -1,26 +1,33 @@
 Feature: Dashboard filter
 
-    Background:
-        Given I open the "Delivery" dashboard
-        And the "Delivery" dashboard displays in view mode
-
-    @nonmutating
     Scenario: I add a Period filter
+        When I choose to create new dashboard
+        And I add a MAP and a CHART and save
+        Then the dashboard displays in view mode
         When I add a "Period" filter
         Then the Period filter is applied to the dashboard
 
-    @nonmutating
     Scenario: I add a Organisation Unit filter
+        Given I open existing dashboard
+        Then the dashboard displays in view mode
         When I add a "Organisation Unit" filter
         Then the Organisation Unit filter is applied to the dashboard
 
-    @nonmutating
     Scenario: I add a Facility Type filter
+        Given I open existing dashboard
+        Then the dashboard displays in view mode
         When I add a "Facility Type" filter
         Then the Facility Type filter is applied to the dashboard
 
-    @nonmutating
     Scenario: I can access the dimensions modal from the filter badge
+        Given I open existing dashboard
         When I add a "Period" filter
         And I click on the "Period" filter badge
         Then the filter modal is opened
+
+    Scenario: I delete a dashboard
+        Given I open existing dashboard
+        When I choose to edit dashboard
+        And I choose to delete dashboard
+        When I confirm delete
+        Then different dashboard displays in view mode

cypress/integration/ui/dashboard_filter/create_dashboard.js

@@ -0,0 +1,68 @@
+import { Given, When, Then } from 'cypress-cucumber-preprocessor/steps'
+import { EXTENDED_TIMEOUT } from '../../../support/utils'
+import { gridItemSel, chartSel, mapSel } from '../../../selectors/dashboardItem'
+import {
+    dashboardChipSel,
+    dashboardTitleSel,
+} from '../../../selectors/viewDashboard'
+import { confirmActionDialogSel } from '../../../selectors/editDashboard'
+
+const TEST_DASHBOARD_TITLE = 'a filter ' + new Date().toUTCString()
+
+When('I add a MAP and a CHART and save', () => {
+    //add the title
+    cy.get('[data-test="dashboard-title-input"]').type(TEST_DASHBOARD_TITLE)
+
+    // add items
+    cy.get('[data-test="item-search"]').click()
+    cy.get('[data-test="item-search"]')
+        .find('input')
+        .type('Inpatient', { force: true })
+
+    // //chart
+    cy.get(
+        '[data-test="menu-item-Inpatient: BMI this year by districts"]'
+    ).click()
+
+    //map
+    cy.get(
+        '[data-test="menu-item-Inpatient: BMI at facility level this year"]'
+    ).click()
+
+    cy.get('[data-test="dhis2-uicore-layer"]').click('topLeft')
+
+    //move things so the dashboard is more compact
+    cy.get(`${gridItemSel}.MAP`)
+        .trigger('mousedown')
+        .trigger('mousemove', { clientX: 600 })
+        .trigger('mouseup')
+
+    //save
+    cy.get('button').contains('Save changes', EXTENDED_TIMEOUT).click()
+})
+
+Given('I open existing dashboard', () => {
+    cy.get(dashboardChipSel, EXTENDED_TIMEOUT)
+        .contains(TEST_DASHBOARD_TITLE)
+        .click()
+})
+
+Then('the dashboard displays in view mode', () => {
+    // check for a map canvas and a highcharts element
+    cy.get(chartSel).should('be.visible')
+    cy.get(mapSel).should('be.visible')
+})
+
+When('I choose to delete dashboard', () => {
+    cy.get('[data-test="delete-dashboard-button"]').click()
+})
+
+When('I confirm delete', () => {
+    cy.get(confirmActionDialogSel).find('button').contains('Delete').click()
+})
+
+Then('different dashboard displays in view mode', () => {
+    cy.get(dashboardTitleSel)
+        .should('be.visible')
+        .and('not.contain', TEST_DASHBOARD_TITLE)
+})

cypress/integration/ui/dashboard_filter/dashboard_filter.js

@@ -1,34 +1,43 @@
 import { Then } from 'cypress-cucumber-preprocessor/steps'
 import {
-    getDashboardItem,
+    gridItemSel,
+    mapLegendButtonSel,
+    mapLegendContentSel,
     chartSubtitleSel,
     chartXAxisLabelSel,
 } from '../../../selectors/dashboardItem'
+import { innerScrollContainerSel } from '../../../selectors/viewDashboard'
+
 import {
     filterBadgeSel,
     dimensionsModalSel,
 } from '../../../selectors/dashboardFilter'
-import { dashboards } from '../../../assets/backends'
 import { EXTENDED_TIMEOUT } from '../../../support/utils'
 
 const PERIOD = 'Last 6 months'
 const OU = 'Sierra Leone'
 const FACILITY_TYPE = 'Clinic'
 
-const chartItemUid = dashboards.Delivery.items.chart.itemUid
-
 /*
 Scenario: I add a Period filter
 */
 
 Then('the Period filter is applied to the dashboard', () => {
     cy.get(filterBadgeSel).contains(`Period: ${PERIOD}`).should('be.visible')
 
-    getDashboardItem(chartItemUid)
+    // check the CHART
+    cy.get(`${gridItemSel}.CHART`)
         .find(chartSubtitleSel, EXTENDED_TIMEOUT)
         .scrollIntoView()
         .contains(PERIOD, EXTENDED_TIMEOUT)
         .should('be.visible')
+
+    cy.get(innerScrollContainerSel).scrollTo('top')
+    // check the MAP
+    cy.get('.dhis2-map-legend-button', EXTENDED_TIMEOUT).trigger('mouseover')
+    cy.get('.dhis2-map-legend-period', EXTENDED_TIMEOUT)
+        .contains(PERIOD)
+        .should('be.visible')
 })
 
 /*
@@ -40,7 +49,8 @@ Then('the Organisation Unit filter is applied to the dashboard', () => {
         .contains(`Organisation Unit: ${OU}`)
         .should('be.visible')
 
-    getDashboardItem(chartItemUid)
+    cy.get(innerScrollContainerSel).scrollTo('bottom')
+    cy.get(`${gridItemSel}.CHART`)
         .find(chartXAxisLabelSel, EXTENDED_TIMEOUT)
         .scrollIntoView()
         .contains(OU, EXTENDED_TIMEOUT)
@@ -55,11 +65,19 @@ Then('the Facility Type filter is applied to the dashboard', () => {
         .contains(`Facility Type: ${FACILITY_TYPE}`)
         .should('be.visible')
 
-    getDashboardItem(chartItemUid)
+    cy.get(innerScrollContainerSel).scrollTo('top')
+    cy.get(`${gridItemSel}.CHART`)
         .find(chartSubtitleSel, EXTENDED_TIMEOUT)
         .scrollIntoView()
         .contains(FACILITY_TYPE, EXTENDED_TIMEOUT)
         .should('be.visible')
+
+    cy.get(innerScrollContainerSel).scrollTo('top')
+    cy.get(mapLegendButtonSel, EXTENDED_TIMEOUT).trigger('mouseover')
+    cy.get(mapLegendContentSel, EXTENDED_TIMEOUT)
+        .find('div')
+        .contains(`Facility Type: ${FACILITY_TYPE}`)
+        .should('be.visible')
 })
 
 Then('the filter modal is opened', () => {

cypress/selectors/dashboardItem.js

@@ -13,3 +13,8 @@ export const getDashboardItem = itemUid =>
 
 export const clickMenuButton = itemUid =>
     getDashboardItem(itemUid).find(itemMenuButton).click()
+
+//map
+
+export const mapLegendButtonSel = '.dhis2-map-legend-button'
+export const mapLegendContentSel = '.dhis2-map-legend-content'

cypress/selectors/viewDashboard.js

@@ -17,5 +17,6 @@ export const dragHandleSel = '[data-test="controlbar-drag-handle"]'
 export const dashboardsBarSel = '[data-test="dashboards-bar"]'
 
 export const outerScrollContainerSel = '[data-test="outer-scroll-container"]'
+export const innerScrollContainerSel = '[data-test="inner-scroll-container"]'
 
 export const editControlBarSel = '[data-test="edit-control-bar"]'

src/components/DashboardContainer.js

@@ -12,6 +12,7 @@ const DashboardContainer = ({ children, covered }) => {
                 'dashboard-scroll-container',
                 covered && classes.covered
             )}
+            data-test="inner-scroll-container"
         >
             {children}
         </div>

src/components/Item/VisualizationItem/Item.js

@@ -1,7 +1,6 @@
 import React, { Component } from 'react'
 import PropTypes from 'prop-types'
 import { connect } from 'react-redux'
-import uniqueId from 'lodash/uniqueId'
 import isEmpty from 'lodash/isEmpty'
 import i18n from '@dhis2/d2-i18n'
 import Visualization from './Visualization/Visualization'
@@ -120,8 +119,6 @@ class Item extends Component {
         }
     }
 
-    getUniqueKey = memoizeOne(() => uniqueId())
-
     onToggleFooter = () => {
         this.setState(
             { showFooter: !this.state.showFooter },
@@ -215,7 +212,6 @@ class Item extends Component {
                     onFatalError={this.onFatalError}
                 >
                     <div
-                        key={this.getUniqueKey(itemFilters)}
                         className="dashboard-item-content"
                         ref={ref => (this.contentRef = ref)}
                     >
@@ -231,6 +227,7 @@ class Item extends Component {
                                         )}
                                         availableWidth={this.getAvailableWidth()}
                                         gridWidth={this.props.gridWidth}
+                                        dashboardMode={dashboardMode}
                                     />
                                 )}
                             </WindowDimensionsCtx.Consumer>

src/components/Item/VisualizationItem/Visualization/DefaultPlugin.js

@@ -2,10 +2,17 @@ import React, { useEffect, useRef } from 'react'
 import PropTypes from 'prop-types'
 import { useD2 } from '@dhis2/app-runtime-adapter-d2'
 import { useConfig } from '@dhis2/app-runtime'
-import { load, unmount } from './plugin'
+import { load } from './plugin'
 import getVisualizationContainerDomId from '../getVisualizationContainerDomId'
 
-const DefaultPlugin = ({ item, activeType, visualization, options, style }) => {
+const DefaultPlugin = ({
+    item,
+    activeType,
+    filterVersion,
+    visualization,
+    options,
+    style,
+}) => {
     const { d2 } = useD2()
     const { baseUrl } = useConfig()
     const credentials = {
@@ -15,6 +22,7 @@ const DefaultPlugin = ({ item, activeType, visualization, options, style }) => {
 
     const prevItem = useRef()
     const prevActiveType = useRef()
+    const prevFilterVersion = useRef()
 
     useEffect(() => {
         load(item, visualization, {
@@ -25,13 +33,18 @@ const DefaultPlugin = ({ item, activeType, visualization, options, style }) => {
 
         prevItem.current = item
         prevActiveType.current = activeType
-
-        return () => unmount(item, activeType)
+        prevFilterVersion.current = filterVersion
     }, [])
 
     useEffect(() => {
-        if (shouldPluginReload()) {
-            unmount(item, prevActiveType.current)
+        if (
+            prevItem.current === item &&
+            (prevActiveType.current !== activeType ||
+                prevFilterVersion.current !== filterVersion)
+        ) {
+            /* Item is the same but type or filters has changed
+             * so necessary to reload
+             */
             load(item, visualization, {
                 credentials,
                 activeType,
@@ -40,24 +53,15 @@ const DefaultPlugin = ({ item, activeType, visualization, options, style }) => {
 
         prevItem.current = item
         prevActiveType.current = activeType
-    }, [item, visualization, activeType])
-
-    /**
-     * Prevent unnecessary re-rendering
-     * TODO: fix this hack
-     */
-    const shouldPluginReload = () => {
-        const reloadAllowed = prevItem.current === item
-        const visChanged = prevActiveType.current !== activeType
-
-        return reloadAllowed && visChanged
-    }
+        prevFilterVersion.current = filterVersion
+    }, [item, visualization, activeType, filterVersion])
 
     return <div id={getVisualizationContainerDomId(item.id)} style={style} />
 }
 
 DefaultPlugin.propTypes = {
     activeType: PropTypes.string,
+    filterVersion: PropTypes.string,
     item: PropTypes.object,
     options: PropTypes.object,
     style: PropTypes.object,