Language fallback without redirect doesn't work on homepage

[sephii]

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

[czpython]

Thanks @sephii,
Traced back to #2736

[alculquicondor]

Can I do a pull request for this or is there still a decision to be made?

[victor-yunenko]

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!

[zain93393]

hey, I would like to work on this issue. I'd appreciate some guidance as I'm new to open source.

[fsbraun]

@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

django-cms/cms/views.py

Line 110 in 097fe82

if get_redirect_on_fallback(current_language) or slug == "":

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!

[Pradhvan]

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. 😊

[sakhawy]

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.

django-cms/cms/views.py

Lines 170 to 186 in ed7e206

	# Only fallback to languages the user is allowed to see
	fallback_languages = [
	language for language in fallbacks
	if language != request_language and language in available_languages
	]
	language_is_unavailable = request_language not in available_languages
	if language_is_unavailable and not fallback_languages:
	# There is no page with the requested language
	# and there's no configured fallbacks
	return _handle_no_page(request)
	elif language_is_unavailable and (redirect_on_fallback or page.is_home):
	# There is no page with the requested language and
	# the user has explicitly requested to redirect on fallbacks,
	# so redirect to the first configured / available fallback language
	fallback = fallback_languages[0]
	redirect_url = page.get_absolute_url(fallback, fallback=False)

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.

[fsbraun]

I think you are going the right way. This is something I would expect. Does this also solve this issue?

[sakhawy]

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.

[fsbraun]

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.

[sakhawy]

Thanks for clearing that up! I'll make the appropriate changes.