Opened 9 years ago

Closed 5 years ago

Last modified 5 years ago

#6581 closed Cleanup/optimization (fixed)

django.contrib.auth.views.redirect_to_login isn't a view

Reported by: steve_cassidy51 Owned by: nobody
Component: contrib.auth Version: master
Severity: Normal Keywords: auth redirect_to_login
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX:

Description (last modified by Ramiro Morales)

The documentation lists redirect_to_login as a view alongside password_change etc but the definition doesn't take a 'request' first argument and so doesn't work properly as a view. Eg. if I use it in urls.py:

 (r'^$', 'django.contrib.auth.views.redirect_to_login', {'next': '/something'})...

then I get an error that 'next' is provided twice since the first arg supplied to the view
is the request object. If I leave out 'next' then I get a huge long URL in the redirect that contains the entire request serialised.

So, either this should be made into a view with the addition of the request parameter or it should be removed from the list of views in the documentation.

Attachments (2)

6581.django.contrib.auth.views.diff (623 bytes) - added by Pete Crosier 9 years ago.
Added request to redirect_to_login
redirect_to_login_doc.diff (523 bytes) - added by steve_cassidy51 8 years ago.
modify documentation to warn that this is not a view

Download all attachments as: .zip

Change History (13)

comment:1 Changed 9 years ago by Pete Crosier

Has patch: set
Keywords: redirect_to_login added
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedDesign decision needed

Simplest case patch attached, though I guess changing this has the potential to break things for a lot of people?

Changed 9 years ago by Pete Crosier

Added request to redirect_to_login

comment:2 Changed 9 years ago by Thomas Güttler

Yes, that's true. It is not a view.

As far as I understand it, it redirects you to login, even if you are already logged in.

Why is redirect_to_login needed at all?

related: #6306

comment:3 Changed 9 years ago by Thomas Güttler

I thought about this again. I guess redirect_to_login is older than reverse(). I think it should be removed and all calls to this
method should be replaced with calls to reverse():

tilldu@r51:~/_django/django$ find -name '*.py' | xargs grep 'redirect_to_login'
./views/generic/create_update.py:from django.contrib.auth.views import redirect_to_login
./views/generic/create_update.py:        return redirect_to_login(request.path)
./views/generic/create_update.py:        return redirect_to_login(request.path)
./views/generic/create_update.py:        return redirect_to_login(request.path)
./contrib/auth/views.py:def redirect_to_login(next, login_url=None, redirect_field_name=REDIRECT_FIELD_NAME):
./contrib/flatpages/views.py:        from django.contrib.auth.views import redirect_to_login
./contrib/flatpages/views.py:        return redirect_to_login(request.path)

Since create_update needs to be rewritten for newforms, only one call is left: views.py of flatpages.

comment:4 Changed 8 years ago by Ramiro Morales

Description: modified (diff)

Changed 8 years ago by steve_cassidy51

Attachment: redirect_to_login_doc.diff added

modify documentation to warn that this is not a view

comment:5 Changed 8 years ago by steve_cassidy51

Added a patch to change the documentation pointing out
that this is not a view - this might be a less troubling
change to make at the moment.

It does seem like a useful shortcut function, it just
doesn't really belong in views.py.

comment:6 Changed 6 years ago by Gabriel Hurley

Component: Contrib appscontrib.auth

comment:7 Changed 5 years ago by Julien Phalip

Type: Cleanup/optimization

comment:8 Changed 5 years ago by Chris Beaven

Easy pickings: unset
Needs documentation: set
Patch needs improvement: set
Severity: Normal
Triage Stage: Design decision neededAccepted

Move it to a contrib.auth.util module but import it in views (with a comment explaining that it's imported for backwards compatibility).

comment:9 Changed 5 years ago by Jannis Leidel

Triage Stage: AcceptedReady for checkin

I don't believe introducing yet another util(s) module is needed here. It's really just a documentation issue.

comment:10 Changed 5 years ago by Jannis Leidel

Resolution: fixed
Status: newclosed

In [16130]:

Fixed #6581 -- Moved documentation of django.contrib.auth.views.redirect_to_login to an own "Helper functions" section.

comment:11 Changed 5 years ago by Jannis Leidel

In [16131]:

[1.3.X] Fixed #6581 -- Moved documentation of django.contrib.auth.views.redirect_to_login to an own "Helper functions" section.

Backport form trunk (r16130).

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