[Django] #27063: Regex url routing + Localization (prefix_default_language)
Django
--------------------------------+--------------------
Reporter: keithhackbarth | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.10
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------+--------------------
I'm having two issues on using a regex based url pattern with
"prefix_default_language" that I was hoping to shed some insight on.
My url.py file:
{{{
urlpatterns += i18n_patterns(
url(r'^(?P<city>.+-parking)/?$', CityView.as_view()),
prefix_default_language=False
)
}}}
Issue 1: Regex that also match language prefixs
If a city that matches my regex but begins with a language prefix, it
shows a 404.
For example: "/de-moines-parking" shows a 404. But "de/de-moines-parking"
returns a 200. Also, cities that don't match one of my enabled languages
match as well "/boise-parking" for example.
Issue 2: Stacking
Also the urls will stack languages, for example, "/de/fr/it/de-moines-
parking" is loading a successful page instead of a 404.
Any insight? Is this a bug or am I missing something obvious in my regex?
Prior to Django 1.10, I used solid_i18n_patterns plugin and did not have
these issues. I posted on #django-users but no one responded.
Thanks in advance,
Keith
--
Ticket URL: <https://code.djangoproject.com/ticket/27063>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: keithhackbarth | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.10
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
May I ask you to create a new ticket for the second issue? It's better
practice to have one issue per ticket.
--
Ticket URL: <https://code.djangoproject.com/ticket/27063#comment:1>
Django
Reporter: keithhackbarth | Owner: nobody
Type: Bug | Status: new
Component: | Version: 1.10
Internationalization |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* cc: urbaniak (added)
* type: Uncategorized => Bug
* component: Uncategorized => Internationalization
Comment:
Krzysztof, could you triage this ticket since you worked on the
`prefix_default_language` feature (#25933)?
--
Ticket URL: <https://code.djangoproject.com/ticket/27063#comment:2>
Django
Reporter: keithhackbarth | Owner: nobody
Type: Bug | Status: new
Component: | Version: 1.10
Internationalization |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by urbaniak):
The language prefix is separated from the rest of the url with `/` instead
of `-`.
So the url for english (with default language `en`) will be `/moines-
parking/` and the same page for german will have url `/de/moines-
parking/`.
You can also translate the `-parking` part using _/gettext.
About the `/de/fr/it/de-moines-parking` issue - it looks like a german
page for city named `fr/it/de-moines`, can you replace the `.*` in city
match with something like `\w` or `[\w-]` to not catch the `/` as part of
the city name? Then I think it should return 404.
--
Ticket URL: <https://code.djangoproject.com/ticket/27063#comment:3>
Django
Reporter: keithhackbarth | Owner: nobody
Type: Bug | Status: new
Component: | Version: 1.10
Internationalization |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by keithhackbarth):
@urbaniak - Not sure I understand the feedback.
Using the below
{{{
urlpatterns += i18n_patterns(
url(r'^(?P<city>[\w-]+-parking)', CityView.as_view()),
prefix_default_language=False
)
}}}
Still reproduces the same two issues. I understand that in theory, the
language prefix is separated from the rest of the url with / instead of -.
But in practice, with a regex, this does not appear to be the case?
Thanks for your help!
--
Ticket URL: <https://code.djangoproject.com/ticket/27063#comment:4>
Django
Reporter: keithhackbarth | Owner: nobody
Type: Bug | Status: new
Component: | Version: 1.10
Internationalization |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by urbaniak):
Language prefix always ends with slash -
https://github.com/django/django/blob/bc1e2d8e8edde6cc7d2657c68242a13ee65a15b8/django/urls/resolvers.py#L419.
It's added before your url regex.
I cannot reproduce the issue with /de/fr/whatever, I've made a testcase
here https://github.com/urbaniak/i18n_patterns_test - can you make a for
of it and try to reproduce this issue?
There's a tests.py file which assumes 404 on those "nested" urls.
response = self.client.get('/de/pl/test-parking/')
self.assertEqual(response.status_code, 404)
self.assertEqual(response.wsgi_request.LANGUAGE_CODE, 'de')
response = self.client.get('/pl/de/test-parking/')
self.assertEqual(response.status_code, 404)
self.assertEqual(response.wsgi_request.LANGUAGE_CODE, 'pl')
--
Ticket URL: <https://code.djangoproject.com/ticket/27063#comment:5>
Django
Reporter: keithhackbarth | Owner: nobody
Type: Bug | Status: new
Component: | Version: 1.10
Internationalization |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by keithhackbarth):
@urbaniak - The issue I'm having is the one described in the ticket
Failing test case:
response = self.client.get('/de-moines-parking/')
self.assertEqual(response.status_code, 200)
self.assertEqual(response.wsgi_request.LANGUAGE_CODE, 'en')
I'm sure I'm missing something obvious. But not working for me.
--
Ticket URL: <https://code.djangoproject.com/ticket/27063#comment:6>
Django
Reporter: keithhackbarth | Owner: nobody
Type: Bug | Status: new
Component: | Version: 1.10
Internationalization |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by urbaniak):
Do you want here the german page for 'moines' city? Or english page for
'de-moines' city?
--
Ticket URL: <https://code.djangoproject.com/ticket/27063#comment:7>
Django
Reporter: keithhackbarth | Owner: nobody
Type: Bug | Status: new
Component: | Version: 1.10
Internationalization |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by keithhackbarth):
English page
--
Ticket URL: <https://code.djangoproject.com/ticket/27063#comment:8>
Django
Reporter: keithhackbarth | Owner: nobody
Type: Bug | Status: new
Component: Internationalization | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Unreviewed => Accepted
Comment:
The
[https://github.com/django/django/blob/38cf9ef390eb0cd0703d99893f826153865f6ba6/django/utils/translation/trans_real.py#L48
regular expression] to extract the language code from the URLconf
interprets "de-moines-parking" as a language. Maybe it makes sense to
validate that against the actual list of languages that the project uses?
--
Ticket URL: <https://code.djangoproject.com/ticket/27063#comment:9>
Django
Reporter: keithhackbarth | Owner: nobody
Type: Bug | Status: new
Component: Internationalization | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by urbaniak):
We can validate agains the project languages or make sure that language
prefix ends with /.
I'll try to submit a pull request with fix this weekend, will try language
validation approach first.
--
Ticket URL: <https://code.djangoproject.com/ticket/27063#comment:10>
Django
Reporter: keithhackbarth | Owner: nobody
Type: Bug | Status: new
Component: Internationalization | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by urbaniak):
I've submited pull request - https://github.com/django/django/pull/7162
Waiting for comments.
@keithhackbarth, can you test django version from this pull request with
your project?
From my tests it's fixing the issue, but want to be sure.
--
Ticket URL: <https://code.djangoproject.com/ticket/27063#comment:11>
Django
Reporter: keithhackbarth | Owner: nobody
Type: Bug | Status: new
Component: Internationalization | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by keithhackbarth):
Thanks @urbaniak. I'll test and get comments sometime this week.
--
Ticket URL: <https://code.djangoproject.com/ticket/27063#comment:12>
Django
Reporter: keithhackbarth | Owner: nobody
Type: Bug | Status: new
Component: Internationalization | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by jayk140):
@urbaniak - As discussed, I added a test for the case in which the
language code is prefixed with a `-` (i.e de-test-parking). With your
updated code and new language_code_prefix_re, the test passes now as
expected results of 200 with language code of 'en'. The case of stacking
language prefixes now returns the expected 404 as well.
Thanks for the help!
--
Ticket URL: <https://code.djangoproject.com/ticket/27063#comment:13>
Django
Reporter: keithhackbarth | Owner: nobody
Type: Bug | Status: new
Component: Internationalization | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/27063#comment:14>
Django
-------------------------------------+-------------------------------------
Reporter: Keith Hackbarth | Owner: nobody
Type: Bug | Status: new
Component: | Version: 1.10
Internationalization |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 1 => 0
* has_patch: 0 => 1
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/27063#comment:15>
Django
-------------------------------------+-------------------------------------
Reporter: Keith Hackbarth | Owner: nobody
Component: | Version: 1.10
Internationalization |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"a01d887a3ad9029ed198b6af974534fec46223f9" a01d887a]:
{{{
#!CommitTicketReference repository=""
revision="a01d887a3ad9029ed198b6af974534fec46223f9"
Fixed #27063 -- Prevented i18n_patterns() from using too much of the URL
as the language.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27063#comment:16>
Django
-------------------------------------+-------------------------------------
Reporter: Keith Hackbarth | Owner: nobody
Type: Bug | Status: closed
Component: | Version: 1.10
Internationalization |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"a7dabe421684e6cb2f2abce659d60fa3bb1bb71b" a7dabe42]:
{{{
#!CommitTicketReference repository=""
revision="a7dabe421684e6cb2f2abce659d60fa3bb1bb71b"
[1.10.x] Fixed #27063 -- Prevented i18n_patterns() from using too much of
the URL as the language.
Backport of a01d887a3ad9029ed198b6af974534fec46223f9 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27063#comment:17>