We have moved to GitHub Issues
Created by Shannon Deminick 16 Apr 2018, 10:23:23 Updated by Shannon Deminick 09 May 2018, 05:43:14
Subtask of: U4-11217
Based on other dicussions, we need to build these APIs. Some may be extension methods, we should keep IPublishedContent as simple as possible.
NOTE: We are not building the "fallback provider" in this task, the fallback provider implementation and design will be part of a future task.
Site()then somehow list it's language variants or can i just get these language variants from the current node?
Sites variants too, UmbracoHelper can have a method to get all URLs for a node and be able to get URLs for a node/culture combination
Notes: I have a draft implementation of "fallback" and "current language".
''Current, default language''
There's an IPublishedVariationContextAccessor of some sort, which provides access to the current PublishedVariationContext, which contains the "current" language and segment. Typically, it would be retrieved from HttpContext. And populated in the pipeline once we know which content item we're rendering, hence which language we're in. It would be injected in IPublishedElement instances, and used when getting values with no specific language. This may require that we slightly refactor getting values, but that should not be too complicated. All in all, not too complicated.
That should already be available through the domains, we might just need to expose it properly.
Goes with routing. --Not sure I understand "''so the API should/could be able to return a collection of IPublishedContent items for the site per variant to get the URL''". Will need to discuss it.--
There's an IPublishedValueFallback interface that is also injected into IPublishedElement instances and is invoked whenever we want a value for a specified language and segment, and get nothing. It has access to the entire published element, and can do whatever it pleases, and return something, or nothing. Not too complicated.
When using the API for IPublishedContent, a user shouldn't need to know about the current culture when writing their template. If they want to output
Model.Name it should be the contextual name - so either invariant if the doc type is invariant, or culture specific based on the matched culture in the request
I'm unsure if we really need something like
IPublishedVariationContextAccessor. We can just use the combination of:
CultureInfo.CurrentCulture (which is the same as Thread.CurrentThread.CurrentCulture)
have done a few changes to the POC late last night, works kinda well
Code is in branch temp8-U4-11227, not a PR yet but getting there
So, on the front-end side, for culture & segment:
Tests are all green, so I ''think'' that model.Value("alias") should now return the proper "current culture" value on the front end - but I still need to test.
And we need to align services with it, so that on the back-end, it's also string.Empty for invariant (and not null anymore) - in progress.
Riiight... so it's a PR now. Requires a general code review. And tests: basically,
model.Value("alias") should return the value for the "current" culture, which should be the Thread.CurrentUICulture as we register (in WebRuntimeComponent) a ThreadCultureVariationContextAccessor. And
model.Value("alias", "fr-FR") should get you the french value (if any).
model.GetCulture("fr-FR").Name will give you the french name, assuming french is published / available. And
model.Cultures would list you the published / available cultures. This is also how we retrieve the url segment, and the published date, per culture.
I ''think'' it makes sense, but probably you'll unearth some oddities.
The amount of code changes here is pretty hard to fully review. I can build the solution but there's problems with saving and publishing content in the back office with YSODs, but i have no idea if I've already fixed this in my branch. There's a lot of merging i'll need to do between this and my branch too.
For now I'm going to merge this into my branch. I don't want to merge this to temp8 since im getting ysods with most things in the back office on this branch.
The property value is not returned contextually and still requires you to pass in a culture to get the correct property value. I can see IPublishedContent.Name and IPublishedContent.UrlSegment are contextual but properties are not. The notes above say this should work but it's not implemented and I can't see any code in there that would make this work.
I can see that if property.HasValue returns null it goes into the PublishedValueFallback.GetValue but that is a different case.
Just like IPublishedContent.Name uses
VariationContextAccessor.VariationContext.Culture, so does the implementation of
IPublishedProperty. However, i can see that the fallback is done in the extensions for the Value extensions, so i'll lave IPublishedProperty alone and implement this in the extensions.
OK ... I've finally found
ContextualizeVariation in the Property implementation. I'll have a look there. Currently i cannot get property values to return at all so something odd is going on.
I think i found the problem which is partly to do with
CompositeStringStringKey see screenshot
Heading off for the day, some things need to be fixed:
*Please read all my comments on the PR itself
*We will not merge this PR to temp8 - I've merged this into my branch U4-11282 and once all testing is completed there we can merge it all to temp8
*I've fixed up various things
**no AdditionalData, the properties are on IDocumentEntitySlim
**Fixed the dictionary in PublishedContent.cs to be InvariantCultureIgnoreCase
**Added a hack to CompositeStringStringKey.cs, you'll see a fixme there but it's needed else you cannot resolve any property value for cultures because the keys don't match (see above)
**Removes exception throwing on PropertyData.cs because if you throw, then nothing works including any deserialization and also since we are actively setting null values to these properties
*Fixes need to be applied to
HasValue since the currently implementation always fails and therefore you can never get the contextual value of a property using the Model.Value approach, though with the above fixes property.GetValue() will work
It seems that not much of this was tested (or maybe it just couldn't be tested?) since resolving any culture variant property value would definitely not work.
If this can be fixed today, i'll continue testing my branch tomorrow, fix any unit tests and put it in review.
Going to work in U4-11282 branch from now on, considering the two branches merged, it makes no sense to maintain the two branches separate at that point. Especially as fully testing one requires, as you've noted, the other.
Dealing w/ fixes - eg we cannot "hack" CompositeStringStringKey, this is a very low level class that should not do magic. Going through all the fixes, etc. today.
Have commented on comments on the PR. Fixing a few of them, points remaining open that I need to work on:
Awesome, i got a lot further today with these changes. I'm going to close this task, since things are 'working', there is of course some remaining issues but we're fixing them as part of U4-11282
Priority: Task - Pri 1
Backwards Compatible: True
Due in version: 8.0.0
Sprint: Sprint 84
Story Points: 5