Use data type configuration to determine default value for empty toggle and slider property values (#17854)

AndyButland · nikolajlauridsen · web-flow · commit 413398afc6cb · 2025-01-08T11:42:13.000+01:00
* Use data type configuration to determine default value for empty toggle property values.

* Added/updated unit tests.

* Fixed failing integration tests.

* Applied similar default value display for the slider property editor and aligned implementation of true/false with this.

* Fixed unit tests.

* Removed "duplicate" JsonPropertyName attributes and added a custom TypeInfoResolver for data type configuration so we can re-use the existing ConfigurationField attributes.

* Minor cleanup

---------

Co-authored-by: nikolajlauridsen &lt;nikolajlauridsen@protonmail.ch&gt;
diff --git a/src/Umbraco.Core/PropertyEditors/SliderConfiguration.cs b/src/Umbraco.Core/PropertyEditors/SliderConfiguration.cs
@@ -1,3 +1,5 @@
+using System.Text.Json.Serialization;
+
 namespace Umbraco.Cms.Core.PropertyEditors;
 
 /// <summary>
@@ -13,4 +15,10 @@ public class SliderConfiguration
 
     [ConfigurationField("maxVal")]
     public decimal MaximumValue { get; set; }
+
+    [ConfigurationField("initVal1")]
+    public decimal InitialValue1 { get; set; }
+
+    [ConfigurationField("initVal2")]
+    public decimal InitialValue2 { get; set; }
 }
diff --git a/src/Umbraco.Core/PropertyEditors/TrueFalseConfiguration.cs b/src/Umbraco.Core/PropertyEditors/TrueFalseConfiguration.cs
@@ -0,0 +1,12 @@
+using System.Text.Json.Serialization;
+
+namespace Umbraco.Cms.Core.PropertyEditors;
+
+/// <summary>
+///     Represents the configuration for the true/false (toggle) value editor.
+/// </summary>
+public class TrueFalseConfiguration
+{
+    [ConfigurationField("default")]
+    public bool InitialState { get; set; }
+}
diff --git a/src/Umbraco.Core/PropertyEditors/TrueFalseConfigurationEditor.cs b/src/Umbraco.Core/PropertyEditors/TrueFalseConfigurationEditor.cs
@@ -0,0 +1,17 @@
+// Copyright (c) Umbraco.
+// See LICENSE for more details.
+
+using Umbraco.Cms.Core.IO;
+
+namespace Umbraco.Cms.Core.PropertyEditors;
+
+/// <summary>
+///     Represents the configuration editor for the true/false (toggle) value editor.
+/// </summary>
+public class TrueFalseConfigurationEditor : ConfigurationEditor<TrueFalseConfiguration>
+{
+    public TrueFalseConfigurationEditor(IIOHelper ioHelper)
+        : base(ioHelper)
+    {
+    }
+}
diff --git a/src/Umbraco.Core/PropertyEditors/ValueConverters/SliderValueConverter.cs b/src/Umbraco.Core/PropertyEditors/ValueConverters/SliderValueConverter.cs
@@ -49,16 +49,29 @@ public override PropertyCacheLevel GetPropertyCacheLevel(IPublishedPropertyType
     /// <inheritdoc />
     public override object? ConvertIntermediateToObject(IPublishedElement owner, IPublishedPropertyType propertyType, PropertyCacheLevel cacheLevel, object? source, bool preview)
     {
-        bool isRange = IsRange(propertyType);
+        SliderConfiguration? configuration = propertyType.DataType.ConfigurationAs<SliderConfiguration>();
+        bool isRange = IsRange(configuration);
 
         var sourceString = source?.ToString();
 
+        // If source is null, the returned value depends on the configured initial values.
+        if (string.IsNullOrEmpty(sourceString))
+        {
+            return isRange
+                ? new Range<decimal>
+                {
+                    Minimum = configuration?.InitialValue1 ?? 0M,
+                    Maximum = configuration?.InitialValue2 ?? 0M
+                }
+                : configuration?.InitialValue1 ?? 0M;
+        }
+
         return isRange
             ? HandleRange(sourceString)
             : HandleDecimal(sourceString);
     }
 
-    private static Range<decimal> HandleRange(string? sourceString)
+    private static Range<decimal> HandleRange(string sourceString)
     {
         if (sourceString is null)
         {
@@ -92,13 +105,8 @@ private static Range<decimal> HandleRange(string? sourceString)
         return new Range<decimal>();
     }
 
-    private static decimal HandleDecimal(string? sourceString)
+    private static decimal HandleDecimal(string sourceString)
     {
-        if (string.IsNullOrEmpty(sourceString))
-        {
-            return default;
-        }
-
         // This used to be a range slider, so we'll assign the minimum value as the new value
         if (sourceString.Contains(','))
         {
@@ -124,5 +132,8 @@ private static bool TryParseDecimal(string? representation, out decimal value)
         => decimal.TryParse(representation, NumberStyles.Number, CultureInfo.InvariantCulture, out value);
 
     private static bool IsRange(IPublishedPropertyType propertyType)
-        => propertyType.DataType.ConfigurationAs<SliderConfiguration>()?.EnableRange == true;
+        => IsRange(propertyType.DataType.ConfigurationAs<SliderConfiguration>());
+
+    private static bool IsRange(SliderConfiguration? configuration)
+        => configuration?.EnableRange == true;
 }
diff --git a/src/Umbraco.Core/PropertyEditors/ValueConverters/YesNoValueConverter.cs b/src/Umbraco.Core/PropertyEditors/ValueConverters/YesNoValueConverter.cs
@@ -14,8 +14,13 @@ public override Type GetPropertyValueType(IPublishedPropertyType propertyType)
     public override PropertyCacheLevel GetPropertyCacheLevel(IPublishedPropertyType propertyType)
         => PropertyCacheLevel.Element;
 
-    public override object ConvertSourceToIntermediate(IPublishedElement owner, IPublishedPropertyType propertyType, object? source, bool preview)
+    public override object? ConvertSourceToIntermediate(IPublishedElement owner, IPublishedPropertyType propertyType, object? source, bool preview)
     {
+        if (source is null)
+        {
+            return null;
+        }
+
         // 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"...)
@@ -35,23 +40,36 @@ public override object ConvertSourceToIntermediate(IPublishedElement owner, IPub
             return bool.TryParse(s, out var result) && result;
         }
 
-        if (source is int)
+        if (source is int sourceAsInt)
         {
-            return (int)source == 1;
+            return sourceAsInt == 1;
         }
 
         // this is required for correct true/false handling in nested content elements
-        if (source is long)
+        if (source is long sourceAsLong)
         {
-            return (long)source == 1;
+            return sourceAsLong == 1;
         }
 
-        if (source is bool)
+        if (source is bool sourceAsBoolean)
         {
-            return (bool)source;
+            return sourceAsBoolean;
         }
 
-        // default value is: false
+        // false for any other value
         return false;
     }
+
+    /// <inheritdoc />
+    public override object? ConvertIntermediateToObject(IPublishedElement owner, IPublishedPropertyType propertyType, PropertyCacheLevel cacheLevel, object? source, bool preview)
+    {
+        // If source is null, whether we return true or false depends on the configured default value (initial state).
+        if (source is null)
+        {
+            TrueFalseConfiguration? configuration = propertyType.DataType.ConfigurationAs<TrueFalseConfiguration>();
+            return configuration?.InitialState ?? false;
+        }
+
+        return (bool)source;
+    }
 }
diff --git a/src/Umbraco.Infrastructure/PropertyEditors/TrueFalsePropertyEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/TrueFalsePropertyEditor.cs
@@ -1,6 +1,8 @@
 // Copyright (c) Umbraco.
 // See LICENSE for more details.
 
+using Microsoft.Extensions.DependencyInjection;
+using Umbraco.Cms.Core.DependencyInjection;
 using Umbraco.Cms.Core.IO;
 using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.Models.Editors;
@@ -18,17 +20,37 @@ namespace Umbraco.Cms.Core.PropertyEditors;
     ValueEditorIsReusable = true)]
 public class TrueFalsePropertyEditor : DataEditor
 {
+    private readonly IIOHelper _ioHelper;
+
     /// <summary>
     ///     Initializes a new instance of the <see cref="TrueFalsePropertyEditor" /> class.
     /// </summary>
+    [Obsolete("Please use the constructor taking all parameters. This constructor will be removed in V17.")]
     public TrueFalsePropertyEditor(IDataValueEditorFactory dataValueEditorFactory)
+        : this(
+              dataValueEditorFactory,
+              StaticServiceProvider.Instance.GetRequiredService<IIOHelper>())
+    {
+    }
+
+    /// <summary>
+    ///     Initializes a new instance of the <see cref="TrueFalsePropertyEditor" /> class.
+    /// </summary>
+    public TrueFalsePropertyEditor(IDataValueEditorFactory dataValueEditorFactory, IIOHelper ioHelper)
         : base(dataValueEditorFactory)
-        => SupportsReadOnly = true;
+    {
+        _ioHelper = ioHelper;
+        SupportsReadOnly = true;
+    }
 
     /// <inheritdoc />
     protected override IDataValueEditor CreateValueEditor()
         => DataValueEditorFactory.Create<TrueFalsePropertyValueEditor>(Attribute!);
 
+    /// <inheritdoc />
+    protected override IConfigurationEditor CreateConfigurationEditor() =>
+        new TrueFalseConfigurationEditor(_ioHelper);
+
     internal class TrueFalsePropertyValueEditor : DataValueEditor
     {
         public TrueFalsePropertyValueEditor(
diff --git a/src/Umbraco.Infrastructure/Serialization/SystemTextConfigurationEditorJsonSerializer.cs b/src/Umbraco.Infrastructure/Serialization/SystemTextConfigurationEditorJsonSerializer.cs
@@ -1,5 +1,7 @@
 using System.Text.Json;
 using System.Text.Json.Serialization;
+using System.Text.Json.Serialization.Metadata;
+using Umbraco.Cms.Core.PropertyEditors;
 using Umbraco.Cms.Core.Serialization;
 
 namespace Umbraco.Cms.Infrastructure.Serialization;
@@ -16,8 +18,9 @@ public SystemTextConfigurationEditorJsonSerializer()
         => _jsonSerializerOptions = new JsonSerializerOptions()
         {
             PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
-            // in some cases, configs aren't camel cased in the DB, so we have to resort to case insensitive
-            // property name resolving when creating configuration objects (deserializing DB configs)
+
+            // In some cases, configs aren't camel cased in the DB, so we have to resort to case insensitive
+            // property name resolving when creating configuration objects (deserializing DB configs).
             PropertyNameCaseInsensitive = true,
             NumberHandling = JsonNumberHandling.AllowReadingFromString,
             Converters =
@@ -26,9 +29,42 @@ public SystemTextConfigurationEditorJsonSerializer()
                 new JsonObjectConverter(),
                 new JsonUdiConverter(),
                 new JsonUdiRangeConverter(),
-                new JsonBooleanConverter()
-            }
+                new JsonBooleanConverter(),
+            },
+
+            // Properties of data type configuration objects are annotated with [ConfigurationField] attributes
+            // that provide the serialized name. Rather than decorating them as well with [JsonPropertyName] attributes
+            // when they differ from the property name, we'll define a custom type info resolver to use the
+            // existing attribute.
+            TypeInfoResolver = new DefaultJsonTypeInfoResolver()
+                .WithAddedModifier(UseAttributeConfiguredPropertyNames()),
         };
 
     protected override JsonSerializerOptions JsonSerializerOptions => _jsonSerializerOptions;
+
+    /// <summary>
+    /// A custom action used to provide property names when they are overridden by
+    /// <see cref="ConfigurationField"/> attributes.
+    /// </summary>
+    /// <remarks>
+    /// Hat-tip: https://stackoverflow.com/a/78063664
+    /// </remarks>
+    private static Action<JsonTypeInfo> UseAttributeConfiguredPropertyNames() => typeInfo =>
+    {
+        if (typeInfo.Kind is not JsonTypeInfoKind.Object)
+        {
+            return;
+        }
+
+        foreach (JsonPropertyInfo property in typeInfo.Properties)
+        {
+            if (property.AttributeProvider?.GetCustomAttributes(typeof(ConfigurationFieldAttribute), true) is { } attributes)
+            {
+                foreach (ConfigurationFieldAttribute attribute in attributes)
+                {
+                    property.Name = attribute.Key;
+                }
+            }
+        }
+    };
 }
diff --git a/tests/Umbraco.Tests.Common/Builders/DataTypeBuilder.cs b/tests/Umbraco.Tests.Common/Builders/DataTypeBuilder.cs
@@ -137,7 +137,7 @@ public override DataType Build()
         var sortOrder = _sortOrder ?? 0;
         var serializer = new SystemTextConfigurationEditorJsonSerializer();
 
-        return new DataType(editor, serializer, parentId)
+        var dataType = new DataType(editor, serializer, parentId)
         {
             Id = id,
             Key = key,
@@ -152,5 +152,7 @@ public override DataType Build()
             DatabaseType = databaseType,
             SortOrder = sortOrder
         };
+
+        return dataType;
     }
 }
diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/PropertyEditorValueConverterTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/PropertyEditorValueConverterTests.cs
@@ -9,6 +9,7 @@
 using Umbraco.Cms.Core.PropertyEditors.ValueConverters;
 using Umbraco.Cms.Core.Strings;
 using Umbraco.Cms.Infrastructure.Serialization;
+using Umbraco.Cms.Tests.Common.Builders;
 
 namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.PropertyEditors;
 
@@ -46,30 +47,54 @@ public void CanConvertDatePickerPropertyEditor(string date, bool expected)
         }
     }
 
-    [TestCase("TRUE", true)]
-    [TestCase("True", true)]
-    [TestCase("true", true)]
-    [TestCase("1", true)]
-    [TestCase(1, true)]
-    [TestCase(true, true)]
-    [TestCase("FALSE", false)]
-    [TestCase("False", false)]
-    [TestCase("false", false)]
-    [TestCase("0", false)]
-    [TestCase(0, false)]
-    [TestCase(false, false)]
-    [TestCase("", false)]
-    [TestCase(null, false)]
-    [TestCase("blah", false)]
-    public void CanConvertYesNoPropertyEditor(object value, bool expected)
+    [TestCase("TRUE", null, true)]
+    [TestCase("True", null, true)]
+    [TestCase("true", null, true)]
+    [TestCase("1", null, true)]
+    [TestCase(1, null, true)]
+    [TestCase(true, null, true)]
+    [TestCase("FALSE", null, false)]
+    [TestCase("False", null, false)]
+    [TestCase("false", null, false)]
+    [TestCase("0", null, false)]
+    [TestCase(0, null, false)]
+    [TestCase(false, null, false)]
+    [TestCase("", null, false)]
+    [TestCase("blah", null, false)]
+    [TestCase(null, false, false)]
+    [TestCase(null, true, true)]
+    public void CanConvertTrueFalsePropertyEditor(object value, bool initialStateConfigurationValue, bool expected)
     {
+        var publishedDataType = CreatePublishedDataType(initialStateConfigurationValue);
+
+        var publishedPropertyTypeMock = new Mock<IPublishedPropertyType>();
+        publishedPropertyTypeMock
+            .SetupGet(p => p.DataType)
+            .Returns(publishedDataType);
+
         var converter = new YesNoValueConverter();
-        var result =
-            converter.ConvertSourceToIntermediate(null, null, value, false); // does not use type for conversion
+        var intermediateResult = converter.ConvertSourceToIntermediate(null, publishedPropertyTypeMock.Object, value, false);
+        var result = converter.ConvertIntermediateToObject(null, publishedPropertyTypeMock.Object, PropertyCacheLevel.Element, intermediateResult, false);
 
         Assert.AreEqual(expected, result);
     }
 
+    private static PublishedDataType CreatePublishedDataType(bool initialStateConfigurationValue)
+    {
+        var dataTypeConfiguration = new TrueFalseConfiguration
+        {
+            InitialState = initialStateConfigurationValue
+        };
+
+        var dateTypeMock = new Mock<IDataType>();
+        dateTypeMock.SetupGet(x => x.Id).Returns(1000);
+        dateTypeMock.SetupGet(x => x.EditorAlias).Returns(global::Umbraco.Cms.Core.Constants.PropertyEditors.Aliases.Boolean);
+        dateTypeMock.SetupGet(x => x.EditorUiAlias).Returns("Umb.PropertyEditorUi.Toggle");
+        dateTypeMock.SetupGet(x => x.ConfigurationObject).Returns(dataTypeConfiguration);
+
+        return new PublishedDataType(dateTypeMock.Object.Id, dateTypeMock.Object.EditorAlias, dateTypeMock.Object.EditorUiAlias, new Lazy<object>(() => dataTypeConfiguration));
+    }
+
     [TestCase("[\"apples\"]", new[] { "apples" })]
     [TestCase("[\"apples\",\"oranges\"]", new[] { "apples", "oranges" })]
     [TestCase("[\"apples\",\"oranges\",\"pears\"]", new[] { "apples", "oranges", "pears" })]

Original file line number	Diff line number	Diff line change
`@@ -14,8 +14,13 @@ public override Type GetPropertyValueType(IPublishedPropertyType propertyType)`
`14`	`14`	`public override PropertyCacheLevel GetPropertyCacheLevel(IPublishedPropertyType propertyType)`
`15`	`15`	`=> PropertyCacheLevel.Element;`
`16`	`16`
`17`		`- public override object ConvertSourceToIntermediate(IPublishedElement owner, IPublishedPropertyType propertyType, object? source, bool preview)`
	`17`	`+ public override object? ConvertSourceToIntermediate(IPublishedElement owner, IPublishedPropertyType propertyType, object? source, bool preview)`
`18`	`18`	`{`
	`19`	`+ if (source is null)`
	`20`	`+ {`
	`21`	`+ return null;`
	`22`	`+ }`
	`23`	`+`
`19`	`24`	`// in xml a boolean is: string`
`20`	`25`	`// in the database a boolean is: string "1" or "0" or empty`
`21`	`26`	`// typically the converter does not need to handle anything else ("true"...)`
`@@ -35,23 +40,36 @@ public override object ConvertSourceToIntermediate(IPublishedElement owner, IPub`
`35`	`40`	`return bool.TryParse(s, out var result) && result;`
`36`	`41`	`}`
`37`	`42`
`38`		`- if (source is int)`
	`43`	`+ if (source is int sourceAsInt)`
`39`	`44`	`{`
`40`		`- return (int)source == 1;`
	`45`	`+ return sourceAsInt == 1;`
`41`	`46`	`}`
`42`	`47`
`43`	`48`	`// this is required for correct true/false handling in nested content elements`
`44`		`- if (source is long)`
	`49`	`+ if (source is long sourceAsLong)`
`45`	`50`	`{`
`46`		`- return (long)source == 1;`
	`51`	`+ return sourceAsLong == 1;`
`47`	`52`	`}`
`48`	`53`
`49`		`- if (source is bool)`
	`54`	`+ if (source is bool sourceAsBoolean)`
`50`	`55`	`{`
`51`		`- return (bool)source;`
	`56`	`+ return sourceAsBoolean;`
`52`	`57`	`}`
`53`	`58`
`54`		`- // default value is: false`
	`59`	`+ // false for any other value`
`55`	`60`	`return false;`
`56`	`61`	`}`
	`62`	`+`
	`63`	`+ /// <inheritdoc />`
	`64`	`+ public override object? ConvertIntermediateToObject(IPublishedElement owner, IPublishedPropertyType propertyType, PropertyCacheLevel cacheLevel, object? source, bool preview)`
	`65`	`+ {`
	`66`	`+ // If source is null, whether we return true or false depends on the configured default value (initial state).`
	`67`	`+ if (source is null)`
	`68`	`+ {`
	`69`	`+ TrueFalseConfiguration? configuration = propertyType.DataType.ConfigurationAs<TrueFalseConfiguration>();`
	`70`	`+ return configuration?.InitialState ?? false;`
	`71`	`+ }`
	`72`	`+`
	`73`	`+ return (bool)source;`
	`74`	`+ }`
`57`	`75`	`}`
Original file line number	Diff line number	Diff line change
`@@ -137,7 +137,7 @@ public override DataType Build()`
`137`	`137`	`var sortOrder = _sortOrder ?? 0;`
`138`	`138`	`var serializer = new SystemTextConfigurationEditorJsonSerializer();`
`139`	`139`
`140`		`- return new DataType(editor, serializer, parentId)`
	`140`	`+ var dataType = new DataType(editor, serializer, parentId)`
`141`	`141`	`{`
`142`	`142`	`Id = id,`
`143`	`143`	`Key = key,`
`@@ -152,5 +152,7 @@ public override DataType Build()`
`152`	`152`	`DatabaseType = databaseType,`
`153`	`153`	`SortOrder = sortOrder`
`154`	`154`	`};`
	`155`	`+`
	`156`	`+ return dataType;`
`155`	`157`	`}`
`156`	`158`	`}`