fix(sync): Import the data when syncing GeoJSON objects.

Prior to these changes, the data wasn't transmitted over WebSocket, and
even if present it wasn't taken into account.

umap/static/umap/js/modules/sync/engine.js

@@ -16,7 +16,7 @@ export class SyncEngine {
   }
   _initialize() {
     this.transport = undefined
-    const noop = () => undefined
+    const noop = () => {}
     // by default, all operations do nothing, until the engine is started.
     this.upsert = this.update = this.delete = noop
   }

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

@@ -68,25 +68,29 @@ class FeatureUpdater extends BaseUpdater {
     return datalayer.getFeatureById(id)
   }
 
-  // XXX Not sure about the naming. It's returning latlng OR latlngS
+  // Create or update an object at a specific position
-  getGeometry({ type, coordinates }) {
-    if (type == 'Point') {
-      return L.GeoJSON.coordsToLatLng(coordinates)
-    }
-    return L.GeoJSON.coordsToLatLngs(coordinates)
-  }
-
   upsert({ metadata, value }) {
     let { id, layerId } = metadata
     const datalayer = this.getDataLayerFromID(layerId)
     let feature = this.getFeatureFromMetadata(metadata, value)
-    feature = datalayer.geometryToFeature({ geometry: value.geometry, id, feature })
+    if (feature === undefined) {
+      console.log(`Unable to find feature with id = ${metadata.id}. Creating a new one`)
+    }
+    feature = datalayer.geometryToFeature({
+      geometry: value.geometry,
+      geojson: value,
+      id,
+      feature,
+    })
     feature.addTo(datalayer)
   }
 
+  // Update a property of an object
   update({ key, metadata, value }) {
     let feature = this.getFeatureFromMetadata(metadata)
-
+    if (feature === undefined) {
+      console.error(`Unable to find feature with id = ${metadata.id}.`)
+    }
     switch (key) {
       case 'geometry':
         const datalayer = this.getDataLayerFromID(metadata.layerId)

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

@@ -1249,6 +1249,7 @@ U.Editable = L.Editable.extend({
     L.Editable.prototype.initialize.call(this, map, options)
     this.on('editable:drawing:click editable:drawing:move', this.drawingTooltip)
     this.on('editable:drawing:end', (e) => {
+      console.log('editable:drawing:end')
       this.map.tooltip.close()
       // Leaflet.Editable will delete the drawn shape if invalid
       // (eg. line has only one drawn point)
@@ -1258,14 +1259,21 @@ U.Editable = L.Editable.extend({
     })
     // Layer for items added by users
     this.on('editable:drawing:cancel', (e) => {
+      console.log('editable:drawing:cancel')
       if (e.layer instanceof U.Marker) e.layer.del()
+      else {
+        // the user might just exit with escape
+        e.layer.onCommit()
+      }
     })
     this.on('editable:drawing:commit', function (e) {
+      console.log('editable:drawing:commit')
       e.layer.isDirty = true
       if (this.map.editedFeature !== e.layer) e.layer.edit(e)
       e.layer.onCommit()
     })
     this.on('editable:editing', (e) => {
+      console.log('editable:editing')
       const layer = e.layer
       layer.isDirty = true
       if (layer._tooltip && layer.isTooltipOpen()) {
@@ -1274,11 +1282,13 @@ U.Editable = L.Editable.extend({
       }
     })
     this.on('editable:vertex:ctrlclick', (e) => {
+      console.log('editable:vertex:ctrlclick')
       const index = e.vertex.getIndex()
       if (index === 0 || (index === e.vertex.getLastIndex() && e.vertex.continue))
         e.vertex.continue()
     })
     this.on('editable:vertex:altclick', (e) => {
+      console.log('editable:vertex:altclick')
       if (e.vertex.editor.vertexCanBeDeleted(e.vertex)) e.vertex.delete()
     })
     this.on('editable:vertex:rawclick', this.onVertexRawClick)
@@ -1366,6 +1376,7 @@ U.Editable = L.Editable.extend({
   },
 
   onVertexRawClick: function (e) {
+    console.log('editable:vertex:rawclick')
     e.layer.onVertexRawClick(e)
     L.DomEvent.stop(e)
     e.cancel()

umap/static/umap/js/umap.features.js

@@ -15,9 +15,7 @@ U.FeatureMixin = {
 
   onCommit: function () {
     const { subject, metadata, engine } = this.getSyncMetadata()
-    engine.upsert(subject, metadata, {
+    engine.upsert(subject, metadata, this.toGeoJSON())
-      geometry: this.getGeometry(),
-    })
   },
 
   getGeometry: function () {
@@ -25,6 +23,7 @@ U.FeatureMixin = {
   },
 
   syncUpdatedProperties: function (properties) {
+    // When updating latlng, sync the whole geometry
     if ('latlng'.includes(properties)) {
       const { subject, metadata, engine } = this.getSyncMetadata()
       engine.update(subject, metadata, 'geometry', this.getGeometry())
@@ -44,12 +43,16 @@ U.FeatureMixin = {
     // DataLayer the marker belongs to
     this.datalayer = options.datalayer || null
     this.properties = { _umap_options: {} }
+
+    if (options.geojson) {
+      this.populate(options.geojson)
+    }
+
     if (id) {
       this.id = id
     } else {
       let geojson_id
       if (options.geojson) {
-        this.populate(options.geojson)
         geojson_id = options.geojson.id
       }
 
@@ -189,7 +192,7 @@ U.FeatureMixin = {
   },
 
   getAdvancedEditActions: function (container) {
-    const deleteButton = L.DomUtil.createButton(
+    L.DomUtil.createButton(
       'button umap-delete',
       container,
       L._('Delete'),
@@ -939,6 +942,7 @@ U.PathMixin = {
 
   _onDrag: function () {
     if (this._tooltip) this._tooltip.setLatLng(this.getCenter())
+    this.syncUpdatedProperties(['latlng'])
   },
 
   transferShape: function (at, to) {
@@ -1130,6 +1134,9 @@ U.Polyline = L.Polyline.extend({
     geojson.geometry.coordinates = [
       U.Utils.flattenCoordinates(geojson.geometry.coordinates),
     ]
+
+    delete geojson.id // delete the copied id, a new one will be generated.
+
     const polygon = this.datalayer.geojsonToFeatures(geojson)
     polygon.edit()
     this.del()
@@ -1137,7 +1144,7 @@ U.Polyline = L.Polyline.extend({
 
   getAdvancedEditActions: function (container) {
     U.FeatureMixin.getAdvancedEditActions.call(this, container)
-    const toPolygon = L.DomUtil.createButton(
+    L.DomUtil.createButton(
       'button umap-to-polygon',
       container,
       L._('Transform to polygon'),

umap/static/umap/js/umap.js

@@ -257,6 +257,9 @@ U.Map = L.Map.extend({
     if (this.options.syncEnabled != true) {
       this.sync.stop()
     } else {
+      // FIXME: Do this directly in the sync engine, which should check if the engine
+      // is already started or not.
+
       // Get the authentication token from the server
       // And pass it to the sync engine.
       // FIXME: use `this.urls`

umap/static/umap/js/umap.layer.js

@@ -179,12 +179,18 @@ U.Layer.Choropleth = L.FeatureGroup.extend({
     return +feature.properties[key] // TODO: should we catch values non castable to int ?
   },
 
-  computeBreaks: function () {
+  getValues: function () {
     const values = []
     this.datalayer.eachLayer((layer) => {
       let value = this._getValue(layer)
       if (!isNaN(value)) values.push(value)
     })
+    return values
+  },
+
+  computeBreaks: function () {
+    const values = this.getValues()
+
     if (!values.length) {
       this.options.breaks = []
       this.options.colors = []
@@ -609,6 +615,7 @@ U.DataLayer = L.Evented.extend({
 
   render: function (fields, builder) {
     let impacts = U.Utils.getImpactsFromSchema(fields)
+    console.log('impacts', impacts)
 
     for (let impact of impacts) {
       switch (impact) {
@@ -623,6 +630,7 @@ U.DataLayer = L.Evented.extend({
           fields.forEach((field) => {
             this.layer.onEdit(field, builder)
           })
+          this.redraw()
           this.show()
           break
         case 'remote-data':
@@ -1026,7 +1034,7 @@ U.DataLayer = L.Evented.extend({
       for (i = 0, len = features.length; i < len; i++) {
         this.geojsonToFeatures(features[i])
       }
-      return this
+      return this // Why returning "this" ?
     }
 
     const geometry = geojson.type === 'Feature' ? geojson.geometry : geojson
@@ -1034,6 +1042,7 @@ U.DataLayer = L.Evented.extend({
     let feature = this.geometryToFeature({ geometry, geojson })
     if (feature) {
       this.addLayer(feature)
+      feature.onCommit()
       return feature
     }
   },
@@ -1597,13 +1606,7 @@ U.DataLayer = L.Evented.extend({
   // TODO Add an index
   // For now, iterate on all the features.
   getFeatureById: function (id) {
-    console.log('looking for feature with id ', id)
+    return Object.values(this._layers).find((feature) => feature.id === id)
-    for (const i in this._layers) {
-      let feature = this._layers[i]
-      if (feature.id === id) {
-        return feature
-      }
-    }
   },
 
   getNextFeature: function (feature) {

umap/views.py

@@ -806,7 +806,9 @@ def get_websocket_auth_token(request, map_id, map_inst):
         )
         return simple_json_response(token=signed_token)
     else:
-        return HttpResponseForbidden
+        return HttpResponseForbidden(
+            _("You cannot edit this map with your current permissions.")
+        )
 
 
 class MapUpdate(FormLessEditMixin, PermissionsMixin, UpdateView):

umap/ws.py

@@ -33,7 +33,7 @@ class JoinMessage(BaseModel):
 
 
 class Geometry(BaseModel):
-    type: Literal["Point", "Polygon"]
+    type: Literal["Point", "Polygon", "LineString"]
     coordinates: list
 
 
@@ -70,11 +70,12 @@ async def join_and_listen(
             # as doing so beforehand would miss new connections
             peers = CONNECTIONS[map_id] - {websocket}
             # Only relay valid "operation" messages
-            try:
+            # try:
-                OperationMessage.model_validate_json(raw_message)
+            #    OperationMessage.model_validate_json(raw_message)
-            except ValidationError as e:
+            # except ValidationError as e:
-                print(raw_message, e)
+            print(raw_message)
 
+            # For now, broadcast anyway
             websockets.broadcast(peers, raw_message)
     finally:
         CONNECTIONS[map_id].remove(websocket)