From 22680a741cae9af4395e7e6c1634a8ffb615c11e Mon Sep 17 00:00:00 2001 From: Kenn Jacobsen Date: Tue, 2 Mar 2021 19:11:39 +0100 Subject: [PATCH] Make integration tests run again + revert to weird logic in test --- .../Compose/RelateOnCopyHandler.cs | 51 +++++++++++++++++++ .../Services/Implement/ContentService.cs | 33 ++---------- .../ContentNotificationExtensions.cs | 30 +++++++++-- .../Notifications/CopiedNotification.cs | 4 +- .../ContentServiceNotificationTests.cs | 44 +++++++++------- 5 files changed, 108 insertions(+), 54 deletions(-) create mode 100644 src/Umbraco.Infrastructure/Compose/RelateOnCopyHandler.cs diff --git a/src/Umbraco.Infrastructure/Compose/RelateOnCopyHandler.cs b/src/Umbraco.Infrastructure/Compose/RelateOnCopyHandler.cs new file mode 100644 index 0000000000..a14b2e70b3 --- /dev/null +++ b/src/Umbraco.Infrastructure/Compose/RelateOnCopyHandler.cs @@ -0,0 +1,51 @@ +using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Events; +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Infrastructure.Services.Notifications; + +namespace Umbraco.Cms.Infrastructure.Compose +{ + // TODO: insert these notification handlers in core composition + public class RelateOnCopyHandler : INotificationHandler> + { + private readonly IRelationService _relationService; + private readonly IAuditService _auditService; + + public RelateOnCopyHandler(IRelationService relationService, IAuditService auditService) + { + _relationService = relationService; + _auditService = auditService; + } + + public void Handle(CopiedNotification notification) + { + if (notification.RelateToOriginal == false) + { + return; + } + + var relationType = _relationService.GetRelationTypeByAlias(Constants.Conventions.RelationTypes.RelateDocumentOnCopyAlias); + + if (relationType == null) + { + relationType = new RelationType(Constants.Conventions.RelationTypes.RelateDocumentOnCopyAlias, + Constants.Conventions.RelationTypes.RelateDocumentOnCopyName, + true, + Constants.ObjectTypes.Document, + Constants.ObjectTypes.Document); + + _relationService.Save(relationType); + } + + var relation = new Relation(notification.Original.Id, notification.Copy.Id, relationType); + _relationService.Save(relation); + + _auditService.Add( + AuditType.Copy, + notification.Copy.WriterId, + notification.Copy.Id, ObjectTypes.GetName(UmbracoObjectTypes.Document), + $"Copied content with Id: '{notification.Copy.Id}' related to original content with Id: '{notification.Original.Id}'"); + } + } +} diff --git a/src/Umbraco.Infrastructure/Services/Implement/ContentService.cs b/src/Umbraco.Infrastructure/Services/Implement/ContentService.cs index 8928b4972d..da24ac4427 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/ContentService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/ContentService.cs @@ -34,7 +34,6 @@ namespace Umbraco.Cms.Core.Services.Implement private readonly ILogger _logger; private IQuery _queryNotTrashed; private readonly IEventAggregator _eventAggregator; - private readonly IRelationService _relationService; #region Constructors @@ -42,7 +41,7 @@ namespace Umbraco.Cms.Core.Services.Implement IEventMessagesFactory eventMessagesFactory, IDocumentRepository documentRepository, IEntityRepository entityRepository, IAuditRepository auditRepository, IContentTypeRepository contentTypeRepository, IDocumentBlueprintRepository documentBlueprintRepository, ILanguageRepository languageRepository, - Lazy propertyValidationService, IShortStringHelper shortStringHelper, IEventAggregator eventAggregator, IRelationService relationService) + Lazy propertyValidationService, IShortStringHelper shortStringHelper, IEventAggregator eventAggregator) : base(provider, loggerFactory, eventMessagesFactory) { _documentRepository = documentRepository; @@ -54,7 +53,6 @@ namespace Umbraco.Cms.Core.Services.Implement _propertyValidationService = propertyValidationService; _shortStringHelper = shortStringHelper; _eventAggregator = eventAggregator; - _relationService = relationService; _logger = loggerFactory.CreateLogger(); } @@ -1300,7 +1298,7 @@ namespace Umbraco.Cms.Core.Services.Implement if (!branchOne) // for branches, handled by SaveAndPublishBranch { scope.Events.Dispatch(TreeChanged, this, new TreeChange(content, changeType).ToEventArgs()); - _eventAggregator.Publish(new PublishingNotification(content, evtMsgs)); + _eventAggregator.Publish(new PublishedNotification(content, evtMsgs)); } // it was not published and now is... descendants that were 'published' (but @@ -2252,12 +2250,7 @@ namespace Umbraco.Cms.Core.Services.Implement scope.Events.Dispatch(TreeChanged, this, new TreeChange(copy, TreeChangeTypes.RefreshBranch).ToEventArgs()); foreach (var x in copies) { - if (relateToOriginal) - { - RelateOnCopy(x.Item1, x.Item2); - } - - _eventAggregator.Publish(new CopiedNotification(x.Item1, x.Item2, parentId, evtMsgs)); + _eventAggregator.Publish(new CopiedNotification(x.Item1, x.Item2, parentId, relateToOriginal, evtMsgs)); } Audit(AuditType.Copy, userId, content.Id); @@ -2267,26 +2260,6 @@ namespace Umbraco.Cms.Core.Services.Implement return copy; } - private void RelateOnCopy(IContent original, IContent copy) - { - var relationType = _relationService.GetRelationTypeByAlias(Constants.Conventions.RelationTypes.RelateDocumentOnCopyAlias); - if (relationType == null) - { - relationType = new RelationType(Constants.Conventions.RelationTypes.RelateDocumentOnCopyAlias, - Constants.Conventions.RelationTypes.RelateDocumentOnCopyName, - true, - Constants.ObjectTypes.Document, - Constants.ObjectTypes.Document); - - _relationService.Save(relationType); - } - - var relation = new Relation(original.Id, copy.Id, relationType); - _relationService.Save(relation); - - Audit(AuditType.Copy, copy.WriterId, copy.Id, $"Copied content with Id: '{copy.Id}' related to original content with Id: '{original.Id}'"); - } - /// /// Sends an to Publication, which executes handlers and events for the 'Send to Publication' action. /// diff --git a/src/Umbraco.Infrastructure/Services/Notifications/ContentNotificationExtensions.cs b/src/Umbraco.Infrastructure/Services/Notifications/ContentNotificationExtensions.cs index 1a05c2f79d..046aa5b07b 100644 --- a/src/Umbraco.Infrastructure/Services/Notifications/ContentNotificationExtensions.cs +++ b/src/Umbraco.Infrastructure/Services/Notifications/ContentNotificationExtensions.cs @@ -23,13 +23,19 @@ namespace Umbraco.Cms.Infrastructure.Services.Notifications /// Determines whether a culture is being published, during a Publishing notification /// public static bool IsPublishingCulture(this PublishingNotification notification, IContent content, string culture) - => content.PublishCultureInfos.TryGetValue(culture, out ContentCultureInfos cultureInfo) && cultureInfo.IsDirty(); + => IsPublishingCulture(content, culture); /// - /// Determines whether a culture is being unpublished, during a Publishing notification + /// Determines whether a culture is being unpublished, during an Publishing notification + /// + public static bool IsUnpublishingCulture(this PublishingNotification notification, IContent content, string culture) + => IsUnpublishingCulture(content, culture); + + /// + /// Determines whether a culture is being unpublished, during a Unpublishing notification /// public static bool IsUnpublishingCulture(this UnpublishingNotification notification, IContent content, string culture) - => content.IsPropertyDirty(ContentBase.ChangeTrackingPrefix.UnpublishedCulture + culture); //bit of a hack since we know that the content implementation tracks changes this way + => IsUnpublishingCulture(content, culture); /// /// Determines whether a culture has been published, during a Published notification @@ -40,8 +46,22 @@ namespace Umbraco.Cms.Infrastructure.Services.Notifications /// /// Determines whether a culture has been unpublished, during a Published notification /// - public static bool HasUnpublishedCulture(this UnpublishedNotification notification, IContent content, string culture) - => content.WasPropertyDirty(ContentBase.ChangeTrackingPrefix.UnpublishedCulture + culture); + public static bool HasUnpublishedCulture(this PublishedNotification notification, IContent content, string culture) + => HasUnpublishedCulture(content, culture); + /// + /// Determines whether a culture has been unpublished, during an Unpublished notification + /// + public static bool HasUnpublishedCulture(this UnpublishedNotification notification, IContent content, string culture) + => HasUnpublishedCulture(content, culture); + + private static bool IsUnpublishingCulture(IContent content, string culture) + => content.IsPropertyDirty(ContentBase.ChangeTrackingPrefix.UnpublishedCulture + culture); + + public static bool IsPublishingCulture(IContent content, string culture) + => content.PublishCultureInfos.TryGetValue(culture, out ContentCultureInfos cultureInfo) && cultureInfo.IsDirty(); + + public static bool HasUnpublishedCulture(IContent content, string culture) + => content.WasPropertyDirty(ContentBase.ChangeTrackingPrefix.UnpublishedCulture + culture); } } diff --git a/src/Umbraco.Infrastructure/Services/Notifications/CopiedNotification.cs b/src/Umbraco.Infrastructure/Services/Notifications/CopiedNotification.cs index f89487cbc0..f70780e1fa 100644 --- a/src/Umbraco.Infrastructure/Services/Notifications/CopiedNotification.cs +++ b/src/Umbraco.Infrastructure/Services/Notifications/CopiedNotification.cs @@ -7,10 +7,11 @@ namespace Umbraco.Cms.Infrastructure.Services.Notifications { public class CopiedNotification : ObjectNotification where T : class { - public CopiedNotification(T original, T copy, int parentId, EventMessages messages) : base(original, messages) + public CopiedNotification(T original, T copy, int parentId, bool relateToOriginal, EventMessages messages) : base(original, messages) { Copy = copy; ParentId = parentId; + RelateToOriginal = relateToOriginal; } public T Original => Target; @@ -18,5 +19,6 @@ namespace Umbraco.Cms.Infrastructure.Services.Notifications public T Copy { get; } public int ParentId { get; } + public bool RelateToOriginal { get; } } } diff --git a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentServiceNotificationTests.cs b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentServiceNotificationTests.cs index 8a093a0427..e872640ebe 100644 --- a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentServiceNotificationTests.cs +++ b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentServiceNotificationTests.cs @@ -367,43 +367,51 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services document.UnpublishCulture("fr-FR"); - var unpublishingWasCalled = false; - var unpublishedWasCalled = false; + var publishingWasCalled = false; + var publishedWasCalled = false; - ContentNotificationHandler.UnpublishingContent += notification => + // TODO: revisit this - it was migrated when removing static events, but the expected result seems illogic - why does this test bind to Published and not Unpublished? + + ContentNotificationHandler.PublishingContent += notification => { - IContent unpublished = notification.UnpublishedEntities.First(); + IContent published = notification.PublishedEntities.First(); - Assert.AreSame(document, unpublished); + Assert.AreSame(document, published); - Assert.IsFalse(notification.IsUnpublishingCulture(unpublished, "en-US")); - Assert.IsTrue(notification.IsUnpublishingCulture(unpublished, "fr-FR")); + Assert.IsFalse(notification.IsPublishingCulture(published, "en-US")); + Assert.IsFalse(notification.IsPublishingCulture(published, "fr-FR")); - unpublishingWasCalled = true; + Assert.IsFalse(notification.IsUnpublishingCulture(published, "en-US")); + Assert.IsTrue(notification.IsUnpublishingCulture(published, "fr-FR")); + + publishingWasCalled = true; }; - ContentNotificationHandler.UnpublishedContent += notification => + ContentNotificationHandler.PublishedContent += notification => { - IContent unpublished = notification.UnpublishedEntities.First(); + IContent published = notification.PublishedEntities.First(); - Assert.AreSame(document, unpublished); + Assert.AreSame(document, published); - Assert.IsFalse(notification.HasUnpublishedCulture(unpublished, "en-US")); - Assert.IsTrue(notification.HasUnpublishedCulture(unpublished, "fr-FR")); + Assert.IsFalse(notification.HasPublishedCulture(published, "en-US")); + Assert.IsFalse(notification.HasPublishedCulture(published, "fr-FR")); - unpublishedWasCalled = true; + Assert.IsFalse(notification.HasUnpublishedCulture(published, "en-US")); + Assert.IsTrue(notification.HasUnpublishedCulture(published, "fr-FR")); + + publishedWasCalled = true; }; try { contentService.CommitDocumentChanges(document); - Assert.IsTrue(unpublishingWasCalled); - Assert.IsTrue(unpublishedWasCalled); + Assert.IsTrue(publishingWasCalled); + Assert.IsTrue(publishedWasCalled); } finally { - ContentNotificationHandler.UnpublishingContent = null; - ContentNotificationHandler.UnpublishedContent = null; + ContentNotificationHandler.PublishingContent = null; + ContentNotificationHandler.PublishedContent = null; } document = contentService.GetById(document.Id);