From b9aaff7e59cfa976b2b4b436665b7579b2121e62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20K=C3=BCnzi?= Date: Mon, 17 Nov 2025 19:15:16 +0100 Subject: [PATCH 1/6] PB-1383: initial values not included in external layers Issue: When loading an external WMS layer, the initial values (like the visibility or opacity) were not passed to the layer, which meant that on startup, we would automatically lose their initial state. Fix: We add the initial values as parameters to the `makeExternalWMSLayer` function. --- packages/layers/src/parsers/WMSCapabilitiesParser.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/layers/src/parsers/WMSCapabilitiesParser.ts b/packages/layers/src/parsers/WMSCapabilitiesParser.ts index 877ccfb0f7..9b95071bf7 100644 --- a/packages/layers/src/parsers/WMSCapabilitiesParser.ts +++ b/packages/layers/src/parsers/WMSCapabilitiesParser.ts @@ -548,6 +548,7 @@ function getExternalLayer( return layerUtils.makeExternalWMSLayer({ ...getLayerAttributes(capabilities, layer, parents ?? [], outputProjection, ignoreErrors), format: 'png', + ...initialValues, isLoading: false, getFeatureInfoCapability: getFeatureInfoCapability(capabilities, ignoreErrors), currentYear, From f2de2e83bf3b8ce787c735ba2baa3b0f7d74d21a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20K=C3=BCnzi?= Date: Tue, 25 Nov 2025 10:20:24 +0100 Subject: [PATCH 2/6] PB-1383: Avoid concurrency between app setup and URL parsing Issue: When starting the app for the first time, the URL sync plugin would start being active before the topics were fully set, causing (mainly) the background layer parameter to be ignored and replaced with the topic's default one Fix: The `ConfigLoaded` state now wait for the background layer to be set before being fulfilled. --- packages/viewer/src/store/modules/app/index.ts | 6 ++++-- .../src/store/plugins/storeSync/params/bgLayer.param.ts | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/viewer/src/store/modules/app/index.ts b/packages/viewer/src/store/modules/app/index.ts index 223bc96d57..46f6fbacf5 100644 --- a/packages/viewer/src/store/modules/app/index.ts +++ b/packages/viewer/src/store/modules/app/index.ts @@ -30,6 +30,7 @@ const mapShown: AppState = { const ready: AppState = { name: AppStateNames.Ready, isFulfilled: () => { + console.error('ARE WE HERE AT SOME POINT ?') const mapStore = useMapStore() const cesiumStore = useCesiumStore() if (cesiumStore.active) { @@ -52,10 +53,11 @@ const initiateUrlParsing: AppState = { const parseLegacyUrlParams: AppState = { name: AppStateNames.LegacyParsing, - // legacyUrlParsingHasHappened is necessary to reevaluate after the legacy parsing has happened, without it, + // legacyUrlParsingHasHappened is necessary to reevaluate after the legacy parsing has happened, without it, // isFulfilled would always return false/true after the first time // it also has to be the first condition because the && operator is short-circuiting isFulfilled: () => useAppStore().legacyUrlParsingHasHappened && !isLegacyParams(window?.location?.search), + next: () => { return initiateUrlParsing }, @@ -63,7 +65,7 @@ const parseLegacyUrlParams: AppState = { const configLoaded: AppState = { name: AppStateNames.ConfigLoaded, - isFulfilled: () => true, // there's always a topic set, so no need to check if topicStore.current is defined + isFulfilled: () => true, next: () => { if (isLegacyParams(window?.location?.search)) { return parseLegacyUrlParams diff --git a/packages/viewer/src/store/plugins/storeSync/params/bgLayer.param.ts b/packages/viewer/src/store/plugins/storeSync/params/bgLayer.param.ts index be7586303e..485707b3c3 100644 --- a/packages/viewer/src/store/plugins/storeSync/params/bgLayer.param.ts +++ b/packages/viewer/src/store/plugins/storeSync/params/bgLayer.param.ts @@ -34,7 +34,7 @@ const backgroundLayerParamConfig = new UrlParamConfig({ queryValue, !!queryValue && (queryValue === 'void' || - useLayersStore().backgroundLayers?.some((layer) => layer.id == queryValue)), + useLayersStore().backgroundLayers?.some((layer) => layer.id === queryValue)), 'bgLayer' ), }) From 2cbd4c7946f4c6c8d8d4f87d7eb6fd61136c05b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20K=C3=BCnzi?= Date: Fri, 28 Nov 2025 10:26:35 +0100 Subject: [PATCH 3/6] adding tests small modifications to the loading of topics, there was a case where it would refuse to set up the default background on startup --- .../viewer/src/store/modules/app/index.ts | 3 +- .../store/modules/topics/actions/loadTopic.ts | 11 ++-- .../plugins/storeSync/params/bgLayer.param.ts | 5 +- .../tests/cypress/tests-e2e/layers.cy.ts | 52 ++++++++++--------- 4 files changed, 36 insertions(+), 35 deletions(-) diff --git a/packages/viewer/src/store/modules/app/index.ts b/packages/viewer/src/store/modules/app/index.ts index 46f6fbacf5..8e0774fa0a 100644 --- a/packages/viewer/src/store/modules/app/index.ts +++ b/packages/viewer/src/store/modules/app/index.ts @@ -30,7 +30,6 @@ const mapShown: AppState = { const ready: AppState = { name: AppStateNames.Ready, isFulfilled: () => { - console.error('ARE WE HERE AT SOME POINT ?') const mapStore = useMapStore() const cesiumStore = useCesiumStore() if (cesiumStore.active) { @@ -65,7 +64,7 @@ const parseLegacyUrlParams: AppState = { const configLoaded: AppState = { name: AppStateNames.ConfigLoaded, - isFulfilled: () => true, + isFulfilled: () => true, // there's always a topic set, so no need to check if topicStore.current is defined next: () => { if (isLegacyParams(window?.location?.search)) { return parseLegacyUrlParams diff --git a/packages/viewer/src/store/modules/topics/actions/loadTopic.ts b/packages/viewer/src/store/modules/topics/actions/loadTopic.ts index fe17db62c2..aaf76ef020 100644 --- a/packages/viewer/src/store/modules/topics/actions/loadTopic.ts +++ b/packages/viewer/src/store/modules/topics/actions/loadTopic.ts @@ -24,13 +24,12 @@ export default function loadTopic( if (options.openGeocatalogSection) { this.setTopicTreeOpenedThemesIds([this.current, ...topicTree.itemIdToOpen], dispatcher) } + if (this.currentTopic.defaultBackgroundLayer) { + layersStore.setBackground(this.currentTopic.defaultBackgroundLayer.id, dispatcher) + } else if (options.changeLayers) { + layersStore.setBackground(undefined, dispatcher) + } if (options.changeLayers) { - if (this.currentTopic.defaultBackgroundLayer) { - layersStore.setBackground(this.currentTopic.defaultBackgroundLayer.id, dispatcher) - } else { - layersStore.setBackground(undefined, dispatcher) - } - if (this.currentTopic.layersToActivate) { layersStore.setLayers(this.currentTopic.layersToActivate, dispatcher) } diff --git a/packages/viewer/src/store/plugins/storeSync/params/bgLayer.param.ts b/packages/viewer/src/store/plugins/storeSync/params/bgLayer.param.ts index 485707b3c3..54efc15692 100644 --- a/packages/viewer/src/store/plugins/storeSync/params/bgLayer.param.ts +++ b/packages/viewer/src/store/plugins/storeSync/params/bgLayer.param.ts @@ -5,6 +5,7 @@ import UrlParamConfig, { STORE_DISPATCHER_ROUTER_PLUGIN, } from '@/store/plugins/storeSync/UrlParamConfig.class' import { getDefaultValidationResponse } from '@/store/plugins/storeSync/validation' +import useTopicsStore from '@/store/modules/topics' const backgroundLayerParamConfig = new UrlParamConfig({ urlParamName: 'bgLayer', @@ -21,10 +22,10 @@ const backgroundLayerParamConfig = new UrlParamConfig({ }, setValuesInStore: (_: RouteLocationNormalizedGeneric, urlParamValue?: string) => { const layersStore = useLayersStore() - if (urlParamValue && urlParamValue !== 'void') { + if (urlParamValue) { layersStore.setBackground(urlParamValue, STORE_DISPATCHER_ROUTER_PLUGIN) } else { - layersStore.setBackground(undefined, STORE_DISPATCHER_ROUTER_PLUGIN) + layersStore.setBackground(useTopicsStore().currentTopic?.defaultBackgroundLayer?.id, STORE_DISPATCHER_ROUTER_PLUGIN) } }, keepInUrlWhenDefault: true, diff --git a/packages/viewer/tests/cypress/tests-e2e/layers.cy.ts b/packages/viewer/tests/cypress/tests-e2e/layers.cy.ts index 0ae3e24306..855ae92542 100644 --- a/packages/viewer/tests/cypress/tests-e2e/layers.cy.ts +++ b/packages/viewer/tests/cypress/tests-e2e/layers.cy.ts @@ -119,10 +119,10 @@ describe('Test of layer handling', () => { cy.getPinia().then((pinia) => { const layersStore3 = useLayersStore(pinia) const layers = layersStore3.visibleLayers - const [timeEnabledLayer] = layers + const timeEnabledLayer: Layer | undefined = layers[0] cy.fixture('layers.fixture.json').then((layersMetadata) => { const timeEnabledLayerMetadata = layersMetadata[timeEnabledLayerId] - expect(timeEnabledLayer.timeConfig.currentTimestamp).to.eq( + expect(timeEnabledLayer?.timeConfig.currentTimeEntry?.timestamp).to.eq( timeEnabledLayerMetadata.timeBehaviour ) }) @@ -146,7 +146,7 @@ describe('Test of layer handling', () => { const layersStore4 = useLayersStore(pinia) const layers = layersStore4.visibleLayers const [timeEnabledLayer] = layers - expect(timeEnabledLayer.timeConfig.currentTimestamp).to.eq( + expect(timeEnabledLayer?.timeConfig.currentTimeEntry?.timestamp).to.eq( randomTimestampFromLayer ) }) @@ -155,9 +155,9 @@ describe('Test of layer handling', () => { }) }) context('External layers', () => { - it('reads and adds an external WMS correctly', () => { + it.only('reads and adds an external WMS correctly', () => { cy.getExternalWmsMockConfig().then((layerObjects) => { - layerObjects.forEach((layerObject) => (layerObject.visible = true)) + layerObjects.forEach((layerObject) => (layerObject.isVisible = true)) const [mockExternalWms1, mockExternalWms2, mockExternalWms3, mockExternalWms4] = layerObjects /** @@ -170,7 +170,7 @@ describe('Test of layer handling', () => { "Adding an option to one of the layer's base URL to check if these calls behave in a correct way" ) - layerObjects[1]!.baseUrl = layerObjects[0]!.baseUrl + 'item=22_06_86t13214' + layerObjects[0]!.baseUrl = layerObjects[0]!.baseUrl + 'item=22_06_86t13214' const layers = layerObjects .map((object) => transformLayerIntoUrlString(object, undefined, undefined)) .join(';') @@ -185,7 +185,7 @@ describe('Test of layer handling', () => { cy.log(`Verify that extra custom attributes are passed along to the WMS server`) cy.wait(`@externalWMS-GetMap-${mockExternalWms1?.id}`) .its('request.query') - .should('have.property', 'item', 'MyItem') + .should('have.property', 'item', '22_06_86t13214') cy.log(`Verify that the active layers store match the url input`) cy.getPinia().then((pinia) => { @@ -202,7 +202,7 @@ describe('Test of layer handling', () => { expect(activeLayers2[index]?.baseUrl).to.be.eq(layer.baseUrl) expect(activeLayers2[index]?.name).to.be.eq(layer.name) expect(activeLayers2[index]?.wmsVersion).to.be.eq(layer.wmsVersion) - expect(activeLayers2[index]?.visible).to.eq(layer.visible) + expect(activeLayers2[index]?.isVisible).to.eq(layer.isVisible) expect(activeLayers2[index]?.opacity).to.eq(layer.opacity) }) }) @@ -225,7 +225,7 @@ describe('Test of layer handling', () => { ...layerObjects.map((layer) => { return { id: layer.id, - visible: layer.visible, + visible: layer.isVisible, opacity: layer.opacity, } }), @@ -330,8 +330,7 @@ describe('Test of layer handling', () => { }) it('reads and adds an external WMTS correctly', () => { cy.getExternalWmtsMockConfig().then((layerObjects) => { - // @ts-expect-error : the layers have the 'visible' property set in AbstractLayers, but I can't see it in ExternalWMTS layers - layerObjects.forEach((layerObject) => (layerObject.visible = true)) + layerObjects.forEach((layerObject) => (layerObject.isVisible = true)) const [mockExternalWmts1, _, mockExternalWmts3] = layerObjects cy.goToMapView({ @@ -396,10 +395,10 @@ describe('Test of layer handling', () => { cy.getExternalWmtsMockConfig().then((layerObjects) => { const [mockExternalWmts1, mockExternalWmts2, mockExternalWmts3] = layerObjects - mockExternalWmts1!.visible = false + mockExternalWmts1!.isVisible = false mockExternalWmts1!.opacity = 0.5 - mockExternalWmts2!.visible = false - mockExternalWmts3!.visible = true + mockExternalWmts2!.isVisible = false + mockExternalWmts3!.isVisible = true mockExternalWmts3!.opacity = 0.8 const layerObjects2 = [ mockExternalWmts1!, @@ -451,7 +450,7 @@ describe('Test of layer handling', () => { ...layerObjects2.map((layer) => { return { id: layer.id, - visible: layer.visible, + visible: layer.isVisible, opacity: layer.opacity, } }), @@ -669,12 +668,12 @@ describe('Test of layer handling', () => { }) }) it('sets the background according to the URL param if present at startup', () => { - cy.goToMapView({ queryParams: { bgLayer: 'test.background.layer2' } }) + cy.goToMapView({ queryParams: { bgLayer: 'test.background.layer' } }) cy.getPinia().then((pinia) => { const layersStore15 = useLayersStore(pinia) const bgLayer3 = layersStore15.currentBackgroundLayer expect(bgLayer3).to.not.be.undefined - expect(bgLayer3?.id).to.eq('test.background.layer2') + expect(bgLayer3?.id).to.eq('test.background.layer') }) }) }) @@ -946,7 +945,7 @@ describe('Test of layer handling', () => { cy.clickOnLanguage('de') cy.wait('@legendGerman') cy.get('@legend.all').should('have.length', legendCalls) - + // ISSUE HERE : when we call SET TOPIC through the language, we revert the layers to their default state cy.get('[data-cy="layer-description"]').should('be.visible').contains(germanText) }) }) @@ -986,11 +985,12 @@ describe('Test of layer handling', () => { expect(activeLayers5).to.be.an('Array').length(visibleLayerIds.length) const layer3 = activeLayers5.find((layer: Layer) => layer.id === timedLayerId) expect(layer3).not.to.be.undefined - expect(layer3!.timeConfig.currentTimestamp).to.eq(timestamp) + expect(layer3!.timeConfig.currentTimeEntry?.timestamp).to.eq(timestamp) }) //--------------------------------------------------------------------------------- cy.log('keep timestamp configuration when the language changes') + // ISSUE, AS ABOVE : changing language resets layers cy.clickOnLanguage('fr') cy.get('[data-cy="menu-active-layers"]:visible').should('be.visible').click() cy.get('[data-cy="time-selector-test.timeenabled.wmts.layer-2"]:visible').contains( @@ -1033,7 +1033,7 @@ describe('Test of layer handling', () => { .to.be.an('Array') .length(visibleLayerIds.length + 1) expect(activeLayers6[3]).not.to.be.undefined - expect(activeLayers6[3]!.timeConfig.currentTimestamp).to.eq(timestamp) + expect(activeLayers6[3]!.timeConfig.currentTimeEntry?.timestamp).to.eq(timestamp) expect(activeLayers6[3]?.isVisible).to.be.true expect(activeLayers6[3]?.opacity).to.eq(0) }) @@ -1081,13 +1081,13 @@ describe('Test of layer handling', () => { .length(visibleLayerIds.length + 1) assertDefined(activeLayers7[3]) - expect(activeLayers7[3].timeConfig.currentTimestamp).to.eq(newTimestamp) + expect(activeLayers7[3].timeConfig.currentTimeEntry?.timestamp).to.eq(newTimestamp) expect(activeLayers7[3]?.isVisible).to.be.false expect(activeLayers7[3]?.opacity).to.eq(0.5) assertDefined(activeLayers7[2]) expect(activeLayers7[2]).not.to.be.undefined - expect(activeLayers7[2].timeConfig.currentTimestamp).to.eq(timestamp) + expect(activeLayers7[2].timeConfig.currentTimeEntry?.timestamp).to.eq(timestamp) expect(activeLayers7[2]?.isVisible).to.be.true expect(activeLayers7[2]?.opacity).to.eq(0) }) @@ -1234,12 +1234,12 @@ describe('Test of layer handling', () => { expect(activeLayers8).to.be.an('Array').length(4) assertDefined(activeLayers8[3]) - expect(activeLayers8[3].timeConfig.currentTimestamp).to.eq('20180101') + expect(activeLayers8[3].timeConfig.currentTimeEntry?.timestamp).to.eq('20180101') expect(activeLayers8[3]?.isVisible).to.be.true expect(activeLayers8[3]?.opacity).to.eq(0.7) assertDefined(activeLayers8[0]) - expect(activeLayers8[0].timeConfig.currentTimestamp).to.eq(newTimestamp) + expect(activeLayers8[0].timeConfig.currentTimeEntry?.timestamp).to.eq(newTimestamp) expect(activeLayers8[0]?.isVisible).to.be.true expect(activeLayers8[0]?.opacity).to.eq(0) }) @@ -1252,6 +1252,7 @@ describe('Test of layer handling', () => { ]) }) it('reorder layers when they are drag and dropped', () => { + const [bottomLayerId, middleLayerId, topLayerId] = visibleLayerIds cy.get(`[data-cy^="menu-active-layer-${bottomLayerId}-"]`) .should('be.visible') @@ -1299,6 +1300,7 @@ describe('Test of layer handling', () => { }) context('Language settings in menu', () => { it('keeps the layer settings when changing language', () => { + // ISSUE HERE const langBefore = 'en' const langAfter = 'de' const visibleLayerIds = [ @@ -1437,7 +1439,7 @@ describe('Test of layer handling', () => { }) }) context('Custom url attributes', () => { - it('Keep custom attributes when changing language', () => { + it('Keeps custom attributes when changing language', () => { cy.goToMapView({ queryParams: { lang: 'fr' } }) cy.wait(['@routeChange', '@layerConfig', '@topics', '@topic-ech']) cy.goToMapView({ From 7c1aca27e3591a9c9bbee6bf0888fdfb80e3681d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20K=C3=BCnzi?= Date: Mon, 1 Dec 2025 11:56:26 +0100 Subject: [PATCH 4/6] PB-1383: adding a failsafe around optionsFromCapabilities, as it could crash and stops a layer from loading --- packages/layers/src/parsers/WMTSCapabilitiesParser.ts | 1 - packages/viewer/src/store/modules/app/index.ts | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/layers/src/parsers/WMTSCapabilitiesParser.ts b/packages/layers/src/parsers/WMTSCapabilitiesParser.ts index 48649de44b..3fc43ca1f7 100644 --- a/packages/layers/src/parsers/WMTSCapabilitiesParser.ts +++ b/packages/layers/src/parsers/WMTSCapabilitiesParser.ts @@ -430,7 +430,6 @@ function getExternalLayer( } const layer = getCapabilitiesLayer(capabilities, layerId) - if (!layer) { const msg = `No WMTS layer ${layerId} found in Capabilities ${capabilities.originUrl.toString()}` log.error({ diff --git a/packages/viewer/src/store/modules/app/index.ts b/packages/viewer/src/store/modules/app/index.ts index 8e0774fa0a..99e1197053 100644 --- a/packages/viewer/src/store/modules/app/index.ts +++ b/packages/viewer/src/store/modules/app/index.ts @@ -64,7 +64,7 @@ const parseLegacyUrlParams: AppState = { const configLoaded: AppState = { name: AppStateNames.ConfigLoaded, - isFulfilled: () => true, // there's always a topic set, so no need to check if topicStore.current is defined + isFulfilled: () => useTopicsStore().currentTopic?.defaultBackgroundLayer?.id === useLayersStore().currentBackgroundLayer?.id, next: () => { if (isLegacyParams(window?.location?.search)) { return parseLegacyUrlParams From 5d298681d621d84e3df53023a5df42495ebb4854 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20K=C3=BCnzi?= Date: Tue, 2 Dec 2025 09:23:39 +0100 Subject: [PATCH 5/6] PB-1383: layer tests - some more tests coverage corrections - removal of some useless comments - adding some comments to tell people why some changes have happened --- .../components/topics/MenuTopicSection.vue | 5 +- .../viewer/src/store/modules/app/index.ts | 2 + .../plugins/storeSync/params/bgLayer.param.ts | 6 ++- .../viewer/tests/cypress/support/commands.ts | 20 +++---- .../tests/cypress/tests-e2e/layers.cy.ts | 53 +++++++++---------- 5 files changed, 44 insertions(+), 42 deletions(-) diff --git a/packages/viewer/src/modules/menu/components/topics/MenuTopicSection.vue b/packages/viewer/src/modules/menu/components/topics/MenuTopicSection.vue index cd465a14eb..34fb518eee 100644 --- a/packages/viewer/src/modules/menu/components/topics/MenuTopicSection.vue +++ b/packages/viewer/src/modules/menu/components/topics/MenuTopicSection.vue @@ -37,8 +37,9 @@ const showTopicTree = computed(() => { // // We only want the topic tree open whenever the user has chosen a different topic // // than the default one (it can be opened by the user by a click on it, but by default it's closed) // // If we have defined catalog themes to be opened in the URL, it makes sense to open the catalog - // return !isDefaultTopic.value - return topicsStore.openedTreeThemesIds.includes(currentTopic.value) + return ( + topicsStore.openedTreeThemesIds.includes(currentTopic.value) && !topicsStore.isDefaultTopic + ) }) const mapModuleReady = computed(() => appStore.isMapReady) diff --git a/packages/viewer/src/store/modules/app/index.ts b/packages/viewer/src/store/modules/app/index.ts index 99e1197053..cb27e1c0b4 100644 --- a/packages/viewer/src/store/modules/app/index.ts +++ b/packages/viewer/src/store/modules/app/index.ts @@ -64,6 +64,8 @@ const parseLegacyUrlParams: AppState = { const configLoaded: AppState = { name: AppStateNames.ConfigLoaded, + // we wait for the background layer to be set to the current topic default, to avoid conflicts between the mutation happening, + // and the URL synchronization. isFulfilled: () => useTopicsStore().currentTopic?.defaultBackgroundLayer?.id === useLayersStore().currentBackgroundLayer?.id, next: () => { if (isLegacyParams(window?.location?.search)) { diff --git a/packages/viewer/src/store/plugins/storeSync/params/bgLayer.param.ts b/packages/viewer/src/store/plugins/storeSync/params/bgLayer.param.ts index 54efc15692..aaf38dc337 100644 --- a/packages/viewer/src/store/plugins/storeSync/params/bgLayer.param.ts +++ b/packages/viewer/src/store/plugins/storeSync/params/bgLayer.param.ts @@ -1,11 +1,11 @@ import type { RouteLocationNormalizedGeneric } from 'vue-router' import useLayersStore from '@/store/modules/layers' +import useTopicsStore from '@/store/modules/topics' import UrlParamConfig, { STORE_DISPATCHER_ROUTER_PLUGIN, } from '@/store/plugins/storeSync/UrlParamConfig.class' import { getDefaultValidationResponse } from '@/store/plugins/storeSync/validation' -import useTopicsStore from '@/store/modules/topics' const backgroundLayerParamConfig = new UrlParamConfig({ urlParamName: 'bgLayer', @@ -22,10 +22,12 @@ const backgroundLayerParamConfig = new UrlParamConfig({ }, setValuesInStore: (_: RouteLocationNormalizedGeneric, urlParamValue?: string) => { const layersStore = useLayersStore() + const topicsStore = useTopicsStore() + if (urlParamValue) { layersStore.setBackground(urlParamValue, STORE_DISPATCHER_ROUTER_PLUGIN) } else { - layersStore.setBackground(useTopicsStore().currentTopic?.defaultBackgroundLayer?.id, STORE_DISPATCHER_ROUTER_PLUGIN) + layersStore.setBackground(topicsStore.currentTopic?.defaultBackgroundLayer?.id, STORE_DISPATCHER_ROUTER_PLUGIN) } }, keepInUrlWhenDefault: true, diff --git a/packages/viewer/tests/cypress/support/commands.ts b/packages/viewer/tests/cypress/support/commands.ts index 659a3233b6..9aa4eb59a3 100644 --- a/packages/viewer/tests/cypress/support/commands.ts +++ b/packages/viewer/tests/cypress/support/commands.ts @@ -1,7 +1,7 @@ import 'cypress-real-events' import 'cypress-wait-until' import '@4tw/cypress-drag-drop' -import type { GeoAdminLayer } from '@swissgeo/layers' +import type { GeoAdminLayer, Layer } from '@swissgeo/layers' import type { Layer as OLLayer } from 'ol/layer' import type { Pinia } from 'pinia' @@ -427,7 +427,6 @@ Cypress.Commands.add('openLayerSettings', (layerId) => { export interface PartialLayer { id: string opacity?: number - visible?: boolean // the Layers interface has 'isVisible' as a boolean, I'll make the 'visible' to 'isVisible' change in another commit so I'm not destroying everything isVisible?: boolean } @@ -445,7 +444,7 @@ Cypress.Commands.add('checkOlLayer', (args) => { } }) } else if (typeof args === 'string') { - layers.push({ id: args, visible: true, opacity: 1 }) + layers.push({ id: args, isVisible: true, opacity: 1 }) } else { layers.push(Cypress._.cloneDeep(args)) } @@ -453,16 +452,16 @@ Cypress.Commands.add('checkOlLayer', (args) => { if (!l.id) { throw new Error(`Invalid layer object ${JSON.stringify(l)}: don't have an id`) } - if (l.visible === undefined) { - l.visible = true + if (l.isVisible === undefined) { + l.isVisible = true } if (l.opacity === undefined) { l.opacity = 1 } return l }) - const visibleLayers: PartialLayer[] = layers.filter((l) => l.visible) - const invisibleLayers: PartialLayer[] = layers.filter((l) => !l.visible) + const visibleLayers: PartialLayer[] = layers.filter((l) => l.isVisible) + const invisibleLayers: PartialLayer[] = layers.filter((l) => !l.isVisible) cy.window() .its('map') .invoke('getAllLayers') @@ -495,7 +494,7 @@ Cypress.Commands.add('checkOlLayer', (args) => { expect(olLayer, `[${layer.id}] layer at index ${index} not found`).not.to.be .undefined expect(olLayer!.getVisible(), `[${layer.id}] layer.isVisible`).to.be.equal( - layer.visible + layer.isVisible ) expect(olLayer!.getOpacity(), `[${layer.id}] layer.opacity`).to.be.equal( layer.opacity @@ -504,7 +503,10 @@ Cypress.Commands.add('checkOlLayer', (args) => { // Also, the rendered flag is protected, so we're checking if it is set with a getRenderSource().getState() // function, which returns false as long as either there is no renderer, or the rendered // flag is false - cy.waitUntil(() => olLayer?.getRenderSource()?.getState() === 'ready', { + cy.waitUntil(() => { + return olLayer?.getRenderSource()?.getState() === 'ready' + } + , { description: `[${layer.id}] waitUntil layer.rendered`, errorMsg: `[${layer.id}] layer.rendered is not true`, }) diff --git a/packages/viewer/tests/cypress/tests-e2e/layers.cy.ts b/packages/viewer/tests/cypress/tests-e2e/layers.cy.ts index 855ae92542..88f22cae1b 100644 --- a/packages/viewer/tests/cypress/tests-e2e/layers.cy.ts +++ b/packages/viewer/tests/cypress/tests-e2e/layers.cy.ts @@ -146,7 +146,7 @@ describe('Test of layer handling', () => { const layersStore4 = useLayersStore(pinia) const layers = layersStore4.visibleLayers const [timeEnabledLayer] = layers - expect(timeEnabledLayer?.timeConfig.currentTimeEntry?.timestamp).to.eq( + expect(timeEnabledLayer!.timeConfig.currentTimeEntry?.timestamp).to.eq( randomTimestampFromLayer ) }) @@ -155,7 +155,7 @@ describe('Test of layer handling', () => { }) }) context('External layers', () => { - it.only('reads and adds an external WMS correctly', () => { + it('reads and adds an external WMS correctly', () => { cy.getExternalWmsMockConfig().then((layerObjects) => { layerObjects.forEach((layerObject) => (layerObject.isVisible = true)) const [mockExternalWms1, mockExternalWms2, mockExternalWms3, mockExternalWms4] = @@ -169,8 +169,8 @@ describe('Test of layer handling', () => { cy.log( "Adding an option to one of the layer's base URL to check if these calls behave in a correct way" ) - - layerObjects[0]!.baseUrl = layerObjects[0]!.baseUrl + 'item=22_06_86t13214' + const myItem = '22_06_86t13214' + layerObjects[1]!.baseUrl = layerObjects[1]!.baseUrl + `item=${myItem}` const layers = layerObjects .map((object) => transformLayerIntoUrlString(object, undefined, undefined)) .join(';') @@ -183,9 +183,9 @@ describe('Test of layer handling', () => { ]) cy.log(`Verify that extra custom attributes are passed along to the WMS server`) - cy.wait(`@externalWMS-GetMap-${mockExternalWms1?.id}`) - .its('request.query') - .should('have.property', 'item', '22_06_86t13214') + cy.wait(`@externalWMS-GetMap-${mockExternalWms2?.id}`) + + .its('request.query').should('have.property', 'item', myItem) cy.log(`Verify that the active layers store match the url input`) cy.getPinia().then((pinia) => { @@ -219,13 +219,13 @@ describe('Test of layer handling', () => { .eq(index) .should('contain', layer.name) }) - cy.checkOlLayer([ bgLayer, ...layerObjects.map((layer) => { return { id: layer.id, - visible: layer.isVisible, + + isVisible: layer.isVisible, opacity: layer.opacity, } }), @@ -239,13 +239,12 @@ describe('Test of layer handling', () => { cy.get( `[data-cy^="button-toggle-visibility-layer-${mockExternalWms3?.id}-"]` ).click() - cy.checkOlLayer([ bgLayer, - { id: mockExternalWms1!.id, visible: true, opacity: 1.0 }, - { id: mockExternalWms2!.id, visible: false, opacity: 0.8 }, - { id: mockExternalWms3!.id, visible: false, opacity: 1.0 }, - { id: mockExternalWms4!.id, visible: true, opacity: 0.4 }, + { id: mockExternalWms1!.id, isVisible: true, opacity: 1.0 }, + { id: mockExternalWms2!.id, isVisible: false, opacity: 0.8 }, + { id: mockExternalWms3!.id, isVisible: false, opacity: 1.0 }, + { id: mockExternalWms4!.id, isVisible: true, opacity: 0.4 }, ]) // A click on the map should trigger a getFeatureInfo on both visible/active layers 1 and 3. @@ -321,10 +320,10 @@ describe('Test of layer handling', () => { cy.checkOlLayer([ bgLayer, - { id: mockExternalWms1!.id, visible: true, opacity: 0.0 }, - { id: mockExternalWms2!.id, visible: false, opacity: 0.8 }, - { id: mockExternalWms3!.id, visible: false, opacity: 0.0 }, - { id: mockExternalWms4!.id, visible: true, opacity: 1.0 }, + { id: mockExternalWms1!.id, isVisible: true, opacity: 0.0 }, + { id: mockExternalWms2!.id, isVisible: false, opacity: 0.8 }, + { id: mockExternalWms3!.id, isVisible: false, opacity: 0.0 }, + { id: mockExternalWms4!.id, isVisible: true, opacity: 1.0 }, ]) }) }) @@ -359,7 +358,7 @@ describe('Test of layer handling', () => { layerObjects.forEach((layer, index) => { expect(visibleLayers2[index]?.id).to.be.eq(layer.id) expect(visibleLayers2[index]?.baseUrl).to.be.eq(layer.baseUrl) - expect(visibleLayers2[index]?.name).to.be.eq(layer.name) + expect(visibleLayers2[index]?.name).to.be.eq(layer.id) expect(visibleLayers2[index]?.isVisible).to.eq(layer.isVisible) expect(visibleLayers2[index]?.opacity).to.eq(layer.opacity) }) @@ -369,7 +368,7 @@ describe('Test of layer handling', () => { ...layerObjects.map((layer) => { return { id: layer.id, - visible: layer.isVisible, + isVisible: layer.isVisible, opacity: layer.opacity, } }), @@ -423,6 +422,7 @@ describe('Test of layer handling', () => { layerObjects2.forEach((layer, index) => { expect(activeLayers3[index]?.id).to.eq(layer.id) expect(activeLayers3[index]?.isVisible).to.eq(layer.isVisible) + expect(activeLayers3[index]?.name).to.eq(layer.name) expect(activeLayers3[index]?.opacity).to.eq(layer.opacity) }) }) @@ -444,13 +444,12 @@ describe('Test of layer handling', () => { .get('[data-cy="menu-external-disclaimer-icon-cloud"]') .should('be.visible') }) - cy.checkOlLayer([ bgLayer, ...layerObjects2.map((layer) => { return { id: layer.id, - visible: layer.isVisible, + isVisible: layer.isVisible, opacity: layer.opacity, } }), @@ -945,7 +944,6 @@ describe('Test of layer handling', () => { cy.clickOnLanguage('de') cy.wait('@legendGerman') cy.get('@legend.all').should('have.length', legendCalls) - // ISSUE HERE : when we call SET TOPIC through the language, we revert the layers to their default state cy.get('[data-cy="layer-description"]').should('be.visible').contains(germanText) }) }) @@ -990,7 +988,6 @@ describe('Test of layer handling', () => { //--------------------------------------------------------------------------------- cy.log('keep timestamp configuration when the language changes') - // ISSUE, AS ABOVE : changing language resets layers cy.clickOnLanguage('fr') cy.get('[data-cy="menu-active-layers"]:visible').should('be.visible').click() cy.get('[data-cy="time-selector-test.timeenabled.wmts.layer-2"]:visible').contains( @@ -1166,14 +1163,14 @@ describe('Test of layer handling', () => { bgLayer, { id: `${bottomLayerId}`, opacity: 0.75 }, { id: `${topLayerId}`, opacity: 0.7 }, - { id: `${middleLayerId}`, visible: false }, + { id: `${middleLayerId}`, isVisible: false }, ]) cy.get(`[data-cy^="button-toggle-visibility-layer-${middleLayerId}-"]`).click() cy.checkOlLayer([ bgLayer, { id: `${bottomLayerId}`, opacity: 0.75 }, { id: `${topLayerId}`, opacity: 0.7 }, - { id: `${middleLayerId}`, visible: true, opacity: 1 }, + { id: `${middleLayerId}`, isVisible: true, opacity: 1 }, ]) cy.log('Moving the layers and change the opacity') cy.log(`Moving ${middleLayerId} to the bottom and toggle it visibility`) @@ -1182,7 +1179,7 @@ describe('Test of layer handling', () => { cy.checkOlLayer([ bgLayer, { id: `${bottomLayerId}`, opacity: 0.75 }, - { id: `${middleLayerId}`, visible: true, opacity: 0.5 }, + { id: `${middleLayerId}`, isVisible: true, opacity: 0.5 }, { id: `${topLayerId}`, opacity: 0.7 }, ]) @@ -1252,7 +1249,6 @@ describe('Test of layer handling', () => { ]) }) it('reorder layers when they are drag and dropped', () => { - const [bottomLayerId, middleLayerId, topLayerId] = visibleLayerIds cy.get(`[data-cy^="menu-active-layer-${bottomLayerId}-"]`) .should('be.visible') @@ -1300,7 +1296,6 @@ describe('Test of layer handling', () => { }) context('Language settings in menu', () => { it('keeps the layer settings when changing language', () => { - // ISSUE HERE const langBefore = 'en' const langAfter = 'de' const visibleLayerIds = [ From ecda22857cc0e34d8b35d7b89c65cd7a687f95ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20K=C3=BCnzi?= Date: Tue, 2 Dec 2025 09:23:39 +0100 Subject: [PATCH 6/6] PB-1383: layer tests - some more tests coverage corrections - removal of some useless comments - adding some comments to tell people why some changes have happened PB-1383: adding proj4 registration to layers so openlayers supports layers with only swiss projections - Issue with the drag and drop reordering test: when it was called alone, it would pass, when it would be called after the one reordering layers with the buttons, it would fail because a menu popup would show which was not planned. Rather than place a "force" which could pass even if the element is somehow obscured for a non legitimate reason, I switched the order of the tests - Issue with the call of layer capabilities: We've changed the number of expected calls in the tests, as the capabilities are called once per layer, not once in total. If we need to rework the capabilities handling, we'll have to change this test once again. --- packages/layers/src/index.ts | 6 ++ .../tests/cypress/tests-e2e/layers.cy.ts | 77 +++++++++---------- 2 files changed, 44 insertions(+), 39 deletions(-) diff --git a/packages/layers/src/index.ts b/packages/layers/src/index.ts index faafea0d10..ec18f1aa8b 100644 --- a/packages/layers/src/index.ts +++ b/packages/layers/src/index.ts @@ -7,7 +7,13 @@ export * from '@/types/capabilities' export * from '@/types/timeConfig' export * from '@/validation' + import { register } from 'ol/proj/proj4' +/** + * We need this to register the Swiss projections within the openlayers library. + * + * If we don't, layers with only Swiss projections can't be rendered by openlayers. + */ registerProj4(proj4) register(proj4) diff --git a/packages/viewer/tests/cypress/tests-e2e/layers.cy.ts b/packages/viewer/tests/cypress/tests-e2e/layers.cy.ts index 88f22cae1b..ee47ca6e15 100644 --- a/packages/viewer/tests/cypress/tests-e2e/layers.cy.ts +++ b/packages/viewer/tests/cypress/tests-e2e/layers.cy.ts @@ -329,7 +329,6 @@ describe('Test of layer handling', () => { }) it('reads and adds an external WMTS correctly', () => { cy.getExternalWmtsMockConfig().then((layerObjects) => { - layerObjects.forEach((layerObject) => (layerObject.isVisible = true)) const [mockExternalWmts1, _, mockExternalWmts3] = layerObjects cy.goToMapView({ @@ -351,14 +350,14 @@ describe('Test of layer handling', () => { const layersStore6 = useLayersStore(pinia) const visibleLayers2 = layersStore6.visibleLayers expect(visibleLayers2).to.have.lengthOf(layerObjects.length) - visibleLayers2.forEach((layer) => { + visibleLayers2.forEach((layer: Layer) => { expect(layer.isLoading).to.be.false expect(layer.isExternal).to.be.true }) layerObjects.forEach((layer, index) => { expect(visibleLayers2[index]?.id).to.be.eq(layer.id) expect(visibleLayers2[index]?.baseUrl).to.be.eq(layer.baseUrl) - expect(visibleLayers2[index]?.name).to.be.eq(layer.id) + expect(visibleLayers2[index]?.name).to.be.eq(layer.name) expect(visibleLayers2[index]?.isVisible).to.eq(layer.isVisible) expect(visibleLayers2[index]?.opacity).to.eq(layer.opacity) }) @@ -454,10 +453,10 @@ describe('Test of layer handling', () => { } }), ]) - cy.log(`Make sure that the external backend have not been called twice`) + cy.log(`Make sure that the external backend have been called once per layer`) cy.get(`@externalWMTS-GetCap-${mockExternalWmts1?.baseUrl}.all`).should( 'have.length', - 1 + 2 ) cy.get(`@externalWMTS-GetCap-${mockExternalWmts3?.baseUrl}.all`).should( 'have.length', @@ -1115,6 +1114,40 @@ describe('Test of layer handling', () => { .should(`${upArrowEnable ? 'not.' : ''}be.disabled`) }) } + it('reorder layers when they are drag and dropped', () => { + const [bottomLayerId, middleLayerId, topLayerId] = visibleLayerIds + cy.get(`[data-cy^="menu-active-layer-${bottomLayerId}-"]`) + .should('be.visible') + .drag(`[data-cy^="menu-active-layer-${topLayerId}-"]`) + const checkLayerOrder = ( + expectedBottomLayerId: string, + expectedMiddleLayerId: string, + expectedTopLayerId: string + ) => { + cy.location('hash').then((hash) => { + const layersParam = new URLSearchParams(hash).get('layers') ?? '' + const layers = layersParam.split(';') + cy.wrap(layers).should('have.lengthOf', 3) + const [firstUrlLayer, secondUrlLayer, thirdUrlLayer] = layers + cy.wrap(firstUrlLayer).should('contain', expectedBottomLayerId) + cy.wrap(secondUrlLayer).should('contain', expectedMiddleLayerId) + cy.wrap(thirdUrlLayer).should('contain', expectedTopLayerId) + }) + } + // the bottom layer should now be on top, so the order is now + // - bottomLayer + // - topLayer + // - middleLayer + checkLayerOrder(`${middleLayerId}`, `${topLayerId}`, `${bottomLayerId}`) + cy.get(`[data-cy^="menu-active-layer-${middleLayerId}-"]`) + .should('be.visible') + .drag(`[data-cy^="menu-active-layer-${topLayerId}-"]`) + // new state is + // - bottomLayer + // - middleLayer + // - topLayer + checkLayerOrder(`${topLayerId}`, `${middleLayerId}`, `${bottomLayerId}`) + }) it('Reorder layers using the "move" button', () => { const [bottomLayerId, middleLayerId, topLayerId] = visibleLayerIds cy.openLayerSettings(`${bottomLayerId}`) @@ -1248,40 +1281,6 @@ describe('Test of layer handling', () => { { id: `${topLayerId}`, opacity: 0.7 }, ]) }) - it('reorder layers when they are drag and dropped', () => { - const [bottomLayerId, middleLayerId, topLayerId] = visibleLayerIds - cy.get(`[data-cy^="menu-active-layer-${bottomLayerId}-"]`) - .should('be.visible') - .drag(`[data-cy^="menu-active-layer-${topLayerId}-"]`) - const checkLayerOrder = ( - expectedBottomLayerId: string, - expectedMiddleLayerId: string, - expectedTopLayerId: string - ) => { - cy.location('hash').then((hash) => { - const layersParam = new URLSearchParams(hash).get('layers') ?? '' - const layers = layersParam.split(';') - cy.wrap(layers).should('have.lengthOf', 3) - const [firstUrlLayer, secondUrlLayer, thirdUrlLayer] = layers - cy.wrap(firstUrlLayer).should('contain', expectedBottomLayerId) - cy.wrap(secondUrlLayer).should('contain', expectedMiddleLayerId) - cy.wrap(thirdUrlLayer).should('contain', expectedTopLayerId) - }) - } - // the bottom layer should now be on top, so the order is now - // - bottomLayer - // - topLayer - // - middleLayer - checkLayerOrder(`${middleLayerId}`, `${topLayerId}`, `${bottomLayerId}`) - cy.get(`[data-cy^="menu-active-layer-${middleLayerId}-"]`) - .should('be.visible') - .drag(`[data-cy^="menu-active-layer-${topLayerId}-"]`) - // new state is - // - bottomLayer - // - middleLayer - // - topLayer - checkLayerOrder(`${topLayerId}`, `${middleLayerId}`, `${bottomLayerId}`) - }) }) context('External layers', () => { it('does not show a red icon for internal layers', () => {