Opened 6 years ago

Closed 4 years ago

Last modified 3 years ago

#10557 closed Bug (wontfix)

FormWizard should not output raw HTML for previous_fields

Reported by: Matthew Flanagan <mattimustang@…> Owned by: nobody
Component: contrib.formtools Version: master
Severity: Normal Keywords: wizard sprintnov13
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX:

Description

When using a FormWizard to generate forms it populates a context variable called previous_fields which contains raw HTML hidden fields for the previous step values and security hashes. This does not play well when you are trying to use the FormWizard in other types markup.

My use case is that I'm using extjs for my application front-end and form presentation (though I could be using something else like XForms for example). I have a templatetag that spits out the appropriate extjs form widget when given a bound field. This fails for the FormWizard previous_fields because it is raw HTML.

The patch attached turns previous_fields into a list of bound field objects that can be iterated over to output the same raw HTML as before or give you more control over the output.

Attachments (3)

form_wizard_prev_fields.diff (3.6 KB) - added by Matthew Flanagan <mattimustang@…> 6 years ago.
patch for class and docs
trac-10557.diff (6.2 KB) - added by steph 4 years ago.
Patch with tests and docs.
trac-10557.2.diff (6.6 KB) - added by steph 4 years ago.
Better patch with argument inspection

Download all attachments as: .zip

Change History (16)

Changed 6 years ago by Matthew Flanagan <mattimustang@…>

patch for class and docs

comment:1 Changed 6 years ago by jacob

  • milestone set to 1.2
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 6 years ago by Matthew Flanagan <mattimustang@…>

See here for a installable module if you don't want to wait until Django 1.2 arrives.

http://wadofstuff.blogspot.com/2009/07/improved-django-formwizard.html

comment:3 Changed 5 years ago by jacob

  • Patch needs improvement set

This doesn't appear to be backwards compatible, which it needs to be if it's to go into 1.2.

comment:4 Changed 5 years ago by ubernostrum

  • milestone changed from 1.2 to 1.3

If it's not backwards-compatible it can't land in 1.2; luckily, you get another release cycle to get it into shape :)

comment:5 Changed 4 years ago by lrekucki

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

It's 1.3 now and apart from not being backwards compatible, looks good to me. This also helps writing tests, because you don't have to parse the raw HTML to get the security_hash for the next step.

comment:6 Changed 4 years ago by lukeplant

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

It's not ready for checkin if it's not backwards compatible, as Jacob said before. This needs a much better patch - for example, a new method that provides the new required format. With docs and tests, of course.

Changed 4 years ago by steph

Patch with tests and docs.

comment:7 Changed 4 years ago by steph

  • Needs tests unset
  • Patch needs improvement unset

.. just added a better patch including tests for the old previous_fields and the new previous_fields_list.

comment:8 Changed 4 years ago by lukeplant

There are backwards compatibility issues with changing the signature of FormWizard.render_template. It is a method that subclasses can override, therefore changes to it must be done with keyword arguments, and any time it is called from the FormWizard class we must check that the method accepts the keyword argument we want to pass (use inspect.getargspec).

Thanks.

Changed 4 years ago by steph

Better patch with argument inspection

comment:9 Changed 4 years ago by steph

  • Keywords sprintnov13 added

comment:10 Changed 4 years ago by SmileyChris

  • Severity set to Normal
  • Type set to Bug

comment:11 Changed 4 years ago by patchhammer

  • Easy pickings unset
  • Patch needs improvement set

trac-10557.2.diff fails to apply cleanly on to trunk

comment:12 Changed 4 years ago by jezdez

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

Superseded by #9200.

comment:13 Changed 3 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

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