WizardView uses get_context_data incorrectly

[bradley.ayers@…]

https://code.djangoproject.com/browser/django/trunk/django/contrib/formtools/wizard/views.py#L538

However docs for every other generic view say get_context_data only takes kwargs, not args. I think it should be changed to form=form

[Julien Phalip]

This is a minor thing, but it makes sense to try and stay consistent here.

[Bradley Ayers <bradley.ayers@…>]

It became a problem when I wrote a mixin that expected the kwargs protocol and it didn't work :(

[Julien Phalip]

It'd be great if you could provide a test case illustrating that mixin problem. The patch itself should then be trivial.

[Bradley Ayers <bradley.ayers@…>]

Something along the lines of:

class NoopMixin(object):
    def get_context_data(self, **kwargs):
        return super(TestMixin, self).get_context_data(**kwargs)


class TestWizard(NoopMixin, WizardView)
    pass
    
    
view = TestWizard.as_view([forms.Form])

[Bradley Ayers <bradley.ayers@…>]

Sorry:

class NoopMixin(object):
    def get_context_data(self, **kwargs):
        return super(NoopMixin, self).get_context_data(**kwargs)

[Julien Phalip]

Thanks, your tickets are much appreciated. If you could provide patches while your mind is fresh working with formtools, it'd help fast-track all of those changes.

[Bradley Ayers <bradley.ayers@…>]

I'll see if I can whip something up in a few hours.

[Julien Phalip]

I'm not sure that what was originally reported here is necessary a problem. It's OK for a class to define a different signature for a method it overrides. In the context of the WizardView the form is quite important, so it would make sense to make it a required parameter in WizardView.get_context_data()'s signature -- if we decide to do so, then it would be a matter of clarifying the documentation. It could also be argued that consistency should be preserved with every other view and therefore that WizardView.get_context_data()'s signature should also be (self, **kwargs). I haven't personally used formtools in real projects yet so I prefer to leave that decision to someone who has (or perhaps to someone like jezdez who's done a lot of work on this). So for now, I'll switch this ticket to DDN.

I've noticed, however, a related but different issue. TemplateView.get_context_data() only accepts **kwargs, yet it is also passed *args by WizardView.get_context_data(). I have corrected that in the attached patch.

[Bradley Ayers <bradley.ayers@…>]

I want to elaborate on the issue I had. I wrote a generic mixin that implements get_context_data(self, **kwargs), and my inheritance was:

class MyView(MyMixin, WizardView):
    pass

I suppose I could have just switched the order of the inheritance. I'm not sure if this would solve every scenario like this though.

[Julien Phalip]

I see. The test case in your original patch only contains a single inheritance. If you could provide a test case using some mixins and where the current method signature poses a problem, then that would make the normalisation of signatures more compelling.

[Jannis Leidel]

Note the related tickets #16074 and #17242 which I think describe the general problem.

[steph]

Updated patch and extended tests

[steph]

I just uploaded a new patch with more tests. I think this is ready for checkin.

[Julien Phalip]

In [17231]:

Fixed #17148 -- Fixed the signature of WizardView.get_context_data() to play nicer with mixin inheritance. Thanks, Bradley Ayers and Stephan Jaekel.