Opened 7 years ago

Closed 6 years ago

Last modified 4 years ago

#9473 closed (fixed)

FormWizard doesn't work with NullableBooleanField

Reported by: LZ Owned by: keithb
Component: contrib.formtools Version: 1.0
Severity: Keywords: form formwizard nullablebooleanfield
Cc: keithb 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 keithb 6 years ago.
Fix and test
fix_9473_2.diff (4.8 KB) - added by keithb 6 years ago.
Same fix, new test

Download all attachments as: .zip

Change History (17)

comment:1 Changed 7 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 7 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 6 years ago by anonymous

  • milestone post-1.0 deleted

Milestone post-1.0 deleted

comment:4 Changed 6 years ago by jacob

  • milestone set to 1.1
  • Triage Stage changed from Unreviewed to Accepted

comment:5 Changed 6 years ago by bthomas

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

comment:6 Changed 6 years ago by keithb

  • Owner changed from nobody to keithb
  • Status changed from new to assigned

Changed 6 years ago by keithb

Fix and test

comment:7 Changed 6 years ago by keithb

  • Cc keithb added
  • Has patch set

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

comment:8 Changed 6 years ago by jacob

  • Triage Stage changed from Accepted to Ready for checkin

comment:9 Changed 6 years ago by jacob

  • Triage Stage changed from Ready for checkin to Accepted

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 6 years ago by bthomas

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 6 years ago by keithb

Same fix, new test

comment:11 Changed 6 years ago by keithb

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 6 years ago by jacob

  • Triage Stage changed from Accepted to Ready for checkin

comment:13 Changed 6 years ago by jacob

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

(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 6 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 4 years ago by jacob

  • milestone 1.1 deleted

Milestone 1.1 deleted

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