From b8577e3af27b3e7650fc60f4fae2990f14ee8353 Mon Sep 17 00:00:00 2001 From: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> Date: Thu, 29 Feb 2024 14:38:47 +0100 Subject: [PATCH] V14: Unpublish multiple cultures (#15789) * Refactor to accept multiple cultures * Add test * Move logic into service * Remember to complete scope * Move scope creation. * Add test * Close scope on early return * Handle invalid cultures with bad request * Handle valid cultures based on content * Change return type if a culture is required * Refactor content publishing service to have 1 unpublish method * Update tests * Return better error * Scope completes --------- Co-authored-by: Bjarke Berg --- .../Document/DocumentControllerBase.cs | 8 ++ .../Document/UnpublishDocumentController.cs | 6 +- src/Umbraco.Cms.Api.Management/OpenApi.json | 8 +- .../Document/UnpublishDocumentRequestModel.cs | 2 +- .../Models/ContentRepositoryExtensions.cs | 2 +- .../Services/ContentPublishingService.cs | 109 +++++++++++++++++- .../Services/IContentPublishingService.cs | 9 +- src/Umbraco.Core/Services/IContentService.cs | 2 +- .../ContentPublishingOperationStatus.cs | 2 + .../ContentPublishingServiceTests.Publish.cs | 2 +- ...ContentPublishingServiceTests.Unpublish.cs | 67 ++++++++++- 11 files changed, 194 insertions(+), 23 deletions(-) diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Document/DocumentControllerBase.cs b/src/Umbraco.Cms.Api.Management/Controllers/Document/DocumentControllerBase.cs index 1cc8b20af8..4c7b58d884 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/Document/DocumentControllerBase.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/Document/DocumentControllerBase.cs @@ -91,6 +91,14 @@ public abstract class DocumentControllerBase : ContentControllerBase .WithTitle("Culture missing") .WithDetail("A culture needs to be specified to execute the operation.") .Build()), + ContentPublishingOperationStatus.CannotPublishInvariantWhenVariant => BadRequest(problemDetailsBuilder + .WithTitle("Cannot publish invariant when variant") + .WithDetail("Cannot publish invariant culture when the document varies by culture.") + .Build()), + ContentPublishingOperationStatus.CannotPublishVariantWhenNotVariant => BadRequest(problemDetailsBuilder + .WithTitle("Cannot publish variant when not variant.") + .WithDetail("Cannot publish a given culture when the document is invariant.") + .Build()), ContentPublishingOperationStatus.ConcurrencyViolation => BadRequest(problemDetailsBuilder .WithTitle("Concurrency violation detected") .WithDetail("An attempt was made to publish a version older than the latest version.") diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Document/UnpublishDocumentController.cs b/src/Umbraco.Cms.Api.Management/Controllers/Document/UnpublishDocumentController.cs index ee2d5763c7..10c14975d1 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/Document/UnpublishDocumentController.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/Document/UnpublishDocumentController.cs @@ -39,13 +39,12 @@ public class UnpublishDocumentController : DocumentControllerBase [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] public async Task Unpublish(Guid id, UnpublishDocumentRequestModel requestModel) { - AuthorizationResult authorizationResult = await _authorizationService.AuthorizeResourceAsync( User, ContentPermissionResource.WithKeys( ActionUnpublish.ActionLetter, id, - requestModel.Culture is not null ? requestModel.Culture.Yield() : Enumerable.Empty()), + requestModel.Cultures ?? Enumerable.Empty()), AuthorizationPolicies.ContentPermissionByResource); if (!authorizationResult.Succeeded) @@ -55,8 +54,9 @@ public class UnpublishDocumentController : DocumentControllerBase Attempt attempt = await _contentPublishingService.UnpublishAsync( id, - requestModel.Culture, + requestModel.Cultures, CurrentUserKey(_backOfficeSecurityAccessor)); + return attempt.Success ? Ok() : DocumentPublishingOperationStatusResult(attempt.Result); diff --git a/src/Umbraco.Cms.Api.Management/OpenApi.json b/src/Umbraco.Cms.Api.Management/OpenApi.json index 0f7eef1fb8..40367c29f7 100644 --- a/src/Umbraco.Cms.Api.Management/OpenApi.json +++ b/src/Umbraco.Cms.Api.Management/OpenApi.json @@ -39602,8 +39602,12 @@ "UnpublishDocumentRequestModel": { "type": "object", "properties": { - "culture": { - "type": "string", + "cultures": { + "uniqueItems": true, + "type": "array", + "items": { + "type": "string" + }, "nullable": true } }, diff --git a/src/Umbraco.Cms.Api.Management/ViewModels/Document/UnpublishDocumentRequestModel.cs b/src/Umbraco.Cms.Api.Management/ViewModels/Document/UnpublishDocumentRequestModel.cs index 199b7ff2c0..d7bdef53fd 100644 --- a/src/Umbraco.Cms.Api.Management/ViewModels/Document/UnpublishDocumentRequestModel.cs +++ b/src/Umbraco.Cms.Api.Management/ViewModels/Document/UnpublishDocumentRequestModel.cs @@ -2,5 +2,5 @@ namespace Umbraco.Cms.Api.Management.ViewModels.Document; public class UnpublishDocumentRequestModel { - public string? Culture { get; set; } + public ISet? Cultures { get; set; } } diff --git a/src/Umbraco.Core/Models/ContentRepositoryExtensions.cs b/src/Umbraco.Core/Models/ContentRepositoryExtensions.cs index 5b7d68a72b..e6862f587b 100644 --- a/src/Umbraco.Core/Models/ContentRepositoryExtensions.cs +++ b/src/Umbraco.Core/Models/ContentRepositoryExtensions.cs @@ -380,7 +380,7 @@ public static class ContentRepositoryExtensions var keepProcessing = true; - if (culture == "*") + if (culture == "*" || (content.ContentType.VariesByCulture() is false && culture is null)) { // all cultures content.ClearPublishInfos(); diff --git a/src/Umbraco.Core/Services/ContentPublishingService.cs b/src/Umbraco.Core/Services/ContentPublishingService.cs index c7a20dcbdd..5792d0b6c9 100644 --- a/src/Umbraco.Core/Services/ContentPublishingService.cs +++ b/src/Umbraco.Core/Services/ContentPublishingService.cs @@ -197,24 +197,123 @@ internal sealed class ContentPublishingService : IContentPublishingService : Attempt.FailWithStatus(ContentPublishingOperationStatus.FailedBranch, branchResult); } - /// - public async Task> UnpublishAsync(Guid key, string? culture, Guid userKey) + + /// + public async Task> UnpublishAsync(Guid key, ISet? cultures, Guid userKey) { using ICoreScope scope = _coreScopeProvider.CreateCoreScope(); IContent? content = _contentService.GetById(key); if (content is null) { + scope.Complete(); return Attempt.Fail(ContentPublishingOperationStatus.ContentNotFound); } var userId = await _userIdKeyResolver.GetAsync(userKey); - PublishResult result = _contentService.Unpublish(content, culture ?? "*", userId); + + Attempt attempt; + if (cultures is null) + { + attempt = await UnpublishInvariantAsync( + content, + userId); + + scope.Complete(); + return attempt; + } + + if (cultures.Any() is false) + { + scope.Complete(); + return Attempt.Fail(ContentPublishingOperationStatus.CultureMissing); + } + + if (cultures.Contains("*")) + { + attempt = await UnpublishAllCulturesAsync( + content, + userId); + } + else + { + attempt = await UnpublishMultipleCultures( + content, + cultures, + userId); + } + scope.Complete(); + + return attempt; + } + + private Task> UnpublishAllCulturesAsync(IContent content, int userId) + { + if (content.ContentType.VariesByCulture() is false) + { + return Task.FromResult(Attempt.Fail(ContentPublishingOperationStatus.CannotPublishVariantWhenNotVariant)); + } + + using ICoreScope scope = _coreScopeProvider.CreateCoreScope(); + PublishResult result = _contentService.Unpublish(content, "*", userId); scope.Complete(); ContentPublishingOperationStatus contentPublishingOperationStatus = ToContentPublishingOperationStatus(result); - return contentPublishingOperationStatus is ContentPublishingOperationStatus.Success + return Task.FromResult(contentPublishingOperationStatus is ContentPublishingOperationStatus.Success ? Attempt.Succeed(ToContentPublishingOperationStatus(result)) - : Attempt.Fail(ToContentPublishingOperationStatus(result)); + : Attempt.Fail(ToContentPublishingOperationStatus(result))); + } + + private async Task> UnpublishMultipleCultures(IContent content, ISet cultures, int userId) + { + using ICoreScope scope = _coreScopeProvider.CreateCoreScope(); + + if (content.ContentType.VariesByCulture() is false) + { + scope.Complete(); + return Attempt.Fail(ContentPublishingOperationStatus.CannotPublishVariantWhenNotVariant); + } + + var validCultures = (await _languageService.GetAllAsync()).Select(x => x.IsoCode).ToArray(); + + foreach (var culture in cultures) + { + if (validCultures.Contains(culture, StringComparer.InvariantCultureIgnoreCase) is false) + { + scope.Complete(); + return Attempt.Fail(ContentPublishingOperationStatus.InvalidCulture); + } + + PublishResult result = _contentService.Unpublish(content, culture, userId); + + ContentPublishingOperationStatus contentPublishingOperationStatus = ToContentPublishingOperationStatus(result); + + if (contentPublishingOperationStatus is not ContentPublishingOperationStatus.Success) + { + return Attempt.Fail(ToContentPublishingOperationStatus(result)); + } + } + + scope.Complete(); + return Attempt.Succeed(ContentPublishingOperationStatus.Success); + } + + + private Task> UnpublishInvariantAsync(IContent content, int userId) + { + using ICoreScope scope = _coreScopeProvider.CreateCoreScope(); + + if (content.ContentType.VariesByCulture()) + { + return Task.FromResult(Attempt.Fail(ContentPublishingOperationStatus.CannotPublishInvariantWhenVariant)); + } + + PublishResult result = _contentService.Unpublish(content, null, userId); + scope.Complete(); + + ContentPublishingOperationStatus contentPublishingOperationStatus = ToContentPublishingOperationStatus(result); + return Task.FromResult(contentPublishingOperationStatus is ContentPublishingOperationStatus.Success + ? Attempt.Succeed(ToContentPublishingOperationStatus(result)) + : Attempt.Fail(ToContentPublishingOperationStatus(result))); } private static ContentPublishingOperationStatus ToContentPublishingOperationStatus(PublishResult publishResult) diff --git a/src/Umbraco.Core/Services/IContentPublishingService.cs b/src/Umbraco.Core/Services/IContentPublishingService.cs index 8275b1867c..0d60da7313 100644 --- a/src/Umbraco.Core/Services/IContentPublishingService.cs +++ b/src/Umbraco.Core/Services/IContentPublishingService.cs @@ -1,5 +1,4 @@ -using Umbraco.Cms.Core.Models; -using Umbraco.Cms.Core.Models.ContentPublishing; +using Umbraco.Cms.Core.Models.ContentPublishing; using Umbraco.Cms.Core.Services.OperationStatus; namespace Umbraco.Cms.Core.Services; @@ -26,11 +25,11 @@ public interface IContentPublishingService Task> PublishBranchAsync(Guid key, IEnumerable cultures, bool force, Guid userKey); /// - /// Unpublishes a single content item. + /// Unpublishes multiple cultures of a single content item. /// /// The key of the root content. - /// The culture to unpublish. Use null to unpublish all cultures. + /// The cultures to unpublish. Use null to unpublish all cultures. /// The identifier of the user performing the operation. /// Status of the publish operation. - Task> UnpublishAsync(Guid key, string? culture, Guid userKey); + Task> UnpublishAsync(Guid key, ISet? cultures, Guid userKey); } diff --git a/src/Umbraco.Core/Services/IContentService.cs b/src/Umbraco.Core/Services/IContentService.cs index 1f424c0892..a97f753f99 100644 --- a/src/Umbraco.Core/Services/IContentService.cs +++ b/src/Umbraco.Core/Services/IContentService.cs @@ -441,7 +441,7 @@ public interface IContentService : IContentServiceBase /// empty. If the content type is invariant, then culture can be either '*' or null or empty. /// /// - PublishResult Unpublish(IContent content, string culture = "*", int userId = Constants.Security.SuperUserId); + PublishResult Unpublish(IContent content, string? culture = "*", int userId = Constants.Security.SuperUserId); /// /// Gets a value indicating whether a document is path-publishable. diff --git a/src/Umbraco.Core/Services/OperationStatus/ContentPublishingOperationStatus.cs b/src/Umbraco.Core/Services/OperationStatus/ContentPublishingOperationStatus.cs index 24cc0b01c5..1c61767ffa 100644 --- a/src/Umbraco.Core/Services/OperationStatus/ContentPublishingOperationStatus.cs +++ b/src/Umbraco.Core/Services/OperationStatus/ContentPublishingOperationStatus.cs @@ -14,6 +14,8 @@ public enum ContentPublishingOperationStatus CultureAwaitingRelease, InTrash, InvalidCulture, + CannotPublishInvariantWhenVariant, + CannotPublishVariantWhenNotVariant, CultureMissing, PathNotPublished, ConcurrencyViolation, diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentPublishingServiceTests.Publish.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentPublishingServiceTests.Publish.cs index 8225f4ea68..ce1cffeb25 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentPublishingServiceTests.Publish.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentPublishingServiceTests.Publish.cs @@ -145,7 +145,7 @@ public partial class ContentPublishingServiceTests Assert.IsTrue(content.PublishedCultures.InvariantContains(langEn.IsoCode)); Assert.IsTrue(content.PublishedCultures.InvariantContains(langDa.IsoCode)); - var unpublishResult = await ContentPublishingService.UnpublishAsync(content.Key, "*", Constants.Security.SuperUserKey); + var unpublishResult = await ContentPublishingService.UnpublishAsync(content.Key, new HashSet() { "*" }, Constants.Security.SuperUserKey); Assert.IsTrue(unpublishResult.Success); Assert.AreEqual(ContentPublishingOperationStatus.Success, unpublishResult.Result); diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentPublishingServiceTests.Unpublish.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentPublishingServiceTests.Unpublish.cs index d290e545ac..4659d4e25d 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentPublishingServiceTests.Unpublish.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentPublishingServiceTests.Unpublish.cs @@ -3,6 +3,7 @@ using Umbraco.Cms.Core; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Services.OperationStatus; using Umbraco.Cms.Tests.Common.Builders; +using Umbraco.Cms.Tests.Common.Builders.Extensions; namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services; @@ -107,7 +108,7 @@ public partial class ContentPublishingServiceTests await ContentPublishingService.PublishAsync(content.Key, MakeModel(new HashSet() { langEn.IsoCode, langDa.IsoCode }), Constants.Security.SuperUserKey); VerifyIsPublished(content.Key); - var result = await ContentPublishingService.UnpublishAsync(content.Key, langEn.IsoCode, Constants.Security.SuperUserKey); + var result = await ContentPublishingService.UnpublishAsync(content.Key, new HashSet { langEn.IsoCode }, Constants.Security.SuperUserKey); Assert.IsTrue(result.Success); Assert.AreEqual(ContentPublishingOperationStatus.Success, result.Result); VerifyIsPublished(content.Key); @@ -133,7 +134,7 @@ public partial class ContentPublishingServiceTests await ContentPublishingService.PublishAsync(content.Key, MakeModel(new HashSet() { langEn.IsoCode, langDa.IsoCode }), Constants.Security.SuperUserKey); VerifyIsPublished(content.Key); - var result = await ContentPublishingService.UnpublishAsync(content.Key, null, Constants.Security.SuperUserKey); + var result = await ContentPublishingService.UnpublishAsync(content.Key, new HashSet(){"*"}, Constants.Security.SuperUserKey); Assert.IsTrue(result.Success); Assert.AreEqual(ContentPublishingOperationStatus.Success, result.Result); VerifyIsNotPublished(content.Key); @@ -142,6 +143,64 @@ public partial class ContentPublishingServiceTests Assert.AreEqual(0, content.PublishedCultures.Count()); } + [Test] + public async Task Can_Unpublish_All_Cultures_With_Multiple_Cultures_Method() + { + var (langEn, langDa, contentType) = await SetupVariantTest(); + + IContent content = new ContentBuilder() + .WithContentType(contentType) + .WithCultureName(langEn.IsoCode, "EN root") + .WithCultureName(langDa.IsoCode, "DA root") + .Build(); + content.SetValue("title", "EN title", culture: langEn.IsoCode); + content.SetValue("title", "DA title", culture: langDa.IsoCode); + ContentService.Save(content); + await ContentPublishingService.PublishAsync(content.Key, MakeModel(new HashSet() { langEn.IsoCode, langDa.IsoCode }), Constants.Security.SuperUserKey); + VerifyIsPublished(content.Key); + + var result = await ContentPublishingService.UnpublishAsync(content.Key, new HashSet() { langEn.IsoCode, langDa.IsoCode }, Constants.Security.SuperUserKey); + Assert.IsTrue(result.Success); + Assert.AreEqual(ContentPublishingOperationStatus.Success, result.Result); + VerifyIsNotPublished(content.Key); + + content = ContentService.GetById(content.Key)!; + Assert.AreEqual(0, content.PublishedCultures.Count()); + } + + [Test] + public async Task Can_Unpublish_Multiple_Cultures() + { + var (langEn, langDa, contentType) = await SetupVariantTest(); + + var langSe = new LanguageBuilder() + .WithCultureInfo("sv-SE") + .Build(); + await LanguageService.CreateAsync(langSe, Constants.Security.SuperUserKey); + + IContent content = new ContentBuilder() + .WithContentType(contentType) + .WithCultureName(langEn.IsoCode, "EN root") + .WithCultureName(langDa.IsoCode, "DA root") + .WithCultureName(langSe.IsoCode, "SE root") + .Build(); + content.SetValue("title", "EN title", culture: langEn.IsoCode); + content.SetValue("title", "DA title", culture: langDa.IsoCode); + content.SetValue("title", "SE title", culture: langSe.IsoCode); + ContentService.Save(content); + await ContentPublishingService.PublishAsync(content.Key, MakeModel(new HashSet() { langEn.IsoCode, langDa.IsoCode, langSe.IsoCode }), Constants.Security.SuperUserKey); + VerifyIsPublished(content.Key); + content = ContentService.GetById(content.Key)!; + Assert.AreEqual(3, content.PublishedCultures.Count()); + + var result = await ContentPublishingService.UnpublishAsync(content.Key, new HashSet { langDa.IsoCode, langSe.IsoCode }, Constants.Security.SuperUserKey); + Assert.IsTrue(result.Success); + Assert.AreEqual(ContentPublishingOperationStatus.Success, result.Result); + VerifyIsPublished(content.Key); + content = ContentService.GetById(content.Key)!; + Assert.AreEqual(1, content.PublishedCultures.Count()); + } + [Test] public async Task Can_Unpublish_All_Cultures_By_Unpublishing_Mandatory_Culture() { @@ -161,7 +220,7 @@ public partial class ContentPublishingServiceTests content = ContentService.GetById(content.Key)!; Assert.AreEqual(2, content.PublishedCultures.Count()); - var result = await ContentPublishingService.UnpublishAsync(content.Key, langEn.IsoCode, Constants.Security.SuperUserKey); + var result = await ContentPublishingService.UnpublishAsync(content.Key, new HashSet { langEn.IsoCode }, Constants.Security.SuperUserKey); Assert.IsTrue(result.Success); Assert.AreEqual(ContentPublishingOperationStatus.Success, result.Result); VerifyIsNotPublished(content.Key); @@ -191,7 +250,7 @@ public partial class ContentPublishingServiceTests content = ContentService.GetById(content.Key)!; Assert.AreEqual(2, content.PublishedCultures.Count()); - var result = await ContentPublishingService.UnpublishAsync(content.Key, langDa.IsoCode, Constants.Security.SuperUserKey); + var result = await ContentPublishingService.UnpublishAsync(content.Key, new HashSet() { langDa.IsoCode }, Constants.Security.SuperUserKey); Assert.IsTrue(result.Success); Assert.AreEqual(ContentPublishingOperationStatus.Success, result.Result); VerifyIsPublished(content.Key);