Split force for publish descendants into separate options for publish unpublish and re-publish unedited (15) (#18270)

* Port server-side updates from 13 implementation.

* Update openapi.json

* Update typed client.

* Ported over front-end amend from 13.

* Handled edge case of publishing invariant root with variant descendants.

* Refactor to enum.

* Resolved CodeScene warning.

* Resolved CodeScene warning.

* Resolved CodeScene warning.

* Applied suggestions from code review.

* Reverted breaking change in integration tests.

* Refactored method name.

Author: AndyButland

src/Umbraco.Cms.Api.Management/Controllers/Document/PublishDocumentWithDescendantsController.cs

@@ -1,11 +1,11 @@
-using Asp.Versioning;
+using Asp.Versioning;
 using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc;
-using Umbraco.Cms.Api.Management.Security.Authorization.Content;
 using Umbraco.Cms.Api.Management.ViewModels.Document;
 using Umbraco.Cms.Core;
 using Umbraco.Cms.Core.Actions;
+using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.Models.ContentPublishing;
 using Umbraco.Cms.Core.Security;
 using Umbraco.Cms.Core.Security.Authorization;
@@ -53,11 +53,27 @@ public async Task<IActionResult> PublishWithDescendants(CancellationToken cancel
         Attempt<ContentPublishingBranchResult, ContentPublishingOperationStatus> attempt = await _contentPublishingService.PublishBranchAsync(
             id,
             requestModel.Cultures,
-            requestModel.IncludeUnpublishedDescendants,
+            BuildPublishBranchFilter(requestModel),
             CurrentUserKey(_backOfficeSecurityAccessor));
 
         return attempt.Success
             ? Ok()
             : DocumentPublishingOperationStatusResult(attempt.Status, failedBranchItems: attempt.Result.FailedItems);
     }
+
+    private static PublishBranchFilter BuildPublishBranchFilter(PublishDocumentWithDescendantsRequestModel requestModel)
+    {
+        PublishBranchFilter publishBranchFilter = PublishBranchFilter.Default;
+        if (requestModel.IncludeUnpublishedDescendants)
+        {
+            publishBranchFilter |= PublishBranchFilter.IncludeUnpublished;
+        }
+
+        if (requestModel.ForceRepublish)
+        {
+            publishBranchFilter |= PublishBranchFilter.ForceRepublish;
+        }
+
+        return publishBranchFilter;
+    }
 }

src/Umbraco.Cms.Api.Management/OpenApi.json

@@ -42795,13 +42795,17 @@
       "PublishDocumentWithDescendantsRequestModel": {
         "required": [
           "cultures",
+          "forceRepublish",
           "includeUnpublishedDescendants"
         ],
         "type": "object",
         "properties": {
           "includeUnpublishedDescendants": {
             "type": "boolean"
           },
+          "forceRepublish": {
+            "type": "boolean"
+          },
           "cultures": {
             "type": "array",
             "items": {

src/Umbraco.Cms.Api.Management/ViewModels/Document/PublishDocumentWithDescendantsRequestModel.cs

@@ -1,8 +1,10 @@
-namespace Umbraco.Cms.Api.Management.ViewModels.Document;
+namespace Umbraco.Cms.Api.Management.ViewModels.Document;
 
 public class PublishDocumentWithDescendantsRequestModel
 {
     public bool IncludeUnpublishedDescendants { get; set; }
 
+    public bool ForceRepublish { get; set; }
+
     public required IEnumerable<string> Cultures { get; set; }
 }

src/Umbraco.Core/Models/PublishBranchFilter.cs

@@ -0,0 +1,28 @@
+namespace Umbraco.Cms.Core.Models;
+
+/// <summary>
+/// Describes the options available with publishing a content branch for force publishing.
+/// </summary>
+[Flags]
+public enum PublishBranchFilter
+{
+    /// <summary>
+    /// The default behavior is to publish only the published content that has changed.
+    /// </summary>
+    Default = 0,
+
+    /// <summary>
+    /// For publishing a branch, publish all changed content, including content that is not published.
+    /// </summary>
+    IncludeUnpublished = 1,
+
+    /// <summary>
+    /// For publishing a branch, force republishing of all published content, including content that has not changed.
+    /// </summary>
+    ForceRepublish = 2,
+
+    /// <summary>
+    /// For publishing a branch, publish all content, including content that is not published and content that has not changed.
+    /// </summary>
+    All = IncludeUnpublished | ForceRepublish,
+}

src/Umbraco.Core/Services/ContentPublishingService.cs

@@ -239,7 +239,12 @@ private async Task<ContentValidationResult> ValidateCurrentContentAsync(IContent
     }
 
     /// <inheritdoc />
+    [Obsolete("This method is not longer used as the 'force' parameter has been split into publishing unpublished and force re-published. Please use the overload containing parameters for those options instead. Will be removed in V17.")]
     public async Task<Attempt<ContentPublishingBranchResult, ContentPublishingOperationStatus>> PublishBranchAsync(Guid key, IEnumerable<string> cultures, bool force, Guid userKey)
+        => await PublishBranchAsync(key, cultures, force ? PublishBranchFilter.IncludeUnpublished : PublishBranchFilter.Default, userKey);
+
+    /// <inheritdoc />
+    public async Task<Attempt<ContentPublishingBranchResult, ContentPublishingOperationStatus>> PublishBranchAsync(Guid key, IEnumerable<string> cultures, PublishBranchFilter publishBranchFilter, Guid userKey)
     {
         using ICoreScope scope = _coreScopeProvider.CreateCoreScope();
         IContent? content = _contentService.GetById(key);
@@ -260,7 +265,7 @@ public async Task<Attempt<ContentPublishingBranchResult, ContentPublishingOperat
         }
 
         var userId = await _userIdKeyResolver.GetAsync(userKey);
-        IEnumerable<PublishResult> result = _contentService.PublishBranch(content, force, cultures.ToArray(), userId);
+        IEnumerable<PublishResult> result = _contentService.PublishBranch(content, publishBranchFilter, cultures.ToArray(), userId);
         scope.Complete();
 
         var itemResults = result.ToDictionary(r => r.Content.Key, ToContentPublishingOperationStatus);

src/Umbraco.Core/Services/ContentService.cs

@@ -2009,15 +2009,15 @@ private bool PublishBranch_PublishCultures(IContent content, HashSet<string> cul
                && _propertyValidationService.Value.IsPropertyDataValid(content, out _, _cultureImpactFactory.ImpactInvariant());
     }
 
-    // utility 'ShouldPublish' func used by SaveAndPublishBranch
-    private static HashSet<string>? PublishBranch_ShouldPublish(ref HashSet<string>? cultures, string c, bool published, bool edited, bool isRoot, bool force)
+    // utility 'ShouldPublish' func used by PublishBranch
+    private static HashSet<string>? PublishBranch_ShouldPublish(ref HashSet<string>? cultures, string c, bool published, bool edited, bool isRoot, PublishBranchFilter publishBranchFilter)
     {
         // if published, republish
         if (published)
         {
             cultures ??= new HashSet<string>(); // empty means 'already published'
 
-            if (edited)
+            if (edited || publishBranchFilter.HasFlag(PublishBranchFilter.ForceRepublish))
             {
                 cultures.Add(c); // <culture> means 'republish this culture'
             }
@@ -2026,7 +2026,7 @@ private bool PublishBranch_PublishCultures(IContent content, HashSet<string> cul
         }
 
         // if not published, publish if force/root else do nothing
-        if (!force && !isRoot)
+        if (!publishBranchFilter.HasFlag(PublishBranchFilter.IncludeUnpublished) && !isRoot)
         {
             return cultures; // null means 'nothing to do'
         }
@@ -2054,16 +2054,18 @@ public IEnumerable<PublishResult> SaveAndPublishBranch(IContent content, bool fo
             var isRoot = c.Id == content.Id;
             HashSet<string>? culturesToPublish = null;
 
+            PublishBranchFilter publishBranchFilter = force ? PublishBranchFilter.IncludeUnpublished : PublishBranchFilter.Default;
+
             // invariant content type
             if (!c.ContentType.VariesByCulture())
             {
-                return PublishBranch_ShouldPublish(ref culturesToPublish, "*", c.Published, c.Edited, isRoot, force);
+                return PublishBranch_ShouldPublish(ref culturesToPublish, "*", c.Published, c.Edited, isRoot, publishBranchFilter);
             }
 
             // variant content type, specific culture
             if (culture != "*")
             {
-                return PublishBranch_ShouldPublish(ref culturesToPublish, culture, c.IsCulturePublished(culture), c.IsCultureEdited(culture), isRoot, force);
+                return PublishBranch_ShouldPublish(ref culturesToPublish, culture, c.IsCulturePublished(culture), c.IsCultureEdited(culture), isRoot, publishBranchFilter);
             }
 
             // variant content type, all cultures
@@ -2073,7 +2075,7 @@ public IEnumerable<PublishResult> SaveAndPublishBranch(IContent content, bool fo
                 // others will have to 'republish this culture'
                 foreach (var x in c.AvailableCultures)
                 {
-                    PublishBranch_ShouldPublish(ref culturesToPublish, x, c.IsCulturePublished(x), c.IsCultureEdited(x), isRoot, force);
+                    PublishBranch_ShouldPublish(ref culturesToPublish, x, c.IsCulturePublished(x), c.IsCultureEdited(x), isRoot, publishBranchFilter);
                 }
 
                 return culturesToPublish;
@@ -2085,23 +2087,31 @@ public IEnumerable<PublishResult> SaveAndPublishBranch(IContent content, bool fo
                 : null; // null means 'nothing to do'
         }
 
-        return PublishBranch(content, force, ShouldPublish, PublishBranch_PublishCultures, userId);
+        return PublishBranch(content, ShouldPublish, PublishBranch_PublishCultures, userId);
     }
 
     [Obsolete($"This method no longer saves content, only publishes it. Please use {nameof(PublishBranch)} instead. Will be removed in V16")]
     public IEnumerable<PublishResult> SaveAndPublishBranch(IContent content, bool force, string[] cultures, int userId = Constants.Security.SuperUserId)
-        => PublishBranch(content, force, cultures, userId);
+        => PublishBranch(content, force ? PublishBranchFilter.IncludeUnpublished : PublishBranchFilter.Default, cultures, userId);
 
     /// <inheritdoc />
+    [Obsolete("This method is not longer used as the 'force' parameter has been split into publishing unpublished and force re-published. Please use the overload containing parameters for those options instead. Will be removed in V16")]
     public IEnumerable<PublishResult> PublishBranch(IContent content, bool force, string[] cultures, int userId = Constants.Security.SuperUserId)
+        => PublishBranch(content, force ? PublishBranchFilter.IncludeUnpublished : PublishBranchFilter.Default, cultures, userId);
+
+    /// <inheritdoc />
+    public IEnumerable<PublishResult> PublishBranch(IContent content, PublishBranchFilter publishBranchFilter, string[] cultures, int userId = Constants.Security.SuperUserId)
     {
         // note: EditedValue and PublishedValue are objects here, so it is important to .Equals()
         // and not to == them, else we would be comparing references, and that is a bad thing
-        cultures ??= Array.Empty<string>();
 
-        if (content.ContentType.VariesByCulture() is false && cultures.Length == 0)
+        cultures = EnsureCultures(content, cultures);
+
+        string? defaultCulture;
+        using (ICoreScope scope = ScopeProvider.CreateCoreScope())
         {
-            cultures = new[] { "*" };
+            defaultCulture = _languageRepository.GetDefaultIsoCode();
+            scope.Complete();
         }
 
         // determines cultures to be published
@@ -2114,34 +2124,50 @@ public IEnumerable<PublishResult> PublishBranch(IContent content, bool force, st
             // invariant content type
             if (!c.ContentType.VariesByCulture())
             {
-                return PublishBranch_ShouldPublish(ref culturesToPublish, "*", c.Published, c.Edited, isRoot, force);
+                return PublishBranch_ShouldPublish(ref culturesToPublish, "*", c.Published, c.Edited, isRoot, publishBranchFilter);
             }
 
             // variant content type, specific cultures
             if (c.Published)
             {
                 // then some (and maybe all) cultures will be 'already published' (unless forcing),
                 // others will have to 'republish this culture'
-                foreach (var x in cultures)
+                foreach (var culture in cultures)
                 {
-                    PublishBranch_ShouldPublish(ref culturesToPublish, x, c.IsCulturePublished(x), c.IsCultureEdited(x), isRoot, force);
+                    // We could be publishing a parent invariant page, with descendents that are variant.
+                    // So convert the invariant request to a request for the default culture.
+                    var specificCulture = culture == "*" ? defaultCulture : culture;
+
+                    PublishBranch_ShouldPublish(ref culturesToPublish, specificCulture, c.IsCulturePublished(specificCulture), c.IsCultureEdited(specificCulture), isRoot, publishBranchFilter);
                 }
 
                 return culturesToPublish;
             }
 
-            // if not published, publish if force/root else do nothing
-            return force || isRoot
+            // if not published, publish if forcing unpublished/root else do nothing
+            return publishBranchFilter.HasFlag(PublishBranchFilter.IncludeUnpublished) || isRoot
                 ? new HashSet<string>(cultures) // means 'publish specified cultures'
                 : null; // null means 'nothing to do'
         }
 
-        return PublishBranch(content, force, ShouldPublish, PublishBranch_PublishCultures, userId);
+        return PublishBranch(content, ShouldPublish, PublishBranch_PublishCultures, userId);
     }
 
+    private static string[] EnsureCultures(IContent content, string[] cultures)
+    {
+        // Ensure consistent indication of "all cultures" for variant content.
+        if (content.ContentType.VariesByCulture() is false && ProvidedCulturesIndicatePublishAll(cultures))
+        {
+            cultures = ["*"];
+        }
+
+        return cultures;
+    }
+
+    private static bool ProvidedCulturesIndicatePublishAll(string[] cultures) => cultures.Length == 0 || (cultures.Length == 1 && cultures[0] == "invariant");
+
     internal IEnumerable<PublishResult> PublishBranch(
         IContent document,
-        bool force,
         Func<IContent, HashSet<string>?> shouldPublish,
         Func<IContent, HashSet<string>, IReadOnlyCollection<ILanguage>, bool> publishCultures,
         int userId = Constants.Security.SuperUserId)
@@ -3116,7 +3142,7 @@ internal IEnumerable<IContent> GetPublishedDescendantsLocked(IContent content)
     {
         var pathMatch = content.Path + ",";
         IQuery<IContent> query = Query<IContent>()
-            .Where(x => x.Id != content.Id && x.Path.StartsWith(pathMatch) /*&& x.Trashed == false*/);
+            .Where(x => x.Id != content.Id && x.Path.StartsWith(pathMatch) /*&& culture.Trashed == false*/);
         IEnumerable<IContent> contents = _documentRepository.Get(query);
 
         // beware! contents contains all published version below content

src/Umbraco.Core/Services/IContentPublishingService.cs

@@ -3,6 +3,7 @@
 using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.Models.ContentPublishing;
 using Umbraco.Cms.Core.Services.OperationStatus;
+using static Umbraco.Cms.Core.Constants.Conventions;
 
 namespace Umbraco.Cms.Core.Services;
 
@@ -26,8 +27,22 @@ public interface IContentPublishingService
     /// <param name="force">A value indicating whether to force-publish content that is not already published.</param>
     /// <param name="userKey">The identifier of the user performing the operation.</param>
     /// <returns>Result of the publish operation.</returns>
+    [Obsolete("This method is not longer used as the 'force' parameter has been extended into options for publishing unpublished and re-publishing changed content. Please use the overload containing the parameter for those options instead. Will be removed in V17.")]
     Task<Attempt<ContentPublishingBranchResult, ContentPublishingOperationStatus>> PublishBranchAsync(Guid key, IEnumerable<string> cultures, bool force, Guid userKey);
 
+    /// <summary>
+    ///     Publishes a content branch.
+    /// </summary>
+    /// <param name="key">The key of the root content.</param>
+    /// <param name="cultures">The cultures to publish.</param>
+    /// <param name="publishBranchFilter">A value indicating options for force publishing unpublished or re-publishing unchanged content.</param>
+    /// <param name="userKey">The identifier of the user performing the operation.</param>
+    /// <returns>Result of the publish operation.</returns>
+    Task<Attempt<ContentPublishingBranchResult, ContentPublishingOperationStatus>> PublishBranchAsync(Guid key, IEnumerable<string> cultures, PublishBranchFilter publishBranchFilter, Guid userKey)
+#pragma warning disable CS0618 // Type or member is obsolete
+        => PublishBranchAsync(key, cultures, publishBranchFilter.HasFlag(PublishBranchFilter.IncludeUnpublished), userKey);
+#pragma warning restore CS0618 // Type or member is obsolete
+
     /// <summary>
     ///     Unpublishes multiple cultures of a single content item.
     /// </summary>

src/Umbraco.Core/Services/IContentService.cs

@@ -419,32 +419,25 @@ public interface IContentService : IContentServiceBase<IContent>
     ///         published. The root of the branch is always published, regardless of <paramref name="force" />.
     ///     </para>
     /// </remarks>
+    [Obsolete("This method is not longer used as the 'force' parameter has been extended into options for publishing unpublished and re-publishing changed content. Please use the overload containing the parameter for those options instead. Will be removed in V17.")]
     IEnumerable<PublishResult> PublishBranch(IContent content, bool force, string[] cultures, int userId = Constants.Security.SuperUserId);
 
-    ///// <summary>
-    ///// Saves and publishes a document branch.
-    ///// </summary>
-    ///// <param name="content">The root document.</param>
-    ///// <param name="force">A value indicating whether to force-publish documents that are not already published.</param>
-    ///// <param name="shouldPublish">A function determining cultures to publish.</param>
-    ///// <param name="publishCultures">A function publishing cultures.</param>
-    ///// <param name="userId">The identifier of the user performing the operation.</param>
-    ///// <remarks>
-    ///// <para>The <paramref name="force"/> parameter determines which documents are published. When <c>false</c>,
-    ///// only those documents that are already published, are republished. When <c>true</c>, all documents are
-    ///// published. The root of the branch is always published, regardless of <paramref name="force"/>.</para>
-    ///// <para>The <paramref name="editing"/> parameter is a function which determines whether a document has
-    ///// changes to publish (else there is no need to publish it). If one wants to publish only a selection of
-    ///// cultures, one may want to check that only properties for these cultures have changed. Otherwise, other
-    ///// cultures may trigger an unwanted republish.</para>
-    ///// <para>The <paramref name="publishCultures"/> parameter is a function to execute to publish cultures, on
-    ///// each document. It can publish all, one, or a selection of cultures. It returns a boolean indicating
-    ///// whether the cultures could be published.</para>
-    ///// </remarks>
-    // IEnumerable<PublishResult> SaveAndPublishBranch(IContent content, bool force,
-    //    Func<IContent, HashSet<string>> shouldPublish,
-    //    Func<IContent, HashSet<string>, bool> publishCultures,
-    //    int userId = Constants.Security.SuperUserId);
+    /// <summary>
+    ///     Publishes a document branch.
+    /// </summary>
+    /// <param name="content">The root document.</param>
+    /// <param name="publishBranchFilter">A value indicating options for force publishing unpublished or re-publishing unchanged content.</param>
+    /// <param name="cultures">The cultures to publish.</param>
+    /// <param name="userId">The identifier of the user performing the operation.</param>
+    /// <remarks>
+    ///     <para>
+    ///         The root of the branch is always published, regardless of <paramref name="publishBranchFilter" />.
+    ///     </para>
+    /// </remarks>
+    IEnumerable<PublishResult> PublishBranch(IContent content, PublishBranchFilter publishBranchFilter, string[] cultures, int userId = Constants.Security.SuperUserId)
+#pragma warning disable CS0618 // Type or member is obsolete
+        => SaveAndPublishBranch(content, publishBranchFilter.HasFlag(PublishBranchFilter.IncludeUnpublished), cultures, userId);
+#pragma warning restore CS0618 // Type or member is obsolete
 
     /// <summary>
     ///     Unpublishes a document.