Fixed #29682 -- Fixed admin change form crash if a view-only model's form has an extra field.

This commit is contained in:
Tim Graham 2018-08-18 16:15:18 -04:00
parent 0e7a9525ba
commit d311124be5
6 changed files with 36 additions and 3 deletions

View File

@ -162,7 +162,7 @@ class AdminReadonlyField:
if form._meta.labels and class_name in form._meta.labels: if form._meta.labels and class_name in form._meta.labels:
label = form._meta.labels[class_name] label = form._meta.labels[class_name]
else: 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: if form._meta.help_texts and class_name in form._meta.help_texts:
help_text = form._meta.help_texts[class_name] help_text = form._meta.help_texts[class_name]

View File

@ -319,7 +319,7 @@ def _get_non_gfk_field(opts, name):
return field 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, 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 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) attr = getattr(model_admin, name)
elif hasattr(model, name): elif hasattr(model, name):
attr = getattr(model, name) attr = getattr(model, name)
elif form and name in form.fields:
attr = form.fields[name]
else: else:
message = "Unable to lookup '%s' on %s" % (name, model._meta.object_name) message = "Unable to lookup '%s' on %s" % (name, model._meta.object_name)
if model_admin: if model_admin:
message += " or %s" % (model_admin.__class__.__name__,) message += " or %s" % (model_admin.__class__.__name__,)
if form:
message += " or %s" % form.__class__.__name__
raise AttributeError(message) raise AttributeError(message)
if hasattr(attr, "short_description"): if hasattr(attr, "short_description"):

View File

@ -38,3 +38,6 @@ Bugfixes
* Made the admin change view redirect to the changelist view after a POST if * Made the admin change view redirect to the changelist view after a POST if
the user has the 'view' permission (:ticket:`29663`). 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`).

View File

@ -286,6 +286,22 @@ class UtilsTests(SimpleTestCase):
("not Really the Model", MockModelAdmin.test_from_model) ("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): def test_label_for_property(self):
# NOTE: cannot use @property decorator, because of # NOTE: cannot use @property decorator, because of
# AttributeError: 'property' object has no attribute 'short_description' # AttributeError: 'property' object has no attribute 'short_description'

View File

@ -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): class ArticleAdmin(admin.ModelAdmin):
list_display = ( list_display = (
'content', 'date', callable_year, 'model_year', 'modeladmin_year', 'content', 'date', callable_year, 'model_year', 'modeladmin_year',
@ -101,10 +109,11 @@ class ArticleAdmin(admin.ModelAdmin):
list_filter = ('date', 'section') list_filter = ('date', 'section')
autocomplete_fields = ('section',) autocomplete_fields = ('section',)
view_on_site = False view_on_site = False
form = ArticleForm
fieldsets = ( fieldsets = (
('Some fields', { ('Some fields', {
'classes': ('collapse',), 'classes': ('collapse',),
'fields': ('title', 'content') 'fields': ('title', 'content', 'extra_form_field'),
}), }),
('Some other fields', { ('Some other fields', {
'classes': ('wide',), 'classes': ('wide',),

View File

@ -1768,6 +1768,7 @@ class AdminViewPermissionsTest(TestCase):
response = self.client.get(article_change_url) response = self.client.get(article_change_url)
self.assertEqual(response.status_code, 200) self.assertEqual(response.status_code, 200)
self.assertEqual(response.context['title'], 'View article') self.assertEqual(response.context['title'], 'View article')
self.assertContains(response, '<label>Extra form field:</label>')
self.assertContains(response, '<a href="/test_admin/admin/admin_views/article/" class="closelink">Close</a>') self.assertContains(response, '<a href="/test_admin/admin/admin_views/article/" class="closelink">Close</a>')
post = self.client.post(article_change_url, change_dict) post = self.client.post(article_change_url, change_dict)
self.assertEqual(post.status_code, 302) self.assertEqual(post.status_code, 302)