Opened 7 years ago

Closed 2 years ago

#10810 closed Bug (fixed)

FormWizard validates the last form twice

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

Description

FormWizard revalidates all the forms on the last step, after validating the last form. So the last one gets validated twice on the same request.
In my app I have captcha on the last form and it does not work with revalidation (and should not I think).
I've made changes to wizard.py, patch attached.

Attachments (1)

wizard_revalidation.diff (1.4 KB) - added by Qrilka 7 years ago.
patch for FormWizard

Download all attachments as: .zip

Change History (16)

Changed 7 years ago by Qrilka

Attachment: wizard_revalidation.diff added

patch for FormWizard

comment:1 Changed 7 years ago by Qrilka

Has patch: set
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

comment:2 Changed 7 years ago by Alex Gaynor

Triage Stage: UnreviewedAccepted

comment:3 Changed 6 years ago by anonymous

Cc: mbonetti@… added

comment:4 Changed 6 years ago by anedvedicky@…

The attached patch solves problem partially, since it assumes the captcha
field is always present at the last form of wizard.

I've proposed a different solution here:

http://code.google.com/p/django-simple-captcha/issues/detail?id=4#c5

I think this should be closed as not-a-bug/won't fix as long
as we agree that all wizard forms must be validated two times:

first validation after each form is submitted

second validation, when all wizard forms are validated together,
once we validate the last form.

comment:5 Changed 5 years ago by Chris Beaven

Severity: Normal
Summary: FormWizardFormWizard validates the last form twice
Type: Bug

comment:6 in reply to:  5 Changed 5 years ago by anedvedicky@…

Replying to SmileyChris:
your reasoning makes perfect sense to me. looks like a missing note in
FormWizzard documentation. Probably just using your sentence in your
comment:


wizard forms must be validated two times:

first validation after each form is submitted
second validation, when all wizard forms are validated together, once we validate the last form.


will make it clear enough. anyway feel free to close it as not-a-bug.

comment:7 Changed 5 years ago by Julien Phalip

Patch needs improvement: set

I don't think we should just close this ticket. If no easy way-around can be found then at least a note should be added to the doc.

comment:8 Changed 5 years ago by Jannis Leidel

Easy pickings: unset
Resolution: wontfix
Status: newclosed

Superseded by #9200.

comment:9 in reply to:  8 ; Changed 4 years ago by hal_robertson@…

Resolution: wontfix
Status: closedreopened
UI/UX: unset

Replying to jezdez:

Superseded by #9200.

why does the last form need to be validated twice?

the above could be modified as:

  1. first validation after each form is submitted
  2. second validation, once we validate the last form, when all wizard forms are validated together (all except the last one again!), .

that way you could retain the ability to put a captcha on the last form

comment:10 in reply to:  9 ; Changed 4 years ago by hal_robertson@…

Replying to hal_robertson@…:

Replying to jezdez:

Superseded by #9200.

why does the last form need to be validated twice?

the above could be modified as:

  1. first validation after each form is submitted
  2. second validation, once we validate the last form, when all wizard forms are validated together (all except the last one again!), .

that way you could retain the ability to put a captcha on the last form

--- views.py	2012-05-27 09:15:46.000000000 -0300
+++ views.py.new	2012-05-27 09:15:25.000000000 -0300
@@ -316,11 +316,11 @@
         # walk through the form list and try to validate the data again.
         for form_key in self.get_form_list():
             form_obj = self.get_form(step=form_key,
                 data=self.storage.get_step_data(form_key),
                 files=self.storage.get_step_files(form_key))
-            if not form_obj.is_valid():
+            if not form_obj.is_valid() and form_key != self.steps.last:
                 return self.render_revalidation_failure(form_key, form_obj, **kwargs)
             final_form_list.append(form_obj)
 
         # render the done view and reset the wizard before returning the
         # response. This is needed to prevent from rendering done with the

comment:11 in reply to:  10 Changed 4 years ago by anonymous

woops... massive mistake there! that if statement should be the other way around!

            if form_key != self.steps.last and not form_obj.is_valid():

sorry...

comment:12 Changed 4 years ago by Aymeric Augustin

Status: reopenednew

comment:13 Changed 4 years ago by steph

Changing the revalidation and making the last step a special case isn't a good idea. Think about a case when someone want's to add a captcha in the first step.. this would break too.

What do you think about a list of "don't revalidate" forms/steps? This would give people a way to mark specific steps as "validating once is fine, skip this form when revalidating everything for the done method".

comment:14 Changed 2 years ago by Luca Lenardi

I'm experiencing the same issue, involving a re-captcha field validated more than once. Using Django 1.6.5.

I agree with steph that hacking around the last step isn't a good idea and it should be something related to any single step or even to specific fields (mark a step or a field to not being re-validated).

I would also like to note that the form parameter passed to render_done() is actually the last step already validated, so as a temporary fix it should be possible to skip the test and store form in the final_form dictionary as is. But, is this the expected behaviour?

            if form_key != self.steps.last and not form_obj.is_valid():
                return self.render_revalidation_failure(form_key, form_obj, **kwargs)
            elif form_key == self.steps.last:
                # the passed form object, is the last step of the wizard
                # and at this point has been already validated.
                form_obj = form
            final_forms[form_key] = form_obj

comment:15 Changed 2 years ago by Greg Chapple

Resolution: fixed
Status: newclosed

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/21. Thanks!

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