Code

Opened 19 months ago

Closed 19 months ago

Last modified 19 months ago

#19545 closed Bug (fixed)

`BaseFormSet`'s `media` and `is_multipart` don't work correctly when `extra=0`

Reported by: charettes Owned by: nobody
Component: Forms Version: master
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)

comment:1 Changed 19 months ago by charettes

  • Has patch set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

I'm wondering if we should also decorate BaseFormSet.empty_form with cached_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.

comment:2 Changed 19 months ago by claudep

  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 19 months ago by claudep

  • Triage Stage changed from Accepted to 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 Changed 19 months ago by charettes

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 Changed 19 months ago by claudep

  • Patch needs improvement set
  • Triage Stage changed from Ready for checkin to Accepted

Let's fix both issues in this ticket.

comment:6 Changed 19 months ago by charettes

  • Patch needs improvement unset
  • Summary changed from `BaseFormSet.media` is always empty when `extra=0` to `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 Changed 19 months ago by Claude Paroz <claude@…>

  • Resolution set to fixed
  • Status changed from new to closed

In 3fc43c964ef7ab52261ec3a164d66b19f06f5cea:

Fixed #19545 -- Make sure media/is_multipart work with empty formsets

comment:8 Changed 19 months ago by Claude Paroz <claude@…>

In 70cc95d1ccab823d30d123d17e864c4867ee244b:

[1.5.x] Fixed #19545 -- Make sure media/is_multipart work with empty formsets

Backport of 3fc43c964e from master.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.