Opened 7 years ago

Closed 4 years ago

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

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 PJCrosier 7 years ago.
Added request to redirect_to_login
redirect_to_login_doc.diff (523 bytes) - added by steve_cassidy51 7 years ago.
modify documentation to warn that this is not a view

Download all attachments as: .zip

Change History (13)

comment:1 Changed 7 years ago by PJCrosier

  • Has patch set
  • Keywords redirect_to_login added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Design decision needed

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

Changed 7 years ago by PJCrosier

Added request to redirect_to_login

comment:2 Changed 7 years ago by guettli

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 7 years ago by guettli

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 7 years ago by ramiro

  • Description modified (diff)

Changed 7 years ago by steve_cassidy51

modify documentation to warn that this is not a view

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

  • Component changed from Contrib apps to contrib.auth

comment:7 Changed 4 years ago by julien

  • Type set to Cleanup/optimization

comment:8 Changed 4 years ago by SmileyChris

  • Easy pickings unset
  • Needs documentation set
  • Patch needs improvement set
  • Severity set to Normal
  • Triage Stage changed from Design decision needed to Accepted

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 4 years ago by jezdez

  • Triage Stage changed from Accepted to Ready 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 4 years ago by jezdez

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

In [16130]:

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

comment:11 Changed 4 years ago by jezdez

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