From c86a6fa8e514132b49c492821b32aa71c19b0333 Mon Sep 17 00:00:00 2001 From: Sven Geusens Date: Mon, 21 Apr 2025 10:30:12 +0200 Subject: [PATCH] V15/fix/sub variant block deletion (#18802) * Fix * Editors with access should be able to clear a blocklist value * Writeup around block element level variation * Dissallow values to be removed a limited language user does not have permissions to * Remove commented out code * improved comments * Improve expose list for limited language access sub variant block lists --- .../BlockValuePropertyValueEditorBase.cs | 96 ++- ...kListElementLevelVariationTests.Editing.cs | 627 +++++++++++++++++- .../block-element-level-variation.md | 67 ++ 3 files changed, 757 insertions(+), 33 deletions(-) create mode 100644 tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/PropertyEditors/block-element-level-variation.md diff --git a/src/Umbraco.Infrastructure/PropertyEditors/BlockValuePropertyValueEditorBase.cs b/src/Umbraco.Infrastructure/PropertyEditors/BlockValuePropertyValueEditorBase.cs index d2a5d0695b..dfdb55fc63 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/BlockValuePropertyValueEditorBase.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/BlockValuePropertyValueEditorBase.cs @@ -10,6 +10,7 @@ using Umbraco.Cms.Core.PropertyEditors.ValueConverters; using Umbraco.Cms.Core.Serialization; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Strings; +using Umbraco.Extensions; namespace Umbraco.Cms.Core.PropertyEditors; @@ -291,6 +292,11 @@ public abstract class BlockValuePropertyValueEditorBase : DataV bool canUpdateInvariantData, HashSet allowedCultures) { + if (canUpdateInvariantData is false && targetValue is null) + { + return sourceValue; + } + BlockEditorData? source = BlockEditorValues.DeserializeAndClean(sourceValue); BlockEditorData? target = BlockEditorValues.DeserializeAndClean(targetValue); @@ -310,31 +316,29 @@ public abstract class BlockValuePropertyValueEditorBase : DataV bool canUpdateInvariantData, HashSet allowedCultures) { - source = UpdateSourceInvariantData(source, target, canUpdateInvariantData); + var mergedInvariant = UpdateSourceInvariantData(source, target, canUpdateInvariantData); - if (source is null && target is null) + // if the structure (invariant) is not defined after merger, the target content does not matter + if (mergedInvariant is null) { return null; } - if (source is null && target?.Layout is not null) + // since we merged the invariant data (layout) before we get to this point + // we just need an empty valid object to run comparisons at this point + if (source is null) { - source = new BlockEditorData([], CreateWithLayout(target.Layout)); - } - else if (target is null && source?.Layout is not null) - { - target = new BlockEditorData([], CreateWithLayout(source.Layout)); + source = new BlockEditorData([], new TValue()); } - // at this point the layout should have been merged or fallback created - if (source is null || target is null) - { - throw new ArgumentException("invalid sourceValue or targetValue"); - } + // update the target with the merged invariant + target!.BlockValue.Layout = mergedInvariant.BlockValue.Layout; // remove all the blocks that are no longer part of the layout target.BlockValue.ContentData.RemoveAll(contentBlock => target.Layout!.Any(layoutItem => layoutItem.ReferencesContent(contentBlock.Key)) is false); + // remove any exposes that no longer have content assigned to them + target.BlockValue.Expose.RemoveAll(expose => target.BlockValue.ContentData.Any(data => data.Key == expose.ContentKey) is false); target.BlockValue.SettingsData.RemoveAll(settingsBlock => target.Layout!.Any(layoutItem => layoutItem.ReferencesSetting(settingsBlock.Key)) is false); @@ -342,16 +346,80 @@ public abstract class BlockValuePropertyValueEditorBase : DataV CleanupVariantValues(source.BlockValue.ContentData, target.BlockValue.ContentData, canUpdateInvariantData, allowedCultures); CleanupVariantValues(source.BlockValue.SettingsData, target.BlockValue.SettingsData, canUpdateInvariantData, allowedCultures); + // every source block value for a culture that is not allowed to be edited should be present on the target + RestoreMissingValues( + source.BlockValue.ContentData, + target.BlockValue.ContentData, + mergedInvariant.Layout!, + (layoutItem, itemData) => layoutItem.ContentKey == itemData.Key, + canUpdateInvariantData, + allowedCultures); + RestoreMissingValues( + source.BlockValue.SettingsData, + target.BlockValue.SettingsData, + mergedInvariant.Layout!, + (layoutItem, itemData) => layoutItem.SettingsKey == itemData.Key, + canUpdateInvariantData, + allowedCultures); + + // update the expose list from source for any blocks that were restored + var missingSourceExposes = + source.BlockValue.Expose.Where(sourceExpose => + target.BlockValue.Expose.Any(targetExpose => targetExpose.ContentKey == sourceExpose.ContentKey) is false + && target.BlockValue.ContentData.Any(data => data.Key == sourceExpose.ContentKey)).ToList(); + foreach (BlockItemVariation missingSourceExpose in missingSourceExposes) + { + target.BlockValue.Expose.Add(missingSourceExpose); + } + return target.BlockValue; } + private void RestoreMissingValues( + List sourceBlockItemData, + List targetBlockItemData, + IEnumerable mergedLayout, + Func relevantBlockItemMatcher, + bool canUpdateInvariantData, + HashSet allowedCultures) + { + IEnumerable blockItemsToCheck = sourceBlockItemData.Where(itemData => + mergedLayout.Any(layoutItem => relevantBlockItemMatcher(layoutItem, itemData))); + foreach (BlockItemData blockItemData in blockItemsToCheck) + { + var relevantValues = blockItemData.Values.Where(value => + (value.Culture is null && canUpdateInvariantData is false) + || (value.Culture is not null && allowedCultures.Contains(value.Culture) is false)).ToList(); + if (relevantValues.Count < 1) + { + continue; + } + + BlockItemData targetBlockData = + targetBlockItemData.FirstOrDefault(itemData => itemData.Key == blockItemData.Key) + ?? new BlockItemData(blockItemData.Key, blockItemData.ContentTypeKey, blockItemData.ContentTypeAlias); + foreach (BlockPropertyValue missingValue in relevantValues.Where(value => targetBlockData.Values.Any(targetValue => + targetValue.Alias == value.Alias + && targetValue.Culture == value.Culture + && targetValue.Segment == value.Segment) is false)) + { + targetBlockData.Values.Add(missingValue); + } + + if (targetBlockItemData.Any(existingBlockItemData => existingBlockItemData.Key == targetBlockData.Key) is false) + { + targetBlockItemData.Add(blockItemData); + } + } + } + private void CleanupVariantValues( List sourceBlockItems, List targetBlockItems, bool canUpdateInvariantData, HashSet allowedCultures) { - // merge the source values into the target values for culture + // merge the source values into the target values per culture foreach (BlockItemData targetBlockItem in targetBlockItems) { BlockItemData? sourceBlockItem = sourceBlockItems.FirstOrDefault(i => i.Key == targetBlockItem.Key); diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/PropertyEditors/BlockListElementLevelVariationTests.Editing.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/PropertyEditors/BlockListElementLevelVariationTests.Editing.cs index 8af4cf4987..d7e8ffc5cb 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/PropertyEditors/BlockListElementLevelVariationTests.Editing.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/PropertyEditors/BlockListElementLevelVariationTests.Editing.cs @@ -14,6 +14,10 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.PropertyEditors; internal partial class BlockListElementLevelVariationTests { + /// + /// Tests whether the user can update the variant values of existing blocks inside an invariant blocklist + /// + /// true => danish only which is not the default. false => admin which is all languages [TestCase(true)] [TestCase(false)] [ConfigureBuilder(ActionName = nameof(ConfigureAllowEditInvariantFromNonDefaultTrue))] @@ -169,6 +173,10 @@ internal partial class BlockListElementLevelVariationTests } } + /// + /// Tests whether the user can add new variant blocks to an invariant blocklist + /// + /// true => danish only which is not the default. false => admin which is all languages [TestCase(true)] [TestCase(false)] [ConfigureBuilder(ActionName = nameof(ConfigureAllowEditInvariantFromNonDefaultTrue))] @@ -299,6 +307,10 @@ internal partial class BlockListElementLevelVariationTests } } + /// + /// Tests whether the user can update the variant values of existing blocks inside an invariant blocklist + /// + /// true => danish only which is not the default. false => admin which is all languages [TestCase(true)] [TestCase(false)] public async Task Can_Handle_Limited_User_Access_To_Languages_Without_AllowEditInvariantFromNonDefault(bool updateWithLimitedUserAccess) @@ -457,6 +469,10 @@ internal partial class BlockListElementLevelVariationTests } } + /// + /// Tests whether the user can add new variant blocks to an invariant blocklist + /// + /// true => danish only which is not the default. false => admin which is all languages [TestCase(true)] [TestCase(false)] public async Task Can_Handle_Limited_User_Access_To_Languages_Without_AllowEditInvariantFromNonDefault_WithoutInitialValue(bool updateWithLimitedUserAccess) @@ -524,14 +540,14 @@ internal partial class BlockListElementLevelVariationTests { InvariantProperties = new[] { - new PropertyValueModel { Alias = "blocks", Value = JsonSerializer.Serialize(blockListValue) } + new PropertyValueModel { Alias = "blocks", Value = JsonSerializer.Serialize(blockListValue) }, }, Variants = new[] { new VariantModel { Name = content.GetCultureName("en-US")!, Culture = "en-US", Properties = [] }, new VariantModel { Name = content.GetCultureName("da-DK")!, Culture = "da-DK", Properties = [] }, - new VariantModel { Name = content.GetCultureName("de-DE")!, Culture = "de-DE", Properties = [] } - } + new VariantModel { Name = content.GetCultureName("de-DE")!, Culture = "de-DE", Properties = [] }, + }, }; var result = await ContentEditingService.UpdateAsync(content.Key, updateModel, userKey); @@ -542,24 +558,10 @@ internal partial class BlockListElementLevelVariationTests Assert.NotNull(savedBlocksValue); blockListValue = JsonSerializer.Deserialize(savedBlocksValue); - // the Danish values should be updated regardless of the executing user - Assert.Multiple(() => - { - Assert.AreEqual("#1: The second content value in Danish", blockListValue.ContentData[0].Values.Single(v => v.Culture == "da-DK").Value); - Assert.AreEqual("#1: The second settings value in Danish", blockListValue.SettingsData[0].Values.Single(v => v.Culture == "da-DK").Value); - - Assert.AreEqual("#2: The second content value in Danish", blockListValue.ContentData[1].Values.Single(v => v.Culture == "da-DK").Value); - Assert.AreEqual("#2: The second settings value in Danish", blockListValue.SettingsData[1].Values.Single(v => v.Culture == "da-DK").Value); - }); - - // limited user access means invariant, English and German should not have been updated - changes should be rolled back to the initial block values + // limited user access means invariant data is inaccessible since AllowEditInvariantFromNonDefault is disabled if (updateWithLimitedUserAccess) { - Assert.Multiple(() => - { - Assert.AreEqual(1, blockListValue.ContentData[0].Values.Count); - Assert.AreEqual(1, blockListValue.ContentData[1].Values.Count); - }); + Assert.IsNull(blockListValue); } else { @@ -581,6 +583,542 @@ internal partial class BlockListElementLevelVariationTests Assert.AreEqual("#2: The second settings value in English", blockListValue.SettingsData[1].Values[1].Value); Assert.AreEqual("#2: The second content value in German", blockListValue.ContentData[1].Values[3].Value); Assert.AreEqual("#2: The second settings value in German", blockListValue.SettingsData[1].Values[3].Value); + + Assert.AreEqual("#1: The second content value in Danish", blockListValue.ContentData[0].Values.Single(v => v.Culture == "da-DK").Value); + Assert.AreEqual("#1: The second settings value in Danish", blockListValue.SettingsData[0].Values.Single(v => v.Culture == "da-DK").Value); + + Assert.AreEqual("#2: The second content value in Danish", blockListValue.ContentData[1].Values.Single(v => v.Culture == "da-DK").Value); + Assert.AreEqual("#2: The second settings value in Danish", blockListValue.SettingsData[1].Values.Single(v => v.Culture == "da-DK").Value); + }); + } + } + + /// + /// Tests whether the user can add/remove new variant blocks to an invariant blocklist + /// + /// true => danish only which is not the default. false => admin which is all languages + [TestCase(true)] + [TestCase(false)] + public async Task Can_Handle_BlockStructureManipulation_For_Limited_Users_Without_AllowEditInvariantFromNonDefault( + bool updateWithLimitedUserAccess) + { + await LanguageService.CreateAsync( + new Language("de-DE", "German"), Constants.Security.SuperUserKey); + var userKey = updateWithLimitedUserAccess + ? (await CreateLimitedUser()).Key + : Constants.Security.SuperUserKey; + + var elementType = CreateElementType(ContentVariation.Culture); + var blockListDataType = await CreateBlockListDataType(elementType); + var contentType = CreateContentType(ContentVariation.Culture, blockListDataType); + var content = CreateContent(contentType, elementType, [], false); + content.SetCultureName("Home (de)", "de-DE"); + ContentService.Save(content); + + var firstContentElementKey = Guid.NewGuid(); + var firstSettingsElementKey = Guid.NewGuid(); + + var secondContentElementKey = Guid.NewGuid(); + var secondSettingsElementKey = Guid.NewGuid(); + + var blockListValue = BlockListPropertyValue( + elementType, + [ + ( + firstContentElementKey, + firstSettingsElementKey, + new BlockProperty( + new List { + new() { Alias = "invariantText", Value = "#1: The first invariant content value" }, + new() { Alias = "variantText", Value = "#1: The first content value in English", Culture = "en-US" }, + new() { Alias = "variantText", Value = "#1: The first content value in Danish", Culture = "da-DK" }, + new() { Alias = "variantText", Value = "#1: The first content value in German", Culture = "de-DE" }, + }, + new List { + new() { Alias = "invariantText", Value = "#1: The first invariant settings value" }, + new() { Alias = "variantText", Value = "#1: The first settings value in English", Culture = "en-US" }, + new() { Alias = "variantText", Value = "#1: The first settings value in Danish", Culture = "da-DK" }, + new() { Alias = "variantText", Value = "#1: The first settings value in German", Culture = "de-DE" }, + }, + null, + null)), + ( + secondContentElementKey, + secondSettingsElementKey, + new BlockProperty( + new List { + new() { Alias = "invariantText", Value = "#2: The first invariant content value" }, + new() { Alias = "variantText", Value = "#2: The first content value in English", Culture = "en-US" }, + new() { Alias = "variantText", Value = "#2: The first content value in Danish", Culture = "da-DK" }, + new() { Alias = "variantText", Value = "#2: The first content value in German", Culture = "de-DE" }, + }, + new List { + new() { Alias = "invariantText", Value = "#2: The first invariant settings value" }, + new() { Alias = "variantText", Value = "#2: The first settings value in English", Culture = "en-US" }, + new() { Alias = "variantText", Value = "#2: The first settings value in Danish", Culture = "da-DK" }, + new() { Alias = "variantText", Value = "#2: The first settings value in German", Culture = "de-DE" }, + }, + null, + null)) + ]); + + content.Properties["blocks"]!.SetValue(JsonSerializer.Serialize(blockListValue)); + ContentService.Save(content); + + var newContentElementKey = Guid.NewGuid(); + RemoveBlock(blockListValue, firstContentElementKey); + AddBlock( + blockListValue, + new BlockItemData + { + Key = newContentElementKey, + ContentTypeAlias = elementType.Alias, + ContentTypeKey = elementType.Key, + Values = new List { + new() { Alias = "invariantText", Value = "#new: The new invariant settings value" }, + new() { Alias = "variantText", Value = "#new: The new settings value in English", Culture = "en-US" }, + new() { Alias = "variantText", Value = "#new: The new settings value in Danish", Culture = "da-DK" }, + new() { Alias = "variantText", Value = "#new: The new settings value in German", Culture = "de-DE" }, + }, + }, + null, + elementType); + + var updateModel = new ContentUpdateModel + { + InvariantProperties = new[] + { + new PropertyValueModel { Alias = "blocks", Value = JsonSerializer.Serialize(blockListValue) } + }, + Variants = new[] + { + new VariantModel { Name = content.GetCultureName("en-US")!, Culture = "en-US", Properties = [] }, + new VariantModel { Name = content.GetCultureName("da-DK")!, Culture = "da-DK", Properties = [] }, + new VariantModel { Name = content.GetCultureName("de-DE")!, Culture = "de-DE", Properties = [] } + }, + }; + + var result = await ContentEditingService.UpdateAsync(content.Key, updateModel, userKey); + Assert.IsTrue(result.Success); + + content = ContentService.GetById(content.Key); + var savedBlocksValue = content?.Properties["blocks"]?.GetValue()?.ToString(); + Assert.NotNull(savedBlocksValue); + blockListValue = JsonSerializer.Deserialize(savedBlocksValue); + + if (updateWithLimitedUserAccess) + { + Assert.Multiple(() => + { + // new one can't be added + Assert.AreEqual(0, blockListValue.Layout[Constants.PropertyEditors.Aliases.BlockList].Count(layoutItem => layoutItem.ContentKey == newContentElementKey)); + Assert.AreEqual(0, blockListValue.ContentData.Sum(contentData => contentData.Values.Count(value => value.Value?.ToString()?.StartsWith("#new") == true))); + Assert.AreEqual(0, blockListValue.SettingsData.Sum(settingsData => settingsData.Values.Count(value => value.Value?.ToString()?.StartsWith("#new") == true))); + + // can't remove first + Assert.AreEqual(1, blockListValue.Layout[Constants.PropertyEditors.Aliases.BlockList].Count(layoutItem => layoutItem.ContentKey == firstContentElementKey)); + Assert.AreEqual(1, blockListValue.Layout[Constants.PropertyEditors.Aliases.BlockList].Count(layoutItem => layoutItem.SettingsKey == firstSettingsElementKey)); + Assert.AreEqual(4, blockListValue.ContentData.Sum(contentData => contentData.Values.Count(value => value.Value?.ToString()?.StartsWith("#1") == true))); + Assert.AreEqual(4, blockListValue.SettingsData.Sum(settingsData => settingsData.Values.Count(value => value.Value?.ToString()?.StartsWith("#1") == true))); + + // second wasn't touched + Assert.AreEqual(1, blockListValue.Layout[Constants.PropertyEditors.Aliases.BlockList].Count(layoutItem => layoutItem.SettingsKey == secondSettingsElementKey)); + Assert.AreEqual(1, blockListValue.Layout[Constants.PropertyEditors.Aliases.BlockList].Count(layoutItem => layoutItem.ContentKey == secondContentElementKey)); + Assert.AreEqual(4, blockListValue.ContentData.Sum(contentData => contentData.Values.Count(value => value.Value?.ToString()?.StartsWith("#2") == true))); + Assert.AreEqual(4, blockListValue.SettingsData.Sum(settingsData => settingsData.Values.Count(value => value.Value?.ToString()?.StartsWith("#2") == true))); + }); + } + else + { + Assert.Multiple(() => + { + // add new one, did not add settings + Assert.AreEqual(1, blockListValue.Layout[Constants.PropertyEditors.Aliases.BlockList].Count(layoutItem => layoutItem.ContentKey == newContentElementKey)); + Assert.AreEqual(4, blockListValue.ContentData.Sum(contentData => contentData.Values.Count(value => value.Value?.ToString()?.StartsWith("#new") == true))); + Assert.AreEqual(0, blockListValue.SettingsData.Sum(settingsData => settingsData.Values.Count(value => value.Value?.ToString()?.StartsWith("#new") == true))); + + // first one removed + Assert.AreEqual(0, blockListValue.Layout[Constants.PropertyEditors.Aliases.BlockList].Count(layoutItem => layoutItem.ContentKey == firstContentElementKey)); + Assert.AreEqual(0, blockListValue.Layout[Constants.PropertyEditors.Aliases.BlockList].Count(layoutItem => layoutItem.SettingsKey == firstSettingsElementKey)); + Assert.AreEqual(0, blockListValue.ContentData.Sum(contentData => contentData.Values.Count(value => value.Value?.ToString()?.StartsWith("#1") == true))); + Assert.AreEqual(0, blockListValue.SettingsData.Sum(settingsData => settingsData.Values.Count(value => value.Value?.ToString()?.StartsWith("#1") == true))); + + // second wasn't touched + Assert.AreEqual(1, blockListValue.Layout[Constants.PropertyEditors.Aliases.BlockList].Count(layoutItem => layoutItem.SettingsKey == secondSettingsElementKey)); + Assert.AreEqual(1, blockListValue.Layout[Constants.PropertyEditors.Aliases.BlockList].Count(layoutItem => layoutItem.ContentKey == secondContentElementKey)); + Assert.AreEqual(4, blockListValue.ContentData.Sum(contentData => contentData.Values.Count(value => value.Value?.ToString()?.StartsWith("#2") == true))); + Assert.AreEqual(4, blockListValue.SettingsData.Sum(settingsData => settingsData.Values.Count(value => value.Value?.ToString()?.StartsWith("#2") == true))); + }); + } + } + + /// + /// Tests whether the user can add/remove new variant blocks to an invariant blocklist + /// + /// true => danish only which is not the default. false => admin which is all languages + [TestCase(true)] + [TestCase(false)] + [ConfigureBuilder(ActionName = nameof(ConfigureAllowEditInvariantFromNonDefaultTrue))] + public async Task Can_Handle_BlockStructureManipulation_For_Limited_Users_With_AllowEditInvariantFromNonDefault( + bool updateWithLimitedUserAccess) + { + await LanguageService.CreateAsync( + new Language("de-DE", "German"), Constants.Security.SuperUserKey); + var userKey = updateWithLimitedUserAccess + ? (await CreateLimitedUser()).Key + : Constants.Security.SuperUserKey; + + var elementType = CreateElementType(ContentVariation.Culture); + var blockListDataType = await CreateBlockListDataType(elementType); + var contentType = CreateContentType(ContentVariation.Culture, blockListDataType); + var content = CreateContent(contentType, elementType, [], false); + content.SetCultureName("Home (de)", "de-DE"); + ContentService.Save(content); + + var firstContentElementKey = Guid.NewGuid(); + var firstSettingsElementKey = Guid.NewGuid(); + + var blockListValue = BlockListPropertyValue( + elementType, + [ + ( + firstContentElementKey, + firstSettingsElementKey, + new BlockProperty( + new List { + new() { Alias = "invariantText", Value = "#1: The first invariant content value" }, + new() { Alias = "variantText", Value = "#1: The first content value in English", Culture = "en-US" }, + new() { Alias = "variantText", Value = "#1: The first content value in Danish", Culture = "da-DK" }, + new() { Alias = "variantText", Value = "#1: The first content value in German", Culture = "de-DE" }, + }, + new List { + new() { Alias = "invariantText", Value = "#1: The first invariant settings value" }, + new() { Alias = "variantText", Value = "#1: The first settings value in English", Culture = "en-US" }, + new() { Alias = "variantText", Value = "#1: The first settings value in Danish", Culture = "da-DK" }, + new() { Alias = "variantText", Value = "#1: The first settings value in German", Culture = "de-DE" }, + }, + null, + null)), + ( + Guid.NewGuid(), + Guid.NewGuid(), + new BlockProperty( + new List { + new() { Alias = "invariantText", Value = "#2: The first invariant content value" }, + new() { Alias = "variantText", Value = "#2: The first content value in English", Culture = "en-US" }, + new() { Alias = "variantText", Value = "#2: The first content value in Danish", Culture = "da-DK" }, + new() { Alias = "variantText", Value = "#2: The first content value in German", Culture = "de-DE" }, + }, + new List { + new() { Alias = "invariantText", Value = "#2: The first invariant settings value" }, + new() { Alias = "variantText", Value = "#2: The first settings value in English", Culture = "en-US" }, + new() { Alias = "variantText", Value = "#2: The first settings value in Danish", Culture = "da-DK" }, + new() { Alias = "variantText", Value = "#2: The first settings value in German", Culture = "de-DE" }, + }, + null, + null)) + ]); + + content.Properties["blocks"]!.SetValue(JsonSerializer.Serialize(blockListValue)); + ContentService.Save(content); + + var newContentElementKey = Guid.NewGuid(); + RemoveBlock(blockListValue, firstContentElementKey); + AddBlock( + blockListValue, + new BlockItemData + { + Key = newContentElementKey, + ContentTypeAlias = elementType.Alias, + ContentTypeKey = elementType.Key, + Values = new List { + new() { Alias = "invariantText", Value = "#new: The new invariant settings value" }, + new() { Alias = "variantText", Value = "#new: The new settings value in English", Culture = "en-US" }, + new() { Alias = "variantText", Value = "#new: The new settings value in Danish", Culture = "da-DK" }, + new() { Alias = "variantText", Value = "#new: The new settings value in German", Culture = "de-DE" }, + }, + }, + null, + elementType); + + var updateModel = new ContentUpdateModel + { + InvariantProperties = new[] + { + new PropertyValueModel { Alias = "blocks", Value = JsonSerializer.Serialize(blockListValue) } + }, + Variants = new[] + { + new VariantModel { Name = content.GetCultureName("en-US")!, Culture = "en-US", Properties = [] }, + new VariantModel { Name = content.GetCultureName("da-DK")!, Culture = "da-DK", Properties = [] }, + new VariantModel { Name = content.GetCultureName("de-DE")!, Culture = "de-DE", Properties = [] } + }, + }; + + var result = await ContentEditingService.UpdateAsync(content.Key, updateModel, userKey); + Assert.IsTrue(result.Success); + + content = ContentService.GetById(content.Key); + var savedBlocksValue = content?.Properties["blocks"]?.GetValue()?.ToString(); + Assert.NotNull(savedBlocksValue); + blockListValue = JsonSerializer.Deserialize(savedBlocksValue); + + // In both cases we are allowed to change the invariant structure + // But the amount of new cultured values we can add differs + Assert.Multiple(() => + { + Assert.AreEqual(1, blockListValue.Layout[Constants.PropertyEditors.Aliases.BlockList].Count(layoutItem => layoutItem.ContentKey == newContentElementKey)); + Assert.AreEqual(0, blockListValue.Layout[Constants.PropertyEditors.Aliases.BlockList].Count(layoutItem => layoutItem.ContentKey == firstContentElementKey)); + Assert.AreEqual(0, blockListValue.Layout[Constants.PropertyEditors.Aliases.BlockList].Count(layoutItem => layoutItem.SettingsKey == firstSettingsElementKey)); + Assert.AreEqual(updateWithLimitedUserAccess ? 2 : 4, blockListValue.ContentData.Sum(contentData => contentData.Values.Count(value => value.Value?.ToString()?.StartsWith("#new") == true))); + }); + } + + /// + /// Tests whether the user can update the variant values of existing blocks inside an invariant blocklist + /// + /// true => danish only which is not the default. false => admin which is all languages + [TestCase(true)] + [TestCase(false)] + public async Task Can_ClearBlocks_Limited_User_Access_To_Languages_Without_AllowEditInvariantFromNonDefault(bool updateWithLimitedUserAccess) + { + await LanguageService.CreateAsync( + new Language("de-DE", "German"), Constants.Security.SuperUserKey); + var userKey = updateWithLimitedUserAccess + ? (await CreateLimitedUser()).Key + : Constants.Security.SuperUserKey; + + var elementType = CreateElementType(ContentVariation.Culture); + var blockListDataType = await CreateBlockListDataType(elementType); + var contentType = CreateContentType(ContentVariation.Culture, blockListDataType); + var content = CreateContent(contentType, elementType, [], false); + content.SetCultureName("Home (de)", "de-DE"); + ContentService.Save(content); + + var blockListValue = BlockListPropertyValue( + elementType, + [ + ( + Guid.NewGuid(), + Guid.NewGuid(), + new BlockProperty( + new List { + new() { Alias = "invariantText", Value = "#1: The first invariant content value" }, + new() { Alias = "variantText", Value = "#1: The first content value in English", Culture = "en-US" }, + new() { Alias = "variantText", Value = "#1: The first content value in Danish", Culture = "da-DK" }, + new() { Alias = "variantText", Value = "#1: The first content value in German", Culture = "de-DE" } + }, + new List { + new() { Alias = "invariantText", Value = "#1: The first invariant settings value" }, + new() { Alias = "variantText", Value = "#1: The first settings value in English", Culture = "en-US" }, + new() { Alias = "variantText", Value = "#1: The first settings value in Danish", Culture = "da-DK" }, + new() { Alias = "variantText", Value = "#1: The first settings value in German", Culture = "de-DE" } + }, + null, + null + ) + ), + ( + Guid.NewGuid(), + Guid.NewGuid(), + new BlockProperty( + new List { + new() { Alias = "invariantText", Value = "#2: The first invariant content value" }, + new() { Alias = "variantText", Value = "#2: The first content value in English", Culture = "en-US" }, + new() { Alias = "variantText", Value = "#2: The first content value in Danish", Culture = "da-DK" }, + new() { Alias = "variantText", Value = "#2: The first content value in German", Culture = "de-DE" } + }, + new List { + new() { Alias = "invariantText", Value = "#2: The first invariant settings value" }, + new() { Alias = "variantText", Value = "#2: The first settings value in English", Culture = "en-US" }, + new() { Alias = "variantText", Value = "#2: The first settings value in Danish", Culture = "da-DK" }, + new() { Alias = "variantText", Value = "#2: The first settings value in German", Culture = "de-DE" } + }, + null, + null + ) + ) + ] + ); + + var serializedBlockListValue = JsonSerializer.Serialize(blockListValue); + content.Properties["blocks"]!.SetValue(serializedBlockListValue); + ContentService.Save(content); + + var updateModel = new ContentUpdateModel + { + InvariantProperties = new[] + { + new PropertyValueModel { Alias = "blocks", Value = null }, + }, + Variants = new[] + { + new VariantModel { Name = content.GetCultureName("en-US")!, Culture = "en-US", Properties = [] }, + new VariantModel { Name = content.GetCultureName("da-DK")!, Culture = "da-DK", Properties = [] }, + new VariantModel { Name = content.GetCultureName("de-DE")!, Culture = "de-DE", Properties = [] }, + }, + }; + + var result = await ContentEditingService.UpdateAsync(content.Key, updateModel, userKey); + Assert.IsTrue(result.Success); + + content = ContentService.GetById(content.Key); + var savedBlocksValue = content?.Properties["blocks"]?.GetValue()?.ToString(); + + // limited user access means English and German should not have been updated - changes should be rolled back to the initial block values + if (updateWithLimitedUserAccess) + { + Assert.NotNull(savedBlocksValue); + Assert.AreEqual(serializedBlockListValue, savedBlocksValue); + } + else + { + Assert.AreEqual("null", savedBlocksValue); + } + } + + /// + /// Tests whether the user can add/remove a value for a given culture + /// + /// true => danish only which is not the default. false => admin which is all languages + [TestCase(true)] + [TestCase(false)] + public async Task Can_Handle_ValueRemoval_For_Limited_Users( + bool updateWithLimitedUserAccess) + { + await LanguageService.CreateAsync( + new Language("de-DE", "German"), Constants.Security.SuperUserKey); + var userKey = updateWithLimitedUserAccess + ? (await CreateLimitedUser()).Key + : Constants.Security.SuperUserKey; + + var elementType = CreateElementType(ContentVariation.Culture); + var blockListDataType = await CreateBlockListDataType(elementType); + var contentType = CreateContentType(ContentVariation.Culture, blockListDataType); + var content = CreateContent(contentType, elementType, [], false); + content.SetCultureName("Home (de)", "de-DE"); + ContentService.Save(content); + + var firstContentElementKey = Guid.NewGuid(); + var firstSettingsElementKey = Guid.NewGuid(); + + var secondContentElementKey = Guid.NewGuid(); + var secondSettingsElementKey = Guid.NewGuid(); + + var blockListValue = BlockListPropertyValue( + elementType, + [ + ( + firstContentElementKey, + firstSettingsElementKey, + new BlockProperty( + new List { + new() { Alias = "invariantText", Value = "#1: The first invariant content value" }, + new() { Alias = "variantText", Value = "#1: The first content value in English", Culture = "en-US" }, + new() { Alias = "variantText", Value = "#1: The first content value in Danish", Culture = "da-DK" }, + new() { Alias = "variantText", Value = "#1: The first content value in German", Culture = "de-DE" }, + }, + new List { + new() { Alias = "invariantText", Value = "#1: The first invariant settings value" }, + new() { Alias = "variantText", Value = "#1: The first settings value in English", Culture = "en-US" }, + new() { Alias = "variantText", Value = "#1: The first settings value in Danish", Culture = "da-DK" }, + new() { Alias = "variantText", Value = "#1: The first settings value in German", Culture = "de-DE" }, + }, + null, + null)), + ( + secondContentElementKey, + secondSettingsElementKey, + new BlockProperty( + new List { + new() { Alias = "invariantText", Value = "#2: The first invariant content value" }, + new() { Alias = "variantText", Value = "#2: The first content value in English", Culture = "en-US" }, + new() { Alias = "variantText", Value = "#2: The first content value in Danish", Culture = "da-DK" }, + new() { Alias = "variantText", Value = "#2: The first content value in German", Culture = "de-DE" }, + }, + new List { + new() { Alias = "invariantText", Value = "#2: The first invariant settings value" }, + new() { Alias = "variantText", Value = "#2: The first settings value in English", Culture = "en-US" }, + new() { Alias = "variantText", Value = "#2: The first settings value in Danish", Culture = "da-DK" }, + new() { Alias = "variantText", Value = "#2: The first settings value in German", Culture = "de-DE" }, + }, + null, + null)) + ]); + + content.Properties["blocks"]!.SetValue(JsonSerializer.Serialize(blockListValue)); + ContentService.Save(content); + + // remove a value the limited user can remove + blockListValue.ContentData.First().Values.RemoveAll(value => value.Culture == "da-DK"); + blockListValue.SettingsData.First().Values.RemoveAll(value => value.Culture == "da-DK"); + // remove a value the admin user can remove + blockListValue.ContentData.First().Values.RemoveAll(value => value.Culture == "en-US"); + blockListValue.SettingsData.First().Values.RemoveAll(value => value.Culture == "en-US"); + + var updateModel = new ContentUpdateModel + { + InvariantProperties = new[] + { + new PropertyValueModel { Alias = "blocks", Value = JsonSerializer.Serialize(blockListValue) } + }, + Variants = new[] + { + new VariantModel { Name = content.GetCultureName("en-US")!, Culture = "en-US", Properties = [] }, + new VariantModel { Name = content.GetCultureName("da-DK")!, Culture = "da-DK", Properties = [] }, + new VariantModel { Name = content.GetCultureName("de-DE")!, Culture = "de-DE", Properties = [] } + }, + }; + + var result = await ContentEditingService.UpdateAsync(content.Key, updateModel, userKey); + Assert.IsTrue(result.Success); + + content = ContentService.GetById(content.Key); + var savedBlocksValue = content?.Properties["blocks"]?.GetValue()?.ToString(); + Assert.NotNull(savedBlocksValue); + blockListValue = JsonSerializer.Deserialize(savedBlocksValue); + + if (updateWithLimitedUserAccess) + { + Assert.Multiple(() => + { + + // Should only have removed the danish value + Assert.AreEqual(1, blockListValue.Layout[Constants.PropertyEditors.Aliases.BlockList].Count(layoutItem => layoutItem.ContentKey == firstContentElementKey)); + Assert.AreEqual(1, blockListValue.Layout[Constants.PropertyEditors.Aliases.BlockList].Count(layoutItem => layoutItem.SettingsKey == firstSettingsElementKey)); + Assert.AreEqual(3, blockListValue.ContentData.Sum(contentData => contentData.Values.Count(value => value.Value?.ToString()?.StartsWith("#1") == true))); + Assert.AreEqual(3, blockListValue.SettingsData.Sum(settingsData => settingsData.Values.Count(value => value.Value?.ToString()?.StartsWith("#1") == true))); + Assert.AreEqual(0, blockListValue.ContentData.First().Values.Count(value => value.Culture == "da-DK")); + Assert.AreEqual(1, blockListValue.ContentData.First().Values.Count(value => value.Culture == "en-US")); + Assert.AreEqual(0, blockListValue.SettingsData.First().Values.Count(value => value.Culture == "da-DK")); + Assert.AreEqual(1, blockListValue.SettingsData.First().Values.Count(value => value.Culture == "en-US")); + + // second wasn't touched + Assert.AreEqual(1, blockListValue.Layout[Constants.PropertyEditors.Aliases.BlockList].Count(layoutItem => layoutItem.SettingsKey == secondSettingsElementKey)); + Assert.AreEqual(1, blockListValue.Layout[Constants.PropertyEditors.Aliases.BlockList].Count(layoutItem => layoutItem.ContentKey == secondContentElementKey)); + Assert.AreEqual(4, blockListValue.ContentData.Sum(contentData => contentData.Values.Count(value => value.Value?.ToString()?.StartsWith("#2") == true))); + Assert.AreEqual(4, blockListValue.SettingsData.Sum(settingsData => settingsData.Values.Count(value => value.Value?.ToString()?.StartsWith("#2") == true))); + }); + } + else + { + Assert.Multiple(() => + { + // both danish and english should be removed + Assert.AreEqual(1, blockListValue.Layout[Constants.PropertyEditors.Aliases.BlockList].Count(layoutItem => layoutItem.ContentKey == firstContentElementKey)); + Assert.AreEqual(1, blockListValue.Layout[Constants.PropertyEditors.Aliases.BlockList].Count(layoutItem => layoutItem.SettingsKey == firstSettingsElementKey)); + Assert.AreEqual(2, blockListValue.ContentData.Sum(contentData => contentData.Values.Count(value => value.Value?.ToString()?.StartsWith("#1") == true))); + Assert.AreEqual(2, blockListValue.SettingsData.Sum(settingsData => settingsData.Values.Count(value => value.Value?.ToString()?.StartsWith("#1") == true))); + Assert.AreEqual(0, blockListValue.ContentData.First().Values.Count(value => value.Culture == "da-DK")); + Assert.AreEqual(0, blockListValue.ContentData.First().Values.Count(value => value.Culture == "en-US")); + Assert.AreEqual(0, blockListValue.SettingsData.First().Values.Count(value => value.Culture == "da-DK")); + Assert.AreEqual(0, blockListValue.SettingsData.First().Values.Count(value => value.Culture == "en-US")); + + // second wasn't touched + Assert.AreEqual(1, blockListValue.Layout[Constants.PropertyEditors.Aliases.BlockList].Count(layoutItem => layoutItem.SettingsKey == secondSettingsElementKey)); + Assert.AreEqual(1, blockListValue.Layout[Constants.PropertyEditors.Aliases.BlockList].Count(layoutItem => layoutItem.ContentKey == secondContentElementKey)); + Assert.AreEqual(4, blockListValue.ContentData.Sum(contentData => contentData.Values.Count(value => value.Value?.ToString()?.StartsWith("#2") == true))); + Assert.AreEqual(4, blockListValue.SettingsData.Sum(settingsData => settingsData.Values.Count(value => value.Value?.ToString()?.StartsWith("#2") == true))); }); } } @@ -940,4 +1478,55 @@ internal partial class BlockListElementLevelVariationTests return user; } + + private void AddBlock(BlockListValue listValue, BlockItemData contentData, BlockItemData? settingsData, IContentType elementType) + { + listValue.ContentData.Add(contentData); + if (settingsData != null) + { + listValue.SettingsData.Add(settingsData); + } + + var cultures = elementType.VariesByCulture() + ? contentData.Values.Select(value => value.Culture) + .WhereNotNull() + .Distinct() + .ToArray() + : [null]; + if (cultures.Any() is false) + { + cultures = [null]; + } + + var segments = elementType.VariesBySegment() + ? contentData.Values.Select(value => value.Segment) + .Distinct() + .ToArray() + : [null]; + + foreach (var exposeItem in cultures.SelectMany(culture => segments.Select(segment => + new BlockItemVariation(contentData.Key, culture, segment)))) + { + listValue.Expose.Add(exposeItem); + } + + + listValue.Layout[Constants.PropertyEditors.Aliases.BlockList] = listValue + .Layout[Constants.PropertyEditors.Aliases.BlockList] + .Append(new BlockListLayoutItem { ContentKey = contentData.Key, SettingsKey = settingsData?.Key }); + } + + private void RemoveBlock(BlockListValue listValue, Guid blockKey) + { + // remove the item from the layout + var layoutItem = listValue.Layout[Constants.PropertyEditors.Aliases.BlockList].First(x => x.ContentKey == blockKey); + listValue.Layout[Constants.PropertyEditors.Aliases.BlockList] = listValue.Layout[Constants.PropertyEditors.Aliases.BlockList].Where(layout => layout.ContentKey != blockKey); + listValue.ContentData.RemoveAll(contentData => contentData.Key == blockKey); + if (layoutItem.SettingsKey != null) + { + listValue.SettingsData.RemoveAll(settingsData => settingsData.Key == layoutItem.SettingsKey); + } + + listValue.Expose.RemoveAll(exposeItem => exposeItem.ContentKey == blockKey); + } } diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/PropertyEditors/block-element-level-variation.md b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/PropertyEditors/block-element-level-variation.md new file mode 100644 index 0000000000..274c95cec5 --- /dev/null +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/PropertyEditors/block-element-level-variation.md @@ -0,0 +1,67 @@ +# Block Element Level Variant + +### Notes +- When talking about variant data, we mean language variant +- Segment variants are not taken into account at this moment but can be expected to work in a similar manner + +### What is it +When an element document type supports variant data (marked as variant and has a property marked as variant) is used +inside an invariant property on a variant document type, that property is considered to have element level variant data +which is a form of partial variant data. + +When a property editor supports partial variant data (`IDataEditor.CanMergePartialPropertyValues()`) the +`ContentEditingService` will run the `IDataEditor.MergeVariantInvariantPropertyValue(...)` method to get a valid +value according to the rules defined for that propertyEditor + +Block Element level variant data is this within all (core) property editors derived from blocks +- Umbraco.BlockList +- Umbraco.BlockGrid +- Umbraco.RichText + +Most logic regarding this feature can be found in `BlockValuePropertyValueEditorBase` + +### Axioms +1. A `null` value for a property, including element level variation based properties, is a valid value +2. The invariant value holds the structure/representation of the underlying variant values +3. The structure takes precedence over the underlying data + +## Editing Data + +### Access to invariant data +- All Languages: The user has access to all languages +- Default Language: The user has access to the language that is defined as the default +- AllowEditInvariantFromNonDefault: Configuration setting + +| All Languages | Default Language | AllowEditInvariantFromNonDefault | Can Edit Invariant | +|---------------|------------------|----------------------------------|--------------------| +| True | Inherits True | N/A | True | +| False | True | N/A | True | +| False | False | True | True | +| False | False | False | False | + + +### Rules derived from the axioms +- A user with access to invariant data is allowed to add or remove blocks even if those blocks hold language variant +data they do not have access to. +- A user without access to invariant data is NOT allowed to add or remove blocks. +- A user can only edit element variant properties for the languages they have access to. +- A user is allowed to clear (set value to `null`) an element level variation as long as they have access to edit invariant data. + +## Exposing +When a block is defined on invariant level but a language has not had its variant fields filled in yet, +the variant version of the block might be empty or considered not ready for publishing. The Expose feature allows +editors to define in which culture a block is ready to be consumed by the publishing process. + +The client currently adds a blocks culture to the expose list when editing for that blocks starts in the culture, +more precisely when inline editor for the block is opened. + +From an API perspective you are allowed to add and remove cultures from the expose list as long as you have the permissions to do so + +### Axioms +- Expose data is linked to the same permissions as variant editing +- Variant blocks that are not exposed for a specific culture should not be processed by the publish feature + +### Rules derived from the axioms +- Only a user with access to a language should be able to remove or add a block to the expose list for that language +- A block that is not exposed for a given language should not exist in the published value of the document for that language +- A block that is not exposed should not be processed when running validation during publishing or by running the validation separately.