diff --git a/umap/static/umap/js/modules/data/layer.js b/umap/static/umap/js/modules/data/layer.js index 91aa4bef..27650e4d 100644 --- a/umap/static/umap/js/modules/data/layer.js +++ b/umap/static/umap/js/modules/data/layer.js @@ -20,6 +20,7 @@ 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, @@ -35,8 +36,9 @@ const LAYER_MAP = LAYER_TYPES.reduce((acc, klass) => { return acc }, {}) -export class DataLayer { +export class DataLayer extends ServerStored { constructor(map, data) { + super() this.map = map this.sync = map.sync_engine.proxy(this) this._index = Array() @@ -58,7 +60,6 @@ export class DataLayer { editMode: 'advanced', } - this._isDirty = false this._isDeleted = false this.setUmapId(data.id) this.setOptions(data) @@ -89,23 +90,16 @@ export class DataLayer { if (this.isVisible()) this.propagateShow() } - set isDirty(status) { - this._isDirty = status + onDirty(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 @@ -582,7 +576,6 @@ export class DataLayer { reset() { if (!this.umap_id) this.erase() - this.resetOptions() this.parentPane.appendChild(this.pane) if (this._leaflet_events_bk && !this._leaflet_events) { @@ -1057,8 +1050,9 @@ export class DataLayer { const headers = this._reference_version ? { 'X-Datalayer-Reference': this._reference_version } : {} - await this._trySave(saveUrl, headers, formData) + const status = await this._trySave(saveUrl, headers, formData) this._geojson = geojson + return status } async _trySave(url, headers, formData) { @@ -1071,7 +1065,13 @@ export class DataLayer { 'This situation is tricky, you have to choose carefully which version is pertinent.' ), async () => { + // Save again this layer await this._trySave(url, {}, formData) + 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() } ) } @@ -1089,11 +1089,11 @@ export class DataLayer { this.setUmapId(data.id) this.updateOptions(data) this.backupOptions() + this.backupData() this.connectToMap() this._loaded = true this.redraw() // Needed for reordering features - this.isDirty = false - this.permissions.save() + return true } } @@ -1102,7 +1102,7 @@ export class DataLayer { await this.map.server.post(this.getDeleteUrl()) } delete this.map.datalayers[stamp(this)] - this.isDirty = false + return true } getMap() { @@ -1130,7 +1130,9 @@ export class DataLayer { } renderLegend() { - for (const container of document.querySelectorAll(`.${this.cssId} .datalayer-legend`)) { + for (const container of document.querySelectorAll( + `.${this.cssId} .datalayer-legend` + )) { container.innerHTML = '' if (this.layer.renderLegend) return this.layer.renderLegend(container) const color = DomUtil.create('span', 'datalayer-color', container) diff --git a/umap/static/umap/js/modules/global.js b/umap/static/umap/js/modules/global.js index 52b23fa1..8affffb1 100644 --- a/umap/static/umap/js/modules/global.js +++ b/umap/static/umap/js/modules/global.js @@ -31,6 +31,7 @@ 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 { SAVEMANAGER } from './saving.js' // Import modules and export them to the global scope. // For the not yet module-compatible JS out there. @@ -48,6 +49,7 @@ window.U = { DataLayer, DataLayerPermissions, Dialog, + SAVEMANAGER, EditPanel, Facets, Formatter, diff --git a/umap/static/umap/js/modules/permissions.js b/umap/static/umap/js/modules/permissions.js index 3300da2a..ce0a2655 100644 --- a/umap/static/umap/js/modules/permissions.js +++ b/umap/static/umap/js/modules/permissions.js @@ -1,26 +1,19 @@ 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 { +export class MapPermissions extends ServerStored { 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( { @@ -200,8 +193,8 @@ export class MapPermissions { ) if (!error) { this.commit() - this.isDirty = false this.map.fire('postsync') + return true } } @@ -233,8 +226,9 @@ export class MapPermissions { } } -export class DataLayerPermissions { +export class DataLayerPermissions extends ServerStored { constructor(datalayer) { + super() this.options = Object.assign( { edit_status: null, @@ -243,16 +237,6 @@ export class DataLayerPermissions { ) 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() { @@ -297,12 +281,13 @@ export class DataLayerPermissions { ) if (!error) { this.commit() - this.isDirty = false + return true } } commit() { this.datalayer.options.permissions = Object.assign( + {}, this.datalayer.options.permissions, this.options ) diff --git a/umap/static/umap/js/modules/saving.js b/umap/static/umap/js/modules/saving.js new file mode 100644 index 00000000..7fe69634 --- /dev/null +++ b/umap/static/umap/js/modules/saving.js @@ -0,0 +1,54 @@ +export class SaveManager { + constructor() { + this._queue = new Set() + } + + get isDirty() { + return Boolean(this._queue.size) + } + + async save() { + for (const obj of this._queue) { + const ok = await obj.save() + if (!ok) break + this.delete(obj) + } + } + + add(obj) { + this._queue.add(obj) + this.checkStatus() + } + + delete(obj) { + this._queue.delete(obj) + this.checkStatus() + } + + has(obj) { + return this._queue.has(obj) + } + + checkStatus() { + document.body.classList.toggle('umap-is-dirty', this._queue.size) + } +} + +export const SAVEMANAGER = new SaveManager() + +export class ServerStored { + set isDirty(status) { + if (status) { + SAVEMANAGER.add(this) + } else { + SAVEMANAGER.delete(this) + } + this.onDirty(status) + } + + get isDirty() { + return SAVEMANAGER.has(this) + } + + onDirty(status) {} +} diff --git a/umap/static/umap/js/umap.controls.js b/umap/static/umap/js/umap.controls.js index c6e85e12..e2494f9c 100644 --- a/umap/static/umap/js/umap.controls.js +++ b/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.save, + this.saveAll, this ) L.DomEvent.on( diff --git a/umap/static/umap/js/umap.js b/umap/static/umap/js/umap.js index 15b928e9..36748023 100644 --- a/umap/static/umap/js/umap.js +++ b/umap/static/umap/js/umap.js @@ -153,13 +153,13 @@ U.Map = L.Map.extend({ this.options.onLoadPanel = 'datafilters' } - let isDirty = false // self status + // TODO: remove me when moved to modules + // and inheriting from ServerStored try { Object.defineProperty(this, 'isDirty', { - get: () => isDirty, + get: () => U.SAVEMANAGER.has(this), set: function (status) { - isDirty = status - this.checkDirty() + U.SAVEMANAGER.add(this) }, }) } catch (e) { @@ -200,7 +200,7 @@ U.Map = L.Map.extend({ this.propagate() } - window.onbeforeunload = () => (this.editEnabled && this.isDirty) || null + window.onbeforeunload = () => (this.editEnabled && U.SAVEMANAGER.isDirty) || null this.backup() }, @@ -601,13 +601,13 @@ U.Map = L.Map.extend({ let used = true switch (e.key) { case 'e': - if (!this.isDirty) this.disableEdit() + if (!U.SAVEMANAGER.isDirty) this.disableEdit() break case 's': - if (this.isDirty) this.save() + if (U.SAVEMANAGER.isDirty) this.saveAll() break case 'z': - if (this.isDirty) this.askForReset() + if (U.SAVEMANAGER.isDirty) this.askForReset() break case 'm': this.editTools.startMarker() @@ -1015,10 +1015,6 @@ 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 +1025,7 @@ U.Map = L.Map.extend({ return properties }, - saveSelf: async function () { + save: async function () { this.rules.commit() const geojson = { type: 'Feature', @@ -1048,7 +1044,7 @@ U.Map = L.Map.extend({ return } if (data.login_required) { - window.onLogin = () => this.save() + window.onLogin = () => this.saveAll() window.open(data.login_required) return } @@ -1093,21 +1089,11 @@ U.Map = L.Map.extend({ return true }, - save: async function () { - if (!this.isDirty) return + saveAll: async function () { + if (!U.SAVEMANAGER.isDirty) return if (this._default_extent) this._setCenterAndZoom() this.backup() - 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 + await U.SAVEMANAGER.save() // 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 diff --git a/umap/static/umap/map.css b/umap/static/umap/map.css index ce59df08..480127cd 100644 --- a/umap/static/umap/map.css +++ b/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 { diff --git a/umap/tests/integration/test_edit_datalayer.py b/umap/tests/integration/test_edit_datalayer.py index f9e10e75..ce433051 100644 --- a/umap/tests/integration/test_edit_datalayer.py +++ b/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(".umap-is-dirty")).to_be_visible() + expect(page.locator("body")).to_have_class(re.compile(".*umap-is-dirty.*")) with page.expect_response(re.compile(".*/datalayer/update/.*")): page.get_by_role("button", name="Save").click() saved = DataLayer.objects.last() diff --git a/umap/tests/integration/test_optimistic_merge.py b/umap/tests/integration/test_optimistic_merge.py index 6fad8653..a4af4b7e 100644 --- a/umap/tests/integration/test_optimistic_merge.py +++ b/umap/tests/integration/test_optimistic_merge.py @@ -276,21 +276,38 @@ 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("new name") + page_one.locator('input[name="name"]').fill("name from page one") 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("custom name") + page_two.locator('input[name="name"]').fill("name from page two") + + # 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"] == "new name" + assert data["features"][0]["properties"]["name"] == "name from page one" + + # 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"] == "custom name" + assert data["features"][0]["properties"]["name"] == "name from page two" + # Map should not be in dirty status anymore + expect(page_two.get_by_text("Cancel edits")).to_be_hidden()