From 03d0f12c823239812da21e5180aaa74dc6fd146e Mon Sep 17 00:00:00 2001 From: Johannes Maron Date: Thu, 18 Mar 2021 11:21:23 +0100 Subject: [PATCH] Fixed #32466 -- Corrected autocomplete to_field resolution for complex cases. In MTI or ForeignKey as primary key cases, it is required to fetch the attname from the field instance on the remote model in order to reliably resolve the to_field_name. Co-authored-by: Johannes Maron Co-authored-by: Mariusz Felisiak Co-authored-by: Carlton Gibson --- django/contrib/admin/views/autocomplete.py | 3 +- django/contrib/admin/widgets.py | 4 +- tests/admin_views/models.py | 32 +++++++++++ tests/admin_views/test_autocomplete_view.py | 56 ++++++++++++++++++- tests/admin_widgets/models.py | 25 ++++++++- .../admin_widgets/test_autocomplete_widget.py | 23 +++++++- tests/admin_widgets/tests.py | 14 ++++- tests/admin_widgets/widgetadmin.py | 4 +- 8 files changed, 153 insertions(+), 8 deletions(-) diff --git a/django/contrib/admin/views/autocomplete.py b/django/contrib/admin/views/autocomplete.py index 6c52ecaeea..3903e4c98c 100644 --- a/django/contrib/admin/views/autocomplete.py +++ b/django/contrib/admin/views/autocomplete.py @@ -90,7 +90,8 @@ class AutocompleteJsonView(BaseListView): type(model_admin).__qualname__ ) - to_field_name = getattr(source_field.remote_field, 'field_name', model_admin.model._meta.pk.name) + to_field_name = getattr(source_field.remote_field, 'field_name', remote_model._meta.pk.attname) + to_field_name = remote_model._meta.get_field(to_field_name).attname if not model_admin.to_field_allowed(request, to_field_name): raise PermissionDenied diff --git a/django/contrib/admin/widgets.py b/django/contrib/admin/widgets.py index 1f438daf2d..aeb74773ac 100644 --- a/django/contrib/admin/widgets.py +++ b/django/contrib/admin/widgets.py @@ -428,7 +428,9 @@ 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) + remote_model_opts = self.field.remote_field.model._meta + to_field_name = getattr(self.field.remote_field, 'field_name', remote_model_opts.pk.attname) + to_field_name = remote_model_opts.get_field(to_field_name).attname 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}) diff --git a/tests/admin_views/models.py b/tests/admin_views/models.py index 4ab2bc8eee..284a4cfacb 100644 --- a/tests/admin_views/models.py +++ b/tests/admin_views/models.py @@ -338,6 +338,24 @@ class Child(models.Model): raise ValidationError('invalid') +class PKChild(models.Model): + """ + Used to check autocomplete to_field resolution when ForeignKey is PK. + """ + parent = models.ForeignKey(Parent, models.CASCADE, primary_key=True) + name = models.CharField(max_length=128) + + class Meta: + ordering = ['parent'] + + def __str__(self): + return self.name + + +class Toy(models.Model): + child = models.ForeignKey(PKChild, models.CASCADE) + + class EmptyModel(models.Model): def __str__(self): return "Primary key = %s" % self.id @@ -615,12 +633,26 @@ class Song(models.Model): class Employee(Person): code = models.CharField(max_length=20) + class Meta: + ordering = ['name'] + class WorkHour(models.Model): datum = models.DateField() employee = models.ForeignKey(Employee, models.CASCADE) +class Manager(Employee): + """ + A multi-layer MTI child. + """ + pass + + +class Bonus(models.Model): + recipient = models.ForeignKey(Manager, on_delete=models.CASCADE) + + class Question(models.Model): big_id = models.BigAutoField(primary_key=True) question = models.CharField(max_length=20) diff --git a/tests/admin_views/test_autocomplete_view.py b/tests/admin_views/test_autocomplete_view.py index c57fb106ed..aa978f7a83 100644 --- a/tests/admin_views/test_autocomplete_view.py +++ b/tests/admin_views/test_autocomplete_view.py @@ -12,7 +12,10 @@ from django.test import RequestFactory, override_settings from django.urls import reverse, reverse_lazy from .admin import AnswerAdmin, QuestionAdmin -from .models import Answer, Author, Authorship, Book, Question +from .models import ( + Answer, Author, Authorship, Bonus, Book, Employee, Manager, Parent, + PKChild, Question, Toy, WorkHour, +) from .tests import AdminViewBasicTestCase PAGINATOR_SIZE = AutocompleteJsonView.paginate_by @@ -37,6 +40,12 @@ site.register(Question, QuestionAdmin) site.register(Answer, AnswerAdmin) site.register(Author, AuthorAdmin) site.register(Book, BookAdmin) +site.register(Employee, search_fields=['name']) +site.register(WorkHour, autocomplete_fields=['employee']) +site.register(Manager, search_fields=['name']) +site.register(Bonus, autocomplete_fields=['recipient']) +site.register(PKChild, search_fields=['name']) +site.register(Toy, autocomplete_fields=['child']) @contextmanager @@ -118,6 +127,51 @@ class AutocompleteJsonViewTests(AdminViewBasicTestCase): 'pagination': {'more': False}, }) + def test_to_field_resolution_with_mti(self): + """ + to_field resolution should correctly resolve for target models using + MTI. Tests for single and multi-level cases. + """ + tests = [ + (Employee, WorkHour, 'employee'), + (Manager, Bonus, 'recipient'), + ] + for Target, Remote, related_name in tests: + with self.subTest(target_model=Target, remote_model=Remote, related_name=related_name): + o = Target.objects.create(name="Frida Kahlo", gender=2, code="painter", alive=False) + opts = { + 'app_label': Remote._meta.app_label, + 'model_name': Remote._meta.model_name, + 'field_name': related_name, + } + request = self.factory.get(self.url, {'term': 'frida', **opts}) + 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(o.pk), 'text': o.name}], + 'pagination': {'more': False}, + }) + + def test_to_field_resolution_with_fk_pk(self): + p = Parent.objects.create(name="Bertie") + c = PKChild.objects.create(parent=p, name="Anna") + opts = { + 'app_label': Toy._meta.app_label, + 'model_name': Toy._meta.model_name, + 'field_name': 'child', + } + request = self.factory.get(self.url, {'term': 'anna', **opts}) + 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(c.pk), 'text': c.name}], + '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 diff --git a/tests/admin_widgets/models.py b/tests/admin_widgets/models.py index 85f71749fd..85ba6c4f36 100644 --- a/tests/admin_widgets/models.py +++ b/tests/admin_widgets/models.py @@ -18,7 +18,11 @@ class Member(models.Model): return self.name -class Band(models.Model): +class Artist(models.Model): + pass + + +class Band(Artist): uuid = models.UUIDField(unique=True, default=uuid.uuid4) name = models.CharField(max_length=100) style = models.CharField(max_length=20) @@ -47,6 +51,25 @@ class Album(models.Model): return self.name +class ReleaseEvent(models.Model): + """ + Used to check that autocomplete widget correctly resolves attname for FK as + PK example. + """ + album = models.ForeignKey(Album, models.CASCADE, primary_key=True) + name = models.CharField(max_length=100) + + class Meta: + ordering = ['name'] + + def __str__(self): + return self.name + + +class VideoStream(models.Model): + release_event = models.ForeignKey(ReleaseEvent, models.CASCADE) + + class HiddenInventoryManager(models.Manager): def get_queryset(self): return super().get_queryset().filter(hidden=False) diff --git a/tests/admin_widgets/test_autocomplete_widget.py b/tests/admin_widgets/test_autocomplete_widget.py index d8ee7e9f3a..279acfe615 100644 --- a/tests/admin_widgets/test_autocomplete_widget.py +++ b/tests/admin_widgets/test_autocomplete_widget.py @@ -5,7 +5,7 @@ from django.forms import ModelChoiceField from django.test import TestCase, override_settings from django.utils import translation -from .models import Album, Band +from .models import Album, Band, ReleaseEvent, VideoStream class AlbumForm(forms.ModelForm): @@ -41,6 +41,18 @@ class RequiredBandForm(forms.Form): ) +class VideoStreamForm(forms.ModelForm): + class Meta: + model = VideoStream + fields = ['release_event'] + widgets = { + 'release_event': AutocompleteSelect( + VideoStream._meta.get_field('release_event'), + admin.site, + ), + } + + @override_settings(ROOT_URLCONF='admin_widgets.urls') class AutocompleteMixinTests(TestCase): empty_option = '' @@ -114,6 +126,15 @@ class AutocompleteMixinTests(TestCase): output = form.as_table() self.assertNotIn(self.empty_option, output) + def test_render_options_fk_as_pk(self): + beatles = Band.objects.create(name='The Beatles', style='rock') + rubber_soul = Album.objects.create(name='Rubber Soul', band=beatles) + release_event = ReleaseEvent.objects.create(name='Test Target', album=rubber_soul) + form = VideoStreamForm(initial={'release_event': release_event.pk}) + output = form.as_table() + selected_option = '' % release_event.pk + self.assertIn(selected_option, output) + def test_media(self): rel = Album._meta.get_field('band').remote_field base_files = ( diff --git a/tests/admin_widgets/tests.py b/tests/admin_widgets/tests.py index 3c41d9cdfa..f701f1abff 100644 --- a/tests/admin_widgets/tests.py +++ b/tests/admin_widgets/tests.py @@ -24,7 +24,7 @@ from django.utils import translation from .models import ( Advisor, Album, Band, Bee, Car, Company, Event, Honeycomb, Individual, Inventory, Member, MyFileField, Profile, School, Student, - UnsafeLimitChoicesTo, + UnsafeLimitChoicesTo, VideoStream, ) from .widgetadmin import site as widget_admin_site @@ -624,7 +624,17 @@ class ForeignKeyRawIdWidgetTest(TestCase): self.assertHTMLEqual( w.render('test', None), '\n' - '' + ) + + def test_render_fk_as_pk_model(self): + rel = VideoStream._meta.get_field('release_event').remote_field + w = widgets.ForeignKeyRawIdWidget(rel, widget_admin_site) + self.assertHTMLEqual( + w.render('test', None), + '\n' + '' ) diff --git a/tests/admin_widgets/widgetadmin.py b/tests/admin_widgets/widgetadmin.py index a471a362fb..a025bc82a7 100644 --- a/tests/admin_widgets/widgetadmin.py +++ b/tests/admin_widgets/widgetadmin.py @@ -2,7 +2,7 @@ from django.contrib import admin from .models import ( Advisor, Album, Band, Bee, Car, CarTire, Event, Inventory, Member, Profile, - School, User, + ReleaseEvent, School, User, VideoStream, ) @@ -47,6 +47,8 @@ site.register(Member) site.register(Band) site.register(Event, EventAdmin) site.register(Album, AlbumAdmin) +site.register(ReleaseEvent, search_fields=['name']) +site.register(VideoStream, autocomplete_fields=['release_event']) site.register(Inventory)