Opened 12 years ago

Closed 12 years ago

#18835 closed Uncategorized (worksforme)

ProcessFormView should implement get_context_data

Reported by: Preston Holmes Owned by: nobody
Component: Generic views Version: 1.4
Severity: Normal Keywords:
Cc: m.r.sopacua@… Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The current implementation of ProcessFormView adds context data inside the get method.

This specifically undermines the modular purpose of supporting a get_context_data method in the design

It means that if you implement a custom subclass, you have to duplicate this behavior in your own get_context_data or get methods, otherwise no form context gets sent to the template

Change History (3)

comment:1 by Preston Holmes, 12 years ago

Easy pickings: unset

In point of fact, this comes into play with any complex descendent of a View

I actually think that perhaps the right fix here, is that all subclasses of View - both builtin ones, and custom ones should

return super(MyClass, self).<HTTPMETHOD>(request, *args, **kwargs)

for any of the http named methods to gather any other custom kwarg behavior along the way in the inheritance chain

caveat: some people will assume that their impl of methods like get_context_data are only being called once, and or are not calling super - so in the end, not such a simple issue either way

in reply to:  1 comment:2 by Melvyn Sopacua, 12 years ago

Cc: m.r.sopacua@… added

Replying to ptone:

caveat: some people will assume that their impl of methods like get_context_data are only being called once, and or are not calling super - so in the end, not such a simple issue either way

Why people will assume this? It is very clearly explained in the documentation. I also don't see a good reason to change this behavior, because it is in fact very modular: You can add anything to the context at any stage in your view and the mixins it is made of. In fact, you can modify context data that is put in the context by a totally different mixin, as long as you keep track of the MRO. The pattern data = super().method(); do_something_with_data; return data is quite common.

This is probably more a documentation issue.

comment:3 by Aymeric Augustin, 12 years ago

Resolution: worksforme
Status: newclosed

Unlike TemplateView, getdoesn't pass its **kwargs to get_context_data; it just drops them. So does post when the form is invalid. As a consequence you can't add things to the context by overriding get and then calling super.

This looks correct to me. ProcessFormView needs to instantiate a form in get and post and process it. It expects to be at the top of the inheritance chain as far as these methods are concerned, and won't call the parent.

Generally speaking, I don't find it a realistic goal to make the generic views "infinitely combinable". They're useful as shortcut to achieve common tasks, less so as building blocks for more complex generic views.

Finally, maybe I'm being dense, but it seems to me that overriding get_context_data in a subclass to add things to the context works.

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