diff --git a/django/contrib/admin/helpers.py b/django/contrib/admin/helpers.py index d0350c3930a..6fb35be1f33 100644 --- a/django/contrib/admin/helpers.py +++ b/django/contrib/admin/helpers.py @@ -162,7 +162,7 @@ class AdminReadonlyField: if form._meta.labels and class_name in form._meta.labels: label = form._meta.labels[class_name] else: - label = label_for_field(field, form._meta.model, model_admin) + label = label_for_field(field, form._meta.model, model_admin, form=form) if form._meta.help_texts and class_name in form._meta.help_texts: help_text = form._meta.help_texts[class_name] diff --git a/django/contrib/admin/utils.py b/django/contrib/admin/utils.py index eae09b2238d..1db552bcd85 100644 --- a/django/contrib/admin/utils.py +++ b/django/contrib/admin/utils.py @@ -319,7 +319,7 @@ def _get_non_gfk_field(opts, name): return field -def label_for_field(name, model, model_admin=None, return_attr=False): +def label_for_field(name, model, model_admin=None, return_attr=False, form=None): """ Return a sensible label for a field name. The name can be a callable, property (but not created with @property decorator), or the name of an @@ -346,10 +346,14 @@ def label_for_field(name, model, model_admin=None, return_attr=False): attr = getattr(model_admin, name) elif hasattr(model, name): attr = getattr(model, name) + elif form and name in form.fields: + attr = form.fields[name] else: message = "Unable to lookup '%s' on %s" % (name, model._meta.object_name) if model_admin: message += " or %s" % (model_admin.__class__.__name__,) + if form: + message += " or %s" % form.__class__.__name__ raise AttributeError(message) if hasattr(attr, "short_description"): diff --git a/docs/releases/2.1.1.txt b/docs/releases/2.1.1.txt index ff37bfefdaf..07912eead24 100644 --- a/docs/releases/2.1.1.txt +++ b/docs/releases/2.1.1.txt @@ -38,3 +38,6 @@ Bugfixes * Made the admin change view redirect to the changelist view after a POST if the user has the 'view' permission (:ticket:`29663`). + +* Fixed admin change view crash for view-only users if the form has an extra + form field (:ticket:`29682`). diff --git a/tests/admin_utils/tests.py b/tests/admin_utils/tests.py index d2697ca87e5..463ba9556d4 100644 --- a/tests/admin_utils/tests.py +++ b/tests/admin_utils/tests.py @@ -286,6 +286,22 @@ class UtilsTests(SimpleTestCase): ("not Really the Model", MockModelAdmin.test_from_model) ) + def test_label_for_field_form_argument(self): + class ArticleForm(forms.ModelForm): + extra_form_field = forms.BooleanField() + + class Meta: + fields = '__all__' + model = Article + + self.assertEqual( + label_for_field('extra_form_field', Article, form=ArticleForm()), + 'Extra form field' + ) + msg = "Unable to lookup 'nonexistent' on Article or ArticleForm" + with self.assertRaisesMessage(AttributeError, msg): + label_for_field('nonexistent', Article, form=ArticleForm()), + def test_label_for_property(self): # NOTE: cannot use @property decorator, because of # AttributeError: 'property' object has no attribute 'short_description' diff --git a/tests/admin_views/admin.py b/tests/admin_views/admin.py index 8565d04a056..04cc6c79e76 100644 --- a/tests/admin_views/admin.py +++ b/tests/admin_views/admin.py @@ -91,6 +91,14 @@ class ChapterXtra1Admin(admin.ModelAdmin): ) +class ArticleForm(forms.ModelForm): + extra_form_field = forms.BooleanField(required=False) + + class Meta: + fields = '__all__' + model = Article + + class ArticleAdmin(admin.ModelAdmin): list_display = ( 'content', 'date', callable_year, 'model_year', 'modeladmin_year', @@ -101,10 +109,11 @@ class ArticleAdmin(admin.ModelAdmin): list_filter = ('date', 'section') autocomplete_fields = ('section',) view_on_site = False + form = ArticleForm fieldsets = ( ('Some fields', { 'classes': ('collapse',), - 'fields': ('title', 'content') + 'fields': ('title', 'content', 'extra_form_field'), }), ('Some other fields', { 'classes': ('wide',), diff --git a/tests/admin_views/tests.py b/tests/admin_views/tests.py index 297d625376e..df1936aa90c 100644 --- a/tests/admin_views/tests.py +++ b/tests/admin_views/tests.py @@ -1768,6 +1768,7 @@ class AdminViewPermissionsTest(TestCase): response = self.client.get(article_change_url) self.assertEqual(response.status_code, 200) self.assertEqual(response.context['title'], 'View article') + self.assertContains(response, '') self.assertContains(response, 'Close') post = self.client.post(article_change_url, change_dict) self.assertEqual(post.status_code, 302)