django-cms
django-cms copied to clipboard
Language fallback without redirect doesn't work on homepage
Summary
For some reason if you configure CMS_LANGUAGES
the following way:
'default': {
'fallbacks': ['en', 'fr'],
'redirect_on_fallback': False,
'public': True,
'hide_untranslated': False
},
Where you'd expect a fallback without redirect for every language, you'll still get a redirect fallback for the homepage because of the following line https://github.com/divio/django-cms/blob/097fe820a05c9b9efd63c36d7f009bf77dff60bb/cms/views.py#L110. I don't see any reason why the homepage should behave differently from other pages. I think the or slug == ""
condition should be removed, or, if there's a good reason for doing so, there should at least be a way to disable it.
Expected behaviour
All pages honour the redirect_on_fallback
setting, even the homepage.
Actual behaviour
The homepage always redirects on fallback, no matter what the value of redirect_on_fallback
is.
Environment
- Python version: 3.4.2
- Django version: 1.9.12
- django CMS version: 3.4.1
Thanks @sephii, Traced back to https://github.com/divio/django-cms/pull/2736
Can I do a pull request for this or is there still a decision to be made?
Personally I prefer the system to stay consistent, I wonder whether somebody could be relying on this behavior now though?
But I definitely like to idea of making that condition configurable & documenting it properly!
hey, I would like to work on this issue. I'd appreciate some guidance as I'm new to open source.
@zain93393 Hi there! Great news. There's already some information on the issue and how to fix it. Since it is quite old, you might want to check if the corresponding line
https://github.com/django-cms/django-cms/blob/097fe820a05c9b9efd63c36d7f009bf77dff60bb/cms/views.py#L110
has changed since then.
To create a pull request (PR), you best fork the django-cms
repo create a new branch fix/issue-5839
and make the changes. Once you have committed the changes to your new branch, you can create a PR from the github page of your clone.
Join the Slack channel #workgroup-pr-review to look for someone to review your PR.
It is good practice to also add a test that verifies the new behavior. But that might be a second step.
Please let me know if you have any questions!
I can take a look at this and pair it with fellow djangonauts to work this out.
@fsbraun I can't seem to add a label to this, can you please update the label to djangonaut for this issue. So it gets added to the djangonaut project board. 😊
Hello everyone! On line 177, a check renders a 404 if the page for which the request language doesn't exist and there are no fallbacks. https://github.com/django-cms/django-cms/blob/ed7e2064e023c29b169084dd53a3b8643ac66715/cms/views.py#L170-L186
Interestingly, there are no calls to _handle_no_page()
after that. So, if redirect_on_fallback
is False, homepage or not, we'll get a 200 rendering the main language's page with no content.
This issue traces back to #2736, and it was mentioned in the linked issue that the alternative to redirection is 404.
So, a solution is to add redirect_on_fallback
to the condition on 177 (page.is_home
on the following condition becomes redundant so it gets removed)
if (language_is_unavailable and
(not fallback_languages or not redirect_on_fallback)):
# There is no page with the requested language
# and either there are no configured fallbacks or
# the user has explicitly requested not to
# redirect on fallbacks.
return _handle_no_page(request)
elif language_is_unavailable and redirect_on_fallback:
[...]
This will have the side-effect of rendering a 404 for any page if redirect_on_fallback
is False (when the request language is unavailable of course).
Before I make the PR, I just want to ask if this is the way to go or not.
I think you are going the right way. This is something I would expect. Does this also solve this issue?
Yes! I just needed confirmation. PR is on the way. I was just worried that the default behavior for a False redirect_on_fallback
wasn't 404.
Ah, I understand. But I fear according to the docs the situation should be:
- No public fallback language: 404 (independent of
redirect_on_fallback
- If public fallback:
-
redirect_on_fallback = True
: Redirect to the fallback language (at its URL which might, e.g., be translated) -
redirect_on_fallback = False
: needs to display the fallback language under the original URL (i.e. display, no 404, and no redirect.)
-
That is the logic for any page, but not the home page apparently.
Thanks for clearing that up! I'll make the appropriate changes.