#14498 closed (fixed)
Forms passed to FormWizard.process_step are not guaranteed to have clean validate data
Reported by: | Staś Małolepszy | Owned by: | Luke Plant |
---|---|---|---|
Component: | contrib.formtools | Version: | 1.2 |
Severity: | Keywords: | regression | |
Cc: | niels.busch@…, stas@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
The docs for FormWizard.process_step at source:trunk/django/contrib/formtools/wizard.py (line 263) read:
Hook for modifying the FormWizard's internal state, given a fully validated Form object. The Form is guaranteed to have clean, valid data.
There is however a case in FormWizard._ _call_ _ when this will not be true.
In source:trunk/django/contrib/formtools/wizard.py (line 95), the forms for each previous step are instantiated but they are not validated, which makes them not have the cleaned_data
property.
for i in range(current_step): form = self.get_form(i, request.POST) if not self._check_security_hash(request.POST.get("hash_%d" % i, ''), request, form): return self.render_hash_failure(request, i) self.process_step(request, form, i)
A solution would be to call form.is_valid in the above loop, or to edit the docs to account for this particular case.
Attachments (1)
Change History (10)
comment:1 by , 14 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:2 by , 14 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:3 by , 14 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:4 by , 14 years ago
follow-up: 6 comment:5 by , 14 years ago
Cc: | added |
---|---|
Resolution: | fixed |
Status: | closed → reopened |
I believe this changeset broke the Formwizard in other ways. It is not possible to alter the form_list in process_step anymore, because the current_step form is instantiated before previous_steps processing has been done. If I change the initial step2 form to a different form when processing step1 the current_step form is clearly not the right one anymore.
Notice I am creating the formwizard in a view, so I don't have previous state cached.
comment:6 by , 14 years ago
Cc: | added |
---|
I can confirm the same issue that niels described. I have a 3-step wizard where the step1 form is set up depending on the user's choices in step0. When I go from step0 to step1 the wizard works as expected, because form = self.get_form(next_step)
is executed after the processing loop from source:django/trunk/django/contrib/formtools/wizard.py#14574#L121.
However, when I go from step1 to step2, step1 gets instantiated with form = self.get_form(current_step, request.POST)
in source:django/trunk/django/contrib/formtools/wizard.py#14574#L96, before the processing loop has a chance to modify the form list. As a result, step1 form is still the one I defined in my view when instantiating my wizard.
This is broken both in 1.2.X and on trunk.
by , 14 years ago
Attachment: | 14498_regression_fix.patch added |
---|
A patch fixing the regression discovered in comment:5
comment:7 by , 14 years ago
Has patch: | set |
---|---|
Keywords: | regression added |
milestone: | → 1.3 |
Version: | SVN → 1.2 |
Updated flags to current state.
comment:8 by , 14 years ago
milestone: | 1.3 |
---|---|
Resolution: | → fixed |
Status: | reopened → closed |
As a technicality, this is now a different bug. Since the original bug remains fixed, it is more helpful to open a new ticket - otherwise the metadata from the original bug is lost, and the change logs referring to bugs becoming confusing. I've opened the new ticket now - #15075
comment:9 by , 14 years ago
Thanks, lukeplant, for opening #15075. I wasn't sure if I should do it or not myself. I'll know better for the next time!
(In [14290]) Fixed #14498 - Forms passed to FormWizard.process_step are not guaranteed to have cleaned_data
Thanks to stas for the report.