[Django] #19910: Double redirect with APPEND_SLASH and i18n_patterns

[Django]

#19910: Double redirect with APPEND_SLASH and i18n_patterns
-------------------------------------+-------------------------------------
Reporter: ssteinberger@… | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: 1.4
Component: Core (URLs) | Keywords: redirect, i18n,
Severity: Normal | append_slash
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Just a thought:
Using settings.APPEND_SLASH = True in combination with i18n_patterns may
lead to unnecessary redirects, like so:

Opening URL: "/some_i18n_url"
Redirect to: "/en/some_i18n_url"
Redirect to: "/en/some_i18n_url/"

It would be more efficient to combine those redirects into.

--
Ticket URL: <https://code.djangoproject.com/ticket/19910>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#19910: Double redirect with APPEND_SLASH and i18n_patterns
-------------------------------------+-------------------------------------
Reporter: ssteinberger@… | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: 1.4

Component: Core (URLs) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: redirect, i18n, | Needs documentation: 0
append_slash | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by claudep):

* needs_better_patch: => 0
* stage: Unreviewed => Accepted
* needs_tests: => 0
* needs_docs: => 0

Comment:

This might require adding a new parameter (force_append_slash) to
`request.get_full_path` as the slash might have to be inserted between the
path and a possible query string. Then `CommonMiddleware` could also
benefit from it.

--
Ticket URL: <https://code.djangoproject.com/ticket/19910#comment:1>

[Django]

#19910: Double redirect with APPEND_SLASH and i18n_patterns
-------------------------------------+-------------------------------------
Reporter: ssteinberger@… | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: 1.4

Component: Core (URLs) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: redirect, i18n, | Needs documentation: 0
append_slash | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Comment (by aaugustin):

It might be interesting to include `RedirectMiddleware` in the scope of
this ticket as it also performs redirects that can interact with
APPEND_SLASH.

--
Ticket URL: <https://code.djangoproject.com/ticket/19910#comment:2>

[Django]

#19910: Double redirect with APPEND_SLASH and i18n_patterns
-------------------------------------+-------------------------------------

Reporter: ssteinberger@… | Owner: bpeschier
Type: | Status: assigned
Cleanup/optimization |
Component: Core (URLs) | Version: 1.4
Severity: Normal | Resolution:
Keywords: redirect, i18n, | Triage Stage: Accepted
append_slash |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by bpeschier):

* owner: nobody => bpeschier
* status: new => assigned
* has_patch: 0 => 1

Comment:

Created a PR for the suggestions above, should we also document the
parameter as part of the public API?

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

[Django]

#19910: Double redirect with APPEND_SLASH and i18n_patterns
-------------------------------------+-------------------------------------

Reporter: ssteinberger@… | Owner: bpeschier
Type: | Status: assigned
Cleanup/optimization |
Component: Core (URLs) | Version: 1.4
Severity: Normal | Resolution:

Keywords: redirect, i18n, | Triage Stage: Ready for
append_slash | checkin

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by timgraham):

* stage: Accepted => Ready for checkin

Comment:

Looks good, pending some cosmetic edits. I don't feel strongly that the
parameter should be documented, but happy to have it if someone sees a use
case for it in user code.

--
Ticket URL: <https://code.djangoproject.com/ticket/19910#comment:4>

[Django]

#19910: Double redirect with APPEND_SLASH and i18n_patterns
-------------------------------------+-------------------------------------

Reporter: ssteinberger@… | Owner: bpeschier
Type: | Status: closed

Cleanup/optimization |
Component: Core (URLs) | Version: 1.4

Severity: Normal | Resolution: fixed

Keywords: redirect, i18n, | Triage Stage: Ready for
append_slash | checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Tim Graham <timograham@…>):

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

Comment:

In [changeset:"9128762f1608f9633005f16c95270058a50ada2b" 9128762]:
{{{
#!CommitTicketReference repository=""
revision="9128762f1608f9633005f16c95270058a50ada2b"
Fixed #19910 -- Added slash to i18n redirect if APPEND_SLASH is set.

This introduces a force_append_slash argument for request.get_full_path()
which is used by RedirectFallbackMiddleware and CommonMiddleware when
handling redirects for settings.APPEND_SLASH.
}}}

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