Browse Source

Fixed #27674 -- Deprecated GeoModelAdmin and OSMGeoAdmin.

Co-authored-by: Mariusz Felisiak <felisiak.mariusz@gmail.com>
Giannis Adamopoulos 4 years ago
parent
commit
4555aa0a48

+ 6 - 2
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',
 ]

+ 36 - 0
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

+ 3 - 0
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

+ 27 - 2
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 <https://www.openstreetmap.org/>`_ street data tiles.
-    See the :ref:`OSMGeoAdmin introduction <osmgeoadmin-intro>`
-    in the tutorial for a usage example.
+
+    .. deprecated:: 4.0
+
+        This class is deprecated. Use :class:`GISModelAdmin` instead.

+ 16 - 18
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 </ref/contrib/admin/index>`
-with support for editing geometry fields.
+:doc:`Django's admin application </ref/contrib/admin/index>` 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:
-
-``OSMGeoAdmin``
-~~~~~~~~~~~~~~~
+``GISModelAdmin``
+~~~~~~~~~~~~~~~~~
 
-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 <proj4>` 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
 

+ 8 - 0
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
 =======================
 

+ 2 - 0
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

+ 5 - 2
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)

+ 35 - 74
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 = """<a href="javascript:geodjango_point.clearFeatures()">Delete all Features</a>"""
-
-        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(
-            '<textarea id="id_point" class="vWKTField required" cols="150"'
+            '<textarea id="id_point" class="vSerializedField required" cols="150"'
             ' rows="10" name="point"></textarea>',
-            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(
-            '<textarea id="id_point" class="vWKTField required" cols="150"'
+            '<textarea id="id_point" class="vSerializedField required" cols="150"'
             ' rows="10" name="point"></textarea>',
-            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

+ 0 - 0
tests/gis_tests/geoadmin_deprecated/__init__.py


+ 0 - 0
tests/gis_tests/geoadmin/admin.py → tests/gis_tests/geoadmin_deprecated/admin.py


+ 21 - 0
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)

+ 119 - 0
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 = """<a href="javascript:geodjango_point.clearFeatures()">Delete all Features</a>"""
+
+        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(
+            '<textarea id="id_point" class="vWKTField required" cols="150"'
+            ' rows="10" name="point"></textarea>',
+            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(
+            '<textarea id="id_point" class="vWKTField required" cols="150"'
+            ' rows="10" name="point"></textarea>',
+            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)

+ 6 - 0
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)),
+]