Clean up leftover block item data when changing element variance (#18804)

src/Umbraco.Infrastructure/PropertyEditors/BlockValuePropertyValueEditorBase.cs

@@ -184,6 +184,12 @@ public abstract class BlockValuePropertyValueEditorBase<TValue, TLayout> : DataV
 
         foreach (BlockItemData item in items)
         {
+            // if changes were made to the element type variations, we need those changes reflected in the block property values.
+            // for regular content this happens when a content type is saved (copies of property values are created in the DB),
+            // but for local block level properties we don't have that kind of handling, so we to do it manually.
+            // to be friendly we'll map "formerly invariant properties" to the default language ISO code instead of performing a
+            // hard reset of the property values (which would likely be the most correct thing to do from a data point of view).
+            item.Values = _blockEditorVarianceHandler.AlignPropertyVarianceAsync(item.Values, culture).GetAwaiter().GetResult();
             foreach (BlockPropertyValue blockPropertyValue in item.Values)
             {
                 IPropertyType? propertyType = blockPropertyValue.PropertyType;
@@ -199,13 +205,6 @@ public abstract class BlockValuePropertyValueEditorBase<TValue, TLayout> : DataV
                     continue;
                 }
 
-                // if changes were made to the element type variation, we need those changes reflected in the block property values.
-                // for regular content this happens when a content type is saved (copies of property values are created in the DB),
-                // but for local block level properties we don't have that kind of handling, so we to do it manually.
-                // to be friendly we'll map "formerly invariant properties" to the default language ISO code instead of performing a
-                // hard reset of the property values (which would likely be the most correct thing to do from a data point of view).
-                _blockEditorVarianceHandler.AlignPropertyVarianceAsync(blockPropertyValue, propertyType, culture).GetAwaiter().GetResult();
-
                 if (!valueEditorsByKey.TryGetValue(propertyType.DataTypeKey, out IDataValueEditor? valueEditor))
                 {
                     var configuration = _dataTypeConfigurationCache.GetConfiguration(propertyType.DataTypeKey);

src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/BlockEditorVarianceHandler.cs

@@ -25,15 +25,7 @@ public sealed class BlockEditorVarianceHandler
         _contentTypeService = contentTypeService;
     }
 
-    /// <summary>
-    /// Aligns a block property value for variance changes.
-    /// </summary>
-    /// <param name="blockPropertyValue">The block property value to align.</param>
-    /// <param name="propertyType">The underlying property type.</param>
-    /// <param name="culture">The culture being handled (null if invariant).</param>
-    /// <remarks>
-    /// Used for aligning variance changes when editing content.
-    /// </remarks>
+    [Obsolete("Please use the method that allows alignment for a collection of values. Scheduled for removal in V17.")]
     public async Task AlignPropertyVarianceAsync(BlockPropertyValue blockPropertyValue, IPropertyType propertyType, string? culture)
     {
         culture ??= await _languageService.GetDefaultIsoCodeAsync();
@@ -45,6 +37,48 @@ public sealed class BlockEditorVarianceHandler
         }
     }
 
+    /// <summary>
+    /// Aligns a collection of block property values for variance changes.
+    /// </summary>
+    /// <param name="blockPropertyValues">The block property values to align.</param>
+    /// <param name="culture">The culture being handled (null if invariant).</param>
+    /// <remarks>
+    /// Used for aligning variance changes when editing content.
+    /// </remarks>
+    public async Task<IList<BlockPropertyValue>> AlignPropertyVarianceAsync(IList<BlockPropertyValue> blockPropertyValues, string? culture)
+    {
+        var defaultIsoCodeAsync = await _languageService.GetDefaultIsoCodeAsync();
+        culture ??= defaultIsoCodeAsync;
+
+        var valuesToRemove = new List<BlockPropertyValue>();
+        foreach (BlockPropertyValue blockPropertyValue in blockPropertyValues)
+        {
+            IPropertyType? propertyType = blockPropertyValue.PropertyType;
+            if (propertyType is null)
+            {
+                throw new ArgumentException("One or more block properties did not have a resolved property type. Block editor values must be resolved before attempting to map them to editor.", nameof(blockPropertyValues));
+            }
+
+            if (propertyType.VariesByCulture() == VariesByCulture(blockPropertyValue))
+            {
+                continue;
+            }
+
+            if (propertyType.VariesByCulture() is false && blockPropertyValue.Culture.InvariantEquals(defaultIsoCodeAsync) is false)
+            {
+                valuesToRemove.Add(blockPropertyValue);
+            }
+            else
+            {
+                blockPropertyValue.Culture = propertyType.VariesByCulture()
+                    ? culture
+                    : null;
+            }
+        }
+
+        return blockPropertyValues.Except(valuesToRemove).ToList();
+    }
+
     /// <summary>
     /// Aligns a block property value for variance changes.
     /// </summary>
@@ -199,6 +233,8 @@ public sealed class BlockEditorVarianceHandler
                 blockValue.Expose.Add(new BlockItemVariation(contentData.Key, value.Culture, value.Segment));
             }
         }
+
+        blockValue.Expose = blockValue.Expose.DistinctBy(e => $"{e.ContentKey}.{e.Culture}.{e.Segment}").ToList();
     }
 
     private static bool VariesByCulture(BlockPropertyValue blockPropertyValue)

tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/PropertyEditors/BlockListElementLevelVariationTests.Editing.cs

@@ -1,11 +1,10 @@
-using Microsoft.Extensions.DependencyInjection;
-using NUnit.Framework;
+using NUnit.Framework;
 using Umbraco.Cms.Core;
-using Umbraco.Cms.Core.Configuration.Models;
 using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.Models.Blocks;
 using Umbraco.Cms.Core.Models.ContentEditing;
 using Umbraco.Cms.Core.Models.Membership;
+using Umbraco.Cms.Core.PropertyEditors;
 using Umbraco.Cms.Core.Services;
 using Umbraco.Cms.Tests.Common.Builders;
 using Umbraco.Cms.Tests.Common.Builders.Extensions;
@@ -742,6 +741,185 @@ internal partial class BlockListElementLevelVariationTests
         }
     }
 
+    [Test]
+    public async Task Can_Align_Culture_Variance_For_Variant_Element_Types()
+    {
+        var elementType = CreateElementType(ContentVariation.Culture);
+        var blockListDataType = await CreateBlockListDataType(elementType);
+        var contentType = CreateContentType(ContentVariation.Nothing, blockListDataType);
+
+        var content = CreateContent(
+            contentType,
+            elementType,
+            new List<BlockPropertyValue>
+            {
+                new() { Alias = "invariantText", Value = "The invariant content value" },
+                new() { Alias = "variantText", Value = "Another invariant content value" }
+            },
+            new List<BlockPropertyValue>
+            {
+                new() { Alias = "invariantText", Value = "The invariant settings value" },
+                new() { Alias = "variantText", Value = "Another invariant settings value" }
+            },
+            false);
+
+        contentType.Variations = ContentVariation.Culture;
+        ContentTypeService.Save(contentType);
+
+        // re-fetch content
+        content = ContentService.GetById(content.Key);
+
+        var valueEditor = (BlockListPropertyEditorBase.BlockListEditorPropertyValueEditor)blockListDataType.Editor!.GetValueEditor();
+
+        var blockListValue = valueEditor.ToEditor(content!.Properties["blocks"]!) as BlockListValue;
+        Assert.IsNotNull(blockListValue);
+        Assert.Multiple(() =>
+        {
+            Assert.AreEqual(1, blockListValue.ContentData.Count);
+            Assert.AreEqual(2, blockListValue.ContentData.First().Values.Count);
+            var invariantValue = blockListValue.ContentData.First().Values.First(value => value.Alias == "invariantText");
+            var variantValue = blockListValue.ContentData.First().Values.First(value => value.Alias == "variantText");
+            Assert.IsNull(invariantValue.Culture);
+            Assert.AreEqual("en-US", variantValue.Culture);
+        });
+        Assert.Multiple(() =>
+        {
+            Assert.AreEqual(1, blockListValue.SettingsData.Count);
+            Assert.AreEqual(2, blockListValue.SettingsData.First().Values.Count);
+            var invariantValue = blockListValue.SettingsData.First().Values.First(value => value.Alias == "invariantText");
+            var variantValue = blockListValue.SettingsData.First().Values.First(value => value.Alias == "variantText");
+            Assert.IsNull(invariantValue.Culture);
+            Assert.AreEqual("en-US", variantValue.Culture);
+        });
+        Assert.Multiple(() =>
+        {
+            Assert.AreEqual(1, blockListValue.Expose.Count);
+            Assert.AreEqual("en-US", blockListValue.Expose.First().Culture);
+        });
+    }
+
+    [TestCase(ContentVariation.Culture)]
+    [TestCase(ContentVariation.Nothing)]
+    public async Task Can_Turn_Invariant_Element_Variant(ContentVariation contentTypeVariation)
+    {
+        var elementType = CreateElementType(ContentVariation.Nothing);
+        var blockListDataType = await CreateBlockListDataType(elementType);
+        var contentType = CreateContentType(contentTypeVariation, blockListDataType);
+
+        var content = CreateContent(
+            contentType,
+            elementType,
+            new List<BlockPropertyValue>
+            {
+                new() { Alias = "invariantText", Value = "The invariant content value" },
+                new() { Alias = "variantText", Value = "Another invariant content value" }
+            },
+            new List<BlockPropertyValue>
+            {
+                new() { Alias = "invariantText", Value = "The invariant settings value" },
+                new() { Alias = "variantText", Value = "Another invariant settings value" }
+            },
+            false);
+
+        elementType.Variations = ContentVariation.Culture;
+        elementType.PropertyTypes.First(p => p.Alias == "variantText").Variations = ContentVariation.Culture;
+        ContentTypeService.Save(elementType);
+
+        // re-fetch content
+        content = ContentService.GetById(content.Key);
+
+        var valueEditor = (BlockListPropertyEditorBase.BlockListEditorPropertyValueEditor)blockListDataType.Editor!.GetValueEditor();
+
+        var blockListValue = valueEditor.ToEditor(content!.Properties["blocks"]!) as BlockListValue;
+        Assert.IsNotNull(blockListValue);
+        Assert.Multiple(() =>
+        {
+            Assert.AreEqual(1, blockListValue.ContentData.Count);
+            Assert.AreEqual(2, blockListValue.ContentData.First().Values.Count);
+            var invariantValue = blockListValue.ContentData.First().Values.First(value => value.Alias == "invariantText");
+            var variantValue = blockListValue.ContentData.First().Values.First(value => value.Alias == "variantText");
+            Assert.IsNull(invariantValue.Culture);
+            Assert.AreEqual("en-US", variantValue.Culture);
+        });
+        Assert.Multiple(() =>
+        {
+            Assert.AreEqual(1, blockListValue.SettingsData.Count);
+            Assert.AreEqual(2, blockListValue.SettingsData.First().Values.Count);
+            var invariantValue = blockListValue.SettingsData.First().Values.First(value => value.Alias == "invariantText");
+            var variantValue = blockListValue.SettingsData.First().Values.First(value => value.Alias == "variantText");
+            Assert.IsNull(invariantValue.Culture);
+            Assert.AreEqual("en-US", variantValue.Culture);
+        });
+        Assert.Multiple(() =>
+        {
+            Assert.AreEqual(1, blockListValue.Expose.Count);
+            Assert.AreEqual("en-US", blockListValue.Expose.First().Culture);
+        });
+    }
+
+    [TestCase(ContentVariation.Nothing)]
+    [TestCase(ContentVariation.Culture)]
+    public async Task Can_Turn_Variant_Element_Invariant(ContentVariation contentTypeVariation)
+    {
+        var elementType = CreateElementType(ContentVariation.Culture);
+        var blockListDataType = await CreateBlockListDataType(elementType);
+        var contentType = CreateContentType(contentTypeVariation, blockListDataType);
+
+        var content = CreateContent(
+            contentType,
+            elementType,
+            new List<BlockPropertyValue>
+            {
+                new() { Alias = "invariantText", Value = "The invariant content value" },
+                new() { Alias = "variantText", Value = "Variant content in English", Culture = "en-US" },
+                new() { Alias = "variantText", Value = "Variant content in Danish", Culture = "da-DK" }
+            },
+            new List<BlockPropertyValue>
+            {
+                new() { Alias = "invariantText", Value = "The invariant settings value" },
+                new() { Alias = "variantText", Value = "Variant settings in English", Culture = "en-US" },
+                new() { Alias = "variantText", Value = "Variant settings in Danish", Culture = "da-DK" }
+            },
+            false);
+
+        elementType.Variations = ContentVariation.Nothing;
+        elementType.PropertyTypes.First(p => p.Alias == "variantText").Variations = ContentVariation.Nothing;
+        ContentTypeService.Save(elementType);
+
+        // re-fetch content
+        content = ContentService.GetById(content.Key);
+
+        var valueEditor = (BlockListPropertyEditorBase.BlockListEditorPropertyValueEditor)blockListDataType.Editor!.GetValueEditor();
+
+        var blockListValue = valueEditor.ToEditor(content!.Properties["blocks"]!) as BlockListValue;
+        Assert.IsNotNull(blockListValue);
+        Assert.Multiple(() =>
+        {
+            Assert.AreEqual(1, blockListValue.ContentData.Count);
+            Assert.AreEqual(2, blockListValue.ContentData.First().Values.Count);
+            var invariantValue = blockListValue.ContentData.First().Values.First(value => value.Alias == "invariantText");
+            var variantValue = blockListValue.ContentData.First().Values.First(value => value.Alias == "variantText");
+            Assert.IsNull(invariantValue.Culture);
+            Assert.IsNull(variantValue.Culture);
+            Assert.AreEqual("Variant content in English", variantValue.Value);
+        });
+        Assert.Multiple(() =>
+        {
+            Assert.AreEqual(1, blockListValue.SettingsData.Count);
+            Assert.AreEqual(2, blockListValue.SettingsData.First().Values.Count);
+            var invariantValue = blockListValue.SettingsData.First().Values.First(value => value.Alias == "invariantText");
+            var variantValue = blockListValue.SettingsData.First().Values.First(value => value.Alias == "variantText");
+            Assert.IsNull(invariantValue.Culture);
+            Assert.IsNull(variantValue.Culture);
+            Assert.AreEqual("Variant settings in English", variantValue.Value);
+        });
+        Assert.Multiple(() =>
+        {
+            Assert.AreEqual(1, blockListValue.Expose.Count);
+            Assert.IsNull(blockListValue.Expose.First().Culture);
+        });
+    }
+
     private async Task<IUser> CreateLimitedUser()
     {
         var userGroupService = GetRequiredService<IUserGroupService>();