#26729 closed Bug (fixed)
TabularInline not respecting form's custom label and help text if set in form's __init__ method
Reported by: | nrogers64 | Owned by: | Paulo |
---|---|---|---|
Component: | contrib.admin | Version: | 1.9 |
Severity: | Normal | Keywords: | |
Cc: | commonzenpython@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Here is my models.py file:
from django.db import models class Author(models.Model): name = models.CharField(max_length=100) class Meta: ordering = ('name',) def __unicode__(self): return self.name class Book(models.Model): author = models.ForeignKey(Author, on_delete=models.CASCADE) title = models.CharField(max_length=100, help_text='The book\'s title.') class Meta: ordering = ('title',) def __unicode__(self): return self.title
Here is my forms.py file:
from django import forms from .models import Book class BookForm1(forms.ModelForm): title = forms.CharField(label='Foo', help_text='Bar') class Meta: model = Book fields = [ 'title', ] class BookForm2(forms.ModelForm): class Meta: model = Book fields = [ 'title', ] def __init__(self, *args, **kwargs): super(BookForm2, self).__init__(*args, **kwargs) self.fields['title'].label = 'Baz' self.fields['title'].help_text = 'Qux'
Here is my admin.py file:
from django.contrib import admin from .forms import BookForm1 from .models import Author, Book class BookInline(admin.TabularInline): model = Book form = BookForm1 class AuthorAdmin(admin.ModelAdmin): inlines = [ BookInline, ] admin.site.register(Author, AuthorAdmin) admin.site.register(Book)
Using the above code, if I go to /admin/name_of_app/author/add/ it works as expected, meaning that the field for the books is labeled "Foo" and its help text says "Bar". However, if I edit admin.py and replace both instances of BookForm1
with BookForm2
, the field for the books is labeled "Title" and its help text says "The book's title.". In other words, it is falling back to the title and help text provided by the model instead of using what's defined in the BookForm2
class' __init__
method. I should also note that if I replace admin.TabularInline
with admin.StackedInline
then both BookForm1
and BookForm2
work as expected.
Attachments (2)
Change History (12)
comment:1 by , 8 years ago
Component: | Uncategorized → contrib.admin |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 8 years ago
I've done a little digging on this.
The problem is that the field labels need to be resolved before rendering the forms and so currently Django will look at the
base_fields
attribute of the formset form class instead of creating an instance of the form class. This means that the __init__
method is never called when generating the table headers and so no modifications there will be considered.
As a solution we would create a throw away instance of the formset form class and fetch the field from its fields
attribute but I don't like the idea of creating an empty form instance without the developer having a way to modify its initialization arguments.
I've attached a path for the suggested solution.
by , 8 years ago
Attachment: | 26729.diff added |
---|
follow-up: 4 comment:3 by , 8 years ago
I'd agree with that analysis. Did you look into why it works for StackedInline
.
comment:4 by , 8 years ago
Replying to timgraham:
I'd agree with that analysis. Did you look into why it works for
StackedInline
.
Yes, StackedInline
does not need to get the field names prior to rendering the forms.
Instead, it just renders each form instance individually and thus uses a BoundField
instance when rendering each field and label.
comment:5 by , 8 years ago
Cc: | added |
---|
comment:6 by , 8 years ago
Looking at this a bit more, I saw we have access to the empty form instance using formset.empty_form
.
I feel much better about using that. Attached is the revised patch.
by , 8 years ago
Attachment: | 26729_v2.diff added |
---|
comment:7 by , 8 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:8 by , 8 years ago
Has patch: | set |
---|
Seems reasonable, although I haven't looked into what might be involved in solving it.