From 75af1a48556299d6f8aa7c0cfc119d8ed75c06ae Mon Sep 17 00:00:00 2001
From: Yohan Boniface <yohanboniface@free.fr>
Date: Fri, 3 Jan 2025 16:06:31 +0100
Subject: [PATCH] fix(sync): handle sync of datalayer delete

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     | 17 +++--
 umap/static/umap/js/modules/sync/updaters.js  |  8 +++
 umap/tests/integration/test_websocket_sync.py | 66 +++++++++++++++++++
 3 files changed, 86 insertions(+), 5 deletions(-)

diff --git a/umap/static/umap/js/modules/data/layer.js b/umap/static/umap/js/modules/data/layer.js
index a6f77a67..d1be28e9 100644
--- a/umap/static/umap/js/modules/data/layer.js
+++ b/umap/static/umap/js/modules/data/layer.js
@@ -574,9 +574,12 @@ export class DataLayer extends ServerStored {
     })
   }
 
-  _delete() {
-    this.isDeleted = true
+  del(sync = 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
diff --git a/umap/static/umap/js/modules/sync/updaters.js b/umap/static/umap/js/modules/sync/updaters.js
index 75c4a9f2..5ab97375 100644
--- a/umap/static/umap/js/modules/sync/updaters.js
+++ b/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 {
diff --git a/umap/tests/integration/test_websocket_sync.py b/umap/tests/integration/test_websocket_sync.py
index c5e56e89..f4e3d7f8 100644
--- a/umap/tests/integration/test_websocket_sync.py
+++ b/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