#19545 closed Bug (fixed)
`BaseFormSet`'s `media` and `is_multipart` don't work correctly when `extra=0`
Reported by: | Simon Charette | Owned by: | nobody |
---|---|---|---|
Component: | Forms | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description
When generating a BaseFormSet
subclass with no extra forms the underlying form's medias are ignored.
Python 2.7.3 (default, Aug 1 2012, 05:14:39) Type "copyright", "credits" or "license" for more information. IPython 0.13.1 -- An enhanced Interactive Python. ? -> Introduction and overview of IPython's features. %quickref -> Quick reference. help -> Python's own help system. object? -> Details about 'object', use 'object??' for extra details. In [1]: from django import forms In [2]: class MediaForm(forms.Form): ...: class Media: ...: js = ('some-file.js',) ...: In [3]: str(forms.formsets.formset_factory(MediaForm)().media) Out[3]: '<script type="text/javascript" src="/static/some-file.js"></script>' In [4]: str(forms.formsets.formset_factory(MediaForm, extra=0)().media) Out[4]: ''
Attachments (2)
Change History (10)
by , 12 years ago
Attachment: | 0001-Fixed-19545-Make-sure-formsets-with-no-extra-forms-p.patch added |
---|
comment:1 by , 12 years ago
Has patch: | set |
---|
comment:2 by , 12 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:3 by , 12 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
I'm not so enthousiast to set a cached_property
. This is not performance-critical code, and with caching, you always create invalidation issue (e.g. call empty_form
before changing self.auto_id
).
BTW, I noticed that empty_form
had unusable kwargs parameters. I will fix that separately.
comment:4 by , 12 years ago
Just noticed is_multipart
has a similar issue.
Should I open another ticket for that or just fix it here and add another test?
comment:5 by , 12 years ago
Patch needs improvement: | set |
---|---|
Triage Stage: | Ready for checkin → Accepted |
Let's fix both issues in this ticket.
by , 12 years ago
Attachment: | 0001-Make-sure-is_multipart-works-with-empty-formsets.patch added |
---|
comment:6 by , 12 years ago
Patch needs improvement: | unset |
---|---|
Summary: | `BaseFormSet.media` is always empty when `extra=0` → `BaseFormSet`'s `media` and `is_multipart` don't work correctly when `extra=0` |
Added a second patch for the is_multipart
issue and changed ticket summary.
comment:7 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
I'm wondering if we should also decorate
BaseFormSet.empty_form
withcached_property
in order to avoid generating a form instance just for accessing it's media property. This instance could be reused when rendering the formset.