Make culture specific update dates work again (#19145)

Author: kjac

src/Umbraco.Core/Models/ContentBase.cs

@@ -301,7 +301,7 @@ public void SetCultureName(string? name, string? culture)
             }
 
             // set
-            else
+            else if (GetCultureName(culture) != name)
             {
                 this.SetCultureInfo(culture!, name, DateTime.Now);
             }
@@ -455,10 +455,12 @@ public void SetValue(string propertyTypeAlias, object? value, string? culture =
                 $"No PropertyType exists with the supplied alias \"{propertyTypeAlias}\".");
         }
 
-        property?.SetValue(value, culture, segment);
-
-        // bump the culture to be flagged for updating
-        this.TouchCulture(culture);
+        var updated = property.SetValue(value, culture, segment);
+        if (updated)
+        {
+            // bump the culture to be flagged for updating
+            this.TouchCulture(culture);
+        }
     }
 
     /// <inheritdoc />

src/Umbraco.Core/Models/Entities/BeingDirtyBase.cs

@@ -162,7 +162,8 @@ protected void SetPropertyValueAndDetectChanges<T>(T? value, ref T? valueRef, st
     /// <param name="propertyName">The property name.</param>
     /// <param name="comparer">A comparer to compare property values.</param>
     /// <param name="changed">A value indicating whether we know values have changed and no comparison is required.</param>
-    protected void DetectChanges<T>(T value, T orig, string propertyName, IEqualityComparer<T> comparer, bool changed)
+    /// <returns>True if a change was detected, false otherwise.</returns>
+    protected bool DetectChanges<T>(T value, T orig, string propertyName, IEqualityComparer<T> comparer, bool changed)
     {
         // compare values
         changed = _withChanges && (changed || !comparer.Equals(orig, value));
@@ -172,6 +173,8 @@ protected void DetectChanges<T>(T value, T orig, string propertyName, IEqualityC
         {
             OnPropertyChanged(propertyName);
         }
+
+        return changed;
     }
 
     #endregion

src/Umbraco.Core/Models/IProperty.cs

@@ -32,7 +32,12 @@ public interface IProperty : IEntity, IRememberBeingDirty
     /// <summary>
     ///     Sets a value.
     /// </summary>
-    void SetValue(object? value, string? culture = null, string? segment = null);
+    /// <returns>true if the value was set (updated), false otherwise.</returns>
+    /// <remarks>
+    /// A false return value does not indicate failure, but rather that the property value was not changed
+    /// (i.e. the value passed in was equal to the current property value).
+    /// </remarks>
+    bool SetValue(object? value, string? culture = null, string? segment = null);
 
     void PublishValues(string? culture = "*", string segment = "*");
 

src/Umbraco.Core/Models/Property.cs

@@ -257,10 +257,8 @@ public void UnpublishValues(string? culture = "*", string? segment = "*")
         }
     }
 
-    /// <summary>
-    ///     Sets a value.
-    /// </summary>
-    public void SetValue(object? value, string? culture = null, string? segment = null)
+    /// <inheritdoc />
+    public bool SetValue(object? value, string? culture = null, string? segment = null)
     {
         culture = culture?.NullOrWhiteSpaceAsNull();
         segment = segment?.NullOrWhiteSpaceAsNull();
@@ -273,15 +271,18 @@ public void SetValue(object? value, string? culture = null, string? segment = nu
 
         (IPropertyValue? pvalue, var change) = GetPValue(culture, segment, true);
 
+        var changed = false;
         if (pvalue is not null)
         {
             var origValue = pvalue.EditedValue;
             var setValue = ConvertAssignedValue(value);
 
             pvalue.EditedValue = setValue;
 
-            DetectChanges(setValue, origValue, nameof(Values), PropertyValueComparer, change);
+            changed = DetectChanges(setValue, origValue, nameof(Values), PropertyValueComparer, change);
         }
+
+        return changed;
     }
 
     public object? ConvertAssignedValue(object? value) =>

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

@@ -347,4 +347,45 @@ public async Task Can_Publish_Culture_With_Unpublished_Parent_Culture()
             Constants.Security.SuperUserKey);
         Assert.IsTrue(publishAttempt.Success);
     }
+
+    [Test]
+    public async Task Republishing_Single_Culture_Does_Not_Change_Publish_Or_Update_Date_For_Other_Cultures()
+    {
+        var (langEn, langDa, langBe, contentType) = await SetupVariantDoctypeAsync();
+        var setupData = await CreateVariantContentAsync(langEn, langDa, langBe, contentType);
+
+        var publishAttempt = await ContentPublishingService.PublishAsync(
+            setupData.Key,
+            [
+                new() { Culture = langEn.IsoCode },
+                new() { Culture = langDa.IsoCode },
+                new() { Culture = langBe.IsoCode },
+            ],
+            Constants.Security.SuperUserKey);
+        Assert.IsTrue(publishAttempt.Success);
+
+        var content = ContentService.GetById(setupData.Key)!;
+        var firstPublishDateEn = content.GetPublishDate(langEn.IsoCode)
+                                 ?? throw new InvalidOperationException("Expected a publish date for EN");
+        var firstPublishDateDa = content.GetPublishDate(langDa.IsoCode)
+                                 ?? throw new InvalidOperationException("Expected a publish date for DA");
+        var firstPublishDateBe = content.GetPublishDate(langBe.IsoCode)
+                                 ?? throw new InvalidOperationException("Expected a publish date for BE");
+
+        Thread.Sleep(100);
+
+        publishAttempt = await ContentPublishingService.PublishAsync(
+            content.Key,
+            [new() { Culture = langEn.IsoCode }],
+            Constants.Security.SuperUserKey);
+        Assert.IsTrue(publishAttempt.Success);
+
+        content = ContentService.GetById(content.Key)!;
+        Assert.AreEqual(firstPublishDateDa, content.GetPublishDate(langDa.IsoCode));
+        Assert.AreEqual(firstPublishDateBe, content.GetPublishDate(langBe.IsoCode));
+
+        var lastPublishDateEn = content.GetPublishDate(langEn.IsoCode)
+                                ?? throw new InvalidOperationException("Expected a publish date for EN");
+        Assert.Greater(lastPublishDateEn, firstPublishDateEn);
+    }
 }

tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.Update.cs

@@ -567,4 +567,115 @@ public async Task Cannot_Update_Variant_Readonly_Property_Value()
             Assert.AreEqual("The initial Danish label value", content.GetValue<string>("variantLabel", "da-DK"));
         });
     }
+
+    [Test]
+    public async Task Updating_Single_Variant_Name_Does_Not_Change_Update_Dates_Of_Other_Vaiants()
+    {
+        var contentType = await CreateVariantContentType(variantTitleAsMandatory: false);
+
+        var createModel = new ContentCreateModel
+        {
+            ContentTypeKey = contentType.Key,
+            ParentKey = Constants.System.RootKey,
+            Properties = [],
+            Variants =
+            [
+                new VariantModel { Culture = "en-US", Name = "Initial English Name" },
+                new VariantModel { Culture = "da-DK", Name = "Initial Danish Name" }
+            ],
+        };
+
+        var createResult = await ContentEditingService.CreateAsync(createModel, Constants.Security.SuperUserKey);
+        Assert.IsTrue(createResult.Success);
+
+        var firstUpdateDateEn = createResult.Result.Content!.GetUpdateDate("en-US")!;
+        var firstUpdateDateDa = createResult.Result.Content!.GetUpdateDate("da-DK")!;
+
+        Thread.Sleep(100);
+
+        var updateModel = new ContentUpdateModel
+        {
+            Properties = [],
+            Variants =
+            [
+                new VariantModel { Culture = "en-US", Name = "Updated English Name" },
+                new VariantModel { Culture = "da-DK", Name = "Initial Danish Name" }
+            ]
+        };
+
+        var updateResult = await ContentEditingService.UpdateAsync(createResult.Result.Content.Key, updateModel, Constants.Security.SuperUserKey);
+        Assert.IsTrue(updateResult.Success);
+        Assert.AreEqual(ContentEditingOperationStatus.Success, updateResult.Status);
+        VerifyUpdate(updateResult.Result.Content);
+
+        // re-get and re-test
+        VerifyUpdate(await ContentEditingService.GetAsync(updateResult.Result.Content!.Key));
+
+        void VerifyUpdate(IContent? updatedContent)
+        {
+            Assert.IsNotNull(updatedContent);
+            Assert.AreEqual(firstUpdateDateDa, updatedContent.GetUpdateDate("da-DK"));
+
+            var lastUpdateDateEn = updatedContent.GetUpdateDate("en-US")
+                                   ?? throw new InvalidOperationException("Expected a publish date for EN");
+            Assert.Greater(lastUpdateDateEn, firstUpdateDateEn);
+        }
+    }
+
+    [Test]
+    public async Task Updating_Single_Variant_Property_Does_Not_Change_Update_Dates_Of_Other_Variants()
+    {
+        var content = await CreateCultureVariantContent();
+        var firstUpdateDateEn = content.GetUpdateDate("en-US")
+                                ?? throw new InvalidOperationException("Expected an update date for EN");
+        var firstUpdateDateDa = content.GetUpdateDate("da-DK")
+                                ?? throw new InvalidOperationException("Expected an update date for DA");
+
+        var updateModel = new ContentUpdateModel
+        {
+            Properties =
+            [
+                new PropertyValueModel
+                {
+                    Alias = "invariantTitle",
+                    Value = "The invariant title"
+                },
+                new PropertyValueModel
+                {
+                    Culture = "en-US",
+                    Alias = "variantTitle",
+                    Value = content.GetValue<string>("variantTitle", "en-US")!
+                },
+                new PropertyValueModel
+                {
+                    Culture = "da-DK",
+                    Alias = "variantTitle",
+                    Value = "The updated Danish title"
+                }
+            ],
+            Variants =
+            [
+                new VariantModel { Culture = "en-US", Name = content.GetCultureName("en-US")! },
+                new VariantModel { Culture = "da-DK", Name = content.GetCultureName("da-DK")! }
+            ]
+        };
+
+        var result = await ContentEditingService.UpdateAsync(content.Key, updateModel, Constants.Security.SuperUserKey);
+        Assert.IsTrue(result.Success);
+        Assert.AreEqual(ContentEditingOperationStatus.Success, result.Status);
+        VerifyUpdate(result.Result.Content);
+
+        // re-get and re-test
+        VerifyUpdate(await ContentEditingService.GetAsync(content.Key));
+
+        void VerifyUpdate(IContent? updatedContent)
+        {
+            Assert.IsNotNull(updatedContent);
+            Assert.AreEqual(firstUpdateDateEn, updatedContent.GetUpdateDate("en-US"));
+
+            var lastUpdateDateDa = updatedContent.GetUpdateDate("da-DK")
+                                   ?? throw new InvalidOperationException("Expected an update date for DA");
+            Assert.Greater(lastUpdateDateDa, firstUpdateDateDa);
+        }
+    }
 }

tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTestsBase.cs

@@ -74,7 +74,7 @@ protected IContentType CreateInvariantContentType(params ITemplate[] templates)
         return contentType;
     }
 
-    protected async Task<IContentType> CreateVariantContentType(ContentVariation variation = ContentVariation.Culture)
+    protected async Task<IContentType> CreateVariantContentType(ContentVariation variation = ContentVariation.Culture, bool variantTitleAsMandatory = true)
     {
         var language = new LanguageBuilder()
             .WithCultureInfo("da-DK")
@@ -88,7 +88,7 @@ protected async Task<IContentType> CreateVariantContentType(ContentVariation var
             .AddPropertyType()
             .WithAlias("variantTitle")
             .WithName("Variant Title")
-            .WithMandatory(true)
+            .WithMandatory(variantTitleAsMandatory)
             .WithVariations(variation)
             .Done()
             .AddPropertyType()

tests/Umbraco.Tests.UnitTests/Umbraco.Core/Models/ContentTests.cs

@@ -29,8 +29,9 @@ public class ContentTests
 
     private readonly PropertyEditorCollection _propertyEditorCollection = new (new DataEditorCollection(() => []));
 
-    [Test]
-    public void Variant_Culture_Names_Track_Dirty_Changes()
+    [TestCase("name-fr", false)]
+    [TestCase("name-fr-updated", true)]
+    public void Variant_Culture_Names_Track_Dirty_Changes(string newName, bool expectedDirty)
     {
         var contentType = new ContentTypeBuilder()
             .WithAlias("contentType")
@@ -58,14 +59,17 @@ public void Variant_Culture_Names_Track_Dirty_Changes()
         Assert.IsTrue(frCultureName.IsPropertyDirty("Date"));
 
         content.ResetDirtyProperties();
+        frCultureName.ResetDirtyProperties();
 
         Assert.IsFalse(content.IsPropertyDirty("CultureInfos")); // it's been reset
         Assert.IsTrue(content.WasPropertyDirty("CultureInfos"));
 
         Thread.Sleep(500); // The "Date" wont be dirty if the test runs too fast since it will be the same date
-        content.SetCultureName("name-fr", langFr);
-        Assert.IsTrue(frCultureName.IsPropertyDirty("Date"));
-        Assert.IsTrue(content.IsPropertyDirty("CultureInfos")); // it's true now since we've updated a name
+        content.SetCultureName(newName, langFr);
+
+        // dirty is only true if we updated the name
+        Assert.AreEqual(expectedDirty, frCultureName.IsPropertyDirty("Date"));
+        Assert.AreEqual(expectedDirty, content.IsPropertyDirty("CultureInfos"));
     }
 
     [Test]
@@ -97,6 +101,7 @@ public void Variant_Published_Culture_Names_Track_Dirty_Changes()
         Assert.IsTrue(frCultureName.IsPropertyDirty("Date"));
 
         content.ResetDirtyProperties();
+        frCultureName.ResetDirtyProperties();
 
         Assert.IsFalse(content.IsPropertyDirty("PublishCultureInfos")); // it's been reset
         Assert.IsTrue(content.WasPropertyDirty("PublishCultureInfos"));