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

@@ -20,7 +20,6 @@ import { translate } from '../i18n.js'
 import { DataLayerPermissions } from '../permissions.js'
 import { Point, LineString, Polygon } from './features.js'
 import TableEditor from '../tableeditor.js'
-import { ServerStored } from '../saving.js'
 
 export const LAYER_TYPES = [
   DefaultLayer,
@@ -36,9 +35,8 @@ const LAYER_MAP = LAYER_TYPES.reduce((acc, klass) => {
   return acc
 }, {})
 
-export class DataLayer extends ServerStored {
+export class DataLayer {
   constructor(map, data) {
-    super()
     this.map = map
     this.sync = map.sync_engine.proxy(this)
     this._index = Array()
@@ -60,6 +58,7 @@ export class DataLayer extends ServerStored {
       editMode: 'advanced',
     }
 
+    this._isDirty = false
     this._isDeleted = false
     this.setUmapId(data.id)
     this.setOptions(data)
@@ -90,16 +89,23 @@ export class DataLayer extends ServerStored {
     if (this.isVisible()) this.propagateShow()
   }
 
-  onDirty(status) {
+  set isDirty(status) {
+    this._isDirty = status
     if (status) {
+      this.map.isDirty = true
       // A layer can be made dirty by indirect action (like dragging layers)
       // we need to have it loaded before saving it.
       if (!this.isLoaded()) this.fetchData()
     } else {
+      this.map.checkDirty()
       this.isDeleted = false
     }
   }
 
+  get isDirty() {
+    return this._isDirty
+  }
+
   set isDeleted(status) {
     this._isDeleted = status
     if (status) this.isDirty = status
@@ -576,6 +582,7 @@ export class DataLayer extends ServerStored {
 
   reset() {
     if (!this.umap_id) this.erase()
+
     this.resetOptions()
     this.parentPane.appendChild(this.pane)
     if (this._leaflet_events_bk && !this._leaflet_events) {
@@ -1050,9 +1057,8 @@ export class DataLayer extends ServerStored {
     const headers = this._reference_version
       ? { 'X-Datalayer-Reference': this._reference_version }
       : {}
-    const status = await this._trySave(saveUrl, headers, formData)
+    await this._trySave(saveUrl, headers, formData)
     this._geojson = geojson
-    return status
   }
 
   async _trySave(url, headers, formData) {
@@ -1065,15 +1071,7 @@ export class DataLayer extends ServerStored {
               'This situation is tricky, you have to choose carefully which version is pertinent.'
           ),
           async () => {
-            // Save again this layer
+            await this._trySave(url, {}, formData)
-            const status = await this._trySave(url, {}, formData)
-            if (status) {
-              this.isDirty = false
-
-              // Call the main save, in case something else needs to be saved
-              // as the conflict stopped the saving flow
-              await this.map.saveAll()
-            }
           }
         )
       }
@@ -1091,11 +1089,11 @@ export class DataLayer extends ServerStored {
       this.setUmapId(data.id)
       this.updateOptions(data)
       this.backupOptions()
-      this.backupData()
       this.connectToMap()
       this._loaded = true
       this.redraw() // Needed for reordering features
-      return true
+      this.isDirty = false
+      this.permissions.save()
     }
   }
 
@@ -1104,7 +1102,7 @@ export class DataLayer extends ServerStored {
       await this.map.server.post(this.getDeleteUrl())
     }
     delete this.map.datalayers[stamp(this)]
-    return true
+    this.isDirty = false
   }
 
   getMap() {
@@ -1132,9 +1130,7 @@ export class DataLayer extends ServerStored {
   }
 
   renderLegend() {
-    for (const container of document.querySelectorAll(
+    for (const container of document.querySelectorAll(`.${this.cssId} .datalayer-legend`)) {
-      `.${this.cssId} .datalayer-legend`
-    )) {
       container.innerHTML = ''
       if (this.layer.renderLegend) return this.layer.renderLegend(container)
       const color = DomUtil.create('span', 'datalayer-color', container)

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

@@ -31,7 +31,6 @@ import { DataLayer, LAYER_TYPES } from './data/layer.js'
 import { DataLayerPermissions, MapPermissions } from './permissions.js'
 import { Point, LineString, Polygon } from './data/features.js'
 import { LeafletMarker, LeafletPolyline, LeafletPolygon } from './rendering/ui.js'
-import * as SAVEMANAGER from './saving.js'
 
 // Import modules and export them to the global scope.
 // For the not yet module-compatible JS out there.
@@ -71,7 +70,6 @@ window.U = {
   Request,
   RequestError,
   Rules,
-  SAVEMANAGER,
   SCHEMA,
   ServerRequest,
   Share,

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

@@ -1,19 +1,26 @@
 import { DomUtil } from '../../vendors/leaflet/leaflet-src.esm.js'
 import { translate } from './i18n.js'
 import { uMapAlert as Alert } from '../components/alerts/alert.js'
-import { ServerStored } from './saving.js'
 import * as Utils from './utils.js'
 
 // Dedicated object so we can deal with a separate dirty status, and thus
 // call the endpoint only when needed, saving one call at each save.
-export class MapPermissions extends ServerStored {
+export class MapPermissions {
   constructor(map) {
-    super()
     this.setOptions(map.options.permissions)
     this.map = map
     this._isDirty = false
   }
 
+  set isDirty(status) {
+    this._isDirty = status
+    if (status) this.map.isDirty = status
+  }
+
+  get isDirty() {
+    return this._isDirty
+  }
+
   setOptions(options) {
     this.options = Object.assign(
       {
@@ -193,8 +200,8 @@ export class MapPermissions extends ServerStored {
     )
     if (!error) {
       this.commit()
+      this.isDirty = false
       this.map.fire('postsync')
-      return true
     }
   }
 
@@ -226,9 +233,8 @@ export class MapPermissions extends ServerStored {
   }
 }
 
-export class DataLayerPermissions extends ServerStored {
+export class DataLayerPermissions {
   constructor(datalayer) {
-    super()
     this.options = Object.assign(
       {
         edit_status: null,
@@ -237,6 +243,16 @@ export class DataLayerPermissions extends ServerStored {
     )
 
     this.datalayer = datalayer
+    this._isDirty = false
+  }
+
+  set isDirty(status) {
+    this._isDirty = status
+    if (status) this.datalayer.isDirty = status
+  }
+
+  get isDirty() {
+    return this._isDirty
   }
 
   get map() {
@@ -281,13 +297,12 @@ export class DataLayerPermissions extends ServerStored {
     )
     if (!error) {
       this.commit()
-      return true
+      this.isDirty = false
     }
   }
 
   commit() {
     this.datalayer.options.permissions = Object.assign(
-      {},
       this.datalayer.options.permissions,
       this.options
     )

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

@@ -1,48 +0,0 @@
-const _queue = new Set()
-
-export let isDirty = false
-
-export async function save() {
-  for (const obj of _queue) {
-    const ok = await obj.save()
-    if (!ok) break
-    remove(obj)
-  }
-}
-
-export function add(obj) {
-  _queue.add(obj)
-  _onUpdate()
-}
-
-export function remove(obj) {
-  _queue.delete(obj)
-  _onUpdate()
-}
-
-export function has(obj) {
-  return _queue.has(obj)
-}
-
-function _onUpdate() {
-  console.log(_queue)
-  isDirty = Boolean(_queue.size)
-  document.body.classList.toggle('umap-is-dirty', isDirty)
-}
-
-export class ServerStored {
-  set isDirty(status) {
-    if (status) {
-      add(this)
-    } else {
-      remove(this)
-    }
-    this.onDirty(status)
-  }
-
-  get isDirty() {
-    return has(this)
-  }
-
-  onDirty(status) {}
-}

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

@@ -734,7 +734,7 @@ const ControlsMixin = {
       'leaflet-control-edit-save button',
       rightContainer,
       L.DomUtil.add('span', '', null, L._('Save')),
-      this.saveAll,
+      this.save,
       this
     )
     L.DomEvent.on(

umap/static/umap/js/umap.js

@@ -153,17 +153,13 @@ U.Map = L.Map.extend({
       this.options.onLoadPanel = 'datafilters'
     }
 
-    // TODO: remove me when moved to modules
+    let isDirty = false // self status
-    // and inheriting from ServerStored
     try {
       Object.defineProperty(this, 'isDirty', {
-        get: () => U.SAVEMANAGER.has(this),
+        get: () => isDirty,
-        set: (status) => {
+        set: function (status) {
-          if (status) {
+          isDirty = status
-            U.SAVEMANAGER.add(this)
+          this.checkDirty()
-          } else {
-            U.SAVEMANAGER.remove(this)
-          }
         },
       })
     } catch (e) {
@@ -204,7 +200,7 @@ U.Map = L.Map.extend({
       this.propagate()
     }
 
-    window.onbeforeunload = () => (this.editEnabled && U.SAVEMANAGER.isDirty) || null
+    window.onbeforeunload = () => (this.editEnabled && this.isDirty) || null
     this.backup()
   },
 
@@ -605,13 +601,13 @@ U.Map = L.Map.extend({
       let used = true
       switch (e.key) {
         case 'e':
-          if (!U.SAVEMANAGER.isDirty) this.disableEdit()
+          if (!this.isDirty) this.disableEdit()
           break
         case 's':
-          if (U.SAVEMANAGER.isDirty) this.saveAll()
+          if (this.isDirty) this.save()
           break
         case 'z':
-          if (U.SAVEMANAGER.isDirty) this.askForReset()
+          if (this.isDirty) this.askForReset()
           break
         case 'm':
           this.editTools.startMarker()
@@ -1019,6 +1015,10 @@ U.Map = L.Map.extend({
     this.onDataLayersChanged()
   },
 
+  checkDirty: function () {
+    this._container.classList.toggle('umap-is-dirty', this.isDirty)
+  },
+
   exportOptions: function () {
     const properties = {}
     for (const option of Object.keys(U.SCHEMA)) {
@@ -1029,7 +1029,7 @@ U.Map = L.Map.extend({
     return properties
   },
 
-  save: async function () {
+  saveSelf: async function () {
     this.rules.commit()
     const geojson = {
       type: 'Feature',
@@ -1048,7 +1048,7 @@ U.Map = L.Map.extend({
       return
     }
     if (data.login_required) {
-      window.onLogin = () => this.saveAll()
+      window.onLogin = () => this.save()
       window.open(data.login_required)
       return
     }
@@ -1093,11 +1093,21 @@ U.Map = L.Map.extend({
     return true
   },
 
-  saveAll: async function () {
+  save: async function () {
-    if (!U.SAVEMANAGER.isDirty) return
+    if (!this.isDirty) return
     if (this._default_extent) this._setCenterAndZoom()
     this.backup()
-    await U.SAVEMANAGER.save()
+    if (this.options.editMode === 'advanced') {
+      // Only save the map if the user has the rights to do so.
+      const ok = await this.saveSelf()
+      if (!ok) return
+    }
+    await this.permissions.save()
+    // Iter over all datalayers, including deleted.
+    for (const datalayer of Object.values(this.datalayers)) {
+      if (datalayer.isDirty) await datalayer.save()
+    }
+    this.isDirty = false
     // Do a blind render for now, as we are not sure what could
     // have changed, we'll be more subtil when we'll remove the
     // save action

umap/static/umap/map.css

@@ -596,7 +596,7 @@ ul.photon-autocomplete {
 }
 .umap-edit-enabled .leaflet-control-edit-save,
 .umap-edit-enabled .leaflet-control-edit-disable,
-.umap-edit-enabled.umap-is-dirty .leaflet-control-edit-cancel {
+.umap-edit-enabled .umap-is-dirty .leaflet-control-edit-cancel {
     display: inline-block;
 }
 .umap-is-dirty .leaflet-control-edit-disable {

umap/tests/integration/test_edit_datalayer.py

@@ -120,7 +120,7 @@ def test_can_change_name(live_server, openmap, page, datalayer):
     page.locator('input[name="name"]').press("Control+a")
     page.locator('input[name="name"]').fill("new name")
     expect(page.locator(".umap-browser .datalayer")).to_contain_text("new name")
-    expect(page.locator("body")).to_have_class(re.compile(".*umap-is-dirty.*"))
+    expect(page.locator(".umap-is-dirty")).to_be_visible()
     with page.expect_response(re.compile(".*/datalayer/update/.*")):
         page.get_by_role("button", name="Save").click()
     saved = DataLayer.objects.last()

umap/tests/integration/test_optimistic_merge.py

@@ -276,38 +276,21 @@ def test_should_display_alert_on_conflict(context, live_server, datalayer, openm
     page_two = context.new_page()
     page_two.goto(f"{live_server.url}{openmap.get_absolute_url()}?edit")
 
-    # Change name on page one and save
     page_one.locator(".leaflet-marker-icon").click(modifiers=["Shift"])
-    page_one.locator('input[name="name"]').fill("name from page one")
+    page_one.locator('input[name="name"]').fill("new name")
     with page_one.expect_response(re.compile(r".*/datalayer/update/.*")):
         page_one.get_by_role("button", name="Save").click()
 
-    # Change name on page two and save
     page_two.locator(".leaflet-marker-icon").click(modifiers=["Shift"])
-    page_two.locator('input[name="name"]').fill("name from page two")
+    page_two.locator('input[name="name"]').fill("custom name")
-
-    # Map should be in dirty status
-    expect(page_two.get_by_text("Cancel edits")).to_be_visible()
     with page_two.expect_response(re.compile(r".*/datalayer/update/.*")):
         page_two.get_by_role("button", name="Save").click()
-
-    # Make sure data is unchanged on the server
     saved = DataLayer.objects.last()
     data = json.loads(Path(saved.geojson.path).read_text())
-    assert data["features"][0]["properties"]["name"] == "name from page one"
+    assert data["features"][0]["properties"]["name"] == "new name"
-
-    # We should have an alert with some actions
     expect(page_two.get_by_text("Whoops! Other contributor(s) changed")).to_be_visible()
-    # Map should still be in dirty status
-    expect(page_two.get_by_text("Cancel edits")).to_be_visible()
-
-    # Override data from page two
     with page_two.expect_response(re.compile(r".*/datalayer/update/.*")):
         page_two.get_by_text("Keep your changes and loose theirs").click()
-
-    # Make sure server has page two data
     saved = DataLayer.objects.last()
     data = json.loads(Path(saved.geojson.path).read_text())
-    assert data["features"][0]["properties"]["name"] == "name from page two"
+    assert data["features"][0]["properties"]["name"] == "custom name"
-    # Map should not be in dirty status anymore
-    expect(page_two.get_by_text("Cancel edits")).to_be_hidden()

umap/tests/integration/test_save.py

@@ -1,35 +0,0 @@
-import re
-
-
-def test_reseting_map_would_remove_from_save_queue(
-    live_server, openmap, page, datalayer
-):
-    page.goto(f"{live_server.url}{openmap.get_absolute_url()}?edit")
-    page.get_by_role("link", name="Edit map name and caption").click()
-    requests = []
-
-    def register_request(request):
-        if request.url.endswith(".png"):
-            return
-        requests.append((request.method, request.url))
-
-    page.on("request", register_request)
-    page.locator('input[name="name"]').click()
-    page.locator('input[name="name"]').fill("new name")
-    page.get_by_role("button", name="Cancel edits").click()
-    page.get_by_role("button", name="OK").click()
-    page.wait_for_timeout(500)
-    page.get_by_role("button", name="Edit").click()
-    page.get_by_role("link", name="Manage layers").click()
-    page.get_by_role("button", name="Edit", exact=True).click()
-    page.locator('input[name="name"]').click()
-    page.locator('input[name="name"]').fill("new datalayer name")
-    with page.expect_response(re.compile(".*/datalayer/update/.*")):
-        page.get_by_role("button", name="Save").click()
-    assert len(requests) == 1
-    assert requests == [
-        (
-            "POST",
-            f"{live_server.url}/en/map/{openmap.pk}/datalayer/update/{datalayer.pk}/",
-        ),
-    ]