From 4555aa0a489cb9dcf764edf12339097cdfa5ff84 Mon Sep 17 00:00:00 2001 From: Giannis Adamopoulos Date: Wed, 9 Dec 2020 19:12:56 +0200 Subject: [PATCH] Fixed #27674 -- Deprecated GeoModelAdmin and OSMGeoAdmin. Co-authored-by: Mariusz Felisiak --- django/contrib/gis/admin/__init__.py | 8 +- django/contrib/gis/admin/options.py | 36 ++++++ docs/internals/deprecation.txt | 3 + docs/ref/contrib/gis/admin.txt | 29 ++++- docs/ref/contrib/gis/tutorial.txt | 34 +++-- docs/releases/4.0.txt | 8 ++ tests/gis_tests/admin.py | 2 + tests/gis_tests/geoadmin/models.py | 7 +- tests/gis_tests/geoadmin/tests.py | 109 ++++++---------- .../gis_tests/geoadmin_deprecated/__init__.py | 0 .../admin.py | 0 tests/gis_tests/geoadmin_deprecated/models.py | 21 ++++ tests/gis_tests/geoadmin_deprecated/tests.py | 119 ++++++++++++++++++ tests/gis_tests/geoadmin_deprecated/urls.py | 6 + 14 files changed, 284 insertions(+), 98 deletions(-) create mode 100644 tests/gis_tests/geoadmin_deprecated/__init__.py rename tests/gis_tests/{geoadmin => geoadmin_deprecated}/admin.py (100%) create mode 100644 tests/gis_tests/geoadmin_deprecated/models.py create mode 100644 tests/gis_tests/geoadmin_deprecated/tests.py create mode 100644 tests/gis_tests/geoadmin_deprecated/urls.py diff --git a/django/contrib/gis/admin/__init__.py b/django/contrib/gis/admin/__init__.py index b0edaf0193a..4abc4898aef 100644 --- a/django/contrib/gis/admin/__init__.py +++ b/django/contrib/gis/admin/__init__.py @@ -2,11 +2,15 @@ from django.contrib.admin import ( HORIZONTAL, VERTICAL, AdminSite, ModelAdmin, StackedInline, TabularInline, action, autodiscover, display, register, site, ) -from django.contrib.gis.admin.options import GeoModelAdmin, OSMGeoAdmin +from django.contrib.gis.admin.options import ( + GeoModelAdmin, GISModelAdmin, OSMGeoAdmin, +) from django.contrib.gis.admin.widgets import OpenLayersWidget __all__ = [ 'HORIZONTAL', 'VERTICAL', 'AdminSite', 'ModelAdmin', 'StackedInline', 'TabularInline', 'action', 'autodiscover', 'display', 'register', 'site', - 'GeoModelAdmin', 'OSMGeoAdmin', 'OpenLayersWidget', + 'GISModelAdmin', 'OpenLayersWidget', + # RemovedInDjango50Warning. + 'GeoModelAdmin', 'OSMGeoAdmin', ] diff --git a/django/contrib/gis/admin/options.py b/django/contrib/gis/admin/options.py index 9b758a742d3..524ba4bdc50 100644 --- a/django/contrib/gis/admin/options.py +++ b/django/contrib/gis/admin/options.py @@ -1,12 +1,38 @@ +import warnings + from django.contrib.admin import ModelAdmin from django.contrib.gis.admin.widgets import OpenLayersWidget from django.contrib.gis.db import models +from django.contrib.gis.forms import OSMWidget from django.contrib.gis.gdal import OGRGeomType from django.forms import Media +from django.utils.deprecation import RemovedInDjango50Warning + +class GeoModelAdminMixin: + gis_widget = OSMWidget + gis_widget_kwargs = {} + + def formfield_for_dbfield(self, db_field, request, **kwargs): + if ( + isinstance(db_field, models.GeometryField) and + (db_field.dim < 3 or self.gis_widget.supports_3d) + ): + kwargs['widget'] = self.gis_widget(**self.gis_widget_kwargs) + return db_field.formfield(**kwargs) + else: + return super().formfield_for_dbfield(db_field, request, **kwargs) + + +class GISModelAdmin(GeoModelAdminMixin, ModelAdmin): + pass + + +# RemovedInDjango50Warning. spherical_mercator_srid = 3857 +# RemovedInDjango50Warning. class GeoModelAdmin(ModelAdmin): """ The administration options class for Geographic models. Map settings @@ -44,6 +70,15 @@ class GeoModelAdmin(ModelAdmin): debug = False widget = OpenLayersWidget + def __init__(self, *args, **kwargs): + warnings.warn( + 'django.contrib.gis.admin.GeoModelAdmin and OSMGeoAdmin are ' + 'deprecated in favor of django.contrib.admin.ModelAdmin and ' + 'django.contrib.gis.admin.GISModelAdmin.', + RemovedInDjango50Warning, stacklevel=2, + ) + super().__init__(*args, **kwargs) + @property def media(self): "Injects OpenLayers JavaScript into the admin." @@ -124,6 +159,7 @@ class GeoModelAdmin(ModelAdmin): return OLMap +# RemovedInDjango50Warning. class OSMGeoAdmin(GeoModelAdmin): map_template = 'gis/admin/osm.html' num_zoom = 20 diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index 09be9e66e48..23af7315dd1 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -54,6 +54,9 @@ details on these changes. * ``django.db.models.functions.TruncQuarter()`` * ``django.db.models.functions.TruncYear()`` +* The ``django.contrib.gis.admin.GeoModelAdmin`` and ``OSMGeoAdmin`` classes + will be removed. + .. _deprecation-removed-in-4.1: 4.1 diff --git a/docs/ref/contrib/gis/admin.txt b/docs/ref/contrib/gis/admin.txt index 6d7e8b77dfd..ee66e3f2c1a 100644 --- a/docs/ref/contrib/gis/admin.txt +++ b/docs/ref/contrib/gis/admin.txt @@ -5,6 +5,24 @@ GeoDjango's admin site .. module:: django.contrib.gis.admin :synopsis: GeoDjango's extensions to the admin site. +``GISModelAdmin`` +================= + +.. versionadded:: 4.0 + +.. class:: GISModelAdmin + + .. attribute:: gis_widget + + The widget class to be used for + :class:`~django.contrib.gis.db.models.GeometryField`. Defaults to + :class:`~django.contrib.gis.forms.widgets.OSMWidget`. + + .. attribute:: gis_widget_kwargs + + The keyword arguments that would be passed to the :attr:`gis_widget`. + Defaults to an empty dictionary. + ``GeoModelAdmin`` ================= @@ -57,6 +75,11 @@ GeoDjango's admin site ``modifiable=False``, actually displays the geometry in a map, but disables the ability to edit its vertices. + .. deprecated:: 4.0 + + This class is deprecated. Use :class:`~django.contrib.admin.ModelAdmin` + instead. + ``OSMGeoAdmin`` =============== @@ -64,5 +87,7 @@ GeoDjango's admin site A subclass of :class:`GeoModelAdmin` that uses a Spherical Mercator projection with `OpenStreetMap `_ street data tiles. - See the :ref:`OSMGeoAdmin introduction ` - in the tutorial for a usage example. + + .. deprecated:: 4.0 + + This class is deprecated. Use :class:`GISModelAdmin` instead. diff --git a/docs/ref/contrib/gis/tutorial.txt b/docs/ref/contrib/gis/tutorial.txt index 0bdd09e1635..1b8f54495a9 100644 --- a/docs/ref/contrib/gis/tutorial.txt +++ b/docs/ref/contrib/gis/tutorial.txt @@ -697,26 +697,26 @@ Putting your data on the map Geographic Admin ---------------- -GeoDjango extends :doc:`Django's admin application ` -with support for editing geometry fields. +:doc:`Django's admin application ` supports editing +geometry fields. Basics ~~~~~~ -GeoDjango also supplements the Django admin by allowing users to create -and modify geometries on a JavaScript slippy map (powered by `OpenLayers`_). +The Django admin allows users to create and modify geometries on a JavaScript +slippy map (powered by `OpenLayers`_). -Let's dive right in. Create a file called ``admin.py`` inside the -``world`` application with the following code:: +Let's dive right in. Create a file called ``admin.py`` inside the ``world`` +application with the following code:: from django.contrib.gis import admin from .models import WorldBorder - admin.site.register(WorldBorder, admin.GeoModelAdmin) + admin.site.register(WorldBorder, admin.ModelAdmin) Next, edit your ``urls.py`` in the ``geodjango`` application folder as follows:: - from django.contrib.gis import admin + from django.contrib import admin from django.urls import include, path urlpatterns = [ @@ -745,24 +745,22 @@ position. .. _Vector Map Level 0: http://web.archive.org/web/20201024202709/https://earth-info.nga.mil/publications/vmap0.html .. _OSGeo: https://www.osgeo.org/ -.. _osmgeoadmin-intro: +``GISModelAdmin`` +~~~~~~~~~~~~~~~~~ -``OSMGeoAdmin`` -~~~~~~~~~~~~~~~ - -With the :class:`~django.contrib.gis.admin.OSMGeoAdmin`, GeoDjango uses -an `OpenStreetMap`_ layer in the admin. +With the :class:`~django.contrib.gis.admin.GISModelAdmin`, GeoDjango uses an +`OpenStreetMap`_ layer in the admin. This provides more context (including street and thoroughfare details) than -available with the :class:`~django.contrib.gis.admin.GeoModelAdmin` -(which uses the `Vector Map Level 0`_ WMS dataset hosted at `OSGeo`_). +available with the :class:`~django.contrib.admin.ModelAdmin` (which uses the +`Vector Map Level 0`_ WMS dataset hosted at `OSGeo`_). The PROJ datum shifting files must be installed (see the :ref:`PROJ installation instructions ` for more details). -If you meet this requirement, then substitute the ``OSMGeoAdmin`` option class +If you meet this requirement, then use the ``GISModelAdmin`` option class in your ``admin.py`` file:: - admin.site.register(WorldBorder, admin.OSMGeoAdmin) + admin.site.register(WorldBorder, admin.GISModelAdmin) .. rubric:: Footnotes diff --git a/docs/releases/4.0.txt b/docs/releases/4.0.txt index af168a6d335..6ec04824f2f 100644 --- a/docs/releases/4.0.txt +++ b/docs/releases/4.0.txt @@ -171,6 +171,10 @@ Minor features * :class:`~django.contrib.gis.gdal.GDALRaster` now allows creating rasters in any GDAL virtual filesystem. +* The new :class:`~django.contrib.gis.admin.GISModelAdmin` class allows + customizing the widget used for ``GeometryField``. This is encouraged instead + of deprecated ``GeoModelAdmin`` and ``OSMGeoAdmin``. + :mod:`django.contrib.messages` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -718,6 +722,10 @@ Miscellaneous respectively in Django 5.0. If you need the previous behavior, explicitly set ``default`` to ``Value([])``, ``Value('[]')``, or ``Value('')``. +* The ``django.contrib.gis.admin.GeoModelAdmin`` and ``OSMGeoAdmin`` classes + are deprecated. Use :class:`~django.contrib.admin.ModelAdmin` and + :class:`~django.contrib.gis.admin.GISModelAdmin` instead. + Features removed in 4.0 ======================= diff --git a/tests/gis_tests/admin.py b/tests/gis_tests/admin.py index f693ef34de3..2662013f781 100644 --- a/tests/gis_tests/admin.py +++ b/tests/gis_tests/admin.py @@ -3,4 +3,6 @@ try: except ImportError: from django.contrib import admin + admin.GISModelAdmin = admin.ModelAdmin + # RemovedInDjango50Warning. admin.OSMGeoAdmin = admin.ModelAdmin diff --git a/tests/gis_tests/geoadmin/models.py b/tests/gis_tests/geoadmin/models.py index 731753f50b9..68390990397 100644 --- a/tests/gis_tests/geoadmin/models.py +++ b/tests/gis_tests/geoadmin/models.py @@ -14,5 +14,8 @@ class City(models.Model): return self.name -site = admin.AdminSite(name='admin_gis') -site.register(City, admin.OSMGeoAdmin) +site = admin.AdminSite(name='gis_admin_modeladmin') +site.register(City, admin.ModelAdmin) + +site_gis = admin.AdminSite(name='gis_admin_gismodeladmin') +site_gis.register(City, admin.GISModelAdmin) diff --git a/tests/gis_tests/geoadmin/tests.py b/tests/gis_tests/geoadmin/tests.py index 67cc8101ef8..00191e193f0 100644 --- a/tests/gis_tests/geoadmin/tests.py +++ b/tests/gis_tests/geoadmin/tests.py @@ -1,98 +1,59 @@ -from django.contrib.gis import admin from django.contrib.gis.geos import Point from django.test import SimpleTestCase, override_settings -from .admin import UnmodifiableAdmin -from .models import City, site +from .models import City, site, site_gis @override_settings(ROOT_URLCONF='django.contrib.gis.tests.geoadmin.urls') class GeoAdminTest(SimpleTestCase): + admin_site = site # ModelAdmin - def test_ensure_geographic_media(self): - geoadmin = site._registry[City] - admin_js = geoadmin.media.render_js() - self.assertTrue(any(geoadmin.openlayers_url in js for js in admin_js)) - - def test_olmap_OSM_rendering(self): - delete_all_btn = """Delete all Features""" - - original_geoadmin = site._registry[City] - params = original_geoadmin.get_map_widget(City._meta.get_field('point')).params - result = original_geoadmin.get_map_widget(City._meta.get_field('point'))( - ).render('point', Point(-79.460734, 40.18476), params) - self.assertIn( - """geodjango_point.layers.base = new OpenLayers.Layer.OSM("OpenStreetMap (Mapnik)");""", - result) - - self.assertIn(delete_all_btn, result) - - site.unregister(City) - site.register(City, UnmodifiableAdmin) - try: - geoadmin = site._registry[City] - params = geoadmin.get_map_widget(City._meta.get_field('point')).params - result = geoadmin.get_map_widget(City._meta.get_field('point'))( - ).render('point', Point(-79.460734, 40.18476), params) - - self.assertNotIn(delete_all_btn, result) - finally: - site.unregister(City) - site.register(City, original_geoadmin.__class__) - - def test_olmap_WMS_rendering(self): - geoadmin = admin.GeoModelAdmin(City, site) - result = geoadmin.get_map_widget(City._meta.get_field('point'))( - ).render('point', Point(-79.460734, 40.18476)) - self.assertIn( - """geodjango_point.layers.base = new OpenLayers.Layer.WMS("OpenLayers WMS", """ - """"http://vmap0.tiles.osgeo.org/wms/vmap0", {layers: 'basic', format: 'image/jpeg'});""", - result) - - def test_olwidget_has_changed(self): - """ - Changes are accurately noticed by OpenLayersWidget. - """ - geoadmin = site._registry[City] - form = geoadmin.get_changelist_form(None)() - has_changed = form.fields['point'].has_changed - - initial = Point(13.4197458572965953, 52.5194108501149799, srid=4326) - data_same = "SRID=3857;POINT(1493879.2754093995 6894592.019687599)" - data_almost_same = "SRID=3857;POINT(1493879.2754093990 6894592.019687590)" - data_changed = "SRID=3857;POINT(1493884.0527237 6894593.8111804)" - - self.assertTrue(has_changed(None, data_changed)) - self.assertTrue(has_changed(initial, "")) - self.assertFalse(has_changed(None, "")) - self.assertFalse(has_changed(initial, data_same)) - self.assertFalse(has_changed(initial, data_almost_same)) - self.assertTrue(has_changed(initial, data_changed)) - - def test_olwidget_empty_string(self): - geoadmin = site._registry[City] + def test_widget_empty_string(self): + geoadmin = self.admin_site._registry[City] form = geoadmin.get_changelist_form(None)({'point': ''}) - with self.assertNoLogs('django.contrib.gis', 'ERROR'): - output = str(form['point']) + with self.assertRaisesMessage(AssertionError, 'no logs'): + with self.assertLogs('django.contrib.gis', 'ERROR'): + output = str(form['point']) self.assertInHTML( - '', - output + output, ) - def test_olwidget_invalid_string(self): - geoadmin = site._registry[City] + def test_widget_invalid_string(self): + geoadmin = self.admin_site._registry[City] form = geoadmin.get_changelist_form(None)({'point': 'INVALID()'}) with self.assertLogs('django.contrib.gis', 'ERROR') as cm: output = str(form['point']) self.assertInHTML( - '', - output + output, ) self.assertEqual(len(cm.records), 1) self.assertEqual( cm.records[0].getMessage(), "Error creating geometry from value 'INVALID()' (String input " - "unrecognized as WKT EWKT, and HEXEWKB.)" + "unrecognized as WKT EWKT, and HEXEWKB.)", ) + + def test_widget_has_changed(self): + geoadmin = self.admin_site._registry[City] + form = geoadmin.get_changelist_form(None)() + has_changed = form.fields['point'].has_changed + + initial = Point(13.4197458572965953, 52.5194108501149799, srid=4326) + data_same = 'SRID=3857;POINT(1493879.2754093995 6894592.019687599)' + data_almost_same = 'SRID=3857;POINT(1493879.2754093990 6894592.019687590)' + data_changed = 'SRID=3857;POINT(1493884.0527237 6894593.8111804)' + + self.assertIs(has_changed(None, data_changed), True) + self.assertIs(has_changed(initial, ''), True) + self.assertIs(has_changed(None, ''), False) + self.assertIs(has_changed(initial, data_same), False) + self.assertIs(has_changed(initial, data_almost_same), False) + self.assertIs(has_changed(initial, data_changed), True) + + +class GISAdminTests(GeoAdminTest): + admin_site = site_gis # GISModelAdmin diff --git a/tests/gis_tests/geoadmin_deprecated/__init__.py b/tests/gis_tests/geoadmin_deprecated/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/gis_tests/geoadmin/admin.py b/tests/gis_tests/geoadmin_deprecated/admin.py similarity index 100% rename from tests/gis_tests/geoadmin/admin.py rename to tests/gis_tests/geoadmin_deprecated/admin.py diff --git a/tests/gis_tests/geoadmin_deprecated/models.py b/tests/gis_tests/geoadmin_deprecated/models.py new file mode 100644 index 00000000000..efd95359773 --- /dev/null +++ b/tests/gis_tests/geoadmin_deprecated/models.py @@ -0,0 +1,21 @@ +from django.contrib.gis.db import models +from django.test import ignore_warnings +from django.utils.deprecation import RemovedInDjango50Warning + +from ..admin import admin + + +class City(models.Model): + name = models.CharField(max_length=30) + point = models.PointField() + + class Meta: + app_label = 'geoadmini_deprecated' + + def __str__(self): + return self.name + + +site = admin.AdminSite(name='admin_gis') +with ignore_warnings(category=RemovedInDjango50Warning): + site.register(City, admin.OSMGeoAdmin) diff --git a/tests/gis_tests/geoadmin_deprecated/tests.py b/tests/gis_tests/geoadmin_deprecated/tests.py new file mode 100644 index 00000000000..4240de98b13 --- /dev/null +++ b/tests/gis_tests/geoadmin_deprecated/tests.py @@ -0,0 +1,119 @@ +from django.contrib.gis import admin +from django.contrib.gis.geos import Point +from django.test import SimpleTestCase, ignore_warnings, override_settings +from django.utils.deprecation import RemovedInDjango50Warning + +from .admin import UnmodifiableAdmin +from .models import City, site + + +@ignore_warnings(category=RemovedInDjango50Warning) +@override_settings(ROOT_URLCONF='django.contrib.gis.tests.geoadmin.urls') +class GeoAdminTest(SimpleTestCase): + + def test_ensure_geographic_media(self): + geoadmin = site._registry[City] + admin_js = geoadmin.media.render_js() + self.assertTrue(any(geoadmin.openlayers_url in js for js in admin_js)) + + def test_olmap_OSM_rendering(self): + delete_all_btn = """Delete all Features""" + + original_geoadmin = site._registry[City] + params = original_geoadmin.get_map_widget(City._meta.get_field('point')).params + result = original_geoadmin.get_map_widget(City._meta.get_field('point'))( + ).render('point', Point(-79.460734, 40.18476), params) + self.assertIn( + """geodjango_point.layers.base = new OpenLayers.Layer.OSM("OpenStreetMap (Mapnik)");""", + result) + + self.assertIn(delete_all_btn, result) + + site.unregister(City) + site.register(City, UnmodifiableAdmin) + try: + geoadmin = site._registry[City] + params = geoadmin.get_map_widget(City._meta.get_field('point')).params + result = geoadmin.get_map_widget(City._meta.get_field('point'))( + ).render('point', Point(-79.460734, 40.18476), params) + + self.assertNotIn(delete_all_btn, result) + finally: + site.unregister(City) + site.register(City, original_geoadmin.__class__) + + def test_olmap_WMS_rendering(self): + geoadmin = admin.GeoModelAdmin(City, site) + result = geoadmin.get_map_widget(City._meta.get_field('point'))( + ).render('point', Point(-79.460734, 40.18476)) + self.assertIn( + """geodjango_point.layers.base = new OpenLayers.Layer.WMS("OpenLayers WMS", """ + """"http://vmap0.tiles.osgeo.org/wms/vmap0", {layers: 'basic', format: 'image/jpeg'});""", + result) + + def test_olwidget_has_changed(self): + """ + Changes are accurately noticed by OpenLayersWidget. + """ + geoadmin = site._registry[City] + form = geoadmin.get_changelist_form(None)() + has_changed = form.fields['point'].has_changed + + initial = Point(13.4197458572965953, 52.5194108501149799, srid=4326) + data_same = "SRID=3857;POINT(1493879.2754093995 6894592.019687599)" + data_almost_same = "SRID=3857;POINT(1493879.2754093990 6894592.019687590)" + data_changed = "SRID=3857;POINT(1493884.0527237 6894593.8111804)" + + self.assertTrue(has_changed(None, data_changed)) + self.assertTrue(has_changed(initial, "")) + self.assertFalse(has_changed(None, "")) + self.assertFalse(has_changed(initial, data_same)) + self.assertFalse(has_changed(initial, data_almost_same)) + self.assertTrue(has_changed(initial, data_changed)) + + def test_olwidget_empty_string(self): + geoadmin = site._registry[City] + form = geoadmin.get_changelist_form(None)({'point': ''}) + with self.assertNoLogs('django.contrib.gis', 'ERROR'): + output = str(form['point']) + self.assertInHTML( + '', + output + ) + + def test_olwidget_invalid_string(self): + geoadmin = site._registry[City] + form = geoadmin.get_changelist_form(None)({'point': 'INVALID()'}) + with self.assertLogs('django.contrib.gis', 'ERROR') as cm: + output = str(form['point']) + self.assertInHTML( + '', + output + ) + self.assertEqual(len(cm.records), 1) + self.assertEqual( + cm.records[0].getMessage(), + "Error creating geometry from value 'INVALID()' (String input " + "unrecognized as WKT EWKT, and HEXEWKB.)" + ) + + +class DeprecationTests(SimpleTestCase): + def test_warning(self): + class DeprecatedOSMGeoAdmin(admin.OSMGeoAdmin): + pass + + class DeprecatedGeoModelAdmin(admin.GeoModelAdmin): + pass + + msg = ( + 'django.contrib.gis.admin.GeoModelAdmin and OSMGeoAdmin are ' + 'deprecated in favor of django.contrib.admin.ModelAdmin and ' + 'django.contrib.gis.admin.GISModelAdmin.' + ) + with self.assertRaisesMessage(RemovedInDjango50Warning, msg): + DeprecatedOSMGeoAdmin(City, site) + with self.assertRaisesMessage(RemovedInDjango50Warning, msg): + DeprecatedGeoModelAdmin(City, site) diff --git a/tests/gis_tests/geoadmin_deprecated/urls.py b/tests/gis_tests/geoadmin_deprecated/urls.py new file mode 100644 index 00000000000..c27b1d7cdaa --- /dev/null +++ b/tests/gis_tests/geoadmin_deprecated/urls.py @@ -0,0 +1,6 @@ +from django.contrib import admin +from django.urls import include, path + +urlpatterns = [ + path('admin/', include(admin.site.urls)), +]