From 3071660acfbdf4b5c59457c8e9dc345d5e8894c5 Mon Sep 17 00:00:00 2001 From: Johannes Maron Date: Tue, 12 Jan 2021 11:37:38 +0100 Subject: [PATCH] Fixed #29010, Fixed #29138 -- Added limit_choices_to and to_field support to autocomplete fields. * Fixed #29010 -- Added limit_choices_to support to autocomplete fields. * Fixed #29138 -- Allowed autocomplete fields to target a custom to_field rather than the PK. --- django/contrib/admin/options.py | 9 +- django/contrib/admin/sites.py | 5 + .../admin/static/admin/js/autocomplete.js | 5 +- django/contrib/admin/views/autocomplete.py | 70 +++++++++-- django/contrib/admin/widgets.py | 17 +-- docs/releases/3.2.txt | 6 + tests/admin_views/models.py | 8 ++ tests/admin_views/test_autocomplete_view.py | 110 ++++++++++++++---- tests/admin_widgets/models.py | 3 +- .../admin_widgets/test_autocomplete_widget.py | 19 +-- tests/admin_widgets/tests.py | 8 +- 11 files changed, 200 insertions(+), 60 deletions(-) diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index 1703dd333e..5214a266e3 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -19,7 +19,6 @@ from django.contrib.admin.utils import ( get_deleted_objects, lookup_needs_distinct, model_format_dict, model_ngettext, quote, unquote, ) -from django.contrib.admin.views.autocomplete import AutocompleteJsonView from django.contrib.admin.widgets import ( AutocompleteSelect, AutocompleteSelectMultiple, ) @@ -225,7 +224,7 @@ class BaseModelAdmin(metaclass=forms.MediaDefiningClass): if 'widget' not in kwargs: if db_field.name in self.get_autocomplete_fields(request): - kwargs['widget'] = AutocompleteSelect(db_field.remote_field, self.admin_site, using=db) + kwargs['widget'] = AutocompleteSelect(db_field, self.admin_site, using=db) elif db_field.name in self.raw_id_fields: kwargs['widget'] = widgets.ForeignKeyRawIdWidget(db_field.remote_field, self.admin_site, using=db) elif db_field.name in self.radio_fields: @@ -255,7 +254,7 @@ class BaseModelAdmin(metaclass=forms.MediaDefiningClass): autocomplete_fields = self.get_autocomplete_fields(request) if db_field.name in autocomplete_fields: kwargs['widget'] = AutocompleteSelectMultiple( - db_field.remote_field, + db_field, self.admin_site, using=db, ) @@ -622,7 +621,6 @@ class ModelAdmin(BaseModelAdmin): return [ path('', wrap(self.changelist_view), name='%s_%s_changelist' % info), path('add/', wrap(self.add_view), name='%s_%s_add' % info), - path('autocomplete/', wrap(self.autocomplete_view), name='%s_%s_autocomplete' % info), path('/history/', wrap(self.history_view), name='%s_%s_history' % info), path('/delete/', wrap(self.delete_view), name='%s_%s_delete' % info), path('/change/', wrap(self.change_view), name='%s_%s_change' % info), @@ -1652,9 +1650,6 @@ class ModelAdmin(BaseModelAdmin): return self.render_change_form(request, context, add=add, change=not add, obj=obj, form_url=form_url) - def autocomplete_view(self, request): - return AutocompleteJsonView.as_view(model_admin=self)(request) - def add_view(self, request, form_url='', extra_context=None): return self.changeform_view(request, None, form_url, extra_context) diff --git a/django/contrib/admin/sites.py b/django/contrib/admin/sites.py index 5b2472bc6f..e200fdeb1e 100644 --- a/django/contrib/admin/sites.py +++ b/django/contrib/admin/sites.py @@ -4,6 +4,7 @@ from weakref import WeakSet from django.apps import apps from django.contrib.admin import ModelAdmin, actions +from django.contrib.admin.views.autocomplete import AutocompleteJsonView from django.contrib.auth import REDIRECT_FIELD_NAME from django.core.exceptions import ImproperlyConfigured from django.db.models.base import ModelBase @@ -255,6 +256,7 @@ class AdminSite: wrap(self.password_change_done, cacheable=True), name='password_change_done', ), + path('autocomplete/', wrap(self.autocomplete_view), name='autocomplete'), path('jsi18n/', wrap(self.i18n_javascript, cacheable=True), name='jsi18n'), path( 'r///', @@ -401,6 +403,9 @@ class AdminSite: request.current_app = self.name return LoginView.as_view(**defaults)(request) + def autocomplete_view(self, request): + return AutocompleteJsonView.as_view(admin_site=self)(request) + def _build_app_dict(self, request, label=None): """ Build the app dictionary. The optional `label` parameter filters models diff --git a/django/contrib/admin/static/admin/js/autocomplete.js b/django/contrib/admin/static/admin/js/autocomplete.js index c922b303a8..c55eee1fa6 100644 --- a/django/contrib/admin/static/admin/js/autocomplete.js +++ b/django/contrib/admin/static/admin/js/autocomplete.js @@ -7,7 +7,10 @@ data: function(params) { return { term: params.term, - page: params.page + page: params.page, + app_label: $element.data('app-label'), + model_name: $element.data('model-name'), + field_name: $element.data('field-name') }; } } diff --git a/django/contrib/admin/views/autocomplete.py b/django/contrib/admin/views/autocomplete.py index 7aa59bf92f..6c52ecaeea 100644 --- a/django/contrib/admin/views/autocomplete.py +++ b/django/contrib/admin/views/autocomplete.py @@ -1,3 +1,5 @@ +from django.apps import apps +from django.core.exceptions import FieldDoesNotExist, PermissionDenied from django.http import Http404, JsonResponse from django.views.generic.list import BaseListView @@ -5,7 +7,7 @@ from django.views.generic.list import BaseListView class AutocompleteJsonView(BaseListView): """Handle AutocompleteWidget's AJAX requests for data.""" paginate_by = 20 - model_admin = None + admin_site = None def get(self, request, *args, **kwargs): """ @@ -15,20 +17,16 @@ class AutocompleteJsonView(BaseListView): pagination: {more: true} } """ - if not self.model_admin.get_search_fields(request): - raise Http404( - '%s must have search_fields for the autocomplete_view.' % - type(self.model_admin).__name__ - ) - if not self.has_perm(request): - return JsonResponse({'error': '403 Forbidden'}, status=403) + self.term, self.model_admin, self.source_field, to_field_name = self.process_request(request) + + if not self.has_perm(request): + raise PermissionDenied - self.term = request.GET.get('term', '') self.object_list = self.get_queryset() context = self.get_context_data() return JsonResponse({ 'results': [ - {'id': str(obj.pk), 'text': str(obj)} + {'id': str(getattr(obj, to_field_name)), 'text': str(obj)} for obj in context['object_list'] ], 'pagination': {'more': context['page_obj'].has_next()}, @@ -41,11 +39,63 @@ class AutocompleteJsonView(BaseListView): def get_queryset(self): """Return queryset based on ModelAdmin.get_search_results().""" qs = self.model_admin.get_queryset(self.request) + qs = qs.complex_filter(self.source_field.get_limit_choices_to()) qs, search_use_distinct = self.model_admin.get_search_results(self.request, qs, self.term) if search_use_distinct: qs = qs.distinct() return qs + def process_request(self, request): + """ + Validate request integrity, extract and return request parameters. + + Since the subsequent view permission check requires the target model + admin, which is determined here, raise PermissionDenied if the + requested app, model or field are malformed. + + Raise Http404 if the target model admin is not configured properly with + search_fields. + """ + term = request.GET.get('term', '') + try: + app_label = request.GET['app_label'] + model_name = request.GET['model_name'] + field_name = request.GET['field_name'] + except KeyError as e: + raise PermissionDenied from e + + # Retrieve objects from parameters. + try: + source_model = apps.get_model(app_label, model_name) + except LookupError as e: + raise PermissionDenied from e + + try: + source_field = source_model._meta.get_field(field_name) + except FieldDoesNotExist as e: + raise PermissionDenied from e + try: + remote_model = source_field.remote_field.model + except AttributeError as e: + raise PermissionDenied from e + try: + model_admin = self.admin_site._registry[remote_model] + except KeyError as e: + raise PermissionDenied from e + + # Validate suitability of objects. + if not model_admin.get_search_fields(request): + raise Http404( + '%s must have search_fields for the autocomplete_view.' % + type(model_admin).__qualname__ + ) + + to_field_name = getattr(source_field.remote_field, 'field_name', model_admin.model._meta.pk.name) + if not model_admin.to_field_allowed(request, to_field_name): + raise PermissionDenied + + return term, model_admin, source_field, to_field_name + def has_perm(self, request, obj=None): """Check if user has permission to access the related model.""" return self.model_admin.has_view_permission(request, obj=obj) diff --git a/django/contrib/admin/widgets.py b/django/contrib/admin/widgets.py index 59d1004d2d..1f438daf2d 100644 --- a/django/contrib/admin/widgets.py +++ b/django/contrib/admin/widgets.py @@ -380,18 +380,17 @@ class AutocompleteMixin: Renders the necessary data attributes for select2 and adds the static form media. """ - url_name = '%s:%s_%s_autocomplete' + url_name = '%s:autocomplete' - def __init__(self, rel, admin_site, attrs=None, choices=(), using=None): - self.rel = rel + def __init__(self, field, admin_site, attrs=None, choices=(), using=None): + self.field = field self.admin_site = admin_site self.db = using self.choices = choices self.attrs = {} if attrs is None else attrs.copy() def get_url(self): - model = self.rel.model - return reverse(self.url_name % (self.admin_site.name, model._meta.app_label, model._meta.model_name)) + return reverse(self.url_name % self.admin_site.name) def build_attrs(self, base_attrs, extra_attrs=None): """ @@ -408,6 +407,9 @@ class AutocompleteMixin: 'data-ajax--delay': 250, 'data-ajax--type': 'GET', 'data-ajax--url': self.get_url(), + 'data-app-label': self.field.model._meta.app_label, + 'data-model-name': self.field.model._meta.model_name, + 'data-field-name': self.field.name, 'data-theme': 'admin-autocomplete', 'data-allow-clear': json.dumps(not self.is_required), 'data-placeholder': '', # Allows clearing of the input. @@ -426,9 +428,10 @@ class AutocompleteMixin: } if not self.is_required and not self.allow_multiple_selected: default[1].append(self.create_option(name, '', '', False, 0)) + to_field_name = getattr(self.field.remote_field, 'field_name', self.field.model._meta.pk.name) choices = ( - (obj.pk, self.choices.field.label_from_instance(obj)) - for obj in self.choices.queryset.using(self.db).filter(pk__in=selected_choices) + (getattr(obj, to_field_name), self.choices.field.label_from_instance(obj)) + for obj in self.choices.queryset.using(self.db).filter(**{'%s__in' % to_field_name: selected_choices}) ) for option_value, option_label in choices: selected = ( diff --git a/docs/releases/3.2.txt b/docs/releases/3.2.txt index a2aa1bb80c..f6c5ae5b62 100644 --- a/docs/releases/3.2.txt +++ b/docs/releases/3.2.txt @@ -119,6 +119,12 @@ Minor features * The admin now supports theming. See :ref:`admin-theming` for more details. +* :attr:`.ModelAdmin.autocomplete_fields` now respects + :attr:`ForeignKey.to_field ` and + :attr:`ForeignKey.limit_choices_to + ` when searching a related + model. + :mod:`django.contrib.admindocs` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/tests/admin_views/models.py b/tests/admin_views/models.py index eed3cae306..5224f5f91a 100644 --- a/tests/admin_views/models.py +++ b/tests/admin_views/models.py @@ -629,6 +629,7 @@ class Question(models.Model): posted = models.DateField(default=datetime.date.today) expires = models.DateTimeField(null=True, blank=True) related_questions = models.ManyToManyField('self') + uuid = models.UUIDField(default=uuid.uuid4, unique=True) def __str__(self): return self.question @@ -636,6 +637,13 @@ class Question(models.Model): class Answer(models.Model): question = models.ForeignKey(Question, models.PROTECT) + question_with_to_field = models.ForeignKey( + Question, models.SET_NULL, + blank=True, null=True, to_field='uuid', + related_name='uuid_answers', + limit_choices_to=~models.Q(question__istartswith='not'), + ) + related_answers = models.ManyToManyField('self') answer = models.CharField(max_length=20) def __str__(self): diff --git a/tests/admin_views/test_autocomplete_view.py b/tests/admin_views/test_autocomplete_view.py index b0b2894659..2b154e7a45 100644 --- a/tests/admin_views/test_autocomplete_view.py +++ b/tests/admin_views/test_autocomplete_view.py @@ -6,6 +6,7 @@ from django.contrib.admin.tests import AdminSeleniumTestCase from django.contrib.admin.views.autocomplete import AutocompleteJsonView from django.contrib.auth.models import Permission, User from django.contrib.contenttypes.models import ContentType +from django.core.exceptions import PermissionDenied from django.http import Http404 from django.test import RequestFactory, override_settings from django.urls import reverse, reverse_lazy @@ -38,10 +39,28 @@ site.register(Author, AuthorAdmin) site.register(Book, BookAdmin) +@contextmanager +def model_admin(model, model_admin, admin_site=site): + org_admin = admin_site._registry.get(model) + if org_admin: + admin_site.unregister(model) + admin_site.register(model, model_admin) + try: + yield + finally: + if org_admin: + admin_site._registry[model] = org_admin + + class AutocompleteJsonViewTests(AdminViewBasicTestCase): - as_view_args = {'model_admin': QuestionAdmin(Question, site)} + as_view_args = {'admin_site': site} + opts = { + 'app_label': Answer._meta.app_label, + 'model_name': Answer._meta.model_name, + 'field_name': 'question' + } factory = RequestFactory() - url = reverse_lazy('autocomplete_admin:admin_views_question_autocomplete') + url = reverse_lazy('autocomplete_admin:autocomplete') @classmethod def setUpTestData(cls): @@ -53,7 +72,7 @@ class AutocompleteJsonViewTests(AdminViewBasicTestCase): def test_success(self): q = Question.objects.create(question='Is this a question?') - request = self.factory.get(self.url, {'term': 'is'}) + request = self.factory.get(self.url, {'term': 'is', **self.opts}) request.user = self.superuser response = AutocompleteJsonView.as_view(**self.as_view_args)(request) self.assertEqual(response.status_code, 200) @@ -63,11 +82,56 @@ class AutocompleteJsonViewTests(AdminViewBasicTestCase): 'pagination': {'more': False}, }) + def test_custom_to_field(self): + q = Question.objects.create(question='Is this a question?') + request = self.factory.get(self.url, {'term': 'is', **self.opts, 'field_name': 'question_with_to_field'}) + request.user = self.superuser + response = AutocompleteJsonView.as_view(**self.as_view_args)(request) + self.assertEqual(response.status_code, 200) + data = json.loads(response.content.decode('utf-8')) + self.assertEqual(data, { + 'results': [{'id': str(q.uuid), 'text': q.question}], + 'pagination': {'more': False}, + }) + + def test_field_does_not_exist(self): + request = self.factory.get(self.url, {'term': 'is', **self.opts, 'field_name': 'does_not_exist'}) + request.user = self.superuser + with self.assertRaises(PermissionDenied): + AutocompleteJsonView.as_view(**self.as_view_args)(request) + + def test_field_no_related_field(self): + request = self.factory.get(self.url, {'term': 'is', **self.opts, 'field_name': 'answer'}) + request.user = self.superuser + with self.assertRaises(PermissionDenied): + AutocompleteJsonView.as_view(**self.as_view_args)(request) + + def test_field_does_not_allowed(self): + request = self.factory.get(self.url, {'term': 'is', **self.opts, 'field_name': 'related_questions'}) + request.user = self.superuser + with self.assertRaises(PermissionDenied): + AutocompleteJsonView.as_view(**self.as_view_args)(request) + + def test_limit_choices_to(self): + # Answer.question_with_to_field defines limit_choices_to to "those not + # starting with 'not'". + q = Question.objects.create(question='Is this a question?') + Question.objects.create(question='Not a question.') + request = self.factory.get(self.url, {'term': 'is', **self.opts, 'field_name': 'question_with_to_field'}) + request.user = self.superuser + response = AutocompleteJsonView.as_view(**self.as_view_args)(request) + self.assertEqual(response.status_code, 200) + data = json.loads(response.content.decode('utf-8')) + self.assertEqual(data, { + 'results': [{'id': str(q.uuid), 'text': q.question}], + 'pagination': {'more': False}, + }) + def test_must_be_logged_in(self): - response = self.client.get(self.url, {'term': ''}) + response = self.client.get(self.url, {'term': '', **self.opts}) self.assertEqual(response.status_code, 200) self.client.logout() - response = self.client.get(self.url, {'term': ''}) + response = self.client.get(self.url, {'term': '', **self.opts}) self.assertEqual(response.status_code, 302) def test_has_view_or_change_permission_required(self): @@ -75,13 +139,12 @@ class AutocompleteJsonViewTests(AdminViewBasicTestCase): Users require the change permission for the related model to the autocomplete view for it. """ - request = self.factory.get(self.url, {'term': 'is'}) + request = self.factory.get(self.url, {'term': 'is', **self.opts}) self.user.is_staff = True self.user.save() request.user = self.user - response = AutocompleteJsonView.as_view(**self.as_view_args)(request) - self.assertEqual(response.status_code, 403) - self.assertJSONEqual(response.content.decode('utf-8'), {'error': '403 Forbidden'}) + with self.assertRaises(PermissionDenied): + AutocompleteJsonView.as_view(**self.as_view_args)(request) for permission in ('view', 'change'): with self.subTest(permission=permission): self.user.user_permissions.clear() @@ -104,14 +167,14 @@ class AutocompleteJsonViewTests(AdminViewBasicTestCase): q2.related_questions.add(q1) q3 = Question.objects.create(question='question 3') q3.related_questions.add(q1) - request = self.factory.get(self.url, {'term': 'question'}) + request = self.factory.get(self.url, {'term': 'question', **self.opts}) request.user = self.superuser class DistinctQuestionAdmin(QuestionAdmin): search_fields = ['related_questions__question', 'question'] - model_admin = DistinctQuestionAdmin(Question, site) - response = AutocompleteJsonView.as_view(model_admin=model_admin)(request) + with model_admin(Question, DistinctQuestionAdmin): + response = AutocompleteJsonView.as_view(**self.as_view_args)(request) self.assertEqual(response.status_code, 200) data = json.loads(response.content.decode('utf-8')) self.assertEqual(len(data['results']), 3) @@ -120,20 +183,22 @@ class AutocompleteJsonViewTests(AdminViewBasicTestCase): class EmptySearchAdmin(QuestionAdmin): search_fields = [] - model_admin = EmptySearchAdmin(Question, site) - msg = 'EmptySearchAdmin must have search_fields for the autocomplete_view.' - with self.assertRaisesMessage(Http404, msg): - model_admin.autocomplete_view(self.factory.get(self.url)) + with model_admin(Question, EmptySearchAdmin): + msg = 'EmptySearchAdmin must have search_fields for the autocomplete_view.' + with self.assertRaisesMessage(Http404, msg): + site.autocomplete_view(self.factory.get(self.url, {'term': '', **self.opts})) def test_get_paginator(self): """Search results are paginated.""" + class PKOrderingQuestionAdmin(QuestionAdmin): + ordering = ['pk'] + Question.objects.bulk_create(Question(question=str(i)) for i in range(PAGINATOR_SIZE + 10)) - model_admin = QuestionAdmin(Question, site) - model_admin.ordering = ['pk'] # The first page of results. - request = self.factory.get(self.url, {'term': ''}) + request = self.factory.get(self.url, {'term': '', **self.opts}) request.user = self.superuser - response = AutocompleteJsonView.as_view(model_admin=model_admin)(request) + with model_admin(Question, PKOrderingQuestionAdmin): + response = AutocompleteJsonView.as_view(**self.as_view_args)(request) self.assertEqual(response.status_code, 200) data = json.loads(response.content.decode('utf-8')) self.assertEqual(data, { @@ -141,9 +206,10 @@ class AutocompleteJsonViewTests(AdminViewBasicTestCase): 'pagination': {'more': True}, }) # The second page of results. - request = self.factory.get(self.url, {'term': '', 'page': '2'}) + request = self.factory.get(self.url, {'term': '', 'page': '2', **self.opts}) request.user = self.superuser - response = AutocompleteJsonView.as_view(model_admin=model_admin)(request) + with model_admin(Question, PKOrderingQuestionAdmin): + response = AutocompleteJsonView.as_view(**self.as_view_args)(request) self.assertEqual(response.status_code, 200) data = json.loads(response.content.decode('utf-8')) self.assertEqual(data, { diff --git a/tests/admin_widgets/models.py b/tests/admin_widgets/models.py index 88bf2b8fca..85f71749fd 100644 --- a/tests/admin_widgets/models.py +++ b/tests/admin_widgets/models.py @@ -19,6 +19,7 @@ class Member(models.Model): class Band(models.Model): + uuid = models.UUIDField(unique=True, default=uuid.uuid4) name = models.CharField(max_length=100) style = models.CharField(max_length=20) members = models.ManyToManyField(Member) @@ -36,7 +37,7 @@ class UnsafeLimitChoicesTo(models.Model): class Album(models.Model): - band = models.ForeignKey(Band, models.CASCADE) + band = models.ForeignKey(Band, models.CASCADE, to_field='uuid') featuring = models.ManyToManyField(Band, related_name='featured') name = models.CharField(max_length=100) cover_art = models.FileField(upload_to='albums') diff --git a/tests/admin_widgets/test_autocomplete_widget.py b/tests/admin_widgets/test_autocomplete_widget.py index 2cb2e8fbf5..d8ee7e9f3a 100644 --- a/tests/admin_widgets/test_autocomplete_widget.py +++ b/tests/admin_widgets/test_autocomplete_widget.py @@ -14,12 +14,12 @@ class AlbumForm(forms.ModelForm): fields = ['band', 'featuring'] widgets = { 'band': AutocompleteSelect( - Album._meta.get_field('band').remote_field, + Album._meta.get_field('band'), admin.site, attrs={'class': 'my-class'}, ), 'featuring': AutocompleteSelect( - Album._meta.get_field('featuring').remote_field, + Album._meta.get_field('featuring'), admin.site, ) } @@ -54,9 +54,12 @@ class AutocompleteMixinTests(TestCase): 'data-ajax--cache': 'true', 'data-ajax--delay': 250, 'data-ajax--type': 'GET', - 'data-ajax--url': '/admin_widgets/band/autocomplete/', + 'data-ajax--url': '/autocomplete/', 'data-theme': 'admin-autocomplete', 'data-allow-clear': 'false', + 'data-app-label': 'admin_widgets', + 'data-field-name': 'band', + 'data-model-name': 'album', 'data-placeholder': '' }) @@ -76,19 +79,19 @@ class AutocompleteMixinTests(TestCase): self.assertJSONEqual(attrs['data-allow-clear'], False) def test_get_url(self): - rel = Album._meta.get_field('band').remote_field + rel = Album._meta.get_field('band') w = AutocompleteSelect(rel, admin.site) url = w.get_url() - self.assertEqual(url, '/admin_widgets/band/autocomplete/') + self.assertEqual(url, '/autocomplete/') def test_render_options(self): beatles = Band.objects.create(name='The Beatles', style='rock') who = Band.objects.create(name='The Who', style='rock') # With 'band', a ForeignKey. - form = AlbumForm(initial={'band': beatles.pk}) + form = AlbumForm(initial={'band': beatles.uuid}) output = form.as_table() - selected_option = '' % beatles.pk - option = '' % who.pk + selected_option = '' % beatles.uuid + option = '' % who.uuid self.assertIn(selected_option, output) self.assertNotIn(option, output) # With 'featuring', a ManyToManyField. diff --git a/tests/admin_widgets/tests.py b/tests/admin_widgets/tests.py index 334a534907..3c41d9cdfa 100644 --- a/tests/admin_widgets/tests.py +++ b/tests/admin_widgets/tests.py @@ -539,13 +539,13 @@ class ForeignKeyRawIdWidgetTest(TestCase): w = widgets.ForeignKeyRawIdWidget(rel, widget_admin_site) self.assertHTMLEqual( - w.render('test', band.pk, attrs={}), - '' - ' ' 'Linkin Park' - '' % {'bandpk': band.pk} + '' % {'banduuid': band.uuid, 'bandpk': band.pk} ) def test_relations_to_non_primary_key(self):