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

Route based Culture Provider #5125

Open
dodyg opened this issue Dec 25, 2019 · 33 comments
Open

Route based Culture Provider #5125

dodyg opened this issue Dec 25, 2019 · 33 comments
Milestone

Comments

@dodyg
Copy link

dodyg commented Dec 25, 2019

Is there any built in way in Orchard Core to ensure that when someone access https://xxx.xx/ar/education-and-scholarships/ they will get a specific culture (in this case Arabic) and when they access https://xxx.xx/en/education-and-scholarships/ then will get English?

The cookie based culture provider doesn't really work well when you don't start from the home page.

@hishamco
Copy link
Member

@dodyg I mentioned such provider few times ago, IMHO either we improve the CCP or create a new branch for RCC and test the result then to decide which path should we go

/cc @jptissot

@dodyg
Copy link
Author

dodyg commented Dec 25, 2019

Ok thanks for the info. I'll write a middleware for my site to deal with this situation or use aCustomRequestCultureProvider .

@dodyg
Copy link
Author

dodyg commented Dec 25, 2019

IMHO either we improve the CCP or create a new branch for RCC and test the result then to decide which path should we go

Cookie based approach make sense if you have the same path for the multi-lingual content item, e.g. "/about-us" that will provide English, Arabic, or French depending on the cookie. In a situation where each language version of a Content Item has their own path, cookie based approach is really confusing to user and can create embarrassing bugs.

@hishamco
Copy link
Member

Ok thanks for the info. I'll write a middleware for my site to deal with this situation or use aCustomRequestCultureProvider .

No need, there's already one built-in provided by Localization APIs

@dodyg
Copy link
Author

dodyg commented Dec 25, 2019

Oh yeah?

@dodyg
Copy link
Author

dodyg commented Dec 25, 2019

Is this the one ContentRequestCultureProvider ?

@hishamco
Copy link
Member

I mean in ASP.NET Localization APIs not OC

@dodyg
Copy link
Author

dodyg commented Dec 25, 2019

Thanks.

This is my quick implementation so far using CustomRequestCultureProvider. Not suitable for general usage.

        public void ConfigureServices(IServiceCollection services)
        {
            services.AddOrchardCms()
                .AddOrchardCore()
                .ConfigureServices(s =>
                {
                    s.AddLiquidFilter<Markdownify2>("markdownify");
                }
                , order: 100);

            services.Configure<RequestLocalizationOptions>(options =>
            {
                options.AddInitialRequestCultureProvider(new CustomRequestCultureProvider(context =>
                {
                    var currentCulture = "en-US";
                    var segments = context.Request.Path.Value.Split(new char[] { '/' }, StringSplitOptions.RemoveEmptyEntries);

                    if (segments.Length > 1 && segments[0].Length == 2)
                    {
                        currentCulture = segments[0] switch
                        {
                            "en" => "en-US",
                            "ar" => "ar-EG",
                            _ => "en-US"
                        };
                    }

                    var requestCulture = new ProviderCultureResult(currentCulture);

                    return Task.FromResult(requestCulture);
                }));
            });
        }

@hishamco
Copy link
Member

segments[0] can match the supported cultures

@dodyg
Copy link
Author

dodyg commented Dec 25, 2019

segments[0] can match the supported cultures

Can you please clarify this? I am not sure what you mean by this.

This is another version that makes the admin section to respect the cookie but to make the rest of the website to simply use the path. This is only relevant if at least one of your language is RTL.

          services.Configure<RequestLocalizationOptions>(options =>
            {
                options.AddInitialRequestCultureProvider(new CustomRequestCultureProvider(context =>
                {
                    var currentCulture = "en-US";
                    var segments = context.Request.Path.Value.Split(new char[] { '/' }, StringSplitOptions.RemoveEmptyEntries);

                    if (segments.Length > 1)
                    {
                        if (segments[0].Length > 2 && segments[0].Contains("Admin", StringComparison.InvariantCultureIgnoreCase) || segments[0].Contains("OrchardCore", StringComparison.InvariantCultureIgnoreCase))
                        {
                            return Task.FromResult<ProviderCultureResult>(null);
                        }

                        currentCulture = segments[0] switch
                        {
                            "en" => "en-US",
                            "ar" => "ar-EG",
                            _ => "en-US"
                        };
                    }

                    var requestCulture = new ProviderCultureResult(currentCulture);

                    return Task.FromResult(requestCulture);
                }));
            });

@hishamco
Copy link
Member

I don't think the switch is needed, because the ASP.NET Core will do the fallback on your behalf, further more, I still prefer the built-in RCP, it will save you much time :)

@dodyg
Copy link
Author

dodyg commented Dec 25, 2019

I will probably end up with RCP but it's the first time I encounter CustomRequestCultureProvider and I am figuring things out what I can use it for.

@hishamco
Copy link
Member

All the best ..

I will keep this open to hear what the team thought

@dodyg
Copy link
Author

dodyg commented Dec 25, 2019

I don't think the switch is needed, because the ASP.NET Core will do the fallback on your behalf,

Here's the thing, ASP.NET Core will do fall back from specific to general, e.g. ar-EG will fallback to ar, en-UK to en, etc.

In my scenario I already setup the supported cultures to specific ar-EG and en-US (don't ask me why, it was set many moons ago before all contents have been uploaded).

So I cannot return ProviderCultureResult("ar") and get ar-EG.

@dodyg
Copy link
Author

dodyg commented Dec 25, 2019

This is the latest iteration that actually use the culture settings from Orchard Core.

// Make the rest of the website to simply respect the path while making the admin section to respect the culture cookie
            services.Configure<RequestLocalizationOptions>(options =>
            {
                options.AddInitialRequestCultureProvider(new CustomRequestCultureProvider(async context =>
                {
                    var localizationService = context.RequestServices.GetService<ILocalizationService>();

                    var defaultCulture = await localizationService.GetDefaultCultureAsync();
                    
                    var segments = context.Request.Path.Value.Split(new char[] { '/' }, StringSplitOptions.RemoveEmptyEntries);

                    if (segments.Length > 1)
                    {
                        if (segments[0].Length > 2 && segments[0].Contains("Admin", StringComparison.InvariantCultureIgnoreCase) || segments[0].Contains("OrchardCore", StringComparison.InvariantCultureIgnoreCase))
                        {
                            return null;
                        }

                        var supportedCultures = (await localizationService.GetSupportedCulturesAsync());

                        foreach(var c in supportedCultures)
                        {
                            if (c.StartsWith(segments[0]))
                                return new ProviderCultureResult(c);
                        }

                    }

                    return new ProviderCultureResult(defaultCulture);
                }));
            });

@hishamco
Copy link
Member

if (c.StartsWith(segments[0]))

Also you could use c == CultureInfo.GetCulture(segments[0]).Parent.Name

Anyhow, glad you find a proper approach to solve your issue, all the best

@dodyg
Copy link
Author

dodyg commented Dec 25, 2019

I think it's awesome one can reprogram the way the request culture works in less than 20 lines of additional code.

@sebastienros sebastienros added this to the backlog milestone Dec 26, 2019
@jaliao
Copy link

jaliao commented Mar 18, 2020

I need this function

do we have any plan or when will release this function?

@dodyg
Copy link
Author

dodyg commented Mar 18, 2020

You can copy this code in our startup.cs #5125 (comment)

@hishamco
Copy link
Member

@jaliao there's no plan yet to implement this yet, but you could start with what @dodyg is already done

Hopefully we will consider this in the future releases, because we 're focus to release 1.0.0

My regards

@jaliao
Copy link

jaliao commented Mar 19, 2020

@dodyg , @hishamco thanks : )

@jaliao
Copy link

jaliao commented Mar 19, 2020

@hishamco
How can i know the function list in the release 1.0.0?

Best regards

@hishamco
Copy link
Member

@jaliao please have a look to the roadmap

@sebastienros
Copy link
Member

I thought we had this feature already, but can't find it.

The solution is to implement a custom display handler, or change the existing one, so when a content is displayed, load its CultureApsect, and set the CurrentThread and CurrentUIThread culture to this.

@sebastienros sebastienros modified the milestones: backlog, 1.0.x Mar 19, 2020
@jtkech
Copy link
Member

jtkech commented Mar 19, 2020

Maybe this one ContentRequestCultureProvider : RequestCultureProvider implemented in the OC.ContentLocalization module, not really a route based provider but based on localized items on which you can set autoroutes whose pattern can contain what you want e.g a culture segment.

@Skrypt
Copy link
Contributor

Skrypt commented Mar 19, 2020

Yeah right this should work?

@sebastienros
Copy link
Member

It can't be a RequestCultureProvider because you don't know what content item is loaded and about to be displayed. But the handler should use the same IFeature to set the culture of the request. Actually I am not even sure we can use a handler, as any time something would call BuildDisplay it could change the culture. It might be set in the ItemController directly.

@jtkech
Copy link
Member

jtkech commented Mar 19, 2020

It can't be a RequestCultureProvider because you don't know what content item is loaded

Hmm, but seems that this is what the ContentRequestCultureProvider is able to do by using

culturePickerService.GetLocalizationFromRouteAsync(httpContext.Request.Path)

@Skrypt
Copy link
Contributor

Skrypt commented Mar 19, 2020

So, also here it will only work if using a LocalizationPart a the content type. For example if you have your own controller that uses a domain path prefix that is the culture it won't magically make your page display in that culture.

@jtkech
Copy link
Member

jtkech commented Mar 19, 2020

Yes i agree it would be just an indirect way to have "localized" route

As i remember there was an mvc provider for this but not used by default, hmm and where i saw something wrong in the code but i don't remember what. I will try to find it again

@Skrypt
Copy link
Contributor

Skrypt commented Mar 20, 2020

After reviewing the ContentRequestCultureProvider last night, I decided that we need to refactor this part. Reason is that a contentItemId + routeUrl is not an immutable value. The routes can be added only once in the Dictionary that adds them in the mvc routing. So it's not safe enough to say that when we search a content item by url that it will return the proper one.

What we need to do here is that when we use the LocalizationPart and the AutoroutePart on a content item. The AutoroutePart pattern should be altered to include the contentItemId at the end of the url automatically. This way, we make sure that the AutoroutePart pattern stay editable, but we also make sure that we always retrieve the proper content item and it's LocalizationSet related one(s).

@sebastienros
Copy link
Member

One route, one content item, if it's not the case then that's a bug. Doesn't mean that two routes can't point to the same content item though, but I don't think we support that.

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

6 participants