diff --git a/umap/static/umap/js/modules/data/layer.js b/umap/static/umap/js/modules/data/layer.js index ce1f83a2..9edd48a4 100644 --- a/umap/static/umap/js/modules/data/layer.js +++ b/umap/static/umap/js/modules/data/layer.js @@ -88,7 +88,6 @@ export class DataLayer extends ServerStored { if (!this.createdOnServer) { if (this.showAtLoad()) this.show() - this.isDirty = true } // Only layers that are displayed on load must be hidden/shown @@ -151,7 +150,6 @@ export class DataLayer extends ServerStored { for (const field of fields) { this.layer.onEdit(field, builder) } - this.redraw() this.show() break case 'remote-data': @@ -592,7 +590,7 @@ export class DataLayer extends ServerStored { options.name = translate('Clone of {name}', { name: this.options.name }) delete options.id const geojson = Utils.CopyJSON(this._geojson) - const datalayer = this._umap.createDataLayer(options) + const datalayer = this._umap.createDirtyDataLayer(options) datalayer.fromGeoJSON(geojson) return datalayer } @@ -1066,7 +1064,7 @@ export class DataLayer extends ServerStored { setReferenceVersion({ response, sync }) { this._referenceVersion = response.headers.get('X-Datalayer-Version') - this.sync.update('_referenceVersion', this._referenceVersion) + if (sync) this.sync.update('_referenceVersion', this._referenceVersion) } async save() { diff --git a/umap/static/umap/js/modules/importer.js b/umap/static/umap/js/modules/importer.js index 02ebd340..1da6d566 100644 --- a/umap/static/umap/js/modules/importer.js +++ b/umap/static/umap/js/modules/importer.js @@ -161,7 +161,7 @@ export default class Importer extends Utils.WithTemplate { get layer() { return ( this._umap.datalayers[this.layerId] || - this._umap.createDataLayer({ name: this.layerName }) + this._umap.createDirtyDataLayer({ name: this.layerName }) ) } diff --git a/umap/static/umap/js/modules/sync/engine.js b/umap/static/umap/js/modules/sync/engine.js index 9ffd3dcf..9094b482 100644 --- a/umap/static/umap/js/modules/sync/engine.js +++ b/umap/static/umap/js/modules/sync/engine.js @@ -222,9 +222,13 @@ export class SyncEngine { * @param {string} payload.sender the uuid of the requesting peer * @param {string} payload.latestKnownHLC the latest known HLC of the requesting peer */ - onListOperationsRequest({ sender, lastKnownHLC }) { + onListOperationsRequest({ sender, message }) { + debug( + `received operations request from peer ${sender} (since ${message.lastKnownHLC})` + ) + this.sendToPeer(sender, 'ListOperationsResponse', { - operations: this._operations.getOperationsSince(lastKnownHLC), + operations: this._operations.getOperationsSince(message.lastKnownHLC), }) } @@ -485,5 +489,5 @@ export class Operations { } function debug(...args) { - console.debug('SYNC ⇆', ...args) + console.debug('SYNC ⇆', ...args.map((x) => JSON.stringify(x))) } diff --git a/umap/static/umap/js/modules/sync/updaters.js b/umap/static/umap/js/modules/sync/updaters.js index eafb51f2..75c4a9f2 100644 --- a/umap/static/umap/js/modules/sync/updaters.js +++ b/umap/static/umap/js/modules/sync/updaters.js @@ -54,7 +54,10 @@ export class MapUpdater extends BaseUpdater { export class DataLayerUpdater extends BaseUpdater { upsert({ value }) { // Upsert only happens when a new datalayer is created. - this._umap.createDataLayer(value, false) + 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 }) { diff --git a/umap/static/umap/js/modules/umap.js b/umap/static/umap/js/modules/umap.js index bd4fc055..84daa7f1 100644 --- a/umap/static/umap/js/modules/umap.js +++ b/umap/static/umap/js/modules/umap.js @@ -324,14 +324,14 @@ export default class Umap extends ServerStored { dataUrl = decodeURIComponent(dataUrl) dataUrl = this.renderUrl(dataUrl) dataUrl = this.proxyUrl(dataUrl) - const datalayer = this.createDataLayer() + const datalayer = this.createDirtyDataLayer() await datalayer .importFromUrl(dataUrl, dataFormat) .then(() => datalayer.zoomTo()) } } else if (data) { data = decodeURIComponent(data) - const datalayer = this.createDataLayer() + const datalayer = this.createDirtyDataLayer() await datalayer.importRaw(data, dataFormat).then(() => datalayer.zoomTo()) } } @@ -599,8 +599,14 @@ export default class Umap extends ServerStored { return datalayer } + createDirtyDataLayer(options) { + const datalayer = this.createDataLayer(options, true) + datalayer.isDirty = true + return datalayer + } + newDataLayer() { - const datalayer = this.createDataLayer({}) + const datalayer = this.createDirtyDataLayer({}) datalayer.edit() } @@ -1389,7 +1395,7 @@ export default class Umap extends ServerStored { fallback.show() return fallback } - return this.createDataLayer() + return this.createDirtyDataLayer() } findDataLayer(method, context) { @@ -1553,7 +1559,7 @@ export default class Umap extends ServerStored { if (type === 'umap') { this.importUmapFile(file, 'umap') } else { - if (!layer) layer = this.createDataLayer({ name: file.name }) + if (!layer) layer = this.createDirtyDataLayer({ name: file.name }) layer.importFromFile(file, type) } } @@ -1579,7 +1585,7 @@ export default class Umap extends ServerStored { delete geojson._storage } delete geojson._umap_options?.id // Never trust an id at this stage - const dataLayer = this.createDataLayer(geojson._umap_options) + const dataLayer = this.createDirtyDataLayer(geojson._umap_options) dataLayer.fromUmapGeoJSON(geojson) } diff --git a/umap/tests/integration/test_websocket_sync.py b/umap/tests/integration/test_websocket_sync.py index 2807bee1..c5e56e89 100644 --- a/umap/tests/integration/test_websocket_sync.py +++ b/umap/tests/integration/test_websocket_sync.py @@ -39,6 +39,9 @@ def test_websocket_connection_can_sync_markers( a_map_el.click(position={"x": 220, "y": 220}) expect(a_marker_pane).to_have_count(1) expect(b_marker_pane).to_have_count(1) + # Peer B should not be in state dirty + expect(peerB.get_by_role("button", name="View")).to_be_visible() + expect(peerB.get_by_role("button", name="Cancel edits")).to_be_hidden() peerA.locator("body").type("Synced name") peerA.locator("body").press("Escape") @@ -415,3 +418,69 @@ def test_should_sync_datalayers(new_page, live_server, websocket_server, tilelay peerA.get_by_role("button", name="Save").click() assert DataLayer.objects.count() == 2 + + +@pytest.mark.xdist_group(name="websockets") +def test_create_and_sync_map( + new_page, live_server, websocket_server, tilelayer, login, user +): + # Create a syncable map with peerA + peerA = login(user, prefix="Page A") + peerA.goto(f"{live_server.url}/en/map/new/") + with peerA.expect_response(re.compile("./map/create/.*")): + peerA.get_by_role("button", name="Save Draft").click() + peerA.get_by_role("link", name="Map advanced properties").click() + peerA.get_by_text("Real-time collaboration", exact=True).click() + peerA.get_by_text("Enable real-time").click() + peerA.get_by_role("link", name="Update permissions and editors").click() + peerA.locator('select[name="share_status"]').select_option(str(Map.PUBLIC)) + with peerA.expect_response(re.compile("./update/settings/.*")): + peerA.get_by_role("button", name="Save").click() + expect(peerA.get_by_role("button", name="Cancel edits")).to_be_hidden() + # Quit edit mode + peerA.get_by_role("button", name="View").click() + + # Open map and go to edit mode with peer B + peerB = new_page("Page B") + peerB.goto(peerA.url) + peerB.get_by_role("button", name="Edit").click() + + # Create a marker from peerA + markersA = peerA.locator(".leaflet-marker-pane > div") + markersB = peerB.locator(".leaflet-marker-pane > div") + expect(markersA).to_have_count(0) + expect(markersB).to_have_count(0) + + # Add a marker from peer A + peerA.get_by_role("button", name="Edit").click() + peerA.get_by_title("Draw a marker").click() + peerA.locator("#map").click(position={"x": 220, "y": 220}) + expect(markersA).to_have_count(1) + expect(markersB).to_have_count(1) + + # Save and quit edit mode again + with peerA.expect_response(re.compile("./datalayer/create/.*")): + peerA.get_by_role("button", name="Save").click() + peerA.get_by_role("button", name="View").click() + expect(markersA).to_have_count(1) + expect(markersB).to_have_count(1) + peerA.wait_for_timeout(500) + expect(markersA).to_have_count(1) + expect(markersB).to_have_count(1) + + # Peer B should not be in state dirty + expect(peerB.get_by_role("button", name="View")).to_be_visible() + expect(peerB.get_by_role("button", name="Cancel edits")).to_be_hidden() + + # Add a marker from peer B + peerB.get_by_title("Draw a marker").click() + peerB.locator("#map").click(position={"x": 200, "y": 200}) + expect(markersB).to_have_count(2) + expect(markersA).to_have_count(1) + with peerB.expect_response(re.compile("./datalayer/update/.*")): + peerB.get_by_role("button", name="Save").click() + expect(markersB).to_have_count(2) + expect(markersA).to_have_count(1) + peerA.get_by_role("button", name="Edit").click() + expect(markersA).to_have_count(2) + expect(markersB).to_have_count(2)