[Django] #35670: Unclear docs for LoginRequiredMiddleware.get_login

Django

unread,

Aug 12, 2024, 12:04:01 PM8/12/24

to django-...@googlegroups.com

#35670: Unclear docs for LoginRequiredMiddleware.get_login_url()
-------------------------------------+-------------------------------------
Reporter: Claude Paroz | Type:
| Cleanup/optimization
Status: new | Component:
| Documentation
Version: 5.1 | Severity: Normal
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
I'm struggling to understand the second sentence of the documentation of
LoginRequiredMiddleware.get_login_url()
(https://docs.djangoproject.com/en/5.1/ref/middleware/#django.contrib.auth.middleware.get_login_url).

`If defined, this returns the login_url set on the login_required()
decorator. Defaults to settings.LOGIN_URL.`

After many reads, I think I get the point of the `If defined` that means
if the `login_required() defines login_url, then... I'm sure we can do
better. Same issue with the docs for `get_redirect_field_name()` below.
--
Ticket URL: <https://code.djangoproject.com/ticket/35670>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,

Aug 12, 2024, 12:06:22 PM8/12/24

to django-...@googlegroups.com

#35670: Unclear docs for LoginRequiredMiddleware.get_login_url()
-------------------------------------+-------------------------------------

Comment (by Claude Paroz):

Maybe something like: "By default, it returns either a `login_url`
attribute set on the view by a `login_required` decorator, or
`settings.LOGIN_URL`."
--
Ticket URL: <https://code.djangoproject.com/ticket/35670#comment:1>

Django

unread,

Aug 12, 2024, 12:57:06 PM8/12/24

to django-...@googlegroups.com

#35670: Unclear docs for LoginRequiredMiddleware.get_login_url()

--------------------------------------+------------------------------------

Reporter: Claude Paroz | Owner: (none)

Type: Cleanup/optimization | Status: new

Component: Documentation | Version: 5.1
Severity: Normal | Resolution:

Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0

--------------------------------------+------------------------------------
Changes (by Sarah Boyce):

* stage: Unreviewed => Accepted

Comment:

Agreed this could be worded better.
{{{
If ``login_url`` is set on the
:func:`~.django.contrib.auth.decorators.login_required`
decorator, this is returned. Otherwise, returns
:setting:`settings.LOGIN_URL <LOGIN_URL>`.
}}}
(as another option)
--
Ticket URL: <https://code.djangoproject.com/ticket/35670#comment:2>

Django

unread,

Aug 13, 2024, 11:34:30 PM8/13/24

to django-...@googlegroups.com

#35670: Unclear docs for LoginRequiredMiddleware.get_login_url()

-------------------------------------+-------------------------------------
Reporter: Claude Paroz | Owner: Samruddhi
Type: | Dharankar
Cleanup/optimization | Status: assigned

-------------------------------------+-------------------------------------
Changes (by Samruddhi Dharankar):

* owner: (none) => Samruddhi Dharankar
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/35670#comment:3>

Django

unread,

Sep 20, 2024, 4:47:42 PM9/20/24

to django-...@googlegroups.com

#35670: Unclear docs for LoginRequiredMiddleware.get_login_url()
-------------------------------------+-------------------------------------
Reporter: Claude Paroz | Owner: Samruddhi
Type: | Dharankar
Cleanup/optimization | Status: assigned
Component: Documentation | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Aditya Chaudhary):

@Samruddhi Dharankar, are you still working in this issue ?
Or shall I assign this to myself ?
--
Ticket URL: <https://code.djangoproject.com/ticket/35670#comment:4>

Django

unread,

Sep 21, 2024, 5:16:05 PM9/21/24

to django-...@googlegroups.com

#35670: Unclear docs for LoginRequiredMiddleware.get_login_url()
-------------------------------------+-------------------------------------

Reporter: Claude Paroz | Owner: Aditya
Type: | Chaudhary

Changes (by Aditya Chaudhary):

* owner: Samruddhi Dharankar => Aditya Chaudhary

--
Ticket URL: <https://code.djangoproject.com/ticket/35670#comment:5>

Django

unread,

Sep 22, 2024, 8:27:06 AM9/22/24

to django-...@googlegroups.com

#35670: Unclear docs for LoginRequiredMiddleware.get_login_url()
-------------------------------------+-------------------------------------
Reporter: Claude Paroz | Owner: Aditya
Type: | Chaudhary
Cleanup/optimization | Status: assigned
Component: Documentation | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls):

* has_patch: 0 => 1

Comment:

[https://github.com/django/django/pull/18610 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/35670#comment:6>

Django

unread,

Oct 2, 2024, 8:54:01 AM10/2/24

to django-...@googlegroups.com

#35670: Unclear docs for LoginRequiredMiddleware.get_login_url()
-------------------------------------+-------------------------------------
Reporter: Claude Paroz | Owner: Aditya
Type: | Chaudhary
Cleanup/optimization | Status: assigned
Component: Documentation | Version: 5.1

Severity: Release blocker | Resolution:

Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Natalia Bidart):

* severity: Normal => Release blocker

Old description:

> I'm struggling to understand the second sentence of the documentation of
> LoginRequiredMiddleware.get_login_url()
> (https://docs.djangoproject.com/en/5.1/ref/middleware/#django.contrib.auth.middleware.get_login_url).
>
> `If defined, this returns the login_url set on the login_required()
> decorator. Defaults to settings.LOGIN_URL.`
>
> After many reads, I think I get the point of the `If defined` that means
> if the `login_required() defines login_url, then... I'm sure we can do
> better. Same issue with the docs for `get_redirect_field_name()` below.

New description:

I'm struggling to understand the second sentence of the documentation of
LoginRequiredMiddleware.get_login_url()
(https://docs.djangoproject.com/en/5.1/ref/middleware/#django.contrib.auth.middleware.get_login_url).

`If defined, this returns the login_url set on the login_required()
decorator. Defaults to settings.LOGIN_URL.`

After many reads, I think I get the point of the `If defined` that means

if the `login_required() defines login_url, then`...

I'm sure we can do better. Same issue with the docs for
`get_redirect_field_name()` below.

--

Comment:

While this is purely a docs change, it strictly qualifies as a release
blocker since it was introduced in
c7fc9f20b49b5889a9a8f47de45165ac443c1a21.
--
Ticket URL: <https://code.djangoproject.com/ticket/35670#comment:7>

Django

unread,

Oct 2, 2024, 10:02:29 AM10/2/24

to django-...@googlegroups.com

#35670: Unclear docs for LoginRequiredMiddleware.get_login_url()
-------------------------------------+-------------------------------------
Reporter: Claude Paroz | Owner: Aditya
Type: | Chaudhary
Cleanup/optimization | Status: assigned
Component: Documentation | Version: 5.1
Severity: Release blocker | Resolution:

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Natalia Bidart):

* stage: Accepted => Ready for checkin

Comment:

Will merge when CI is green, I also tweaked `get_redirect_field_name()`
since it had the same English construct, making it challenging to
understand.
--
Ticket URL: <https://code.djangoproject.com/ticket/35670#comment:8>

Django

unread,

Oct 2, 2024, 12:15:31 PM10/2/24

to django-...@googlegroups.com

#35670: Unclear docs for LoginRequiredMiddleware.get_login_url()
-------------------------------------+-------------------------------------
Reporter: Claude Paroz | Owner: Aditya
Type: | Chaudhary

Cleanup/optimization | Status: closed
Component: Documentation | Version: 5.1
Severity: Release blocker | Resolution: fixed

Changes (by GitHub <noreply@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"efc3b0c627f7e3cb4e337280ecd2483758dcb0a5" efc3b0c6]:
{{{#!CommitTicketReference repository=""
revision="efc3b0c627f7e3cb4e337280ecd2483758dcb0a5"
Fixed #35670 -- Clarified the return value for LoginRequiredMiddleware's
methods.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35670#comment:9>

Django

unread,

Oct 2, 2024, 12:16:21 PM10/2/24

to django-...@googlegroups.com

#35670: Unclear docs for LoginRequiredMiddleware.get_login_url()
-------------------------------------+-------------------------------------
Reporter: Claude Paroz | Owner: Aditya
Type: | Chaudhary
Cleanup/optimization | Status: closed
Component: Documentation | Version: 5.1
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Natalia <124304+nessita@…>):

In [changeset:"bf64ac3567c892d60d2b157e866c509a37b14918" bf64ac3]:
{{{#!CommitTicketReference repository=""
revision="bf64ac3567c892d60d2b157e866c509a37b14918"
[5.1.x] Fixed #35670 -- Clarified the return value for
LoginRequiredMiddleware's methods.

Backport of efc3b0c627f7e3cb4e337280ecd2483758dcb0a5 from main.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35670#comment:10>

Reply all

Reply to author

Forward

[Django] #35670: Unclear docs for LoginRequiredMiddleware.get_login_url()

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django