Code

Opened 3 years ago

Closed 23 months ago

Last modified 17 months ago

#17228 closed Cleanup/optimization (fixed)

params context variable from TemplateView is inconsistent with other get_context_data implementations

Reported by: ptone Owned by: nobody
Component: Generic views Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The general pattern of get_context_data is to return a context dictionary where the kwargs passed to the method are used to update the context.

This pattern is true for all classes that implement get_context_data except for TemplateView, which adds all the kwargs as a 'params' item.

This is as documented but is inconsistent and makes it trickier to finish up work on #16074 which requires that all implementations of get_context_data call super.

The cleanest design would be a pattern of:

def get_context_data(self, **kwargs):
    context = super(.... )(kwargs)
    # class specific stuff here
    return context

I propose that the use of a params item be deprecated, removed from the docs, and preserved through deprecation by both updating the context with kwargs, and defining the explicit 'params' item in TemplateView.

Attachments (0)

Change History (9)

comment:1 Changed 3 years ago by ptone

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Summary changed from params context variable is from TemplateView is inconsistent with other get_context_data implementations to params context variable from TemplateView is inconsistent with other get_context_data implementations

comment:2 Changed 3 years ago by ptone

The reason for params is to allow easier migration from direct_to_template:

https://docs.djangoproject.com/en/dev/ref/generic-views/#django-views-generic-simple-direct-to-template

However, I still think that the pattern is deviant enough from the main CBV to warrant a deprecation, which won't completely kick in until a version after function based views are completely removed from Django.

comment:3 Changed 3 years ago by aaugustin

  • Triage Stage changed from Unreviewed to Accepted

comment:4 Changed 3 years ago by ptone

I have inadvertently essentially duplicated this issue in #17381 however there is a subtle difference.

My greater gripe is with taking all kwargs to get_context_data and stuffing it into the params context variable.

If the legacy behavior is to be supported, it would be better IMO to have the get method call self.get_context_data thusly:

context = self.get_context_data(params=kwargs)

instead of

context = self.get_context_data(**kwargs)

This would allow get_context_data to be called with other kwargs in a call chain from a subclass without all those kwargs getting sucked into the params context variable.

comment:5 Changed 3 years ago by anonymous

see #17381 for some related discussion

comment:6 Changed 23 months ago by Andrew Godwin <andrew@…>

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

In [f04bb6d798b07aa5e7c1d99d700fa6ddc7d39e62]:

Fixed #17228 -- params context variable is inconsistent

Remove the params variable from the context and just put the variables
in directly.

This had not been committed previously as the original pattern was used
in the functional generic views and we wanted consistency between them,
but django.views.generic.simple.direct_to_template is now gone so we can
do it 'right'.

comment:7 Changed 17 months ago by Claude Paroz <claude@…>

In 56e553129f554f83c8e99ef3368544921dbd8a82:

Fixed #19714 -- Updated documentation about TemplateView context

Thanks Aramgutang for the report. Refs #17228.

comment:8 Changed 17 months ago by Claude Paroz <claude@…>

In 5ebd0452ba6ff93980e6f9a8958f6ea01c835ffe:

[1.5.x] Fixed #19714 -- Updated documentation about TemplateView context

Thanks Aramgutang for the report. Refs #17228.
Backport of 56e553129 from master.

comment:9 Changed 17 months ago by Claude Paroz <claude@…>

In 5ebd0452ba6ff93980e6f9a8958f6ea01c835ffe:

[1.5.x] Fixed #19714 -- Updated documentation about TemplateView context

Thanks Aramgutang for the report. Refs #17228.
Backport of 56e553129 from master.

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.