Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#16442 closed Bug (invalid)

step not passed to get_form when using NamedUrls

Reported by: guillaume@… Owned by: nobody
Component: Forms Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no



I have noticed in the code of NamedUrlWizardView that you don't pass the step name to get_form.

I think it should pass it as else the only we have to know what step we are in is to use self.steps.current, which can bring some bugs. For example in my use case for the same step I need to customize get_form and get_form_initial. But in my get_form_initial I need to use get_form with data parameter to retrieve a previous form data to generate my new form. If I use self.steps.current it would bring a recursive loop.

Here is a piece of code that show the problem:

class wizard(SessionWizardView, NamedUrlSessionWizardView):

    def _calc_attributes(self):
        sale_form = self.get_form(step=self.STEP_PRODUCT,['step_data'][self.STEP_PRODUCT])
        sale =
        self.common_attributes = CommonAttribute.objects.filter(for_type=sale.type)

    def get_form_initial(self, step):
        if step == self.steps.current
            initials = []
            # add stuff in initials
            return initials
        return super(SaleWizard, self).get_form_initial(step)

    def get_form(self, step=None, data=None, files=None):
        if step == self.steps.current:
            formset = inlineformset_factory(Sale, ProductStock, can_delete=True,
            kwargs = self.get_form_kwargs(step)
                'data': data,
                'files': files,
                'prefix': self.get_form_prefix(step, self.form_list[step]),
                'initial': self.get_form_initial(step),
            return formset(**kwargs)
        return super(SaleWizard, self).get_form(step, data, files)

Would be great if it could be fixed!


Attachments (1)

patch-16442.diff (1.9 KB) - added by anonymous 7 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 7 years ago by anonymous

To be more precise it's when we do a get or post on the form that NamedUrlWizard doesn't pass the step name to get_form.

Also I made a mistake in the code:

 def get_form(self, step=None, data=None, files=None):
        if self.STEP_STOCKS == self.steps.current:
             # It will always be true even if I am calling get_form with an other step parameter. Leading to always come through the loop and make recursive calls indifinitely

Changed 7 years ago by anonymous

Attachment: patch-16442.diff added

comment:2 Changed 7 years ago by anonymous

Has patch: set

comment:3 Changed 7 years ago by Aymeric Augustin

Resolution: needsinfo
Status: newclosed

I've think I have more or less understood your problem, but your explanation is quite confusing, so I may have missed something :-/

The current implementation of get_form begins with:

        if step is None:
            step = self.steps.current

Can't you just replicate this logic in your implementation? e.g.

        if step is None:

or even better:

        if step is None or step == self.steps.current:

comment:4 Changed 7 years ago by anonymous

Resolution: needsinfo
Status: closedreopened


I am sorry if I haven't been clear in my explanation, I am going to try to do better:

If I use your solution, it's equivalent to what I wrote before, so it won't work.

Here is the workflow that is happening now, if I use your solution:

  1. NamedURLs calls get_info with no step variable, so step=None
  2. I check if self.steps.current == self.STEP_STOCK, if it is I run my custom code, else I run the default behavior
  3. I run my custom code, in my custom code I will call get_info with step variable step=self.STEP_PRODUCT, I do that to get back some info from my previous step
  4. I in get_info I am again checking self.steps.current == self.STEP_STOCKS, and of course this hasn't changed so
  5. My custom code will be ran again
  6. We have a recursive loop

Is it more clear?


comment:5 Changed 7 years ago by Aymeric Augustin

Resolution: invalid
Status: reopenedclosed

OK, the problem is definitely not in Django. Your patch adds step=self.steps.current to invocations of get_form, but that isn't necessary, since get_forms begins by setting steps to self.steps.current if it isn't provided in argument. The docstring is clear enough:

Constructs the form for a given step. If no step is defined, the current step will be determined automatically.

We generally avoid solving user problems on Trac because that leads to abuse — this is not a support forum — but given the time I have spent on this, here is it anyway. The problem is in 4. Instead of:

        if self.steps.current == self.STEP_STOCKS:

You should write:

        if step is None:
            step = self.steps.current
        if step == self.STEP_STOCKS:

Thus, when step is set to self.STEP_PRODUCT, you don't loop.

comment:6 Changed 7 years ago by Jacob

milestone: 1.4

Milestone 1.4 deleted

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