Opened 16 years ago

Closed 14 years ago

#9200 closed New feature (fixed)

Add a session based form wizard

Reported by: David Durham Owned by: David Durham
Component: contrib.formtools Version: dev
Severity: Normal Keywords: session wizard forms
Cc: edcrypt@…, andrehcampos@…, donspauldingii@…, nate@…, shaun@…, domen@…, ckd, flosch@…, dougal85@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The provided patch adds a class that can be extended to create form wizards that:

1) Use GETs for form displays (except in the case of validation errors) and POSTs for form submissions.
2) Successful POSTs are redirected to GET the next page in the wizard. This removes the POST from browser history so that back and refresh pretty much just work.

I have provided plenty of methods that can be overridden for customization purposes. Right now, I think I'm probably storing too much info in the session scope, in particular request.POST data, cleaned_data, and other things. I have not tested with multipart form data. I do not have documentation or tests, just wanted to see if you're interested in having this in contrib.

Attachments (8)

wizard_tests.patch (6.8 KB ) - added by David Durham 16 years ago.
wizard_patch.txt (16.7 KB ) - added by David Durham 16 years ago.
formtools_patch.txt (41.5 KB ) - added by David Durham 16 years ago.
includes SessionWizard class, tests and documentation
session_wizard_patch.txt (42.1 KB ) - added by David Durham 16 years ago.
did some more testing, fixed a couple bugs, fixed a small problem with the docs
session_wizard_patch.diff (42.1 KB ) - added by ElliottM 16 years ago.
Same patch but in diff format
ticket9200-1.diff (138.4 KB ) - added by Jannis Leidel 14 years ago.
Updated patch (including changes for secure cookie from #12417)
ticket9200-2.diff (140.3 KB ) - added by Jannis Leidel 14 years ago.
9200-3.diff (156.8 KB ) - added by Jannis Leidel 14 years ago.
Updates as requested.

Download all attachments as: .zip

Change History (47)

comment:1 by Eduardo de Oliveira Padoan, 16 years ago

Cc: edcrypt@… added
Needs documentation: set
Needs tests: set
Triage Stage: UnreviewedDesign decision needed

comment:2 by hanne.moa@…, 16 years ago

I'm certainly interested; I've hacked the normal form wizard a bit to allow previews by putting the final POST into the session and redirecting to a new page with a submit-button and a dump of the data. (Oh and wizards which can take ModelForms with instances of course... can't have a wizard to change anything without that.)

What would be really nice is to allow each page/slide of the wizard to have its own url, then supporting forward/back would be trivially easy. Not needed for simple survey-things of course but handy when the wizard is "windows"-style, used to configure something.

I've yet to see a need to dynamically change a wizard too.

comment:3 by David Durham, 16 years ago

Each form page does have its own URL. Currently the URL has to be something like r'wizard/(?P<page0>\d+)/$' but it allows me to link directly to /wizard/0/, /wizard/1/ and so forth. In fact, even if I had a "back" button on the form that did nothing but go back, it should use a GET rather than POSTing the form.

comment:4 by hanne.moa@…, 16 years ago

Brilliant! So, now we wait for the design decision? Could just release it as an app/library too though.

by David Durham, 16 years ago

Attachment: wizard_tests.patch added

by David Durham, 16 years ago

Attachment: wizard_patch.txt added

comment:5 by Eduardo de Oliveira Padoan, 16 years ago

Needs tests: unset
Patch needs improvement: set

Instead of passing a list of forms (the form_list argument on SessionWizard.init()), would be nice if we could pass a sequence in the format:

(('step_slug', Form), ...)

Instead of r'wizard/(?P<page0>\d+)/$', that'd be r'wizard/(?P<step_slug>\w+)/$'.
The form_list can be tuned on a dict ( {'step_slug': Form, ...} ) doing form_dict = dict(form_list) , as you need to get the form given the url argument.

This change would allow us to have wizards with named pages, not just numbered.

comment:6 by David Durham, 16 years ago

That's a nice feature, so I'm working on adding it along side the current list init.

comment:7 by andrehcampos@…, 16 years ago

Cc: andrehcampos@… added

comment:8 by Eduardo de Oliveira Padoan, 16 years ago

Needs documentation: unset
Patch needs improvement: unset

comment:9 by Eduardo de Oliveira Padoan, 16 years ago

Keywords: forms added

by David Durham, 16 years ago

Attachment: formtools_patch.txt added

includes SessionWizard class, tests and documentation

comment:10 by donspaulding, 16 years ago

Cc: donspauldingii@… added

I haven't fully looked over the approach you've taken here, but I thought I might share what I've done here at work along these lines. I've put my SessionFormWizard up on DjangoSnippets.org, in case there are any ideas to be taken from it. I'll give this a more thorough look soon though and leave comments when appropriate.

comment:11 by Jacob, 16 years ago

There's a fair bit of duplicate code here. A better approach would be an abstract wizard base class and concrete hidden-form and session-based subclasses.

in reply to:  11 comment:12 by David Durham, 16 years ago

Replying to jacob:

There's a fair bit of duplicate code here. A better approach would be an abstract wizard base class and concrete hidden-form and session-based subclasses.

Can you give me some more pointers to any methods you have in mind. I'm happy to make the changes, just don't immediately specific enough commonality.

by David Durham, 16 years ago

Attachment: session_wizard_patch.txt added

did some more testing, fixed a couple bugs, fixed a small problem with the docs

comment:13 by anonymous, 16 years ago

It fails with PicklingError if there are any formsets in the forms

by ElliottM, 16 years ago

Attachment: session_wizard_patch.diff added

Same patch but in diff format

comment:14 by Nate, 16 years ago

Cc: nate@… added

I rolled one of these for myself a while back, and noticed that it was on the 1.1 wishlist. I see there's already a patch here, but you guys may find this implementation helpful. You can find a copy here. I'd upload it here, but I'm not sure it will be useful and I don't want to add clutter.

comment:15 by marcob, 16 years ago

In doc sessionwizard.txt:

form_data = self._get_all_cleaned_data(request.session)
self._clear_wizard_data_from_session(request.session)

Should be:

form_data = self._get_all_cleaned_data(request)
self._clear_wizard_data_from_session(request)

Twice :-)

comment:16 by Jannis Leidel, 15 years ago

milestone: 1.2
Triage Stage: Design decision neededAccepted

comment:17 by Domen Kožar, 15 years ago

Note about my usage of Nate's Session based Wizard:

I needed option to not fill all forms until the last one, so I had to change the following parts:

  • in finish() form validation, force is_bound=True to override django default behaviour making empty forms not valid
  • add request parameter to is_last() function in order to inspect unexpected wizard finish

Also note the following, which is very imporatant:

  • django upload handlers use cStringIO object which is NOT pickle-able. Use StringIO.StringIO instead.

comment:18 by Domen Kožar, 15 years ago

Cc: domen@… added

comment:19 by steph, 15 years ago

I started working on a rewrite of the formwizard with pluggable storages. I think it would be easier to work on the wizard in an third party module because there are many approaches to meet the requirements. Feed free to fork or ask for commit rights. Maybe we could get some stuff done to contribute the module to django in 1.3 - I don't think we'll get the module stable until the 1.2 freeze.

http://github.com/stephrdev/django-formwizard

comment:20 by Dana Spiegel, 15 years ago

Seeing as 1.2 is almost ready to be released, is there any chance that this patch will see the light of day in 1.2? I know its on the low-priority list, but I would love to know whether I should wait for it, or just forge ahead with my own solution.

Also, is there any indication about solving this issue for ModelForms, and not just regular Forms?

in reply to:  20 comment:21 by David Durham, 15 years ago

Replying to danaspiegel:

Seeing as 1.2 is almost ready to be released, is there any chance that this patch will see the light of day in 1.2? I know its on the low-priority list, but I would love to know whether I should wait for it, or just forge ahead with my own solution.

You can just take the code and use it outside of the django namespace, since this feature won't make it into the 1.2 release. Or forge ahead with your own of course.

Also, is there any indication about solving this issue for ModelForms, and not just regular Forms?

The patch that I submitted works with ModelForms. At least the model forms I tested it with. Someone else had a pickling error with field sets, but I was not able to reproduce that problem.

comment:22 by James Bennett, 15 years ago

milestone: 1.2

1.2 is feature-frozen, moving this feature request off the milestone.

comment:23 by Dana Spiegel, 15 years ago

In implementing this code (ddurham, thanks for providing it!), I ran into an issue where the usage of ModelForms doesn't work properly when stepping back to previous pages. Specifically, when using either ForeignKey or ManyToMany relationships, you cannot use the cleaned data to re-create new versions of the model instance since the representation of the form data for these two field types is different from the way you set init data. I've fixed the code to work for my usage, and will try to post the changes (though I don't know if they cover all cases or are implemented appropriately.

I believe this issue must be fixed before this patch is accepted.

in reply to:  23 comment:24 by David Durham, 15 years ago

Owner: changed from nobody to David Durham
Status: newassigned

Replying to danaspiegel:

In implementing this code (ddurham, thanks for providing it!), I ran into an issue where the usage of ModelForms doesn't work properly when stepping back to previous pages. Specifically, when using either ForeignKey or ManyToMany relationships, you cannot use the cleaned data to re-create new versions of the model instance since the representation of the form data for these two field types is different from the way you set init data. I've fixed the code to work for my usage, and will try to post the changes (though I don't know if they cover all cases or are implemented appropriately.

I believe this issue must be fixed before this patch is accepted.

I think I ran into this issue as well. You can see in the GET method where I check if the form is a model form and do things a little differently.

333 if issubclass(form_class, forms.ModelForm):

334 form = form_class(instance=form_class.Meta.model(page_data))
335 else:
336 form = form_class(initial=page_data)

Where page_data is the cleaned_data. If you had to do something different, could you post your code?

comment:25 by Dana Spiegel, 15 years ago

I haven't had time to put together a patch (nor am I sure this is the best way to do this) but here's the code:

    def GET(self, request, page_key):
        """
        Initialize a form if necessary, and display the form/page identified by 
        page_key.
        """
        page_data = self._get_cleaned_data(request, page_key)
        if page_data is None:
            form = self._get_form_classes(request)[page_key]()
        else:
            form_class = self._get_form_classes(request)[page_key]
            if issubclass(form_class, forms.ModelForm):
                # We need to manually create a new instance of the form's model, since we need to specially handle ManyToMany fields
                instance = form_class.Meta.model()
                for attribute_name, attribute_value in page_data.items():
                    if hasattr(instance, attribute_name):
                        setattr(instance, attribute_name, attribute_value)
                        if isinstance(attribute_value, base.Model):
                            # when the related object is a model object, we need to explicitly set the form field's value
                            page_data[attribute_name] = attribute_value.pk
                    elif isinstance(getattr(form_class.Meta.model, attribute_name), fields.related.ReverseSingleRelatedObjectDescriptor):
                        page_data[attribute_name] = attribute_value.pk
                    elif isinstance(getattr(form_class.Meta.model, attribute_name), fields.related.ReverseManyRelatedObjectsDescriptor):
                        page_data[attribute_name] = [value.pk for value in attribute_value]
                form = form_class(instance=instance, initial=page_data)
            else:
                form = form_class(initial=page_data)
        
        return self._show_form(request, page_key, form)

comment:26 by ckd, 15 years ago

Cc: ckd added
Resolution: fixed
Status: assignedclosed

comment:27 by ckd, 15 years ago

Resolution: fixed
Status: closedreopened

comment:28 by ckd, 15 years ago

Status: reopenednew

comment:29 by Dana Spiegel, 15 years ago

This code unfortunately also doesn't support ManyToMany relationships where you need to present an inline FormSet. I think that actually, the functionality of this code isn't much beyond the existing FormWizard shipping with 1.2, and though its cleaner, if there's going to be an upgrade to the FormWizard functionality, I think that there needs to be some rethinking about how it should work.

At the very least, it should provide over-rideable methods for getting the forms for a step, so that you can provide multiple forms (to support inline formsets) and their related instances, and actually support multiple forms per step.

If someone takes this on, I'd be happy to provide some complex use-cases and code to help figure out the right architecture.

I vote for "Patch Needs Improvement" (though I won't change the setting on this ticket since I'm not a core developer for Django)

comment:30 by steph, 15 years ago

my implementation works with ManyToMany and FormSet too. you may give it a try. I'm working on a proposal to get it into django 1.3 too.. django-formwizard docs are missing (basicly same as current formwizard), but feel free to contact me or open issues on github.

comment:31 by Shaun Cutts, 15 years ago

Cc: shaun@… added

comment:32 by anonymous, 15 years ago

Cc: flosch@… added

comment:33 by patchhammer, 14 years ago

Easy pickings: unset
Patch needs improvement: set
Severity: Normal
Type: Uncategorized

session_wizard_patch.diff fails to apply cleanly on to trunk

comment:34 by Jannis Leidel, 14 years ago

Type: UncategorizedNew feature

comment:35 by Dougal Matthews, 14 years ago

Cc: dougal85@… added

by Jannis Leidel, 14 years ago

Attachment: ticket9200-1.diff added

Updated patch (including changes for secure cookie from #12417)

comment:37 by Jannis Leidel, 14 years ago

FYI, https://github.com/jezdez/django/compare/feature/formwizard has the latest code (similar to the newly attached patch).

comment:38 by Jannis Leidel, 14 years ago

Patch needs improvement: unset

by Jannis Leidel, 14 years ago

Attachment: ticket9200-2.diff added

by Jannis Leidel, 14 years ago

Attachment: 9200-3.diff added

Updates as requested.

comment:39 by Jannis Leidel, 14 years ago

Resolution: fixed
Status: newclosed

In [16307]:

Fixed #9200 -- Added new form wizard to formtools based on class based views. Many thanks to Stephan Jäkel, ddurham and ElliottM for their work.

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