fix(sync): handle sync of datalayer delete (#2416)

fix #2268

There is a tricky choice to do: the delete actually occurs in two
times, first the datalayer is hidden from the UI and set as "deleted"
(this can then be undone) then at next "save" it will totally removed.

When syncing, given we removed the "reset/undo" feature for now, and
because it was simpler, I decide to do both step in once.

When working on a proper "undo/redo", we may challenge this choice
again.

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

@@ -574,9 +574,12 @@ export class DataLayer extends ServerStored {
     })
   }
 
-  _delete() {
+  del(sync = true) {
-    this.isDeleted = true
     this.erase()
+    if (sync) {
+      this.isDeleted = true
+      this.sync.delete()
+    }
   }
 
   empty() {
@@ -819,7 +822,7 @@ export class DataLayer extends ServerStored {
         <i class="icon icon-24 icon-delete"></i>${translate('Delete')}
       </button>`)
     deleteButton.addEventListener('click', () => {
-      this._delete()
+      this.del()
       this._umap.editPanel.close()
     })
     advancedButtons.appendChild(deleteButton)
@@ -1147,10 +1150,14 @@ export class DataLayer extends ServerStored {
     if (this.createdOnServer) {
       await this._umap.server.post(this.getDeleteUrl())
     }
-    delete this._umap.datalayers[stamp(this)]
+    this.commitDelete()
     return true
   }
 
+  commitDelete() {
+    delete this._umap.datalayers[stamp(this)]
+  }
+
   getName() {
     return this.options.name || translate('Untitled layer')
   }
@@ -1221,7 +1228,7 @@ export class DataLayer extends ServerStored {
           this._umap.dialog
             .confirm(translate('Are you sure you want to delete this layer?'))
             .then(() => {
-              this._delete()
+              this.del()
             })
         },
         this

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

@@ -72,6 +72,14 @@ export class DataLayerUpdater extends BaseUpdater {
     }
     datalayer.render([key])
   }
+
+  delete({ metadata }) {
+    const datalayer = this.getDataLayerFromID(metadata.id)
+    if (datalayer) {
+      datalayer.del(false)
+      datalayer.commitDelete()
+    }
+  }
 }
 
 export class FeatureUpdater extends BaseUpdater {

umap/tests/base.py

@@ -127,6 +127,9 @@ class DataLayerFactory(factory.django.DjangoModelFactory):
     def _adjust_kwargs(cls, **kwargs):
         if "data" in kwargs:
             data = copy.deepcopy(kwargs.pop("data"))
+            data.setdefault("_umap_options", {})
+            if "name" in data["_umap_options"] and kwargs["name"] == cls.name:
+                kwargs["name"] = data["_umap_options"]["name"]
             if "settings" not in kwargs:
                 kwargs["settings"] = data.get("_umap_options", {})
         else:
@@ -135,7 +138,6 @@ class DataLayerFactory(factory.django.DjangoModelFactory):
                 **DataLayerFactory.settings._defaults,
                 **kwargs["settings"],
             }
-        data.setdefault("_umap_options", {})
         kwargs["settings"]["name"] = kwargs["name"]
         data["_umap_options"]["name"] = kwargs["name"]
         data.setdefault("type", "FeatureCollection")

umap/tests/integration/test_websocket_sync.py

@@ -420,6 +420,72 @@ def test_should_sync_datalayers(new_page, live_server, websocket_server, tilelay
     assert DataLayer.objects.count() == 2
 
 
+@pytest.mark.xdist_group(name="websockets")
+def test_should_sync_datalayers_delete(
+    new_page, live_server, websocket_server, tilelayer
+):
+    map = MapFactory(name="sync", edit_status=Map.ANONYMOUS)
+    map.settings["properties"]["syncEnabled"] = True
+    map.save()
+    data1 = {
+        "type": "FeatureCollection",
+        "features": [
+            {
+                "type": "Feature",
+                "properties": {
+                    "name": "Point 1",
+                },
+                "geometry": {"type": "Point", "coordinates": [0.065918, 48.385442]},
+            },
+        ],
+        "_umap_options": {
+            "name": "datalayer 1",
+        },
+    }
+    data2 = {
+        "type": "FeatureCollection",
+        "features": [
+            {
+                "type": "Feature",
+                "properties": {
+                    "name": "Point 2",
+                },
+                "geometry": {"type": "Point", "coordinates": [3.55957, 49.767074]},
+            },
+        ],
+        "_umap_options": {
+            "name": "datalayer 2",
+        },
+    }
+    DataLayerFactory(map=map, data=data1)
+    DataLayerFactory(map=map, data=data2)
+
+    # 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")
+
+    peerA.get_by_role("button", name="Open browser").click()
+    expect(peerA.get_by_text("datalayer 1")).to_be_visible()
+    expect(peerA.get_by_text("datalayer 2")).to_be_visible()
+    peerB.get_by_role("button", name="Open browser").click()
+    expect(peerB.get_by_text("datalayer 1")).to_be_visible()
+    expect(peerB.get_by_text("datalayer 2")).to_be_visible()
+
+    # Delete "datalayer 2" in peerA
+    peerA.locator(".datalayer").get_by_role("button", name="Delete layer").first.click()
+    peerA.get_by_role("button", name="OK").click()
+    expect(peerA.get_by_text("datalayer 2")).to_be_hidden()
+    expect(peerB.get_by_text("datalayer 2")).to_be_hidden()
+
+    # Save delete to the server
+    with peerA.expect_response(re.compile(".*/datalayer/delete/.*")):
+        peerA.get_by_role("button", name="Save").click()
+    expect(peerA.get_by_text("datalayer 2")).to_be_hidden()
+    expect(peerB.get_by_text("datalayer 2")).to_be_hidden()
+
+
 @pytest.mark.xdist_group(name="websockets")
 def test_create_and_sync_map(
     new_page, live_server, websocket_server, tilelayer, login, user