geosolutions-it#10489: Fix geosolutions-it#10489 Problems with GeoSto…

…ry map configurations merge process Description: - implament new approach in onChange for Editor in MapEditor file to fix the app crash issue if user edit layer properties of existing geostory - remove the prev implemented approach - remove unused unit tests
mahmoudadel54 · Sep 5, 2024 · 0f8186b · 0f8186b
1 parent 1bee05f
commit 0f8186b
Show file tree

Hide file tree

Showing 5 changed files with 15 additions and 89 deletions.
diff --git a/web/client/components/geostory/common/MapEditor.jsx b/web/client/components/geostory/common/MapEditor.jsx
@@ -31,6 +31,7 @@ import Message from '../../I18N/Message';
 
 import MapConfiguratorTabs from './map/MapConfiguratorTabs';
 import withMapConfiguratorTabs from './enhancers/withMapConfiguratorTabs';
+import set from 'lodash/fp/set';
 
 
 const StepHeader = ({title, description}) => (
@@ -102,7 +103,15 @@ const MapEditor = ({
                         closeNodeEditor={closeNodeEditor}
                         editNode={editNode}
                         map={map}
-                        onChange={onChange}/>
+                        onChange={(path, value) => {
+                            const config = set(path, value, { map });
+                            const { layers, groups } = config?.map || {};
+                            if (path.includes('map.layers[')) {
+                                onChangeMap('layers', layers, "replace");
+                            } else {
+                                onChangeMap('groups', groups, "replace");
+                            }
+                        }}/>
                 ] ||
                 [
                     <MapConfigurator

diff --git a/web/client/components/geostory/common/enhancers/__tests__/map-test.jsx b/web/client/components/geostory/common/enhancers/__tests__/map-test.jsx
@@ -62,39 +62,6 @@ describe("geostory media map component enhancers", () => {
         }));
         ReactDOM.render(<Provider store={store}><Sink resourceId={resourceId} map={{}}/></Provider>, document.getElementById("container"));
     });
-    it('withMapEnhancer generate correct props for geostory with layers array includes empty items', (done) => {
-        const resources = [{id: "1", type: "map", data: {id: "2", layers: [
-            {
-                name: "layer01", center: {x: 1, y: 1, crs: 'EPSG:4326'}, zoom: 1
-            }, {
-                name: "layer02", center: {x: 2, y: 2, crs: 'EPSG:4326'}, zoom: 2
-            }, {
-                name: "layer03", center: {x: 3, y: 3, crs: 'EPSG:4326'}, zoom: 3
-            }, { name: "layer04", center: {x: 4, y: 4, crs: 'EPSG:4326'}, zoom: 4, "visibility": true}], groups: [], context: "1"}}];
-        const store = {
-            subscribe: () => {}, getState: () => ({geostory: {currentStory: {resources}}})
-        };
-        const resourceId = "1";
-        const Sink = withMapEnhancer(createSink( props => {
-            expect(props).toBeTruthy();
-            expect(props.map).toBeTruthy();
-            expect(props.map).toEqual({id: "2", layers: [ {
-                name: "layer01", center: {x: 1, y: 1, crs: 'EPSG:4326'}, zoom: 1
-            }, {
-                name: "layer02", center: {x: 2, y: 2, crs: 'EPSG:4326'}, zoom: 2
-            }, {
-                name: "layer03", center: {x: 3, y: 3, crs: 'EPSG:4326'}, zoom: 3
-            }, { name: "layer04", center: {x: 4, y: 4, crs: 'EPSG:4326'}, zoom: 4, "visibility": false
-            }], groups: [], context: "1"});
-            done();
-        }));
-        let layersWithEmptyItems = [];
-        layersWithEmptyItems.length = 3;
-        layersWithEmptyItems.push({visibility: false});
-        ReactDOM.render(<Provider store={store}><Sink resourceId={resourceId} map={{
-            id: "2", layers: layersWithEmptyItems, groups: [], context: "1"
-        }}/></Provider>, document.getElementById("container"));
-    });
     it('withLocalMapState generate correct props', (done) => {
         const Sink = withLocalMapState(createSink( props => {
             expect(props).toExist();

diff --git a/web/client/components/geostory/common/enhancers/map.jsx b/web/client/components/geostory/common/enhancers/map.jsx
@@ -40,11 +40,8 @@ export default compose(
             const resource = find(resources, { id: resourceId }) || {};
             const baseMap = omit(resource.data, ['context']);
             const isLegacyGeostory = (map?.layers || [])?.indexOf(null) !== -1 || (map?.groups || [])?.indexOf(null) !== -1;
-            // 'layersGroupsHasEmptyItems' is a flag that indicates if the geostory 'map' object in section includes empty layers/groups
-            // the layers/groups array may include empty items in case change layer properties e.g: opacity,title ...etc
-            const layersGroupsHasEmptyItems = (map?.layers || []).includes(undefined) || (map?.groups || []).includes(undefined);
             const cleanedMap = {...map, layers: (map.layers || baseMap?.layers || []).map(lay => lay ? lay : undefined), groups: (map.groups || baseMap?.groups || []).map(gr => gr ? gr : undefined)};         // for better initiating cleanedMap layers in case 'map.layers = undefined' -> baseMap.layers check is added in fallBack
-            return { map: createMapObject(baseMap, cleanedMap, isLegacyGeostory, layersGroupsHasEmptyItems)};
+            return { map: createMapObject(baseMap, cleanedMap, isLegacyGeostory)};
         }
     ));
 /**
@@ -63,8 +60,8 @@ export const withFocusedContentMap = compose(
  */
 export const handleMapUpdate = withHandlers({
     onChangeMap: ({update, focusedContent = {}}) =>
-        (path, value) => {
-            update(`${focusedContent.path}.map.${path}`, value, "merge");
+        (path, value, mode = "merge") => {
+            update(`${focusedContent.path}.map.${path}`, value, mode);
         },
     onChange: ({update, focusedContent = {}}) =>
         (path, value, mode = 'merge') => {

diff --git a/web/client/utils/GeoStoryUtils.js b/web/client/utils/GeoStoryUtils.js
@@ -172,12 +172,11 @@ export const applyDefaults = (options = {}) => merge({}, DEFAULT_MAP_OPTIONS, op
  * @param {object} baseMap initial map object
  * @param {object} overrides object to override with
  * @param {bool} isLegacyGeostory boolean that indicates if the geostory is legacy one or new
- * @param {bool} layersGroupsHasEmptyItems boolean that indicates if the geostory map object in section includes empty layers/groups
  * @return {object} options merged with defaults
  */
-export const createMapObject = (baseMap = {}, overrides = {}, isLegacyGeostory = false, layersGroupsHasEmptyItems = false) => {
+export const createMapObject = (baseMap = {}, overrides = {}, isLegacyGeostory = false) => {
     const mergedMap = merge({}, baseMap, overrides);
-    if (isLegacyGeostory || layersGroupsHasEmptyItems) {
+    if (isLegacyGeostory) {
         return mergedMap;
     }
     return {

diff --git a/web/client/utils/__tests__/GeoStoryUtils-test.js b/web/client/utils/__tests__/GeoStoryUtils-test.js
@@ -487,52 +487,6 @@ describe("GeoStory Utils", () => {
         }, true);
         expect(res).toEqual(merged);
     });
-    it('test override layers in  createMapObject with layers array includes empty items ', () => {
-        let layersWithEmptyItems = [];
-        layersWithEmptyItems.length = 3;
-        layersWithEmptyItems.push({visibility: false});
-        const merged = {
-            zoomControl: true,
-            mapInfoControl: false,
-            mapOptions: {
-                scrollWheelZoom: false,
-                interactions: {
-                    mouseWheelZoom: false,
-                    mouseClick: false,
-                    dragPan: true
-                }
-            },
-            layers: [{
-                name: "layer01", center: {x: 1, y: 1, crs: 'EPSG:4326'}, zoom: 1
-            }, {
-                name: "layer02", center: {x: 2, y: 2, crs: 'EPSG:4326'}, zoom: 2
-            }, {
-                name: "layer03", center: {x: 3, y: 3, crs: 'EPSG:4326'}, zoom: 3
-            }, {
-                name: "layer04", center: {x: 4, y: 4, crs: 'EPSG:4326'}, zoom: 4, visibility: false
-            }],
-            groups: []
-        };
-        const res = createMapObject({...DEFAULT_MAP_OPTIONS, layers: [{
-            name: "layer01", center: {x: 1, y: 1, crs: 'EPSG:4326'}, zoom: 1
-        }, {
-            name: "layer02", center: {x: 2, y: 2, crs: 'EPSG:4326'}, zoom: 2
-        }, {
-            name: "layer03", center: {x: 3, y: 3, crs: 'EPSG:4326'}, zoom: 3
-        }, {
-            name: "layer04", center: {x: 4, y: 4, crs: 'EPSG:4326'}, zoom: 4, visibility: true
-        }], groups: []}, {
-            mapOptions: {
-                scrollWheelZoom: false,
-                interactions: {
-                    mouseClick: false
-                }
-            },
-            layers: layersWithEmptyItems,
-            groups: []
-        }, false, true);
-        expect(res).toEqual(merged);
-    });
     it('test testRegex', () => {
         const title = "title";
         expect(testRegex(title, "it")).toBe(true);