From d07d9f61d281372530874d66dd4357006ed18916 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Tue, 30 Jan 2024 17:26:48 +0100 Subject: [PATCH] chore: refactore datalayers loading --- umap/static/umap/js/umap.js | 66 ++++--------------- umap/static/umap/js/umap.layer.js | 10 +-- .../integration/test_anonymous_owned_map.py | 9 ++- umap/tests/integration/test_edit_datalayer.py | 7 +- umap/tests/integration/test_import.py | 6 +- umap/tests/integration/test_owned_map.py | 4 ++ 6 files changed, 37 insertions(+), 65 deletions(-) diff --git a/umap/static/umap/js/umap.js b/umap/static/umap/js/umap.js index a794b778..a011c09a 100644 --- a/umap/static/umap/js/umap.js +++ b/umap/static/umap/js/umap.js @@ -186,7 +186,7 @@ L.U.Map.include({ // Needs locate control and hash to exist this.initCenter() this.handleLimitBounds() - this.initDatalayers() + this.initDataLayers() if (this.options.displayCaptionOnLoad) { // Retrocompat @@ -239,8 +239,6 @@ L.U.Map.include({ this._default_extent = true this.options.name = L._('Untitled map') this.options.editMode = 'advanced' - const datalayer = this.createDataLayer() - datalayer.connectToMap() this.enableEdit() let dataUrl = L.Util.queryString('dataUrl', null) const dataFormat = L.Util.queryString('dataFormat', 'geojson') @@ -276,8 +274,6 @@ L.U.Map.include({ this.options.onLoadPanel === 'datafilters' ) this.openFacet() - }) - this.onceDataLoaded(function () { const slug = L.Util.queryString('feature') if (slug && this.features_index[slug]) this.features_index[slug].view() if (L.Util.queryString('edit')) { @@ -422,56 +418,22 @@ L.U.Map.include({ if (this.options.scaleControl) this._controls.scale.addTo(this) }, - initDatalayers: function () { - for (let j = 0; j < this.options.datalayers.length; j++) { - this.createDataLayer(this.options.datalayers[j]) + initDataLayers: async function (datalayers) { + datalayers = datalayers || this.options.datalayers + for (const options of datalayers) { + this.createDataLayer(options) } - this.loadDatalayers() + await this.loadDataLayers() }, - loadDatalayers: function (force) { - const total = this.datalayers_index.length - // toload => datalayer metadata remaining to load (synchronous) - // dataToload => datalayer data remaining to load (asynchronous) - let toload = total, - dataToload = total - let datalayer - const loaded = () => { - this.datalayersLoaded = true - this.fire('datalayersloaded') - } - const decrementToLoad = () => { - toload-- - if (toload === 0) loaded() - } - const dataLoaded = () => { - this.dataLoaded = true - this.fire('dataloaded') - } - const decrementDataToLoad = () => { - dataToload-- - if (dataToload === 0) dataLoaded() - } - this.eachDataLayer(function (datalayer) { - if (force && !datalayer.hasDataLoaded()) { - datalayer.show() - } - if (datalayer.showAtLoad() || force) { - datalayer.onceLoaded(decrementToLoad) - } else { - decrementToLoad() - } - if (datalayer.showAtLoad() || force) { - datalayer.onceDataLoaded(decrementDataToLoad) - } else { - decrementDataToLoad({ sourceTarget: datalayer }) - } - }) - if (total === 0) { - // no datalayer - loaded() - dataLoaded() + loadDataLayers: async function () { + this.datalayersLoaded = true + this.fire('datalayersloaded') + for (const datalayer of Object.values(this.datalayers)) { + if (datalayer.showAtLoad()) await datalayer.show() } + this.dataloaded = true + this.fire('dataloaded') }, indexDatalayers: function () { @@ -504,7 +466,7 @@ L.U.Map.include({ onceDataLoaded: function (callback, context) { // Once datalayers **data** have been loaded - if (this.dataLoaded) { + if (this.dataloaded) { callback.call(context || this, this) } else { this.once('dataloaded', callback, context) diff --git a/umap/static/umap/js/umap.layer.js b/umap/static/umap/js/umap.layer.js index 9dd8d8c4..a088e498 100644 --- a/umap/static/umap/js/umap.layer.js +++ b/umap/static/umap/js/umap.layer.js @@ -581,8 +581,10 @@ L.U.DataLayer = L.Evented.extend({ this.backupOptions() this.connectToMap() this.permissions = new L.U.DataLayerPermissions(this) - if (this.showAtLoad()) this.show() - if (!this.umap_id) this.isDirty = true + if (!this.umap_id) { + if (this.showAtLoad()) this.show() + this.isDirty = true + } this.onceLoaded(function () { this.map.on('moveend', this.onMoveEnd, this) @@ -1431,9 +1433,9 @@ L.U.DataLayer = L.Evented.extend({ return features }, - show: function () { - if (!this.isLoaded()) this.fetchData() + show: async function () { this.map.addLayer(this.layer) + if (!this.isLoaded()) await this.fetchData() this.fire('show') }, diff --git a/umap/tests/integration/test_anonymous_owned_map.py b/umap/tests/integration/test_anonymous_owned_map.py index 19090454..ca0c4b66 100644 --- a/umap/tests/integration/test_anonymous_owned_map.py +++ b/umap/tests/integration/test_anonymous_owned_map.py @@ -1,5 +1,4 @@ import re -from time import sleep import pytest from django.core.signing import get_cookie_signer @@ -126,10 +125,14 @@ def test_anonymous_can_add_marker_on_editable_layer( def test_can_change_perms_after_create(tilelayer, live_server, page): page.goto(f"{live_server.url}/en/map/new") + # Create a layer + page.get_by_title("Manage layers").click() + page.get_by_role("button", name="Add a layer").click() + page.locator("input[name=name]").fill("Layer 1") save = page.get_by_role("button", name="Save") expect(save).to_be_visible() - save.click() - sleep(1) # Let save ajax go back + with page.expect_response(re.compile(r".*/datalayer/create/.*")): + save.click() edit_permissions = page.get_by_title("Update permissions and editors") expect(edit_permissions).to_be_visible() edit_permissions.click() diff --git a/umap/tests/integration/test_edit_datalayer.py b/umap/tests/integration/test_edit_datalayer.py index dc393400..0f379add 100644 --- a/umap/tests/integration/test_edit_datalayer.py +++ b/umap/tests/integration/test_edit_datalayer.py @@ -9,9 +9,10 @@ def test_should_have_fieldset_for_layer_type_properties(page, live_server, tilel expect(button).to_be_visible() button.click() - edit = page.locator("#umap-ui-container").get_by_title("Edit", exact=True) - expect(edit).to_be_visible() - edit.click() + # Create a layer + page.get_by_title("Manage layers").click() + page.get_by_role("button", name="Add a layer").click() + page.locator("input[name=name]").fill("Layer 1") select = page.locator("#umap-ui-container .umap-field-type select") expect(select).to_be_visible() diff --git a/umap/tests/integration/test_import.py b/umap/tests/integration/test_import.py index ac5a3cdc..2f56b99c 100644 --- a/umap/tests/integration/test_import.py +++ b/umap/tests/integration/test_import.py @@ -21,7 +21,7 @@ def test_umap_import_from_file(live_server, datalayer, page): expect(button).to_be_visible() button.click() layers = page.locator(".umap-browse-datalayers li") - expect(layers).to_have_count(3) + expect(layers).to_have_count(2) nonloaded = page.locator(".umap-browse-datalayers li.off") expect(nonloaded).to_have_count(1) assert file_input.input_value() @@ -37,7 +37,7 @@ def test_umap_import_geojson_from_textarea(live_server, datalayer, page): paths = page.locator("path") expect(markers).to_have_count(0) expect(paths).to_have_count(0) - expect(layers).to_have_count(1) + expect(layers).to_have_count(0) button = page.get_by_title("Import data") expect(button).to_be_visible() button.click() @@ -48,7 +48,7 @@ def test_umap_import_geojson_from_textarea(live_server, datalayer, page): button = page.get_by_role("button", name="Import", exact=True) expect(button).to_be_visible() button.click() - # No layer has been created + # A layer has been created expect(layers).to_have_count(1) expect(markers).to_have_count(2) expect(paths).to_have_count(3) diff --git a/umap/tests/integration/test_owned_map.py b/umap/tests/integration/test_owned_map.py index 3f10204b..780bb0ef 100644 --- a/umap/tests/integration/test_owned_map.py +++ b/umap/tests/integration/test_owned_map.py @@ -191,6 +191,10 @@ def test_create(tilelayer, live_server, login, user): def test_can_change_perms_after_create(tilelayer, live_server, login, user): page = login(user) page.goto(f"{live_server.url}/en/map/new") + # Create a layer + page.get_by_title("Manage layers").click() + page.get_by_role("button", name="Add a layer").click() + page.locator("input[name=name]").fill("Layer 1") save = page.get_by_role("button", name="Save") expect(save).to_be_visible() save.click()