Skip to content

Commit

Permalink
Merge pull request #1108 from geoadmin/bug-PB-1116-drawing-name-fix
Browse files Browse the repository at this point in the history
  • Loading branch information
pakb authored Nov 1, 2024
2 parents d19e1e0 + c0d9364 commit 26230fa
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 38 deletions.
6 changes: 6 additions & 0 deletions src/api/layers/KMLLayer.class.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,12 @@ export default class KMLLayer extends AbstractLayer {
this.kmlMetadata = kmlMetadata
if (kmlData) {
this.name = parseKmlName(kmlData)
if (!this.name || this.name === '') {
this.name = isLocalFile
? kmlFileUrl
: // only keeping what is after the last slash
kmlFileUrl.split('/').pop()
}
this.isLoading = false
} else {
this.isLoading = true
Expand Down
7 changes: 7 additions & 0 deletions src/modules/drawing/DrawingModule.vue
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import VectorLayer from 'ol/layer/Vector'
import VectorSource from 'ol/source/Vector'
import { computed, inject, onBeforeUnmount, onMounted, provide, ref, watch } from 'vue'
import { useI18n } from 'vue-i18n'
import { useStore } from 'vuex'
import { IS_TESTING_WITH_CYPRESS } from '@/config/staging.config'
Expand All @@ -22,6 +23,7 @@ const olMap = inject('olMap')
const drawingInteractions = ref(null)
const isNewDrawing = ref(true)
const i18n = useI18n()
const store = useStore()
const availableIconSets = computed(() => store.state.drawing.iconSets)
const projection = computed(() => store.state.position.projection)
Expand Down Expand Up @@ -130,6 +132,11 @@ onMounted(() => {
if (hasKml.value) {
isNewDrawing.value = false
addKmlToDrawing()
} else {
store.dispatch('setDrawingName', {
name: i18n.t('draw_layer_label'),
...dispatcher,
})
}
olMap.addLayer(drawingLayer)
Expand Down
42 changes: 20 additions & 22 deletions src/modules/drawing/components/DrawingToolbox.vue
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@ import { useStore } from 'vuex'
import { EditableFeatureTypes } from '@/api/features/EditableFeature.class'
import DrawingExporter from '@/modules/drawing/components/DrawingExporter.vue'
import DrawingHeader from '@/modules/drawing/components/DrawingHeader.vue'
import DrawingToolboxButton from '@/modules/drawing/components/DrawingToolboxButton.vue'
import SharePopup from '@/modules/drawing/components/SharePopup.vue'
import { DrawingState } from '@/modules/drawing/lib/export-utils'
import useSaveKmlOnChange from '@/modules/drawing/useKmlDataManagement.composable'
import ModalWithBackdrop from '@/utils/components/ModalWithBackdrop.vue'
import { useTippyTooltip } from '@/utils/composables/useTippyTooltip.js'
import DrawingHeader from './DrawingHeader.vue'
import { useTippyTooltip } from '@/utils/composables/useTippyTooltip'
import debounce from '@/utils/debounce'
const dispatcher = { dispatcher: 'DrawingToolbox.vue' }
Expand All @@ -39,7 +39,10 @@ const isDrawingLineOrMeasure = computed(() =>
)
)
const activeKmlLayer = computed(() => store.getters.activeKmlLayer)
const drawingName = ref(activeKmlLayer.value?.name || i18n.t('draw_layer_label'))
const drawingName = computed({
get: () => store.state.drawing.name,
set: (value) => debounceSaveDrawingName(value),
})
const isDrawingStateError = computed(() => saveState.value < 0)
/** Return a different translation key depending on the saving status */
const drawingStateMessage = computed(() => {
Expand All @@ -64,7 +67,7 @@ function onCloseClearConfirmation(confirmed) {
store.dispatch('clearDrawingFeatures', dispatcher)
store.dispatch('clearAllSelectedFeatures', dispatcher)
drawingLayer.getSource().clear()
debounceSaveDrawing({ drawingName: drawingName.value })
debounceSaveDrawing()
store.dispatch('setDrawingMode', { mode: null, ...dispatcher })
}
}
Expand All @@ -78,40 +81,35 @@ onMounted(() => {
}
})
watch(drawingName, () => {
sanitizeDrawingName()
debounceSaveDrawing({
// Lowering the risk of the user changing the name and leaving the drawing module without the name being saved.
// It is terrible design that this can occur, but moving the drawing name management other places raises the complexity tenfold
// so that is a trade-off we can live with.
debounceTime: 500,
drawingName: drawingName.value.trim(),
})
})
watch(activeKmlLayer, () => {
if (activeKmlLayer.value) {
// no need for the message telling the user the drawing is empty, and he can't edit the drawing name
removeNoActiveKmlWarning()
}
})
function sanitizeDrawingName() {
drawingName.value = DOMPurify.sanitize(drawingName.value, {
USE_PROFILES: { xml: true },
})
}
function closeDrawing() {
emits('closeDrawing')
}
function selectDrawingMode(drawingMode) {
store.dispatch('setDrawingMode', { mode: drawingMode, ...dispatcher })
}
function onDeleteLastPoint() {
emits('removeLastPoint')
}
const debounceSaveDrawingName = debounce(async (newName) => {
await store.dispatch('setDrawingName', {
// sanitizing to avoid any XSS vector
name: DOMPurify.sanitize(newName, {
USE_PROFILES: { xml: true },
}).trim(),
...dispatcher,
})
debounceSaveDrawing()
}, 200)
</script>

<template>
Expand Down
35 changes: 19 additions & 16 deletions src/modules/drawing/useKmlDataManagement.composable.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { computed, inject, ref } from 'vue'
import { useI18n } from 'vue-i18n'
import { useStore } from 'vuex'

import { createKml, getKmlUrl, updateKml } from '@/api/files.api'
Expand All @@ -17,6 +18,7 @@ const saveState = ref(DrawingState.INITIAL)
export default function useSaveKmlOnChange(drawingLayerDirectReference) {
const drawingLayer = inject('drawingLayer', drawingLayerDirectReference)

const i18n = useI18n()
const store = useStore()
const online = computed(() => store.state.drawing.online)
const projection = computed(() => store.state.position.projection)
Expand All @@ -26,6 +28,9 @@ export default function useSaveKmlOnChange(drawingLayerDirectReference) {
const temporaryKml = computed(() =>
store.state.layers.systemLayers.find((l) => l.id === temporaryKmlId.value)
)
const drawingName = computed(
() => store.state.drawing.name ?? activeKmlLayer.value?.name ?? i18n.t('draw_layer_label')
)

let addKmlLayerTimeout = null
const savesInProgress = ref([])
Expand All @@ -46,6 +51,10 @@ export default function useSaveKmlOnChange(drawingLayerDirectReference) {
const features = parseKml(availableKmlLayer, projection.value, availableIconSets.value)
log.debug('Add features to drawing layer', features, drawingLayer)
drawingLayer.getSource().addFeatures(features)
store.dispatch('setDrawingName', {
name: availableKmlLayer.name,
...dispatcher,
})
store.dispatch('setDrawingFeatures', {
featureIds: features.map((feature) => feature.getId()),
...dispatcher,
Expand Down Expand Up @@ -75,7 +84,7 @@ export default function useSaveKmlOnChange(drawingLayerDirectReference) {
}
}

async function saveDrawing({ retryOnError = true, drawingName = null }) {
async function saveDrawing({ retryOnError = true }) {
try {
log.debug(
`Save drawing retryOnError ${retryOnError}, differSaveDrawing=${differSaveDrawingTimeout}`
Expand All @@ -85,35 +94,34 @@ export default function useSaveKmlOnChange(drawingLayerDirectReference) {
const kmlData = generateKmlString(
projection.value,
drawingLayer.getSource().getFeatures(),
drawingName ?? activeKmlLayer.value?.name
drawingName.value
)
if (online.value) {
await saveOnlineDrawing(kmlData, drawingName)
await saveOnlineDrawing(kmlData)
} else {
await saveLocalDrawing(kmlData, drawingName)
await saveLocalDrawing(kmlData)
}
saveState.value = DrawingState.SAVED
} catch (e) {
log.error('Could not save KML layer: ', e)
saveState.value = DrawingState.SAVE_ERROR
if (!IS_TESTING_WITH_CYPRESS && retryOnError) {
// Retry saving in 5 seconds
debounceSaveDrawing({ debounceTime: 5000, retryOnError: false, drawingName })
debounceSaveDrawing({ debounceTime: 5000, retryOnError: false })
}
}
}

/**
* @param {String} kmlData
* @param {String} [drawingName=null] Default is `null`
* @returns {Promise<void>}
*/
async function saveOnlineDrawing(kmlData, drawingName = null) {
async function saveOnlineDrawing(kmlData) {
if (!activeKmlLayer.value?.adminId) {
// creation of the new KML (copy or new)
const kmlMetadata = await createKml(kmlData)
const kmlLayer = new KMLLayer({
name: drawingName ?? activeKmlLayer.value?.name,
name: drawingName.value,
kmlFileUrl: getKmlUrl(kmlMetadata.id),
visible: true,
opacity: activeKmlLayer.value?.opacity, // re-use current KML layer opacity, or null
Expand Down Expand Up @@ -152,12 +160,11 @@ export default function useSaveKmlOnChange(drawingLayerDirectReference) {

/**
* @param {String} kmlData
* @param {String} [drawingName=null] Default is `null`
* @returns {Promise<void>}
*/
async function saveLocalDrawing(kmlData, drawingName = null) {
async function saveLocalDrawing(kmlData) {
const kmlLayer = new KMLLayer({
name: drawingName,
name: drawingName.value,
kmlFileUrl: temporaryKmlId.value,
visible: true,
opacity: 1,
Expand All @@ -170,11 +177,7 @@ export default function useSaveKmlOnChange(drawingLayerDirectReference) {
}
}

async function debounceSaveDrawing({
debounceTime = 2000,
retryOnError = true,
drawingName = null,
} = {}) {
async function debounceSaveDrawing({ debounceTime = 2000, retryOnError = true } = {}) {
log.debug(
`Debouncing save drawing debounceTime=${debounceTime} differSaveDrawingTimeout=${differSaveDrawingTimeout}`
)
Expand Down
12 changes: 12 additions & 0 deletions src/store/modules/drawing.store.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@ export default {
* @type {String | null}
*/
temporaryKmlId: null,
/**
* The name of the drawing, or null if no drawing is currently edited.
*
* @type {String | null}
*/
name: null,
},
getters: {
isDrawingEmpty(state) {
Expand Down Expand Up @@ -118,6 +124,9 @@ export default {
dispatcher,
})
},
setDrawingName({ commit }, { name, dispatcher }) {
commit('setDrawingName', { name, dispatcher })
},
},
mutations: {
setDrawingMode: (state, { mode }) => (state.mode = mode),
Expand All @@ -132,5 +141,8 @@ export default {
state.online = online
state.temporaryKmlId = kmlId
},
setDrawingName(state, { name }) {
state.name = name
},
},
}

0 comments on commit 26230fa

Please sign in to comment.