Opened 15 years ago

Closed 13 years ago

Last modified 12 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: dev
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: no

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@…> 15 years ago.
patch for class and docs
trac-10557.diff (6.2 KB ) - added by steph 13 years ago.
Patch with tests and docs.
trac-10557.2.diff (6.6 KB ) - added by steph 13 years ago.
Better patch with argument inspection

Download all attachments as: .zip

Change History (16)

by Matthew Flanagan <mattimustang@…>, 15 years ago

patch for class and docs

comment:1 by Jacob, 15 years ago

milestone: 1.2
Triage Stage: UnreviewedAccepted

comment:2 by Matthew Flanagan <mattimustang@…>, 15 years ago

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 by Jacob, 14 years ago

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 by James Bennett, 14 years ago

milestone: 1.21.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 by Łukasz Rekucki, 13 years ago

Patch needs improvement: unset
Triage Stage: AcceptedReady 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 by Luke Plant, 13 years ago

Needs tests: set
Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

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.

by steph, 13 years ago

Attachment: trac-10557.diff added

Patch with tests and docs.

comment:7 by steph, 13 years ago

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 by Luke Plant, 13 years ago

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.

by steph, 13 years ago

Attachment: trac-10557.2.diff added

Better patch with argument inspection

comment:9 by steph, 13 years ago

Keywords: sprintnov13 added

comment:10 by Chris Beaven, 13 years ago

Severity: Normal
Type: Bug

comment:11 by patchhammer, 13 years ago

Easy pickings: unset
Patch needs improvement: set

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

comment:12 by Jannis Leidel, 13 years ago

Resolution: wontfix
Status: newclosed

Superseded by #9200.

comment:13 by Jacob, 12 years ago

milestone: 1.3

Milestone 1.3 deleted

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