From 42ea97717544575a37fca38baf7dd38ce05b88b0 Mon Sep 17 00:00:00 2001 From: Pascal Barth Date: Tue, 26 Nov 2024 12:54:21 +0100 Subject: [PATCH 1/5] PB-1221 : partially out of bound KML was ignored entirely The KML extent detection was not performing as expected with KML that had only one dimension out of LV95 bounds. To counteract that, I've reverted the logic in the extent calculation, transforming LV95 bounds into WGS84 to do the matching instead of transforming a potentially big WGS84 extent to LV95 (lead to big math error...) --- src/utils/__tests__/extentUtils.spec.js | 38 +++++++++++++------- src/utils/extentUtils.js | 29 +++++++++------ tests/cypress/tests-e2e/importToolFile.cy.js | 4 +-- 3 files changed, 47 insertions(+), 24 deletions(-) diff --git a/src/utils/__tests__/extentUtils.spec.js b/src/utils/__tests__/extentUtils.spec.js index d712c0ef2d..56b99160d4 100644 --- a/src/utils/__tests__/extentUtils.spec.js +++ b/src/utils/__tests__/extentUtils.spec.js @@ -7,6 +7,13 @@ import { getExtentIntersectionWithCurrentProjection } from '@/utils/extentUtils' describe('Test extent utils', () => { describe('reproject and cut extent within projection bounds', () => { + function expectExtentIs(toBeTested, expected, acceptableDelta = 0.5) { + expect(toBeTested).to.be.an('Array').lengthOf(4) + expected.forEach((value, index) => { + expect(toBeTested[index]).to.be.approximately(value, acceptableDelta) + }) + } + it('handles well wrong inputs and returns null', () => { expect(getExtentIntersectionWithCurrentProjection()).to.be.null expect(getExtentIntersectionWithCurrentProjection(null, null, null)).to.be.null @@ -18,7 +25,7 @@ describe('Test extent utils', () => { const singleCoordinate = [8.2, 47.5] const singleCoordinateInLV95 = reprojectAndRound(WGS84, LV95, singleCoordinate) const extent = [singleCoordinate, singleCoordinate].flat() - expect(getExtentIntersectionWithCurrentProjection(extent, WGS84, LV95)).to.deep.equal([ + expectExtentIs(getExtentIntersectionWithCurrentProjection(extent, WGS84, LV95), [ ...singleCoordinateInLV95, ...singleCoordinateInLV95, ]) @@ -33,15 +40,21 @@ describe('Test extent utils', () => { expect(getExtentIntersectionWithCurrentProjection(extent, WGS84, LV95)).to.be.null }) it('reproject and cut an extent that is greater than LV95 extent on all sides', () => { - const projectedExtent = getExtentIntersectionWithCurrentProjection( - [-2.4, 35, 21.3, 51.7], - WGS84, - LV95 + expectExtentIs( + getExtentIntersectionWithCurrentProjection([-2.4, 35, 21.3, 51.7], WGS84, LV95), + [...LV95.bounds.bottomLeft, ...LV95.bounds.topRight] + ) + }) + it('reproject and cut an extent that is partially bigger than LV95 bounds', () => { + expectExtentIs( + getExtentIntersectionWithCurrentProjection( + // extent of file linked to PB-1221 + [-122.08, -33.85, 151.21, 51.5], + WGS84, + LV95 + ), + [...LV95.bounds.bottomLeft, ...LV95.bounds.topRight] ) - expect(projectedExtent).to.deep.equal([ - ...LV95.bounds.bottomLeft, - ...LV95.bounds.topRight, - ]) }) it('only gives back the portion of an extent that is within LV95 bounds', () => { const singleCoordinateInsideLV95 = [7.54, 48.12] @@ -51,9 +64,10 @@ describe('Test extent utils', () => { singleCoordinateInsideLV95 ) const overlappingExtent = [0, 0, ...singleCoordinateInsideLV95] - expect( - getExtentIntersectionWithCurrentProjection(overlappingExtent, WGS84, LV95) - ).to.deep.equal([...LV95.bounds.bottomLeft, ...singleCoordinateInLV95]) + expectExtentIs( + getExtentIntersectionWithCurrentProjection(overlappingExtent, WGS84, LV95), + [...LV95.bounds.bottomLeft, ...singleCoordinateInLV95] + ) }) }) }) diff --git a/src/utils/extentUtils.js b/src/utils/extentUtils.js index 9048287da3..1f66c1eb1f 100644 --- a/src/utils/extentUtils.js +++ b/src/utils/extentUtils.js @@ -84,26 +84,35 @@ export function getExtentIntersectionWithCurrentProjection( ) { return null } - let extentInCurrentProjection = flattenExtent(extent) + let currentProjectionAsExtentProjection = currentProjection.bounds.flatten if (extentProjection.epsg !== currentProjection.epsg) { - extentInCurrentProjection = projExtent( - extentProjection, + // we used to reproject the extent here, but there's problem arising if current projection is LV95 and + // the extent is going a little bit out of Switzerland. + // As LV95 is quite location-locked, the further we get, the bigger the mathematical errors start growing. + // So to counteract that, we transform the current projection bounds in the extent projection to do the comparison. + currentProjectionAsExtentProjection = projExtent( currentProjection, - extentInCurrentProjection + extentProjection, + currentProjectionAsExtentProjection ) } - extentInCurrentProjection = getExtentIntersection( - extentInCurrentProjection, - currentProjection.bounds.flatten + let finalExtent = getExtentIntersection( + flattenExtent(extent), + currentProjectionAsExtentProjection ) if ( - !extentInCurrentProjection || + !finalExtent || // OL now populates the extent with Infinity when nothing is in common, instead returning a null value - extentInCurrentProjection.every((value) => Math.abs(value) === Infinity) + finalExtent.every((value) => Math.abs(value) === Infinity) ) { return null } - return flattenExtent(extentInCurrentProjection) + if (extentProjection.epsg !== currentProjection.epsg) { + // if we transformed the current projection extent above, we now need to output the correct proj + finalExtent = projExtent(extentProjection, currentProjection, finalExtent) + } + + return flattenExtent(finalExtent) } /** diff --git a/tests/cypress/tests-e2e/importToolFile.cy.js b/tests/cypress/tests-e2e/importToolFile.cy.js index 760c0ed0e1..8032dd10d6 100644 --- a/tests/cypress/tests-e2e/importToolFile.cy.js +++ b/tests/cypress/tests-e2e/importToolFile.cy.js @@ -224,7 +224,7 @@ describe('The Import File Tool', () => { ) const expectedLayerId = 'external-kml-file.kml' const expectedOnlineLayerId = 'https://example.com/second-valid-kml-file.kml' - const acceptedDelta = 0.1 + const acceptedDelta = 0.2 const checkLocation = (expected, result) => { expect(result).to.be.an('Array') expect(result.length).to.eq(2) @@ -821,7 +821,7 @@ describe('The Import File Tool', () => { cy.closeMenuIfMobile() cy.get('[data-cy="window-close"]').click() - cy.get('[data-cy="ol-map"]').click(150, 250) + cy.get('[data-cy="ol-map"]').click(170, 250) cy.log('Check that the error is displayed in the profile popup') cy.get('[data-cy="show-profile"]').click() From 4e091ae9435bb571a001e61bc8fad6c99096bb15 Mon Sep 17 00:00:00 2001 From: Pascal Barth Date: Tue, 26 Nov 2024 13:31:08 +0100 Subject: [PATCH 2/5] PB-1211 : do not add ; if layer with feature selection is alone When opening the app with a layerId=featureId URL param, the ; was still added to the layers URL param after processing the feature pre-selection. This means the layers URL param was malformed and an error showed up. --- src/utils/legacyLayerParamUtils.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/utils/legacyLayerParamUtils.js b/src/utils/legacyLayerParamUtils.js index e6adbd36b3..fcf493ee5e 100644 --- a/src/utils/legacyLayerParamUtils.js +++ b/src/utils/legacyLayerParamUtils.js @@ -243,7 +243,10 @@ export function handleLegacyFeaturePreSelectionParam(params, store, newQuery) { // if there are no layers parameters at all, we need to create one newQuery.layers = '' } - newQuery.layers += `;${layerId}@features=${featuresIds.split(',').join(':')}` + if (newQuery.layers.length > 0) { + newQuery.layers += ';' + } + newQuery.layers += `${layerId}@features=${featuresIds.split(',').join(':')}` } }) } From d0fae6a6c253bc732d6581852126562cf140159d Mon Sep 17 00:00:00 2001 From: Pascal Barth Date: Tue, 26 Nov 2024 14:00:31 +0100 Subject: [PATCH 3/5] PB-1222 : fix mishandling of feature ID that are "number-like" some feature IDs ends with something like "1314.070" and the layer parsing code was transforming them into number, removing the trailing zero from the ID. This broke many "Link to object" link generated by the backend. Also fixing an issue with the time slider warning not being wrapped in an array, and generating a crash when its value was not set to something valid --- .../storeSync/TimeSliderParamConfig.class.js | 8 +++++--- .../__tests__/layersParamParser.spec.js | 16 ++++++++++++++++ src/router/storeSync/layersParamParser.js | 4 ++++ 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/router/storeSync/TimeSliderParamConfig.class.js b/src/router/storeSync/TimeSliderParamConfig.class.js index 93742cb1e9..c5024f2505 100644 --- a/src/router/storeSync/TimeSliderParamConfig.class.js +++ b/src/router/storeSync/TimeSliderParamConfig.class.js @@ -48,9 +48,11 @@ function validateUrlInput(store, query) { ) if (store.getters.visibleLayers.filter((layer) => layer.hasMultipleTimestamps).length === 0) { - validationObject['warnings'] = new WarningMessage( - 'time_slider_no_time_layer_active_url_warning', - {} + if (!validationObject.warnings) { + validationObject.warnings = [] + } + validationObject.warnings.push( + new WarningMessage('time_slider_no_time_layer_active_url_warning', {}) ) } return validationObject diff --git a/src/router/storeSync/__tests__/layersParamParser.spec.js b/src/router/storeSync/__tests__/layersParamParser.spec.js index f7c9e8cf8b..cf5c09b3ff 100644 --- a/src/router/storeSync/__tests__/layersParamParser.spec.js +++ b/src/router/storeSync/__tests__/layersParamParser.spec.js @@ -127,6 +127,22 @@ describe('Testing layersParamParser', () => { ) }) }) + it('parses correctly a pre-selected feature on a layer', () => { + const layerId = 'fake-layer-id' + const featureId = '1234.050' // some of our IDs end with 0, we have to make sure they are treated as string and not numbers + const result = parseLayersParam(`${layerId}@features=${featureId}`) + checkParsedLayer(result[0], layerId, true, undefined, { + features: featureId, + }) + }) + it('parses correctly multiple pre-selected features on a single layer', () => { + const layerId = 'fake-layer-id' + const featureIds = ['1234.560', 'iAmSomeId'] + const result = parseLayersParam(`${layerId}@features=${featureIds.join(':')}`) + checkParsedLayer(result[0], layerId, true, undefined, { + features: featureIds.join(':'), + }) + }) describe('Visibility/Opacity parsing', () => { it('Parses correctly the visible when specified', () => { diff --git a/src/router/storeSync/layersParamParser.js b/src/router/storeSync/layersParamParser.js index fb4fbcf358..6771a50503 100644 --- a/src/router/storeSync/layersParamParser.js +++ b/src/router/storeSync/layersParamParser.js @@ -115,6 +115,10 @@ export function parseLayersParam(queryValue) { let parsedValue if (value === 'true' || value === 'false') { parsedValue = 'true' === value + } else if (key === 'features') { + // some IDs are "numbers", such as 1314.070, but we NEED the trailing zero + // (they shouldn't be parsed as numbers) + parsedValue = value } else if (isNumber(value)) { parsedValue = Number(value) } else if (key === 'year' && value.toLowerCase() === 'none') { From a654fc007dee50150055293d21241865ecdf79f6 Mon Sep 17 00:00:00 2001 From: Felix Sommer Date: Tue, 26 Nov 2024 11:19:26 +0100 Subject: [PATCH 4/5] PB-1128: fix permalink custom attributes --- src/store/modules/layers.store.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/store/modules/layers.store.js b/src/store/modules/layers.store.js index 28ca5c7686..e4b9bc30ce 100644 --- a/src/store/modules/layers.store.js +++ b/src/store/modules/layers.store.js @@ -338,6 +338,7 @@ const actions = { const clone = layerConfig.clone() clone.visible = layer.visible clone.opacity = layer.opacity + clone.customAttributes = layer.customAttributes if (layer.timeConfig) { clone.timeConfig.updateCurrentTimeEntry( clone.timeConfig.getTimeEntryForYear(layer.timeConfig.currentYear) From 64cf53f8364137a3dd273e14df1d4726cd13af24 Mon Sep 17 00:00:00 2001 From: Felix Sommer Date: Tue, 26 Nov 2024 13:55:26 +0100 Subject: [PATCH 5/5] PB-1128: add test --- tests/cypress/tests-e2e/layers.cy.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/cypress/tests-e2e/layers.cy.js b/tests/cypress/tests-e2e/layers.cy.js index e0e8135ec3..d77b36fc62 100644 --- a/tests/cypress/tests-e2e/layers.cy.js +++ b/tests/cypress/tests-e2e/layers.cy.js @@ -1368,4 +1368,20 @@ describe('Test of layer handling', () => { cy.wait('@geojson-data', { timeout: 5000 }) }) }) + context('Custom url attributes', () => { + it('Keep custom attributes when changing language', () => { + cy.goToMapView({ + lang: 'fr', + }) + cy.wait(['@routeChange', '@layers', '@topics', '@topic-ech']) + cy.goToMapView({ + lang: 'en', + bgLayer: 'ch.swisstopo.pixelkarte-farbe', + topic: 'ech', + layers: 'WMS|https://wms.geo.admin.ch/?item=2024-10-30t103151|test.background.layer@item=2024-10-30t103151', + }) + cy.wait(['@routeChange', '@layers', '@topics', '@topic-ech']) + cy.url().should('contain', 'item=2024-10-30t103151') + }) + }) })