From bb922d1418f351b504b9e8c35828704887f12a80 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Tue, 15 Aug 2023 22:55:59 +0200 Subject: [PATCH 1/2] Call DataLayer.show instead of manually adding layer to the map --- umap/static/umap/js/umap.layer.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/umap/static/umap/js/umap.layer.js b/umap/static/umap/js/umap.layer.js index a0d677e5..85f740ea 100644 --- a/umap/static/umap/js/umap.layer.js +++ b/umap/static/umap/js/umap.layer.js @@ -313,7 +313,7 @@ L.U.DataLayer = L.Evented.extend({ const Class = L.U.Layer[this.options.type] || L.U.Layer.Default this.layer = new Class(this) this.eachLayer((feature) => this.showFeature(feature)) - if (visible) this.map.addLayer(this.layer) + if (visible) this.show() this.propagateRemote() }, @@ -335,12 +335,15 @@ L.U.DataLayer = L.Evented.extend({ fetchData: function () { if (!this.umap_id) return + if (this._loading) return + this._loading = true this.map.get(this._dataUrl(), { callback: function (geojson, response) { this._last_modified = response.getResponseHeader('Last-Modified') this.fromUmapGeoJSON(geojson) this.backupOptions() this.fire('loaded') + this._loading = false }, context: this, }) From fa090b89df5a94bc6de6157bf39b88b6739527d9 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Sun, 20 Aug 2023 09:48:01 +0200 Subject: [PATCH 2/2] Store DataLayer's settings in DB This allows to known the full datalayer behaviour without needing to load all the data, including the zoom from and to (new settings), but also the color for example. This will help also understanding datalayers usage and making stats. But no data migration is provided, it's retrocompatible (data migration in OSM FR servers would be huge, so let's see if it's really needed). --- umap/forms.py | 2 +- umap/migrations/0012_datalayer_settings.py | 19 +++++++ umap/models.py | 20 ++++++- umap/static/umap/js/umap.layer.js | 12 ++-- umap/tests/base.py | 64 ++++++++++++---------- umap/tests/test_datalayer_views.py | 2 + 6 files changed, 79 insertions(+), 40 deletions(-) create mode 100644 umap/migrations/0012_datalayer_settings.py diff --git a/umap/forms.py b/umap/forms.py index cff7150b..f64efbea 100644 --- a/umap/forms.py +++ b/umap/forms.py @@ -59,7 +59,7 @@ class DataLayerForm(forms.ModelForm): class Meta: model = DataLayer - fields = ('geojson', 'name', 'display_on_load', 'rank') + fields = ('geojson', 'name', 'display_on_load', 'rank', 'settings') class MapSettingsForm(forms.ModelForm): diff --git a/umap/migrations/0012_datalayer_settings.py b/umap/migrations/0012_datalayer_settings.py new file mode 100644 index 00000000..9b262061 --- /dev/null +++ b/umap/migrations/0012_datalayer_settings.py @@ -0,0 +1,19 @@ +# Generated by Django 4.2.2 on 2023-08-16 05:24 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("umap", "0011_alter_map_edit_status_alter_map_share_status"), + ] + + operations = [ + migrations.AddField( + model_name="datalayer", + name="settings", + field=models.JSONField( + blank=True, default=dict, null=True, verbose_name="settings" + ), + ), + ] diff --git a/umap/models.py b/umap/models.py index 9ef8ca10..edf1852a 100644 --- a/umap/models.py +++ b/umap/models.py @@ -176,10 +176,14 @@ class Map(NamedModel): settings.AUTH_USER_MODEL, blank=True, verbose_name=_("editors") ) edit_status = models.SmallIntegerField( - choices=EDIT_STATUS, default=get_default_edit_status, verbose_name=_("edit status") + choices=EDIT_STATUS, + default=get_default_edit_status, + verbose_name=_("edit status"), ) share_status = models.SmallIntegerField( - choices=SHARE_STATUS, default=get_default_share_status, verbose_name=_("share status") + choices=SHARE_STATUS, + default=get_default_share_status, + verbose_name=_("share status"), ) settings = models.JSONField( blank=True, null=True, verbose_name=_("settings"), default=dict @@ -308,6 +312,9 @@ class DataLayer(NamedModel): help_text=_("Display this layer on load."), ) rank = models.SmallIntegerField(default=0) + settings = models.JSONField( + blank=True, null=True, verbose_name=_("settings"), default=dict + ) class Meta: ordering = ("rank",) @@ -340,7 +347,14 @@ class DataLayer(NamedModel): @property def metadata(self): - return {"name": self.name, "id": self.pk, "displayOnLoad": self.display_on_load} + # Retrocompat: minimal settings for maps not saved after settings property + # has been introduced + obj = self.settings or { + "name": self.name, + "displayOnLoad": self.display_on_load, + } + obj["id"] = self.pk + return obj def clone(self, map_inst=None): new = self.__class__.objects.get(pk=self.pk) diff --git a/umap/static/umap/js/umap.layer.js b/umap/static/umap/js/umap.layer.js index 85f740ea..8f22dcb8 100644 --- a/umap/static/umap/js/umap.layer.js +++ b/umap/static/umap/js/umap.layer.js @@ -248,11 +248,6 @@ L.U.DataLayer = L.Evented.extend({ } this.setUmapId(data.id) this.setOptions(data) - this.backupOptions() - this.connectToMap() - if (this.displayedOnLoad()) this.show() - if (!this.umap_id) this.isDirty = true - // Retrocompat if (this.options.remoteData && this.options.remoteData.from) { this.options.fromZoom = this.options.remoteData.from @@ -260,11 +255,15 @@ L.U.DataLayer = L.Evented.extend({ if (this.options.remoteData && this.options.remoteData.to) { this.options.toZoom = this.options.remoteData.to } + this.backupOptions() + this.connectToMap() + if (this.displayedOnLoad() && this.showAtZoom()) this.show() + if (!this.umap_id) this.isDirty = true this.onceLoaded(function () { this.map.on('moveend', this.onMoveEnd, this) - this.map.on('zoomend', this.onZoomEnd, this) }) + this.map.on('zoomend', this.onZoomEnd, this) }, onMoveEnd: function (e) { @@ -1185,6 +1184,7 @@ L.U.DataLayer = L.Evented.extend({ formData.append('name', this.options.name) formData.append('display_on_load', !!this.options.displayOnLoad) formData.append('rank', this.getRank()) + formData.append('settings', JSON.stringify(this.options)) // Filename support is shaky, don't do it for now. const blob = new Blob([JSON.stringify(geojson)], { type: 'application/json' }) formData.append('geojson', blob) diff --git a/umap/tests/base.py b/umap/tests/base.py index 24271178..0beab93a 100644 --- a/umap/tests/base.py +++ b/umap/tests/base.py @@ -27,10 +27,11 @@ class TileLayerFactory(factory.django.DjangoModelFactory): class UserFactory(factory.django.DjangoModelFactory): - username = 'Joe' + username = "Joe" email = factory.LazyAttribute( - lambda a: '{0}@example.com'.format(a.username).lower()) - password = factory.PostGenerationMethodCall('set_password', '123123') + lambda a: "{0}@example.com".format(a.username).lower() + ) + password = factory.PostGenerationMethodCall("set_password", "123123") class Meta: model = User @@ -41,32 +42,32 @@ class MapFactory(factory.django.DjangoModelFactory): slug = "test-map" center = DEFAULT_CENTER settings = { - 'geometry': { - 'coordinates': [13.447265624999998, 48.94415123418794], - 'type': 'Point' + "geometry": { + "coordinates": [13.447265624999998, 48.94415123418794], + "type": "Point", }, - 'properties': { - 'datalayersControl': True, - 'description': 'Which is just the Danube, at the end', - 'displayCaptionOnLoad': False, - 'displayDataBrowserOnLoad': False, - 'displayPopupFooter': False, - 'licence': '', - 'miniMap': False, - 'moreControl': True, - 'name': 'Cruising on the Donau', - 'scaleControl': True, - 'tilelayer': { - 'attribution': u'\xa9 OSM Contributors', - 'maxZoom': 18, - 'minZoom': 0, - 'url_template': 'http://{s}.osm.fr/{z}/{x}/{y}.png' + "properties": { + "datalayersControl": True, + "description": "Which is just the Danube, at the end", + "displayCaptionOnLoad": False, + "displayDataBrowserOnLoad": False, + "displayPopupFooter": False, + "licence": "", + "miniMap": False, + "moreControl": True, + "name": "Cruising on the Donau", + "scaleControl": True, + "tilelayer": { + "attribution": "\xa9 OSM Contributors", + "maxZoom": 18, + "minZoom": 0, + "url_template": "http://{s}.osm.fr/{z}/{x}/{y}.png", }, - 'tilelayersControl': True, - 'zoom': 7, - 'zoomControl': True + "tilelayersControl": True, + "zoom": 7, + "zoomControl": True, }, - 'type': 'Feature' + "type": "Feature", } licence = factory.SubFactory(LicenceFactory) @@ -81,7 +82,10 @@ class DataLayerFactory(factory.django.DjangoModelFactory): name = "test datalayer" description = "test description" display_on_load = True - geojson = factory.django.FileField(data="""{"type":"FeatureCollection","features":[{"type":"Feature","geometry":{"type":"Point","coordinates":[13.68896484375,48.55297816440071]},"properties":{"_umap_options":{"color":"DarkCyan","iconClass":"Ball"},"name":"Here","description":"Da place anonymous again 755"}}],"_umap_options":{"displayOnLoad":true,"name":"Donau","id":926}}""") # noqa + settings = {"displayOnLoad": True, "browsable": True, name: "test datalayer"} + geojson = factory.django.FileField( + data="""{"type":"FeatureCollection","features":[{"type":"Feature","geometry":{"type":"Point","coordinates":[13.68896484375,48.55297816440071]},"properties":{"_umap_options":{"color":"DarkCyan","iconClass":"Ball"},"name":"Here","description":"Da place anonymous again 755"}}],"_umap_options":{"displayOnLoad":true,"name":"Donau","id":926}}""" + ) # noqa class Meta: model = DataLayer @@ -90,7 +94,7 @@ class DataLayerFactory(factory.django.DjangoModelFactory): def login_required(response): assert response.status_code == 200 j = json.loads(response.content.decode()) - assert 'login_required' in j - redirect_url = reverse('login') - assert j['login_required'] == redirect_url + assert "login_required" in j + redirect_url = reverse("login") + assert j["login_required"] == redirect_url return True diff --git a/umap/tests/test_datalayer_views.py b/umap/tests/test_datalayer_views.py index 3693725e..cf4b6b8d 100644 --- a/umap/tests/test_datalayer_views.py +++ b/umap/tests/test_datalayer_views.py @@ -17,6 +17,7 @@ def post_data(): return { "name": "name", "display_on_load": True, + "settings": '{"displayOnLoad": true, "browsable": true, "name": "name"}', "rank": 0, "geojson": '{"type":"FeatureCollection","features":[{"type":"Feature","geometry":{"type":"Polygon","coordinates":[[[-3.1640625,53.014783245859235],[-3.1640625,51.86292391360244],[-0.50537109375,51.385495069223204],[1.16455078125,52.38901106223456],[-0.41748046875,53.91728101547621],[-2.109375,53.85252660044951],[-3.1640625,53.014783245859235]]]},"properties":{"_umap_options":{},"name":"Ho god, sounds like a polygouine"}},{"type":"Feature","geometry":{"type":"LineString","coordinates":[[1.8017578124999998,51.16556659836182],[-0.48339843749999994,49.710272582105695],[-3.1640625,50.0923932109388],[-5.60302734375,51.998410382390325]]},"properties":{"_umap_options":{},"name":"Light line"}},{"type":"Feature","geometry":{"type":"Point","coordinates":[0.63720703125,51.15178610143037]},"properties":{"_umap_options":{},"name":"marker he"}}],"_umap_options":{"displayOnLoad":true,"name":"new name","id":1668,"remoteData":{},"color":"LightSeaGreen","description":"test"}}', } @@ -61,6 +62,7 @@ def test_update(client, datalayer, map, post_data): j = json.loads(response.content.decode()) assert "id" in j assert datalayer.pk == j["id"] + assert j["browsable"] is True assert Path(modified_datalayer.geojson.path).exists()