step not passed to get_form when using NamedUrls

[guillaume@…]

Hi,

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, data=self.storage.data['step_data'][self.STEP_PRODUCT])
        sale = sale_form.save(commit=False)
        self.common_attributes = CommonAttribute.objects.filter(for_type=sale.type)

    def get_form_initial(self, step):
        if step == self.steps.current
            self._calc_attributes()
            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:
            self._calc_attributes()
            formset = inlineformset_factory(Sale, ProductStock, can_delete=True,
                                         extra=self.attributes_len,
                                         formset=BaseStockFormSet)
            kwargs = self.get_form_kwargs(step)
            kwargs.update({
                '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!

Thanks

[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

[Aymeric Augustin]

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:
            ...

[anonymous]

Hi,

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?

Thanks!

[Aymeric Augustin]

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.

[Jacob]

Milestone 1.4 deleted