From f64fd47a7627ed6ffe2df2a32ded6ee528a784eb Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Fri, 7 Jul 2023 08:06:01 +0200 Subject: [PATCH] Fixed #9602 -- Added AdminSite.get_model_admin(). This allows retrieving an admin class for the given model class without using internal attributes. --- django/contrib/admin/checks.py | 34 ++++++++------- django/contrib/admin/filters.py | 12 ++++-- django/contrib/admin/options.py | 47 +++++++++++++-------- django/contrib/admin/sites.py | 8 +++- django/contrib/admin/utils.py | 4 +- django/contrib/admin/views/autocomplete.py | 6 ++- docs/ref/contrib/admin/index.txt | 7 +++ docs/releases/5.0.txt | 3 ++ tests/admin_changelist/tests.py | 4 +- tests/admin_checks/tests.py | 3 +- tests/admin_ordering/tests.py | 4 +- tests/admin_registration/tests.py | 42 ++++++++++++------ tests/admin_views/test_autocomplete_view.py | 8 +++- tests/gis_tests/geoadmin/tests.py | 10 ++--- tests/modeladmin/tests.py | 4 +- 15 files changed, 127 insertions(+), 69 deletions(-) diff --git a/django/contrib/admin/checks.py b/django/contrib/admin/checks.py index 45d895dff06..f5aa9b55d46 100644 --- a/django/contrib/admin/checks.py +++ b/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): diff --git a/django/contrib/admin/filters.py b/django/contrib/admin/filters.py index f6f6d0dd6fd..197d39bf21a 100644 --- a/django/contrib/admin/filters.py +++ b/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) diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index b0635669e93..f9760664dd3 100644 --- a/django/contrib/admin/options.py +++ b/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( - request - ), - can_delete_related=related_modeladmin.has_delete_permission( - request - ), - can_view_related=related_modeladmin.has_view_permission( - request - ), + 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_change_related": related_modeladmin.has_change_permission( + request + ), + "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( diff --git a/django/contrib/admin/sites.py b/django/contrib/admin/sites.py index 1e289876f84..41ce85db798 100644 --- a/django/contrib/admin/sites.py +++ b/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. diff --git a/django/contrib/admin/utils.py b/django/contrib/admin/utils.py index bb5f9501229..92ef1639b4b 100644 --- a/django/contrib/admin/utils.py +++ b/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( diff --git a/django/contrib/admin/views/autocomplete.py b/django/contrib/admin/views/autocomplete.py index 130848b5517..051b9a31b9c 100644 --- a/django/contrib/admin/views/autocomplete.py +++ b/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. diff --git a/docs/ref/contrib/admin/index.txt b/docs/ref/contrib/admin/index.txt index 2ae7e3958f7..c8965f9b74d 100644 --- a/docs/ref/contrib/admin/index.txt +++ b/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 diff --git a/docs/releases/5.0.txt b/docs/releases/5.0.txt index f309721d447..d39703888a4 100644 --- a/docs/releases/5.0.txt +++ b/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` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/tests/admin_changelist/tests.py b/tests/admin_changelist/tests.py index 176b49c66ce..8fdd343b185 100644 --- a/tests/admin_changelist/tests.py +++ b/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") diff --git a/tests/admin_checks/tests.py b/tests/admin_checks/tests.py index 417c8504ede..d2d1eb219e2 100644 --- a/tests/admin_checks/tests.py +++ b/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 diff --git a/tests/admin_ordering/tests.py b/tests/admin_ordering/tests.py index 486e222d018..57f8fc7c6f7 100644 --- a/tests/admin_ordering/tests.py +++ b/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) diff --git a/tests/admin_registration/tests.py b/tests/admin_registration/tests.py index 25f317d322a..c8caf3f202e 100644 --- a/tests/admin_registration/tests.py +++ b/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): diff --git a/tests/admin_views/test_autocomplete_view.py b/tests/admin_views/test_autocomplete_view.py index 90533ea5b0d..f8ab32717a9 100644 --- a/tests/admin_views/test_autocomplete_view.py +++ b/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: diff --git a/tests/gis_tests/geoadmin/tests.py b/tests/gis_tests/geoadmin/tests.py index a7e5700b49f..869307328e8 100644 --- a/tests/gis_tests/geoadmin/tests.py +++ b/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) diff --git a/tests/modeladmin/tests.py b/tests/modeladmin/tests.py index 627029c9cfe..f2763ecc0fb 100644 --- a/tests/modeladmin/tests.py +++ b/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,