From f6f5723b7be0639cb4ff3ef0961db4f7d716725d Mon Sep 17 00:00:00 2001 From: Paul Johnson Date: Tue, 12 Oct 2021 09:18:42 +0100 Subject: [PATCH] Resolve incorrect ContentSavedState for failed publish Closes #11290 (for v8) --- .../Implement/DocumentRepository.cs | 14 +++++ .../Services/ContentServiceTests.cs | 63 ++++++++++++++++++- 2 files changed, 75 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs index f5d993070c04..5716fbe129c4 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs +++ b/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 diff --git a/src/Umbraco.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs index d9a90b74d808..64e88b94131d 100644 --- a/src/Umbraco.Tests/Services/ContentServiceTests.cs +++ b/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("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")); }