Opened 7 years ago

Closed 6 years ago

Last modified 3 years ago

#6893 closed (fixed)

FormWizard does not set the step value correctly

Reported by: siddhi Owned by: anonymous
Component: contrib.formtools Version: master
Severity: Keywords: form wizard formtools contrib
Cc: join.together@…, newijk@…, v.oostveen@…, adam@…, mpjung@…, philipp@…, rokclimb15@…, bthomas@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

The FormWizard class in django.contrib.formtools.wizard has a method called get_template which can be overridden to return a template for each step of the form submission process. This get_template method gets the step value as a parameter. However, no matter what step of the form processing we are in, this parameter is always 0.

The step value everywhere else is either computed dynamically or passed as a parameter. Only the get_template call uses self.step which is initialised to 0 and then never set again as we move through the steps.

The patch contains two fixes:

  1. The call to get_template uses the step that is passed as a parameter (and has the correct value) instead of using self.step
  2. self.step is updated at the end of processing

The patch also contains two test cases to check this. One of the tests should fail with the old code. Both tests should pass with the patched code.

As an aside, self.step is now used only in the repr method, so maybe it can be removed from there and then removed entirely from the FormWizard class?

Attachments (2)

form_wizard_patch.diff (3.4 KB) - added by siddhi 7 years ago.
Patch for the code and test cases for this bug
form_wizard_patch_2.diff (3.3 KB) - added by wamberg 7 years ago.
Updated patch to r8172

Download all attachments as: .zip

Change History (22)

Changed 7 years ago by siddhi

Patch for the code and test cases for this bug

comment:1 Changed 7 years ago by anonymous

  • Cc join.together@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 7 years ago by trbs

  • Cc v.oostveen@… added

Also hit this problem today and the patch looks fine to me.

At this moment, I've written a decorator for the formwizard.render_template method to inject the assignment of the self.step variable.
The code below is just a HACK and is by no means a permanent solution.

## Hack in FormWizard until django ticket #6893 is solved
def formwizard_render_template_decorator(func):
    def inject_assign_step(klass, request, form, previous_fields, step, *args, **kwargs):
        klass.step = step
        return func(klass, request, form, previous_fields, step, *args, **kwargs)
    return inject_assign_step

FormWizard.render_template = formwizard_render_template_decorator(FormWizard.render_template)

If you want to use this hack note two important things:

  1. This should only be applied once ! (thus not per FormWizard (sub)class definition)
  2. It must be removed after a fix is made !

I've only added this to the comments for people that do not like to patch the django installation there running.
But like said above this does require you to remember to remove it when a solution/patch hits django trunk.

comment:3 Changed 7 years ago by anonymous

  • Cc adam@… added

comment:4 Changed 7 years ago by telenieko

  • Triage Stage changed from Unreviewed to Accepted

comment:5 Changed 7 years ago by dan90

  • Cc dan90 added

comment:6 Changed 7 years ago by dan90

  • Cc dan90 removed

comment:7 Changed 7 years ago by anonymous

  • milestone set to 1.0

Since this is a bug, I'm putting it in 1.0

comment:8 Changed 7 years ago by Michael P. Jung

  • Cc mpjung@… added

comment:9 Changed 7 years ago by anonymous

  • Cc newijk@… added

comment:10 Changed 7 years ago by philwo

  • Cc philipp@… added

Changed 7 years ago by wamberg

Updated patch to r8172

comment:11 Changed 7 years ago by kmtracey

Closed #8099 as a dup.

comment:12 Changed 7 years ago by mcroydon

  • Owner changed from nobody to mcroydon
  • Status changed from new to assigned

comment:13 Changed 7 years ago by mcroydon

  • Owner changed from mcroydon to nobody
  • Status changed from assigned to new

Tests confirm original bug and pass after the fix is applied.

comment:14 Changed 7 years ago by anonymous

  • Cc rokclimb15@… added

comment:15 Changed 7 years ago by bthomas

  • Cc bthomas@… added

comment:16 Changed 7 years ago by anonymous

  • Owner changed from nobody to anonymous
  • Status changed from new to assigned

comment:17 Changed 7 years ago by jacob

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [8603]) Fixed #6893: FormWizard now properly updates its step value. Thanks, siddhi and wamberg.

comment:18 Changed 6 years ago by sunrise

  • Resolution fixed deleted
  • Status changed from closed to reopened
This works ok for me.. but It probably needs feedback. djangoproject Air Jordan

comment:19 Changed 6 years ago by kmtracey

  • Resolution set to fixed
  • Status changed from reopened to closed

comment:20 Changed 3 years ago by jacob

  • milestone 1.0 deleted

Milestone 1.0 deleted

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