From f2f6046c0f92ff1faed057da0711ac478eef439c Mon Sep 17 00:00:00 2001 From: Claude Paroz Date: Sun, 18 Aug 2024 15:29:30 +0200 Subject: [PATCH] Fixed #25706 -- Refactored geometry widgets to remove inline JavaScript. Refactored GIS-related JavaScript initialization to eliminate inline scripts from templates. Added support for specifying a base layer using the new `base_layer_name` attribute on `BaseGeometryWidget`, allowing custom map tile providers via user-defined JavaScript. As a result, the `gis/openlayers-osm.html` template was removed. Thanks Sarah Boyce for reviews. Co-authored-by: Natalia <124304+nessita@users.noreply.github.com> --- django/contrib/gis/forms/widgets.py | 41 ++++---- .../contrib/gis/static/gis/js/OLMapWidget.js | 54 ++++++++++- .../gis/templates/gis/openlayers-osm.html | 12 --- .../contrib/gis/templates/gis/openlayers.html | 36 ++----- docs/ref/contrib/gis/forms-api.txt | 74 ++++++++++++-- docs/releases/6.0.txt | 8 ++ js_tests/gis/mapwidget.test.js | 83 +++++++++++++++- tests/gis_tests/test_geoforms.py | 96 +++++++++++++++---- 8 files changed, 308 insertions(+), 96 deletions(-) delete mode 100644 django/contrib/gis/templates/gis/openlayers-osm.html diff --git a/django/contrib/gis/forms/widgets.py b/django/contrib/gis/forms/widgets.py index 55895ae9f3..c8f9f1208e 100644 --- a/django/contrib/gis/forms/widgets.py +++ b/django/contrib/gis/forms/widgets.py @@ -1,11 +1,9 @@ import logging -from django.conf import settings from django.contrib.gis import gdal from django.contrib.gis.geometry import json_regex from django.contrib.gis.geos import GEOSException, GEOSGeometry from django.forms.widgets import Widget -from django.utils import translation logger = logging.getLogger("django.contrib.gis") @@ -16,6 +14,7 @@ class BaseGeometryWidget(Widget): Render a map using the WKT of the geometry. """ + base_layer = None geom_type = "GEOMETRY" map_srid = 4326 display_raw = False @@ -24,9 +23,10 @@ class BaseGeometryWidget(Widget): template_name = "" # set on subclasses def __init__(self, attrs=None): - self.attrs = {} - for key in ("geom_type", "map_srid", "display_raw"): - self.attrs[key] = getattr(self, key) + self.attrs = { + key: getattr(self, key) + for key in ("base_layer", "geom_type", "map_srid", "display_raw") + } if attrs: self.attrs.update(attrs) @@ -61,26 +61,16 @@ class BaseGeometryWidget(Widget): self.map_srid, err, ) - + context["serialized"] = self.serialize(value) geom_type = gdal.OGRGeomType(self.attrs["geom_type"]).name - context.update( - self.build_attrs( - self.attrs, - { - "name": name, - "module": "geodjango_%s" % name.replace("-", "_"), # JS-safe - "serialized": self.serialize(value), - "geom_type": "Geometry" if geom_type == "Unknown" else geom_type, - "STATIC_URL": settings.STATIC_URL, - "LANGUAGE_BIDI": translation.get_language_bidi(), - **(attrs or {}), - }, - ) + context["widget"]["attrs"]["geom_name"] = ( + "Geometry" if geom_type == "Unknown" else geom_type ) return context class OpenLayersWidget(BaseGeometryWidget): + base_layer = "nasaWorldview" template_name = "gis/openlayers.html" map_srid = 3857 @@ -112,14 +102,15 @@ class OSMWidget(OpenLayersWidget): An OpenLayers/OpenStreetMap-based widget. """ - template_name = "gis/openlayers-osm.html" + base_layer = "osm" default_lon = 5 default_lat = 47 default_zoom = 12 def __init__(self, attrs=None): - super().__init__() - for key in ("default_lon", "default_lat", "default_zoom"): - self.attrs[key] = getattr(self, key) - if attrs: - self.attrs.update(attrs) + if attrs is None: + attrs = {} + attrs.setdefault("default_lon", self.default_lon) + attrs.setdefault("default_lat", self.default_lat) + attrs.setdefault("default_zoom", self.default_zoom) + super().__init__(attrs=attrs) diff --git a/django/contrib/gis/static/gis/js/OLMapWidget.js b/django/contrib/gis/static/gis/js/OLMapWidget.js index a545036c9f..bea4aab863 100644 --- a/django/contrib/gis/static/gis/js/OLMapWidget.js +++ b/django/contrib/gis/static/gis/js/OLMapWidget.js @@ -58,8 +58,16 @@ class MapWidget { this.options[property] = options[property]; } } - if (!options.base_layer) { - this.options.base_layer = new ol.layer.Tile({source: new ol.source.OSM()}); + + // Options' base_layer can be empty, or contain a layerBuilder key, or + // be a layer already constructed. + const base_layer = options.base_layer; + if (typeof base_layer === 'string' && base_layer in MapWidget.layerBuilder) { + this.baseLayer = MapWidget.layerBuilder[base_layer](); + } else if (base_layer && typeof base_layer !== 'string') { + this.baseLayer = base_layer; + } else { + this.baseLayer = MapWidget.layerBuilder.osm(); } this.map = this.createMap(); @@ -120,7 +128,7 @@ class MapWidget { createMap() { return new ol.Map({ target: this.options.map_id, - layers: [this.options.base_layer], + layers: [this.baseLayer], view: new ol.View({ zoom: this.options.default_zoom }) @@ -231,3 +239,43 @@ class MapWidget { document.getElementById(this.options.id).value = jsonFormat.writeGeometry(geometry); } } + +// Static property assignment (ES6-compatible) +MapWidget.layerBuilder = { + nasaWorldview: () => { + return new ol.layer.Tile({ + source: new ol.source.XYZ({ + attributions: "NASA Worldview", + maxZoom: 8, + url: "https://map1{a-c}.vis.earthdata.nasa.gov/wmts-webmerc/" + + "BlueMarble_ShadedRelief_Bathymetry/default/%7BTime%7D/" + + "GoogleMapsCompatible_Level8/{z}/{y}/{x}.jpg" + }) + }); + }, + osm: () => { + return new ol.layer.Tile({source: new ol.source.OSM()}); + } +}; + +function initMapWidgetInSection(section) { + const maps = []; + + section.querySelectorAll(".dj_map_wrapper").forEach((wrapper) => { + // Avoid initializing map widget on an empty form. + if (wrapper.id.includes('__prefix__')) { + return; + } + const options = JSON.parse(wrapper.querySelector("#mapwidget-options").textContent); + options.id = wrapper.querySelector("textarea").id; + options.map_id = wrapper.querySelector(".dj_map").id; + maps.push(new MapWidget(options)); + }); + + return maps; +}; + +document.addEventListener("DOMContentLoaded", () => { + initMapWidgetInSection(document); + document.addEventListener('formset:added', (ev) => {initMapWidgetInSection(ev.target);}); +}); diff --git a/django/contrib/gis/templates/gis/openlayers-osm.html b/django/contrib/gis/templates/gis/openlayers-osm.html deleted file mode 100644 index 88b1c8c2b6..0000000000 --- a/django/contrib/gis/templates/gis/openlayers-osm.html +++ /dev/null @@ -1,12 +0,0 @@ -{% extends "gis/openlayers.html" %} -{% load l10n %} - -{% block options %}{{ block.super }} -options['default_lon'] = {{ default_lon|unlocalize }}; -options['default_lat'] = {{ default_lat|unlocalize }}; -options['default_zoom'] = {{ default_zoom|unlocalize }}; -{% endblock %} - -{% block base_layer %} -var base_layer = new ol.layer.Tile({source: new ol.source.OSM()}); -{% endblock %} diff --git a/django/contrib/gis/templates/gis/openlayers.html b/django/contrib/gis/templates/gis/openlayers.html index f9f7e5fa51..80fa57934b 100644 --- a/django/contrib/gis/templates/gis/openlayers.html +++ b/django/contrib/gis/templates/gis/openlayers.html @@ -1,32 +1,10 @@ -{% load i18n l10n %} +{% load i18n %} -
-
+
+
{% if not disabled %}{% translate "Delete all Features" %}{% endif %} - {% if display_raw %}

{% translate "Debugging window (serialized value)" %}

{% endif %} - - + {% if widget.attrs.display_raw %}

{% translate "Debugging window (serialized value)" %}

{% endif %} + + {{ widget.attrs|json_script:"mapwidget-options" }}
diff --git a/docs/ref/contrib/gis/forms-api.txt b/docs/ref/contrib/gis/forms-api.txt index 61308c5933..c05cef65d0 100644 --- a/docs/ref/contrib/gis/forms-api.txt +++ b/docs/ref/contrib/gis/forms-api.txt @@ -96,6 +96,14 @@ Widget attributes GeoDjango widgets are template-based, so their attributes are mostly different from other Django widget attributes. +.. attribute:: BaseGeometryWidget.base_layer + + .. versionadded:: 6.0 + + A string that specifies the identifier for the default base map layer to be + used by the corresponding JavaScript map widget. It is passed as part of + the widget options when rendering, allowing the ``MapWidget`` to determine + which map tile provider or base layer to initialize (default is ``None``). .. attribute:: BaseGeometryWidget.geom_type @@ -137,15 +145,29 @@ Widget classes This is an abstract base widget containing the logic needed by subclasses. You cannot directly use this widget for a geometry field. - Note that the rendering of GeoDjango widgets is based on a template, - identified by the :attr:`template_name` class attribute. + Note that the rendering of GeoDjango widgets is based on a base layer name, + identified by the :attr:`base_layer` class attribute. ``OpenLayersWidget`` .. class:: OpenLayersWidget - This is the default widget used by all GeoDjango form fields. - ``template_name`` is ``gis/openlayers.html``. + This is the default widget used by all GeoDjango form fields. Attributes + are: + + .. attribute:: base_layer + + .. versionadded:: 6.0 + + ``nasaWorldview`` + + .. attribute:: template_name + + ``gis/openlayers.html``. + + .. attribute:: map_srid + + ``3857`` ``OpenLayersWidget`` and :class:`OSMWidget` use the ``ol.js`` file hosted on the ``cdn.jsdelivr.net`` content-delivery network. You can subclass @@ -157,12 +179,14 @@ Widget classes .. class:: OSMWidget - This widget uses an OpenStreetMap base layer to display geographic objects - on. Attributes are: + This widget specialized :class:`OpenLayersWidget` and uses an OpenStreetMap + base layer to display geographic objects on. Attributes are: - .. attribute:: template_name + .. attribute:: base_layer - ``gis/openlayers-osm.html`` + .. versionadded:: 6.0 + + ``osm`` .. attribute:: default_lat .. attribute:: default_lon @@ -179,3 +203,37 @@ Widget classes tiles. .. _FAQ answer: https://help.openstreetmap.org/questions/10920/how-to-embed-a-map-in-my-https-site + + .. versionchanged:: 6.0 + + The ``OSMWidget`` no longer uses a custom template. Consequently, the + ``gis/openlayers-osm.html`` template was removed. + +.. _geometry-widgets-customization: + +Customizing the base layer used in OpenLayers-based widgets +----------------------------------------------------------- + +.. versionadded:: 6.0 + +To customize the base layer displayed in OpenLayers-based geometry widgets, +define a new layer builder in a custom JavaScript file. For example: + +.. code-block:: javascript + :caption: ``path-to-file.js`` + + MapWidget.layerBuilder.custom_layer_name = function () { + // Return an OpenLayers layer instance. + return new ol.layer.Tile({source: new ol.source.()}); + }; + +Then, subclass a standard geometry widget and set the ``base_layer``:: + + from django.contrib.gis.forms.widgets import OpenLayersWidget + + + class YourCustomWidget(OpenLayersWidget): + base_layer = "custom_layer_name" + + class Media: + js = ["path-to-file.js"] diff --git a/docs/releases/6.0.txt b/docs/releases/6.0.txt index ade85a2173..118ad43cc9 100644 --- a/docs/releases/6.0.txt +++ b/docs/releases/6.0.txt @@ -73,6 +73,9 @@ Minor features function rotates a geometry by a specified angle around the origin or a specified point. +* The new :attr:`.BaseGeometryWidget.base_layer` attribute allows specifying a + JavaScript map base layer, enabling customization of map tile providers. + :mod:`django.contrib.messages` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -332,6 +335,11 @@ Miscellaneous refactored to use Python's :py:class:`email.message.Message` for parsing. Input headers exceeding 10000 characters will now raise :exc:`ValueError`. +* Widgets from :mod:`django.contrib.gis.forms.widgets` now render without + inline JavaScript in templates. If you have customized any geometry widgets + or their templates, you may need to :ref:`update them + ` to match the new layout. + .. _deprecated-features-6.0: Features deprecated in 6.0 diff --git a/js_tests/gis/mapwidget.test.js b/js_tests/gis/mapwidget.test.js index e0cc617a1e..723c63d22b 100644 --- a/js_tests/gis/mapwidget.test.js +++ b/js_tests/gis/mapwidget.test.js @@ -1,4 +1,4 @@ -/* global QUnit, MapWidget */ +/* global QUnit, MapWidget, ol */ 'use strict'; QUnit.module('gis.OLMapWidget'); @@ -91,3 +91,84 @@ QUnit.test('MapWidget.IsCollection', function(assert) { widget = new MapWidget(options); assert.ok(widget.options.is_collection); }); + +QUnit.test('MapWidget.layerBuilder.osm returns OSM layer', function(assert) { + const layer = MapWidget.layerBuilder.osm(); + assert.ok(layer instanceof ol.layer.Tile, 'Layer is Tile'); + assert.ok(layer.getSource() instanceof ol.source.OSM, 'Source is OSM'); +}); + +QUnit.test('MapWidget.layerBuilder.nasaWorldview returns XYZ layer', function(assert) { + const layer = MapWidget.layerBuilder.nasaWorldview(); + assert.ok(layer instanceof ol.layer.Tile, 'Layer is Tile'); + assert.ok(layer.getSource() instanceof ol.source.XYZ, 'Source is XYZ'); + assert.ok(layer.getSource().getUrls()[0].includes('earthdata.nasa.gov'), 'URL is NASA-hosted'); +}); + +QUnit.test('MapWidget uses default OSM base layer when none specified', function(assert) { + const widget = new MapWidget({ + id: 'id_point', + map_id: 'id_point_map', + geom_name: 'Point' + }); + assert.ok(widget.baseLayer.getSource() instanceof ol.source.OSM, 'Default base layer is OSM'); +}); + +QUnit.test('MapWidget uses named base layer from layerBuilder', function(assert) { + const widget = new MapWidget({ + id: 'id_point', + map_id: 'id_point_map', + geom_name: 'Point', + base_layer: 'nasaWorldview' + }); + assert.ok(widget.baseLayer.getSource() instanceof ol.source.XYZ, 'Uses named base layer from builder'); +}); + +QUnit.test('MapWidget uses passed-in base layer object directly', function(assert) { + const customLayer = new ol.layer.Tile({source: new ol.source.OSM()}); + const widget = new MapWidget({ + id: 'id_point', + map_id: 'id_point_map', + geom_name: 'Point', + base_layer: customLayer + }); + assert.strictEqual(widget.baseLayer, customLayer, 'Uses provided layer object'); +}); + +QUnit.test('initMapWidgetInSection initializes widgets and skips __prefix__', function(assert) { + const wrapper1 = document.createElement('div'); + wrapper1.className = 'dj_map_wrapper'; + wrapper1.id = 'id_point_map_wrapper'; + wrapper1.innerHTML = ` + +
+ + `; + document.body.appendChild(wrapper1); + + const wrapper2 = document.createElement('div'); + wrapper2.className = 'dj_map_wrapper'; + wrapper2.id = 'form-__prefix__-map_wrapper'; + wrapper2.innerHTML = ` + +
+ + `; + + document.body.appendChild(wrapper2); + + const maps = window.initMapWidgetInSection(document); + + assert.equal(maps.length, 1, 'Only one map widget is initialized'); + assert.ok(maps[0] instanceof MapWidget, 'Map is instance of MapWidget'); + assert.equal(maps[0].options.id, 'id_point', 'Correct widget was initialized'); + assert.equal(maps[0].options.map_id, 'id_point_map', 'Map ID is correct'); + + // Clean up + wrapper1.remove(); + wrapper2.remove(); +}); diff --git a/tests/gis_tests/test_geoforms.py b/tests/gis_tests/test_geoforms.py index c351edaaad..23f94edd0e 100644 --- a/tests/gis_tests/test_geoforms.py +++ b/tests/gis_tests/test_geoforms.py @@ -4,6 +4,7 @@ from django.contrib.gis import forms from django.contrib.gis.forms import BaseGeometryWidget, OpenLayersWidget from django.contrib.gis.geos import GEOSGeometry from django.core.exceptions import ValidationError +from django.template.defaultfilters import json_script from django.test import SimpleTestCase, override_settings from django.utils.html import escape @@ -183,6 +184,37 @@ class GeometryFieldTest(SimpleTestCase): "unrecognized as WKT EWKT, and HEXEWKB.)", ) + def test_override_attrs(self): + self.assertIsNone(forms.BaseGeometryWidget.base_layer) + self.assertEqual(forms.BaseGeometryWidget.geom_type, "GEOMETRY") + self.assertEqual(forms.BaseGeometryWidget.map_srid, 4326) + self.assertIs(forms.BaseGeometryWidget.display_raw, False) + + class PointForm(forms.Form): + p = forms.PointField( + widget=forms.OpenLayersWidget( + attrs={ + "base_layer": "some-test-file", + "map_srid": 1234, + } + ), + ) + + form = PointForm() + rendered = form.as_p() + + attrs = { + "base_layer": "some-test-file", + "geom_type": "POINT", + "map_srid": 1234, + "display_raw": False, + "required": True, + "id": "id_p", + "geom_name": "Point", + } + expected = json_script(attrs, "mapwidget-options") + self.assertInHTML(expected, rendered) + class SpecializedFieldTest(SimpleTestCase): def setUp(self): @@ -250,15 +282,29 @@ class SpecializedFieldTest(SimpleTestCase): ), } - def assertMapWidget(self, form_instance): + def assertMapWidget(self, form_instance, geom_name): """ Make sure the MapWidget js is passed in the form media and a MapWidget is actually created """ self.assertTrue(form_instance.is_valid()) rendered = form_instance.as_p() - self.assertIn("new MapWidget(options);", rendered) - self.assertIn("map_srid: 3857,", rendered) + + map_fields = [ + f for f in form_instance if isinstance(f.field, forms.GeometryField) + ] + for map_field in map_fields: + attrs = { + "base_layer": "nasaWorldview", + "geom_type": map_field.field.geom_type, + "map_srid": 3857, + "display_raw": False, + "required": True, + "id": map_field.id_for_label, + "geom_name": geom_name, + } + expected = json_script(attrs, "mapwidget-options") + self.assertInHTML(expected, rendered) self.assertIn("gis/js/OLMapWidget.js", str(form_instance.media)) def assertTextarea(self, geom, rendered): @@ -279,7 +325,7 @@ class SpecializedFieldTest(SimpleTestCase): geom = self.geometries["point"] form = PointForm(data={"p": geom}) self.assertTextarea(geom, form.as_p()) - self.assertMapWidget(form) + self.assertMapWidget(form, "Point") self.assertFalse(PointForm().is_valid()) invalid = PointForm(data={"p": "some invalid geom"}) self.assertFalse(invalid.is_valid()) @@ -295,7 +341,7 @@ class SpecializedFieldTest(SimpleTestCase): geom = self.geometries["multipoint"] form = PointForm(data={"p": geom}) self.assertTextarea(geom, form.as_p()) - self.assertMapWidget(form) + self.assertMapWidget(form, "MultiPoint") self.assertFalse(PointForm().is_valid()) for invalid in [ @@ -310,7 +356,7 @@ class SpecializedFieldTest(SimpleTestCase): geom = self.geometries["linestring"] form = LineStringForm(data={"f": geom}) self.assertTextarea(geom, form.as_p()) - self.assertMapWidget(form) + self.assertMapWidget(form, "LineString") self.assertFalse(LineStringForm().is_valid()) for invalid in [ @@ -325,7 +371,7 @@ class SpecializedFieldTest(SimpleTestCase): geom = self.geometries["multilinestring"] form = LineStringForm(data={"f": geom}) self.assertTextarea(geom, form.as_p()) - self.assertMapWidget(form) + self.assertMapWidget(form, "MultiLineString") self.assertFalse(LineStringForm().is_valid()) for invalid in [ @@ -340,7 +386,7 @@ class SpecializedFieldTest(SimpleTestCase): geom = self.geometries["polygon"] form = PolygonForm(data={"p": geom}) self.assertTextarea(geom, form.as_p()) - self.assertMapWidget(form) + self.assertMapWidget(form, "Polygon") self.assertFalse(PolygonForm().is_valid()) for invalid in [ @@ -355,7 +401,7 @@ class SpecializedFieldTest(SimpleTestCase): geom = self.geometries["multipolygon"] form = PolygonForm(data={"p": geom}) self.assertTextarea(geom, form.as_p()) - self.assertMapWidget(form) + self.assertMapWidget(form, "MultiPolygon") self.assertFalse(PolygonForm().is_valid()) for invalid in [ @@ -370,7 +416,7 @@ class SpecializedFieldTest(SimpleTestCase): geom = self.geometries["geometrycollection"] form = GeometryForm(data={"g": geom}) self.assertTextarea(geom, form.as_p()) - self.assertMapWidget(form) + self.assertMapWidget(form, "GeometryCollection") self.assertFalse(GeometryForm().is_valid()) for invalid in [ @@ -393,8 +439,8 @@ class OSMWidgetTest(SimpleTestCase): form = PointForm(data={"p": geom}) rendered = form.as_p() - self.assertIn("ol.source.OSM()", rendered) - self.assertIn("id: 'id_p',", rendered) + self.assertIn('"base_layer": "osm"', rendered) + self.assertIn('