Code

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#15328 closed (fixed)

Class-based Views (CBV) documentation mistake on method_decorator approach for @login_required

Reported by: airstrike Owned by: gabrielhurley
Component: Documentation Version: 1.3-beta
Severity: Keywords: CBV, class-based views, method_decorator, login_required, decorators, documentation, docs, mixin, login
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

http://docs.djangoproject.com/en/dev//topics/class-based-views/#decorating-the-class

The code described in the documentation does not work. My attempt to run the code is registered here: http://dpaste.com/hold/423359/

Per Łukasz Rekucki, on django-users, it should be changed to:

class ProtectedView(TemplateView):
   template_name = 'secret.html'

   @method_decorator(login_required)
   def dispatch(self, *args, **kwargs):
       return super(ProtectedView, self).dispatch(*args, **kwargs)

I also recommend mentioning alternatives to providing the same functionality, such as the LoginMixin that was suggested by on the same django-users thread:

import settings

class LoginMixin(object):
    def get_test_func(self):
        return getattr(self, 'test_func', lambda u: u.is_authenticated())
    def get_login_url(self):
        return getattr(self, 'login_url', settings.LOGIN_URL)
    def get_redirect_field_name(self):
        return getattr(self, 'redirect_field_name', None)
    def dispatch(self, request, *args, **kwargs):
        from django.contrib.auth.decorators import user_passes_test
    
        return user_passes_test(
            self.get_test_func(),
            login_url = self.get_login_url(),
            redirect_field_name = self.get_redirect_field_name()
        )(super(LoginMixin, self).dispatch
        )(request, *args, **kwargs)

Finally, thank you for providing us with this functionality and documentation. It really makes the code for my views leaner, cleaner, meaner.



Best regards,

André Terra

Attachments (0)

Change History (3)

comment:1 Changed 3 years ago by gabrielhurley

  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to gabrielhurley
  • Patch needs improvement unset
  • Status changed from new to assigned
  • Triage Stage changed from Unreviewed to Accepted

Yep, taking *args as a parameter is gonna be necessary with the way method_decorator works...

As for LoginMixin, I think it should be treated as a separate issue and ought to have it's own ticket. I have no strong feelings on it, personally. It's a useful recipe, but it strikes me that if it's useful enough to be formally written out in the docs it might as well be included as a mixin OTB. Either way I'm going to treat it as an extraneous issue and not part of the resolution for the actual error in the docs.

comment:2 Changed 3 years ago by gabrielhurley

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

In [15563]:

Fixed #15328 -- Corrected an example in the CBV docs and added a note about the parameters passed by method_deorator to the method on the class. Thanks to airstrike for the report and lrekucki for the correction.

comment:3 Changed 3 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

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.