From f750bca453c12d4cc4321868eb1197c9916f8243 Mon Sep 17 00:00:00 2001 From: Sven Geusens Date: Tue, 5 Sep 2023 10:14:06 +0200 Subject: [PATCH] V10/bugfix/14543 publish descendants (#14763) * WIP: Fix publish descendants and related notifications * Removed related entitities from publish notification * Fixed root not being saved when publishingWithDescendants * Updated integrationtests to reflect the update view on when to save the root when its part of a branch * PR formatting fix Co-authored-by: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> * PR Cleanup Co-authored-by: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> * Spicing up the codebase with some PR pattern matching Co-authored-by: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> --------- Co-authored-by: Sven Geusens Co-authored-by: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> --- src/Umbraco.Core/Services/ContentService.cs | 42 ++++++++++++++----- .../ContentServicePublishBranchTests.cs | 12 +++--- 2 files changed, 37 insertions(+), 17 deletions(-) diff --git a/src/Umbraco.Core/Services/ContentService.cs b/src/Umbraco.Core/Services/ContentService.cs index 1d8811ba50..e9d0f8f4fd 100644 --- a/src/Umbraco.Core/Services/ContentService.cs +++ b/src/Umbraco.Core/Services/ContentService.cs @@ -1950,7 +1950,7 @@ public class ContentService : RepositoryService, IContentService cultures = new HashSet(); // empty means 'already published' } - if (edited) + if (isRoot || edited) { cultures.Add(c); // means 'republish this culture' } @@ -2105,11 +2105,13 @@ public class ContentService : RepositoryService, IContentService } // deal with the branch root - if it fails, abort - PublishResult? result = SaveAndPublishBranchItem(scope, document, shouldPublish, publishCultures, true, publishedDocuments, eventMessages, userId, allLangs); - if (result != null) + var rootPublishNotificationState = new Dictionary(); + PublishResult? rootResult = SaveAndPublishBranchItem(scope, document, shouldPublish, publishCultures, true, + publishedDocuments, eventMessages, userId, allLangs, rootPublishNotificationState); + if (rootResult != null) { - results.Add(result); - if (!result.Success) + results.Add(rootResult); + if (!rootResult.Success) { return results; } @@ -2122,6 +2124,7 @@ public class ContentService : RepositoryService, IContentService int count; var page = 0; const int pageSize = 100; + PublishResult? result = null; do { count = 0; @@ -2140,7 +2143,8 @@ public class ContentService : RepositoryService, IContentService } // no need to check path here, parent has to be published here - result = SaveAndPublishBranchItem(scope, d, shouldPublish, publishCultures, false, publishedDocuments, eventMessages, userId, allLangs); + result = SaveAndPublishBranchItem(scope, d, shouldPublish, publishCultures, false, + publishedDocuments, eventMessages, userId, allLangs,null); if (result != null) { results.Add(result); @@ -2164,7 +2168,12 @@ public class ContentService : RepositoryService, IContentService // (SaveAndPublishBranchOne does *not* do it) scope.Notifications.Publish( new ContentTreeChangeNotification(document, TreeChangeTypes.RefreshBranch, eventMessages)); - scope.Notifications.Publish(new ContentPublishedNotification(publishedDocuments, eventMessages)); + if (rootResult?.Success is true) + { + scope.Notifications.Publish( + new ContentPublishedNotification(rootResult!.Content!, eventMessages) + .WithState(rootPublishNotificationState)); + } scope.Complete(); } @@ -2175,6 +2184,9 @@ public class ContentService : RepositoryService, IContentService // shouldPublish: a function determining whether the document has changes that need to be published // note - 'force' is handled by 'editing' // publishValues: a function publishing values (using the appropriate PublishCulture calls) + /// Only set this when processing a the root of the branch + /// Published notification will not be send when this property is set + /// private PublishResult? SaveAndPublishBranchItem( ICoreScope scope, IContent document, @@ -2185,7 +2197,8 @@ public class ContentService : RepositoryService, IContentService ICollection publishedDocuments, EventMessages evtMsgs, int userId, - IReadOnlyCollection allLangs) + IReadOnlyCollection allLangs, + IDictionary? rootPublishingNotificationState) { HashSet? culturesToPublish = shouldPublish(document); @@ -2214,10 +2227,17 @@ public class ContentService : RepositoryService, IContentService return new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, document); } - PublishResult result = CommitDocumentChangesInternal(scope, document, evtMsgs, allLangs, savingNotification.State, userId, true, isRoot); - if (result.Success) + var notificationState = rootPublishingNotificationState ?? new Dictionary(); + PublishResult result = CommitDocumentChangesInternal(scope, document, evtMsgs, allLangs, notificationState, userId, true, isRoot); + if (!result.Success) { - publishedDocuments.Add(document); + return result; + } + + publishedDocuments.Add(document); + if (rootPublishingNotificationState == null) + { + scope.Notifications.Publish(new ContentPublishedNotification(result.Content!, evtMsgs).WithState(notificationState)); } return result; diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentServicePublishBranchTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentServicePublishBranchTests.cs index e5ef5789db..f6e6aaf0a7 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentServicePublishBranchTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentServicePublishBranchTests.cs @@ -91,7 +91,7 @@ public class ContentServicePublishBranchTests : UmbracoIntegrationTest AssertPublishResults( r, x => x.Result, - PublishResultType.SuccessPublishAlready, + PublishResultType.SuccessPublish, // During branch publishing, the change detection of the root branch runs AFTER the check to process the branchItem => root is always saved&Published as the intent requires it. PublishResultType.SuccessPublishAlready, PublishResultType.SuccessPublishAlready); @@ -138,7 +138,7 @@ public class ContentServicePublishBranchTests : UmbracoIntegrationTest AssertPublishResults( r, x => x.Result, - PublishResultType.SuccessPublishAlready, + PublishResultType.SuccessPublish, // During branch publishing, the change detection of the root branch runs AFTER the check to process the branchItem => root is always saved&Published as the intent requires it. PublishResultType.SuccessPublishAlready, PublishResultType.SuccessPublishAlready, PublishResultType.SuccessPublish, @@ -183,7 +183,7 @@ public class ContentServicePublishBranchTests : UmbracoIntegrationTest var r = ContentService.SaveAndPublishBranch(vRoot, false) .ToArray(); // no culture specified so "*" is used, so all cultures - Assert.AreEqual(PublishResultType.SuccessPublishAlready, r[0].Result); + Assert.AreEqual(PublishResultType.SuccessPublishCulture, r[0].Result); // During branch publishing, the change detection of the root branch runs AFTER the check to process the branchItem => root is always saved&Published as the intent requires it. Assert.AreEqual(PublishResultType.SuccessPublishCulture, r[1].Result); } @@ -219,7 +219,7 @@ public class ContentServicePublishBranchTests : UmbracoIntegrationTest var saveResult = ContentService.Save(iv1); var r = ContentService.SaveAndPublishBranch(vRoot, false, "de").ToArray(); - Assert.AreEqual(PublishResultType.SuccessPublishAlready, r[0].Result); + Assert.AreEqual(PublishResultType.SuccessPublishCulture, r[0].Result); // During branch publishing, the change detection of the root branch runs AFTER the check to process the branchItem => root is always saved&Published as the intent requires it. Assert.AreEqual(PublishResultType.SuccessPublishCulture, r[1].Result); } @@ -379,7 +379,7 @@ public class ContentServicePublishBranchTests : UmbracoIntegrationTest AssertPublishResults( r, x => x.Result, - PublishResultType.SuccessPublishAlready, + PublishResultType.SuccessPublish, // During branch publishing, the change detection of the root branch runs AFTER the check to process the branchItem => root is always saved&Published as the intent requires it. PublishResultType.SuccessPublish, PublishResultType.SuccessPublishCulture); @@ -405,7 +405,7 @@ public class ContentServicePublishBranchTests : UmbracoIntegrationTest AssertPublishResults( r, x => x.Result, - PublishResultType.SuccessPublishAlready, + PublishResultType.SuccessPublish, // During branch publishing, the change detection of the root branch runs AFTER the check to process the branchItem => root is always saved&Published as the intent requires it. PublishResultType.SuccessPublish, PublishResultType.SuccessPublishCulture);