From 8502e9f04974660c80efc74f6e60ed566b3f52b3 Mon Sep 17 00:00:00 2001 From: Alexander Gaevsky Date: Sat, 9 Jan 2016 22:15:21 +0200 Subject: [PATCH] [1.8.x] Fixed #26060 -- Fixed crash with reverse OneToOneField in ModelAdmin.readonly_fields. Backport of 9a33d3d76497d9e198de942ee1236c452231262f from master --- django/contrib/admin/helpers.py | 2 +- django/contrib/admin/utils.py | 2 +- docs/releases/1.8.9.txt | 3 ++ tests/admin_views/admin.py | 23 ++++++----- tests/admin_views/models.py | 7 +++- tests/admin_views/tests.py | 72 +++++++++++++++++++++++++++++---- 6 files changed, 90 insertions(+), 19 deletions(-) diff --git a/django/contrib/admin/helpers.py b/django/contrib/admin/helpers.py index 1b6a0ff8ad..36886bf222 100644 --- a/django/contrib/admin/helpers.py +++ b/django/contrib/admin/helpers.py @@ -203,7 +203,7 @@ class AdminReadonlyField(object): else: result_repr = linebreaksbr(result_repr) else: - if isinstance(f.rel, ManyToManyRel) and value is not None: + if isinstance(getattr(f, 'rel', None), ManyToManyRel) and value is not None: result_repr = ", ".join(map(six.text_type, value.all())) else: result_repr = display_for_field(value, f) diff --git a/django/contrib/admin/utils.py b/django/contrib/admin/utils.py index 8ddba506b0..1ea73d647b 100644 --- a/django/contrib/admin/utils.py +++ b/django/contrib/admin/utils.py @@ -376,7 +376,7 @@ def display_for_field(value, field): from django.contrib.admin.templatetags.admin_list import _boolean_icon from django.contrib.admin.views.main import EMPTY_CHANGELIST_VALUE - if field.flatchoices: + if getattr(field, 'flatchoices', None): return dict(field.flatchoices).get(value, EMPTY_CHANGELIST_VALUE) # NullBooleanField needs special-case null-handling, so it comes # before the general null test. diff --git a/docs/releases/1.8.9.txt b/docs/releases/1.8.9.txt index 8c5445b549..86bb03d916 100644 --- a/docs/releases/1.8.9.txt +++ b/docs/releases/1.8.9.txt @@ -26,3 +26,6 @@ Bugfixes * Fixed a crash when using an ``__in`` lookup inside a ``Case`` expression (:ticket:`26071`). + +* Fixed a crash when using a reverse ``OneToOneField`` in + ``ModelAdmin.readonly_fields`` (:ticket:`26060`). diff --git a/tests/admin_views/admin.py b/tests/admin_views/admin.py index 7a6b25b519..68960dfbc1 100644 --- a/tests/admin_views/admin.py +++ b/tests/admin_views/admin.py @@ -34,15 +34,15 @@ from .models import ( InlineReference, InlineReferer, Inquisition, Language, Link, MainPrepopulated, ModelWithStringPrimaryKey, NotReferenced, OldSubscriber, OtherStory, Paper, Parent, ParentWithDependentChildren, Person, Persona, - Picture, Pizza, Plot, PlotDetails, PluggableSearchPerson, Podcast, Post, - PrePopulatedPost, PrePopulatedPostLargeSlug, PrePopulatedSubPost, Promo, - Question, Recipe, Recommendation, Recommender, ReferencedByGenRel, - ReferencedByInline, ReferencedByParent, RelatedPrepopulated, Report, - Reservation, Restaurant, RowLevelChangePermissionModel, Section, - ShortMessage, Simple, Sketch, State, Story, StumpJoke, Subscriber, - SuperVillain, Telegram, Thing, Topping, UnchangeableObject, - UndeletableObject, UnorderedObject, UserMessenger, Villain, Vodcast, - Whatsit, Widget, Worker, WorkHour, + Picture, Pizza, Plot, PlotDetails, PlotProxy, PluggableSearchPerson, + Podcast, Post, PrePopulatedPost, PrePopulatedPostLargeSlug, + PrePopulatedSubPost, Promo, Question, Recipe, Recommendation, Recommender, + ReferencedByGenRel, ReferencedByInline, ReferencedByParent, + RelatedPrepopulated, Report, Reservation, Restaurant, + RowLevelChangePermissionModel, Section, ShortMessage, Simple, Sketch, + State, Story, StumpJoke, Subscriber, SuperVillain, Telegram, Thing, + Topping, UnchangeableObject, UndeletableObject, UnorderedObject, + UserMessenger, Villain, Vodcast, Whatsit, Widget, Worker, WorkHour, ) @@ -853,6 +853,10 @@ class InlineRefererAdmin(admin.ModelAdmin): inlines = [InlineReferenceInline] +class PlotReadonlyAdmin(admin.ModelAdmin): + readonly_fields = ('plotdetails',) + + class GetFormsetsArgumentCheckingAdmin(admin.ModelAdmin): fields = ['name'] @@ -907,6 +911,7 @@ site.register(Villain) site.register(SuperVillain) site.register(Plot) site.register(PlotDetails) +site.register(PlotProxy, PlotReadonlyAdmin) site.register(CyclicOne) site.register(CyclicTwo) site.register(WorkHour, WorkHourAdmin) diff --git a/tests/admin_views/models.py b/tests/admin_views/models.py index d68c44fd36..92182419c4 100644 --- a/tests/admin_views/models.py +++ b/tests/admin_views/models.py @@ -501,12 +501,17 @@ class Plot(models.Model): @python_2_unicode_compatible class PlotDetails(models.Model): details = models.CharField(max_length=100) - plot = models.OneToOneField(Plot) + plot = models.OneToOneField(Plot, null=True, blank=True) def __str__(self): return self.details +class PlotProxy(Plot): + class Meta: + proxy = True + + @python_2_unicode_compatible class SecretHideout(models.Model): """ Secret! Not registered with the admin! """ diff --git a/tests/admin_views/tests.py b/tests/admin_views/tests.py index 881ef51d56..cb804dca8b 100644 --- a/tests/admin_views/tests.py +++ b/tests/admin_views/tests.py @@ -52,11 +52,11 @@ from .models import ( FoodDelivery, FunkyTag, Gallery, Grommet, Inquisition, Language, Link, MainPrepopulated, ModelWithStringPrimaryKey, OtherStory, Paper, Parent, ParentWithDependentChildren, Person, Persona, Picture, Pizza, Plot, - PluggableSearchPerson, Podcast, Post, Promo, Question, RelatedPrepopulated, - Report, Restaurant, RowLevelChangePermissionModel, Section, ShortMessage, - Simple, Story, Subscriber, Telegram, Topping, UnchangeableObject, - UndeletableObject, UnorderedObject, Vodcast, Whatsit, Widget, Worker, - WorkHour, + PlotDetails, PluggableSearchPerson, Podcast, Post, Promo, Question, + RelatedPrepopulated, Report, Restaurant, RowLevelChangePermissionModel, + Section, ShortMessage, Simple, Story, Subscriber, Telegram, Topping, + UnchangeableObject, UndeletableObject, UnorderedObject, Villain, Vodcast, + Whatsit, Widget, Worker, WorkHour, ) @@ -64,6 +64,44 @@ ERROR_MESSAGE = "Please enter the correct username and password \ for a staff account. Note that both fields may be case-sensitive." +class AdminFieldExtractionMixin(object): + """ + Helper methods for extracting data from AdminForm. + """ + def get_admin_form_fields(self, response): + """ + Return a list of AdminFields for the AdminForm in the response. + """ + admin_form = response.context['adminform'] + fieldsets = list(admin_form) + + field_lines = [] + for fieldset in fieldsets: + field_lines += list(fieldset) + + fields = [] + for field_line in field_lines: + fields += list(field_line) + + return fields + + def get_admin_fields(self, response): + """ + Return the fields for the response's AdminForm. + """ + return [f for f in self.get_admin_form_fields(response)] + + def get_admin_field(self, response, field_name): + """ + Return the field for the given field_name. + """ + fields = self.get_admin_fields(response) + for field in fields: + name = field.field['name'] if isinstance(field.field, dict) else field.field.name + if name == field_name: + return field + + @override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',), ROOT_URLCONF="admin_views.urls", USE_I18N=True, USE_L10N=False, LANGUAGE_CODE='en') @@ -4029,7 +4067,7 @@ class SeleniumAdminViewsIETests(SeleniumAdminViewsFirefoxTests): @override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',), ROOT_URLCONF="admin_views.urls") -class ReadonlyTest(TestCase): +class ReadonlyTest(AdminFieldExtractionMixin, TestCase): fixtures = ['admin-views-users.xml'] def setUp(self): @@ -4126,7 +4164,7 @@ class ReadonlyTest(TestCase): self.assertContains(response, '

No opinion

', html=True) self.assertNotContains(response, '

(None)

') - def test_readonly_backwards_ref(self): + def test_readonly_manytomany_backwards_ref(self): """ Regression test for #16433 - backwards references for related objects broke if the related field is read-only due to the help_text attribute @@ -4137,6 +4175,26 @@ class ReadonlyTest(TestCase): response = self.client.get(reverse('admin:admin_views_topping_add')) self.assertEqual(response.status_code, 200) + def test_readonly_onetoone_backwards_ref(self): + """ + Can reference a reverse OneToOneField in ModelAdmin.readonly_fields. + """ + v1 = Villain.objects.create(name='Adam') + pl = Plot.objects.create(name='Test Plot', team_leader=v1, contact=v1) + pd = PlotDetails.objects.create(details='Brand New Plot', plot=pl) + + response = self.client.get(reverse('admin:admin_views_plotproxy_change', args=(pl.pk,))) + field = self.get_admin_field(response, 'plotdetails') + self.assertEqual(field.contents(), 'Brand New Plot') + + # The reverse relation also works if the OneToOneField is null. + pd.plot = None + pd.save() + + response = self.client.get(reverse('admin:admin_views_plotproxy_change', args=(pl.pk,))) + field = self.get_admin_field(response, 'plotdetails') + self.assertEqual(force_text(field.contents()), '(None)') + def test_readonly_field_overrides(self): """ Regression test for #22087 - ModelForm Meta overrides are ignored by