Opened 5 years ago

Closed 5 years ago

Last modified 4 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

Change History (3)

comment:1 Changed 5 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 5 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 4 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

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