From 389380d8fa89c4ea515de20233479a2f525b9417 Mon Sep 17 00:00:00 2001 From: Mole Date: Tue, 21 Jun 2022 11:44:28 +0200 Subject: [PATCH] V10: Fix sending content notification (#12597) * Add mappers to map between ContentItemDisplay and ContentItemDisplayWithSchedule * Ensure SendingContentNotification is always sent * Add custom setup hook for UmbracoTestServerTestBase * Add test showing bug/fix * Test schedule being mapped correctly * Obsolete the old constructor * Removed TODO --- .../OutgoingEditorModelEventAttribute.cs | 48 +++++- .../Mapping/ContentMapDefinition.cs | 123 +++++++++++++ .../UmbracoTestServerTestBase.cs | 15 +- .../OutgoingEditorModelEventFilterTests.cs | 163 ++++++++++++++++++ 4 files changed, 341 insertions(+), 8 deletions(-) create mode 100644 tests/Umbraco.Tests.Integration/Umbraco.Web.BackOffice/Filters/OutgoingEditorModelEventFilterTests.cs diff --git a/src/Umbraco.Web.BackOffice/Filters/OutgoingEditorModelEventAttribute.cs b/src/Umbraco.Web.BackOffice/Filters/OutgoingEditorModelEventAttribute.cs index 64ac33b1aa..171e8a1bf9 100644 --- a/src/Umbraco.Web.BackOffice/Filters/OutgoingEditorModelEventAttribute.cs +++ b/src/Umbraco.Web.BackOffice/Filters/OutgoingEditorModelEventAttribute.cs @@ -3,12 +3,15 @@ using System.Collections; using System.Collections.Generic; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.Extensions.DependencyInjection; using Umbraco.Cms.Core.Dashboards; using Umbraco.Cms.Core.Events; +using Umbraco.Cms.Core.Mapping; using Umbraco.Cms.Core.Models.ContentEditing; using Umbraco.Cms.Core.Notifications; using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Web; +using Umbraco.Cms.Web.Common.DependencyInjection; using Umbraco.Extensions; namespace Umbraco.Cms.Web.BackOffice.Filters @@ -29,20 +32,34 @@ namespace Umbraco.Cms.Web.BackOffice.Filters private readonly IUmbracoContextAccessor _umbracoContextAccessor; private readonly IBackOfficeSecurityAccessor _backOfficeSecurityAccessor; - + private readonly IUmbracoMapper _mapper; private readonly IEventAggregator _eventAggregator; - public OutgoingEditorModelEventFilter( + [ActivatorUtilitiesConstructor]public OutgoingEditorModelEventFilter( IUmbracoContextAccessor umbracoContextAccessor, - IBackOfficeSecurityAccessor backOfficeSecurityAccessor, IEventAggregator eventAggregator) - { + IBackOfficeSecurityAccessor backOfficeSecurityAccessor, IEventAggregator eventAggregator, + IUmbracoMapper mapper){ _umbracoContextAccessor = umbracoContextAccessor ?? throw new ArgumentNullException(nameof(umbracoContextAccessor)); _backOfficeSecurityAccessor = backOfficeSecurityAccessor ?? throw new ArgumentNullException(nameof(backOfficeSecurityAccessor)); _eventAggregator = eventAggregator ?? throw new ArgumentNullException(nameof(eventAggregator)); - } + _mapper = mapper; + } + + [Obsolete("Please use constructor that takes an IUmbracoMapper, scheduled for removal in V12")] + public OutgoingEditorModelEventFilter( + IUmbracoContextAccessor umbracoContextAccessor, + IBackOfficeSecurityAccessor backOfficeSecurityAccessor, + IEventAggregator eventAggregator) + : this( + umbracoContextAccessor, + backOfficeSecurityAccessor, + eventAggregator, + StaticServiceProvider.Instance.GetRequiredService()) + { + } public void OnActionExecuted(ActionExecutedContext context) { @@ -72,7 +89,26 @@ namespace Umbraco.Cms.Web.BackOffice.Filters case ContentItemDisplay content: _eventAggregator.Publish(new SendingContentNotification(content, umbracoContext)); break; - case MediaItemDisplay media: + case ContentItemDisplayWithSchedule contentWithSchedule: + // This is a bit weird, since ContentItemDisplayWithSchedule was introduced later, + // the SendingContentNotification only accepts ContentItemDisplay, + // which means we have to map it to this before sending the notification. + ContentItemDisplay? display = _mapper.Map(contentWithSchedule); + if (display is null) + { + // This will never happen. + break; + } + + // Now that the display is mapped to the non-schedule one we can publish the notification. + _eventAggregator.Publish(new SendingContentNotification(display, umbracoContext)); + + // We want the changes the handler makes to take effect. + // So we have to map these changes back to the existing ContentItemWithSchedule. + // To avoid losing the schedule information we add the old variants to context. + _mapper.Map(display, contentWithSchedule, mapperContext => mapperContext.Items[nameof(contentWithSchedule.Variants)] = contentWithSchedule.Variants); + break; + case MediaItemDisplay media: _eventAggregator.Publish(new SendingMediaNotification(media, umbracoContext)); break; case MemberDisplay member: diff --git a/src/Umbraco.Web.BackOffice/Mapping/ContentMapDefinition.cs b/src/Umbraco.Web.BackOffice/Mapping/ContentMapDefinition.cs index 24cd1c5cbe..9921b59224 100644 --- a/src/Umbraco.Web.BackOffice/Mapping/ContentMapDefinition.cs +++ b/src/Umbraco.Web.BackOffice/Mapping/ContentMapDefinition.cs @@ -107,7 +107,130 @@ namespace Umbraco.Cms.Web.BackOffice.Mapping mapper.Define((source, context) => new ContentVariantDisplay(), Map); mapper.Define((source, context) => new ContentVariantScheduleDisplay(), Map); + mapper.Define((source, context) => new ContentItemDisplay(), Map); + mapper.Define((source, context) => new ContentItemDisplayWithSchedule(), Map); + + mapper.Define((source, context) => new ContentVariantScheduleDisplay(), Map); + mapper.Define((source, context) => new ContentVariantDisplay(), Map); + } + + // Umbraco.Code.MapAll + private void Map(ContentVariantScheduleDisplay source, ContentVariantDisplay target, MapperContext context) + { + target.CreateDate = source.CreateDate; + target.DisplayName = source.DisplayName; + target.Language = source.Language; + target.Name = source.Name; + target.PublishDate = source.PublishDate; + target.Segment = source.Segment; + target.State = source.State; + target.Tabs = source.Tabs; + target.UpdateDate = source.UpdateDate; + } + + // Umbraco.Code.MapAll + private void Map(ContentItemDisplay source, ContentItemDisplayWithSchedule target, MapperContext context) + { + target.AllowedActions = source.AllowedActions; + target.AllowedTemplates = source.AllowedTemplates; + target.AllowPreview = source.AllowPreview; + target.ContentApps = source.ContentApps; + target.ContentDto = source.ContentDto; + target.ContentTypeAlias = source.ContentTypeAlias; + target.ContentTypeId = source.ContentTypeId; + target.ContentTypeKey = source.ContentTypeKey; + target.ContentTypeName = source.ContentTypeName; + target.DocumentType = source.DocumentType; + target.Errors = source.Errors; + target.Icon = source.Icon; + target.Id = source.Id; + target.IsBlueprint = source.IsBlueprint; + target.IsChildOfListView = source.IsChildOfListView; + target.IsContainer = source.IsContainer; + target.IsElement = source.IsElement; + target.Key = source.Key; + target.Owner = source.Owner; + target.ParentId = source.ParentId; + target.Path = source.Path; + target.PersistedContent = source.PersistedContent; + target.SortOrder = source.SortOrder; + target.TemplateAlias = source.TemplateAlias; + target.TemplateId = source.TemplateId; + target.Trashed = source.Trashed; + target.TreeNodeUrl = source.TreeNodeUrl; + target.Udi = source.Udi; + target.UpdateDate = source.UpdateDate; + target.Updater = source.Updater; + target.Urls = source.Urls; + target.Variants = context.MapEnumerable(source.Variants); + } + + // Umbraco.Code.MapAll + private void Map(ContentVariantDisplay source, ContentVariantScheduleDisplay target, MapperContext context) + { + target.CreateDate = source.CreateDate; + target.DisplayName = source.DisplayName; + target.Language = source.Language; + target.Name = source.Name; + target.PublishDate = source.PublishDate; + target.Segment = source.Segment; + target.State = source.State; + target.Tabs = source.Tabs; + target.UpdateDate = source.UpdateDate; + + // We'll only try and map the ReleaseDate/ExpireDate if the "old" ContentVariantScheduleDisplay is in the context, otherwise we'll just skip it quietly. + _ = context.Items.TryGetValue(nameof(ContentItemDisplayWithSchedule.Variants), out var variants); + if (variants is IEnumerable scheduleDisplays) + { + ContentVariantScheduleDisplay? item = scheduleDisplays.FirstOrDefault(x => x.Language?.Id == source.Language?.Id && x.Segment == source.Segment); + + if (item is null) + { + // If we can't find the old variants display, we'll just not try and map it. + return; + } + + target.ReleaseDate = item.ReleaseDate; + target.ExpireDate = item.ExpireDate; } + } + + // Umbraco.Code.MapAll + private static void Map(ContentItemDisplayWithSchedule source, ContentItemDisplay target, MapperContext context) + { + target.AllowedActions = source.AllowedActions; + target.AllowedTemplates = source.AllowedTemplates; + target.AllowPreview = source.AllowPreview; + target.ContentApps = source.ContentApps; + target.ContentDto = source.ContentDto; + target.ContentTypeAlias = source.ContentTypeAlias; + target.ContentTypeId = source.ContentTypeId; + target.ContentTypeKey = source.ContentTypeKey; + target.ContentTypeName = source.ContentTypeName; + target.DocumentType = source.DocumentType; + target.Errors = source.Errors; + target.Icon = source.Icon; + target.Id = source.Id; + target.IsBlueprint = source.IsBlueprint; + target.IsChildOfListView = source.IsChildOfListView; + target.IsContainer = source.IsContainer; + target.IsElement = source.IsElement; + target.Key = source.Key; + target.Owner = source.Owner; + target.ParentId = source.ParentId; + target.Path = source.Path; + target.PersistedContent = source.PersistedContent; + target.SortOrder = source.SortOrder; + target.TemplateAlias = source.TemplateAlias; + target.TemplateId = source.TemplateId; + target.Trashed = source.Trashed; + target.TreeNodeUrl = source.TreeNodeUrl; + target.Udi = source.Udi; + target.UpdateDate = source.UpdateDate; + target.Updater = source.Updater; + target.Urls = source.Urls; + target.Variants = source.Variants; + } // Umbraco.Code.MapAll private static void Map(IContent source, ContentPropertyCollectionDto target, MapperContext context) diff --git a/tests/Umbraco.Tests.Integration/TestServerTest/UmbracoTestServerTestBase.cs b/tests/Umbraco.Tests.Integration/TestServerTest/UmbracoTestServerTestBase.cs index f1f8b124e8..1ec90424f3 100644 --- a/tests/Umbraco.Tests.Integration/TestServerTest/UmbracoTestServerTestBase.cs +++ b/tests/Umbraco.Tests.Integration/TestServerTest/UmbracoTestServerTestBase.cs @@ -42,6 +42,16 @@ namespace Umbraco.Cms.Tests.Integration.TestServerTest protected WebApplicationFactory Factory { get; private set; } + /// + /// Hook for altering UmbracoBuilder setup + /// + /// + /// Can also be used for registering test doubles. + /// + protected virtual void CustomTestSetup(IUmbracoBuilder builder) + { + } + [SetUp] public void Setup() { @@ -233,8 +243,9 @@ namespace Umbraco.Cms.Tests.Integration.TestServerTest .AddWebsite() .AddUmbracoSqlServerSupport() .AddUmbracoSqliteSupport() - .AddTestServices(TestHelper) // This is the important one! - .Build(); + .AddTestServices(TestHelper); // This is the important one! + CustomTestSetup(builder); + builder.Build(); } /// diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Web.BackOffice/Filters/OutgoingEditorModelEventFilterTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Web.BackOffice/Filters/OutgoingEditorModelEventFilterTests.cs new file mode 100644 index 0000000000..e627a3300f --- /dev/null +++ b/tests/Umbraco.Tests.Integration/Umbraco.Web.BackOffice/Filters/OutgoingEditorModelEventFilterTests.cs @@ -0,0 +1,163 @@ +using System; +using System.Linq; +using System.Net; +using System.Net.Http; +using System.Threading.Tasks; +using NUnit.Framework; +using Umbraco.Cms.Core.DependencyInjection; +using Umbraco.Cms.Core.Events; +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.Models.ContentEditing; +using Umbraco.Cms.Core.Notifications; +using Umbraco.Cms.Core.Serialization; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Tests.Common.Builders; +using Umbraco.Cms.Tests.Common.Builders.Extensions; +using Umbraco.Cms.Tests.Integration.TestServerTest; +using Umbraco.Cms.Web.BackOffice.Controllers; +using Umbraco.Cms.Web.Common.Formatters; +using Umbraco.Extensions; + +namespace Umbraco.Cms.Tests.Integration.Umbraco.Web.BackOffice.Filters; + +[TestFixture] +public class OutgoingEditorModelEventFilterTests : UmbracoTestServerTestBase +{ + private static int _messageCount; + private static Action _handler; + + protected override void CustomTestSetup(IUmbracoBuilder builder) + { + builder.AddNotificationHandler(); + } + + [TearDown] + public void Reset() => ResetNotifications(); + + [Test] + public async Task Content_Item_With_Schedule_Raises_SendingContentNotification() + { + IContentTypeService contentTypeService = GetRequiredService(); + IContentService contentService = GetRequiredService(); + IJsonSerializer serializer = GetRequiredService(); + + var contentType = new ContentTypeBuilder().Build(); + contentTypeService.Save(contentType); + + var contentToRequest = new ContentBuilder() + .WithoutIdentity() + .WithContentType(contentType) + .Build(); + + contentService.Save(contentToRequest); + + _handler = notification => notification.Content.AllowPreview = false; + + var url = PrepareApiControllerUrl(x => x.GetById(contentToRequest.Id)); + + HttpResponseMessage response = await Client.GetAsync(url); + + Assert.AreEqual(HttpStatusCode.OK, response.StatusCode); + + var text = await response.Content.ReadAsStringAsync(); + text = text.TrimStart(AngularJsonMediaTypeFormatter.XsrfPrefix); + var display = serializer.Deserialize(text); + + Assert.AreEqual(1, _messageCount); + Assert.IsNotNull(display); + Assert.IsFalse(display.AllowPreview); + } + + [Test] + public async Task Publish_Schedule_Is_Mapped_Correctly() + { + const string UsIso = "en-US"; + const string DkIso = "da-DK"; + const string SweIso = "sv-SE"; + var contentTypeService = GetRequiredService(); + var contentService = GetRequiredService(); + var localizationService = GetRequiredService(); + IJsonSerializer serializer = GetRequiredService(); + + var contentType = new ContentTypeBuilder() + .WithContentVariation(ContentVariation.Culture) + .Build(); + contentTypeService.Save(contentType); + + var dkLang = new LanguageBuilder() + .WithCultureInfo(DkIso) + .WithIsDefault(false) + .Build(); + + var sweLang = new LanguageBuilder() + .WithCultureInfo(SweIso) + .WithIsDefault(false) + .Build(); + + localizationService.Save(dkLang); + localizationService.Save(sweLang); + + var content = new ContentBuilder() + .WithoutIdentity() + .WithContentType(contentType) + .WithCultureName(UsIso, "Same Name") + .WithCultureName(SweIso, "Same Name") + .WithCultureName(DkIso, "Same Name") + .Build(); + + contentService.Save(content); + var schedule = new ContentScheduleCollection(); + + var dkReleaseDate = new DateTime(2022, 06, 22, 21, 30, 42); + var dkExpireDate = new DateTime(2022, 07, 15, 18, 00, 00); + + var sweReleaseDate = new DateTime(2022, 06, 23, 22, 30, 42); + var sweExpireDate = new DateTime(2022, 07, 10, 14, 20, 00); + schedule.Add(DkIso, dkReleaseDate, dkExpireDate); + schedule.Add(SweIso, sweReleaseDate, sweExpireDate); + contentService.PersistContentSchedule(content, schedule); + + var url = PrepareApiControllerUrl(x => x.GetById(content.Id)); + + HttpResponseMessage response = await Client.GetAsync(url); + + Assert.AreEqual(HttpStatusCode.OK, response.StatusCode); + + var text = await response.Content.ReadAsStringAsync(); + text = text.TrimStart(AngularJsonMediaTypeFormatter.XsrfPrefix); + var display = serializer.Deserialize(text); + + Assert.IsNotNull(display); + Assert.AreEqual(1, _messageCount); + + var dkVariant = display.Variants.FirstOrDefault(x => x.Language?.IsoCode == DkIso); + Assert.IsNotNull(dkVariant); + Assert.AreEqual(dkReleaseDate, dkVariant.ReleaseDate); + Assert.AreEqual(dkExpireDate, dkVariant.ExpireDate); + + var sweVariant = display.Variants.FirstOrDefault(x => x.Language?.IsoCode == SweIso); + Assert.IsNotNull(sweVariant); + Assert.AreEqual(sweReleaseDate, sweVariant.ReleaseDate); + Assert.AreEqual(sweExpireDate, sweVariant.ExpireDate); + + var usVariant = display.Variants.FirstOrDefault(x => x.Language?.IsoCode == UsIso); + Assert.IsNotNull(usVariant); + Assert.IsNull(usVariant.ReleaseDate); + Assert.IsNull(usVariant.ExpireDate); + } + + private void ResetNotifications() + { + _messageCount = 0; + _handler = null; + } + + private class FilterEventHandler : INotificationHandler + { + public void Handle(SendingContentNotification notification) + { + _messageCount += 1; + _handler?.Invoke(notification); + } + } +}