Added better models builder support

[mattbrailsford]

This PR aims to add better models builder support. The value converter now does clever stuff to workout the target model and lets models builder know so it should know the target Type of the wrapped property editor assuming it has a value converter with a models builder type defined.

With this, where as before your used to have to do (and still can)

@using Our.Umbraco.Vorto.Extensions
@inherits UmbracoViewPage

@Model.GetVortoValue("myProp", "es", fallbackCultureName: "en")

You can now instead do (assuming Models Builder is configured)

@using Our.Umbraco.Vorto.Extensions
@inherits UmbracoViewPage<MyPage>

@Model.MyProp.GetValue("es", fallbackCultureName: "en")

The beauty of this is that Models Builder knows the inner type and you can straight away access the strongly typed inner model. For example, assuming MyProp is a content picker, you can do the following:

@Model.MyProp.GetValue("es", fallbackCultureName: "en").Name

Things to note are, that using the GetValue methods on the Models Builder property doesn't allow recursive properties. Given this is pretty much true throughout Models Builder though, I don't see that as a problem. And you can always revert back to the old approach which will still work.

What do people think of the syntax? Is there a better approach we could follow?

Also, due to this update, there has been a huge change in how the value converter works. Where we previously converted property values on the fly, we now convert all the values in the value converter and then in the extension methods we are just doing lookups. This should work the same as before for the old API, but I'd like some feedback on this from others if possible.

[mattbrailsford]

For anyone that wants to try it, you can find the build server artifacts (package files) here

https://ci.appveyor.com/project/UMCO/umbraco-vorto/build/1.6.1.130/artifacts

[readingdancer]

Hi Matt,

This looks really interesting, I'm currently using Vorto on a new client project like this:

var category = (ContentModels.FaqCategory)publishedContent;
@category.GetVortoValue<string>("categoryName")

What I'd like is for it to do something similar to your change, so that they syntax was:

@category.GetValue().CategoryName

Ideally I'd like the language to use the current culture and to fall back to a culture defined somewhere as the default setting, so you don't need to repeat it on every call, which it seems you would have to do with your example above?

FYI.. My example above is within a foreach loop...

I might be missing something ;)

Cheers, Chris

[mattbrailsford]

Hey Chris,

So yea, I think the only thing I would need to add is the default fallback culture support for this.

What are we thinking, registering it in an app event handler?

Vorto.DefaultFallbackCultureName = "en";

[mattbrailsford]

Or, could be web.config?

[mattbrailsford]

Or both :)

[readingdancer]

I guess if it's easy to do both it would give people the flexibility to do whatever suits their solution. For the site I am working on, it's an American site with English as the primary language and Spanish as the secondary language, I don't think this will ever change and they have no intention to add any other language in the future, hence being able to specify the fallback in one place and then forget about it would be great :)

Also the least number of key strokes to get the property data out will save us all time, so that would be awesome if it's possible.

Cheers, Chris

[mattbrailsford]

I guess there is the question of, if we have a default culture defined globally, would there be a need to access any other culture. I mean, you could in theory have

@category.CategoryName

[mattbrailsford]

Ok, so a default fallback culture feature has been added, so you can either set via an application event handler

Vorto.DefaultFallbackCutltureName = "en";

Or set a web.config value

<add key="Vorto:DefaultFallbackCultureName" value="en" />

Which now means, if you just want to get a value for the current UI culture, falling back to the default, you can do

@Model.MyProp.GetValue()

[readingdancer]

I didn't realise you might be able to make it work without the GetValue() that would be really sweet and 12 less key strokes for every time we access a property :)

[readingdancer]

Hi @mattbrailsford , you have a typo in your method name:
DefaultFallbackCutltureName => DefaultFallbackCultureName

[mattbrailsford]

Typical me :)

[mattbrailsford]

Hmm, I may not be able to do the direct access thing actually. Well, I could, but it would mean there would be no way for people to access the other cultures should they need to.

[mattbrailsford]

Ok, so I've tweaked the API as follows

You can use GetValue but I've now made the cultureName mandatory. So you'd need to do:

@Model.MyProp.GetValue("en")

The should be concidered the approach to use if you want an explicit culture and / or to override the default culture / default value.

As shorthand, to access a specific culture is now

@Model.MyProp["en"]

This is a nice, shorter way to get a culture value, but doesn't support fallback etc

And as a shorthand to get the current culture (falling back to the global default culture) would be

@Model.MyProp.Current

Only questio here is, is it ok for GetValue to have cultureName as mandatory. I've done it to enforce the use of .Current as the recommended way of accessin the current culture value as with cultureName being optional GetValue() would be the exact same as .Default.

Thoughts?

[mattbrailsford]

Now I've written this, I guess there may be a call for GetValue cultueName to be optional, incase you want the current culture value, with a different fallback than the global?

[Mantus667]

I would keep GetValue() and make cultureName optional.
Also maybe provide a overwrite with a culture as parameter?

Also for a global fallback the Vorto.DefaultFallbackCutltureName is a good option.
My concern here would be an instance with multiple websites in it. But with that option you could set the property in your master view and have it for all of the request. Or you can use an ActionFilter to set it.

[mattbrailsford]

Question, with the syntax @Model.MyProp["en"] do you think this should use the global fallback if there isn't that given culture key? Or would that be unintuitive?

[emmagarland]

@mattbrailsford Interesting question - I was wondering that too, when I put "en" instead of "en_US" and it threw a KeyNotFoundException. Maybe handle this with a specific error message?

I would have expected "en" to work or use "en-US" at least since it is in the same language family, but that is just my personal preference. Using the global fallback/default culture, not sure that is the behaviour everyone would want - I'd prefer it to just return null/empty string (since it doesn't exist) and handle the exception, or include a useful error message.

Maybe "UseGlobalFallbackIfMissingLanguageKey" or something could be an option in the Vorto setup/config, defaulting to true?

On another note, I can't see to make the web.config inclusion work to set my global fallback - am I right to just add this for Vorto to pick it up, or do I need to do something else too?:

<add key="Vorto:DefaultFallbackCultureName" value="fr" />

[mattbrailsford]

Hey @emmagarland, good suggestions. I've gone and implemented those so that the ["en"] syntax now does a best match lookup (without fallback). This is in commit 4fca26c and you kind find the latest build on the build server if you wanna try out the update.

I've tested the webconfig thing, and your syntax is right, but it seems to be working ok for me. Maybe the tweaks I just made fixed it, but I'd love it if you could double check with the latest build?

[emmagarland]

@mattbrailsford cool stuff, will install the latest package from the build server and test it out this avo focusing on the web.config and best match lookup 👍

[emmagarland]

Can confirm:

• "en" in view now falls back to "en-US" and shows the text string instead of exception
• "en-FR" wrong key in view now returns an empty string instead of exception
• "fr" in view shows the French language
• .Current in view returns the default culture "en-US"

Just checked and "fr" or "zh-Hans" in web.config are not yet working to overwrite my default "en-US" language fallback. I have debugged using the source code and it is:

• setting my default culture to "en-US" via the Thread.CurrentThread.CurrentUICulture.Name
• setting the fallback culture specified on the next line (fallbackCultureName is "zh-Hans").

However, the first part of the argument on line 89 VortoValueOfTApi.cs "GetValue" method is coming back as false so it is never setting the fallbackCulture

EqualityComparer<T>.Default.Equals(result, default(T))

If I force this to be true it returns my expected fallback culture from the web.config, so the web.config part is working its just not choosing fallback for me.

Also, not sure how you feel about writing to the Umbraco logger if the key was not found? Don't wanna clog up the logs but it does seem like something we might want to know about if the key was deleted or there was a typo or something.

On that note, are the keys ever accessible strongly-typed from Umbraco language dictionary, so you could just do Language.English instead of "en" then avoiding any magic strings?

Cheers!

[mattbrailsford]

Hey @emmagarland that's interesting. What property editor are you wrapping that the EqualityComparer didn't work on? I'd like to do some testing around that.

[emmagarland]

@mattbrailsford it is the standard TextArea with these setting:

[mattbrailsford]

@readingdancer sure. Any help testing would be appreciated. Really just want to make sure it doesn't explode anyones installs and that the API makes sense.

[emmagarland]

Just to update, I've been using it on an internal project since I installed it and still working as expected.

[emmagarland]

I tend to create a new string property on the model, and do

@Model.MyProperty?.Current