Merge f105336d48 into 4dd50690b1

Broken since c27e675152

docs-users/requirements.txt

@@ -1,5 +1,5 @@
 # Force rtfd to use a recent version of mkdocs
 mkdocs==1.6.1
 pymdown-extensions==10.14.3
-mkdocs-material==9.5.50
+mkdocs-material==9.6.3
 mkdocs-static-i18n==1.3.0

docs/requirements.txt

@@ -1,5 +1,5 @@
 # Force rtfd to use a recent version of mkdocs
 mkdocs==1.6.1
 pymdown-extensions==10.14.3
-mkdocs-material==9.5.50
+mkdocs-material==9.6.3
 mkdocs-static-i18n==1.3.0

pyproject.toml

@@ -28,7 +28,7 @@ classifiers = [
   "Programming Language :: Python :: 3.12",
 ]
 dependencies = [
-  "Django==5.1.5",
+  "Django==5.1.6",
   "django-agnocomplete==2.2.0",
   "django-environ==0.12.0",
   "django-probes==1.7.0",
@@ -44,10 +44,10 @@ dependencies = [
 [project.optional-dependencies]
 dev = [
   "hatch==1.14.0",
-  "ruff==0.9.3",
+  "ruff==0.9.6",
   "djlint==1.36.4",
   "mkdocs==1.6.1",
-  "mkdocs-material==9.5.50",
+  "mkdocs-material==9.6.3",
   "mkdocs-static-i18n==1.3.0",
   "vermin==1.6.0",
   "pymdown-extensions==10.14.3",
@@ -58,11 +58,11 @@ test = [
   "factory-boy==3.3.3",
   "playwright>=1.39",
   "pytest==8.3.4",
-  "pytest-django==4.9.0",
+  "pytest-django==4.10.0",
   "pytest-playwright==0.7.0",
   "pytest-rerunfailures==15.0",
   "pytest-xdist>=3.5.0,<4",
-  "moto[s3]==5.0.27"
+  "moto[s3]==5.0.28"
 ]
 docker = [
   "uwsgi==2.0.28",

umap/models.py

@@ -523,17 +523,27 @@ class DataLayer(NamedModel):
     def metadata(self, request=None):
         # Retrocompat: minimal settings for maps not saved after settings property
         # has been introduced
-        obj = self.settings or {
-            "name": self.name,
-            "displayOnLoad": self.display_on_load,
-        }
+        metadata = self.settings
+        if not metadata:
+            # Fallback to file for old datalayers.
+            data = json.loads(self.geojson.read().decode())
+            metadata = data.get("_umap_options")
+            if not metadata:
+                metadata = {
+                    "name": self.name,
+                    "displayOnLoad": self.display_on_load,
+                }
+            # Save it to prevent file reading at each map load.
+            self.settings = metadata
+            # Do not update the modified_at.
+            self.save(update_fields=["settings"])
         if self.old_id:
-            obj["old_id"] = self.old_id
-        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
+            metadata["old_id"] = self.old_id
+        metadata["id"] = self.pk
+        metadata["permissions"] = {"edit_status": self.edit_status}
+        metadata["editMode"] = "advanced" if self.can_edit(request) else "disabled"
+        metadata["_referenceVersion"] = self.reference_version
+        return metadata
 
     def clone(self, map_inst=None):
         new = self.__class__.objects.get(pk=self.pk)

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

@@ -45,8 +45,6 @@ export class DataLayer extends ServerStored {
     this._features = {}
     this._geojson = null
     this._propertiesIndex = []
-    this._loaded = false // Are layer metadata loaded
-    this._dataloaded = false // Are layer data loaded
 
     this._leafletMap = leafletMap
     this.parentPane = this._leafletMap.getPane('overlayPane')
@@ -85,6 +83,7 @@ export class DataLayer extends ServerStored {
     this.connectToMap()
     this.permissions = new DataLayerPermissions(this._umap, this)
 
+    this._needsFetch = this.createdOnServer
     if (!this.createdOnServer) {
       if (this.showAtLoad()) this.show()
     }
@@ -243,7 +242,7 @@ export class DataLayer extends ServerStored {
   }
 
   dataChanged() {
-    if (!this.hasDataLoaded()) return
+    if (!this.isLoaded()) return
     this._umap.onDataLayersChanged()
     this.layer.dataChanged()
   }
@@ -252,13 +251,13 @@ export class DataLayer extends ServerStored {
     if (!geojson) return []
     const features = this.addData(geojson, sync)
     this._geojson = geojson
+    this._needsFetch = false
     this.onDataLoaded()
     this.dataChanged()
     return features
   }
 
   onDataLoaded() {
-    this._dataloaded = true
     this.renderLegend()
   }
 
@@ -268,7 +267,6 @@ export class DataLayer extends ServerStored {
     if (geojson._umap_options) this.setOptions(geojson._umap_options)
     if (this.isRemoteLayer()) await this.fetchRemoteData()
     else this.fromGeoJSON(geojson, false)
-    this._loaded = true
   }
 
   clear() {
@@ -320,7 +318,7 @@ export class DataLayer extends ServerStored {
 
   async fetchRemoteData(force) {
     if (!this.isRemoteLayer()) return
-    if (!this.hasDynamicData() && this.hasDataLoaded() && !force) return
+    if (!this.hasDynamicData() && this.isLoaded() && !force) return
     if (!this.isVisible()) return
     // Keep non proxied url for later use in Alert.
     const remoteUrl = this._umap.renderUrl(this.options.remoteData.url)
@@ -345,11 +343,7 @@ export class DataLayer extends ServerStored {
   }
 
   isLoaded() {
-    return !this.createdOnServer || this._loaded
-  }
-
-  hasDataLoaded() {
-    return this._dataloaded
+    return !this._needsFetch
   }
 
   backupOptions() {
@@ -633,8 +627,6 @@ export class DataLayer extends ServerStored {
     this.propagateDelete()
     this._leaflet_events_bk = this._leaflet_events
     this.clear()
-    delete this._loaded
-    delete this._dataloaded
   }
 
   reset() {
@@ -652,7 +644,6 @@ export class DataLayer extends ServerStored {
     this.hide()
     if (this.isRemoteLayer()) this.fetchRemoteData()
     else if (this._geojson_bk) this.fromGeoJSON(this._geojson_bk)
-    this._loaded = true
     this.show()
     this.isDirty = false
   }
@@ -1108,9 +1099,7 @@ export class DataLayer extends ServerStored {
 
   async save() {
     if (this.isDeleted) return await this.saveDelete()
-    if (!this.isLoaded()) {
-      return
-    }
+    if (!this.isLoaded()) return
     const geojson = this.umapGeoJSON()
     const formData = new FormData()
     formData.append('name', this.options.name)
@@ -1172,7 +1161,6 @@ export class DataLayer extends ServerStored {
       this.backupOptions()
       this.backupData()
       this.connectToMap()
-      this._loaded = true
       this.redraw() // Needed for reordering features
       return true
     }

umap/static/umap/js/modules/form/builder.js

@@ -63,7 +63,7 @@ export class Form extends Utils.WithEvents {
       try {
         value = value[sub]
       } catch {
-        console.log(field)
+        console.debug(field)
       }
     }
     return value
@@ -142,49 +142,50 @@ export class MutatingForm extends Form {
       slugKey: 'PropertyInput',
       labelKey: 'PropertyInput',
     }
-    for (const [key, schema] of Object.entries(SCHEMA)) {
-      if (schema.type === Boolean) {
-        if (schema.nullable) schema.handler = 'NullableChoices'
-        else schema.handler = 'Switch'
-      } else if (schema.type === 'Text') {
-        schema.handler = 'Textarea'
-      } else if (schema.type === Number) {
-        if (schema.step) schema.handler = 'Range'
-        else schema.handler = 'IntInput'
-      } else if (schema.choices) {
-        const text_length = schema.choices.reduce(
+    for (const [key, defaults] of Object.entries(SCHEMA)) {
+      const properties = Object.assign({}, defaults)
+      if (properties.type === Boolean) {
+        if (properties.nullable) properties.handler = 'NullableChoices'
+        else properties.handler = 'Switch'
+      } else if (properties.type === 'Text') {
+        properties.handler = 'Textarea'
+      } else if (properties.type === Number) {
+        if (properties.step) properties.handler = 'Range'
+        else properties.handler = 'IntInput'
+      } else if (properties.choices) {
+        const text_length = properties.choices.reduce(
           (acc, [_, label]) => acc + label.length,
           0
         )
         // Try to be smart and use MultiChoice only
         // for choices where labels are shorts…
         if (text_length < 40) {
-          schema.handler = 'MultiChoice'
+          properties.handler = 'MultiChoice'
         } else {
-          schema.handler = 'Select'
-          schema.selectOptions = schema.choices
+          properties.handler = 'Select'
+          properties.selectOptions = properties.choices
         }
       } else {
         switch (key) {
           case 'color':
           case 'fillColor':
-            schema.handler = 'ColorPicker'
+            properties.handler = 'ColorPicker'
             break
           case 'iconUrl':
-            schema.handler = 'IconUrl'
+            properties.handler = 'IconUrl'
             break
           case 'licence':
-            schema.handler = 'LicenceChooser'
+            properties.handler = 'LicenceChooser'
             break
         }
       }
 
       if (customHandlers[key]) {
-        schema.handler = customHandlers[key]
+        properties.handler = customHandlers[key]
       }
       // Input uses this key for its type attribute
-      delete schema.type
-      this.defaultProperties[key] = schema
+      delete properties.type
+      this.defaultProperties[key] = properties
     }
   }
 
@@ -202,7 +203,7 @@ export class MutatingForm extends Form {
   getTemplate(helper) {
     let template
     if (helper.properties.inheritable) {
-      const extraClassName = helper.get(true) === undefined ? ' undefined' : ''
+      const extraClassName = this.getter(helper.field) === undefined ? ' undefined' : ''
       template = `
         <div class="umap-field-${helper.name} formbox inheritable${extraClassName}">
           <div class="header" data-ref=header>

umap/static/umap/js/modules/form/fields.js

@@ -80,11 +80,13 @@ class BaseElement {
     this.input.value = ''
   }
 
-  get(own) {
-    if (!this.properties.inheritable || own) return this.builder.getter(this.field)
+  get() {
+    if (!this.properties.inheritable) return this.builder.getter(this.field)
     const path = this.field.split('.')
     const key = path[path.length - 1]
-    return this.obj.getOption(key) || SCHEMA[key]?.default
+    const value = this.obj.getOption(key)
+    if (value === undefined) return SCHEMA[key]?.default
+    return value
   }
 
   toHTML() {
@@ -1164,7 +1166,7 @@ Fields.MultiChoice = class extends BaseElement {
 
 Fields.TernaryChoices = class extends Fields.MultiChoice {
   getDefault() {
-    return 'null'
+    return null
   }
 
   toJS() {
@@ -1195,7 +1197,7 @@ Fields.NullableChoices = class extends Fields.TernaryChoices {
       this.properties.choices || [
         [true, translate('always')],
         [false, translate('never')],
-        ['null', translate('hidden')],
+        [null, translate('hidden')],
       ]
     )
   }

umap/static/umap/js/modules/rendering/layers/classified.js

@@ -75,7 +75,7 @@ const ClassifiedMixin = {
   },
 
   renderLegend: function (container) {
-    if (!this.datalayer.hasDataLoaded()) return
+    if (!this.datalayer.isLoaded()) return
     const parent = DomUtil.create('ul', '', container)
     const items = this.getLegendItems()
     for (const [color, label] of items) {

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

@@ -10,6 +10,11 @@ export async function save() {
   }
 }
 
+export function clear() {
+  _queue.clear()
+  onUpdate()
+}
+
 function add(obj) {
   _queue.add(obj)
   onUpdate()

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

@@ -450,7 +450,7 @@ export const SCHEMA = {
     choices: [
       [true, translate('always')],
       [false, translate('never')],
-      ['null', translate('on hover')],
+      [null, translate('on hover')],
     ],
   },
   slideshow: {

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

@@ -2,6 +2,7 @@ import * as Utils from '../utils.js'
 import { HybridLogicalClock } from './hlc.js'
 import { DataLayerUpdater, FeatureUpdater, MapUpdater } from './updaters.js'
 import { WebSocketTransport } from './websocket.js'
+import * as SaveManager from '../saving.js'
 
 // Start reconnecting after 2 seconds, then double the delay each time
 // maxing out at 32 seconds.
@@ -125,6 +126,16 @@ export class SyncEngine {
     this._send({ verb: 'delete', subject, metadata, key })
   }
 
+  saved() {
+    if (this.offline) return
+    if (this.transport) {
+      this.transport.send('SavedMessage', {
+        sender: this.peerId,
+        lastKnownHLC: this._operations.getLastKnownHLC(),
+      })
+    }
+  }
+
   _send(inputMessage) {
     const message = this._operations.addLocal(inputMessage)
 
@@ -168,6 +179,8 @@ export class SyncEngine {
       } else if (payload.message.verb === 'ListOperationsResponse') {
         this.onListOperationsResponse(payload)
       }
+    } else if (kind === 'SavedMessage') {
+      this.onSavedMessage(payload)
     } else {
       throw new Error(`Received unknown message from the websocket server: ${kind}`)
     }
@@ -280,6 +293,13 @@ export class SyncEngine {
     // Else: apply
   }
 
+  onSavedMessage({ sender, lastKnownHLC }) {
+    debug(`received saved message from peer ${sender}`, lastKnownHLC)
+    if (lastKnownHLC === this._operations.getLastKnownHLC() && SaveManager.isDirty) {
+      SaveManager.clear()
+    }
+  }
+
   /**
    * Send a message to another peer (via the transport layer)
    *
@@ -350,7 +370,7 @@ export class Operations {
   }
 
   /**
-   * Tick the clock and add store the passed message in the operations list.
+   * Tick the clock and store the passed message in the operations list.
    *
    * @param {*} inputMessage
    * @returns {*} clock-aware message

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

@@ -55,9 +55,6 @@ export class DataLayerUpdater extends BaseUpdater {
   upsert({ value }) {
     // Upsert only happens when a new datalayer is created.
     const datalayer = this._umap.createDataLayer(value, false)
-    // Prevent the layer to get data from the server, as it will get it
-    // from the sync.
-    datalayer._loaded = true
   }
 
   update({ key, metadata, value }) {
@@ -92,7 +89,7 @@ export class FeatureUpdater extends BaseUpdater {
   upsert({ metadata, value }) {
     const { id, layerId } = metadata
     const datalayer = this.getDataLayerFromID(layerId)
-    const feature = this.getFeatureFromMetadata(metadata, value)
+    const feature = this.getFeatureFromMetadata(metadata)
 
     if (feature) {
       feature.geometry = value.geometry
@@ -109,7 +106,7 @@ export class FeatureUpdater extends BaseUpdater {
       return
     }
     if (key === 'geometry') {
-      const feature = this.getFeatureFromMetadata(metadata, value)
+      const feature = this.getFeatureFromMetadata(metadata)
       feature.geometry = value
     } else {
       this.updateObjectValue(feature, key, value)

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

@@ -684,6 +684,7 @@ export default class Umap extends ServerStored {
         Alert.success(translate('Map has been saved!'))
       })
     }
+    this.sync.saved()
     this.fire('saved')
   }
 

umap/sync/app.py

@@ -14,6 +14,7 @@ from .payloads import (
     OperationMessage,
     PeerMessage,
     Request,
+    SavedMessage,
 )
 
 
@@ -165,6 +166,10 @@ class Peer:
                 case OperationMessage():
                     await self.broadcast(text_data)
 
+                # Broadcast the new map state to connected peers
+                case SavedMessage():
+                    await self.broadcast(text_data)
+
                 # Send peer messages to the proper peer
                 case PeerMessage():
                     await self.send_to(incoming.root.recipient, text_data)

umap/sync/payloads.py

@@ -30,10 +30,17 @@ class PeerMessage(BaseModel):
     message: dict
 
 
+class SavedMessage(BaseModel):
+    kind: Literal["SavedMessage"] = "SavedMessage"
+    lastKnownHLC: str
+
+
 class Request(RootModel):
     """Any message coming from the websocket should be one of these, and will be rejected otherwise."""
 
-    root: Union[PeerMessage, OperationMessage] = Field(discriminator="kind")
+    root: Union[PeerMessage, OperationMessage, SavedMessage] = Field(
+        discriminator="kind"
+    )
 
 
 class JoinResponse(BaseModel):

umap/tests/integration/test_edit_map.py

@@ -210,3 +210,19 @@ def test_sortkey_impacts_datalayerindex(map, live_server, page):
     assert "Z First" == first_listed_feature.text_content()
     assert "Y Second" == second_listed_feature.text_content()
     assert "X Third" == third_listed_feature.text_content()
+
+
+def test_hover_tooltip_setting_should_be_persistent(live_server, map, page):
+    map.settings["properties"]["showLabel"] = None
+    map.edit_status = Map.ANONYMOUS
+    map.save()
+    page.goto(f"{live_server.url}{map.get_absolute_url()}?edit")
+    page.get_by_role("button", name="Map advanced properties").click()
+    page.get_by_text("Default interaction options").click()
+    expect(page.get_by_text("on hover")).to_be_visible()
+    expect(page.locator(".umap-field-showLabel")).to_match_aria_snapshot("""
+        - text: Display label
+        - button "clear"
+        - text: always never on hover
+    """)
+    expect(page.locator(".umap-field-showLabel input[value=null]")).to_be_checked()

umap/tests/integration/test_view_marker.py

@@ -181,3 +181,13 @@ def test_only_visible_markers_are_added_to_dom(live_server, map, page):
     )
     expect(markers).to_have_count(1)
     expect(tooltips).to_have_count(1)
+
+
+def test_should_display_tooltip_on_hover(live_server, map, page, bootstrap):
+    map.settings["properties"]["showLabel"] = None
+    map.settings["properties"]["labelKey"] = "Foo {name}"
+    map.save()
+    page.goto(f"{live_server.url}{map.get_absolute_url()}")
+    expect(page.get_by_text("Foo test marker")).to_be_hidden()
+    page.locator(".leaflet-marker-icon").hover()
+    expect(page.get_by_text("Foo test marker")).to_be_visible()

umap/tests/integration/test_websocket_sync.py

@@ -398,7 +398,9 @@ def test_should_sync_datalayers(new_page, asgi_live_server, tilelayer):
     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()
+    # Show features for this layer in the brower.
+    peerB.get_by_role("heading", name="Layer 2").locator(".datalayer-name").click()
+    expect(peerB.locator("li").filter(has_text="Layer 2")).to_be_visible()
     peerB.locator(".panel.left").get_by_role("button", name="Show/hide layer").nth(
         1
     ).click()
@@ -421,9 +423,11 @@ def test_should_sync_datalayers(new_page, asgi_live_server, tilelayer):
         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()
+    # Peer A should not be in dirty state
+    expect(peerA.locator("body")).not_to_have_class(re.compile(".*umap-is-dirty.*"))
+
+    # Peer A should only have two markers
+    expect(peerA.locator(".leaflet-marker-icon")).to_have_count(2)
 
     assert DataLayer.objects.count() == 2
 
@@ -557,3 +561,46 @@ def test_create_and_sync_map(new_page, asgi_live_server, tilelayer, login, user)
     peerA.get_by_role("button", name="Edit").click()
     expect(markersA).to_have_count(2)
     expect(markersB).to_have_count(2)
+
+
+@pytest.mark.xdist_group(name="websockets")
+def test_should_sync_saved_status(new_page, asgi_live_server, tilelayer):
+    map = MapFactory(name="sync", edit_status=Map.ANONYMOUS)
+    map.settings["properties"]["syncEnabled"] = True
+    map.save()
+
+    # Create two tabs
+    peerA = new_page("Page A")
+    peerA.goto(f"{asgi_live_server.url}{map.get_absolute_url()}?edit")
+    peerB = new_page("Page B")
+    peerB.goto(f"{asgi_live_server.url}{map.get_absolute_url()}?edit")
+
+    # Create a new marker from peerA
+    peerA.get_by_title("Draw a marker").click()
+    peerA.locator("#map").click(position={"x": 220, "y": 220})
+
+    # Peer A should be in dirty state
+    expect(peerA.locator("body")).to_have_class(re.compile(".*umap-is-dirty.*"))
+
+    # Peer B should not be in dirty state
+    expect(peerB.locator("body")).not_to_have_class(re.compile(".*umap-is-dirty.*"))
+
+    # Create a new marker from peerB
+    peerB.get_by_title("Draw a marker").click()
+    peerB.locator("#map").click(position={"x": 200, "y": 250})
+
+    # Peer B should be in dirty state
+    expect(peerB.locator("body")).to_have_class(re.compile(".*umap-is-dirty.*"))
+
+    # Peer A should still be in dirty state
+    expect(peerA.locator("body")).to_have_class(re.compile(".*umap-is-dirty.*"))
+
+    # Save layer to the server from peerA
+    with peerA.expect_response(re.compile(".*/datalayer/create/.*")):
+        peerA.get_by_role("button", name="Save").click()
+
+    # Peer B should not be in dirty state
+    expect(peerB.locator("body")).not_to_have_class(re.compile(".*umap-is-dirty.*"))
+
+    # Peer A should not be in dirty state
+    expect(peerA.locator("body")).not_to_have_class(re.compile(".*umap-is-dirty.*"))