Opened 8 years ago

Closed 7 years ago

Last modified 5 years ago

#9473 closed (fixed)

FormWizard doesn't work with NullableBooleanField

Reported by: LZ Owned by: Keith Bussell
Component: contrib.formtools Version: 1.0
Severity: Keywords: form formwizard nullablebooleanfield
Cc: Keith Bussell Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Create a series of forms. Make the first one contain a NullableBooleanField. Then, place the forms in a (contrib.formtools.) Wizard.

If the user set the value of the field to "True", it will succeed on the first submit. However, when there is a second submit, it will fail with a cryptic Exception message. It turns out that there's a render_hash_failure.

This is my first time trying Django, so I only have a few hours experience, but I think the problem comes from

http://code.djangoproject.com/browser/django/trunk/django/contrib/formtools/utils.py :
19 data = [(bf.name, bf.field.clean(bf.data) or ) for bf in form]

Thank you!

Attachments (2)

fix_9473.diff (4.5 KB) - added by Keith Bussell 7 years ago.
Fix and test
fix_9473_2.diff (4.8 KB) - added by Keith Bussell 7 years ago.
Same fix, new test

Download all attachments as: .zip

Change History (17)

comment:1 Changed 8 years ago by LZ

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

I meant NullBooleanField, BTW. Sorry for the error.

comment:2 Changed 8 years ago by LZ

I debugged this further. It turns out what widgets.NullBooleanSelect.value_from_datadict(self, data, files, name) needs to accept the 'True' and 'False' string values in addition to the True and False boolean values.

comment:3 Changed 8 years ago by (none)

milestone: post-1.0

Milestone post-1.0 deleted

comment:4 Changed 8 years ago by Jacob

milestone: 1.1
Triage Stage: UnreviewedAccepted

comment:5 Changed 8 years ago by Bob Thomas

This is similar to #9336, which describes a similar problem for CheckboxInput

comment:6 Changed 7 years ago by Keith Bussell

Owner: changed from nobody to Keith Bussell
Status: newassigned

Changed 7 years ago by Keith Bussell

Attachment: fix_9473.diff added

Fix and test

comment:7 Changed 7 years ago by Keith Bussell

Cc: Keith Bussell added
Has patch: set

I applied the change suggested by LZ and added a test.

comment:8 Changed 7 years ago by Jacob

Triage Stage: AcceptedReady for checkin

comment:9 Changed 7 years ago by Jacob

Triage Stage: Ready for checkinAccepted

Actually, maybe you can pull the info you need out of the context instead of using that regex. Take a look; I'm bumping this back out of rfc.

comment:10 Changed 7 years ago by Bob Thomas

This isn't really a bug in FormWizard. I don't think the regression tests have to involve it at all; you can just test the widget/field directly. See patch on #9336

Changed 7 years ago by Keith Bussell

Attachment: fix_9473_2.diff added

Same fix, new test

comment:11 Changed 7 years ago by Keith Bussell

fix_9473_2.diff updates the test to use the previous_fields context member instead of parsing the whole html response. Also broke up the test function a bit to make it more reusable for other tests in the future.

comment:12 Changed 7 years ago by Jacob

Triage Stage: AcceptedReady for checkin

comment:13 Changed 7 years ago by Jacob

Resolution: fixed
Status: assignedclosed

(In [10316]) Fixed #9473: FormWizard now works with NullBooleanFields. As a bonus, we now have the beginnings of a test suite for FormWizard. Thanks, Keith Bussell.

comment:14 Changed 7 years ago by Jacob

(In [10321]) [1.0.X] Fixed #9473: FormWizard now works with NullBooleanFields. As a bonus, we now have the beginnings of a test suite for FormWizard. Thanks, Keith Bussell. Backport of r10316 and r10319 from trunk.

comment:15 Changed 5 years ago by Jacob

milestone: 1.1

Milestone 1.1 deleted

Note: See TracTickets for help on using tickets.
Back to Top