#22006 closed Cleanup/optimization (fixed)

DRY login_required method should be documented

Reported by: django@… Owned by: mockforest
Component: Documentation Version: 1.6
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

See https://code.djangoproject.com/ticket/16626 for the discussion.

Currently, the docs suggest decorating each class separately, they should also suggest the mixin as recommended in the ticket above, for setting login_required (or others) on multiple classes.

Note: Creating a ProtectedView superclass and subclassing doesn't work for the reason as specified in the Note in the current docs:
https://docs.djangoproject.com/en/dev/topics/class-based-views/intro/#decorating-the-class

Attachments (3)

ticket-22006.diff (811 bytes) - added by chrismarceloph 14 months ago.
ticket-22006-fixed.diff (852 bytes) - added by mockforest 14 months ago.
added @classmethod
ticket-22006.2.diff (856 bytes) - added by mockforest 14 months ago.
lines cut to max length of 80

Download all attachments as: .zip

Change History (17)

comment:1 Changed 14 months ago by timo

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Cleanup/optimization

comment:2 Changed 14 months ago by avendael

  • Owner changed from nobody to anonymous
  • Status changed from new to assigned

Changed 14 months ago by chrismarceloph

comment:3 Changed 14 months ago by chrismarceloph@…

  • Has patch set

Submitted patch. (Sorry did not claim it first, first-timer here.)

comment:4 Changed 14 months ago by avendael

  • Owner changed from anonymous to avendael

comment:5 Changed 14 months ago by avendael

  • Owner avendael deleted
  • Status changed from assigned to new

comment:6 Changed 14 months ago by avendael

No worries :)

comment:7 Changed 14 months ago by dzhibas

@chrismarceloph in your patch method as_view needs to be decorated with @classmethod
see https://code.djangoproject.com/ticket/16626 last comment

i would suggest to put that example into https://docs.djangoproject.com/en/dev/topics/class-based-views/intro/#using-mixins and then from https://docs.djangoproject.com/en/dev/topics/auth/default/#applying-permissions-to-generic-views refer to that example which is written in mixins sections instead of decorating dispatch

#nlsprint14

comment:8 Changed 14 months ago by dzhibas

and if you are planning to finish this ticket, please register here and assign it to yourself

comment:9 Changed 14 months ago by mockforest

  • Owner set to mockforest
  • Status changed from new to assigned

@dzhibas: Thanks! I've registered and am planning to finish this ticket.

If I understand it correctly, you're suggesting I move the example to https://docs.djangoproject.com/en/dev/topics/class-based-views/intro/#using-mixins and refer to it from https://docs.djangoproject.com/en/dev/topics/auth/default/#applying-permissions-to-generic-views? Sounds good; I think that section needs an example too.

comment:10 Changed 14 months ago by timo

  • Patch needs improvement set

comment:11 Changed 14 months ago by django@…

I think he just means to say you need to add @classmethod on line 12 of your diff.

I'd probably also clarify the following;

Changed 14 months ago by mockforest

added @classmethod

Changed 14 months ago by mockforest

lines cut to max length of 80

comment:12 Changed 14 months ago by mockforest

I didn't see the first line of @dzhibas's comment, sorry. Patch fixed.

And now I'm not so sure about the suggestions I feel like the example belongs to three separate sections:

What would be the best way to do this?

Version 0, edited 14 months ago by mockforest (next)

comment:13 Changed 14 months ago by mockforest

  • Patch needs improvement unset

comment:14 Changed 14 months ago by timo

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

In 584044566414d3f4595337225aa23b52a089a680:

Fixed #22006 -- Documented how to write a login_required mixin for CBVs.

Thanks django at patjack.co.uk for the suggestion and mockforest
for the draft patch.

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