Code

#18835 closed Uncategorized (worksforme)

ProcessFormView should implement get_context_data

Reported by: ptone 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

Attachments (0)

Change History (3)

comment:1 follow-up: Changed 20 months ago by ptone

  • Easy pickings unset
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement 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

comment:2 in reply to: ↑ 1 Changed 20 months ago by msopacua

  • 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 Changed 17 months ago by aaugustin

  • Resolution set to worksforme
  • Status changed from new to closed

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.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.