When adding toggle to existing doctypes with an initial state of True, the value is False on the frontend until manually updated

[sebastiandammark]

If I create a checkbox datatype and configures it to be true(checked) as default. It will be false(unchecked) until saved with another value. The backend UI will show it as true(checked), but it's not.

It's not enough to publish the document.
You have to uncheck the property and then publish. Then check the property and publish again to set it to true(checked).

Umbraco version

I am seeing this issue on Umbraco version: Umbraco version 8.10.2

[nul800sebastiaan]

I'm afraid I can't reproduce this on 8.12.2 @sebastiandammark, but maybe I'm doing something wrong/different than you?

Gave it a go in 8.10.2 as well, but it "just" works for me. I would suggest you check what is saved for that property in the database, maybe that will give a clue?

I'll close this for now, but let me know if you have additional steps to reproduce.

[sebastiandammark]

Hi @nul800sebastiaan

Take a look at this screenshot:

The last line gives an empty output, where it should be true.

[abjerner]

I was just playing around with the new 8.13.0, so I tried reproducing this.

Say I'm adding a new property to an existing document type, and using a new Toggle data type, and set the default value to true, the following code returns false:

<pre>@Model.Value("test")</pre>

For content that haven't been saved after the property was added, there won't be a value in the database. Even though I would expect my test property to return true.

The data type must be this one:

https://github.com/umbraco/Umbraco-CMS/blob/v8/contrib/src/Umbraco.Web/PropertyEditors/TrueFalsePropertyEditor.cs

And the value converter is here (line 47 is the fallback value):

https://github.com/umbraco/Umbraco-CMS/blob/v8/contrib/src/Umbraco.Core/PropertyEditors/ValueConverters/YesNoValueConverter.cs#L47

But if a given property doesn't have a value, the value converter doesn't check the default value of the data type:

https://github.com/umbraco/Umbraco-CMS/blob/v8/contrib/src/Umbraco.Web/PropertyEditors/TrueFalseConfiguration.cs#L11

[marcemarc]

Is this why we changed it so it was no longer called the 'default value' because it doesn't behave like a 'default value' but because it was called 'default value' people believed what you describing above would be how it 'should work'

Now it's called 'Initial State' - The initial state for the toggle, when it is displayed for the first time in the backoffice. eg for a new content item

There is some details on this issue: #8420

Not saying it shouldn't work like a default value, but at the moment it isn't designed to be!

[abjerner]

@marcemarc sounds like the reason why. Thanks for sharing the history 👍

I think by changing the value converter a bit, we can get very closed to what most people are expecting.

Once you create a new content item with a Toggl property, the value configured as the Initial state will be saved in the database. I think this behavior hard to change, and I'd reckon its okay in most cases.

But via value converter we can control the initial value even if the property is added after the content item has been created. I don't we'll loose anything doing so, so it's worth making the change.

Question is whether something like this should be considered a breaking change (personally I don't think so). I may try playing a bit around with the value converter tomorrow. At the very least, one could overwrite the value converter locally.

[abjerner]

As mentioned before, the fallback is handled at line 47 in YesNoValueConverter.cs:

https://github.com/umbraco/Umbraco-CMS/blob/v8/contrib/src/Umbraco.Core/PropertyEditors/ValueConverters/YesNoValueConverter.cs#L47

Changing return false; to return propertyType.DataType.Configuration is TrueFalseConfiguration config && config.Default; should make it behave like what I think we'd want.

In local projects, the value converter below can be added (it automatically overwrites Umbraco's YesNoValueConverter.cs) to address the issue without upgrading:

using System;
using Umbraco.Core;
using Umbraco.Core.Models.PublishedContent;
using Umbraco.Core.PropertyEditors;
using Umbraco.Web.PropertyEditors;

namespace UmbracoEightThirteen {
    
    public class YesNoValueConverter : PropertyValueConverterBase {
        
        public override bool IsConverter(IPublishedPropertyType propertyType)
            => propertyType.EditorAlias == Constants.PropertyEditors.Aliases.Boolean;

        public override Type GetPropertyValueType(IPublishedPropertyType propertyType)
            => typeof(bool);

        public override PropertyCacheLevel GetPropertyCacheLevel(IPublishedPropertyType propertyType)
            => PropertyCacheLevel.Element;

        public override object ConvertSourceToIntermediate(IPublishedElement owner, IPublishedPropertyType propertyType, object source, bool preview) {
            // in xml a boolean is: string
            // in the database a boolean is: string "1" or "0" or empty
            // typically the converter does not need to handle anything else ("true"...)
            // however there are cases where the value passed to the converter could be a non-string object, e.g. int, bool

            if (source is string s) {
                if (s.Length == 0 || s == "0")
                    return false;

                if (s == "1")
                    return true;

                return bool.TryParse(s, out bool result) && result;
            }

            if (source is int)
                return (int)source == 1;

            // this is required for correct true/false handling in nested content elements
            if (source is long)
                return (long)source == 1;

            if (source is bool)
                return (bool)source;

            // Fallback as no value is saved
            return propertyType.DataType.Configuration is TrueFalseConfiguration config && config.Default;

        }

        // default ConvertSourceToObject just returns source ie a boolean value

        public override object ConvertIntermediateToXPath(IPublishedElement owner, IPublishedPropertyType propertyType, PropertyCacheLevel referenceCacheLevel, object inter, bool preview) {
            // source should come from ConvertSource and be a boolean already
            return (bool)inter ? "1" : "0";
        }
    }

}

I think the updated behavior will still match the description for the prevalue, so I'll go work on a PR as well 😛

[nul800sebastiaan]

I see, I thought this was for new documents. As with everything else, when you add a property to a doctype, we never ever update values for all of the existing content nodes as that is a big performance hit.

It gets a bit complicated here, since: yes, you could update the valueconverter to see if there's a null value and then return the "initial state" for that datatype, but this will require a lookup for that datatype which is not so nice. We need to cache that properly and do cache invalidation if the datatype configuration changes (and now we also need to be aware that datatypes can be shared between document types, so you might run into other "fun" scenarios when you meant to change the initial state only for 1 doctype and not for all of them, and suddenly half your site looks different!).

Of course it will be a breaking change because well.. people are for some reason or another might be expecting the current behavior now (https://xkcd.com/1172/).

Think it's fine to do this, but what we really need is an actual default values feature: #7859

It is just unfortunate that the the toggle currently seems to have a sort-of "default" value as the only datatype. This is something that we didn't think through when adding the initial state.
Actually, we do have another one, which might make it more clear how this could be perceived as a breaking change: the Numeric type has a configuration for a minimum value, you could expect that when you add it to a document type that it would always show up as at least the minimum value, not as 0 (default for int). But that's hardly going to be true for all existing documents!

Anyway, sorry for the novel. I think it will be okay to change this IF we're sure propertyType.DataType.Configuration is nicely cached @abjerner. We'll mark it as a breaking change to be on the safe side, but I don't expect it to cause problems.

[umbrabot]

Hi @sebastiandammark,

We're writing to let you know that we would love some help with this issue. We feel that this issue is ideal to flag for a community member to work on it. Once flagged here, folk looking for issues to work on will know to look at yours. Of course, please feel free work on this yourself ;-). If there are any changes to this status, we'll be sure to let you know.

For more information about issues and states, have a look at this blog post

Thanks muchly, from your friendly Umbraco GitHub bot :-)

[abjerner]

@nul800sebastiaan isn't the point with the PublishedDataType instances that they are in fact cached?

If I play around, I'm getting the same hash code for data type and the configuration respectively for repeated requests. So unless Umbraco is overwriting the hash code, I think that should mean I'm getting the same instance back each time for the data type.

Updating the data type via the backoffice will result in new hash codes for both the data type and the configuration, which is what I would expect.

[abjerner]

Oh snap - TrueFalseConfiguration is located in the Umbraco.Web project, so we can't see it from the YesNoValueConverter class, as that one is located in the Umbraco.Core project. This makes things a bit more complicated.

Any suggestions? Reflection? 🙃

Or should we introduced a new interface in Umbraco.Core that TrueFalseConfiguration could implement? Seems like a bit of extra work 🤷

[umbrabot]

Hiya @sebastiandammark,

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 🤖 🙂

[Nikkelmann]

@umbrabot still relevant
This bug can still be reproduced in version 10.2.0

[Wiggee11]

@Nikkelmann raised a slightly different issue relating to this - #13134

This issue is to do with the block list settings not being applied unless the settings window is opened.