diff --git a/src/Umbraco.Core/Services/PropertyValidationService.cs b/src/Umbraco.Core/Services/PropertyValidationService.cs index 1bed040ba1..d1cc509110 100644 --- a/src/Umbraco.Core/Services/PropertyValidationService.cs +++ b/src/Umbraco.Core/Services/PropertyValidationService.cs @@ -1,6 +1,8 @@ using System.ComponentModel.DataAnnotations; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; using Umbraco.Cms.Core.Cache; +using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Dictionary; using Umbraco.Cms.Core.Models; @@ -17,6 +19,8 @@ public class PropertyValidationService : IPropertyValidationService private readonly PropertyEditorCollection _propertyEditors; private readonly IValueEditorCache _valueEditorCache; private readonly ICultureDictionary _cultureDictionary; + private readonly ILanguageService _languageService; + private readonly ContentSettings _contentSettings; [Obsolete("Use the constructor that accepts ICultureDictionary. Will be removed in V15.")] public PropertyValidationService( @@ -28,18 +32,40 @@ public class PropertyValidationService : IPropertyValidationService { } + [Obsolete("Use the constructor that accepts ILanguageService and ContentSettings options. Will be removed in V17.")] public PropertyValidationService( PropertyEditorCollection propertyEditors, IDataTypeService dataTypeService, ILocalizedTextService textService, IValueEditorCache valueEditorCache, ICultureDictionary cultureDictionary) + : this( + propertyEditors, + dataTypeService, + textService, + valueEditorCache, + cultureDictionary, + StaticServiceProvider.Instance.GetRequiredService(), + StaticServiceProvider.Instance.GetRequiredService>()) + { + } + + public PropertyValidationService( + PropertyEditorCollection propertyEditors, + IDataTypeService dataTypeService, + ILocalizedTextService textService, + IValueEditorCache valueEditorCache, + ICultureDictionary cultureDictionary, + ILanguageService languageService, + IOptions contentSettings) { _propertyEditors = propertyEditors; _dataTypeService = dataTypeService; _textService = textService; _valueEditorCache = valueEditorCache; _cultureDictionary = cultureDictionary; + _languageService = languageService; + _contentSettings = contentSettings.Value; } /// @@ -66,6 +92,19 @@ public class PropertyValidationService : IPropertyValidationService propertyType.PropertyEditorAlias); } + // only validate culture invariant properties if + // - AllowEditInvariantFromNonDefault is true, or + // - the default language is being validated, or + // - the underlying data editor supports partial property value merging (e.g. block level variance) + var defaultCulture = _languageService.GetDefaultIsoCodeAsync().GetAwaiter().GetResult(); + if (propertyType.VariesByCulture() is false + && _contentSettings.AllowEditInvariantFromNonDefault is false + && validationContext.CulturesBeingValidated.InvariantContains(defaultCulture) is false + && dataEditor.CanMergePartialPropertyValues(propertyType) is false) + { + return []; + } + return ValidatePropertyValue(dataEditor, dataType, postedValue, propertyType.Mandatory, propertyType.ValidationRegExp, propertyType.MandatoryMessage, propertyType.ValidationRegExpMessage, validationContext); } diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/PropertyEditors/BlockListElementLevelVariationTests.Validation.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/PropertyEditors/BlockListElementLevelVariationTests.Validation.cs index 8e8878e9bc..ea2e680293 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/PropertyEditors/BlockListElementLevelVariationTests.Validation.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/PropertyEditors/BlockListElementLevelVariationTests.Validation.cs @@ -3,6 +3,7 @@ using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.Blocks; using Umbraco.Cms.Core.Models.ContentEditing; using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Tests.Integration.Attributes; namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.PropertyEditors; @@ -172,7 +173,15 @@ internal partial class BlockListElementLevelVariationTests } [Test] - public async Task Can_Validate_Invalid_Properties_Specific_Culture_Only() + [ConfigureBuilder(ActionName = nameof(ConfigureAllowEditInvariantFromNonDefaultTrue))] + public async Task Can_Validate_Invalid_Properties_Specific_Culture_Only_With_AllowEditInvariantFromNonDefault() + => await Can_Validate_Invalid_Properties_Specific_Culture_Only(); + + [Test] + public async Task Can_Validate_Invalid_Properties_Specific_Culture_Only_Without_AllowEditInvariantFromNonDefault() + => await Can_Validate_Invalid_Properties_Specific_Culture_Only(); + + private async Task Can_Validate_Invalid_Properties_Specific_Culture_Only() { var elementType = CreateElementTypeWithValidation(); var blockListDataType = await CreateBlockListDataType(elementType); @@ -214,6 +223,8 @@ internal partial class BlockListElementLevelVariationTests contentType, new[] { "en-US" }); + // NOTE: since the default culture is being validated, we expect the same result regardless + // of the AllowEditInvariantFromNonDefault configuration var errors = result.ValidationErrors.ToArray(); Assert.Multiple(() => { @@ -338,7 +349,15 @@ internal partial class BlockListElementLevelVariationTests } [Test] - public async Task Can_Validate_Missing_Properties_Nested_Blocks_Specific_Culture_Only() + [ConfigureBuilder(ActionName = nameof(ConfigureAllowEditInvariantFromNonDefaultTrue))] + public async Task Can_Validate_Missing_Properties_Nested_Blocks_Specific_Culture_Only_With_AllowEditInvariantFromNonDefault() + => Can_Validate_Missing_Properties_Nested_Blocks_Specific_Culture_Only(true); + + [Test] + public async Task Can_Validate_Missing_Properties_Nested_Blocks_Specific_Culture_Only_Without_AllowEditInvariantFromNonDefault() + => Can_Validate_Missing_Properties_Nested_Blocks_Specific_Culture_Only(false); + + private async Task Can_Validate_Missing_Properties_Nested_Blocks_Specific_Culture_Only(bool expectedInvariantValidationErrors) { var (rootElementType, nestedElementType) = await CreateElementTypeWithValidationAndNestedBlocksAsync(); var rootBlockListDataType = await CreateBlockListDataType(rootElementType); @@ -448,19 +467,39 @@ internal partial class BlockListElementLevelVariationTests new[] { "da-DK" }); var errors = result.ValidationErrors.ToArray(); - Assert.Multiple(() => + + // NOTE: since the default culture is not being validated, we expect different results depending + // on the AllowEditInvariantFromNonDefault configuration + + if (expectedInvariantValidationErrors) { - Assert.AreEqual(6, errors.Length); - Assert.IsTrue(errors.All(error => error.Alias == "blocks" && error.Culture == null && error.Segment == null)); + Assert.Multiple(() => + { + Assert.AreEqual(6, errors.Length); + Assert.IsTrue(errors.All(error => error.Alias == "blocks" && error.Culture == null && error.Segment == null)); - Assert.IsNotNull(errors.FirstOrDefault(error => error.JsonPath == ".contentData[0].values[0].value.contentData[0].values[?(@.alias == 'invariantText' && @.culture == null && @.segment == null)].value")); - Assert.IsNotNull(errors.FirstOrDefault(error => error.JsonPath == ".contentData[0].values[0].value.contentData[0].values[?(@.alias == 'variantText' && @.culture == 'da-DK' && @.segment == null)].value")); - Assert.IsNotNull(errors.FirstOrDefault(error => error.JsonPath == ".contentData[0].values[?(@.alias == 'variantText' && @.culture == 'da-DK' && @.segment == null)].value")); + Assert.IsNotNull(errors.FirstOrDefault(error => error.JsonPath == ".contentData[0].values[0].value.contentData[0].values[?(@.alias == 'invariantText' && @.culture == null && @.segment == null)].value")); + Assert.IsNotNull(errors.FirstOrDefault(error => error.JsonPath == ".contentData[0].values[0].value.contentData[0].values[?(@.alias == 'variantText' && @.culture == 'da-DK' && @.segment == null)].value")); + Assert.IsNotNull(errors.FirstOrDefault(error => error.JsonPath == ".contentData[0].values[?(@.alias == 'variantText' && @.culture == 'da-DK' && @.segment == null)].value")); - Assert.IsNotNull(errors.FirstOrDefault(error => error.JsonPath == ".settingsData[0].values[0].value.settingsData[0].values[?(@.alias == 'invariantText' && @.culture == null && @.segment == null)].value")); - Assert.IsNotNull(errors.FirstOrDefault(error => error.JsonPath == ".settingsData[0].values[0].value.settingsData[0].values[?(@.alias == 'variantText' && @.culture == 'da-DK' && @.segment == null)].value")); - Assert.IsNotNull(errors.FirstOrDefault(error => error.JsonPath == ".settingsData[0].values[?(@.alias == 'invariantText' && @.culture == null && @.segment == null)].value")); - }); + Assert.IsNotNull(errors.FirstOrDefault(error => error.JsonPath == ".settingsData[0].values[0].value.settingsData[0].values[?(@.alias == 'invariantText' && @.culture == null && @.segment == null)].value")); + Assert.IsNotNull(errors.FirstOrDefault(error => error.JsonPath == ".settingsData[0].values[0].value.settingsData[0].values[?(@.alias == 'variantText' && @.culture == 'da-DK' && @.segment == null)].value")); + Assert.IsNotNull(errors.FirstOrDefault(error => error.JsonPath == ".settingsData[0].values[?(@.alias == 'invariantText' && @.culture == null && @.segment == null)].value")); + }); + } + else + { + Assert.Multiple(() => + { + Assert.AreEqual(3, errors.Length); + Assert.IsTrue(errors.All(error => error.Alias == "blocks" && error.Culture == null && error.Segment == null)); + + Assert.IsNotNull(errors.FirstOrDefault(error => error.JsonPath == ".contentData[0].values[0].value.contentData[0].values[?(@.alias == 'variantText' && @.culture == 'da-DK' && @.segment == null)].value")); + Assert.IsNotNull(errors.FirstOrDefault(error => error.JsonPath == ".contentData[0].values[?(@.alias == 'variantText' && @.culture == 'da-DK' && @.segment == null)].value")); + + Assert.IsNotNull(errors.FirstOrDefault(error => error.JsonPath == ".settingsData[0].values[0].value.settingsData[0].values[?(@.alias == 'variantText' && @.culture == 'da-DK' && @.segment == null)].value")); + }); + } } [Test] diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentPublishingServiceTests.Publish.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentPublishingServiceTests.Publish.cs index a8abd95f98..5951af2bb5 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentPublishingServiceTests.Publish.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentPublishingServiceTests.Publish.cs @@ -2,8 +2,11 @@ using NUnit.Framework; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.ContentPublishing; +using Umbraco.Cms.Core.Models.Membership; +using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Services.OperationStatus; using Umbraco.Cms.Tests.Common.Builders; +using Umbraco.Cms.Tests.Integration.Attributes; namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services; @@ -325,6 +328,82 @@ public partial class ContentPublishingServiceTests Assert.AreEqual(2, content.PublishedCultures.Count()); } + [TestCase(true, "da-DK")] + [TestCase(false, "en-US")] + [TestCase(false, "en-US", "da-DK")] + public async Task Publish_Invalid_Invariant_Property_WithoutAllowEditInvariantFromNonDefault(bool expectedSuccess, params string[] culturesToRepublish) + => await Publish_Invalid_Invariant_Property(expectedSuccess, culturesToRepublish); + + [TestCase(false, "da-DK")] + [TestCase(false, "en-US")] + [TestCase(false, "en-US", "da-DK")] + [ConfigureBuilder(ActionName = nameof(ConfigureAllowEditInvariantFromNonDefaultTrue))] + public async Task Publish_Invalid_Invariant_Property_WithAllowEditInvariantFromNonDefault(bool expectedSuccess, params string[] culturesToRepublish) + => await Publish_Invalid_Invariant_Property(expectedSuccess, culturesToRepublish); + + private async Task Publish_Invalid_Invariant_Property(bool expectedSuccess, params string[] culturesToRepublish) + { + var contentType = await SetupVariantInvariantTest(); + + IContent content = new ContentBuilder() + .WithContentType(contentType) + .WithCultureName("en-US", "EN") + .WithCultureName("da-DK", "DA") + .Build(); + content.SetValue("variantValue", "EN value", culture: "en-US"); + content.SetValue("variantValue", "DA value", culture: "da-DK"); + content.SetValue("invariantValue", "Invariant value"); + ContentService.Save(content); + + var result = await ContentPublishingService.PublishAsync(content.Key, MakeModel(new HashSet { "en-US", "da-DK" }), Constants.Security.SuperUserKey); + Assert.IsTrue(result.Success); + + content = ContentService.GetById(content.Key)!; + content.SetValue("variantValue", "EN value updated", culture: "en-US"); + content.SetValue("variantValue", "DA value updated", culture: "da-DK"); + content.SetValue("invariantValue", null); + ContentService.Save(content); + + result = await ContentPublishingService.PublishAsync(content.Key, MakeModel(new HashSet(culturesToRepublish)), Constants.Security.SuperUserKey); + + content = ContentService.GetById(content.Key)!; + + Assert.Multiple(() => + { + Assert.AreEqual(null, content.GetValue("invariantValue", published: false)); + Assert.AreEqual("EN value updated", content.GetValue("variantValue", culture: "en-US", published: false)); + Assert.AreEqual("DA value updated", content.GetValue("variantValue", culture: "da-DK", published: false)); + + Assert.AreEqual("Invariant value", content.GetValue("invariantValue", published: true)); + }); + + if (expectedSuccess) + { + Assert.Multiple(() => + { + Assert.IsTrue(result.Success); + + var expectedPublishedEnglishValue = culturesToRepublish.Contains("en-US") + ? "EN value updated" + : "EN value"; + var expectedPublishedDanishValue = culturesToRepublish.Contains("da-DK") + ? "DA value updated" + : "DA value"; + Assert.AreEqual(expectedPublishedEnglishValue, content.GetValue("variantValue", culture: "en-US", published: true)); + Assert.AreEqual(expectedPublishedDanishValue, content.GetValue("variantValue", culture: "da-DK", published: true)); + }); + } + else + { + Assert.Multiple(() => + { + Assert.IsFalse(result.Success); + Assert.AreEqual("EN value", content.GetValue("variantValue", culture: "en-US", published: true)); + Assert.AreEqual("DA value", content.GetValue("variantValue", culture: "da-DK", published: true)); + }); + } + } + [Test] public async Task Cannot_Publish_Non_Existing_Content() { diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentPublishingServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentPublishingServiceTests.cs index ab44a3131f..6e5cf55fc7 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentPublishingServiceTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentPublishingServiceTests.cs @@ -1,5 +1,7 @@ -using NUnit.Framework; +using Microsoft.Extensions.DependencyInjection; +using NUnit.Framework; using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.ContentPublishing; @@ -109,6 +111,41 @@ public partial class ContentPublishingServiceTests : UmbracoIntegrationTestWithC return (langEn, langDa, contentType); } + private async Task SetupVariantInvariantTest() + { + var langDa = new LanguageBuilder() + .WithCultureInfo("da-DK") + .Build(); + await LanguageService.CreateAsync(langDa, Constants.Security.SuperUserKey); + + var key = Guid.NewGuid(); + var contentType = new ContentTypeBuilder() + .WithAlias("variantInvariantContent") + .WithName("Variant Invariant Content") + .WithKey(key) + .WithContentVariation(ContentVariation.Culture) + .AddAllowedContentType() + .WithKey(key) + .WithAlias("variantInvariantContent") + .Done() + .AddPropertyType() + .WithAlias("variantValue") + .WithVariations(ContentVariation.Culture) + .WithMandatory(true) + .Done() + .AddPropertyType() + .WithAlias("invariantValue") + .WithVariations(ContentVariation.Nothing) + .WithMandatory(true) + .Done() + .Build(); + + contentType.AllowedAsRoot = true; + await ContentTypeService.SaveAsync(contentType, Constants.Security.SuperUserKey); + + return contentType; + } + protected override void CustomTestSetup(IUmbracoBuilder builder) => builder .AddNotificationHandler() @@ -132,4 +169,7 @@ public partial class ContentPublishingServiceTests : UmbracoIntegrationTestWithC public void Handle(ContentUnpublishingNotification notification) => UnpublishingContent?.Invoke(notification); } + + public static void ConfigureAllowEditInvariantFromNonDefaultTrue(IUmbracoBuilder builder) + => builder.Services.Configure(config => config.AllowEditInvariantFromNonDefault = true); } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Models/VariationTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Models/VariationTests.cs index ef8b8733c7..e6cb811f01 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Models/VariationTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Models/VariationTests.cs @@ -1,10 +1,12 @@ // Copyright (c) Umbraco. // See LICENSE for more details. +using Microsoft.Extensions.Options; using Moq; using NUnit.Framework; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Cache; +using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.Dictionary; using Umbraco.Cms.Core.IO; using Umbraco.Cms.Core.Models; @@ -654,6 +656,8 @@ public class VariationTests dataTypeService, Mock.Of(), new ValueEditorCache(), - Mock.Of()); + Mock.Of(), + Mock.Of(), + Mock.Of>()); } } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Services/PropertyValidationServiceTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Services/PropertyValidationServiceTests.cs index 097a0495e5..a5273d31fc 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Services/PropertyValidationServiceTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Services/PropertyValidationServiceTests.cs @@ -1,10 +1,12 @@ // Copyright (c) Umbraco. // See LICENSE for more details. +using Microsoft.Extensions.Options; using Moq; using NUnit.Framework; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Cache; +using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.Dictionary; using Umbraco.Cms.Core.IO; using Umbraco.Cms.Core.Models; @@ -44,7 +46,14 @@ public class PropertyValidationServiceTests var propEditors = new PropertyEditorCollection(new DataEditorCollection(() => new[] { dataEditor })); - validationService = new PropertyValidationService(propEditors, dataTypeService.Object, Mock.Of(),new ValueEditorCache(), Mock.Of()); + validationService = new PropertyValidationService( + propEditors, + dataTypeService.Object, + Mock.Of(), + new ValueEditorCache(), + Mock.Of(), + Mock.Of(), + Mock.Of>()); } [Test]