From 0c52c35ae389822c98ec2e5d226fb9f2443a9c05 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Fri, 3 Jan 2025 16:05:12 +0100 Subject: [PATCH 1/2] chore(tests): use name from data when defined in DataLayerFactory --- umap/tests/base.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/umap/tests/base.py b/umap/tests/base.py index c1bb44e2..5e382981 100644 --- a/umap/tests/base.py +++ b/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") From 75af1a48556299d6f8aa7c0cfc119d8ed75c06ae Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Fri, 3 Jan 2025 16:06:31 +0100 Subject: [PATCH 2/2] 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 { ${translate('Delete')} `) 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