{% page_language_url %} doesn't work for non cms handled urls #1348

Closed
wants to merge 3 commits into
from

Projects

None yet

4 participants

@rasca
Contributor
rasca commented Jul 16, 2012

{% page_language_url %} and {% language_chooser %} should work for urls not managed by the cms, such as the admin urls.

This is stated in the docs: "If the current URL is not handled by the CMS and no set_language_changer function is provided it will take the exact same URL as the current one and will only change the language prefix.".

This functionally is necessary because when using MultilingualURLMiddleware the setlang view not longer works. Right now it's not possible to change the language inside an arbitrary admin page.

The root problem is in https://github.com/divio/django-cms/blob/develop/menus/templatetags/menu_tags.py#L370 , the unmanaged url doesn't get a Page object so page.get_absolute_url() raises an exception and no url is returned.

How do you think would be the best way to solve this?

@rasca rasca Fixed #1348 corrected page_language_url on non CMS managed urls
The language chooser wasn't translating the urls on non managed CMS
views. simple_language_changer was deprecated since now that's the
default behavior.

Added the base 2.4 release notes.
4cff319
@rasca rasca added a commit to rasca/django-cms that referenced this pull request Jul 19, 2012
@rasca rasca Fixed #1348 corrected page_language_url on non CMS managed urls
The language chooser wasn't translating the urls on non managed CMS
views. simple_language_changer was deprecated since now that's the
default behavior.

Added the base 2.4 release notes.
a388063
@ojii ojii commented on an outdated diff Jul 19, 2012
docs/upgrade/2.4.rst
@@ -0,0 +1,37 @@
+#################
+2.4 release notes
@ojii
ojii Jul 19, 2012 Collaborator

should have "(in development)" added.

@ojii ojii commented on the diff Jul 19, 2012
docs/upgrade/2.4.rst
@@ -0,0 +1,37 @@
+#################
+2.4 release notes
+#################
+
+*****************
+What's new in 2.4
+*****************
+
+..
+ New feature
+ ===========
+
+..
+ Feature description
@ojii
ojii Jul 19, 2012 Collaborator

shouldn't really be here, should it?

@rasca
rasca Jul 19, 2012 Contributor

I've added this as comments so the the structure remains, and the next one adding one can simple uncomment and change the text. Should I remove it completely?

@ojii
ojii Jul 19, 2012 Collaborator

oh, I missed the comment bits, my bad.

@ojii ojii and 1 other commented on an outdated diff Jul 19, 2012
menus/utils.py
@@ -66,7 +68,7 @@ def _wrapped(request, *args, **kwargs):
return _wrapped
return _decorator
-class _SimpleLanguageChanger(object):
+class NonManagedLanguageChanger(object):
@ojii
ojii Jul 19, 2012 Collaborator

while an edge-case, an alias for _SimpleLanguageChanger should be preserved for now, with a warning raised if used if possible.

@rasca
rasca Jul 19, 2012 Contributor

What remains is simple_language_changer() with the deprecation warning there.

@ojii
ojii Jul 19, 2012 Collaborator

fair enough, has an underscore after all.

@ojii ojii commented on an outdated diff Jul 19, 2012
menus/utils.py
@@ -94,9 +96,11 @@ def get_page_path(self, lang):
return ''
def simple_language_changer(func):
+ warnings.warn("simple_language_chooser is deprecated and will be removed in 2.5!", DeprecationWarning)
@ojii
ojii Jul 19, 2012 Collaborator

maybe a little more info that it's now no longer necessary and will be the default. So people don't freak out too much when they read this :D

@travisbot

This pull request passes (merged 2e5ac31 into dc00224).

@rasca
Contributor
rasca commented Jul 24, 2012

Don't merge this PR.. there's still some work to be done.

Issues right now:

  • when in a subpage in a apphook the url produced is in the correct lang but point to the root of the app hook, and not the subpage.
  • when using the page_language_url inside the admin/cms/page/id it redirects to that page in the frontend and not in the admin.
@rasca rasca Fixed page_language_url for subpages in apphooks
There some leakage in the tests, running them in different order changes
the outcome.
605f71e
@rasca
Contributor
rasca commented Jul 25, 2012

So, there's some leakage in the tests by changing the order they get executed:

(django-cms)ivan@mp-ivan:~/Sites/django-cms$ python runtests.py ApphooksTestCase ApphooksPageLanguageUrlTestCase
Creating test database for alias 'default'...
......F
======================================================================
FAIL: test_page_language_url_for_apphook (cms.tests.apphooks.ApphooksPageLanguageUrlTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/ivan/Sites/django-cms/cms/tests/apphooks.py", line 245, in test_page_language_url_for_apphook
    self.assertEqual(url, '/en/child_page/child_child_page/extra_1/')
AssertionError: u'/en/child_page/child_child_page/' != '/en/child_page/child_child_page/extra_1/'

----------------------------------------------------------------------
Ran 7 tests in 2.896s

FAILED (failures=1)
Destroying test database for alias 'default'...
(django-cms)ivan@mp-ivan:~/Sites/django-cms$ python runtests.py ApphooksPageLanguageUrlTestCase ApphooksTestCase
Creating test database for alias 'default'...
.......
----------------------------------------------------------------------
Ran 7 tests in 2.894s

OK
Destroying test database for alias 'default'...

Once this is fixed I'll move on to the last remaining issue of page_language_url: using it inside admin/cms/page/id

@ojii
Collaborator
ojii commented Aug 9, 2012

in current develop the test leakage should be fixed, can you verify this?

@digi604
Member
digi604 commented Aug 16, 2012

Please merge/rebase

@digi604 digi604 commented on the diff Aug 28, 2012
menus/templatetags/menu_tags.py
url = "/%s" % lang + request._language_changer(lang)
else:
- page = request.current_page
- if page == "dummy":
@digi604
digi604 Aug 28, 2012 Member

Is the "dummy" not used anymore?

@digi604 digi604 pushed a commit that closed this pull request Oct 3, 2012
@rasca rasca Fixed #1348 corrected page_language_url on non CMS managed urls
The language chooser wasn't translating the urls on non managed CMS
views. simple_language_changer was deprecated since now that's the
default behavior.

Added the base 2.4 release notes.
4cff319
@digi604 digi604 closed this in 4cff319 Oct 3, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment