From f719e9b76836ffab3f9e15915a453ae1aa542801 Mon Sep 17 00:00:00 2001 From: NillasKA Date: Fri, 29 Aug 2025 08:58:57 +0200 Subject: [PATCH 01/13] Adding signs to variants and adjusting HasPendingChangesSignProvider.cs --- .../Signs/HasPendingChangesSignProvider.cs | 14 ++++++------- .../DocumentVariantItemResponseModel.cs | 20 ++++++++++++++++++- .../Document/DocumentVariantResponseModel.cs | 20 ++++++++++++++++++- 3 files changed, 45 insertions(+), 9 deletions(-) diff --git a/src/Umbraco.Cms.Api.Management/Services/Signs/HasPendingChangesSignProvider.cs b/src/Umbraco.Cms.Api.Management/Services/Signs/HasPendingChangesSignProvider.cs index 50c421a792e1..b49449a53537 100644 --- a/src/Umbraco.Cms.Api.Management/Services/Signs/HasPendingChangesSignProvider.cs +++ b/src/Umbraco.Cms.Api.Management/Services/Signs/HasPendingChangesSignProvider.cs @@ -27,9 +27,9 @@ public Task PopulateSignsAsync(IEnumerable itemViewModels) { foreach (TItem item in itemViewModels) { - if (HasPendingChanges(item)) + foreach (IHasSigns variant in HasPendingChanges(item)) { - item.AddSign(Alias); + variant.AddSign(Alias); } } @@ -39,11 +39,11 @@ public Task PopulateSignsAsync(IEnumerable itemViewModels) /// /// Determines if the given item has any variant that has pending changes. /// - private bool HasPendingChanges(object item) => item switch + private static IEnumerable HasPendingChanges(object item) => item switch { - DocumentTreeItemResponseModel { Variants: var v } when v.Any(x => x.State == DocumentVariantState.PublishedPendingChanges) => true, - DocumentCollectionResponseModel { Variants: var v } when v.Any(x => x.State == DocumentVariantState.PublishedPendingChanges) => true, - DocumentItemResponseModel { Variants: var v } when v.Any(x => x.State == DocumentVariantState.PublishedPendingChanges) => true, - _ => false, + DocumentTreeItemResponseModel { Variants: var v } => v.Where(x => x.State == DocumentVariantState.PublishedPendingChanges), + DocumentCollectionResponseModel { Variants: var v } => v.Where(x => x.State == DocumentVariantState.PublishedPendingChanges), + DocumentItemResponseModel { Variants: var v } => v.Where(x => x.State == DocumentVariantState.PublishedPendingChanges), + _ => [], }; } diff --git a/src/Umbraco.Cms.Api.Management/ViewModels/Document/DocumentVariantItemResponseModel.cs b/src/Umbraco.Cms.Api.Management/ViewModels/Document/DocumentVariantItemResponseModel.cs index ccdc16ec0d17..3e4d8913f8a5 100644 --- a/src/Umbraco.Cms.Api.Management/ViewModels/Document/DocumentVariantItemResponseModel.cs +++ b/src/Umbraco.Cms.Api.Management/ViewModels/Document/DocumentVariantItemResponseModel.cs @@ -2,7 +2,25 @@ namespace Umbraco.Cms.Api.Management.ViewModels.Document; -public class DocumentVariantItemResponseModel : VariantItemResponseModelBase +public class DocumentVariantItemResponseModel : VariantItemResponseModelBase, IHasSigns { + private readonly List _signs = []; + + public Guid Id { get; } + + public IEnumerable Signs + { + get => _signs.AsEnumerable(); + set + { + _signs.Clear(); + _signs.AddRange(value); + } + } + + public void AddSign(string alias) => _signs.Add(new SignModel { Alias = alias }); + + public void RemoveSign(string alias) => _signs.RemoveAll(x => x.Alias == alias); + public required DocumentVariantState State { get; set; } } diff --git a/src/Umbraco.Cms.Api.Management/ViewModels/Document/DocumentVariantResponseModel.cs b/src/Umbraco.Cms.Api.Management/ViewModels/Document/DocumentVariantResponseModel.cs index b6990c1b3c71..cfa3ef58096e 100644 --- a/src/Umbraco.Cms.Api.Management/ViewModels/Document/DocumentVariantResponseModel.cs +++ b/src/Umbraco.Cms.Api.Management/ViewModels/Document/DocumentVariantResponseModel.cs @@ -2,7 +2,7 @@ namespace Umbraco.Cms.Api.Management.ViewModels.Document; -public class DocumentVariantResponseModel : VariantResponseModelBase +public class DocumentVariantResponseModel : VariantResponseModelBase, IHasSigns { public DocumentVariantState State { get; set; } @@ -11,4 +11,22 @@ public class DocumentVariantResponseModel : VariantResponseModelBase public DateTimeOffset? ScheduledPublishDate { get; set; } public DateTimeOffset? ScheduledUnpublishDate { get; set; } + + private readonly List _signs = []; + + public Guid Id { get; } + + public IEnumerable Signs + { + get => _signs.AsEnumerable(); + set + { + _signs.Clear(); + _signs.AddRange(value); + } + } + + public void AddSign(string alias) => _signs.Add(new SignModel { Alias = alias }); + + public void RemoveSign(string alias) => _signs.RemoveAll(x => x.Alias == alias); } From 18e9033cd9a31c422efe42cd27b31b983fb04a26 Mon Sep 17 00:00:00 2001 From: NillasKA Date: Mon, 1 Sep 2025 12:46:59 +0200 Subject: [PATCH 02/13] HasPendingChangesSignProvider.cs now populates variants & refactoring to move logic to DocumentPresentationFactory.cs --- .../Factories/DocumentPresentationFactory.cs | 53 ++++++++- .../Signs/HasPendingChangesSignProvider.cs | 27 ++--- .../Services/Signs/ISignProvider.cs | 3 +- .../HasPendingChangesSignProviderTests.cs | 102 +++++------------- 4 files changed, 88 insertions(+), 97 deletions(-) diff --git a/src/Umbraco.Cms.Api.Management/Factories/DocumentPresentationFactory.cs b/src/Umbraco.Cms.Api.Management/Factories/DocumentPresentationFactory.cs index f0e3c9901e62..e54c289a476d 100644 --- a/src/Umbraco.Cms.Api.Management/Factories/DocumentPresentationFactory.cs +++ b/src/Umbraco.Cms.Api.Management/Factories/DocumentPresentationFactory.cs @@ -1,10 +1,13 @@ +using Microsoft.Extensions.DependencyInjection; using Umbraco.Cms.Api.Management.Mapping.Content; +using Umbraco.Cms.Api.Management.Services.Signs; using Umbraco.Cms.Api.Management.ViewModels; using Umbraco.Cms.Api.Management.ViewModels.Document; using Umbraco.Cms.Api.Management.ViewModels.Document.Item; using Umbraco.Cms.Api.Management.ViewModels.DocumentBlueprint.Item; using Umbraco.Cms.Api.Management.ViewModels.DocumentType; using Umbraco.Cms.Core; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Mapping; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.ContentPublishing; @@ -23,7 +26,9 @@ internal sealed class DocumentPresentationFactory : IDocumentPresentationFactory private readonly IPublicAccessService _publicAccessService; private readonly TimeProvider _timeProvider; private readonly IIdKeyMap _idKeyMap; + private readonly SignProviderCollection _signProviderCollection; + [Obsolete("Please use the controller with all parameters. Scheduled for removal in Umbraco 18")] public DocumentPresentationFactory( IUmbracoMapper umbracoMapper, IDocumentUrlFactory documentUrlFactory, @@ -31,6 +36,25 @@ public DocumentPresentationFactory( IPublicAccessService publicAccessService, TimeProvider timeProvider, IIdKeyMap idKeyMap) + : this( + umbracoMapper, + documentUrlFactory, + templateService, + publicAccessService, + timeProvider, + idKeyMap, + StaticServiceProvider.Instance.GetRequiredService()) + { + } + + public DocumentPresentationFactory( + IUmbracoMapper umbracoMapper, + IDocumentUrlFactory documentUrlFactory, + ITemplateService templateService, + IPublicAccessService publicAccessService, + TimeProvider timeProvider, + IIdKeyMap idKeyMap, + SignProviderCollection signProviderCollection) { _umbracoMapper = umbracoMapper; _documentUrlFactory = documentUrlFactory; @@ -38,6 +62,7 @@ public DocumentPresentationFactory( _publicAccessService = publicAccessService; _timeProvider = timeProvider; _idKeyMap = idKeyMap; + _signProviderCollection = signProviderCollection; } [Obsolete("Schedule for removal in v17")] @@ -105,6 +130,8 @@ public DocumentItemResponseModel CreateItemResponseModel(IDocumentEntitySlim ent responseModel.Variants = CreateVariantsItemResponseModels(entity); + PopulateSignsOnDocuments(responseModel).GetAwaiter().GetResult(); + return responseModel; } @@ -125,23 +152,29 @@ public IEnumerable CreateVariantsItemResponseM { if (entity.Variations.VariesByCulture() is false) { - yield return new() + var model = new DocumentVariantItemResponseModel() { Name = entity.Name ?? string.Empty, State = DocumentVariantStateHelper.GetState(entity, null), Culture = null, }; + + PopulateSignsOnVariants(model).GetAwaiter().GetResult(); + yield return model; yield break; } foreach (KeyValuePair cultureNamePair in entity.CultureNames) { - yield return new() + var model = new DocumentVariantItemResponseModel() { Name = cultureNamePair.Value, Culture = cultureNamePair.Key, State = DocumentVariantStateHelper.GetState(entity, cultureNamePair.Key) }; + + PopulateSignsOnVariants(model).GetAwaiter().GetResult(); + yield return model; } } @@ -256,4 +289,20 @@ public Attempt, ContentPublishingOperationStat return Attempt.SucceedWithStatus(ContentPublishingOperationStatus.Success, model); } + + private async Task PopulateSignsOnDocuments(DocumentItemResponseModel model) + { + foreach (ISignProvider signProvider in _signProviderCollection.Where(x => x.CanProvideSigns())) + { + await signProvider.PopulateSignsAsync(model); + } + } + + private async Task PopulateSignsOnVariants(DocumentVariantItemResponseModel model) + { + foreach (ISignProvider signProvider in _signProviderCollection.Where(x => x.CanProvideSigns())) + { + await signProvider.PopulateSignsAsync(model); + } + } } diff --git a/src/Umbraco.Cms.Api.Management/Services/Signs/HasPendingChangesSignProvider.cs b/src/Umbraco.Cms.Api.Management/Services/Signs/HasPendingChangesSignProvider.cs index b49449a53537..551d9fb84768 100644 --- a/src/Umbraco.Cms.Api.Management/Services/Signs/HasPendingChangesSignProvider.cs +++ b/src/Umbraco.Cms.Api.Management/Services/Signs/HasPendingChangesSignProvider.cs @@ -1,8 +1,5 @@ using Umbraco.Cms.Api.Management.ViewModels; using Umbraco.Cms.Api.Management.ViewModels.Document; -using Umbraco.Cms.Api.Management.ViewModels.Document.Collection; -using Umbraco.Cms.Api.Management.ViewModels.Document.Item; -using Umbraco.Cms.Api.Management.ViewModels.Tree; using Umbraco.Cms.Core; namespace Umbraco.Cms.Api.Management.Services.Signs; @@ -17,20 +14,17 @@ public class HasPendingChangesSignProvider : ISignProvider /// public bool CanProvideSigns() where TItem : IHasSigns => - typeof(TItem) == typeof(DocumentTreeItemResponseModel) || - typeof(TItem) == typeof(DocumentCollectionResponseModel) || - typeof(TItem) == typeof(DocumentItemResponseModel); + typeof(TItem) == typeof(DocumentVariantItemResponseModel) || + typeof(TItem) == typeof(DocumentVariantResponseModel); + /// - public Task PopulateSignsAsync(IEnumerable itemViewModels) + public Task PopulateSignsAsync(TItem item) where TItem : IHasSigns { - foreach (TItem item in itemViewModels) + if (HasPendingChanges(item)) { - foreach (IHasSigns variant in HasPendingChanges(item)) - { - variant.AddSign(Alias); - } + item.AddSign(Alias); } return Task.CompletedTask; @@ -39,11 +33,10 @@ public Task PopulateSignsAsync(IEnumerable itemViewModels) /// /// Determines if the given item has any variant that has pending changes. /// - private static IEnumerable HasPendingChanges(object item) => item switch + private static bool HasPendingChanges(object item) => item switch { - DocumentTreeItemResponseModel { Variants: var v } => v.Where(x => x.State == DocumentVariantState.PublishedPendingChanges), - DocumentCollectionResponseModel { Variants: var v } => v.Where(x => x.State == DocumentVariantState.PublishedPendingChanges), - DocumentItemResponseModel { Variants: var v } => v.Where(x => x.State == DocumentVariantState.PublishedPendingChanges), - _ => [], + DocumentVariantItemResponseModel variant => variant.State == DocumentVariantState.PublishedPendingChanges, + DocumentVariantResponseModel variant => variant.State == DocumentVariantState.PublishedPendingChanges, + _ => false, }; } diff --git a/src/Umbraco.Cms.Api.Management/Services/Signs/ISignProvider.cs b/src/Umbraco.Cms.Api.Management/Services/Signs/ISignProvider.cs index e0324f05c875..e36e10d74f90 100644 --- a/src/Umbraco.Cms.Api.Management/Services/Signs/ISignProvider.cs +++ b/src/Umbraco.Cms.Api.Management/Services/Signs/ISignProvider.cs @@ -18,7 +18,6 @@ bool CanProvideSigns() /// Populates the provided item view models with signs. /// /// Type of item view model supporting signs. - /// The collection of item view models to be populated with signs. - Task PopulateSignsAsync(IEnumerable itemViewModels) + Task PopulateSignsAsync(TItem item) where TItem : IHasSigns; } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Cms.Api.Management/Services/Signs/HasPendingChangesSignProviderTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Cms.Api.Management/Services/Signs/HasPendingChangesSignProviderTests.cs index 7bc743591961..78f60464c268 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Cms.Api.Management/Services/Signs/HasPendingChangesSignProviderTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Cms.Api.Management/Services/Signs/HasPendingChangesSignProviderTests.cs @@ -11,116 +11,66 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Cms.Api.Management.Services.Signs; internal class HasPendingChangesSignProviderTests { [Test] - public void HasPendingChangesSignProvider_Can_Provide_Document_Tree_Signs() + public void HasPendingChangesSignProvider_Can_Provide_Variant_Item_Signs() { var sut = new HasPendingChangesSignProvider(); - Assert.IsTrue(sut.CanProvideSigns()); + Assert.IsTrue(sut.CanProvideSigns()); } [Test] - public void HasPendingChangesSignProvider_Can_Provide_Document_Collection_Signs() + public void HasPendingChangesSignProvider_Can_Provide_Variant_Signs() { var sut = new HasPendingChangesSignProvider(); - Assert.IsTrue(sut.CanProvideSigns()); + Assert.IsTrue(sut.CanProvideSigns()); } [Test] - public void HasPendingChangesSignProvider_Can_Provide_Document_Item_Signs() + public async Task HasPendingChangesSignProvider_Should_Populate_Variant_Item_Signs() { var sut = new HasPendingChangesSignProvider(); - Assert.IsTrue(sut.CanProvideSigns()); - } - [Test] - public async Task HasPendingChangesSignProvider_Should_Populate_Document_Tree_Signs() - { - var sut = new HasPendingChangesSignProvider(); + var variant1 = new DocumentVariantItemResponseModel() + { + State = DocumentVariantState.PublishedPendingChanges, Culture = null, Name = "Test", + }; - var viewModels = new List + var variant2 = new DocumentVariantItemResponseModel() { - new() { Id = Guid.NewGuid() }, - new() - { - Id = Guid.NewGuid(), Variants = - [ - new() - { - State = DocumentVariantState.PublishedPendingChanges, - Culture = null, - Name = "Test", - }, - ], - }, + State = DocumentVariantState.Published, Culture = null, Name = "Test", }; - await sut.PopulateSignsAsync(viewModels); + await sut.PopulateSignsAsync(variant1); + await sut.PopulateSignsAsync(variant2); - Assert.AreEqual(viewModels[0].Signs.Count(), 0); - Assert.AreEqual(viewModels[1].Signs.Count(), 1); + Assert.AreEqual(variant1.Signs.Count(), 1); + Assert.AreEqual(variant2.Signs.Count(), 0); - var signModel = viewModels[1].Signs.First(); + var signModel = variant1.Signs.First(); Assert.AreEqual("Umb.PendingChanges", signModel.Alias); } [Test] - public async Task HasPendingChangesSignProvider_Should_Populate_Document_Collection_Signs() + public async Task HasPendingChangesSignProvider_Should_Populate_Variant_Signs() { var sut = new HasPendingChangesSignProvider(); - var viewModels = new List + var variant1 = new DocumentVariantResponseModel() { - new() { Id = Guid.NewGuid() }, - new() - { - Id = Guid.NewGuid(), Variants = - [ - new() - { - State = DocumentVariantState.PublishedPendingChanges, - Culture = null, - Name = "Test", - }, - ], - }, + State = DocumentVariantState.PublishedPendingChanges, Culture = null, Name = "Test", }; - await sut.PopulateSignsAsync(viewModels); - - Assert.AreEqual(viewModels[0].Signs.Count(), 0); - Assert.AreEqual(viewModels[1].Signs.Count(), 1); - - var signModel = viewModels[1].Signs.First(); - Assert.AreEqual("Umb.PendingChanges", signModel.Alias); - } - - [Test] - public async Task HasPendingChangesSignProvider_Should_Populate_Document_Item_Signs() - { - var sut = new HasPendingChangesSignProvider(); - - var viewModels = new List + var variant2 = new DocumentVariantResponseModel() { - new() { Id = Guid.NewGuid() }, - new() - { - Id = Guid.NewGuid(), Variants = - [ - new() - { - State = DocumentVariantState.PublishedPendingChanges, - Culture = null, - Name = "Test", - }, - ], - }, + State = DocumentVariantState.Published, Culture = null, Name = "Test", }; - await sut.PopulateSignsAsync(viewModels); + await sut.PopulateSignsAsync(variant1); + await sut.PopulateSignsAsync(variant2); - Assert.AreEqual(viewModels[0].Signs.Count(), 0); - Assert.AreEqual(viewModels[1].Signs.Count(), 1); + Assert.AreEqual(variant1.Signs.Count(), 1); + Assert.AreEqual(variant2.Signs.Count(), 0); - var signModel = viewModels[1].Signs.First(); + var signModel = variant1.Signs.First(); Assert.AreEqual("Umb.PendingChanges", signModel.Alias); } } From e9508566eeaa9e88c42f140b9326016a8ba0d129 Mon Sep 17 00:00:00 2001 From: NillasKA Date: Tue, 2 Sep 2025 13:52:25 +0200 Subject: [PATCH 03/13] Working HasScheduleSignProvider.cs to provide variant signs --- .../Services/Signs/HasScheduleSignProvider.cs | 104 +++++++++++++++++- 1 file changed, 100 insertions(+), 4 deletions(-) diff --git a/src/Umbraco.Cms.Api.Management/Services/Signs/HasScheduleSignProvider.cs b/src/Umbraco.Cms.Api.Management/Services/Signs/HasScheduleSignProvider.cs index 599d10ae67e5..60952d50ed7c 100644 --- a/src/Umbraco.Cms.Api.Management/Services/Signs/HasScheduleSignProvider.cs +++ b/src/Umbraco.Cms.Api.Management/Services/Signs/HasScheduleSignProvider.cs @@ -1,7 +1,9 @@ using Umbraco.Cms.Api.Management.ViewModels; +using Umbraco.Cms.Api.Management.ViewModels.Document; using Umbraco.Cms.Api.Management.ViewModels.Document.Collection; using Umbraco.Cms.Api.Management.ViewModels.Document.Item; using Umbraco.Cms.Api.Management.ViewModels.Tree; +using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Services; using Constants = Umbraco.Cms.Core.Constants; @@ -29,13 +31,107 @@ public bool CanProvideSigns() typeof(TItem) == typeof(DocumentItemResponseModel); /// - public Task PopulateSignsAsync(IEnumerable itemViewModels) + public Task PopulateSignsAsync(IEnumerable items) where TItem : IHasSigns { - IEnumerable contentKeysScheduledForPublishing = _contentService.GetScheduledContentKeys(itemViewModels.Select(x => x.Id)); - foreach (Guid key in contentKeysScheduledForPublishing) + IEnumerable keys = _contentService.GetScheduledContentKeys(items.Select(x => x.Id).ToArray()); + + foreach (Guid key in keys) { - itemViewModels.First(x => x.Id == key).AddSign(Alias); + TItem item = items.First(x => x.Id == key); + + switch (item) + { + case DocumentTreeItemResponseModel document: + { + var variants = new List(); + if (document.Variants.Count() == 1) + { + DocumentVariantItemResponseModel variant = document.Variants.First(); + variant.AddSign(Alias); + variants.Add(variant); + } + else + { + IEnumerable schedules = + _contentService.GetContentScheduleByContentId(key).GetSchedule(); + IEnumerable culturesWithSchedule = schedules + .Select(schedule => schedule.Culture); + + foreach (DocumentVariantItemResponseModel variant in document.Variants) + { + if (variant.Culture != null && culturesWithSchedule.Contains(variant.Culture)) + { + variant.AddSign(Alias); + variants.Add(variant); + } + } + } + + document.Variants = variants; + break; + } + + case DocumentCollectionResponseModel document: + { + var variants = new List(); + if (document.Variants.Count() == 1) + { + DocumentVariantResponseModel variant = document.Variants.First(); + variant.AddSign(Alias); + variants.Add(variant); + } + else + { + IEnumerable schedules = + _contentService.GetContentScheduleByContentId(key).GetSchedule(); + IEnumerable culturesWithSchedule = schedules + .Select(schedule => schedule.Culture); + + foreach (DocumentVariantResponseModel variant in document.Variants) + { + if (variant.Culture != null && culturesWithSchedule.Contains(variant.Culture)) + { + variant.AddSign(Alias); + variants.Add(variant); + } + } + } + + document.Variants = variants; + break; + } + + case DocumentItemResponseModel document: + { + var variants = new List(); + if (document.Variants.Count() == 1) + { + DocumentVariantItemResponseModel variant = document.Variants.First(); + variant.AddSign(Alias); + variants.Add(variant); + } + else + { + IEnumerable schedules = + _contentService.GetContentScheduleByContentId(key).GetSchedule(); + IEnumerable culturesWithSchedule = schedules + .Select(schedule => schedule.Culture); + + foreach (DocumentVariantItemResponseModel variant in document.Variants) + { + if (variant.Culture != null && culturesWithSchedule.Contains(variant.Culture)) + { + variant.AddSign(Alias); + variants.Add(variant); + } + } + } + + document.Variants = variants; + break; + } + } } return Task.CompletedTask; From 7780c4b62cac19dd453f813f02c02b10e839432c Mon Sep 17 00:00:00 2001 From: NillasKA Date: Tue, 2 Sep 2025 14:05:46 +0200 Subject: [PATCH 04/13] Refactoring ISignProvider.cs to take an IEnumerable again --- src/Umbraco.Cms.Api.Management/Services/Signs/ISignProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Cms.Api.Management/Services/Signs/ISignProvider.cs b/src/Umbraco.Cms.Api.Management/Services/Signs/ISignProvider.cs index e36e10d74f90..e47a128e741a 100644 --- a/src/Umbraco.Cms.Api.Management/Services/Signs/ISignProvider.cs +++ b/src/Umbraco.Cms.Api.Management/Services/Signs/ISignProvider.cs @@ -18,6 +18,6 @@ bool CanProvideSigns() /// Populates the provided item view models with signs. /// /// Type of item view model supporting signs. - Task PopulateSignsAsync(TItem item) + Task PopulateSignsAsync(IEnumerable items) where TItem : IHasSigns; } From 233c5201711c325c648321c460de76ff112d1673 Mon Sep 17 00:00:00 2001 From: NillasKA Date: Tue, 2 Sep 2025 14:11:49 +0200 Subject: [PATCH 05/13] Moving code from controllers to factories --- .../ContentCollectionControllerBase.cs | 11 ------- .../ByKeyDocumentCollectionController.cs | 1 - .../Item/ItemDocumentItemController.cs | 22 +------------ .../ByKeyMediaCollectionController.cs | 1 - .../ContentCollectionPresentationFactory.cs | 31 ++++++++++++++++++- .../Factories/DocumentPresentationFactory.cs | 16 +++++----- 6 files changed, 40 insertions(+), 42 deletions(-) diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Content/ContentCollectionControllerBase.cs b/src/Umbraco.Cms.Api.Management/Controllers/Content/ContentCollectionControllerBase.cs index a38ade606019..e41f16cee62f 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/Content/ContentCollectionControllerBase.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/Content/ContentCollectionControllerBase.cs @@ -121,15 +121,4 @@ protected IActionResult ContentCollectionOperationStatusResult(ContentCollection StatusCode = StatusCodes.Status500InternalServerError, }, }); - - /// - /// Populates the signs for the collection response models. - /// - protected async Task PopulateSigns(IEnumerable itemViewModels) - { - foreach (ISignProvider signProvider in _signProviders.Where(x => x.CanProvideSigns())) - { - await signProvider.PopulateSignsAsync(itemViewModels); - } - } } diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Document/Collection/ByKeyDocumentCollectionController.cs b/src/Umbraco.Cms.Api.Management/Controllers/Document/Collection/ByKeyDocumentCollectionController.cs index 5ab89ba2748c..83ef32972c51 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/Document/Collection/ByKeyDocumentCollectionController.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/Document/Collection/ByKeyDocumentCollectionController.cs @@ -85,7 +85,6 @@ public async Task ByKey( } List collectionResponseModels = await _documentCollectionPresentationFactory.CreateCollectionModelAsync(collectionAttempt.Result!); - await PopulateSigns(collectionResponseModels); return CollectionResult(collectionResponseModels, collectionAttempt.Result!.Items.Total); } } diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Document/Item/ItemDocumentItemController.cs b/src/Umbraco.Cms.Api.Management/Controllers/Document/Item/ItemDocumentItemController.cs index f67f880e57d0..d65183c82423 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/Document/Item/ItemDocumentItemController.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/Document/Item/ItemDocumentItemController.cs @@ -17,25 +17,14 @@ public class ItemDocumentItemController : DocumentItemControllerBase { private readonly IEntityService _entityService; private readonly IDocumentPresentationFactory _documentPresentationFactory; - private readonly SignProviderCollection _signProviders; [ActivatorUtilitiesConstructor] public ItemDocumentItemController( IEntityService entityService, - IDocumentPresentationFactory documentPresentationFactory, - SignProviderCollection signProvider) + IDocumentPresentationFactory documentPresentationFactory) { _entityService = entityService; _documentPresentationFactory = documentPresentationFactory; - _signProviders = signProvider; - } - - [Obsolete("Please use the constructor with all parameters. Scheduled for removal in Umbraco 18")] - public ItemDocumentItemController( - IEntityService entityService, - IDocumentPresentationFactory documentPresentationFactory) - : this(entityService, documentPresentationFactory, StaticServiceProvider.Instance.GetRequiredService()) - { } [HttpGet] @@ -55,15 +44,6 @@ public async Task Item( .OfType(); IEnumerable responseModels = documents.Select(_documentPresentationFactory.CreateItemResponseModel); - await PopulateSigns(responseModels); return Ok(responseModels); } - - private async Task PopulateSigns(IEnumerable itemViewModels) - { - foreach (ISignProvider signProvider in _signProviders.Where(x => x.CanProvideSigns())) - { - await signProvider.PopulateSignsAsync(itemViewModels); - } - } } diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Media/Collection/ByKeyMediaCollectionController.cs b/src/Umbraco.Cms.Api.Management/Controllers/Media/Collection/ByKeyMediaCollectionController.cs index 184cfe4de036..7029e9311d36 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/Media/Collection/ByKeyMediaCollectionController.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/Media/Collection/ByKeyMediaCollectionController.cs @@ -84,7 +84,6 @@ public async Task ByKey( } List collectionResponseModels = await _mediaCollectionPresentationFactory.CreateCollectionModelAsync(collectionAttempt.Result!); - await PopulateSigns(collectionResponseModels); return CollectionResult(collectionResponseModels, collectionAttempt.Result!.Items.Total); } } diff --git a/src/Umbraco.Cms.Api.Management/Factories/ContentCollectionPresentationFactory.cs b/src/Umbraco.Cms.Api.Management/Factories/ContentCollectionPresentationFactory.cs index 0607df51084c..f151d6337bf4 100644 --- a/src/Umbraco.Cms.Api.Management/Factories/ContentCollectionPresentationFactory.cs +++ b/src/Umbraco.Cms.Api.Management/Factories/ContentCollectionPresentationFactory.cs @@ -1,4 +1,7 @@ +using Microsoft.Extensions.DependencyInjection; +using Umbraco.Cms.Api.Management.Services.Signs; using Umbraco.Cms.Api.Management.ViewModels.Content; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Mapping; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.ContentEditing; @@ -13,9 +16,24 @@ public abstract class ContentCollectionPresentationFactory _mapper = mapper; + [Obsolete("Please use the controller with all parameters, will be removed in Umbraco 18")] + protected ContentCollectionPresentationFactory(IUmbracoMapper mapper) + : this( + mapper, + StaticServiceProvider.Instance.GetRequiredService()) + { + } + + protected ContentCollectionPresentationFactory( + IUmbracoMapper mapper, + SignProviderCollection signProviderCollection) + { + _mapper = mapper; + _signProviderCollection = signProviderCollection; + } public async Task> CreateCollectionModelAsync(ListViewPagedModel contentCollection) { @@ -36,8 +54,19 @@ public async Task> CreateCollectionModelAsync(Lis await SetUnmappedProperties(contentCollection, collectionResponseModels); + + await PopulateSigns(collectionResponseModels); + return collectionResponseModels; } protected virtual Task SetUnmappedProperties(ListViewPagedModel contentCollection, List collectionResponseModels) => Task.CompletedTask; + + private async Task PopulateSigns(IEnumerable models) + { + foreach (ISignProvider signProvider in _signProviderCollection.Where(x => x.CanProvideSigns())) + { + await signProvider.PopulateSignsAsync(models); + } + } } diff --git a/src/Umbraco.Cms.Api.Management/Factories/DocumentPresentationFactory.cs b/src/Umbraco.Cms.Api.Management/Factories/DocumentPresentationFactory.cs index e54c289a476d..b932b71ac588 100644 --- a/src/Umbraco.Cms.Api.Management/Factories/DocumentPresentationFactory.cs +++ b/src/Umbraco.Cms.Api.Management/Factories/DocumentPresentationFactory.cs @@ -130,7 +130,7 @@ public DocumentItemResponseModel CreateItemResponseModel(IDocumentEntitySlim ent responseModel.Variants = CreateVariantsItemResponseModels(entity); - PopulateSignsOnDocuments(responseModel).GetAwaiter().GetResult(); + PopulateSignsOnDocuments(responseModel); return responseModel; } @@ -159,7 +159,7 @@ public IEnumerable CreateVariantsItemResponseM Culture = null, }; - PopulateSignsOnVariants(model).GetAwaiter().GetResult(); + PopulateSignsOnVariants(model); yield return model; yield break; } @@ -173,7 +173,7 @@ public IEnumerable CreateVariantsItemResponseM State = DocumentVariantStateHelper.GetState(entity, cultureNamePair.Key) }; - PopulateSignsOnVariants(model).GetAwaiter().GetResult(); + PopulateSignsOnVariants(model); yield return model; } } @@ -290,19 +290,21 @@ public Attempt, ContentPublishingOperationStat return Attempt.SucceedWithStatus(ContentPublishingOperationStatus.Success, model); } - private async Task PopulateSignsOnDocuments(DocumentItemResponseModel model) + private void PopulateSignsOnDocuments(DocumentItemResponseModel model) { foreach (ISignProvider signProvider in _signProviderCollection.Where(x => x.CanProvideSigns())) { - await signProvider.PopulateSignsAsync(model); + IEnumerable models = [model]; + signProvider.PopulateSignsAsync(models).GetAwaiter().GetResult(); } } - private async Task PopulateSignsOnVariants(DocumentVariantItemResponseModel model) + private void PopulateSignsOnVariants(DocumentVariantItemResponseModel model) { foreach (ISignProvider signProvider in _signProviderCollection.Where(x => x.CanProvideSigns())) { - await signProvider.PopulateSignsAsync(model); + IEnumerable models = [model]; + signProvider.PopulateSignsAsync(models).GetAwaiter().GetResult(); } } } From 388f8db1fc4248be275ef54d2e3aa3ff04787a3f Mon Sep 17 00:00:00 2001 From: NillasKA Date: Tue, 2 Sep 2025 14:12:12 +0200 Subject: [PATCH 06/13] Refactoring HasPendingChangesSignProvider.cs to use the right Interface method --- .../Services/Signs/HasPendingChangesSignProvider.cs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/Umbraco.Cms.Api.Management/Services/Signs/HasPendingChangesSignProvider.cs b/src/Umbraco.Cms.Api.Management/Services/Signs/HasPendingChangesSignProvider.cs index 551d9fb84768..e7563aa31071 100644 --- a/src/Umbraco.Cms.Api.Management/Services/Signs/HasPendingChangesSignProvider.cs +++ b/src/Umbraco.Cms.Api.Management/Services/Signs/HasPendingChangesSignProvider.cs @@ -19,12 +19,15 @@ public bool CanProvideSigns() /// - public Task PopulateSignsAsync(TItem item) + public Task PopulateSignsAsync(IEnumerable items) where TItem : IHasSigns { - if (HasPendingChanges(item)) + foreach (TItem item in items) { - item.AddSign(Alias); + if (HasPendingChanges(item)) + { + item.AddSign(Alias); + } } return Task.CompletedTask; From 94d07b7fd26b89fc8ae10f85a9bdd6b8df42f432 Mon Sep 17 00:00:00 2001 From: NillasKA Date: Tue, 2 Sep 2025 14:13:02 +0200 Subject: [PATCH 07/13] Refactoring HasScheduleSignProvider.cs to be less bloated, and more readable (hopefully) --- .../Services/Signs/HasScheduleSignProvider.cs | 154 ++++++++---------- 1 file changed, 66 insertions(+), 88 deletions(-) diff --git a/src/Umbraco.Cms.Api.Management/Services/Signs/HasScheduleSignProvider.cs b/src/Umbraco.Cms.Api.Management/Services/Signs/HasScheduleSignProvider.cs index 60952d50ed7c..a17b23084fd8 100644 --- a/src/Umbraco.Cms.Api.Management/Services/Signs/HasScheduleSignProvider.cs +++ b/src/Umbraco.Cms.Api.Management/Services/Signs/HasScheduleSignProvider.cs @@ -3,7 +3,6 @@ using Umbraco.Cms.Api.Management.ViewModels.Document.Collection; using Umbraco.Cms.Api.Management.ViewModels.Document.Item; using Umbraco.Cms.Api.Management.ViewModels.Tree; -using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Services; using Constants = Umbraco.Cms.Core.Constants; @@ -35,105 +34,84 @@ public Task PopulateSignsAsync(IEnumerable items) where TItem : IHasSigns { IEnumerable keys = _contentService.GetScheduledContentKeys(items.Select(x => x.Id).ToArray()); + var itemsById = items.ToDictionary(x => x.Id); foreach (Guid key in keys) { - TItem item = items.First(x => x.Id == key); + if (!itemsById.TryGetValue(key, out TItem? item)) + { + continue; + } switch (item) { - case DocumentTreeItemResponseModel document: - { - var variants = new List(); - if (document.Variants.Count() == 1) - { - DocumentVariantItemResponseModel variant = document.Variants.First(); - variant.AddSign(Alias); - variants.Add(variant); - } - else - { - IEnumerable schedules = - _contentService.GetContentScheduleByContentId(key).GetSchedule(); - IEnumerable culturesWithSchedule = schedules - .Select(schedule => schedule.Culture); - - foreach (DocumentVariantItemResponseModel variant in document.Variants) - { - if (variant.Culture != null && culturesWithSchedule.Contains(variant.Culture)) - { - variant.AddSign(Alias); - variants.Add(variant); - } - } - } - - document.Variants = variants; + case DocumentTreeItemResponseModel docTree: + docTree.Variants = BuildVariants(docTree.Variants, key); break; - } - - case DocumentCollectionResponseModel document: - { - var variants = new List(); - if (document.Variants.Count() == 1) - { - DocumentVariantResponseModel variant = document.Variants.First(); - variant.AddSign(Alias); - variants.Add(variant); - } - else - { - IEnumerable schedules = - _contentService.GetContentScheduleByContentId(key).GetSchedule(); - IEnumerable culturesWithSchedule = schedules - .Select(schedule => schedule.Culture); - - foreach (DocumentVariantResponseModel variant in document.Variants) - { - if (variant.Culture != null && culturesWithSchedule.Contains(variant.Culture)) - { - variant.AddSign(Alias); - variants.Add(variant); - } - } - } - - document.Variants = variants; + + case DocumentCollectionResponseModel docColl: + docColl.Variants = BuildVariants(docColl.Variants, key); break; - } - - case DocumentItemResponseModel document: - { - var variants = new List(); - if (document.Variants.Count() == 1) - { - DocumentVariantItemResponseModel variant = document.Variants.First(); - variant.AddSign(Alias); - variants.Add(variant); - } - else - { - IEnumerable schedules = - _contentService.GetContentScheduleByContentId(key).GetSchedule(); - IEnumerable culturesWithSchedule = schedules - .Select(schedule => schedule.Culture); - - foreach (DocumentVariantItemResponseModel variant in document.Variants) - { - if (variant.Culture != null && culturesWithSchedule.Contains(variant.Culture)) - { - variant.AddSign(Alias); - variants.Add(variant); - } - } - } - - document.Variants = variants; + + case DocumentItemResponseModel docItem: + docItem.Variants = BuildVariants(docItem.Variants, key); break; - } } } return Task.CompletedTask; } + + private List BuildVariants(IEnumerable variants, Guid id) + { + var listVariants = variants.ToList(); + if (listVariants.Count == 1) + { + DocumentVariantItemResponseModel variant = listVariants[0]; + variant.AddSign(Alias); + return [variant]; + } + + IEnumerable cultures = GetCulturesWithSchedule(id); + var result = new List(listVariants.Count); + foreach (DocumentVariantItemResponseModel variant in listVariants) + { + if (variant.Culture != null && cultures.Contains(variant.Culture)) + { + variant.AddSign(Alias); + result.Add(variant); + } + } + + return result; + } + + private List BuildVariants(IEnumerable variants, Guid id) + { + var listVariants = variants.ToList(); + if (listVariants.Count == 1) + { + DocumentVariantResponseModel variant = listVariants[0]; + variant.AddSign(Alias); + return [variant]; + } + + IEnumerable cultures = GetCulturesWithSchedule(id); + var result = new List(listVariants.Count); + foreach (DocumentVariantResponseModel variant in listVariants) + { + if (variant.Culture != null && cultures.Contains(variant.Culture)) + { + variant.AddSign(Alias); + result.Add(variant); + } + } + + return result; + } + + private IEnumerable GetCulturesWithSchedule(Guid id) => + _contentService.GetContentScheduleByContentId(id) + .GetSchedule() + .Select(s => s.Culture); } From 86cf9098c793c4f4d9b14a570b14095e38eced35 Mon Sep 17 00:00:00 2001 From: NillasKA Date: Tue, 2 Sep 2025 14:43:36 +0200 Subject: [PATCH 08/13] Refactoring tests to look at variants and include a list --- .../HasPendingChangesSignProviderTests.cs | 58 +++++++++++-------- .../Signs/HasScheduleSignProviderTests.cs | 35 +++++++---- 2 files changed, 57 insertions(+), 36 deletions(-) diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Cms.Api.Management/Services/Signs/HasPendingChangesSignProviderTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Cms.Api.Management/Services/Signs/HasPendingChangesSignProviderTests.cs index 78f60464c268..58dcb07c3203 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Cms.Api.Management/Services/Signs/HasPendingChangesSignProviderTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Cms.Api.Management/Services/Signs/HasPendingChangesSignProviderTests.cs @@ -29,23 +29,28 @@ public async Task HasPendingChangesSignProvider_Should_Populate_Variant_Item_Sig { var sut = new HasPendingChangesSignProvider(); - var variant1 = new DocumentVariantItemResponseModel() + var variants = new List { - State = DocumentVariantState.PublishedPendingChanges, Culture = null, Name = "Test", + new() + { + State = DocumentVariantState.PublishedPendingChanges, + Culture = null, + Name = "Test", + }, + new() + { + State = DocumentVariantState.Published, + Culture = null, + Name = "Test2", + }, }; - var variant2 = new DocumentVariantItemResponseModel() - { - State = DocumentVariantState.Published, Culture = null, Name = "Test", - }; - - await sut.PopulateSignsAsync(variant1); - await sut.PopulateSignsAsync(variant2); + await sut.PopulateSignsAsync(variants); - Assert.AreEqual(variant1.Signs.Count(), 1); - Assert.AreEqual(variant2.Signs.Count(), 0); + Assert.AreEqual(variants[0].Signs.Count(), 1); + Assert.AreEqual(variants[1].Signs.Count(), 0); - var signModel = variant1.Signs.First(); + var signModel = variants[0].Signs.First(); Assert.AreEqual("Umb.PendingChanges", signModel.Alias); } @@ -54,23 +59,28 @@ public async Task HasPendingChangesSignProvider_Should_Populate_Variant_Signs() { var sut = new HasPendingChangesSignProvider(); - var variant1 = new DocumentVariantResponseModel() - { - State = DocumentVariantState.PublishedPendingChanges, Culture = null, Name = "Test", - }; - - var variant2 = new DocumentVariantResponseModel() + var variants = new List { - State = DocumentVariantState.Published, Culture = null, Name = "Test", + new() + { + State = DocumentVariantState.PublishedPendingChanges, + Culture = null, + Name = "Test", + }, + new() + { + State = DocumentVariantState.Published, + Culture = null, + Name = "Test2", + }, }; - await sut.PopulateSignsAsync(variant1); - await sut.PopulateSignsAsync(variant2); + await sut.PopulateSignsAsync(variants); - Assert.AreEqual(variant1.Signs.Count(), 1); - Assert.AreEqual(variant2.Signs.Count(), 0); + Assert.AreEqual(variants[0].Signs.Count(), 1); + Assert.AreEqual(variants[1].Signs.Count(), 0); - var signModel = variant1.Signs.First(); + var signModel = variants[0].Signs.First(); Assert.AreEqual("Umb.PendingChanges", signModel.Alias); } } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Cms.Api.Management/Services/Signs/HasScheduleSignProviderTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Cms.Api.Management/Services/Signs/HasScheduleSignProviderTests.cs index 48b97ff30b76..2b0ab834b17b 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Cms.Api.Management/Services/Signs/HasScheduleSignProviderTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Cms.Api.Management/Services/Signs/HasScheduleSignProviderTests.cs @@ -1,6 +1,8 @@ using Moq; using NUnit.Framework; using Umbraco.Cms.Api.Management.Services.Signs; +using Umbraco.Cms.Api.Management.ViewModels.Content; +using Umbraco.Cms.Api.Management.ViewModels.Document; using Umbraco.Cms.Api.Management.ViewModels.Document.Collection; using Umbraco.Cms.Api.Management.ViewModels.Document.Item; using Umbraco.Cms.Api.Management.ViewModels.Tree; @@ -53,17 +55,20 @@ public async Task HasScheduleSignProvider_Should_Populate_Document_Tree_Signs() .Returns([entities[1].Key]); var sut = new HasScheduleSignProvider(contentServiceMock.Object); + var variant1 = new DocumentVariantItemResponseModel() { State = DocumentVariantState.Published, Name = "Test" }; + var variant2 = new DocumentVariantItemResponseModel() { State = DocumentVariantState.PublishedPendingChanges, Name = "Test" }; + var viewModels = new List { - new() { Id = entities[0].Key }, new() { Id = entities[1].Key }, + new() { Id = entities[0].Key, Variants = [variant1] }, new() { Id = entities[1].Key, Variants = [variant2] }, }; await sut.PopulateSignsAsync(viewModels); - Assert.AreEqual(viewModels[0].Signs.Count(), 0); - Assert.AreEqual(viewModels[1].Signs.Count(), 1); + Assert.AreEqual(viewModels[0].Variants.First().Signs.Count(), 0); + Assert.AreEqual(viewModels[1].Variants.First().Signs.Count(), 1); - var signModel = viewModels[1].Signs.First(); + var signModel = viewModels[1].Variants.First().Signs.First(); Assert.AreEqual("Umb.ScheduledForPublish", signModel.Alias); } @@ -81,17 +86,20 @@ public async Task HasScheduleSignProvider_Should_Populate_Document_Collection_Si .Returns([entities[1].Key]); var sut = new HasScheduleSignProvider(contentServiceMock.Object); + var variant1 = new DocumentVariantResponseModel() { State = DocumentVariantState.Published, Name = "Test" }; + var variant2 = new DocumentVariantResponseModel() { State = DocumentVariantState.PublishedPendingChanges, Name = "Test" }; + var viewModels = new List { - new() { Id = entities[0].Key }, new() { Id = entities[1].Key }, + new() { Id = entities[0].Key, Variants = [variant1] }, new() { Id = entities[1].Key, Variants = [variant2] }, }; await sut.PopulateSignsAsync(viewModels); - Assert.AreEqual(viewModels[0].Signs.Count(), 0); - Assert.AreEqual(viewModels[1].Signs.Count(), 1); + Assert.AreEqual(viewModels[0].Variants.First().Signs.Count(), 0); + Assert.AreEqual(viewModels[1].Variants.First().Signs.Count(), 1); - var signModel = viewModels[1].Signs.First(); + var signModel = viewModels[1].Variants.First().Signs.First(); Assert.AreEqual("Umb.ScheduledForPublish", signModel.Alias); } @@ -109,17 +117,20 @@ public async Task HasScheduleSignProvider_Should_Populate_Document_Item_Signs() .Returns([entities[1].Key]); var sut = new HasScheduleSignProvider(contentServiceMock.Object); + var variant1 = new DocumentVariantItemResponseModel() { State = DocumentVariantState.Published, Name = "Test" }; + var variant2 = new DocumentVariantItemResponseModel() { State = DocumentVariantState.PublishedPendingChanges, Name = "Test" }; + var viewModels = new List { - new() { Id = entities[0].Key }, new() { Id = entities[1].Key }, + new() { Id = entities[0].Key, Variants = [variant1] }, new() { Id = entities[1].Key, Variants = [variant2] }, }; await sut.PopulateSignsAsync(viewModels); - Assert.AreEqual(viewModels[0].Signs.Count(), 0); - Assert.AreEqual(viewModels[1].Signs.Count(), 1); + Assert.AreEqual(viewModels[0].Variants.First().Signs.Count(), 0); + Assert.AreEqual(viewModels[1].Variants.First().Signs.Count(), 1); - var signModel = viewModels[1].Signs.First(); + var signModel = viewModels[1].Variants.First().Signs.First(); Assert.AreEqual("Umb.ScheduledForPublish", signModel.Alias); } } From 145c18e645f01a9ef439d38e7ccba0c252fe6721 Mon Sep 17 00:00:00 2001 From: NillasKA Date: Wed, 3 Sep 2025 13:12:55 +0200 Subject: [PATCH 09/13] Changing instantiation to be better --- .../Factories/DocumentPresentationFactory.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Umbraco.Cms.Api.Management/Factories/DocumentPresentationFactory.cs b/src/Umbraco.Cms.Api.Management/Factories/DocumentPresentationFactory.cs index b932b71ac588..37c127a24079 100644 --- a/src/Umbraco.Cms.Api.Management/Factories/DocumentPresentationFactory.cs +++ b/src/Umbraco.Cms.Api.Management/Factories/DocumentPresentationFactory.cs @@ -294,8 +294,7 @@ private void PopulateSignsOnDocuments(DocumentItemResponseModel model) { foreach (ISignProvider signProvider in _signProviderCollection.Where(x => x.CanProvideSigns())) { - IEnumerable models = [model]; - signProvider.PopulateSignsAsync(models).GetAwaiter().GetResult(); + signProvider.PopulateSignsAsync([model]).GetAwaiter().GetResult(); } } @@ -303,8 +302,7 @@ private void PopulateSignsOnVariants(DocumentVariantItemResponseModel model) { foreach (ISignProvider signProvider in _signProviderCollection.Where(x => x.CanProvideSigns())) { - IEnumerable models = [model]; - signProvider.PopulateSignsAsync(models).GetAwaiter().GetResult(); + signProvider.PopulateSignsAsync([model]).GetAwaiter().GetResult(); } } } From f1ff549a7c8bed5bff3ef723df0bfbb5ea70b69b Mon Sep 17 00:00:00 2001 From: NillasKA Date: Wed, 3 Sep 2025 13:13:37 +0200 Subject: [PATCH 10/13] Fixed minor logic issue in HasScheduleSignProvider.cs --- .../Services/Signs/HasScheduleSignProvider.cs | 56 ++++++++++--------- 1 file changed, 29 insertions(+), 27 deletions(-) diff --git a/src/Umbraco.Cms.Api.Management/Services/Signs/HasScheduleSignProvider.cs b/src/Umbraco.Cms.Api.Management/Services/Signs/HasScheduleSignProvider.cs index a17b23084fd8..e67c0cf2d52c 100644 --- a/src/Umbraco.Cms.Api.Management/Services/Signs/HasScheduleSignProvider.cs +++ b/src/Umbraco.Cms.Api.Management/Services/Signs/HasScheduleSignProvider.cs @@ -3,6 +3,7 @@ using Umbraco.Cms.Api.Management.ViewModels.Document.Collection; using Umbraco.Cms.Api.Management.ViewModels.Document.Item; using Umbraco.Cms.Api.Management.ViewModels.Tree; +using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Services; using Constants = Umbraco.Cms.Core.Constants; @@ -45,16 +46,16 @@ public Task PopulateSignsAsync(IEnumerable items) switch (item) { - case DocumentTreeItemResponseModel docTree: - docTree.Variants = BuildVariants(docTree.Variants, key); + case DocumentTreeItemResponseModel documentTreeItemResponseModel: + documentTreeItemResponseModel.Variants = BuildVariants(documentTreeItemResponseModel.Variants, key); break; - case DocumentCollectionResponseModel docColl: - docColl.Variants = BuildVariants(docColl.Variants, key); + case DocumentCollectionResponseModel documentCollectionResponseModel: + documentCollectionResponseModel.Variants = BuildVariants(documentCollectionResponseModel.Variants, key); break; - case DocumentItemResponseModel docItem: - docItem.Variants = BuildVariants(docItem.Variants, key); + case DocumentItemResponseModel documentItemResponseModel: + documentItemResponseModel.Variants = BuildVariants(documentItemResponseModel.Variants, key); break; } } @@ -62,56 +63,57 @@ public Task PopulateSignsAsync(IEnumerable items) return Task.CompletedTask; } - private List BuildVariants(IEnumerable variants, Guid id) + private IEnumerable BuildVariants( + IEnumerable variants, Guid id) { - var listVariants = variants.ToList(); - if (listVariants.Count == 1) + DocumentVariantItemResponseModel[] listVariants = variants.ToArray(); + if (listVariants.Length == 1) { DocumentVariantItemResponseModel variant = listVariants[0]; variant.AddSign(Alias); - return [variant]; + return listVariants; } - IEnumerable cultures = GetCulturesWithSchedule(id); - var result = new List(listVariants.Count); + IEnumerable schedules = GetSchedule(id); foreach (DocumentVariantItemResponseModel variant in listVariants) { - if (variant.Culture != null && cultures.Contains(variant.Culture)) + bool isScheduled = schedules.Any(schedule => schedule.Date > DateTime.Now && string.Equals(schedule.Culture, variant.Culture)); + + if (isScheduled) { variant.AddSign(Alias); - result.Add(variant); } } - return result; + return listVariants; } - private List BuildVariants(IEnumerable variants, Guid id) + private IEnumerable BuildVariants( + IEnumerable variants, Guid id) { - var listVariants = variants.ToList(); - if (listVariants.Count == 1) + DocumentVariantResponseModel[] listVariants = variants.ToArray(); + if (listVariants.Length == 1) { DocumentVariantResponseModel variant = listVariants[0]; variant.AddSign(Alias); - return [variant]; + return listVariants; } - IEnumerable cultures = GetCulturesWithSchedule(id); - var result = new List(listVariants.Count); + IEnumerable schedules = GetSchedule(id); + var result = new List(listVariants.Length); foreach (DocumentVariantResponseModel variant in listVariants) { - if (variant.Culture != null && cultures.Contains(variant.Culture)) + bool isScheduled = schedules.Any(schedule => schedule.Date > DateTime.Now && string.Equals(schedule.Culture, variant.Culture)); + + if (isScheduled) { variant.AddSign(Alias); - result.Add(variant); } } return result; } - private IEnumerable GetCulturesWithSchedule(Guid id) => - _contentService.GetContentScheduleByContentId(id) - .GetSchedule() - .Select(s => s.Culture); + private IEnumerable GetSchedule(Guid id) => + _contentService.GetContentScheduleByContentId(id).FullSchedule; } From 46efb3d785ca20ab41b4a79947aefc3bc928fd3b Mon Sep 17 00:00:00 2001 From: NillasKA Date: Thu, 4 Sep 2025 10:54:21 +0200 Subject: [PATCH 11/13] Refactoring to include just 1 database call. --- .../Services/Signs/HasScheduleSignProvider.cs | 72 ++++++++++--------- .../Repositories/IDocumentRepository.cs | 4 +- src/Umbraco.Core/Services/ContentService.cs | 28 ++++++-- src/Umbraco.Core/Services/IContentService.cs | 16 ++++- .../Implement/DocumentRepository.cs | 40 ++++++----- 5 files changed, 100 insertions(+), 60 deletions(-) diff --git a/src/Umbraco.Cms.Api.Management/Services/Signs/HasScheduleSignProvider.cs b/src/Umbraco.Cms.Api.Management/Services/Signs/HasScheduleSignProvider.cs index e67c0cf2d52c..e9f949a57ac8 100644 --- a/src/Umbraco.Cms.Api.Management/Services/Signs/HasScheduleSignProvider.cs +++ b/src/Umbraco.Cms.Api.Management/Services/Signs/HasScheduleSignProvider.cs @@ -3,6 +3,7 @@ using Umbraco.Cms.Api.Management.ViewModels.Document.Collection; using Umbraco.Cms.Api.Management.ViewModels.Document.Item; using Umbraco.Cms.Api.Management.ViewModels.Tree; +using Umbraco.Cms.Core; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Services; using Constants = Umbraco.Cms.Core.Constants; @@ -17,11 +18,16 @@ internal class HasScheduleSignProvider : ISignProvider private const string Alias = Constants.Conventions.Signs.Prefix + "ScheduledForPublish"; private readonly IContentService _contentService; + private readonly IIdKeyMap _keyMap; /// /// Initializes a new instance of the class. /// - public HasScheduleSignProvider(IContentService contentService) => _contentService = contentService; + public HasScheduleSignProvider(IContentService contentService, IIdKeyMap keyMap) + { + _contentService = contentService; + _keyMap = keyMap; + } /// public bool CanProvideSigns() @@ -34,12 +40,16 @@ public bool CanProvideSigns() public Task PopulateSignsAsync(IEnumerable items) where TItem : IHasSigns { - IEnumerable keys = _contentService.GetScheduledContentKeys(items.Select(x => x.Id).ToArray()); - var itemsById = items.ToDictionary(x => x.Id); - - foreach (Guid key in keys) + IDictionary> schedules = _contentService.GetContentSchedulesByIds(items.Select(x => x.Id).ToArray()); + foreach (TItem item in items) { - if (!itemsById.TryGetValue(key, out TItem? item)) + Attempt itemId = _keyMap.GetIdForKey(item.Id, UmbracoObjectTypes.Document); + if (itemId.Success is false) + { + continue; + } + + if (!schedules.TryGetValue(itemId.Result, out IEnumerable? contentSchedules)) { continue; } @@ -47,15 +57,15 @@ public Task PopulateSignsAsync(IEnumerable items) switch (item) { case DocumentTreeItemResponseModel documentTreeItemResponseModel: - documentTreeItemResponseModel.Variants = BuildVariants(documentTreeItemResponseModel.Variants, key); + documentTreeItemResponseModel.Variants = PopulateVariants(documentTreeItemResponseModel.Variants, contentSchedules); break; case DocumentCollectionResponseModel documentCollectionResponseModel: - documentCollectionResponseModel.Variants = BuildVariants(documentCollectionResponseModel.Variants, key); + documentCollectionResponseModel.Variants = PopulateVariants(documentCollectionResponseModel.Variants, contentSchedules); break; case DocumentItemResponseModel documentItemResponseModel: - documentItemResponseModel.Variants = BuildVariants(documentItemResponseModel.Variants, key); + documentItemResponseModel.Variants = PopulateVariants(documentItemResponseModel.Variants, contentSchedules); break; } } @@ -63,21 +73,21 @@ public Task PopulateSignsAsync(IEnumerable items) return Task.CompletedTask; } - private IEnumerable BuildVariants( - IEnumerable variants, Guid id) + private IEnumerable PopulateVariants( + IEnumerable variants, IEnumerable schedules) { - DocumentVariantItemResponseModel[] listVariants = variants.ToArray(); - if (listVariants.Length == 1) + DocumentVariantItemResponseModel[] variantsArray = variants.ToArray(); + if (variantsArray.Length == 1) { - DocumentVariantItemResponseModel variant = listVariants[0]; + DocumentVariantItemResponseModel variant = variantsArray[0]; variant.AddSign(Alias); - return listVariants; + return variantsArray; } - IEnumerable schedules = GetSchedule(id); - foreach (DocumentVariantItemResponseModel variant in listVariants) + foreach (DocumentVariantItemResponseModel variant in variantsArray) { - bool isScheduled = schedules.Any(schedule => schedule.Date > DateTime.Now && string.Equals(schedule.Culture, variant.Culture)); + ContentSchedule? schedule = schedules.FirstOrDefault(x => x.Culture == variant.Culture); + bool isScheduled = schedule != null && schedule.Date > DateTime.Now && string.Equals(schedule.Culture, variant.Culture); if (isScheduled) { @@ -85,25 +95,24 @@ private IEnumerable BuildVariants( } } - return listVariants; + return variantsArray; } - private IEnumerable BuildVariants( - IEnumerable variants, Guid id) + private IEnumerable PopulateVariants( + IEnumerable variants, IEnumerable schedules) { - DocumentVariantResponseModel[] listVariants = variants.ToArray(); - if (listVariants.Length == 1) + DocumentVariantResponseModel[] variantsArray = variants.ToArray(); + if (variantsArray.Length == 1) { - DocumentVariantResponseModel variant = listVariants[0]; + DocumentVariantResponseModel variant = variantsArray[0]; variant.AddSign(Alias); - return listVariants; + return variantsArray; } - IEnumerable schedules = GetSchedule(id); - var result = new List(listVariants.Length); - foreach (DocumentVariantResponseModel variant in listVariants) + foreach (DocumentVariantResponseModel variant in variantsArray) { - bool isScheduled = schedules.Any(schedule => schedule.Date > DateTime.Now && string.Equals(schedule.Culture, variant.Culture)); + ContentSchedule? schedule = schedules.FirstOrDefault(x => x.Culture == variant.Culture); + bool isScheduled = schedule != null && schedule.Date > DateTime.Now && string.Equals(schedule.Culture, variant.Culture); if (isScheduled) { @@ -111,9 +120,6 @@ private IEnumerable BuildVariants( } } - return result; + return variantsArray; } - - private IEnumerable GetSchedule(Guid id) => - _contentService.GetContentScheduleByContentId(id).FullSchedule; } diff --git a/src/Umbraco.Core/Persistence/Repositories/IDocumentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/IDocumentRepository.cs index 9ff75c2b3d11..2ea536e902c4 100644 --- a/src/Umbraco.Core/Persistence/Repositories/IDocumentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/IDocumentRepository.cs @@ -53,11 +53,11 @@ public interface IDocumentRepository : IContentRepository, IReadR /// /// Gets the content keys from the provided collection of keys that are scheduled for publishing. /// - /// The content keys. + /// The IDs of the documents. /// /// The provided collection of content keys filtered for those that are scheduled for publishing. /// - IEnumerable GetScheduledContentKeys(Guid[] keys) => []; + IDictionary> GetContentSchedulesByIds(int[] documentIds); /// /// Get the count of published items diff --git a/src/Umbraco.Core/Services/ContentService.cs b/src/Umbraco.Core/Services/ContentService.cs index 016d4a83caf5..a53d50db8388 100644 --- a/src/Umbraco.Core/Services/ContentService.cs +++ b/src/Umbraco.Core/Services/ContentService.cs @@ -1,3 +1,4 @@ +using System.Collections.Immutable; using System.Diagnostics.CodeAnalysis; using System.Runtime.InteropServices; using Microsoft.Extensions.DependencyInjection; @@ -1009,21 +1010,38 @@ public IEnumerable GetPagedContentInRecycleBin(long pageIndex, int pag /// - public IEnumerable GetScheduledContentKeys(IEnumerable keys) + public IDictionary> GetContentSchedulesByIds(int[] documentIds) { - Guid[] idsA = keys.ToArray(); - if (idsA.Length == 0) + if (documentIds.Length == 0) { - return Enumerable.Empty(); + return ImmutableDictionary>.Empty; } using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true)) { scope.ReadLock(Constants.Locks.ContentTree); - return _documentRepository.GetScheduledContentKeys(idsA); + scope.ReadLock(Constants.Locks.Languages); + return _documentRepository.GetContentSchedulesByIds(documentIds); } } + public IDictionary> GetContentSchedulesByIds(Guid[] keys) + { + List contentIds = []; + foreach (var key in keys) + { + Attempt contentId = _idKeyMap.GetIdForKey(key, UmbracoObjectTypes.Document); + if (contentId.Success is false) + { + continue; + } + + contentIds.Add(contentId.Result); + } + + return GetContentSchedulesByIds(contentIds.ToArray()); + } + /// /// Checks if the passed in can be published based on the ancestors publish state. /// diff --git a/src/Umbraco.Core/Services/IContentService.cs b/src/Umbraco.Core/Services/IContentService.cs index 228d7fa6dbea..7ee50f1a6f2d 100644 --- a/src/Umbraco.Core/Services/IContentService.cs +++ b/src/Umbraco.Core/Services/IContentService.cs @@ -276,13 +276,23 @@ IContent CreateBlueprintFromContent(IContent blueprint, string name, int userId bool HasChildren(int id); /// - /// Gets the content keys from the provided collection of keys that are scheduled for publishing. + /// Gets a dictionary of content Ids and their matching content schedules. + /// + /// The IDs of the documents. + /// + /// A dictionary with a nodeId and an IEnumerable of matching ContentSchedules. + /// + IDictionary> GetContentSchedulesByIds(int[] documentIds); + + /// + /// Gets a dictionary of content Ids and their matching content schedules. /// /// The content keys. /// - /// The provided collection of content keys filtered for those that are scheduled for publishing. + /// A dictionary with a nodeId and an IEnumerable of matching ContentSchedules. /// - IEnumerable GetScheduledContentKeys(IEnumerable keys) => []; + IDictionary> GetContentSchedulesByIds(Guid[] keys); + #endregion diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DocumentRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DocumentRepository.cs index 1279d621860b..6d5756632140 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DocumentRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DocumentRepository.cs @@ -1672,24 +1672,30 @@ public IEnumerable GetContentForRelease(DateTime date) } /// - public IEnumerable GetScheduledContentKeys(Guid[] keys) + public IDictionary> GetContentSchedulesByIds(int[] documentIds) { - var action = ContentScheduleAction.Release.ToString(); - DateTime now = DateTime.UtcNow; - - Sql sql = SqlContext.Sql(); - sql - .Select(x => x.UniqueId) - .From() - .InnerJoin().On(left => left.NodeId, right => right.NodeId) - .InnerJoin().On(left => left.NodeId, right => right.NodeId) - .WhereIn(x => x.UniqueId, keys) - .WhereIn(x => x.NodeId, Sql() - .Select(x => x.NodeId) - .From() - .Where(x => x.Action == action && x.Date >= now)); - - return Database.Fetch(sql); + Sql sql = Sql() + .Select() + .From() + .WhereIn(contentScheduleDto => contentScheduleDto.NodeId, documentIds); + + List? contentScheduleDtos = Database.Fetch(sql); + + IDictionary> dictionary = contentScheduleDtos + .GroupBy(contentSchedule => contentSchedule.NodeId) + .ToDictionary( + group => group.Key, + group => group.Select(scheduleDto => new ContentSchedule( + scheduleDto.Id, + LanguageRepository.GetIsoCodeById(scheduleDto.LanguageId) ?? Constants.System.InvariantCulture, + scheduleDto.Date, + scheduleDto.Action == ContentScheduleAction.Release.ToString() + ? ContentScheduleAction.Release + : ContentScheduleAction.Expire)) + .ToList().AsEnumerable()); // We have to materialize it here, + // to avoid this being used after the scope is disposed. + + return dictionary; } /// From 39529205af9f3c8811e527a445784a4447141adb Mon Sep 17 00:00:00 2001 From: NillasKA Date: Thu, 4 Sep 2025 12:23:33 +0200 Subject: [PATCH 12/13] Adjusting tests to use the new methods. --- .../Services/ContentServiceTests.cs | 27 ++++- .../Signs/HasScheduleSignProviderTests.cs | 105 +++++++++++++----- 2 files changed, 102 insertions(+), 30 deletions(-) diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/ContentServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/ContentServiceTests.cs index 0271b77e8482..8844eba3e871 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/ContentServiceTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/ContentServiceTests.cs @@ -688,7 +688,7 @@ public void Can_Get_Content_For_Expiration() } [Test] - public void Can_Get_Scheduled_Content_Keys() + public void Can_Get_Content_Schedules_By_Keys() { // Arrange var root = ContentService.GetById(Textpage.Id); @@ -699,11 +699,32 @@ public void Can_Get_Scheduled_Content_Keys() ContentService.Publish(content, content.AvailableCultures.ToArray()); // Act - var keys = ContentService.GetScheduledContentKeys([Textpage.Key, Subpage.Key, Subpage2.Key]).ToList(); + var keys = ContentService.GetContentSchedulesByIds([Textpage.Key, Subpage.Key, Subpage2.Key]).ToList(); // Assert Assert.AreEqual(1, keys.Count); - Assert.AreEqual(Subpage.Key, keys.First()); + Assert.AreEqual(keys[0].Key, Subpage.Id); + Assert.AreEqual(keys[0].Value.First().Id, contentSchedule.FullSchedule.First().Id); + } + + [Test] + public void Can_Get_Content_Schedules_By_Content_Id() + { + // Arrange + var root = ContentService.GetById(Textpage.Id); + ContentService.Publish(root!, root!.AvailableCultures.ToArray()); + var content = ContentService.GetById(Subpage.Id); + var contentSchedule = ContentScheduleCollection.CreateWithEntry(DateTime.UtcNow.AddDays(1), null); + ContentService.PersistContentSchedule(content!, contentSchedule); + ContentService.Publish(content, content.AvailableCultures.ToArray()); + + // Act + var keys = ContentService.GetContentSchedulesByIds([Textpage.Id, Subpage.Id, Subpage2.Id]).ToList(); + + // Assert + Assert.AreEqual(1, keys.Count); + Assert.AreEqual(keys[0].Key, Subpage.Id); + Assert.AreEqual(keys[0].Value.First().Id, contentSchedule.FullSchedule.First().Id); } [Test] diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Cms.Api.Management/Services/Signs/HasScheduleSignProviderTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Cms.Api.Management/Services/Signs/HasScheduleSignProviderTests.cs index 2b0ab834b17b..7292b3a9ae18 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Cms.Api.Management/Services/Signs/HasScheduleSignProviderTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Cms.Api.Management/Services/Signs/HasScheduleSignProviderTests.cs @@ -6,6 +6,8 @@ using Umbraco.Cms.Api.Management.ViewModels.Document.Collection; using Umbraco.Cms.Api.Management.ViewModels.Document.Item; using Umbraco.Cms.Api.Management.ViewModels.Tree; +using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.Entities; using Umbraco.Cms.Core.Services; @@ -18,8 +20,9 @@ internal class HasScheduleSignProviderTests public void HasScheduleSignProvider_Can_Provide_Document_Tree_Signs() { var contentServiceMock = new Mock(); + var idKeyMapMock = new Mock(); - var sut = new HasScheduleSignProvider(contentServiceMock.Object); + var sut = new HasScheduleSignProvider(contentServiceMock.Object, idKeyMapMock.Object); Assert.IsTrue(sut.CanProvideSigns()); } @@ -27,8 +30,9 @@ public void HasScheduleSignProvider_Can_Provide_Document_Tree_Signs() public void HasScheduleSignProvider_Can_Provide_Document_Collection_Signs() { var contentServiceMock = new Mock(); + var idKeyMapMock = new Mock(); - var sut = new HasScheduleSignProvider(contentServiceMock.Object); + var sut = new HasScheduleSignProvider(contentServiceMock.Object, idKeyMapMock.Object); Assert.IsTrue(sut.CanProvideSigns()); } @@ -36,8 +40,9 @@ public void HasScheduleSignProvider_Can_Provide_Document_Collection_Signs() public void HasScheduleSignProvider_Can_Provide_Document_Item_Signs() { var contentServiceMock = new Mock(); + var idKeyMapMock = new Mock(); - var sut = new HasScheduleSignProvider(contentServiceMock.Object); + var sut = new HasScheduleSignProvider(contentServiceMock.Object, idKeyMapMock.Object); Assert.IsTrue(sut.CanProvideSigns()); } @@ -49,26 +54,37 @@ public async Task HasScheduleSignProvider_Should_Populate_Document_Tree_Signs() new() { Key = Guid.NewGuid(), Name = "Item 1" }, new() { Key = Guid.NewGuid(), Name = "Item 2" }, }; + var idKeyMapMock = new Mock(); + idKeyMapMock.Setup(x => x.GetIdForKey(entities[0].Key, UmbracoObjectTypes.Document)) + .Returns(Attempt.Succeed(1)); + idKeyMapMock.Setup(x => x.GetIdForKey(entities[1].Key, UmbracoObjectTypes.Document)) + .Returns(Attempt.Succeed(2)); + + Guid[] keys = entities.Select(x => x.Key).ToArray(); var contentServiceMock = new Mock(); contentServiceMock - .Setup(x => x.GetScheduledContentKeys(It.IsAny>())) - .Returns([entities[1].Key]); - var sut = new HasScheduleSignProvider(contentServiceMock.Object); + .Setup(x => x.GetContentSchedulesByIds(keys)) + .Returns(CreateContentSchedules()); + + + var sut = new HasScheduleSignProvider(contentServiceMock.Object, idKeyMapMock.Object); - var variant1 = new DocumentVariantItemResponseModel() { State = DocumentVariantState.Published, Name = "Test" }; - var variant2 = new DocumentVariantItemResponseModel() { State = DocumentVariantState.PublishedPendingChanges, Name = "Test" }; + var variant1 = new DocumentVariantItemResponseModel() { State = DocumentVariantState.Published, Name = "Test1", Culture = "en-EN" }; + var variant2 = new DocumentVariantItemResponseModel() { State = DocumentVariantState.Published, Name = "Test1", Culture = "da-DA" }; + var variant3 = new DocumentVariantItemResponseModel() { State = DocumentVariantState.PublishedPendingChanges, Name = "Test" }; var viewModels = new List { - new() { Id = entities[0].Key, Variants = [variant1] }, new() { Id = entities[1].Key, Variants = [variant2] }, + new() { Id = entities[0].Key, Variants = [variant1, variant2] }, new() { Id = entities[1].Key, Variants = [variant3] }, }; await sut.PopulateSignsAsync(viewModels); - Assert.AreEqual(viewModels[0].Variants.First().Signs.Count(), 0); + Assert.AreEqual(viewModels[0].Variants.FirstOrDefault(x => x.Culture == "da-DA").Signs.Count(), 0); + Assert.AreEqual(viewModels[0].Variants.FirstOrDefault(x => x.Culture == "en-EN").Signs.Count(), 1); Assert.AreEqual(viewModels[1].Variants.First().Signs.Count(), 1); - var signModel = viewModels[1].Variants.First().Signs.First(); + var signModel = viewModels[0].Variants.First().Signs.First(); Assert.AreEqual("Umb.ScheduledForPublish", signModel.Alias); } @@ -80,26 +96,36 @@ public async Task HasScheduleSignProvider_Should_Populate_Document_Collection_Si new() { Key = Guid.NewGuid(), Name = "Item 1" }, new() { Key = Guid.NewGuid(), Name = "Item 2" }, }; + var idKeyMapMock = new Mock(); + idKeyMapMock.Setup(x => x.GetIdForKey(entities[0].Key, UmbracoObjectTypes.Document)) + .Returns(Attempt.Succeed(1)); + idKeyMapMock.Setup(x => x.GetIdForKey(entities[1].Key, UmbracoObjectTypes.Document)) + .Returns(Attempt.Succeed(2)); + + Guid[] keys = entities.Select(x => x.Key).ToArray(); var contentServiceMock = new Mock(); contentServiceMock - .Setup(x => x.GetScheduledContentKeys(It.IsAny>())) - .Returns([entities[1].Key]); - var sut = new HasScheduleSignProvider(contentServiceMock.Object); + .Setup(x => x.GetContentSchedulesByIds(keys)) + .Returns(CreateContentSchedules()); + + var sut = new HasScheduleSignProvider(contentServiceMock.Object, idKeyMapMock.Object); - var variant1 = new DocumentVariantResponseModel() { State = DocumentVariantState.Published, Name = "Test" }; - var variant2 = new DocumentVariantResponseModel() { State = DocumentVariantState.PublishedPendingChanges, Name = "Test" }; + var variant1 = new DocumentVariantResponseModel() { State = DocumentVariantState.Published, Name = "Test1", Culture = "en-EN" }; + var variant2 = new DocumentVariantResponseModel() { State = DocumentVariantState.Published, Name = "Test1", Culture = "da-DA" }; + var variant3 = new DocumentVariantResponseModel() { State = DocumentVariantState.PublishedPendingChanges, Name = "Test" }; var viewModels = new List { - new() { Id = entities[0].Key, Variants = [variant1] }, new() { Id = entities[1].Key, Variants = [variant2] }, + new() { Id = entities[0].Key, Variants = [variant1, variant2] }, new() { Id = entities[1].Key, Variants = [variant3] }, }; await sut.PopulateSignsAsync(viewModels); - Assert.AreEqual(viewModels[0].Variants.First().Signs.Count(), 0); + Assert.AreEqual(viewModels[0].Variants.FirstOrDefault(x => x.Culture == "da-DA").Signs.Count(), 0); + Assert.AreEqual(viewModels[0].Variants.FirstOrDefault(x => x.Culture == "en-EN").Signs.Count(), 1); Assert.AreEqual(viewModels[1].Variants.First().Signs.Count(), 1); - var signModel = viewModels[1].Variants.First().Signs.First(); + var signModel = viewModels[0].Variants.First().Signs.First(); Assert.AreEqual("Umb.ScheduledForPublish", signModel.Alias); } @@ -111,26 +137,51 @@ public async Task HasScheduleSignProvider_Should_Populate_Document_Item_Signs() new() { Key = Guid.NewGuid(), Name = "Item 1" }, new() { Key = Guid.NewGuid(), Name = "Item 2" }, }; + var idKeyMapMock = new Mock(); + idKeyMapMock.Setup(x => x.GetIdForKey(entities[0].Key, UmbracoObjectTypes.Document)) + .Returns(Attempt.Succeed(1)); + idKeyMapMock.Setup(x => x.GetIdForKey(entities[1].Key, UmbracoObjectTypes.Document)) + .Returns(Attempt.Succeed(2)); + + Guid[] keys = entities.Select(x => x.Key).ToArray(); var contentServiceMock = new Mock(); contentServiceMock - .Setup(x => x.GetScheduledContentKeys(It.IsAny>())) - .Returns([entities[1].Key]); - var sut = new HasScheduleSignProvider(contentServiceMock.Object); + .Setup(x => x.GetContentSchedulesByIds(keys)) + .Returns(CreateContentSchedules()); - var variant1 = new DocumentVariantItemResponseModel() { State = DocumentVariantState.Published, Name = "Test" }; - var variant2 = new DocumentVariantItemResponseModel() { State = DocumentVariantState.PublishedPendingChanges, Name = "Test" }; + var sut = new HasScheduleSignProvider(contentServiceMock.Object, idKeyMapMock.Object); + + var variant1 = new DocumentVariantItemResponseModel() { State = DocumentVariantState.Published, Name = "Test1", Culture = "en-EN" }; + var variant2 = new DocumentVariantItemResponseModel() { State = DocumentVariantState.Published, Name = "Test1", Culture = "da-DA" }; + var variant3 = new DocumentVariantItemResponseModel() { State = DocumentVariantState.PublishedPendingChanges, Name = "Test" }; var viewModels = new List { - new() { Id = entities[0].Key, Variants = [variant1] }, new() { Id = entities[1].Key, Variants = [variant2] }, + new() { Id = entities[0].Key, Variants = [variant1, variant2] }, new() { Id = entities[1].Key, Variants = [variant3] }, }; await sut.PopulateSignsAsync(viewModels); - Assert.AreEqual(viewModels[0].Variants.First().Signs.Count(), 0); + Assert.AreEqual(viewModels[0].Variants.FirstOrDefault(x => x.Culture == "da-DA").Signs.Count(), 0); + Assert.AreEqual(viewModels[0].Variants.FirstOrDefault(x => x.Culture == "en-EN").Signs.Count(), 1); Assert.AreEqual(viewModels[1].Variants.First().Signs.Count(), 1); - var signModel = viewModels[1].Variants.First().Signs.First(); + var signModel = viewModels[0].Variants.First().Signs.First(); Assert.AreEqual("Umb.ScheduledForPublish", signModel.Alias); } + + private Dictionary> CreateContentSchedules() + { + Dictionary> contentSchedules = new Dictionary>(); + + contentSchedules.Add(1, [ + new ContentSchedule("en-EN", DateTime.Now.AddDays(1), ContentScheduleAction.Release), // Scheduled for release + new ContentSchedule("da-DA", DateTime.Now.AddDays(-1), ContentScheduleAction.Release) // Not Scheduled for release + ]); + contentSchedules.Add(2, [ + new ContentSchedule("*", DateTime.Now.AddDays(1), ContentScheduleAction.Release) // Scheduled for release + ]); + + return contentSchedules; + } } From a9fdc0bc65d9b1a9733d8091aff41df0212403a2 Mon Sep 17 00:00:00 2001 From: NillasKA Date: Thu, 4 Sep 2025 13:44:07 +0200 Subject: [PATCH 13/13] Reverted breaking changes --- .../Repositories/IDocumentRepository.cs | 3 ++- src/Umbraco.Core/Services/ContentService.cs | 20 +++++++------------ src/Umbraco.Core/Services/IContentService.cs | 12 ++--------- .../Services/ContentServiceTests.cs | 20 ------------------- 4 files changed, 11 insertions(+), 44 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/IDocumentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/IDocumentRepository.cs index 2ea536e902c4..6ac6470a8575 100644 --- a/src/Umbraco.Core/Persistence/Repositories/IDocumentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/IDocumentRepository.cs @@ -1,3 +1,4 @@ +using System.Collections.Immutable; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.Membership; @@ -57,7 +58,7 @@ public interface IDocumentRepository : IContentRepository, IReadR /// /// The provided collection of content keys filtered for those that are scheduled for publishing. /// - IDictionary> GetContentSchedulesByIds(int[] documentIds); + IDictionary> GetContentSchedulesByIds(int[] documentIds) => ImmutableDictionary>.Empty; /// /// Get the count of published items diff --git a/src/Umbraco.Core/Services/ContentService.cs b/src/Umbraco.Core/Services/ContentService.cs index a53d50db8388..5d4ed2f98ca8 100644 --- a/src/Umbraco.Core/Services/ContentService.cs +++ b/src/Umbraco.Core/Services/ContentService.cs @@ -1010,23 +1010,13 @@ public IEnumerable GetPagedContentInRecycleBin(long pageIndex, int pag /// - public IDictionary> GetContentSchedulesByIds(int[] documentIds) + public IDictionary> GetContentSchedulesByIds(Guid[] keys) { - if (documentIds.Length == 0) + if (keys.Length == 0) { return ImmutableDictionary>.Empty; } - using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true)) - { - scope.ReadLock(Constants.Locks.ContentTree); - scope.ReadLock(Constants.Locks.Languages); - return _documentRepository.GetContentSchedulesByIds(documentIds); - } - } - - public IDictionary> GetContentSchedulesByIds(Guid[] keys) - { List contentIds = []; foreach (var key in keys) { @@ -1039,7 +1029,11 @@ public IDictionary> GetContentSchedulesByIds(G contentIds.Add(contentId.Result); } - return GetContentSchedulesByIds(contentIds.ToArray()); + using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true)) + { + scope.ReadLock(Constants.Locks.ContentTree); + return _documentRepository.GetContentSchedulesByIds(contentIds.ToArray()); + } } /// diff --git a/src/Umbraco.Core/Services/IContentService.cs b/src/Umbraco.Core/Services/IContentService.cs index 7ee50f1a6f2d..280cf1847a5a 100644 --- a/src/Umbraco.Core/Services/IContentService.cs +++ b/src/Umbraco.Core/Services/IContentService.cs @@ -1,3 +1,4 @@ +using System.Collections.Immutable; using Microsoft.Extensions.DependencyInjection; using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Models; @@ -275,15 +276,6 @@ IContent CreateBlueprintFromContent(IContent blueprint, string name, int userId /// bool HasChildren(int id); - /// - /// Gets a dictionary of content Ids and their matching content schedules. - /// - /// The IDs of the documents. - /// - /// A dictionary with a nodeId and an IEnumerable of matching ContentSchedules. - /// - IDictionary> GetContentSchedulesByIds(int[] documentIds); - /// /// Gets a dictionary of content Ids and their matching content schedules. /// @@ -291,7 +283,7 @@ IContent CreateBlueprintFromContent(IContent blueprint, string name, int userId /// /// A dictionary with a nodeId and an IEnumerable of matching ContentSchedules. /// - IDictionary> GetContentSchedulesByIds(Guid[] keys); + IDictionary> GetContentSchedulesByIds(Guid[] keys) => ImmutableDictionary>.Empty; #endregion diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/ContentServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/ContentServiceTests.cs index 8844eba3e871..e19221638a90 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/ContentServiceTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/ContentServiceTests.cs @@ -707,26 +707,6 @@ public void Can_Get_Content_Schedules_By_Keys() Assert.AreEqual(keys[0].Value.First().Id, contentSchedule.FullSchedule.First().Id); } - [Test] - public void Can_Get_Content_Schedules_By_Content_Id() - { - // Arrange - var root = ContentService.GetById(Textpage.Id); - ContentService.Publish(root!, root!.AvailableCultures.ToArray()); - var content = ContentService.GetById(Subpage.Id); - var contentSchedule = ContentScheduleCollection.CreateWithEntry(DateTime.UtcNow.AddDays(1), null); - ContentService.PersistContentSchedule(content!, contentSchedule); - ContentService.Publish(content, content.AvailableCultures.ToArray()); - - // Act - var keys = ContentService.GetContentSchedulesByIds([Textpage.Id, Subpage.Id, Subpage2.Id]).ToList(); - - // Assert - Assert.AreEqual(1, keys.Count); - Assert.AreEqual(keys[0].Key, Subpage.Id); - Assert.AreEqual(keys[0].Value.First().Id, contentSchedule.FullSchedule.First().Id); - } - [Test] public void Can_Get_Content_For_Release() {