diff --git a/src/components/app/App.jsx b/src/components/app/App.jsx index 91a8f4ab13..0b9cdf7a1a 100644 --- a/src/components/app/App.jsx +++ b/src/components/app/App.jsx @@ -8,6 +8,7 @@ import DownloadSettings from '../download/DownloadSettings.jsx' import LayersPanel from '../layers/LayersPanel.jsx' import MapPosition from '../map/MapPosition.jsx' import AppMenu from './AppMenu.jsx' +import ConfirmLeaveModal from './ConfirmLeaveModal.jsx' import DetailsPanel from './DetailsPanel.jsx' import ModalContainer from './ModalContainer.jsx' import './App.css' @@ -28,7 +29,7 @@ const App = () => { ) }, []) - useLoadMap() + const { locationToConfirm, confirmLeave, cancelLeave } = useLoadMap() useLoadDataStore() useLayersLoader() @@ -69,6 +70,12 @@ const App = () => { )} + {locationToConfirm && ( + + )} ) } diff --git a/src/components/app/ConfirmLeaveModal.jsx b/src/components/app/ConfirmLeaveModal.jsx new file mode 100644 index 0000000000..937d84b146 --- /dev/null +++ b/src/components/app/ConfirmLeaveModal.jsx @@ -0,0 +1,47 @@ +import i18n from '@dhis2/d2-i18n' +import { + Modal, + ModalTitle, + ModalContent, + ModalActions, + Button, + ButtonStrip, +} from '@dhis2/ui' +import PropTypes from 'prop-types' +import React from 'react' + +const ConfirmLeaveModal = ({ onCancel, onConfirm }) => ( + + {i18n.t('Discard unsaved changes?')} + + {i18n.t( + 'Are you sure you want to leave this map? Any unsaved changes will be lost.' + )} + + + + + + + + +) + +ConfirmLeaveModal.propTypes = { + onCancel: PropTypes.func.isRequired, + onConfirm: PropTypes.func.isRequired, +} + +export default ConfirmLeaveModal diff --git a/src/components/app/FileMenu.jsx b/src/components/app/FileMenu.jsx index cfd1b6c188..3e4f769682 100644 --- a/src/components/app/FileMenu.jsx +++ b/src/components/app/FileMenu.jsx @@ -207,7 +207,7 @@ const FileMenu = ({ onFileMenuAction }) => { saveAsAlert.show({ msg: getSavedMessage(getMapName(name)) }) if (res.response.uid) { - history.push(`/${res.response.uid}`) + history.push(`/${res.response.uid}`, { isSaving: true }) } } } diff --git a/src/components/app/__tests__/ConfirmLeaveModal.spec.jsx b/src/components/app/__tests__/ConfirmLeaveModal.spec.jsx new file mode 100644 index 0000000000..65daad002d --- /dev/null +++ b/src/components/app/__tests__/ConfirmLeaveModal.spec.jsx @@ -0,0 +1,25 @@ +import { render, fireEvent, screen } from '@testing-library/react' +import React from 'react' +import ConfirmLeaveModal from '../ConfirmLeaveModal.jsx' + +describe('ConfirmLeaveModal', () => { + it('calls onCancel when the cancel button is clicked', () => { + const onCancel = jest.fn() + render() + + fireEvent.click(screen.getByTestId('confirm-leave-modal-option-cancel')) + + expect(onCancel).toHaveBeenCalledTimes(1) + }) + + it('calls onConfirm when the leave button is clicked', () => { + const onConfirm = jest.fn() + render() + + fireEvent.click( + screen.getByTestId('confirm-leave-modal-option-confirm') + ) + + expect(onConfirm).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/components/app/__tests__/useLoadMap.spec.js b/src/components/app/__tests__/useLoadMap.spec.js new file mode 100644 index 0000000000..0360d37780 --- /dev/null +++ b/src/components/app/__tests__/useLoadMap.spec.js @@ -0,0 +1,181 @@ +import { renderHook, act, waitFor } from '@testing-library/react' +import history from '../../../util/history.js' +import { useLoadMap } from '../useLoadMap.js' + +const mockDispatch = jest.fn() +let mockMapState +let mockSavedMapState + +jest.mock('query-string', () => ({ + parse: jest.fn(() => ({})), +})) + +jest.mock('react-redux', () => ({ + useDispatch: () => mockDispatch, + useSelector: (selector) => + selector({ map: mockMapState, savedMap: mockSavedMapState }), +})) + +const mockEngine = { + query: jest.fn().mockResolvedValue({}), + mutate: jest.fn().mockResolvedValue({}), +} + +jest.mock('@dhis2/app-runtime', () => ({ + useDataEngine: () => mockEngine, +})) + +jest.mock('@dhis2/app-service-alerts', () => ({ + useAlert: () => ({ show: jest.fn() }), +})) + +const mockCachedData = { + systemSettings: { keyDefaultBaseMap: 'osm' }, + basemaps: [{ id: 'osm' }], +} + +jest.mock('../../cachedDataProvider/CachedDataProvider.jsx', () => ({ + useCachedData: () => mockCachedData, +})) + +jest.mock('../../../util/requests.js', () => ({ + fetchMap: jest.fn((args) => + Promise.resolve({ + id: args.id, + basemap: { id: 'osm' }, + mapViews: [], + }) + ), +})) + +jest.mock('../../../util/basemaps.js', () => ({ + getBasemapOrFallback: ({ id }) => ({ id }), +})) + +const dirtyMap = { id: 'map1', mapViews: [{ id: 'layer1', opacity: 0.5 }] } +const savedMap = { id: 'map1', mapViews: [{ id: 'layer1', opacity: 1 }] } + +describe('useLoadMap - unsaved changes guard', () => { + beforeEach(() => { + mockDispatch.mockClear() + mockMapState = savedMap + mockSavedMapState = savedMap + }) + + const mountOnMap = async (mapId) => { + history.replace(`/${mapId}`) + const hook = renderHook(() => useLoadMap()) + // Let the initial mount-time loadMap() resolve so previousParamsRef + // picks up this map id before we simulate further navigation. + await act(async () => { + await Promise.resolve() + await Promise.resolve() + }) + return hook + } + + it('shows a confirm dialog when dirty and switching to a different map', async () => { + mockMapState = dirtyMap + const { result } = await mountOnMap('map1') + + act(() => { + history.push('/map2') + }) + + expect(result.current.locationToConfirm).not.toBeNull() + }) + + it('does not gate navigation when the map is not dirty', async () => { + mockMapState = savedMap + const { result } = await mountOnMap('map1') + mockDispatch.mockClear() + + act(() => { + history.push('/map2') + }) + + expect(result.current.locationToConfirm).toBeNull() + await waitFor(() => expect(mockDispatch).toHaveBeenCalled()) + }) + + it('confirmLeave navigates to the pending location and clears it', async () => { + mockMapState = dirtyMap + const { result } = await mountOnMap('map1') + + act(() => { + history.push('/map2') + }) + expect(result.current.locationToConfirm).not.toBeNull() + + mockDispatch.mockClear() + await act(async () => { + result.current.confirmLeave() + await Promise.resolve() + await Promise.resolve() + }) + + expect(result.current.locationToConfirm).toBeNull() + expect(mockDispatch).toHaveBeenCalled() + }) + + it('cancelLeave clears the pending location without loading it', async () => { + mockMapState = dirtyMap + const { result } = await mountOnMap('map1') + + act(() => { + history.push('/map2') + }) + expect(result.current.locationToConfirm).not.toBeNull() + + mockDispatch.mockClear() + act(() => { + result.current.cancelLeave() + }) + + expect(result.current.locationToConfirm).toBeNull() + expect(mockDispatch).not.toHaveBeenCalled() + }) + + it('does not gate a Save As redirect even when dirty', async () => { + mockMapState = dirtyMap + const { result } = await mountOnMap('map1') + mockDispatch.mockClear() + + act(() => { + history.push('/map2', { isSaving: true }) + }) + + expect(result.current.locationToConfirm).toBeNull() + await waitFor(() => expect(mockDispatch).toHaveBeenCalled()) + }) + + it('does not gate re-opening the same map while dirty', async () => { + mockMapState = dirtyMap + const { result } = await mountOnMap('map1') + mockDispatch.mockClear() + + act(() => { + history.replace('/map1') + }) + + expect(result.current.locationToConfirm).toBeNull() + await waitFor(() => expect(mockDispatch).toHaveBeenCalled()) + }) + + it('clears a pending confirm when the browser Back button reverts the navigation itself', async () => { + mockMapState = dirtyMap + const { result } = await mountOnMap('map1') + + act(() => { + history.push('/map2') + }) + expect(result.current.locationToConfirm).not.toBeNull() + + act(() => { + history.back() + }) + await waitFor(() => expect(history.location.pathname).toBe('/map1')) + + expect(result.current.locationToConfirm).toBeNull() + }) +}) diff --git a/src/components/app/useLoadMap.js b/src/components/app/useLoadMap.js index d035e5a2e8..0d27689f27 100644 --- a/src/components/app/useLoadMap.js +++ b/src/components/app/useLoadMap.js @@ -11,6 +11,7 @@ import { ALERT_CRITICAL, ALERT_MESSAGE_DYNAMIC, } from '../../constants/alerts.js' +import { useUnsavedChangesGuard } from '../../hooks/useUnsavedChangesGuard.js' import { CURRENT_AO_KEY } from '../../util/analyticalObject.js' import { dataStatisticsMutation } from '../../util/apiDataStatistics.js' import { getBasemapOrFallback } from '../../util/basemaps.js' @@ -89,6 +90,14 @@ export const useLoadMap = () => { [basemaps, defaultBasemap, dispatch, engine] ) + const { + locationToConfirm, + setLocationToConfirm, + isDirtyNow, + confirmLeave, + cancelLeave, + } = useUnsavedChangesGuard(loadMap) + useEffect(() => { loadMap(history.location) }, [loadMap]) @@ -122,11 +131,17 @@ export const useLoadMap = () => { ) const params = getHashUrlParams(location) - - if ( - action === 'REPLACE' || + const isSwitchingMap = previousParamsRef.current.mapId !== params.mapId - ) { + + if (isSwitchingMap && !location.state?.isSaving && isDirtyNow()) { + setLocationToConfirm(location) + return + } + + setLocationToConfirm(null) + + if (action === 'REPLACE' || isSwitchingMap) { loadMap(location) return } @@ -144,5 +159,7 @@ export const useLoadMap = () => { }) return () => unlisten?.() - }, [loadMap, dispatch]) + }, [loadMap, dispatch, isDirtyNow, setLocationToConfirm]) + + return { locationToConfirm, confirmLeave, cancelLeave } } diff --git a/src/components/map/MapName.jsx b/src/components/map/MapName.jsx index 9e098b21ff..2213682bf4 100644 --- a/src/components/map/MapName.jsx +++ b/src/components/map/MapName.jsx @@ -1,14 +1,24 @@ +import i18n from '@dhis2/d2-i18n' import React from 'react' import { useSelector } from 'react-redux' +import { useMapDirty } from '../../hooks/useMapDirty.js' import styles from './styles/MapName.module.css' const MapName = () => { const name = useSelector((state) => state.map.displayName) const downloadMode = useSelector((state) => state.ui.downloadMode) + const dirty = useMapDirty() return !downloadMode && name ? (
-
{name}
+
+ {name} + {dirty && ( + + {` - ${i18n.t('Edited')}`} + + )} +
) : null } diff --git a/src/components/map/__tests__/MapName.spec.jsx b/src/components/map/__tests__/MapName.spec.jsx new file mode 100644 index 0000000000..28e908bd42 --- /dev/null +++ b/src/components/map/__tests__/MapName.spec.jsx @@ -0,0 +1,67 @@ +import { render } from '@testing-library/react' +import React from 'react' +import { Provider } from 'react-redux' +import configureMockStore from 'redux-mock-store' +import { useMapDirty } from '../../../hooks/useMapDirty.js' +import MapName from '../MapName.jsx' + +jest.mock('../../../hooks/useMapDirty.js') + +const mockStore = configureMockStore() + +const renderWithState = ({ displayName, downloadMode = false }) => + render( + + + + ) + +describe('MapName', () => { + afterEach(() => { + jest.clearAllMocks() + }) + + it('renders nothing when there is no map name', () => { + useMapDirty.mockReturnValue(false) + + const { queryByTestId } = renderWithState({ displayName: undefined }) + + expect(queryByTestId('map-name')).toBeNull() + }) + + it('renders nothing in download mode', () => { + useMapDirty.mockReturnValue(false) + + const { queryByTestId } = renderWithState({ + displayName: 'My map', + downloadMode: true, + }) + + expect(queryByTestId('map-name')).toBeNull() + }) + + it('renders the name without a suffix when not dirty', () => { + useMapDirty.mockReturnValue(false) + + const { getByTestId, queryByTestId } = renderWithState({ + displayName: 'My map', + }) + + expect(getByTestId('map-name')).toHaveTextContent('My map') + expect(queryByTestId('map-name-edited')).toBeNull() + }) + + it('appends the "- Edited" suffix when dirty', () => { + useMapDirty.mockReturnValue(true) + + const { getByTestId } = renderWithState({ displayName: 'My map' }) + + expect(getByTestId('map-name-edited')).toHaveTextContent('Edited') + expect(getByTestId('map-name')).toHaveTextContent('My map - Edited') + }) +}) diff --git a/src/components/map/styles/MapName.module.css b/src/components/map/styles/MapName.module.css index 42c0d05c11..eed9da3fc9 100644 --- a/src/components/map/styles/MapName.module.css +++ b/src/components/map/styles/MapName.module.css @@ -19,3 +19,7 @@ padding: 6px var(--spacers-dp8); line-height: 17px; } + +.edited { + color: var(--colors-grey700); +} diff --git a/src/hooks/__tests__/useMapDirty.spec.js b/src/hooks/__tests__/useMapDirty.spec.js new file mode 100644 index 0000000000..bb710aa1da --- /dev/null +++ b/src/hooks/__tests__/useMapDirty.spec.js @@ -0,0 +1,85 @@ +import { renderHook, act } from '@testing-library/react' +import React from 'react' +import { Provider } from 'react-redux' +import { createStore } from 'redux' +import { changeLayerOpacity } from '../../actions/layers.js' +import { setMap } from '../../actions/map.js' +import rootReducer from '../../reducers/index.js' +import { useMapDirty } from '../useMapDirty.js' + +const savedMapConfig = { + id: 'abc', + name: 'My map', + basemap: { id: 'osm', opacity: 1, isVisible: true }, + mapViews: [{ id: 'layer1', opacity: 1, isVisible: true }], +} + +const renderWithStore = () => { + const store = createStore(rootReducer) + store.dispatch(setMap(savedMapConfig)) + + const { result } = renderHook(() => useMapDirty(), { + wrapper: ({ children }) => ( + {children} + ), + }) + + return { store, result } +} + +describe('useMapDirty', () => { + beforeEach(() => { + jest.useFakeTimers() + }) + + afterEach(() => { + jest.useRealTimers() + }) + + it('is false right after a map is loaded', () => { + const { result } = renderWithStore() + + act(() => { + jest.advanceTimersByTime(300) + }) + + expect(result.current).toBe(false) + }) + + it('becomes true after the debounce window once the map changes', () => { + const { store, result } = renderWithStore() + + act(() => { + store.dispatch(changeLayerOpacity('layer1', 0.5)) + }) + + // Still inside the debounce window + expect(result.current).toBe(false) + + act(() => { + jest.advanceTimersByTime(300) + }) + + expect(result.current).toBe(true) + }) + + it('only settles once after rapid successive changes (simulated slider drag)', () => { + const { store, result } = renderWithStore() + + act(() => { + for (let opacity = 0.9; opacity >= 0.5; opacity -= 0.1) { + store.dispatch(changeLayerOpacity('layer1', opacity)) + jest.advanceTimersByTime(50) // faster than the 300ms debounce + } + }) + + // The debounce window keeps getting reset, so it never fired yet + expect(result.current).toBe(false) + + act(() => { + jest.advanceTimersByTime(300) + }) + + expect(result.current).toBe(true) + }) +}) diff --git a/src/hooks/__tests__/useUnsavedChangesGuard.spec.js b/src/hooks/__tests__/useUnsavedChangesGuard.spec.js new file mode 100644 index 0000000000..a602bdd878 --- /dev/null +++ b/src/hooks/__tests__/useUnsavedChangesGuard.spec.js @@ -0,0 +1,120 @@ +import { renderHook, act, waitFor } from '@testing-library/react' +import history from '../../util/history.js' +import { useUnsavedChangesGuard } from '../useUnsavedChangesGuard.js' + +let mockMapState +let mockSavedMapState + +jest.mock('query-string', () => ({ + parse: jest.fn(() => ({})), +})) + +jest.mock('react-redux', () => ({ + useSelector: (selector) => + selector({ map: mockMapState, savedMap: mockSavedMapState }), +})) + +const dirtyMap = { id: 'map1', mapViews: [{ id: 'layer1', opacity: 0.5 }] } +const savedMap = { id: 'map1', mapViews: [{ id: 'layer1', opacity: 1 }] } + +describe('useUnsavedChangesGuard', () => { + beforeEach(() => { + mockMapState = savedMap + mockSavedMapState = savedMap + }) + + it('isDirtyNow reflects the current map vs savedMap comparison', () => { + mockMapState = dirtyMap + const { result } = renderHook(() => useUnsavedChangesGuard(jest.fn())) + + expect(result.current.isDirtyNow()).toBe(true) + }) + + it('isDirtyNow is false when the map matches the saved snapshot', () => { + const { result } = renderHook(() => useUnsavedChangesGuard(jest.fn())) + + expect(result.current.isDirtyNow()).toBe(false) + }) + + it('confirmLeave calls loadLocation with the pending location and clears it', () => { + const loadLocation = jest.fn() + const { result } = renderHook(() => + useUnsavedChangesGuard(loadLocation) + ) + + act(() => { + result.current.setLocationToConfirm({ pathname: '/map2' }) + }) + expect(result.current.locationToConfirm).toEqual({ + pathname: '/map2', + }) + + act(() => { + result.current.confirmLeave() + }) + + expect(loadLocation).toHaveBeenCalledWith({ pathname: '/map2' }) + expect(result.current.locationToConfirm).toBeNull() + }) + + it('confirmLeave is a no-op when there is nothing pending', () => { + const loadLocation = jest.fn() + const { result } = renderHook(() => + useUnsavedChangesGuard(loadLocation) + ) + + act(() => { + result.current.confirmLeave() + }) + + expect(loadLocation).not.toHaveBeenCalled() + }) + + it('cancelLeave clears the pending location and navigates back', async () => { + const { result } = renderHook(() => useUnsavedChangesGuard(jest.fn())) + + act(() => { + history.push('/before-cancel') + history.push('/map2') + }) + expect(history.location.pathname).toBe('/map2') + + act(() => { + result.current.setLocationToConfirm({ pathname: '/map2' }) + }) + + act(() => { + result.current.cancelLeave() + }) + + expect(result.current.locationToConfirm).toBeNull() + // history.back() navigates via the browser's popstate mechanism, + // which resolves asynchronously rather than within this act() call. + await waitFor(() => + expect(history.location.pathname).toBe('/before-cancel') + ) + }) + + it('prevents the tab from closing via beforeunload when dirty', () => { + mockMapState = dirtyMap + renderHook(() => useUnsavedChangesGuard(jest.fn())) + + const event = new Event('beforeunload', { cancelable: true }) + act(() => { + window.dispatchEvent(event) + }) + + expect(event.defaultPrevented).toBe(true) + }) + + it('does not block beforeunload when not dirty', () => { + renderHook(() => useUnsavedChangesGuard(jest.fn())) + + const event = new Event('beforeunload', { cancelable: true }) + act(() => { + window.dispatchEvent(event) + }) + + expect(event.defaultPrevented).toBe(false) + }) +}) diff --git a/src/hooks/useMapDirty.js b/src/hooks/useMapDirty.js new file mode 100644 index 0000000000..f48e376117 --- /dev/null +++ b/src/hooks/useMapDirty.js @@ -0,0 +1,23 @@ +import { useEffect, useRef, useState } from 'react' +import { useSelector } from 'react-redux' +import { isMapDirty } from '../util/mapDirty.js' + +const DEBOUNCE_MS = 300 + +export const useMapDirty = () => { + const map = useSelector((state) => state.map) + const savedMap = useSelector((state) => state.savedMap) + const [dirty, setDirty] = useState(false) + const timeoutRef = useRef() + + useEffect(() => { + clearTimeout(timeoutRef.current) + timeoutRef.current = setTimeout( + () => setDirty(isMapDirty(map, savedMap)), + DEBOUNCE_MS + ) + return () => clearTimeout(timeoutRef.current) + }, [map, savedMap]) + + return dirty +} diff --git a/src/hooks/useUnsavedChangesGuard.js b/src/hooks/useUnsavedChangesGuard.js new file mode 100644 index 0000000000..2937451abd --- /dev/null +++ b/src/hooks/useUnsavedChangesGuard.js @@ -0,0 +1,53 @@ +import i18n from '@dhis2/d2-i18n' +import { useCallback, useEffect, useRef, useState } from 'react' +import { useSelector } from 'react-redux' +import history from '../util/history.js' +import { isMapDirty } from '../util/mapDirty.js' + +export const useUnsavedChangesGuard = (loadLocation) => { + const mapRef = useRef() + mapRef.current = useSelector((state) => state.map) + const savedMapRef = useRef() + savedMapRef.current = useSelector((state) => state.savedMap) + + const [locationToConfirm, setLocationToConfirm] = useState(null) + + const isDirtyNow = useCallback( + () => isMapDirty(mapRef.current, savedMapRef.current), + [] + ) + + useEffect(() => { + const onBeforeUnload = (event) => { + if (isDirtyNow()) { + event.preventDefault() + // Required for triggering the unload confirmation dialog + // See: https://developer.mozilla.org/en-US/docs/Web/API/Window/beforeunload_event + event.returnValue = i18n.t('You have unsaved changes.') + } + } + + window.addEventListener('beforeunload', onBeforeUnload) + return () => window.removeEventListener('beforeunload', onBeforeUnload) + }, [isDirtyNow]) + + const confirmLeave = useCallback(() => { + if (locationToConfirm) { + loadLocation(locationToConfirm) + setLocationToConfirm(null) + } + }, [locationToConfirm, loadLocation]) + + const cancelLeave = useCallback(() => { + setLocationToConfirm(null) + history.back() + }, []) + + return { + locationToConfirm, + setLocationToConfirm, + isDirtyNow, + confirmLeave, + cancelLeave, + } +} diff --git a/src/reducers/__tests__/savedMap.spec.js b/src/reducers/__tests__/savedMap.spec.js new file mode 100644 index 0000000000..4906bc6b32 --- /dev/null +++ b/src/reducers/__tests__/savedMap.spec.js @@ -0,0 +1,53 @@ +import * as types from '../../constants/actionTypes.js' +import savedMap from '../savedMap.js' + +describe('savedMap reducer', () => { + it('defaults to null', () => { + expect(savedMap(undefined, { type: '@@INIT' })).toBe(null) + }) + + it('MAP_SET replaces state with the payload', () => { + const payload = { id: 'abc', name: 'My map', mapViews: [] } + + expect(savedMap({ id: 'old' }, { type: types.MAP_SET, payload })).toBe( + payload + ) + }) + + it('MAP_NEW resets to null', () => { + const state = { id: 'abc', name: 'My map', mapViews: [] } + + expect(savedMap(state, { type: types.MAP_NEW })).toBe(null) + }) + + it('MAP_PROPS_SET merges props into the current snapshot', () => { + const state = { id: 'abc', name: 'Old name', mapViews: [] } + + const result = savedMap(state, { + type: types.MAP_PROPS_SET, + payload: { name: 'New name', displayName: 'New name' }, + }) + + expect(result).toEqual({ + id: 'abc', + name: 'New name', + displayName: 'New name', + mapViews: [], + }) + }) + + it('MAP_PROPS_SET is a no-op when there is no saved snapshot yet', () => { + expect( + savedMap(null, { + type: types.MAP_PROPS_SET, + payload: { name: 'New name' }, + }) + ).toBe(null) + }) + + it('returns the same state for unrelated actions', () => { + const state = { id: 'abc', mapViews: [] } + + expect(savedMap(state, { type: types.DATA_TABLE_TOGGLE })).toBe(state) + }) +}) diff --git a/src/reducers/index.js b/src/reducers/index.js index dae6948282..b3720ded7a 100644 --- a/src/reducers/index.js +++ b/src/reducers/index.js @@ -11,6 +11,7 @@ import layerEdit from './layerEdit.js' import layerSources from './layerSources.js' import map from './map.js' import orgUnitProfile from './orgUnitProfile.js' +import savedMap from './savedMap.js' import ui from './ui.js' export default combineReducers({ @@ -24,6 +25,7 @@ export default combineReducers({ layerSources, map, orgUnitProfile, + savedMap, ui, feature, featureProfile, diff --git a/src/reducers/savedMap.js b/src/reducers/savedMap.js new file mode 100644 index 0000000000..3db47630e3 --- /dev/null +++ b/src/reducers/savedMap.js @@ -0,0 +1,16 @@ +import * as types from '../constants/actionTypes.js' + +const savedMap = (state = null, action) => { + switch (action.type) { + case types.MAP_NEW: + return null + case types.MAP_SET: + return action.payload + case types.MAP_PROPS_SET: + return state ? { ...state, ...action.payload } : state + default: + return state + } +} + +export default savedMap diff --git a/src/util/__tests__/mapDirty.spec.js b/src/util/__tests__/mapDirty.spec.js new file mode 100644 index 0000000000..fc39ad909a --- /dev/null +++ b/src/util/__tests__/mapDirty.spec.js @@ -0,0 +1,122 @@ +import { isMapDirty } from '../mapDirty.js' + +const baseMap = () => ({ + id: 'abc', + name: 'My map', + basemap: { id: 'osm', opacity: 1, isVisible: true }, + mapViews: [{ id: 'layer1', opacity: 1, isVisible: true }], +}) + +describe('isMapDirty', () => { + it('is false when there is no saved snapshot yet', () => { + expect(isMapDirty(baseMap(), null)).toBe(false) + }) + + it('is false for identical maps', () => { + expect(isMapDirty(baseMap(), baseMap())).toBe(false) + }) + + it('is true when a layer is added', () => { + const map = baseMap() + map.mapViews = [...map.mapViews, { id: 'layer2', opacity: 1 }] + + expect(isMapDirty(map, baseMap())).toBe(true) + }) + + it('is true when a layer is removed', () => { + const map = baseMap() + map.mapViews = [] + + expect(isMapDirty(map, baseMap())).toBe(true) + }) + + it('is true when the basemap changes', () => { + const map = baseMap() + map.basemap = { ...map.basemap, id: 'terrain' } + + expect(isMapDirty(map, baseMap())).toBe(true) + }) + + it('is true when a layer opacity changes', () => { + const map = baseMap() + map.mapViews = [{ ...map.mapViews[0], opacity: 0.5 }] + + expect(isMapDirty(map, baseMap())).toBe(true) + }) + + it('ignores ephemeral map-level fields', () => { + const map = baseMap() + map.coordinatePopup = { lat: 1, lng: 2 } + map.alerts = [{ message: 'oops' }] + + expect(isMapDirty(map, baseMap())).toBe(false) + }) + + it('ignores ephemeral layer-level fields', () => { + const map = baseMap() + map.mapViews = [ + { + ...map.mapViews[0], + isLoading: true, + coordinate: { lat: 1, lng: 2 }, + dataFilters: { field: 'x' }, + isExpanded: false, + alerts: [{ message: 'oops' }], + }, + ] + + expect(isMapDirty(map, baseMap())).toBe(false) + }) + + it('ignores ephemeral basemap fields', () => { + const map = baseMap() + map.basemap = { ...map.basemap, isExpanded: false } + + expect(isMapDirty(map, baseMap())).toBe(false) + }) + + it('ignores a client-only bounds field present on map but absent on savedMap', () => { + const map = { + ...baseMap(), + bounds: [ + [-1, -1], + [1, 1], + ], + } + + expect(isMapDirty(map, baseMap())).toBe(false) + }) + + it('ignores loader-added fields right after a fresh load (regression)', () => { + const map = baseMap() + map.mapViews = [ + { + ...map.mapViews[0], + isLoaded: true, + isLoading: false, + loadError: null, + data: [{ type: 'Feature' }], + periods: [{ id: '2024' }], + valuesByPeriod: { 2024: [1, 2, 3] }, + legend: { items: [] }, + alerts: [{ message: 'partial data' }], + }, + ] + + expect(isMapDirty(map, baseMap())).toBe(false) + }) + + it('still catches a real content change on a loaded layer', () => { + const map = baseMap() + map.mapViews = [ + { + ...map.mapViews[0], + isLoaded: true, + data: [{ type: 'Feature' }], + opacity: 0.3, + }, + ] + + expect(isMapDirty(map, baseMap())).toBe(true) + }) +}) diff --git a/src/util/favorites.js b/src/util/favorites.js index 520c919873..1be72539d5 100644 --- a/src/util/favorites.js +++ b/src/util/favorites.js @@ -10,7 +10,7 @@ import { } from '../constants/layers.js' // TODO: get latitude, longitude, zoom from map + basemap: 'none' -const validMapProperties = [ +export const validMapProperties = [ // 'basemap' and 'basemaps' removed — set exclusively by getBasemapPayload 'id', 'latitude', @@ -25,7 +25,7 @@ const validMapProperties = [ 'subscribers', ] -const validLayerProperties = [ +export const validLayerProperties = [ 'aggregationType', 'areaRadius', 'geometryCentroid', diff --git a/src/util/mapDirty.js b/src/util/mapDirty.js new file mode 100644 index 0000000000..ce268f8eb9 --- /dev/null +++ b/src/util/mapDirty.js @@ -0,0 +1,18 @@ +import { isEqual, pick } from 'lodash' +import { validLayerProperties, validMapProperties } from './favorites.js' + +// Loaders replace a layer's config with `{ ...config, ...loadedFields }` on +// every load, so compare only the saved-payload allowlist, not raw objects. +const layerFields = [...validLayerProperties, 'isVisible'] +const basemapFields = ['id', 'opacity', 'isVisible'] + +const stripLayer = (layer) => pick(layer, layerFields) + +const stripMap = (map) => ({ + ...pick(map, validMapProperties), + basemap: pick(map.basemap, basemapFields), + mapViews: map.mapViews.map(stripLayer), +}) + +export const isMapDirty = (map, savedMap) => + !!savedMap && !isEqual(stripMap(map), stripMap(savedMap))