Opened 11 years ago
Closed 10 years ago
#21644 closed New feature (fixed)
FormWizard needs confirmation step logic
Reported by: | nickname123 | Owned by: | nobody |
---|---|---|---|
Component: | contrib.formtools | Version: | 1.6 |
Severity: | Normal | Keywords: | |
Cc: | bnafta@…, albrecht.andi@… | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
The FormWizard needs to handle a confirmation step that is capable of easily outputting a readonly copy of all step data. It is currently very difficult to support a confirmation step in the FormWizard workflow. The confirmation step needs access to the "final_form_list" created in the render_done() method and passed to the done() method. This is important so the user can see the data he is confirming.
I.e. the confirmation step as presented in the documentation https://docs.djangoproject.com/en/1.6/ref/contrib/formtools/form-wizard/#using-a-different-template-for-each-form does not satisfy the requirements for confirmation since the user cannot actually see his order that he is supposed to confirm.
This could be worked into the render_done() method, or broken into two steps. However, currently, one must duplicate a ton of code to render a confirmation view that outputs a readonly copy of the previous steps.
This could be implemented in a way that allows the developer to "opt out" of the confirmation if the specific FormWizard in question does not need a confirmation step.
EDIT: this cannot currently be performed easily in the done() method override because render_done() calls self.storage.reset() after it calls the done() method.
Change History (16)
comment:1 by , 11 years ago
Type: | Uncategorized → Bug |
---|
comment:2 by , 11 years ago
Type: | Bug → New feature |
---|
comment:3 by , 11 years ago
comment:4 by , 11 years ago
Description: | modified (diff) |
---|
follow-up: 13 comment:5 by , 11 years ago
Triage Stage: | Unreviewed → Accepted |
---|
I haven't looked at the code, but I'm open to the proposal if this is indeed overly difficult at the moment. It should be implemented in an "opt-in" fashion for backwards compatibility, not "opt-out" as proposed.
follow-up: 7 comment:6 by , 11 years ago
To my understanding, it would be interesting to add, optionally, in the confirmation step, a checkbox for each field, so that this step would only be completed if all were marked checkbox. That would be quite useful for me, because with this, I would force the User to stay longer on the page and increase the chances of him actually read what you wrote in forms and make any corrections.
comment:7 by , 11 years ago
Real example used in the registration form to apply for job positions in the government of the city where I live.
Registration is only completed if all checkbox are selected.
This type of strategy was used to force the User to identify and correct any errors in completion and to avoid further legal problems.
comment:8 by , 11 years ago
Cc: | added |
---|
comment:9 by , 11 years ago
I have taken a mixin approach in the project I am currently working on until I solved the problem in a satisfactory manner before creating patches.
I have several changes I have been working on with formwizards to make them accommodate my needs. I was planning to finish the wizard before submitting patches, but since others are interested I figured I should discuss.
My current approach seems to cover the cases presented I would like to hear other's thoughts.
Requirements:
- implemented in an opt-out fashion (timo)
- must support multiple confirmation steps (me)
- must allow arbitrary placement of confirmation forms in step order (me)
- must allow the user to present submitted data of previous steps (me)
To accomplish this I define a base confirmation form class. The only caveat is that no two confirmation steps should use the same confirmation form class. But this is a minor inconvenience for a large gain in functionality that could easily be documented and checked at runtime.
This allows for logic like the following:
if isinstance(form, BaseConfirmationForm): # pass information to form about previous steps
I took this approach because adding confirmation support to the wizard is as simple as defining a step that uses a subclass of the BaseConfirmationForm for its form.
I am hesitant to post my mixin because the last time I posted mixins versus a diff I didn't get a positive response. But I have not modified the wizard class at this point to provide a diff.
The shorthand of my changes are as follows:
The BaseConfirmationForm defines a set_forms method that accepts the forms of previous steps. I took this approach because I could not find a better approach for passing previous step data to the form. This way requires minimal changes to the wizard class but allows the developer unlimited flexibility with the business logic for a particular confirmation form... whether it is a simple check box to agree to Terms Of Service, or a complete confirmation form that requires the user to check it step, or something in between. My particular wizard needs both.
In wizard.get_form, if the step form is an instance of BaseConfirmationForm, it is provided with the forms from previous steps.
In wizard.render, if the form is an instance of BaseConfirmationForm, the forms are checked for validity. If they are invalid, render_revalidation_failure
is called on the first step for which form validation fails.
So far, this seems to work out well. I would appreciate feedback from someone who can give me a yae or nae for moving forward. My changes rely on https://code.djangoproject.com/ticket/21667. So I can post my mixins, but they will seem much more significant than they actually are. There isn't actually much code change required but I didn't want to force my project to use a custom fork of django at this point.
follow-ups: 11 12 comment:10 by , 11 years ago
nickname123 you have a test project that uses its implementation on github? If not, you could create it? I would test it
comment:11 by , 11 years ago
Replying to Fabio Caritas Barrionuevo da Luz <bnafta@…>:
nickname123 you have a test project that uses its implementation on github? If not, you could create it? I would test it
I will have some time in two weeks to put a demo together. I do not have anything together in a shareable state at this time.
comment:12 by , 11 years ago
Replying to Fabio Caritas Barrionuevo da Luz <bnafta@…>:
nickname123 you have a test project that uses its implementation on github? If not, you could create it? I would test it
I have pulled apart the relevant code and made a test project. It is very simple but I think it demonstrates the concept well enough. I can go into more detail if needed. Keep in mind, the mixin code would be minimal as a diff, but I had to copy entire methods with the mixin approach. There are actually very few changes.
https://github.com/thenewguy/django_formwizard_mixins
I did this quickly during lunch. Tested it with django 1.6.4 from pip briefly. Let me know if you have trouble making it work. I used a new python 2.7 virtualenv and did pip install django. Then used the runserver.
comment:13 by , 11 years ago
Replying to timo:
I haven't looked at the code, but I'm open to the proposal if this is indeed overly difficult at the moment. It should be implemented in an "opt-in" fashion for backwards compatibility, not "opt-out" as proposed.
Hello Timo. Would you take a look at the test project in my previous comment and let me know if you are satisfied with the approach? It is "opt-in" as requested.
comment:14 by , 10 years ago
Cc: | added |
---|
comment:15 by , 10 years ago
Component: | Generic views → contrib.formtools |
---|
comment:16 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
formtools has been extracted into its own repository (https://github.com/django/django-formtools/). Because of this, the issue tracking for this package has been moved to GitHub issues. I'm going to close this ticket, but I've created a GitHub issue to replace it where the conversation can continue: https://github.com/django/django-formtools/issues/17. Thanks!
I forgot to explain that this cannot currently be performed easily in the done() method override because render_done() calls self.storage.reset() after it calls the done() method.
If someone could add this to the ticket description I would be grateful. Thanks