Rich text editor: Treat an "empty" value as a non-value (closes #20454) (#20719)

* Make the RTE treat an "empty" value as a non-value

* Additional tests

* Add tests for invariant and variant content.

---------

Co-authored-by: Andy Butland <[email protected]>

Author: kjac

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

@@ -72,6 +72,19 @@ public override PropertyCacheLevel GetPropertyCacheLevel(IPublishedPropertyType
         // to be cached at the published snapshot level, because we have no idea what the block renderings may depend on actually.
         PropertyCacheLevel.Snapshot;
 
+    /// <inheritdoc />
+    public override bool? IsValue(object? value, PropertyValueLevel level)
+        => level switch
+        {
+            // we cannot determine if an RTE has a value at source level, because some RTEs might
+            // be saved with an "empty" representation like {"markup":"","blocks":null}.
+            PropertyValueLevel.Source => null,
+            // we assume the RTE has a value if the intermediate value has markup beyond an empty paragraph tag.
+            PropertyValueLevel.Inter => value is IRichTextEditorIntermediateValue { Markup.Length: > 0 } intermediateValue
+                                        && intermediateValue.Markup != "<p></p>",
+            _ => throw new ArgumentOutOfRangeException(nameof(level), level, null)
+        };
+
     // to counterweigh the cache level, we're going to do as much of the heavy lifting as we can while converting source to intermediate
     public override object? ConvertSourceToIntermediate(IPublishedElement owner, IPublishedPropertyType propertyType, object? source, bool preview)
     {

tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/PropertyEditors/RichTextPropertyEditorTests.cs

@@ -1,13 +1,19 @@
 using NUnit.Framework;
 using Umbraco.Cms.Core;
+using Umbraco.Cms.Core.Cache;
 using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.Models.Blocks;
+using Umbraco.Cms.Core.Notifications;
 using Umbraco.Cms.Core.PropertyEditors;
+using Umbraco.Cms.Core.PublishedCache;
 using Umbraco.Cms.Core.Serialization;
 using Umbraco.Cms.Core.Services;
+using Umbraco.Cms.Core.Sync;
 using Umbraco.Cms.Tests.Common.Builders;
+using Umbraco.Cms.Tests.Common.Builders.Extensions;
 using Umbraco.Cms.Tests.Common.Testing;
 using Umbraco.Cms.Tests.Integration.Testing;
+using Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services;
 
 namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.PropertyEditors;
 
@@ -23,6 +29,14 @@ internal sealed class RichTextPropertyEditorTests : UmbracoIntegrationTest
 
     private IJsonSerializer JsonSerializer => GetRequiredService<IJsonSerializer>();
 
+    private IPublishedContentCache PublishedContentCache => GetRequiredService<IPublishedContentCache>();
+
+    protected override void CustomTestSetup(IUmbracoBuilder builder)
+    {
+        builder.AddNotificationHandler<ContentTreeChangeNotification, ContentTreeChangeDistributedCacheNotificationHandler>();
+        builder.Services.AddUnique<IServerMessenger, ContentEventsTests.LocalServerMessenger>();
+    }
+
     [Test]
     public void Can_Use_Markup_String_As_Value()
     {
@@ -180,4 +194,103 @@ public void Can_Track_Block_Tags()
         Assert.IsNotNull(tags.Single(tag => tag.Text == "Tag Two"));
         Assert.IsNotNull(tags.Single(tag => tag.Text == "Tag Three"));
     }
+
+    [TestCase(null, false)]
+    [TestCase("", false)]
+    [TestCase("""{"markup":"","blocks":null}""", false)]
+    [TestCase("""{"markup":"<p></p>","blocks":null}""", false)]
+    [TestCase("abc", true)]
+    [TestCase("""{"markup":"abc","blocks":null}""", true)]
+    public async Task Can_Handle_Empty_Value_Representations_For_Invariant_Content(string? rteValue, bool expectedHasValue)
+    {
+        var contentType = await CreateContentTypeForEmptyValueTests();
+
+        var content = new ContentBuilder()
+            .WithContentType(contentType)
+            .WithName("Page")
+            .WithPropertyValues(
+                new
+                {
+                    rte = rteValue
+                })
+            .Build();
+
+        var contentResult = ContentService.Save(content);
+        Assert.IsTrue(contentResult.Success);
+
+        var publishResult = ContentService.Publish(content, []);
+        Assert.IsTrue(publishResult.Success);
+
+        var publishedContent = await PublishedContentCache.GetByIdAsync(content.Key);
+        Assert.IsNotNull(publishedContent);
+
+        var publishedProperty = publishedContent.Properties.First(property => property.Alias == "rte");
+        Assert.AreEqual(expectedHasValue, publishedProperty.HasValue());
+
+        Assert.AreEqual(expectedHasValue, publishedContent.HasValue("rte"));
+    }
+
+    [TestCase(null, false)]
+    [TestCase("", false)]
+    [TestCase("""{"markup":"","blocks":null}""", false)]
+    [TestCase("""{"markup":"<p></p>","blocks":null}""", false)]
+    [TestCase("abc", true)]
+    [TestCase("""{"markup":"abc","blocks":null}""", true)]
+    public async Task Can_Handle_Empty_Value_Representations_For_Variant_Content(string? rteValue, bool expectedHasValue)
+    {
+        var contentType = await CreateContentTypeForEmptyValueTests(ContentVariation.Culture);
+
+        var content = new ContentBuilder()
+            .WithContentType(contentType)
+            .WithName("Page")
+            .WithCultureName("en-US", "Page")
+            .WithPropertyValues(
+                new
+                {
+                    rte = rteValue
+                },
+                "en-US")
+            .Build();
+
+        var contentResult = ContentService.Save(content);
+        Assert.IsTrue(contentResult.Success);
+
+        var publishResult = ContentService.Publish(content, ["en-US"]);
+        Assert.IsTrue(publishResult.Success);
+
+        var publishedContent = await PublishedContentCache.GetByIdAsync(content.Key);
+        Assert.IsNotNull(publishedContent);
+
+        var publishedProperty = publishedContent.Properties.First(property => property.Alias == "rte");
+        Assert.AreEqual(expectedHasValue, publishedProperty.HasValue("en-US"));
+
+        Assert.AreEqual(expectedHasValue, publishedContent.HasValue("rte", "en-US"));
+    }
+
+    private async Task<IContentType> CreateContentTypeForEmptyValueTests(ContentVariation contentVariation = ContentVariation.Nothing)
+    {
+        var contentType = new ContentTypeBuilder()
+            .WithAlias("myPage")
+            .WithName("My Page")
+            .WithContentVariation(contentVariation)
+            .AddPropertyGroup()
+                .WithAlias("content")
+                .WithName("Content")
+                .WithSupportsPublishing(true)
+                .AddPropertyType()
+                    .WithPropertyEditorAlias(Constants.PropertyEditors.Aliases.RichText)
+                    .WithDataTypeId(Constants.DataTypes.RichtextEditor)
+                    .WithValueStorageType(ValueStorageType.Ntext)
+                    .WithAlias("rte")
+                    .WithName("RTE")
+                    .WithVariations(contentVariation)
+                    .Done()
+                .Done()
+            .Build();
+
+        var contentTypeResult = await ContentTypeService.CreateAsync(contentType, Constants.Security.SuperUserKey);
+        Assert.IsTrue(contentTypeResult.Success);
+
+        return contentType;
+    }
 }