Resolve incorrect ContentSavedState for failed publish

Closes #11290 (for v8)

Author: Paul Johnson

src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs

@@ -520,6 +520,7 @@ protected override void PersistNewItem(IContent entity)
         protected override void PersistUpdatedItem(IContent entity)
         {
             var isEntityDirty = entity.IsDirty();
+            var editedSnapshot = entity.Edited;
 
             // check if we need to make any database changes at all
             if ((entity.PublishedState == PublishedState.Published || entity.PublishedState == PublishedState.Unpublished)
@@ -621,6 +622,19 @@ protected override void PersistUpdatedItem(IContent entity)
                 if (!publishing && entity.PublishName != entity.Name)
                     edited = true;
 
+                // To establish the new value of "edited" we compare all properties publishedValue to editedValue and look
+                // for differences.
+                //
+                // If we SaveAndPublish but the publish fails (e.g. already scheduled for release)
+                // we have lost the publishedValue on IContent (in memory vs database) so we cannot correctly make that comparison.
+                //
+                // This is a slight change to behaviour, historically a publish, followed by change & save, followed by undo change & save
+                // would change edited back to false.
+                if (!publishing && editedSnapshot)
+                {
+                    edited = true;
+                }
+
                 if (entity.ContentType.VariesByCulture())
                 {
                     // bump dates to align cultures to version

src/Umbraco.Tests/Services/ContentServiceTests.cs

@@ -1375,6 +1375,65 @@ public void Cannot_Publish_Content_Awaiting_Release()
             Assert.AreEqual(PublishResultType.FailedPublishAwaitingRelease, published.Result);
         }
 
+        // V9 - Tests.Integration
+        [Test]
+        public void Failed_Publish_Should_Not_Update_Edited_State_When_Edited_True()
+        {
+            const int rootNodeId = NodeDto.NodeIdSeed + 2;
+
+            // Arrange
+            var contentService = ServiceContext.ContentService;
+            var content = contentService.GetById(rootNodeId);
+            contentService.SaveAndPublish(content);
+
+            content.Properties[0].SetValue("Foo", culture: string.Empty);
+            content.ContentSchedule.Add(DateTime.Now.AddHours(2), null);
+            contentService.Save(content);
+
+            // Act
+            var result = contentService.SaveAndPublish(content, userId: Constants.Security.SuperUserId);
+
+            // Assert
+            Assert.Multiple(() =>
+            {
+                Assert.IsFalse(result.Success);
+                Assert.IsTrue(result.Content.Published);
+                Assert.AreEqual(PublishResultType.FailedPublishAwaitingRelease, result.Result);
+
+                // We changed property data
+                Assert.IsTrue(result.Content.Edited, "result.Content.Edited");
+            });
+        }
+
+        // V9 - Tests.Integration
+        [Test]
+        public void Failed_Publish_Should_Not_Update_Edited_State_When_Edited_False()
+        {
+            const int rootNodeId = NodeDto.NodeIdSeed + 2;
+
+            // Arrange
+            var contentService = ServiceContext.ContentService;
+            var content = contentService.GetById(rootNodeId);
+            contentService.SaveAndPublish(content);
+
+            content.ContentSchedule.Add(DateTime.Now.AddHours(2), null);
+            contentService.Save(content);
+
+            // Act
+            var result = contentService.SaveAndPublish(content, userId: Constants.Security.SuperUserId);
+
+            // Assert
+            Assert.Multiple(() =>
+            {
+                Assert.IsFalse(result.Success);
+                Assert.IsTrue(result.Content.Published);
+                Assert.AreEqual(PublishResultType.FailedPublishAwaitingRelease, result.Result);
+
+                // We didn't change any property data
+                Assert.IsFalse(result.Content.Edited, "result.Content.Edited");
+            });
+        }
+
         [Test]
         public void Cannot_Publish_Culture_Awaiting_Release()
         {
@@ -2176,7 +2235,7 @@ public void Can_Rollback_Version_On_Content()
             contentService.Save(rollback2);
 
             Assert.IsTrue(rollback2.Published);
-            Assert.IsFalse(rollback2.Edited); // all changes cleared!
+            Assert.IsTrue(rollback2.Edited); // Still edited, change of behaviour
 
             Assert.AreEqual("Jane Doe", rollback2.GetValue<string>("author"));
             Assert.AreEqual("Text Page 2 ReReUpdated", rollback2.Name);
@@ -2195,7 +2254,7 @@ public void Can_Rollback_Version_On_Content()
             content.CopyFrom(rollto);
             content.Name = rollto.PublishName; // must do it explicitely AND must pick the publish one!
             contentService.Save(content);
-            Assert.IsFalse(content.Edited);
+            Assert.IsTrue(content.Edited); // Still edited, change of behaviour
             Assert.AreEqual("Text Page 2 ReReUpdated", content.Name);
             Assert.AreEqual("Jane Doe", content.GetValue("author"));
         }