Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PB-1299, PB-1344: Opening Drawing module with pre-loaded empty KML breaks the drawing module #1220

Merged
merged 1 commit into from
Jan 28, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/modules/drawing/DrawingModule.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions src/modules/drawing/components/DrawingToolbox.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})
}
}

Expand Down
10 changes: 6 additions & 4 deletions src/modules/drawing/useKmlDataManagement.composable.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
6 changes: 1 addition & 5 deletions src/views/MapView.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down
1 change: 1 addition & 0 deletions tests/cypress/fixtures/post-kml.fixture.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"admin_id":"jNE8NfIrQ7qQO0wnWCagQQ","author":"web-mapviewer","author_version":"1.0.0","created":"2025-01-27T08:23:07.158+00:00","empty":false,"id":"ARY66ohMQqSXwG4FA2pqeA","links":{"kml":"https://sys-public.dev.bgdi.ch/api/kml/files/ARY66ohMQqSXwG4FA2pqeA","self":"https://sys-public.dev.bgdi.ch/api/kml/admin/ARY66ohMQqSXwG4FA2pqeA"},"success":true,"updated":"2025-01-27T08:23:07.158+00:00"}
6 changes: 1 addition & 5 deletions tests/cypress/support/drawing.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import pako from 'pako'

import { EditableFeatureTypes } from '@/api/features/EditableFeature.class'
import { BREAKPOINT_PHONE_WIDTH } from '@/config/responsive.config'
import { GREEN, RED } from '@/utils/featureStyleUtils'
import { randomIntBetween } from '@/utils/numberUtils'

Expand Down Expand Up @@ -127,10 +126,7 @@ 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.openMenuIfMobile()
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
// pointer events
Expand Down
107 changes: 88 additions & 19 deletions tests/cypress/tests-e2e/drawing.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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')
})
})
})
})
Expand Down Expand Up @@ -1564,4 +1564,73 @@ 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')

cy.intercept('POST', `**/api/kml/admin**`).as('post-new-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()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it might maybe make sense to check that the KML created was a new one, by intercepting the request that creates it on the backend

cy.wait('@post-new-kml')
// drawing a marker should create a new KML layer and overwrite the empty 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)
})
})
})
Loading