From 0f8186b2a718c35ad5ce86fd80a1ca669e7682b2 Mon Sep 17 00:00:00 2001 From: mahmoudadel Date: Wed, 28 Aug 2024 19:41:52 +0300 Subject: [PATCH] #10489: Fix #10489 Problems with GeoStory 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 --- .../components/geostory/common/MapEditor.jsx | 11 ++++- .../common/enhancers/__tests__/map-test.jsx | 33 ------------- .../geostory/common/enhancers/map.jsx | 9 ++-- web/client/utils/GeoStoryUtils.js | 5 +- .../utils/__tests__/GeoStoryUtils-test.js | 46 ------------------- 5 files changed, 15 insertions(+), 89 deletions(-) diff --git a/web/client/components/geostory/common/MapEditor.jsx b/web/client/components/geostory/common/MapEditor.jsx index 7313d692db..d618232cdb 100644 --- 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"); + } + }}/> ] || [ { })); ReactDOM.render(, 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(, 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 index ebab6f7984..039ab6574f 100644 --- 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 index a29dde4e7f..01a0f3b999 100644 --- 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 index 1c950f7a6c..d2b87aebc2 100644 --- 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);