mirror of
				https://github.com/django/django.git
				synced 2025-10-31 09:41:08 +00:00 
			
		
		
		
	[2.1.x] Fixed #29682 -- Fixed admin change form crash if a view-only model's form has an extra field.
Backport of d311124be5 from master
			
			
This commit is contained in:
		| @@ -167,7 +167,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] | ||||||
|   | |||||||
| @@ -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"): | ||||||
|   | |||||||
| @@ -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`). | ||||||
|   | |||||||
| @@ -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' | ||||||
|   | |||||||
| @@ -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',), | ||||||
|   | |||||||
| @@ -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) | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user