fix: crash when filtering a map that is being viewed as chart (#2246)

* fix: mapViews were already extracted prior to getFilteredVisualization * fix: do not remove id if we are using the original plugin
dhis2 · Mar 15, 2023 · d1cf7b8 · d1cf7b8
1 parent 3e5ee53
commit d1cf7b8
Show file tree

Hide file tree

Showing 5 changed files with 32 additions and 40 deletions.
diff --git a/src/components/Item/VisualizationItem/Visualization/MapPlugin.js b/src/components/Item/VisualizationItem/Visualization/MapPlugin.js
@@ -7,7 +7,7 @@ import NoVisualizationMessage from './NoVisualizationMessage.js'
 
 const mapViewIsEELayer = (mapView) => mapView.layer.includes('earthEngine')
 
-const MapPlugin = ({ visualization, style, ...pluginProps }) => {
+const MapPlugin = ({ visualization, ...pluginProps }) => {
     const { isDisconnected: offline } = useDhis2ConnectionStatus()
 
     if (offline && visualization.mapViews?.find(mapViewIsEELayer)) {
@@ -20,17 +20,10 @@ const MapPlugin = ({ visualization, style, ...pluginProps }) => {
         )
     }
 
-    return (
-        <IframePlugin
-            visualization={visualization}
-            style={style}
-            {...pluginProps}
-        />
-    )
+    return <IframePlugin visualization={visualization} {...pluginProps} />
 }
 
 MapPlugin.propTypes = {
-    style: PropTypes.object,
     visualization: PropTypes.object,
 }
 

diff --git a/src/components/Item/VisualizationItem/Visualization/Visualization.js b/src/components/Item/VisualizationItem/Visualization/Visualization.js
@@ -54,20 +54,16 @@ const Visualization = ({
         [availableHeight, availableWidth]
     )
 
-    const visualizationConfig = useMemo(
-        () => getVisualizationConfig(visualization, originalType, activeType),
-        [visualization, activeType, originalType]
-    )
+    const visualizationConfig = useMemo(() => {
+        if (originalType === EVENT_VISUALIZATION) {
+            return visualization
+        }
 
-    const filteredVisualization = useMemo(
-        () =>
-            getFilteredVisualization(
-                visualizationConfig,
-                itemFilters,
-                originalType
-            ),
-        [visualizationConfig, itemFilters, originalType]
-    )
+        return getFilteredVisualization(
+            getVisualizationConfig(visualization, originalType, activeType),
+            itemFilters
+        )
+    }, [visualization, activeType, originalType, itemFilters])
 
     const filterVersion = useMemo(() => uniqueId(), [])
 
@@ -104,7 +100,7 @@ const Visualization = ({
         case VISUALIZATION: {
             return (
                 <IframePlugin
-                    visualization={filteredVisualization}
+                    visualization={visualizationConfig}
                     {...iFramePluginProps}
                 />
             )
@@ -136,22 +132,20 @@ const Visualization = ({
                 </>
             )
         }
-
         case MAP: {
             return (
                 <MapPlugin
-                    visualization={filteredVisualization}
+                    visualization={visualizationConfig}
                     {...iFramePluginProps}
                 />
             )
         }
-
         default: {
             return pluginIsAvailable(activeType || item.type, d2) ? (
                 <LegacyPlugin
                     item={item}
                     activeType={activeType}
-                    visualization={filteredVisualization}
+                    visualization={visualizationConfig}
                     filterVersion={filterVersion}
                     style={style}
                     {...rest}

diff --git a/src/components/Item/VisualizationItem/Visualization/__tests__/getVisualizationConfig.spec.js b/src/components/Item/VisualizationItem/Visualization/__tests__/getVisualizationConfig.spec.js
@@ -21,7 +21,6 @@ describe('getVisualizationConfig', () => {
         const actualResult = getVisualizationConfig(visualization, MAP, MAP)
         const expectedResult = {
             ...visualization,
-            id: undefined,
         }
 
         expect(actualResult).toEqual(expectedResult)

diff --git a/src/components/Item/VisualizationItem/Visualization/getFilteredVisualization.js b/src/components/Item/VisualizationItem/Visualization/getFilteredVisualization.js
@@ -1,13 +1,11 @@
-import { MAP } from '../../../../modules/itemTypes.js'
-
 const mapViewIsThematicOrEvent = (mapView) =>
     mapView.layer.includes('thematic') || mapView.layer.includes('event')
 
 const getFilteredMap = (visualization, filters) => {
     // apply filters only to thematic and event layers
     const mapViews = visualization.mapViews.map((mapView) => {
         if (mapViewIsThematicOrEvent(mapView)) {
-            return getFilteredVisualization(mapView, filters)
+            return getFilteredNonMap(mapView, filters)
         }
 
         return mapView
@@ -19,15 +17,7 @@ const getFilteredMap = (visualization, filters) => {
     }
 }
 
-const getFilteredVisualization = (visualization, filters, originalType) => {
-    if (!Object.keys(filters).length) {
-        return visualization
-    }
-
-    if (originalType === MAP) {
-        return getFilteredMap(visualization, filters)
-    }
-
+const getFilteredNonMap = (visualization, filters) => {
     // deep clone objects in filters to avoid changing the visualization in the Redux store
     const visRows = visualization.rows.map((obj) => ({ ...obj }))
     const visColumns = visualization.columns.map((obj) => ({ ...obj }))
@@ -64,4 +54,16 @@ const getFilteredVisualization = (visualization, filters, originalType) => {
     }
 }
 
+const getFilteredVisualization = (visualization, filters) => {
+    if (!Object.keys(filters).length) {
+        return visualization
+    }
+
+    if (visualization.mapViews) {
+        return getFilteredMap(visualization, filters)
+    }
+
+    return getFilteredNonMap(visualization, filters)
+}
+
 export default getFilteredVisualization
diff --git a/src/components/Item/VisualizationItem/Visualization/getVisualizationConfig.js b/src/components/Item/VisualizationItem/Visualization/getVisualizationConfig.js
@@ -12,6 +12,10 @@ const getWithoutId = (object) => ({
 })
 
 const getVisualizationConfig = (visualization, originalType, activeType) => {
+    if (originalType === activeType) {
+        return visualization
+    }
+
     if (originalType === MAP && originalType !== activeType) {
         const thematicMapViews = getThematicMapViews(visualization)