From 8cc6aee0cd2402edbf45d0770c4d96de97b6a7db Mon Sep 17 00:00:00 2001 From: Sven Geusens Date: Mon, 21 Aug 2023 09:31:33 +0200 Subject: [PATCH 1/7] WIP: Fix publish descendants and related notifications --- .../ContentPublishedNotification.cs | 2 + .../Notifications/NotificationExtensions.cs | 17 +++++++ src/Umbraco.Core/Services/ContentService.cs | 47 ++++++++++++++----- 3 files changed, 53 insertions(+), 13 deletions(-) diff --git a/src/Umbraco.Core/Notifications/ContentPublishedNotification.cs b/src/Umbraco.Core/Notifications/ContentPublishedNotification.cs index 0400155d3cb1..5e8ebd6bc2e3 100644 --- a/src/Umbraco.Core/Notifications/ContentPublishedNotification.cs +++ b/src/Umbraco.Core/Notifications/ContentPublishedNotification.cs @@ -19,4 +19,6 @@ public ContentPublishedNotification(IEnumerable target, EventMessages } public IEnumerable PublishedEntities => Target; + + public IEnumerable? RelatedPublishedEntities; } diff --git a/src/Umbraco.Core/Notifications/NotificationExtensions.cs b/src/Umbraco.Core/Notifications/NotificationExtensions.cs index 540cf0840a25..04d67bf0c523 100644 --- a/src/Umbraco.Core/Notifications/NotificationExtensions.cs +++ b/src/Umbraco.Core/Notifications/NotificationExtensions.cs @@ -1,3 +1,5 @@ +using Umbraco.Cms.Core.Models; + namespace Umbraco.Cms.Core.Notifications; public static class NotificationExtensions @@ -13,4 +15,19 @@ public static T WithStateFrom(this T notification, TSource source) where T : IStatefulNotification where TSource : IStatefulNotification => notification.WithState(source.State); + + public static ContentPublishedNotification WithRelatedPublishedEntities( + this ContentPublishedNotification notification, IList? relatedEntities) + { + if (relatedEntities?.Any() != true) + { + return notification; + } + + notification.RelatedPublishedEntities = relatedEntities.Where(relatedEntity => + notification.PublishedEntities.Any(publishedEntity => + publishedEntity.Id != relatedEntity.Id)); + + return notification; + } } diff --git a/src/Umbraco.Core/Services/ContentService.cs b/src/Umbraco.Core/Services/ContentService.cs index 1d8811ba5017..fc810f80c0e1 100644 --- a/src/Umbraco.Core/Services/ContentService.cs +++ b/src/Umbraco.Core/Services/ContentService.cs @@ -1950,10 +1950,10 @@ private bool SaveAndPublishBranch_PublishCultures(IContent content, HashSet(); // empty means 'already published' } - if (edited) - { + // if (edited) + // { cultures.Add(c); // means 'republish this culture' - } + // } return cultures; } @@ -2105,11 +2105,13 @@ internal IEnumerable SaveAndPublishBranch( } // 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 @@ internal IEnumerable SaveAndPublishBranch( int count; var page = 0; const int pageSize = 100; + PublishResult? result = null; do { count = 0; @@ -2140,7 +2143,8 @@ internal IEnumerable SaveAndPublishBranch( } // 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,13 @@ internal IEnumerable SaveAndPublishBranch( // (SaveAndPublishBranchOne does *not* do it) scope.Notifications.Publish( new ContentTreeChangeNotification(document, TreeChangeTypes.RefreshBranch, eventMessages)); - scope.Notifications.Publish(new ContentPublishedNotification(publishedDocuments, eventMessages)); + if (rootResult?.Success == true) + { + scope.Notifications.Publish( + new ContentPublishedNotification(rootResult!.Content!, eventMessages) + .WithState(rootPublishNotificationState) + .WithRelatedPublishedEntities(publishedDocuments)); + } scope.Complete(); } @@ -2175,6 +2185,9 @@ internal IEnumerable SaveAndPublishBranch( // 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 +2198,8 @@ internal IEnumerable SaveAndPublishBranch( ICollection publishedDocuments, EventMessages evtMsgs, int userId, - IReadOnlyCollection allLangs) + IReadOnlyCollection allLangs, + IDictionary? rootPublishingNotificationState) { HashSet? culturesToPublish = shouldPublish(document); @@ -2214,10 +2228,17 @@ internal IEnumerable SaveAndPublishBranch( 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) + { + return result; + } + + publishedDocuments.Add(document); + if (rootPublishingNotificationState == null) { - publishedDocuments.Add(document); + scope.Notifications.Publish(new ContentPublishedNotification(result.Content!, evtMsgs).WithState(notificationState)); } return result; From c50cd0223dd1bbe1dc7fa658f320552b426aab4e Mon Sep 17 00:00:00 2001 From: Sven Geusens Date: Wed, 30 Aug 2023 13:21:07 +0200 Subject: [PATCH 2/7] Removed related entitities from publish notification --- .../Notifications/ContentPublishedNotification.cs | 2 -- .../Notifications/NotificationExtensions.cs | 15 --------------- src/Umbraco.Core/Services/ContentService.cs | 3 +-- 3 files changed, 1 insertion(+), 19 deletions(-) diff --git a/src/Umbraco.Core/Notifications/ContentPublishedNotification.cs b/src/Umbraco.Core/Notifications/ContentPublishedNotification.cs index 5e8ebd6bc2e3..0400155d3cb1 100644 --- a/src/Umbraco.Core/Notifications/ContentPublishedNotification.cs +++ b/src/Umbraco.Core/Notifications/ContentPublishedNotification.cs @@ -19,6 +19,4 @@ public ContentPublishedNotification(IEnumerable target, EventMessages } public IEnumerable PublishedEntities => Target; - - public IEnumerable? RelatedPublishedEntities; } diff --git a/src/Umbraco.Core/Notifications/NotificationExtensions.cs b/src/Umbraco.Core/Notifications/NotificationExtensions.cs index 04d67bf0c523..a390f33362c5 100644 --- a/src/Umbraco.Core/Notifications/NotificationExtensions.cs +++ b/src/Umbraco.Core/Notifications/NotificationExtensions.cs @@ -15,19 +15,4 @@ public static T WithStateFrom(this T notification, TSource source) where T : IStatefulNotification where TSource : IStatefulNotification => notification.WithState(source.State); - - public static ContentPublishedNotification WithRelatedPublishedEntities( - this ContentPublishedNotification notification, IList? relatedEntities) - { - if (relatedEntities?.Any() != true) - { - return notification; - } - - notification.RelatedPublishedEntities = relatedEntities.Where(relatedEntity => - notification.PublishedEntities.Any(publishedEntity => - publishedEntity.Id != relatedEntity.Id)); - - return notification; - } } diff --git a/src/Umbraco.Core/Services/ContentService.cs b/src/Umbraco.Core/Services/ContentService.cs index fc810f80c0e1..692b4d8edfbe 100644 --- a/src/Umbraco.Core/Services/ContentService.cs +++ b/src/Umbraco.Core/Services/ContentService.cs @@ -2172,8 +2172,7 @@ internal IEnumerable SaveAndPublishBranch( { scope.Notifications.Publish( new ContentPublishedNotification(rootResult!.Content!, eventMessages) - .WithState(rootPublishNotificationState) - .WithRelatedPublishedEntities(publishedDocuments)); + .WithState(rootPublishNotificationState)); } scope.Complete(); From 561ff4fc35d2d28dc7475c000992b93ce9c49758 Mon Sep 17 00:00:00 2001 From: Sven Geusens Date: Wed, 30 Aug 2023 13:21:35 +0200 Subject: [PATCH 3/7] Fixed root not being saved when publishingWithDescendants --- src/Umbraco.Core/Services/ContentService.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Umbraco.Core/Services/ContentService.cs b/src/Umbraco.Core/Services/ContentService.cs index 692b4d8edfbe..6d6322f5f7a5 100644 --- a/src/Umbraco.Core/Services/ContentService.cs +++ b/src/Umbraco.Core/Services/ContentService.cs @@ -1950,10 +1950,10 @@ private bool SaveAndPublishBranch_PublishCultures(IContent content, HashSet(); // empty means 'already published' } - // if (edited) - // { + if (isRoot || edited) + { cultures.Add(c); // means 'republish this culture' - // } + } return cultures; } From 32eaa92cb7fb907a7277e1f6e603ebfd4458bd36 Mon Sep 17 00:00:00 2001 From: Sven Geusens Date: Mon, 4 Sep 2023 15:03:36 +0200 Subject: [PATCH 4/7] Updated integrationtests to reflect the update view on when to save the root when its part of a branch --- .../Services/ContentServicePublishBranchTests.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentServicePublishBranchTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentServicePublishBranchTests.cs index e5ef5789db3a..f6e6aaf0a789 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 void Can_Publish_Invariant_Branch(int method) 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 void Can_Publish_Invariant_Branch(int method) 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 void Can_Publish_Variant_Branch_When_No_Changes_On_Root_All_Cultures() 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 void Can_Publish_Variant_Branch_When_No_Changes_On_Root_Specific_Culture( 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 void Can_Publish_Mixed_Branch_1() 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 void Can_Publish_MixedBranch_2() 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); From 6b18273d60677737d5e372d9e3236414ead051f5 Mon Sep 17 00:00:00 2001 From: Sven Geusens Date: Tue, 5 Sep 2023 08:53:12 +0200 Subject: [PATCH 5/7] PR formatting fix Co-authored-by: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> --- src/Umbraco.Core/Services/ContentService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Core/Services/ContentService.cs b/src/Umbraco.Core/Services/ContentService.cs index 6d6322f5f7a5..913ffb21b1c7 100644 --- a/src/Umbraco.Core/Services/ContentService.cs +++ b/src/Umbraco.Core/Services/ContentService.cs @@ -2107,7 +2107,7 @@ internal IEnumerable SaveAndPublishBranch( // deal with the branch root - if it fails, abort var rootPublishNotificationState = new Dictionary(); PublishResult? rootResult = SaveAndPublishBranchItem(scope, document, shouldPublish, publishCultures, true, - publishedDocuments, eventMessages, userId, allLangs,rootPublishNotificationState); + publishedDocuments, eventMessages, userId, allLangs, rootPublishNotificationState); if (rootResult != null) { results.Add(rootResult); From ab451ac06041883b2c3d66e87acaf9346626ed58 Mon Sep 17 00:00:00 2001 From: Sven Geusens Date: Tue, 5 Sep 2023 08:54:07 +0200 Subject: [PATCH 6/7] PR Cleanup Co-authored-by: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> --- src/Umbraco.Core/Notifications/NotificationExtensions.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Umbraco.Core/Notifications/NotificationExtensions.cs b/src/Umbraco.Core/Notifications/NotificationExtensions.cs index a390f33362c5..540cf0840a25 100644 --- a/src/Umbraco.Core/Notifications/NotificationExtensions.cs +++ b/src/Umbraco.Core/Notifications/NotificationExtensions.cs @@ -1,5 +1,3 @@ -using Umbraco.Cms.Core.Models; - namespace Umbraco.Cms.Core.Notifications; public static class NotificationExtensions From 433b6f33209ff04c03e6c64416829059f515f225 Mon Sep 17 00:00:00 2001 From: Sven Geusens Date: Tue, 5 Sep 2023 08:54:41 +0200 Subject: [PATCH 7/7] Spicing up the codebase with some PR pattern matching Co-authored-by: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> --- src/Umbraco.Core/Services/ContentService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Core/Services/ContentService.cs b/src/Umbraco.Core/Services/ContentService.cs index 913ffb21b1c7..e9d0f8f4fd15 100644 --- a/src/Umbraco.Core/Services/ContentService.cs +++ b/src/Umbraco.Core/Services/ContentService.cs @@ -2168,7 +2168,7 @@ internal IEnumerable SaveAndPublishBranch( // (SaveAndPublishBranchOne does *not* do it) scope.Notifications.Publish( new ContentTreeChangeNotification(document, TreeChangeTypes.RefreshBranch, eventMessages)); - if (rootResult?.Success == true) + if (rootResult?.Success is true) { scope.Notifications.Publish( new ContentPublishedNotification(rootResult!.Content!, eventMessages)