Opened 3 months ago

Closed 6 weeks ago

Last modified 6 weeks ago

#35670 closed Cleanup/optimization (fixed)

Unclear docs for LoginRequiredMiddleware.get_login_url()

Reported by: Claude Paroz Owned by: Aditya Chaudhary
Component: Documentation Version: 5.1
Severity: Release blocker Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description (last modified by Natalia Bidart)

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.

Change History (10)

comment:1 by Claude Paroz, 3 months ago

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."

comment:2 by Sarah Boyce, 3 months ago

Triage Stage: UnreviewedAccepted

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)

comment:3 by Samruddhi Dharankar, 3 months ago

Owner: set to Samruddhi Dharankar
Status: newassigned

comment:4 by Aditya Chaudhary, 8 weeks ago

@Samruddhi Dharankar, are you still working in this issue ?
Or shall I assign this to myself ?

comment:5 by Aditya Chaudhary, 8 weeks ago

Owner: changed from Samruddhi Dharankar to Aditya Chaudhary

comment:6 by Jacob Walls, 8 weeks ago

Has patch: set

comment:7 by Natalia Bidart, 6 weeks ago

Description: modified (diff)
Severity: NormalRelease blocker

While this is purely a docs change, it strictly qualifies as a release blocker since it was introduced in c7fc9f20b49b5889a9a8f47de45165ac443c1a21.

comment:8 by Natalia Bidart, 6 weeks ago

Triage Stage: AcceptedReady for checkin

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.

comment:9 by GitHub <noreply@…>, 6 weeks ago

Resolution: fixed
Status: assignedclosed

In efc3b0c6:

Fixed #35670 -- Clarified the return value for LoginRequiredMiddleware's methods.

comment:10 by Natalia <124304+nessita@…>, 6 weeks ago

In bf64ac3:

[5.1.x] Fixed #35670 -- Clarified the return value for LoginRequiredMiddleware's methods.

Backport of efc3b0c627f7e3cb4e337280ecd2483758dcb0a5 from main.

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