Opened 6 years ago

Closed 9 months 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 6 years ago.
patch for FormWizard

Download all attachments as: .zip

Change History (16)

Changed 6 years ago by Qrilka

patch for FormWizard

comment:1 Changed 6 years ago by Qrilka

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

comment:2 Changed 6 years ago by Alex

  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 5 years ago by anonymous

  • Cc mbonetti@… added

comment:4 Changed 4 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 follow-up: Changed 4 years ago by SmileyChris

  • Severity set to Normal
  • Summary changed from FormWizard to FormWizard validates the last form twice
  • Type set to Bug

comment:6 in reply to: ↑ 5 Changed 4 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 4 years ago by julien

  • 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 follow-up: Changed 4 years ago by jezdez

  • Easy pickings unset
  • Resolution set to wontfix
  • Status changed from new to closed

Superseded by #9200.

comment:9 in reply to: ↑ 8 ; follow-up: Changed 3 years ago by hal_robertson@…

  • Resolution wontfix deleted
  • Status changed from closed to reopened
  • 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 ; follow-up: Changed 3 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 3 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 2 years ago by aaugustin

  • Status changed from reopened to new

comment:13 Changed 2 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 12 months ago by lucalenardi

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 9 months ago by gchp

  • Resolution set to fixed
  • Status changed from new to 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/21. Thanks!

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