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

[RFC] disable langs in Internationalized routing and redirecting #36266

Closed
hectorprats opened this issue Mar 30, 2020 · 10 comments
Closed

[RFC] disable langs in Internationalized routing and redirecting #36266

hectorprats opened this issue Mar 30, 2020 · 10 comments

Comments

@hectorprats
Copy link

hectorprats commented Mar 30, 2020

Description
By creating a website, I have discovered that I need to temporarily hide a language.
But to delete all the routes in the controllers (annotations) and to put them back in the future seems absurd to me.

I am designing a new feature to be able to close a language completely (http code 404) or that you can make a temporary redirection to another language.

It can be very useful for SEO.

For this reason, I beg you to contribute ideas and suggestions to improve it. I would like to finish it as soon as possible.

Example

#config\packages\routing.yaml
framework:
    router:
        utf8: true
        langs:
            disabled:
                pt:
                    http_code: 307
                    lang_destiny: en
                es:
#           disabled: 'es'                (simple method getting 404 code in Spanish pages)

With this yaml, you can redirect 307 from the Portuguese version to the English version and 404 to the Spanish version.

@blowski
Copy link

blowski commented Mar 30, 2020

Sounds good, though I'm not an expert in i18n or SEO so can't comment.

One tiny bit of feedback, though - I think you want lang_destination instead of lang_destiny.

@hectorprats
Copy link
Author

Sounds good, though I'm not an expert in i18n or SEO so can't comment.

One tiny bit of feedback, though - I think you want lang_destination instead of lang_destiny.

Thanks. I changed it.

@rosier
Copy link
Contributor

rosier commented Mar 30, 2020

How about using redirect_to instead of lang_destination ?

And is http_code: 307 the default config if a temporary redirection to another language is enabled?

@hectorprats
Copy link
Author

How about using redirect_to instead of lang_destination ?

And is http_code: 307 the default config if a temporary redirection to another language is enabled?

The default http_code is 404.

@hectorprats
Copy link
Author

Finally works with any of this three methods:

framework:
    router:
        utf8: true
        langs:
#            disabled:
#                es:
#                    http_code: 307
#                    redirect_to: en
#                pt:
#                nl:
#            disabled: ['nl', 'pt']
#            disabled: pt

What else? :-)

@javiereguiluz
Copy link
Member

I'm not sure we should add this to Symfony core.

  1. Your first issue is about quickly adding/removing locales without changing all routes in all controllers. This can already be done in Symfony: 1) define the available locales in a parameter (e.g. https://github.com/symfony/demo/blob/83f6242bf67bffe234956801e79d21258a1355f9/config/services.yaml#L6) and use that parameter to prefix and restrict all routes (e.g. https://github.com/symfony/demo/blob/master/config/routes/annotations.yaml)

  2. The redirection between languages look too custom to me. Sure, in some cases it may be enough to redirect XX to YY ... but in the kind of complex apps needing this feature, you'd need to do partial redirects, use different HTTP status depending on each content section, etc. It'd be better to do this in a kernel.request listener, where you can implement any redirection logic.

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Mar 31, 2020

@javiereguiluz I'm not sure I understand your "1.": i18n routes don't have to use prefixes, that's their point. Forcing a prefix-based layout just to be able to disable one locale doesn't seam to address the issue to me.

@hectorprats I'm not sure we need any new configuration: we already have an enabled_locales config entry, and a default_locale one.
About redirections, this should be a separate concern from "disabled": a redirected locale is not "disabled". It is "redirected".

Implementation wise, this could be implemented as a routing loader, coulnd't it?

(Note that these are just my first thoughts to contribute to the discussion, I might have missed many things :) )

@hectorprats
Copy link
Author

hectorprats commented Mar 31, 2020

@javiereguiluz Thanks for the input.

  1. Precisely my problem is that my web does not have prefixes.
  2. It's true, I also thought it might be too custom. But any website that uses languages ​​would be required to have its own development to temporarily disable a language (for any reason).
    But that's why it is a very general solution to disable a language. Not personalized by routes.

@nicolas-grekas I think you're right. I can improve it a lot in the use of variables.
And I'm going to look at the routing loader.

thanks to both of you.

I'm going to let the day pass (or 2 or 3) to finish talking about this and see the final way to tackle it.

@Nyholm
Copy link
Member

Nyholm commented May 3, 2020

I reviewed the code a bit.

Im 👎 If you want to temporary disable a language I suggest to write a RequestListener.

@fabpot
Copy link
Member

fabpot commented May 3, 2020

Let's close then.

@fabpot fabpot closed this as completed May 3, 2020
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

8 participants