Code

Opened 5 years ago

Last modified 13 months ago

#10810 new Bug

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 5 years ago.
patch for FormWizard

Download all attachments as: .zip

Change History (14)

Changed 5 years ago by Qrilka

patch for FormWizard

comment:1 Changed 5 years ago by Qrilka

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

comment:2 Changed 5 years ago by Alex

  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 4 years ago by anonymous

  • Cc mbonetti@… added

comment:4 Changed 3 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 3 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 3 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 3 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 3 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 23 months 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 23 months 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 23 months 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 13 months ago by aaugustin

  • Status changed from reopened to new

comment:13 Changed 13 months 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".

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as new
The owner will be changed from nobody to anonymous. Next status will be 'assigned'
as The resolution will be set. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.