umbraco / Umbraco-CMS

Umbraco is a free and open source .NET content management system helping you deliver delightful digital experiences.
https://umbraco.com
MIT License
4.42k stars 2.67k forks source link

Not possible to access alternate language for Nested Content property #7692

Closed NikRimington closed 2 years ago

NikRimington commented 4 years ago

I'm unable to access language variants for a property of type NestedContent with vary by culture enabled. No error logged/thrown, but .Value returns empty/default values. This is also means that standard fallback behaviour does not work.

Steps to reproduce

  1. Install Umbraco with the starter kit
  2. Create a new Doc Type called "menu item" with 2 properties, "title" and "content picker". This should be an Element type as it will be used with nested content
  3. Set up a second language and set the fallback to default English US that is already there.
  4. Create a "Menu Builder" data type of type nested content using the new doc type created in step 2.
  5. Edit the home page to make it variable by culture
  6. Add a new property called "Nested Menu" that uses the Menu Builder data type and set it to be variable by culture.
  7. Populate only the English instance of this property.
  8. Create an empty variant of the home page for the second language created in step 3.
  9. Modify the home page template to render out this new menu. e.g. code that looks like this:
<section>
    <div>Testing behaviour</div>
    @if(Model.NestedMenu != null && Model.NestedMenu.Any())
    {
        foreach(MenuItem menuItem in Model.NestedMenu)
        {
            <div>@menuItem.Title</div>
        }
        foreach(MenuItem menuItem in Model.Value<IEnumerable<MenuItem>>("nestedMenu", "en-us"))
        {
            <div>E: @menuItem.Title</div>
        }
        foreach(MenuItem menuItem in Model.Value<IEnumerable<MenuItem>>("nestedMenu", "cy-gb"))
        {
            <div>W: @menuItem.Title</div>
        }
    }
</section>

Expected result (assuming welsh was used as second language)

Testing behaviour should render followed by 2 instances of the same content on the default home page, on the alternate language home page the same should also appear.

What shows

English: image

Alternate language: image

nul800sebastiaan commented 4 years ago

I would've thought that this should work as with other properties. Which Umbraco version are you using?

NikRimington commented 4 years ago

We have the issue on an 8.4.1 site, but my repo steps were tested on an 8.5.3 via nuget with starter kit.

NikRimington commented 4 years ago

I've just pulled down the latest v8/dev source and repeated the process there and it doesn't work there either just as an FYI :-)

dawoe commented 4 years ago

Hi Nic,

Have you tried refreshing the published cache in the dashboard in the developers section. I found that sometimes is needed when you modify or create doctypes.

Still have to find out why by the way.

Dave

NikRimington commented 4 years ago

Hmm, so I think I'm getting closer. I'm using Models Builder models, but they don't pass in a fallback option so it's always null when .Value is called.

So this means that Models Builder Models can't handle fallback for language. If you use .Value and explicitly say use the language fallback you can get the value!

So... is this a bug with Models Builder behaviour? I would expect that if I've created a multi-lingual site and set the languages to fall back to another one if not set then it should do that without any Dev work required!

kjac commented 4 years ago

@NikRimington this has been the behavior for all of V8. As such we can't really change the behavior explicitly, as it would be a huge behavioral breaking change.

That being said I do agree that ModelsBuilder has never really played nice with fallback options. While .Value() gives you an option to use fallback (mind you, you still have to be explicit in your choice of fallback), ModelsBuilder does not. Which makes sense as ModelsBuilder exposes values as type safe properties and thus has no parameters for fallback.

However: It would be clever if one could supply a fallback behavior for the generated models, thus enabling us to force whatever fallback for every property. This could be done in a non-breaking fashion, as long as the default fallback behavior would be None.

NikRimington commented 4 years ago

Could we add a config value that Models builder uses to determine what fallback method should be used? If that value isn't set then it can fallback to none? I don't know enough about how models builder works to know how feasible that is but I don't see why it shouldn't be possible?

kjac commented 4 years ago

@NikRimington that was kinda where I was getting at with my last comment. But having thought about it some more I now sort of disagree with myself: We can't have one behavior for ModelsBuilder and another for the "normal rendering" (.Value()).

In other words: If we're do to anything it would be to add a means (by config or code) to dictate the default fallback behavior. And if I'm not mistaken, this is already an option by code, although a rather complicated one - thus the appeal to be able to change it by config.

I'll dig a bit further into this subject later.

On a side note: Even if we could change/dictate the default fallback behavior, it's not completely certain that it would help in your concrete case. It's also a question of how "no entries" are saved by NC and if "no entries" counts as a value or not for the given culture. To test this, see if you can produce your expected results using .Value() (with a language fallback) in your test case instead of using ModelsBuilder properties.

skttl commented 4 years ago

On Zbu.ModelsBuilder there is an issue about having the properties as methods, so you could add culture, fallbacks etc. where you need it. https://github.com/zpqrtbnk/Zbu.ModelsBuilder/issues/206

This would be great to have in the embedded version.

ronaldbarendse commented 4 years ago

The changes to support culture, segment, fallback and default values are already done within ModelsBuilder, so these overloads get generated: https://github.com/OurModelsBuilder/Our.ModelsBuilder/issues/206#issuecomment-526084143 (the related commits are referenced in this issue).

It would be nice if Embedded also added these changes indeed!

umbrabot commented 2 years ago

Hiya @NikRimington,

Just wanted to let you know that we noticed that this issue got a bit stale and might not be relevant any more.

We will close this issue for now but we're happy to open it up again if you think it's still relevant (for example: it's a feature request that's not yet implemented, or it's a bug that's not yet been fixed).

To open it this issue up again, you can write @umbrabot still relevant in a new comment as the first line. It would be super helpful for us if on the next line you could let us know why you think it's still relevant.

For example:

@umbrabot still relevant This bug can still be reproduced in version x.y.z

This will reopen the issue in the next few hours.

Thanks, from your friendly Umbraco GitHub bot :robot: :slightly_smiling_face: