Publishing: Validate segment values on scheduled publish, publish with descendants and create/update-and-publish (closes #24558)

src/Umbraco.Core/Services/PropertyValidationService.cs

@@ -129,7 +129,7 @@
     }
 
     /// <inheritdoc />
     public bool IsPropertyDataValid(IPublishableContentBase content, out IProperty[] invalidProperties, CultureImpact? impact)
     {
         // select invalid properties
         invalidProperties = content.Properties.Where(x =>
@@ -141,6 +141,11 @@
                 return false;
             }
 
+            if (HasInvalidSegmentValue(content, x, impact))
+            {
+                return true;
+            }
+
             // impacts invariant = validate invariant property, invariant culture
             if (impact.ImpactsOnlyInvariantCulture)
             {
@@ -190,13 +195,68 @@
         return invalidProperties.Length == 0;
     }
 
+    /// <summary>
+    ///     Determines whether the property holds an invalid value in a non-default segment of the cultures the impact publishes.
+    /// </summary>
+    private bool HasInvalidSegmentValue(IPublishableContentBase content, IProperty property, CultureImpact impact)
+    {
+        if (property.PropertyType.VariesBySegment() is false)
+        {
+            return false;
+        }
+
+        var variesByCulture = property.PropertyType.VariesByCulture();
+        IEnumerable<string> culturesBeingValidated = [];
+
+        if (impact.ImpactsAllCultures is false)
+        {
+            if (variesByCulture)
+            {
+                if (impact.ImpactsOnlyInvariantCulture)
+                {
+                    return false;
+                }
+            }
+            else if (impact.ImpactsExplicitCulture)
+            {
+                if (GetDataEditor(property.PropertyType)?.CanMergePartialPropertyValues(property.PropertyType) is true)
+                {
+                    culturesBeingValidated = [impact.Culture!];
+                }
+                else if (impact.ImpactsAlsoInvariantProperties is false && content.Published)
+                {
+                    return false;
+                }
+            }
+        }
+
+        var culture = variesByCulture ? impact.Culture : null;
+
+        // a culture invariant value applies to every culture, so when all cultures are impacted it is validated for all of them
+        var validateInvariantValueForAllCultures = variesByCulture is false && impact.ImpactsAllCultures;
+
+        return property.Values.Any(value =>
+            value.Segment.IsNullOrWhiteSpace() is false
+            && property.PropertyType.SupportsVariation(value.Culture, value.Segment, true)
+            && (impact.ImpactsAllCultures || value.Culture.InvariantEquals(culture))
+            && IsValidPropertyValue(
+                property,
+                value.EditedValue,
+                new PropertyValidationContext
+                {
+                    Culture = validateInvariantValueForAllCultures ? "*" : value.Culture,
+                    Segment = value.Segment,
+                    CulturesBeingValidated = culturesBeingValidated,
+                }) is false);
+    }
+
     /// <inheritdoc />
     // TODO (V20): Make this internal rather than removing it entirely in V20, so the
     // referencing unit tests can continue to exercise this code.
     // Also remove the obsolete code warning suppressions added around the various
     // callers to this method (including the unit tests).
     [Obsolete("Property level validation is not going to be supported moving forward. Please use content level validation with IsPropertyDataValid instead. Scheduled for removal in Umbraco 20.")]
     public bool IsPropertyValid(IProperty property, PropertyValidationContext validationContext)
     {
         // NOTE - the pvalue and vvalues logic in here is borrowed directly from the Property.Values setter so if you are wondering what that's all about, look there.
         // The underlying Property._pvalue and Property._vvalues are not exposed but we can re-create these values ourselves which is what it's doing.

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

@@ -1,4 +1,4 @@
 using NUnit.Framework;
 using Umbraco.Cms.Core;
 using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.Models.Blocks;
@@ -14,6 +14,42 @@
 {
     private IContentValidationService ContentValidationService => GetRequiredService<IContentValidationService>();
 
+    /// <summary>
+    /// Tests whether validating for all cultures includes the culture variant block values held in a non-default
+    /// segment of a culture invariant block property.
+    /// </summary>
+    [TestCase("Valid value in Danish", true)]
+    [TestCase("Invalid value in Danish", false)]
+    public async Task Validates_Culture_Variant_Block_Values_In_Non_Default_Segments_For_All_Cultures(string segmentValueInDanish, bool expectedValid)
+    {
+        var elementType = await CreateElementTypeWithValidationAsync();
+        var blockListDataType = await CreateBlockListDataType(elementType);
+        var contentType = await CreateContentType(ContentVariation.CultureAndSegment, blockListDataType, ContentVariation.Segment);
+
+        var content = CreateContent(
+            contentType,
+            elementType,
+            [BlockPropertyForSegment(null, "Valid value in Danish"), BlockPropertyForSegment("s1", segmentValueInDanish)],
+            false);
+
+        var isValid = GetRequiredService<IPropertyValidationService>().IsPropertyDataValid(content, out IProperty[] invalidProperties, CultureImpact.All);
+
+        Assert.AreEqual(expectedValid, isValid);
+        Assert.AreEqual(expectedValid ? 0 : 1, invalidProperties.Length);
+
+        BlockProperty BlockPropertyForSegment(string? segment, string valueInDanish)
+        {
+            return new BlockProperty(BlockValues(), BlockValues(), null, segment);
+
+            List<BlockPropertyValue> BlockValues() =>
+            [
+                new() { Alias = "invariantText", Value = "Valid invariant value" },
+                new() { Alias = "variantText", Value = "Valid value in English", Culture = "en-US" },
+                new() { Alias = "variantText", Value = valueInDanish, Culture = "da-DK" },
+            ];
+        }
+    }
+
     [Test]
     public async Task Can_Validate_Invalid_Properties()
     {

tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentPublishingServiceTests.Publish.cs

@@ -1,4 +1,4 @@
 using NUnit.Framework;
 using Umbraco.Cms.Core;
 using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.Models.ContentPublishing;
@@ -779,6 +779,79 @@
         VerifyIsPublished(content.Key);
     }
 
+    [TestCase(ContentVariation.Segment)]
+    [TestCase(ContentVariation.CultureAndSegment)]
+    public async Task Cannot_Publish_Branch_Descendant_With_Invalid_Value_In_Non_Default_Segment(ContentVariation variation)
+    {
+        var contentType = await SetupSegmentVariantTest(variation);
+        var culture = variation.VariesByCulture() ? "en-US" : null;
+
+        IContent root = BuildSegmentVariantContent(contentType, culture, "Root");
+        root.SetValue("title", "Valid default value", culture);
+        ContentService.Save(root);
+
+        IContent child = BuildSegmentVariantContent(contentType, culture, "Child", root);
+        child.SetValue("title", "Valid default value", culture);
+        child.SetValue("title", "Invalid seg-1 value", culture, "seg-1");
+        ContentService.Save(child);
+
+        var result = await ContentPublishingService.PublishBranchAsync(
+            root.Key,
+            culture is null ? _allCultures : [culture],
+            PublishBranchFilter.IncludeUnpublished,
+            Constants.Security.SuperUserKey,
+            false);
+
+        Assert.IsFalse(result.Success);
+        Assert.AreEqual(ContentPublishingOperationStatus.FailedBranch, result.Status);
+        AssertBranchResultSuccess(result.Result, root.Key);
+        AssertBranchResultFailed(result.Result, (child.Key, ContentPublishingOperationStatus.ContentInvalid));
+        VerifyIsPublished(root.Key);
+        VerifyIsNotPublished(child.Key);
+    }
+
+    [TestCase(ContentVariation.Segment)]
+    [TestCase(ContentVariation.CultureAndSegment)]
+    public async Task Scheduled_Publish_Does_Not_Publish_Content_With_Invalid_Value_In_Non_Default_Segment(ContentVariation variation)
+    {
+        var contentType = await SetupSegmentVariantTest(variation);
+        var culture = variation.VariesByCulture() ? "en-US" : null;
+
+        IContent content = BuildSegmentVariantContent(contentType, culture, "Segment Test");
+        content.SetValue("title", "Valid default value", culture);
+        content.SetValue("title", "Valid seg-1 value", culture, "seg-1");
+        ContentService.Save(content);
+
+        var scheduleResult = await ContentPublishingService.PublishAsync(
+            content.Key,
+            [
+                new CulturePublishScheduleModel
+                {
+                    Culture = culture,
+                    Schedule = new ContentScheduleModel { PublishDate = DateTimeOffset.UtcNow.AddDays(1) },
+                }
+            ],
+            Constants.Security.SuperUserKey);
+        Assert.IsTrue(scheduleResult.Success);
+
+        content = ContentService.GetById(content.Key)!;
+        content.SetValue("title", "Invalid seg-1 value", culture, "seg-1");
+        ContentService.Save(content);
+
+        var results = ContentService.PerformScheduledPublish(DateTime.UtcNow.AddDays(2)).ToArray();
+
+        var contentResult = results.Single(r => r.Content.Key == content.Key);
+        Assert.AreEqual(PublishResultType.FailedPublishContentInvalid, contentResult.Result);
+        VerifyIsNotPublished(content.Key);
+    }
+
+    private static IContent BuildSegmentVariantContent(IContentType contentType, string? culture, string name, IContent? parent = null)
+    {
+        ContentBuilder builder = new ContentBuilder().WithContentType(contentType);
+        builder = culture is null ? builder.WithName(name) : builder.WithCultureName(culture, name);
+        return parent is null ? builder.Build() : builder.WithParent(parent).Build();
+    }
+
     [TestCase(PublishBranchFilter.Default, true)]
     [TestCase(PublishBranchFilter.ForceRepublish, false)]
     public async Task Publish_Branch_Validates_Unchanged_Published_Root_Only_When_Republishing(PublishBranchFilter publishBranchFilter, bool expectSuccess)

tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Services/PropertyValidationServiceTests.cs

@@ -293,6 +293,79 @@
         Assert.AreEqual(4, invalid.Length);
     }
 
+    [TestCase(ContentVariation.Segment, null, "Invalid", false)]
+    [TestCase(ContentVariation.Segment, null, "Valid", true)]
+    [TestCase(ContentVariation.CultureAndSegment, "en-US", "Invalid", false)]
+    [TestCase(ContentVariation.CultureAndSegment, "en-US", "Valid", true)]
+    public void Validate_Values_In_Non_Default_Segments(ContentVariation variation, string? culture, string segmentValue, bool expectedValid)
+    {
+        MockObjects(out var validationService, out var dataType);
+
+        var property = new Property(
+            new PropertyType(ShortStringHelper, dataType, "test1")
+            {
+                ValidationRegExp = "^Valid.*$",
+                Variations = variation,
+            });
+        property.SetValue("Valid", culture);
+        property.SetValue(segmentValue, culture, "segment-1");
+
+        var content = Mock.Of<IContent>(x => x.Properties == new PropertyCollection(new[] { property }));
+        CultureImpact impact = culture is null ? CultureImpact.Invariant : CultureImpact.Explicit(culture, true);
+
+        var result = validationService.IsPropertyDataValid(content, out var invalid, impact);
+
+        Assert.AreEqual(expectedValid, result);
+        Assert.AreEqual(expectedValid ? 0 : 1, invalid.Length);
+    }
+
+    [TestCase(ContentVariation.Segment, null)]
+    [TestCase(ContentVariation.CultureAndSegment, "en-US")]
+    public void Does_Not_Require_Values_In_Non_Default_Segments_Of_Mandatory_Properties(ContentVariation variation, string? culture)
+    {
+        MockObjects(out var validationService, out var dataType);
+
+        var property = new Property(
+            new PropertyType(ShortStringHelper, dataType, "test1")
+            {
+                Mandatory = true,
+                Variations = variation,
+            });
+        property.SetValue("Hello", culture);
+        property.SetValue(string.Empty, culture, "segment-1");
+
+        var content = Mock.Of<IContent>(x => x.Properties == new PropertyCollection(new[] { property }));
+        CultureImpact impact = culture is null ? CultureImpact.Invariant : CultureImpact.Explicit(culture, true);
+
+        var result = validationService.IsPropertyDataValid(content, out var invalid, impact);
+
+        Assert.IsTrue(result);
+        Assert.IsEmpty(invalid);
+    }
+
+    [Test]
+    public void Does_Not_Validate_Segment_Values_Of_Cultures_Not_Impacted()
+    {
+        MockObjects(out var validationService, out var dataType);
+
+        var property = new Property(
+            new PropertyType(ShortStringHelper, dataType, "test1")
+            {
+                ValidationRegExp = "^Valid.*$",
+                Variations = ContentVariation.CultureAndSegment,
+            });
+        property.SetValue("Valid", "en-US");
+        property.SetValue("Valid", "en-US", "segment-1");
+        property.SetValue("Invalid", "da-DK", "segment-1");
+
+        var content = Mock.Of<IContent>(x => x.Properties == new PropertyCollection(new[] { property }));
+
+        var result = validationService.IsPropertyDataValid(content, out var invalid, CultureImpact.Explicit("en-US", true));
+
+        Assert.IsTrue(result);
+        Assert.IsEmpty(invalid);
+    }
+
     [TestCase(null)]
     [TestCase(24)]
     [TestCase("test")]