fix: fix event name in drop util (#2261)

Rebase of https://github.com/umap-project/umap/pull/2233

package.json

@@ -40,7 +40,7 @@
     "@placemarkio/tokml": "0.3.4",
     "@tmcw/togeojson": "^5.8.0",
     "colorbrewer": "1.5.7",
-    "csv2geojson": "5.1.2",
+    "csv2geojson": "github:umap-project/csv2geojson#patched",
     "dompurify": "3.1.7",
     "georsstogeojson": "^0.2.0",
     "jsdom": "^24.0.0",

umap/migrations/0023_alter_datalayer_uuid.py

@@ -0,0 +1,19 @@
+# Generated by Django 5.1.2 on 2024-11-15 11:03
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("umap", "0022_add_team"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="datalayer",
+            name="uuid",
+            field=models.UUIDField(
+                editable=False, primary_key=True, serialize=False, unique=True
+            ),
+        ),
+    ]

umap/models.py

@@ -250,7 +250,7 @@ class Map(NamedModel):
                 "hash": False,
                 "scrollWheelZoom": False,
                 "noControl": True,
-                "umap_id": self.pk,
+                "id": self.pk,
                 "schema": self.extra_schema,
                 "slideshow": {},
             }
@@ -444,9 +444,7 @@ class DataLayer(NamedModel):
         (COLLABORATORS, _("Editors and team only")),
         (OWNER, _("Owner only")),
     )
-    uuid = models.UUIDField(
+    uuid = models.UUIDField(unique=True, primary_key=True, editable=False)
-        unique=True, primary_key=True, default=uuid.uuid4, editable=False
-    )
     old_id = models.IntegerField(null=True, blank=True)
     map = models.ForeignKey(Map, on_delete=models.CASCADE)
     description = models.TextField(blank=True, null=True, verbose_name=_("description"))
@@ -525,12 +523,13 @@ class DataLayer(NamedModel):
         obj["id"] = self.pk
         obj["permissions"] = {"edit_status": self.edit_status}
         obj["editMode"] = "advanced" if self.can_edit(request) else "disabled"
+        obj["_referenceVersion"] = self.reference_version
         return obj
 
     def clone(self, map_inst=None):
         new = self.__class__.objects.get(pk=self.pk)
         new._state.adding = True
-        new.pk = None
+        new.pk = uuid.uuid4()
         if map_inst:
             new.map = map_inst
         new.geojson = File(new.geojson.file.file)
@@ -543,11 +542,20 @@ class DataLayer(NamedModel):
             valid_prefixes.append(name.startswith("%s_" % self.old_id))
         return any(valid_prefixes) and name.endswith(".geojson")
 
+    def extract_version_number(self, path):
+        version = path.split(".")[0]
+        if "_" in version:
+            return version.split("_")[-1]
+        return version
+
+    @property
+    def reference_version(self):
+        return self.extract_version_number(self.geojson.path)
+
     def version_metadata(self, name):
-        els = name.split(".")[0].split("_")
         return {
             "name": name,
-            "at": els[1],
+            "at": self.extract_version_number(name),
             "size": self.geojson.storage.size(self.get_version_path(name)),
         }
 

umap/static/umap/js/modules/data/features.js

@@ -136,7 +136,7 @@ class Feature {
       subject: 'feature',
       metadata: {
         id: this.id,
-        layerId: this.datalayer?.umap_id || null,
+        layerId: this.datalayer.id,
         featureType: this.getClassName(),
       },
     }

umap/static/umap/js/modules/data/layer.js

@@ -38,7 +38,7 @@ const LAYER_MAP = LAYER_TYPES.reduce((acc, klass) => {
 }, {})
 
 export class DataLayer extends ServerStored {
-  constructor(umap, leafletMap, data) {
+  constructor(umap, leafletMap, data = {}) {
     super()
     this._umap = umap
     this.sync = umap.sync_engine.proxy(this)
@@ -51,10 +51,7 @@ export class DataLayer extends ServerStored {
 
     this._leafletMap = leafletMap
     this.parentPane = this._leafletMap.getPane('overlayPane')
-    this.pane = this._leafletMap.createPane(
+    this.pane = this._leafletMap.createPane(`datalayer${stamp(this)}`, this.parentPane)
-      `datalayer${stamp(this)}`,
-      this.parentPane
-    )
     this.pane.dataset.id = stamp(this)
     // FIXME: should be on layer
     this.renderer = L.svg({ pane: this.pane })
@@ -66,7 +63,11 @@ export class DataLayer extends ServerStored {
     }
 
     this._isDeleted = false
-    this.setUmapId(data.id)
+    this._referenceVersion = data._referenceVersion
+    // Do not save it later.
+    delete data._referenceVersion
+    data.id = data.id || crypto.randomUUID()
+
     this.setOptions(data)
 
     if (!Utils.isObject(this.options.remoteData)) {
@@ -84,7 +85,8 @@ export class DataLayer extends ServerStored {
     this.backupOptions()
     this.connectToMap()
     this.permissions = new DataLayerPermissions(this._umap, this)
-    if (!this.umap_id) {
+
+    if (!this.createdOnServer) {
       if (this.showAtLoad()) this.show()
       this.isDirty = true
     }
@@ -95,6 +97,14 @@ export class DataLayer extends ServerStored {
     if (this.isVisible()) this.propagateShow()
   }
 
+  get id() {
+    return this.options.id
+  }
+
+  get createdOnServer() {
+    return Boolean(this._referenceVersion)
+  }
+
   onDirty(status) {
     if (status) {
       // A layer can be made dirty by indirect action (like dragging layers)
@@ -121,9 +131,7 @@ export class DataLayer extends ServerStored {
   getSyncMetadata() {
     return {
       subject: 'datalayer',
-      metadata: {
+      metadata: { id: this.id },
-        id: this.umap_id || null,
-      },
     }
   }
 
@@ -160,7 +168,7 @@ export class DataLayer extends ServerStored {
   autoLoaded() {
     if (!this._umap.datalayersFromQueryString) return this.options.displayOnLoad
     const datalayerIds = this._umap.datalayersFromQueryString
-    let loadMe = datalayerIds.includes(this.umap_id.toString())
+    let loadMe = datalayerIds.includes(this.id.toString())
     if (this.options.old_id) {
       loadMe = loadMe || datalayerIds.includes(this.options.old_id.toString())
     }
@@ -215,13 +223,13 @@ export class DataLayer extends ServerStored {
   }
 
   async fetchData() {
-    if (!this.umap_id) return
+    if (!this.createdOnServer) return
     if (this._loading) return
     this._loading = true
     const [geojson, response, error] = await this._umap.server.get(this._dataUrl())
     if (!error) {
-      this._reference_version = response.headers.get('X-Datalayer-Version')
+      this.setReferenceVersion({ response, sync: false })
-      // FIXME: for now this property is set dynamically from backend
+      // FIXME: for now the _umap_options property is set dynamically from backend
       // And thus it's not in the geojson file in the server
       // So do not let all options to be reset
       // Fix is a proper migration so all datalayers settings are
@@ -257,6 +265,7 @@ export class DataLayer extends ServerStored {
 
   async fromUmapGeoJSON(geojson) {
     if (geojson._storage) geojson._umap_options = geojson._storage // Retrocompat
+    geojson._umap_options.id = this.id
     if (geojson._umap_options) this.setOptions(geojson._umap_options)
     if (this.isRemoteLayer()) await this.fetchRemoteData()
     else this.fromGeoJSON(geojson, false)
@@ -313,18 +322,13 @@ export class DataLayer extends ServerStored {
   }
 
   isLoaded() {
-    return !this.umap_id || this._loaded
+    return !this.createdOnServer || this._loaded
   }
 
   hasDataLoaded() {
     return this._dataloaded
   }
 
-  setUmapId(id) {
-    // Datalayer is null when listening creation form
-    if (!this.umap_id && id) this.umap_id = id
-  }
-
   backupOptions() {
     this._backupOptions = Utils.CopyJSON(this.options)
   }
@@ -357,8 +361,8 @@ export class DataLayer extends ServerStored {
 
   _dataUrl() {
     let url = this._umap.urls.get('datalayer_view', {
-      pk: this.umap_id,
+      pk: this.id,
-      map_id: this._umap.properties.umap_id,
+      map_id: this._umap.id,
     })
 
     // No browser cache for owners/editors.
@@ -437,7 +441,7 @@ export class DataLayer extends ServerStored {
       // otherwise the layer becomes uneditable.
       this.makeFeatures(geojson, sync)
     } catch (err) {
-      console.log('Error with DataLayer', this.umap_id)
+      console.log('Error with DataLayer', this.id)
       console.error(err)
     }
   }
@@ -524,22 +528,22 @@ export class DataLayer extends ServerStored {
 
   getDeleteUrl() {
     return this._umap.urls.get('datalayer_delete', {
-      pk: this.umap_id,
+      pk: this.id,
-      map_id: this._umap.properties.umap_id,
+      map_id: this._umap.id,
     })
   }
 
   getVersionsUrl() {
     return this._umap.urls.get('datalayer_versions', {
-      pk: this.umap_id,
+      pk: this.id,
-      map_id: this._umap.properties.umap_id,
+      map_id: this._umap.id,
     })
   }
 
   getVersionUrl(name) {
     return this._umap.urls.get('datalayer_version', {
-      pk: this.umap_id,
+      pk: this.id,
-      map_id: this._umap.properties.umap_id,
+      map_id: this._umap.id,
       name: name,
     })
   }
@@ -579,7 +583,8 @@ export class DataLayer extends ServerStored {
   }
 
   reset() {
-    if (!this.umap_id) this.erase()
+    if (!this.createdOnServer) this.erase()
+
     this.resetOptions()
     this.parentPane.appendChild(this.pane)
     if (this._leaflet_events_bk && !this._leaflet_events) {
@@ -809,7 +814,7 @@ export class DataLayer extends ServerStored {
       },
       this
     )
-    if (this.umap_id) {
+    if (this.createdOnServer) {
       const filename = `${Utils.slugify(this.options.name)}.geojson`
       const download = Utils.loadTemplate(`
         <a class="button" href="${this._dataUrl()}" download="${filename}">
@@ -1034,6 +1039,11 @@ export class DataLayer extends ServerStored {
     return this.isReadOnly() || this.isRemoteLayer()
   }
 
+  setReferenceVersion({ response, sync }) {
+    this._referenceVersion = response.headers.get('X-Datalayer-Version')
+    this.sync.update('_referenceVersion', this._referenceVersion)
+  }
+
   async save() {
     if (this.isDeleted) return await this.saveDelete()
     if (!this.isLoaded()) {
@@ -1048,14 +1058,15 @@ export class DataLayer extends ServerStored {
     // Filename support is shaky, don't do it for now.
     const blob = new Blob([JSON.stringify(geojson)], { type: 'application/json' })
     formData.append('geojson', blob)
-    const saveUrl = this._umap.urls.get('datalayer_save', {
+    const saveURL = this._umap.urls.get('datalayer_save', {
-      map_id: this._umap.properties.umap_id,
+      map_id: this._umap.id,
-      pk: this.umap_id,
+      pk: this.id,
+      created: this.createdOnServer,
     })
-    const headers = this._reference_version
+    const headers = this._referenceVersion
-      ? { 'X-Datalayer-Reference': this._reference_version }
+      ? { 'X-Datalayer-Reference': this._referenceVersion }
       : {}
-    const status = await this._trySave(saveUrl, headers, formData)
+    const status = await this._trySave(saveURL, headers, formData)
     this._geojson = geojson
     return status
   }
@@ -1090,11 +1101,12 @@ export class DataLayer extends ServerStored {
         this.fromGeoJSON(data.geojson)
         delete data.geojson
       }
-      this._reference_version = response.headers.get('X-Datalayer-Version')
+      delete data.id
-      this.sync.update('_reference_version', this._reference_version)
+      delete data._referenceVersion
-
-      this.setUmapId(data.id)
       this.updateOptions(data)
+
+      this.setReferenceVersion({ response, sync: true })
+
       this.backupOptions()
       this.backupData()
       this.connectToMap()
@@ -1105,7 +1117,7 @@ export class DataLayer extends ServerStored {
   }
 
   async saveDelete() {
-    if (this.umap_id) {
+    if (this.createdOnServer) {
       await this._umap.server.post(this.getDeleteUrl())
     }
     delete this._umap.datalayers[stamp(this)]

umap/static/umap/js/modules/formatter.js

@@ -81,6 +81,8 @@ export class Formatter {
       {
         delimiter: 'auto',
         includeLatLon: false,
+        sexagesimal: false,
+        parseLatLon: (raw) => Number.parseFloat(raw.toString().replace(',', '.')),
       },
       (err, result) => {
         // csv2geojson fallback to null geometries when it cannot determine
@@ -115,7 +117,9 @@ export class Formatter {
   }
 
   async fromGeoRSS(str) {
-    const GeoRSSToGeoJSON = await import('../../vendors/georsstogeojson/GeoRSSToGeoJSON.js')
+    const GeoRSSToGeoJSON = await import(
+      '../../vendors/georsstogeojson/GeoRSSToGeoJSON.js'
+    )
     return GeoRSSToGeoJSON.parse(this.toDom(str))
   }
 

umap/static/umap/js/modules/permissions.js

@@ -149,7 +149,7 @@ export class MapPermissions extends ServerStored {
 
   edit() {
     if (this._umap.properties.editMode !== 'advanced') return
-    if (!this._umap.properties.umap_id) {
+    if (!this._umap.id) {
       Alert.info(translate('Please save the map first'))
       return
     }
@@ -199,13 +199,13 @@ export class MapPermissions extends ServerStored {
 
   getUrl() {
     return this._umap.urls.get('map_update_permissions', {
-      map_id: this._umap.properties.umap_id,
+      map_id: this._umap.id,
     })
   }
 
   getAttachUrl() {
     return this._umap.urls.get('map_attach_owner', {
-      map_id: this._umap.properties.umap_id,
+      map_id: this._umap.id,
     })
   }
 
@@ -262,8 +262,8 @@ export class DataLayerPermissions extends ServerStored {
 
   getUrl() {
     return this._umap.urls.get('datalayer_permissions', {
-      map_id: this._umap.properties.umap_id,
+      map_id: this._umap.id,
-      pk: this.datalayer.umap_id,
+      pk: this.datalayer.id,
     })
   }
 

umap/static/umap/js/modules/schema.js

@@ -563,4 +563,9 @@ export const SCHEMA = {
     type: Object,
     impacts: ['data'],
   },
+
+  _referenceVersion: {
+    type: Number,
+    impacts: ['data'],
+  },
 }

umap/static/umap/js/modules/share.js

@@ -61,7 +61,7 @@ export default class Share {
       translate('All data and settings of the map')
     )
     const downloadUrl = this._umap.urls.get('map_download', {
-      map_id: this._umap.properties.umap_id,
+      map_id: this._umap.id,
     })
     const link = Utils.loadTemplate(`
       <div>
@@ -205,8 +205,8 @@ class IframeExporter {
     }
     if (this.options.keepCurrentDatalayers) {
       this._umap.eachDataLayer((datalayer) => {
-        if (datalayer.isVisible() && datalayer.umap_id) {
+        if (datalayer.isVisible() && datalayer.createdOnServer) {
-          datalayers.push(datalayer.umap_id)
+          datalayers.push(datalayer.id)
         }
       })
       this.queryString.datalayers = datalayers.join(',')

umap/static/umap/js/modules/sync/updaters.js

@@ -32,8 +32,7 @@ class BaseUpdater {
   }
 
   getDataLayerFromID(layerId) {
-    if (layerId) return this._umap.getDataLayerByUmapId(layerId)
+    return this._umap.getDataLayerByUmapId(layerId)
-    return this._umap.defaultEditDataLayer()
   }
 
   applyMessage(payload) {
@@ -54,7 +53,7 @@ export class MapUpdater extends BaseUpdater {
 
 export class DataLayerUpdater extends BaseUpdater {
   upsert({ value }) {
-    // Inserts does not happen (we use multiple updates instead).
+    // Upsert only happens when a new datalayer is created.
     this._umap.createDataLayer(value, false)
     this._umap.render([])
   }

umap/static/umap/js/modules/umap.js

@@ -43,6 +43,10 @@ export default class Umap extends ServerStored {
     this.init(element, geojson)
   }
 
+  get id() {
+    return this.properties.id
+  }
+
   async init(element, geojson) {
     this.properties = Object.assign(
       {
@@ -166,7 +170,7 @@ export default class Umap extends ServerStored {
     }
 
     // Creation mode
-    if (!this.properties.umap_id) {
+    if (!this.id) {
       if (!this.properties.preview) {
         this.isDirty = true
         this.enableEdit()
@@ -285,7 +289,7 @@ export default class Umap extends ServerStored {
     }
     if (this.searchParams.has('download')) {
       const download_url = this.urls.get('map_download', {
-        map_id: this.properties.umap_id,
+        map_id: this.id,
       })
       window.location = download_url
     }
@@ -563,7 +567,7 @@ export default class Umap extends ServerStored {
   createDataLayer(options = {}, sync = true) {
     options.name =
       options.name || `${translate('Layer')} ${this.datalayersIndex.length + 1}`
-    const datalayer = new DataLayer(this, this._leafletMap, options, sync)
+    const datalayer = new DataLayer(this, this._leafletMap, options)
 
     if (sync !== false) {
       datalayer.sync.upsert(datalayer.options)
@@ -1098,7 +1102,7 @@ export default class Umap extends ServerStored {
     formData.append('name', this.properties.name)
     formData.append('center', JSON.stringify(this.geometry()))
     formData.append('settings', JSON.stringify(geojson))
-    const uri = this.urls.get('map_save', { map_id: this.properties.umap_id })
+    const uri = this.urls.get('map_save', { map_id: this.id })
     const [data, _, error] = await this.server.post(uri, {}, formData)
     // FIXME: login_required response will not be an error, so it will not
     // stop code while it should
@@ -1113,8 +1117,8 @@ export default class Umap extends ServerStored {
       return
     }
     this.properties.user = data.user
-    if (!this.properties.umap_id) {
+    if (!this.id) {
-      this.properties.umap_id = data.id
+      this.properties.id = data.id
       this.permissions.setProperties(data.permissions)
       this.permissions.commit()
       if (data.permissions?.anonymous_edit_url) {
@@ -1228,7 +1232,7 @@ export default class Umap extends ServerStored {
       this.sync.stop()
     } else {
       const ws_token_uri = this.urls.get('map_websocket_auth_token', {
-        map_id: this.properties.umap_id,
+        map_id: this.id,
       })
       await this.sync.authenticate(
         ws_token_uri,
@@ -1398,8 +1402,10 @@ export default class Umap extends ServerStored {
     this.editPanel.open({ content: container })
   }
 
-  getDataLayerByUmapId(umap_id) {
+  getDataLayerByUmapId(id) {
-    return this.findDataLayer((d) => d.umap_id === umap_id)
+    const datalayer = this.findDataLayer((d) => d.id === id)
+    if (!datalayer) throw new Error(`Can't find datalayer with id ${id}`)
+    return datalayer
   }
 
   firstVisibleDatalayer() {
@@ -1433,10 +1439,10 @@ export default class Umap extends ServerStored {
   }
 
   async star() {
-    if (!this.properties.umap_id) {
+    if (!this.id) {
       return Alert.error(translate('Please save the map first'))
     }
-    const url = this.urls.get('map_star', { map_id: this.properties.umap_id })
+    const url = this.urls.get('map_star', { map_id: this.id })
     const [data, response, error] = await this.server.post(url)
     if (error) {
       return
@@ -1530,7 +1536,7 @@ export default class Umap extends ServerStored {
     this.dialog
       .confirm(translate('Are you sure you want to delete this map?'))
       .then(async () => {
-        const url = this.urls.get('map_delete', { map_id: this.properties.umap_id })
+        const url = this.urls.get('map_delete', { map_id: this.id })
         const [data, response, error] = await this.server.post(url)
         if (data.redirect) window.location = data.redirect
       })
@@ -1542,7 +1548,7 @@ export default class Umap extends ServerStored {
         translate('Are you sure you want to clone this map and all its datalayers?')
       )
       .then(async () => {
-        const url = this.urls.get('map_clone', { map_id: this.properties.umap_id })
+        const url = this.urls.get('map_clone', { map_id: this.id })
         const [data, response, error] = await this.server.post(url)
         if (data.redirect) window.location = data.redirect
       })
@@ -1552,7 +1558,7 @@ export default class Umap extends ServerStored {
     const sendLink =
       this.properties.urls.map_send_edit_link &&
       this.urls.get('map_send_edit_link', {
-        map_id: this.properties.umap_id,
+        map_id: this.id,
       })
     await this.server.post(sendLink, {}, formData)
   }

umap/static/umap/js/modules/urls.js

@@ -25,8 +25,8 @@ export default class URLs {
   }
 
   // Update the layer if pk is passed, create otherwise.
-  datalayer_save({ map_id, pk }, ...options) {
+  datalayer_save({ map_id, pk, created }, ...options) {
-    if (pk) return this.get('datalayer_update', { map_id, pk }, ...options)
+    if (created) return this.get('datalayer_update', { map_id, pk }, ...options)
     return this.get('datalayer_create', { map_id, pk }, ...options)
   }
 }

umap/static/umap/js/umap.controls.js

@@ -357,20 +357,20 @@ U.DropControl = L.Class.extend({
     L.DomEvent.off(this.dropzone, 'dragleave', this.dragleave, this)
   },
 
-  dragenter: function (e) {
+  dragenter: function (event) {
-    L.DomEvent.stop(e)
+    L.DomEvent.stop(event)
     this.map.scrollWheelZoom.disable()
     this.dropzone.classList.add('umap-dragover')
   },
 
-  dragover: (e) => {
+  dragover: (event) => {
-    L.DomEvent.stop(e)
+    L.DomEvent.stop(event)
   },
 
   drop: function (event) {
     this.map.scrollWheelZoom.enable()
     this.dropzone.classList.remove('umap-dragover')
-    L.DomEvent.stop(e)
+    L.DomEvent.stop(event)
     for (const file of event.dataTransfer.files) {
       this.map._umap.processFileToImport(file)
     }

umap/static/umap/unittests/URLs.js

@@ -8,10 +8,10 @@ import URLs from '../js/modules/urls.js'
 describe('URLs', () => {
   // Mock server URLs that will be used for testing
   const mockServerUrls = {
-    map_create: '/maps/create',
+    map_create: '/maps/create/',
-    map_update: '/maps/{map_id}/update',
+    map_update: '/maps/{map_id}/update/',
-    datalayer_create: '/maps/{map_id}/datalayers/create',
+    datalayer_create: '/maps/{map_id}/datalayers/{pk}/create/',
-    datalayer_update: '/maps/{map_id}/datalayers/{pk}/update',
+    datalayer_update: '/maps/{map_id}/datalayers/{pk}/update/',
   }
 
   let urls = new URLs(mockServerUrls)
@@ -22,37 +22,37 @@ describe('URLs', () => {
     })
 
     it('should return the correct templated URL for known urlNames', () => {
-      expect(urls.get('map_create')).to.be.equal('/maps/create')
+      expect(urls.get('map_create')).to.be.equal('/maps/create/')
-      expect(urls.get('map_update', { map_id: '123' })).to.be.equal('/maps/123/update')
+      expect(urls.get('map_update', { map_id: '123' })).to.be.equal('/maps/123/update/')
     })
 
     it('should return the correct templated URL when provided with parameters', () => {
       expect(urls.get('datalayer_update', { map_id: '123', pk: '456' })).to.be.equal(
-        '/maps/123/datalayers/456/update'
+        '/maps/123/datalayers/456/update/'
       )
     })
   })
 
   describe('map_save()', () => {
     it('should return the create URL if no map_id is provided', () => {
-      expect(urls.map_save({})).to.be.equal('/maps/create')
+      expect(urls.map_save({})).to.be.equal('/maps/create/')
     })
 
     it('should return the update URL if a map_id is provided', () => {
-      expect(urls.map_save({ map_id: '123' })).to.be.equal('/maps/123/update')
+      expect(urls.map_save({ map_id: '123' })).to.be.equal('/maps/123/update/')
     })
   })
 
   describe('datalayer_save()', () => {
-    it('should return the create URL if no pk is provided', () => {
+    it('should return the create URL if created is false', () => {
-      expect(urls.datalayer_save({ map_id: '123' })).to.be.equal(
+      expect(urls.datalayer_save({ map_id: '123', pk: '00000000-0000-0000-0000-000000000000', created: false })).to.be.equal(
-        '/maps/123/datalayers/create'
+        '/maps/123/datalayers/00000000-0000-0000-0000-000000000000/create/'
       )
     })
 
-    it('should return the update URL if a pk is provided', () => {
+    it('should return the update URL if created is true', () => {
-      expect(urls.datalayer_save({ map_id: '123', pk: '456' })).to.be.equal(
+      expect(urls.datalayer_save({ map_id: '123', pk: '00000000-0000-0000-0000-000000000000', created: true })).to.be.equal(
-        '/maps/123/datalayers/456/update'
+        '/maps/123/datalayers/00000000-0000-0000-0000-000000000000/update/'
       )
     })
   })

umap/static/umap/vendors/csv2geojson/csv2geojson.js

@@ -81,6 +81,8 @@ function csv2geojson(x, options, callback) {
     }
 
     options.delimiter = options.delimiter || ',';
+    options.parseLatLon = options.parseLatLon || parseFloat;
+    options.sexagesimal = options.sexagesimal !== false;
 
     var latfield = options.latfield || '',
         lonfield = options.lonfield || '',
@@ -129,6 +131,7 @@ function csv2geojson(x, options, callback) {
 
     if (!latfield) latfield = guessLatHeader(parsed[0]);
     if (!lonfield) lonfield = guessLonHeader(parsed[0]);
+
     var noGeometry = (!latfield || !lonfield);
 
     if (noGeometry) {
@@ -152,13 +155,15 @@ function csv2geojson(x, options, callback) {
                 lonf, latf,
                 a;
 
-            a = sexagesimal(lonk, 'EW');
+            if (options.sexagesimal) {
-            if (a) lonk = a;
+                a = sexagesimal(lonk, 'EW');
-            a = sexagesimal(latk, 'NS');
+                if (a) lonk = a;
-            if (a) latk = a;
+                a = sexagesimal(latk, 'NS');
+                if (a) latk = a;
+            }
 
-            lonf = parseFloat(lonk);
+            lonf = options.parseLatLon(lonk);
-            latf = parseFloat(latk);
+            latf = options.parseLatLon(latk);
 
             if (isNaN(lonf) ||
                 isNaN(latf)) {
@@ -179,8 +184,8 @@ function csv2geojson(x, options, callback) {
                     geometry: {
                         type: 'Point',
                         coordinates: [
-                            parseFloat(lonf),
+                            lonf,
-                            parseFloat(latf)
+                            latf
                         ]
                     }
                 });

umap/tests/base.py

@@ -1,5 +1,6 @@
 import copy
 import json
+import uuid
 
 import factory
 from django.contrib.auth import get_user_model
@@ -113,6 +114,7 @@ class MapFactory(factory.django.DjangoModelFactory):
 
 
 class DataLayerFactory(factory.django.DjangoModelFactory):
+    uuid = factory.LazyFunction(lambda: uuid.uuid4())
     map = factory.SubFactory(MapFactory)
     name = "test datalayer"
     description = "test description"

umap/tests/integration/test_edit_datalayer.py

@@ -129,32 +129,43 @@ def test_can_change_name(live_server, openmap, page, datalayer):
 
 
 def test_can_create_new_datalayer(live_server, openmap, page, datalayer):
+    """
+    Test the creation and editing of a new datalayer.
+
+    This test verifies that:
+    1. A new datalayer can be created and saved.
+    2. The newly created datalayer appears in the UI and is saved in the database.
+    3. Editing the same datalayer updates it instead of creating a new one.
+    4. The UI reflects the changes and the database is updated correctly.
+    5. The 'dirty' state of the map is managed correctly during these operations.
+    """
+
     page.goto(
         f"{live_server.url}{openmap.get_absolute_url()}?edit&onLoadPanel=databrowser"
     )
     page.get_by_role("link", name="Manage layers").click()
     page.get_by_role("button", name="Add a layer").click()
     page.locator('input[name="name"]').click()
-    page.locator('input[name="name"]').fill("my new layer")
+    page.locator('input[name="name"]').fill("Layer A")
-    expect(page.get_by_text("my new layer")).to_be_visible()
+    expect(page.get_by_text("Layer A")).to_be_visible()
     with page.expect_response(re.compile(".*/datalayer/create/.*")):
         page.get_by_role("button", name="Save").click()
     assert DataLayer.objects.count() == 2
     saved = DataLayer.objects.last()
-    assert saved.name == "my new layer"
+    assert saved.name == "Layer A"
     expect(page.locator(".umap-is-dirty")).to_be_hidden()
     # Edit again, it should not create a new datalayer
     page.get_by_role("link", name="Manage layers").click()
     page.locator(".panel.right").get_by_title("Edit", exact=True).first.click()
     page.locator('input[name="name"]').click()
-    page.locator('input[name="name"]').fill("my new layer with a new name")
+    page.locator('input[name="name"]').fill("Layer A with a new name")
-    expect(page.get_by_text("my new layer with a new name")).to_be_visible()
+    expect(page.get_by_text("Layer A with a new name")).to_be_visible()
     page.get_by_role("button", name="Save").click()
     with page.expect_response(re.compile(".*/datalayer/update/.*")):
         page.get_by_role("button", name="Save").click()
     assert DataLayer.objects.count() == 2
     saved = DataLayer.objects.last()
-    assert saved.name == "my new layer with a new name"
+    assert saved.name == "Layer A with a new name"
     expect(page.locator(".umap-is-dirty")).to_be_hidden()
 
 

umap/tests/integration/test_import.py

@@ -92,11 +92,11 @@ def test_umap_import_from_textarea(live_server, tilelayer, page, settings):
     expect(
         page.locator('img[src="https://tile.openstreetmap.fr/hot/6/32/21.png"]')
     ).to_be_visible()
-    # Should not have imported umap_id, while in the file options
+    # Should not have imported id, while in the file options
-    assert not page.evaluate("U.MAP.properties.umap_id")
+    assert not page.evaluate("U.MAP.properties.id")
     with page.expect_response(re.compile(r".*/datalayer/create/.*")):
         page.get_by_role("button", name="Save").click()
-    assert page.evaluate("U.MAP.properties.umap_id")
+    assert page.evaluate("U.MAP.properties.id")
 
 
 def test_import_geojson_from_textarea(tilelayer, live_server, page):
@@ -494,6 +494,39 @@ def test_import_csv_without_valid_latlon_headers(tilelayer, live_server, page):
     expect(page.locator('umap-alert div[data-level="error"]')).to_be_visible()
 
 
+def test_import_csv_with_commas_in_latlon(tilelayer, live_server, page, settings):
+    settings.UMAP_ALLOW_ANONYMOUS = True
+    page.goto(f"{live_server.url}/map/new/")
+    page.get_by_title("Open browser").click()
+    layers = page.locator(".umap-browser .datalayer")
+    markers = page.locator(".leaflet-marker-icon")
+    page.get_by_title("Import data").click()
+    textarea = page.locator(".umap-upload textarea")
+    textarea.fill("lat;lon;foobar\n12,24;48,34;mypoint\n12,23;48,35;mypoint2")
+    page.locator('select[name="format"]').select_option("csv")
+    page.get_by_role("button", name="Import data", exact=True).click()
+    expect(layers).to_have_count(1)
+    expect(markers).to_have_count(2)
+    with page.expect_response(re.compile(r".*/datalayer/create/.*")):
+        page.get_by_role("button", name="Save").click()
+    datalayer = DataLayer.objects.last()
+    saved_data = json.loads(Path(datalayer.geojson.path).read_text())
+    assert saved_data["features"][0]["geometry"] == {
+        "coordinates": [
+            48.35,
+            12.23,
+        ],
+        "type": "Point",
+    }
+    assert saved_data["features"][1]["geometry"] == {
+        "coordinates": [
+            48.34,
+            12.24,
+        ],
+        "type": "Point",
+    }
+
+
 def test_create_remote_data(page, live_server, tilelayer):
     def handle(route):
         route.fulfill(

umap/tests/integration/test_optimistic_merge.py

@@ -49,6 +49,7 @@ def test_created_markers_are_merged(context, live_server, tilelayer):
         "name": "test datalayer",
         "editMode": "advanced",
         "inCaption": True,
+        "id": str(datalayer.pk),
     }
 
     # Now navigate to this map from another tab
@@ -78,12 +79,14 @@ def test_created_markers_are_merged(context, live_server, tilelayer):
         sleep(1)
     # No change after the save
     expect(marker_pane_p2).to_have_count(2)
-    assert DataLayer.objects.get(pk=datalayer.pk).settings == {
+    datalayer_v2 = DataLayer.objects.get(pk=datalayer.pk)
+    assert datalayer_v2.settings == {
         "browsable": True,
         "displayOnLoad": True,
         "name": "test datalayer",
         "inCaption": True,
         "editMode": "advanced",
+        "id": str(datalayer.pk),
     }
 
     # Now create another marker in the first tab
@@ -94,7 +97,8 @@ def test_created_markers_are_merged(context, live_server, tilelayer):
         save_p1.click()
     # Should now get the other marker too
     expect(marker_pane_p1).to_have_count(3)
-    assert DataLayer.objects.get(pk=datalayer.pk).settings == {
+    datalayer_v3 = DataLayer.objects.get(pk=datalayer.pk)
+    assert datalayer_v3.settings == {
         "browsable": True,
         "displayOnLoad": True,
         "name": "test datalayer",
@@ -112,7 +116,8 @@ def test_created_markers_are_merged(context, live_server, tilelayer):
         save_p1.click()
         sleep(1)
     # Should now get the other marker too
-    assert DataLayer.objects.get(pk=datalayer.pk).settings == {
+    datalayer_v4 = DataLayer.objects.get(pk=datalayer.pk)
+    assert datalayer_v4.settings == {
         "browsable": True,
         "displayOnLoad": True,
         "name": "test datalayer",
@@ -132,7 +137,8 @@ def test_created_markers_are_merged(context, live_server, tilelayer):
         save_p2.click()
         sleep(1)
     # Should now get the other markers too
-    assert DataLayer.objects.get(pk=datalayer.pk).settings == {
+    datalayer_v5 = DataLayer.objects.get(pk=datalayer.pk)
+    assert datalayer_v5.settings == {
         "browsable": True,
         "displayOnLoad": True,
         "name": "test datalayer",

umap/tests/integration/test_websocket_sync.py

@@ -3,7 +3,7 @@ import re
 import pytest
 from playwright.sync_api import expect
 
-from umap.models import Map
+from umap.models import DataLayer, Map
 
 from ..base import DataLayerFactory, MapFactory
 
@@ -267,7 +267,7 @@ def test_websocket_connection_can_sync_cloned_polygons(
     b_polygon = peerB.locator("path")
 
     # Clone on peer B and save
-    b_polygon.click(button="right")
+    b_polygon.click(button="right", delay=200)
     peerB.get_by_role("button", name="Clone this feature").click()
 
     expect(peerB.locator("path")).to_have_count(2)
@@ -343,3 +343,75 @@ def test_websocket_connection_can_sync_late_joining_peer(
 
     # Clean up: close edit mode
     peerB.locator("body").press("Escape")
+
+
+@pytest.mark.xdist_group(name="websockets")
+def test_should_sync_datalayers(new_page, live_server, websocket_server, tilelayer):
+    map = MapFactory(name="sync", edit_status=Map.ANONYMOUS)
+    map.settings["properties"]["syncEnabled"] = True
+    map.save()
+
+    assert not DataLayer.objects.count()
+
+    # Create two tabs
+    peerA = new_page("Page A")
+    peerA.goto(f"{live_server.url}{map.get_absolute_url()}?edit")
+    peerB = new_page("Page B")
+    peerB.goto(f"{live_server.url}{map.get_absolute_url()}?edit")
+
+    # Create a new layer from peerA
+    peerA.get_by_role("link", name="Manage layers").click()
+    peerA.get_by_role("button", name="Add a layer").click()
+
+    # Check layer has been sync to peerB
+    peerB.get_by_role("button", name="Open browser").click()
+    expect(peerB.get_by_text("Layer 1")).to_be_visible()
+
+    # Draw a marker in layer 1 from peerA
+    peerA.get_by_role("link", name="Draw a marker (Ctrl+M)").click()
+    peerA.locator("#map").click()
+
+    # Check marker is visible from peerB
+    expect(peerB.locator(".leaflet-marker-icon")).to_be_visible()
+
+    # Save layer to the server
+    with peerA.expect_response(re.compile(".*/datalayer/create/.*")):
+        peerA.get_by_role("button", name="Save").click()
+
+    assert DataLayer.objects.count() == 1
+
+    # Create another layer from peerA and draw a marker on it (without saving to server)
+    peerA.get_by_role("link", name="Manage layers").click()
+    peerA.get_by_role("button", name="Add a layer").click()
+    peerA.get_by_role("link", name="Draw a marker (Ctrl+M)").click()
+    peerA.locator("#map").click()
+
+    # Make sure this new marker is in Layer 2 for peerB
+    expect(peerB.get_by_text("Layer 2")).to_be_visible()
+    peerB.locator(".panel.left").get_by_role("button", name="Show/hide layer").nth(
+        1
+    ).click()
+    expect(peerB.locator(".leaflet-marker-icon")).to_be_visible()
+
+    # Now draw a marker from peerB
+    peerB.get_by_role("link", name="Draw a marker (Ctrl+M)").click()
+    peerB.locator("#map").click()
+    peerB.locator('input[name="name"]').fill("marker from peerB")
+
+    # Save from peer B
+    with peerB.expect_response(re.compile(".*/datalayer/create/.*")):
+        peerB.get_by_role("button", name="Save").click()
+
+    assert DataLayer.objects.count() == 2
+
+    # Check this new marker is visible from peerA
+    peerA.get_by_role("button", name="Open browser").click()
+    peerA.locator(".panel.left").get_by_role("button", name="Show/hide layer").nth(
+        1
+    ).click()
+
+    # Now peerA saves the layer 2 to the server
+    with peerA.expect_response(re.compile(".*/datalayer/update/.*")):
+        peerA.get_by_role("button", name="Save").click()
+
+    assert DataLayer.objects.count() == 2

umap/tests/test_datalayer_views.py

@@ -1,6 +1,7 @@
 import json
 from copy import deepcopy
 from pathlib import Path
+from uuid import uuid4
 
 import pytest
 from django.core.files.base import ContentFile
@@ -105,6 +106,21 @@ def test_gzip_should_be_created_if_accepted(client, datalayer, map, post_data):
     assert Path(flat).stat().st_mtime_ns == Path(gzipped).stat().st_mtime_ns
 
 
+def test_create_datalayer(client, map, post_data):
+    uuid = str(uuid4())
+    url = reverse("datalayer_create", args=(map.pk, uuid))
+    client.login(username=map.owner.username, password="123123")
+    response = client.post(url, post_data, follow=True)
+    assert response.status_code == 200
+    new_datalayer = DataLayer.objects.get(pk=uuid)
+    assert new_datalayer.name == "name"
+    assert new_datalayer.rank == 0
+    # Test response is a json
+    data = json.loads(response.content.decode())
+    assert "id" in data
+    assert data["id"] == uuid
+
+
 def test_update(client, datalayer, map, post_data):
     url = reverse("datalayer_update", args=(map.pk, datalayer.pk))
     client.login(username=map.owner.username, password="123123")

umap/urls.py

@@ -155,8 +155,8 @@ map_urls = [
         views.MapClone.as_view(),
         name="map_clone",
     ),
-    re_path(
+    path(
-        r"^map/(?P<map_id>[\d]+)/datalayer/create/$",
+        "map/<int:map_id>/datalayer/create/<uuid:pk>/",
         views.DataLayerCreate.as_view(),
         name="datalayer_create",
     ),

umap/views.py

@@ -598,7 +598,7 @@ class MapDetailMixin(SessionMixin):
             "tilelayers": TileLayer.get_list(),
             "editMode": self.edit_mode,
             "schema": Map.extra_schema,
-            "umap_id": self.get_umap_id(),
+            "id": self.get_id(),
             "starred": self.is_starred(),
             "licences": dict((l.name, l.json) for l in Licence.objects.all()),
             "share_statuses": [
@@ -655,7 +655,7 @@ class MapDetailMixin(SessionMixin):
     def edit_mode(self):
         return "advanced"
 
-    def get_umap_id(self):
+    def get_id(self):
         return None
 
     def is_starred(self):
@@ -725,6 +725,8 @@ class MapView(MapDetailMixin, PermissionsMixin, DetailView):
         return self.object.get_absolute_url()
 
     def get_datalayers(self):
+        # When initializing datalayers from map, we cannot get the reference version
+        # the normal way, which is from the header X-Reference-Version
         return [dl.metadata(self.request) for dl in self.object.datalayer_set.all()]
 
     @property
@@ -736,7 +738,7 @@ class MapView(MapDetailMixin, PermissionsMixin, DetailView):
             edit_mode = "simple"
         return edit_mode
 
-    def get_umap_id(self):
+    def get_id(self):
         return self.object.pk
 
     def get_short_url(self):
@@ -1181,9 +1183,19 @@ class DataLayerCreate(FormLessEditMixin, GZipMixin, CreateView):
 
     def form_valid(self, form):
         form.instance.map = self.kwargs["map_inst"]
+
+        uuid = self.kwargs["pk"]
+        # Check if UUID already exists
+        if DataLayer.objects.filter(uuid=uuid).exists():
+            return HttpResponseBadRequest("UUID already exists")
+
+        form.instance.uuid = uuid
         self.object = form.save()
-        # Simple response with only metadata (including new id)
+        assert uuid == self.object.uuid
-        response = simple_json_response(**self.object.metadata(self.request))
+
+        # Simple response with only metadata
+        data = self.object.metadata(self.request)
+        response = simple_json_response(**data)
         response["X-Datalayer-Version"] = self.version
         return response