From 90aebde107e676b6100cd4fa8a56e0b268bffbd9 Mon Sep 17 00:00:00 2001 From: Paul Johnson Date: Tue, 12 Oct 2021 09:18:42 +0100 Subject: [PATCH 1/2] Cherry picked fix from v8 and updated test for v9 --- .../Implement/DocumentRepository.cs | 14 +++ .../Services/ContentServiceTests.cs | 105 +++++++++++++++++- 2 files changed, 117 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DocumentRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DocumentRepository.cs index 1ed8404f78fd..4c9b19f1a91f 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DocumentRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DocumentRepository.cs @@ -553,6 +553,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) @@ -659,6 +660,19 @@ protected override void PersistUpdatedItem(IContent entity) 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()) { // names also impact 'edited' diff --git a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentServiceTests.cs b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentServiceTests.cs index d75d667b1ca7..25c8835ba87d 100644 --- a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentServiceTests.cs @@ -1348,6 +1348,107 @@ public void Cannot_Publish_Content_Awaiting_Release() Assert.AreEqual(PublishResultType.FailedPublishAwaitingRelease, published.Result); } + [Test] + public void Failed_Publish_Should_Not_Update_Edited_State_When_Edited_True() + { + // Arrange + IContentService contentService = GetRequiredService(); + IContentTypeService contentTypeService = GetRequiredService(); + + IContentType contentType = new ContentTypeBuilder() + .WithId(0) + .AddPropertyType() + .WithAlias("header") + .WithValueStorageType(ValueStorageType.Integer) + .WithPropertyEditorAlias(Constants.PropertyEditors.Aliases.TextBox) + .WithName("header") + .Done() + .WithContentVariation(ContentVariation.Nothing) + .Build(); + + contentTypeService.Save(contentType); + + Content content = new ContentBuilder() + .WithId(0) + .WithName("Home") + .WithContentType(contentType) + .AddPropertyData() + .WithKeyValue("header", "Cool header") + .Done() + .Build(); + + 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() + { + // Arrange + IContentService contentService = GetRequiredService(); + IContentTypeService contentTypeService = GetRequiredService(); + + IContentType contentType = new ContentTypeBuilder() + .WithId(0) + .AddPropertyType() + .WithAlias("header") + .WithValueStorageType(ValueStorageType.Integer) + .WithPropertyEditorAlias(Constants.PropertyEditors.Aliases.TextBox) + .WithName("header") + .Done() + .WithContentVariation(ContentVariation.Nothing) + .Build(); + + contentTypeService.Save(contentType); + + Content content = new ContentBuilder() + .WithId(0) + .WithName("Home") + .WithContentType(contentType) + .AddPropertyData() + .WithKeyValue("header", "Cool header") + .Done() + .Build(); + + 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() { @@ -2151,7 +2252,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); // till edited, change of behaviour Assert.AreEqual("Jane Doe", rollback2.GetValue("author")); Assert.AreEqual("Text Page 2 ReReUpdated", rollback2.Name); @@ -2170,7 +2271,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")); } From 303976b5d7c63e95079915b16847fe0e6c1e49b1 Mon Sep 17 00:00:00 2001 From: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> Date: Tue, 12 Oct 2021 11:30:49 +0200 Subject: [PATCH 2/2] Update src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentServiceTests.cs --- .../Umbraco.Infrastructure/Services/ContentServiceTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentServiceTests.cs b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentServiceTests.cs index 25c8835ba87d..ad189cc02a86 100644 --- a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentServiceTests.cs @@ -2252,7 +2252,7 @@ public void Can_Rollback_Version_On_Content() ContentService.Save(rollback2); Assert.IsTrue(rollback2.Published); - Assert.IsTrue(rollback2.Edited); // till edited, change of behaviour + Assert.IsTrue(rollback2.Edited); // Still edited, change of behaviour Assert.AreEqual("Jane Doe", rollback2.GetValue("author")); Assert.AreEqual("Text Page 2 ReReUpdated", rollback2.Name);