From ea47ec7792bbbb47d272072b909fa425c5209d73 Mon Sep 17 00:00:00 2001 From: Felix Sommer Date: Thu, 23 Jan 2025 10:36:46 +0100 Subject: [PATCH] PB-1299: Going into drawing module with pre-loaded empty KML breaks the drawing module --- src/modules/drawing/DrawingModule.vue | 2 +- .../drawing/components/DrawingToolbox.vue | 6 + .../useKmlDataManagement.composable.js | 10 +- src/views/MapView.vue | 6 +- tests/cypress/support/drawing.js | 6 +- tests/cypress/tests-e2e/drawing.cy.js | 105 ++++++++++++++---- 6 files changed, 105 insertions(+), 30 deletions(-) diff --git a/src/modules/drawing/DrawingModule.vue b/src/modules/drawing/DrawingModule.vue index 2b0ff1a075..c9cae1555d 100644 --- a/src/modules/drawing/DrawingModule.vue +++ b/src/modules/drawing/DrawingModule.vue @@ -55,7 +55,7 @@ const showAddVertexButton = computed(() => { const hasKml = computed(() => { if (online.value) { - return !!activeKmlLayer.value + return !!activeKmlLayer.value && !activeKmlLayer.value.isEmpty() } return !!store.state.layers.systemLayers.find( (l) => l.id === store.state.drawing.temporaryKmlId diff --git a/src/modules/drawing/components/DrawingToolbox.vue b/src/modules/drawing/components/DrawingToolbox.vue index 05a1fac164..f5cb55cea7 100644 --- a/src/modules/drawing/components/DrawingToolbox.vue +++ b/src/modules/drawing/components/DrawingToolbox.vue @@ -100,6 +100,12 @@ function onCloseClearConfirmation(confirmed) { drawingLayer.getSource().clear() debounceSaveDrawing() store.dispatch('setDrawingMode', { mode: null, ...dispatcher }) + store.dispatch('removeLayer', { + layerId: activeKmlLayer.value.id, + isExternal: activeKmlLayer.value.isExternal, + baseUrl: activeKmlLayer.value.baseUrl, + ...dispatcher, + }) } } diff --git a/src/modules/drawing/useKmlDataManagement.composable.js b/src/modules/drawing/useKmlDataManagement.composable.js index 515a28a223..39f324928b 100644 --- a/src/modules/drawing/useKmlDataManagement.composable.js +++ b/src/modules/drawing/useKmlDataManagement.composable.js @@ -138,10 +138,12 @@ export default function useSaveKmlOnChange(drawingLayerDirectReference) { ...dispatcher, }) } - await store.dispatch('addLayer', { - layer: kmlLayer, - ...dispatcher, - }) + if (!kmlLayer.isEmpty()) { + await store.dispatch('addLayer', { + layer: kmlLayer, + ...dispatcher, + }) + } } else { // if a KMLLayer is already defined, we update it const kmlMetadata = await updateKml( diff --git a/src/views/MapView.vue b/src/views/MapView.vue index dcb78c1910..17e726abd1 100644 --- a/src/views/MapView.vue +++ b/src/views/MapView.vue @@ -40,11 +40,7 @@ const showDragAndDropOverlay = computed(() => store.state.ui.showDragAndDropOver const isOpeningNewTab = computed(() => store.state.ui.isOpeningNewTab) const showNotSharedDrawingWarning = computed(() => store.getters.showNotSharedDrawingWarning) const loadDrawingModule = computed(() => { - return ( - (!activeKmlLayer.value || activeKmlLayer.value?.kmlData) && - isDrawingMode.value && - !is3DActive.value - ) + return isDrawingMode.value && !is3DActive.value }) onMounted(() => { diff --git a/tests/cypress/support/drawing.js b/tests/cypress/support/drawing.js index f793670b9f..c08f49afef 100644 --- a/tests/cypress/support/drawing.js +++ b/tests/cypress/support/drawing.js @@ -129,7 +129,11 @@ Cypress.Commands.add('goToDrawing', (queryParams = {}, withHash = true) => { Cypress.Commands.add('openDrawingMode', () => { const viewportWidth = Cypress.config('viewportWidth') if (viewportWidth && viewportWidth < BREAKPOINT_PHONE_WIDTH) { - cy.get('[data-cy="menu-button"]').click() + cy.readStoreValue('state.ui.showMenu').then((isMenuCurrentlyOpen) => { + if (!isMenuCurrentlyOpen) { + cy.get('[data-cy="menu-button"]').click() + } + }) } cy.get('[data-cy="menu-tray-drawing-section"]').should('be.visible').click() // Make sure that the map pointer events are unregistered to avoid intereference with drawing diff --git a/tests/cypress/tests-e2e/drawing.cy.js b/tests/cypress/tests-e2e/drawing.cy.js index 3a7180bb75..34b82b6e5d 100644 --- a/tests/cypress/tests-e2e/drawing.cy.js +++ b/tests/cypress/tests-e2e/drawing.cy.js @@ -25,7 +25,7 @@ import { RED, SMALL, } from '@/utils/featureStyleUtils' -import { LEGACY_ICON_XML_SCALE_FACTOR } from '@/utils/kmlUtils' +import { EMPTY_KML_DATA, LEGACY_ICON_XML_SCALE_FACTOR } from '@/utils/kmlUtils' import { randomIntBetween } from '@/utils/numberUtils' const isNonEmptyArray = (value) => { @@ -1044,31 +1044,31 @@ describe('Drawing module tests', () => { const newKmlId = interception.response.body.id expect(newKmlId).to.not.eq(kmlId) - // there should be only one KML layer left in the layers, and it's the one just saved - cy.window() - .its('store.getters.activeKmlLayer') - .should('have.property', 'fileId', newKmlId) + // The just cleared KML should not be in the active layer list anymore + cy.window().its('store.getters.activeKmlLayer').should('be.null') cy.log(`Check that adding a new feature update the new kml ${newKmlId}`) // Add another feature and checking that we do not create subsequent copies (we now have the adminId for this KML) cy.clickDrawingTool(EditableFeatureTypes.ANNOTATION) cy.get('[data-cy="ol-map"]').click('center') - cy.wait('@update-kml').its('response.body.id').should('eq', newKmlId) + cy.wait('@post-kml').then((interception2) => { + const newNewKmlId = interception2.response.body.id - cy.log('Check the active layer list making sure that there is only the new') - cy.closeDrawingMode() + cy.log('Check the active layer list making sure that there is only the new') + cy.closeDrawingMode() - cy.log( - `Check that the old kml has been removed from the active layer and that the new one has been added` - ) - cy.get( - `[data-cy^="active-layer-name-${getServiceKmlBaseUrl()}api/kml/files/${kmlId}-"]` - ).should('not.exist') - cy.get( - `[data-cy^="active-layer-name-${getServiceKmlBaseUrl()}api/kml/files/${newKmlId}-"]` - ) - .should('be.visible') - .contains('Drawing') + cy.log( + `Check that the old kml has been removed from the active layer and that the new one has been added` + ) + cy.get( + `[data-cy^="active-layer-name-${getServiceKmlBaseUrl()}api/kml/files/${kmlId}-"]` + ).should('not.exist') + cy.get( + `[data-cy^="active-layer-name-${getServiceKmlBaseUrl()}api/kml/files/${newNewKmlId}-"]` + ) + .should('be.visible') + .contains('Drawing') + }) }) }) }) @@ -1564,4 +1564,71 @@ describe('Drawing module tests', () => { cy.get('[data-cy="popover"] [data-cy="drawing-style-popup"]').should('not.exist') }) }) + + it('receives an empty KML and can use drawing mode', () => { + function verifyActiveKmlLayerEmptyWithError() { + cy.window() + .its('store.getters.activeKmlLayer') + .then((layer) => { + expect(layer.fileId).to.eq(fileId) + expect(layer.name).to.eq('KML') + expect(layer.hasError).to.be.true + expect(layer.kmlData).to.be.null + expect(layer.errorMessages).not.to.be.undefined + expect(layer.errorMessages).not.to.be.undefined + expect(layer.errorMessages.size).to.eq(1) + expect(layer.errorMessages.values().next().value.msg).to.eq( + 'kml_gpx_file_empty' + ) + }) + } + cy.intercept('GET', `**/api/kml/files/**`, (req) => { + const headers = { 'Cache-Control': 'no-cache' } + req.reply(EMPTY_KML_DATA, headers) + }).as('get-empty-kml') + + const fileId = 'zBnMZymwTLSNg__5f8yv6g' + // load map with an injected kml layer containing an empty KML + cy.goToMapView({ + layers: [`KML|https://sys-public.dev.bgdi.ch/api/kml/files/${fileId}`].join(';'), + }) + + cy.wait('@get-empty-kml') + // there should be only one KML layer left in the layers, and it's the one just saved + verifyActiveKmlLayerEmptyWithError() + + cy.openMenuIfMobile() + + cy.get('[data-cy="button-has-error"]').should('be.visible') + + cy.openDrawingMode() + + cy.get('[data-cy="drawing-toolbox-file-name-input"]', { timeout: 15000 }).should( + 'be.visible' + ) + cy.closeDrawingMode() + + // saving the drawing without drawing anything should not change the empty KML layer + verifyActiveKmlLayerEmptyWithError() + + cy.openDrawingMode() + + cy.get('[data-cy="drawing-toolbox-file-name-input"]', { timeout: 15000 }).should( + 'be.visible' + ) + cy.clickDrawingTool(EditableFeatureTypes.MARKER) + cy.get('[data-cy="ol-map"]').click(120, 240) + cy.closeDrawingMode() + + // drawing a marker should not change the empty KML layer and create a new one + cy.window() + .its('store.getters.activeKmlLayer') + .then((layer) => { + expect(layer.fileId).not.to.eq(fileId) + expect(layer.name).to.eq('Drawing') + expect(layer.hasError).to.be.false + expect(layer.kmlData).not.to.be.null + expect(layer.errorMessages.size).to.eq(0) + }) + }) })