Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Applying format_suffix_patterns breaks Django's i18n_patterns #2278

Closed
thedrow opened this issue Dec 15, 2014 · 10 comments
Closed

Applying format_suffix_patterns breaks Django's i18n_patterns #2278

thedrow opened this issue Dec 15, 2014 · 10 comments

Comments

@thedrow
Copy link
Contributor

thedrow commented Dec 15, 2014

Using i18n_patterns as the following:

urlpatterns = i18n_patterns('',
    url('news', 'myviews.news', name='news')
)
urlpatterns = format_suffix_patterns(urlpatterns)

Breaks the LocaleMiddleware's assumption that localized urls are resolved using the LocaleRegexURLResolver class resulting in:

>>> from django.core.urlresolvers import reverse
>>> from django.utils.translation import activate
>>> reverse('news')
'/en-us/news/'
>>> activate('ja')
>>> reverse('news')
'/en-us/news/' # The returned value should be /ja/news/

Removing the call to format_suffix_patterns causes the reverse method to resolve the URLs correctly.

urlpatterns = i18n_patterns('',
    url('news', 'myviews.news', name='news')
)
>>> from django.core.urlresolvers import reverse
>>> from django.utils.translation import activate
>>> reverse('news')
'/en-us/news/'
>>> activate('ja')
>>> reverse('news')
'/ja/news/'

Could this be fixed and backported to 2.4?

@tomchristie
Copy link
Member

Assume this is valid against 3.0?

Could this be fixed and backported to 2.4?

Regarding backporting to 2.4, see this thread: https://groups.google.com/forum/#!topic/django-rest-framework/NHcDGGPaqNw

@thedrow
Copy link
Contributor Author

thedrow commented Dec 15, 2014

I have not tried 3.0.

@thedrow
Copy link
Contributor Author

thedrow commented Dec 15, 2014

I can reproduce this problem with 3.x as well.

@tomchristie
Copy link
Member

Breaks the LocaleMiddleware assumption that localized urls are resolved using the LocaleRegexURLResolver class

Could you give any more detail on this?

@thedrow
Copy link
Contributor Author

thedrow commented Dec 15, 2014

When initializing the LocaleMiddleware as part of the request cycle it marks the request's url as either localized or not.
See the Django source:

class LocaleMiddleware(object):
    """
    This is a very simple middleware that parses a request
    and decides what translation object to install in the current
    thread context. This allows pages to be dynamically
    translated to the language the user desires (if the language
    is available, of course).
    """

    def __init__(self):
        self._supported_languages = SortedDict(settings.LANGUAGES)
        self._is_language_prefix_patterns_used = False
        for url_pattern in get_resolver(None).url_patterns:
            if isinstance(url_pattern, LocaleRegexURLResolver):
                self._is_language_prefix_patterns_used = True
                break
    # ...

When applying format_suffix_patterns and setting a breakpoint inside the if statement you don't hit the breakpoint and self._is_language_prefix_patterns_used is set to False so no localization is applied to the URL. Without format_suffix_patterns everything works as expected.

Regardless of LocaleMiddleware it seems that format_suffix_patterns causes the URL resolver to change in a way that prevents it from re-evaluating it's locale prefix which means that it won't reverse correctly.

@tomchristie tomchristie added this to the 3.0.2 Release milestone Dec 15, 2014
@tomchristie
Copy link
Member

Okay. I guess the resolution for this will be something like wrapping the return result in LocaleRegexURLResolver somewhere here https://github.com/tomchristie/django-rest-framework/blob/master/rest_framework/urlpatterns.py#L20 tho not obvious if it is as simple as that or not.

If you or anyone else fancies taking on a failing test pull request for this that'd be most welcome.

@thedrow
Copy link
Contributor Author

thedrow commented Dec 15, 2014

I don't think I have the time for it this week. Maybe on friday but no guarantees.

@tomchristie
Copy link
Member

Presumably applying them the other way around will work as expected?

@tomchristie
Copy link
Member

Assuming that's the case then I think we can consider this a documentation issue.

@thedrow
Copy link
Contributor Author

thedrow commented Dec 28, 2014

Have you verified that the code example works?

I just ensured format_suffix_patterns does not apply itself on i18n_pattern's urls.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants