Browse Source

Fixed #9602 -- Added AdminSite.get_model_admin().

This allows retrieving an admin class for the given model class without
using internal attributes.
Mariusz Felisiak 1 year ago
parent
commit
f64fd47a76

+ 19 - 15
django/contrib/admin/checks.py

@@ -220,6 +220,8 @@ class BaseModelAdminChecks:
         ManyToManyField and that the item has a related ModelAdmin with
         search_fields defined.
         """
+        from django.contrib.admin.sites import NotRegistered
+
         try:
             field = obj.model._meta.get_field(field_name)
         except FieldDoesNotExist:
@@ -234,8 +236,9 @@ class BaseModelAdminChecks:
                     obj=obj,
                     id="admin.E038",
                 )
-            related_admin = obj.admin_site._registry.get(field.remote_field.model)
-            if related_admin is None:
+            try:
+                related_admin = obj.admin_site.get_model_admin(field.remote_field.model)
+            except NotRegistered:
                 return [
                     checks.Error(
                         'An admin for model "%s" has to be registered '
@@ -248,19 +251,20 @@ class BaseModelAdminChecks:
                         id="admin.E039",
                     )
                 ]
-            elif not related_admin.search_fields:
-                return [
-                    checks.Error(
-                        '%s must define "search_fields", because it\'s '
-                        "referenced by %s.autocomplete_fields."
-                        % (
-                            related_admin.__class__.__name__,
-                            type(obj).__name__,
-                        ),
-                        obj=obj.__class__,
-                        id="admin.E040",
-                    )
-                ]
+            else:
+                if not related_admin.search_fields:
+                    return [
+                        checks.Error(
+                            '%s must define "search_fields", because it\'s '
+                            "referenced by %s.autocomplete_fields."
+                            % (
+                                related_admin.__class__.__name__,
+                                type(obj).__name__,
+                            ),
+                            obj=obj.__class__,
+                            id="admin.E040",
+                        )
+                    ]
             return []
 
     def _check_raw_id_fields(self, obj):

+ 9 - 3
django/contrib/admin/filters.py

@@ -257,10 +257,16 @@ class RelatedFieldListFilter(FieldListFilter):
         """
         Return the model admin's ordering for related field, if provided.
         """
-        related_admin = model_admin.admin_site._registry.get(field.remote_field.model)
-        if related_admin is not None:
+        from django.contrib.admin.sites import NotRegistered
+
+        try:
+            related_admin = model_admin.admin_site.get_model_admin(
+                field.remote_field.model
+            )
+        except NotRegistered:
+            return ()
+        else:
             return related_admin.get_ordering(request)
-        return ()
 
     def field_choices(self, field, request, model_admin):
         ordering = self.field_admin_ordering(field, request, model_admin)

+ 24 - 13
django/contrib/admin/options.py

@@ -160,6 +160,8 @@ class BaseModelAdmin(metaclass=forms.MediaDefiningClass):
 
         If kwargs are given, they're passed to the form Field's constructor.
         """
+        from django.contrib.admin.sites import NotRegistered
+
         # If the field specifies choices, we don't need to look for special
         # admin widgets - we just need to use a select widget of some kind.
         if db_field.choices:
@@ -185,23 +187,27 @@ class BaseModelAdmin(metaclass=forms.MediaDefiningClass):
             # rendered output. formfield can be None if it came from a
             # OneToOneField with parent_link=True or a M2M intermediary.
             if formfield and db_field.name not in self.raw_id_fields:
-                related_modeladmin = self.admin_site._registry.get(
-                    db_field.remote_field.model
-                )
-                wrapper_kwargs = {}
-                if related_modeladmin:
-                    wrapper_kwargs.update(
-                        can_add_related=related_modeladmin.has_add_permission(request),
-                        can_change_related=related_modeladmin.has_change_permission(
+                try:
+                    related_modeladmin = self.admin_site.get_model_admin(
+                        db_field.remote_field.model
+                    )
+                except NotRegistered:
+                    wrapper_kwargs = {}
+                else:
+                    wrapper_kwargs = {
+                        "can_add_related": related_modeladmin.has_add_permission(
                             request
                         ),
-                        can_delete_related=related_modeladmin.has_delete_permission(
+                        "can_change_related": related_modeladmin.has_change_permission(
                             request
                         ),
-                        can_view_related=related_modeladmin.has_view_permission(
+                        "can_delete_related": related_modeladmin.has_delete_permission(
                             request
                         ),
-                    )
+                        "can_view_related": related_modeladmin.has_view_permission(
+                            request
+                        ),
+                    }
                 formfield.widget = widgets.RelatedFieldWidgetWrapper(
                     formfield.widget,
                     db_field.remote_field,
@@ -246,8 +252,13 @@ class BaseModelAdmin(metaclass=forms.MediaDefiningClass):
         ordering.  Otherwise don't specify the queryset, let the field decide
         (return None in that case).
         """
-        related_admin = self.admin_site._registry.get(db_field.remote_field.model)
-        if related_admin is not None:
+        from django.contrib.admin.sites import NotRegistered
+
+        try:
+            related_admin = self.admin_site.get_model_admin(db_field.remote_field.model)
+        except NotRegistered:
+            return None
+        else:
             ordering = related_admin.get_ordering(request)
             if ordering is not None and ordering != ():
                 return db_field.remote_field.model._default_manager.using(db).order_by(

+ 7 - 1
django/contrib/admin/sites.py

@@ -121,7 +121,7 @@ class AdminSite:
                 )
 
             if self.is_registered(model):
-                registered_admin = str(self._registry[model])
+                registered_admin = str(self.get_model_admin(model))
                 msg = "The model %s is already registered " % model.__name__
                 if registered_admin.endswith(".ModelAdmin"):
                     # Most likely registered without a ModelAdmin subclass.
@@ -166,6 +166,12 @@ class AdminSite:
         """
         return model in self._registry
 
+    def get_model_admin(self, model):
+        try:
+            return self._registry[model]
+        except KeyError:
+            raise NotRegistered(f"The model {model.__name__} is not registered.")
+
     def add_action(self, action, name=None):
         """
         Register an action to be available globally.

+ 3 - 1
django/contrib/admin/utils.py

@@ -144,7 +144,9 @@ def get_deleted_objects(objs, request, admin_site):
         no_edit_link = "%s: %s" % (capfirst(opts.verbose_name), obj)
 
         if admin_site.is_registered(model):
-            if not admin_site._registry[model].has_delete_permission(request, obj):
+            if not admin_site.get_model_admin(model).has_delete_permission(
+                request, obj
+            ):
                 perms_needed.add(opts.verbose_name)
             try:
                 admin_url = reverse(

+ 4 - 2
django/contrib/admin/views/autocomplete.py

@@ -74,6 +74,8 @@ class AutocompleteJsonView(BaseListView):
         Raise Http404 if the target model admin is not configured properly with
         search_fields.
         """
+        from django.contrib.admin.sites import NotRegistered
+
         term = request.GET.get("term", "")
         try:
             app_label = request.GET["app_label"]
@@ -97,8 +99,8 @@ class AutocompleteJsonView(BaseListView):
         except AttributeError as e:
             raise PermissionDenied from e
         try:
-            model_admin = self.admin_site._registry[remote_model]
-        except KeyError as e:
+            model_admin = self.admin_site.get_model_admin(remote_model)
+        except NotRegistered as e:
             raise PermissionDenied from e
 
         # Validate suitability of objects.

+ 7 - 0
docs/ref/contrib/admin/index.txt

@@ -3024,6 +3024,13 @@ Templates can override or extend base admin templates as described in
     Raises ``django.contrib.admin.sites.NotRegistered`` if a model isn't
     already registered.
 
+.. method:: AdminSite.get_model_admin(model)
+
+    .. versionadded:: 5.0
+
+    Returns an admin class for the given model class. Raises
+    ``django.contrib.admin.sites.NotRegistered`` if a model isn't registered.
+
 .. method:: AdminSite.get_log_entries(request)
 
     .. versionadded:: 5.0

+ 3 - 0
docs/releases/5.0.txt

@@ -145,6 +145,9 @@ Minor features
 
 * ``XRegExp`` is upgraded from version 3.2.0 to 5.1.1.
 
+* The new :meth:`.AdminSite.get_model_admin` method returns an admin class for
+  the given model class.
+
 :mod:`django.contrib.admindocs`
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

+ 2 - 2
tests/admin_changelist/tests.py

@@ -878,7 +878,7 @@ class ChangeListTests(TestCase):
         user_parents = self._create_superuser("parents")
 
         # Test with user 'noparents'
-        m = custom_site._registry[Child]
+        m = custom_site.get_model_admin(Child)
         request = self._mocked_authenticated_request("/child/", user_noparents)
         response = m.changelist_view(request)
         self.assertNotContains(response, "Parent object")
@@ -903,7 +903,7 @@ class ChangeListTests(TestCase):
 
         # Test default implementation
         custom_site.register(Child, ChildAdmin)
-        m = custom_site._registry[Child]
+        m = custom_site.get_model_admin(Child)
         request = self._mocked_authenticated_request("/child/", user_noparents)
         response = m.changelist_view(request)
         self.assertContains(response, "Parent object")

+ 1 - 2
tests/admin_checks/tests.py

@@ -276,8 +276,7 @@ class SystemChecksTestCase(SimpleTestCase):
         class MyBookAdmin(admin.ModelAdmin):
             def check(self, **kwargs):
                 errors = super().check(**kwargs)
-                author_admin = self.admin_site._registry.get(Author)
-                if author_admin is None:
+                if not self.admin_site.is_registered(Author):
                     errors.append("AuthorAdmin missing!")
                 return errors
 

+ 2 - 2
tests/admin_ordering/tests.py

@@ -152,10 +152,10 @@ class TestRelatedFieldsAdminOrdering(TestCase):
             site.unregister(Band)
 
     def check_ordering_of_field_choices(self, correct_ordering):
-        fk_field = site._registry[Song].formfield_for_foreignkey(
+        fk_field = site.get_model_admin(Song).formfield_for_foreignkey(
             Song.band.field, request=None
         )
-        m2m_field = site._registry[Song].formfield_for_manytomany(
+        m2m_field = site.get_model_admin(Song).formfield_for_manytomany(
             Song.other_interpreters.field, request=None
         )
         self.assertEqual(list(fk_field.queryset), correct_ordering)

+ 28 - 14
tests/admin_registration/tests.py

@@ -22,13 +22,13 @@ class TestRegistration(SimpleTestCase):
 
     def test_bare_registration(self):
         self.site.register(Person)
-        self.assertIsInstance(self.site._registry[Person], admin.ModelAdmin)
+        self.assertIsInstance(self.site.get_model_admin(Person), admin.ModelAdmin)
         self.site.unregister(Person)
         self.assertEqual(self.site._registry, {})
 
     def test_registration_with_model_admin(self):
         self.site.register(Person, NameAdmin)
-        self.assertIsInstance(self.site._registry[Person], NameAdmin)
+        self.assertIsInstance(self.site.get_model_admin(Person), NameAdmin)
         self.site.unregister(Person)
         self.assertEqual(self.site._registry, {})
 
@@ -57,22 +57,28 @@ class TestRegistration(SimpleTestCase):
 
     def test_registration_with_star_star_options(self):
         self.site.register(Person, search_fields=["name"])
-        self.assertEqual(self.site._registry[Person].search_fields, ["name"])
+        self.assertEqual(self.site.get_model_admin(Person).search_fields, ["name"])
+
+    def test_get_model_admin_unregister_model(self):
+        msg = "The model Person is not registered."
+        with self.assertRaisesMessage(admin.sites.NotRegistered, msg):
+            self.site.get_model_admin(Person)
 
     def test_star_star_overrides(self):
         self.site.register(
             Person, NameAdmin, search_fields=["name"], list_display=["__str__"]
         )
-        self.assertEqual(self.site._registry[Person].search_fields, ["name"])
-        self.assertEqual(self.site._registry[Person].list_display, ["__str__"])
-        self.assertTrue(self.site._registry[Person].save_on_top)
+        person_admin = self.site.get_model_admin(Person)
+        self.assertEqual(person_admin.search_fields, ["name"])
+        self.assertEqual(person_admin.list_display, ["__str__"])
+        self.assertIs(person_admin.save_on_top, True)
 
     def test_iterable_registration(self):
         self.site.register([Person, Place], search_fields=["name"])
-        self.assertIsInstance(self.site._registry[Person], admin.ModelAdmin)
-        self.assertEqual(self.site._registry[Person].search_fields, ["name"])
-        self.assertIsInstance(self.site._registry[Place], admin.ModelAdmin)
-        self.assertEqual(self.site._registry[Place].search_fields, ["name"])
+        self.assertIsInstance(self.site.get_model_admin(Person), admin.ModelAdmin)
+        self.assertEqual(self.site.get_model_admin(Person).search_fields, ["name"])
+        self.assertIsInstance(self.site.get_model_admin(Place), admin.ModelAdmin)
+        self.assertEqual(self.site.get_model_admin(Place).search_fields, ["name"])
         self.site.unregister([Person, Place])
         self.assertEqual(self.site._registry, {})
 
@@ -116,18 +122,26 @@ class TestRegistrationDecorator(SimpleTestCase):
 
     def test_basic_registration(self):
         register(Person)(NameAdmin)
-        self.assertIsInstance(self.default_site._registry[Person], admin.ModelAdmin)
+        self.assertIsInstance(
+            self.default_site.get_model_admin(Person), admin.ModelAdmin
+        )
         self.default_site.unregister(Person)
 
     def test_custom_site_registration(self):
         register(Person, site=self.custom_site)(NameAdmin)
-        self.assertIsInstance(self.custom_site._registry[Person], admin.ModelAdmin)
+        self.assertIsInstance(
+            self.custom_site.get_model_admin(Person), admin.ModelAdmin
+        )
 
     def test_multiple_registration(self):
         register(Traveler, Place)(NameAdmin)
-        self.assertIsInstance(self.default_site._registry[Traveler], admin.ModelAdmin)
+        self.assertIsInstance(
+            self.default_site.get_model_admin(Traveler), admin.ModelAdmin
+        )
         self.default_site.unregister(Traveler)
-        self.assertIsInstance(self.default_site._registry[Place], admin.ModelAdmin)
+        self.assertIsInstance(
+            self.default_site.get_model_admin(Place), admin.ModelAdmin
+        )
         self.default_site.unregister(Place)
 
     def test_wrapped_class_not_a_model_admin(self):

+ 6 - 2
tests/admin_views/test_autocomplete_view.py

@@ -3,6 +3,7 @@ import json
 from contextlib import contextmanager
 
 from django.contrib import admin
+from django.contrib.admin.sites import NotRegistered
 from django.contrib.admin.tests import AdminSeleniumTestCase
 from django.contrib.admin.views.autocomplete import AutocompleteJsonView
 from django.contrib.auth.models import Permission, User
@@ -61,8 +62,11 @@ site.register(Toy, autocomplete_fields=["child"])
 
 @contextmanager
 def model_admin(model, model_admin, admin_site=site):
-    org_admin = admin_site._registry.get(model)
-    if org_admin:
+    try:
+        org_admin = admin_site.get_model_admin(model)
+    except NotRegistered:
+        org_admin = None
+    else:
         admin_site.unregister(model)
     admin_site.register(model, model_admin)
     try:

+ 5 - 5
tests/gis_tests/geoadmin/tests.py

@@ -9,7 +9,7 @@ class GeoAdminTest(SimpleTestCase):
     admin_site = site  # ModelAdmin
 
     def test_widget_empty_string(self):
-        geoadmin = self.admin_site._registry[City]
+        geoadmin = self.admin_site.get_model_admin(City)
         form = geoadmin.get_changelist_form(None)({"point": ""})
         with self.assertRaisesMessage(AssertionError, "no logs"):
             with self.assertLogs("django.contrib.gis", "ERROR"):
@@ -21,7 +21,7 @@ class GeoAdminTest(SimpleTestCase):
         )
 
     def test_widget_invalid_string(self):
-        geoadmin = self.admin_site._registry[City]
+        geoadmin = self.admin_site.get_model_admin(City)
         form = geoadmin.get_changelist_form(None)({"point": "INVALID()"})
         with self.assertLogs("django.contrib.gis", "ERROR") as cm:
             output = str(form["point"])
@@ -38,7 +38,7 @@ class GeoAdminTest(SimpleTestCase):
         )
 
     def test_widget_has_changed(self):
-        geoadmin = self.admin_site._registry[City]
+        geoadmin = self.admin_site.get_model_admin(City)
         form = geoadmin.get_changelist_form(None)()
         has_changed = form.fields["point"].has_changed
 
@@ -59,7 +59,7 @@ class GISAdminTests(GeoAdminTest):
     admin_site = site_gis  # GISModelAdmin
 
     def test_default_gis_widget_kwargs(self):
-        geoadmin = self.admin_site._registry[City]
+        geoadmin = self.admin_site.get_model_admin(City)
         form = geoadmin.get_changelist_form(None)()
         widget = form["point"].field.widget
         self.assertEqual(widget.attrs["default_lat"], 47)
@@ -67,7 +67,7 @@ class GISAdminTests(GeoAdminTest):
         self.assertEqual(widget.attrs["default_zoom"], 12)
 
     def test_custom_gis_widget_kwargs(self):
-        geoadmin = site_gis_custom._registry[City]
+        geoadmin = site_gis_custom.get_model_admin(City)
         form = geoadmin.get_changelist_form(None)()
         widget = form["point"].field.widget
         self.assertEqual(widget.attrs["default_lat"], 55)

+ 2 - 2
tests/modeladmin/tests.py

@@ -871,7 +871,7 @@ class ModelAdminTests(TestCase):
             username="bob", email="bob@test.com", password="test"
         )
         self.site.register(Band, ModelAdmin)
-        ma = self.site._registry[Band]
+        ma = self.site.get_model_admin(Band)
         (
             deletable_objects,
             model_count,
@@ -898,7 +898,7 @@ class ModelAdminTests(TestCase):
                 return False
 
         self.site.register(Band, TestModelAdmin)
-        ma = self.site._registry[Band]
+        ma = self.site.get_model_admin(Band)
         (
             deletable_objects,
             model_count,