Code

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#15722 closed Bug (fixed)

Conditional Formset Rendering/Validation

Reported by: mlavin Owned by: nobody
Component: Forms Version: 1.3
Severity: Normal Keywords:
Cc: ironfroggy@…, mmanfre@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

#14655 introduced formsets as iterables and defined __len__ as the number of forms. However this causes a side effect in that if there are no forms then bool returns False. Meaning if you have a condition in your template which looks for the formset in the context before rendering

{% if formset %}
{# formset rendering would go here... #}
{% endif %}

then if there are no forms this will not render. It also means that the management form will not be rendered. On POST if the formset is bound without the management form data it raises a ValidationError on __init__.

There are possible work-arounds:

1.) Override __nonzero___ on the formset to return True

2.) Change conditional statements to look for the existence of the management form instead i.e. {% if formset.management_form %}

Attachments (1)

15722.diff (1.3 KB) - added by mlavin 3 years ago.
Patch for fix with test.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 3 years ago by calvinspealman

  • Cc ironfroggy@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 3 years ago by manfre

  • Cc mmanfre@… added

comment:3 Changed 3 years ago by lukeplant

  • Type set to Bug

comment:4 Changed 3 years ago by lukeplant

  • Severity set to Normal

comment:5 Changed 3 years ago by carljm

  • Triage Stage changed from Unreviewed to Accepted

I think we should override nonzero to return True. It's slightly unusual to have len(obj) == 0 and bool(obj) is True, but I think it's sensible here: len() reflecting the number of forms is useful, but a formset with no forms is not really "empty", as the management form alone is still functionally significant.

Changed 3 years ago by mlavin

Patch for fix with test.

comment:6 Changed 3 years ago by mlavin

  • Easy pickings unset
  • Has patch set
  • UI/UX unset

comment:7 Changed 3 years ago by kmtracey

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

In [16756]:

Fixed #15722: ensure formsets evaluate to True even if they have no forms. Thanks mlavin.

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.