From 2767f18c9cef78d419370300dc80bbb6fbe2207f Mon Sep 17 00:00:00 2001 From: kjac Date: Tue, 31 Mar 2026 17:17:06 +0200 Subject: [PATCH 1/4] Backport #21910 to V13 --- .../Changes/ContentTypeChangeExtensions.cs | 16 +++++++++ ...IndexingNotificationHandler.ContentType.cs | 36 +++++++++---------- .../PublishedSnapshotServiceEventHandler.cs | 4 +-- .../ContentTypeChangeExtensionsTests.cs | 31 ++++++++++++++++ 4 files changed, 66 insertions(+), 21 deletions(-) create mode 100644 tests/Umbraco.Tests.UnitTests/Umbraco.Core/Extensions/ContentTypeChangeExtensionsTests.cs diff --git a/src/Umbraco.Core/Services/Changes/ContentTypeChangeExtensions.cs b/src/Umbraco.Core/Services/Changes/ContentTypeChangeExtensions.cs index d45a2267bc89..53f65d457c31 100644 --- a/src/Umbraco.Core/Services/Changes/ContentTypeChangeExtensions.cs +++ b/src/Umbraco.Core/Services/Changes/ContentTypeChangeExtensions.cs @@ -18,4 +18,20 @@ public static bool HasTypesAny(this ContentTypeChangeTypes change, ContentTypeCh public static bool HasTypesNone(this ContentTypeChangeTypes change, ContentTypeChangeTypes types) => (change & types) == ContentTypeChangeTypes.None; + + /// + /// Determines whether the change has structural change impact. + /// + /// The change to check. + /// true if the change has structural impact; otherwise, false. + public static bool IsStructuralChange(this ContentTypeChangeTypes change) => + change.HasType(ContentTypeChangeTypes.RefreshMain); + + /// + /// Determines whether the change has non-structural change impact. + /// + /// The change to check. + /// true if the change has non-structural impact; otherwise, false. + public static bool IsNonStructuralChange(this ContentTypeChangeTypes change) => + change.HasType(ContentTypeChangeTypes.RefreshOther) && !change.HasType(ContentTypeChangeTypes.RefreshMain); } diff --git a/src/Umbraco.Infrastructure/Search/IndexingNotificationHandler.ContentType.cs b/src/Umbraco.Infrastructure/Search/IndexingNotificationHandler.ContentType.cs index 17f7350a0d9d..75e0cd85bdb4 100644 --- a/src/Umbraco.Infrastructure/Search/IndexingNotificationHandler.ContentType.cs +++ b/src/Umbraco.Infrastructure/Search/IndexingNotificationHandler.ContentType.cs @@ -52,16 +52,16 @@ public void Handle(ContentTypeCacheRefresherNotification args) throw new NotSupportedException(); } - var changedIds = new Dictionary removedIds, List refreshedIds, List otherIds)>(); + var changedIds = new Dictionary removedIds, List refreshedIds)>(); foreach (ContentTypeCacheRefresher.JsonPayload payload in (ContentTypeCacheRefresher.JsonPayload[])args .MessageObject) { if (!changedIds.TryGetValue( payload.ItemType, - out (List removedIds, List refreshedIds, List otherIds) idLists)) + out (List removedIds, List refreshedIds) idLists)) { - idLists = (removedIds: new List(), refreshedIds: new List(), otherIds: new List()); + idLists = (removedIds: new List(), refreshedIds: new List()); changedIds.Add(payload.ItemType, idLists); } @@ -73,28 +73,24 @@ public void Handle(ContentTypeCacheRefresherNotification args) { idLists.refreshedIds.Add(payload.Id); } - else if (payload.ChangeTypes.HasType(ContentTypeChangeTypes.RefreshOther)) - { - idLists.otherIds.Add(payload.Id); - } } - foreach (KeyValuePair removedIds, List refreshedIds, List otherIds)> ci in + foreach (KeyValuePair removedIds, List refreshedIds)> ci in changedIds) { - if (ci.Value.refreshedIds.Count > 0 || ci.Value.otherIds.Count > 0) + if (ci.Value.refreshedIds.Count > 0) { switch (ci.Key) { case var itemType when itemType == typeof(IContentType).Name: - RefreshContentOfContentTypes(ci.Value.refreshedIds.Concat(ci.Value.otherIds).Distinct() + RefreshContentOfContentTypes(ci.Value.refreshedIds.Distinct() .ToArray()); break; case var itemType when itemType == typeof(IMediaType).Name: - RefreshMediaOfMediaTypes(ci.Value.refreshedIds.Concat(ci.Value.otherIds).Distinct().ToArray()); + RefreshMediaOfMediaTypes(ci.Value.refreshedIds.Distinct().ToArray()); break; case var itemType when itemType == typeof(IMemberType).Name: - RefreshMemberOfMemberTypes(ci.Value.refreshedIds.Concat(ci.Value.otherIds).Distinct() + RefreshMemberOfMemberTypes(ci.Value.refreshedIds.Distinct() .ToArray()); break; } @@ -154,6 +150,10 @@ private void RefreshContentOfContentTypes(int[] contentTypeIds) const int pageSize = 500; var page = 0; var total = long.MaxValue; + + // track which Ids have their paths are published + var publishChecked = new Dictionary(); + while (page * pageSize < total) { IEnumerable contentToRefresh = _contentService.GetPagedOfTypes( @@ -165,20 +165,20 @@ private void RefreshContentOfContentTypes(int[] contentTypeIds) // order by shallowest to deepest, this allows us to check it's published state without checking every item Ordering.By("Path")); - // track which Ids have their paths are published - var publishChecked = new Dictionary(); - foreach (IContent c in contentToRefresh) { var isPublished = false; if (c.Published) { - if (!publishChecked.TryGetValue(c.ParentId, out isPublished)) + if (publishChecked.TryGetValue(c.ParentId, out isPublished) is false) { - // nothing by parent id, so query the service and cache the result for the next child to check against isPublished = _contentService.IsPathPublished(c); - publishChecked[c.Id] = isPublished; + + // the parent *must* be published it the entire path is published + publishChecked[c.ParentId] = isPublished; } + + publishChecked[c.Id] = isPublished; } _umbracoIndexingHandler.ReIndexForContent(c, isPublished); diff --git a/src/Umbraco.PublishedCache.NuCache/PublishedSnapshotServiceEventHandler.cs b/src/Umbraco.PublishedCache.NuCache/PublishedSnapshotServiceEventHandler.cs index f122077498ee..de8661ce6097 100644 --- a/src/Umbraco.PublishedCache.NuCache/PublishedSnapshotServiceEventHandler.cs +++ b/src/Umbraco.PublishedCache.NuCache/PublishedSnapshotServiceEventHandler.cs @@ -53,9 +53,7 @@ public void Handle(ContentRefreshNotification notification) => public void Handle(ContentTypeRefreshedNotification notification) { - const ContentTypeChangeTypes types // only for those that have been refreshed - = ContentTypeChangeTypes.RefreshMain | ContentTypeChangeTypes.RefreshOther; - var contentTypeIds = notification.Changes.Where(x => x.ChangeTypes.HasTypesAny(types)).Select(x => x.Item.Id) + var contentTypeIds = notification.Changes.Where(x => x.ChangeTypes.IsStructuralChange()).Select(x => x.Item.Id) .ToArray(); if (contentTypeIds.Any()) { diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Extensions/ContentTypeChangeExtensionsTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Extensions/ContentTypeChangeExtensionsTests.cs new file mode 100644 index 000000000000..6ac7aa7bb697 --- /dev/null +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Extensions/ContentTypeChangeExtensionsTests.cs @@ -0,0 +1,31 @@ +using NUnit.Framework; +using Umbraco.Cms.Core.Services.Changes; +using Umbraco.Extensions; + +namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Extensions; + +[TestFixture] +public class ContentTypeChangeExtensionsTests +{ + [TestCase(ContentTypeChangeTypes.RefreshMain, true)] + [TestCase(ContentTypeChangeTypes.RefreshMain | ContentTypeChangeTypes.RefreshOther, true)] + [TestCase(ContentTypeChangeTypes.RefreshMain | ContentTypeChangeTypes.Create, true)] + [TestCase(ContentTypeChangeTypes.RefreshOther, false)] + [TestCase(ContentTypeChangeTypes.None, false)] + [TestCase(ContentTypeChangeTypes.Create, false)] + [TestCase(ContentTypeChangeTypes.Remove, false)] + [TestCase(ContentTypeChangeTypes.RefreshOther | ContentTypeChangeTypes.Remove, false)] + public void IsStructuralChange(ContentTypeChangeTypes change, bool expected) => + Assert.AreEqual(expected, change.IsStructuralChange()); + + [TestCase(ContentTypeChangeTypes.RefreshOther, true)] + [TestCase(ContentTypeChangeTypes.RefreshOther | ContentTypeChangeTypes.Create, true)] + [TestCase(ContentTypeChangeTypes.RefreshOther | ContentTypeChangeTypes.Remove, true)] + [TestCase(ContentTypeChangeTypes.RefreshMain, false)] + [TestCase(ContentTypeChangeTypes.RefreshMain | ContentTypeChangeTypes.RefreshOther, false)] + [TestCase(ContentTypeChangeTypes.None, false)] + [TestCase(ContentTypeChangeTypes.Create, false)] + [TestCase(ContentTypeChangeTypes.Remove, false)] + public void IsNonStructuralChange(ContentTypeChangeTypes change, bool expected) => + Assert.AreEqual(expected, change.IsNonStructuralChange()); +} From 9207665b3e7b0e4196a09ceefbb9ea240271ccfa Mon Sep 17 00:00:00 2001 From: Kenn Jacobsen Date: Wed, 1 Apr 2026 08:28:55 +0200 Subject: [PATCH 2/4] Update src/Umbraco.Infrastructure/Search/IndexingNotificationHandler.ContentType.cs Co-authored-by: Andy Butland --- .../Search/IndexingNotificationHandler.ContentType.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Infrastructure/Search/IndexingNotificationHandler.ContentType.cs b/src/Umbraco.Infrastructure/Search/IndexingNotificationHandler.ContentType.cs index 75e0cd85bdb4..c9784f01c645 100644 --- a/src/Umbraco.Infrastructure/Search/IndexingNotificationHandler.ContentType.cs +++ b/src/Umbraco.Infrastructure/Search/IndexingNotificationHandler.ContentType.cs @@ -174,7 +174,7 @@ private void RefreshContentOfContentTypes(int[] contentTypeIds) { isPublished = _contentService.IsPathPublished(c); - // the parent *must* be published it the entire path is published + // the parent *must* be published if the entire path is published publishChecked[c.ParentId] = isPublished; } From c00551fbf7d60ccdafb548de8afe4e2206c41d77 Mon Sep 17 00:00:00 2001 From: kjac Date: Wed, 1 Apr 2026 08:44:29 +0200 Subject: [PATCH 3/4] Backport #21856 as well --- src/Umbraco.Core/Models/ContentTypeBase.cs | 10 +- .../Models/ContentTypeCompositionBase.cs | 21 ++ .../Changes/ContentTypeChangeTypes.cs | 21 +- .../Services/ContentTypeServiceTests.cs | 328 ++++++++++++++++++ 4 files changed, 372 insertions(+), 8 deletions(-) diff --git a/src/Umbraco.Core/Models/ContentTypeBase.cs b/src/Umbraco.Core/Models/ContentTypeBase.cs index 6131e1b6809d..61e07bd790bd 100644 --- a/src/Umbraco.Core/Models/ContentTypeBase.cs +++ b/src/Umbraco.Core/Models/ContentTypeBase.cs @@ -443,21 +443,21 @@ public override bool IsDirty() /// Please note that resetting the dirty properties could potentially /// obstruct the saving of a new or updated entity. /// - public override void ResetDirtyProperties() + public override void ResetDirtyProperties(bool rememberDirty) { - base.ResetDirtyProperties(); + base.ResetDirtyProperties(rememberDirty); // loop through each property group to reset the property types var propertiesReset = new List(); foreach (PropertyGroup propertyGroup in PropertyGroups) { - propertyGroup.ResetDirtyProperties(); + propertyGroup.ResetDirtyProperties(rememberDirty); if (propertyGroup.PropertyTypes is not null) { foreach (IPropertyType propertyType in propertyGroup.PropertyTypes) { - propertyType.ResetDirtyProperties(); + propertyType.ResetDirtyProperties(rememberDirty); propertiesReset.Add(propertyType.Id); } } @@ -467,7 +467,7 @@ public override void ResetDirtyProperties() // but don't re-reset ones we've already done. foreach (IPropertyType propertyType in PropertyTypes.Where(x => propertiesReset.Contains(x.Id) == false)) { - propertyType.ResetDirtyProperties(); + propertyType.ResetDirtyProperties(rememberDirty); } } diff --git a/src/Umbraco.Core/Models/ContentTypeCompositionBase.cs b/src/Umbraco.Core/Models/ContentTypeCompositionBase.cs index b7b9af6231c4..e447d919d30f 100644 --- a/src/Umbraco.Core/Models/ContentTypeCompositionBase.cs +++ b/src/Umbraco.Core/Models/ContentTypeCompositionBase.cs @@ -13,6 +13,7 @@ public abstract class ContentTypeCompositionBase : ContentTypeBase, IContentType { private List _contentTypeComposition = new(); private List _removedContentTypeKeyTracker = new(); + private bool _hasCompositionBeenRemoved; protected ContentTypeCompositionBase(IShortStringHelper shortStringHelper, int parentId) : base(shortStringHelper, parentId) @@ -104,6 +105,24 @@ IPropertyType AcquireProperty(IPropertyType propertyType) } } + /// + /// A boolean flag indicating if a composition has been removed from this instance. + /// + /// + /// This is currently (specifically) used in order to know that we need to refresh the content cache which + /// needs to occur when a composition has been removed from a content type + /// + [IgnoreDataMember] + internal bool HasCompositionTypeBeenRemoved + { + get => _hasCompositionBeenRemoved; + private set + { + _hasCompositionBeenRemoved = value; + OnPropertyChanged(nameof(HasCompositionTypeBeenRemoved)); + } + } + /// public IEnumerable GetOriginalComposedPropertyTypes() => GetRawComposedPropertyTypes(); @@ -179,6 +198,8 @@ public bool RemoveContentType(string alias) _removedContentTypeKeyTracker.AddRange(compositionIdsToRemove); } + HasCompositionTypeBeenRemoved = true; + OnPropertyChanged(nameof(ContentTypeComposition)); return _contentTypeComposition.Remove(contentTypeComposition); diff --git a/src/Umbraco.Core/Services/Changes/ContentTypeChangeTypes.cs b/src/Umbraco.Core/Services/Changes/ContentTypeChangeTypes.cs index 4346a278cc28..4393bc6fd634 100644 --- a/src/Umbraco.Core/Services/Changes/ContentTypeChangeTypes.cs +++ b/src/Umbraco.Core/Services/Changes/ContentTypeChangeTypes.cs @@ -11,14 +11,29 @@ public enum ContentTypeChangeTypes : byte Create = 1, /// - /// Content type changes impact only the Content type being saved + /// Content type changes directly impact existing content of this content type. /// + /// + /// These changes are "destructive" of nature. They include: + /// - Changing the content type alias. + /// - Removing a property type or a composition. + /// - Changing the alias of a property type (this effectively corresponds to removing a property type). + /// - Changing variance, either at property or content type level. + /// RefreshMain = 2, /// - /// Content type changes impacts the content type being saved and others used that are composed of it + /// Content type changes that do not directly impact existing content of this content type. /// - RefreshOther = 4, // changed, other change + /// + /// These changes are "constructive" of nature, and include all changes not included in + /// - for example: + /// - Adding a property type or a composition. + /// - Rearranging property types or groups. + /// - Changes to name, description, icon etc. + /// - Changes to other content type settings, i.e. allowed child types and version cleanup. + /// + RefreshOther = 4, /// /// Content type was removed diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentTypeServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentTypeServiceTests.cs index 3083fac61e5a..ac8cb924a6ce 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentTypeServiceTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentTypeServiceTests.cs @@ -5,12 +5,16 @@ using System.Linq; using NUnit.Framework; using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Exceptions; using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.Models.ContentEditing; using Umbraco.Cms.Core.Notifications; using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Core.Services.Changes; +using Umbraco.Cms.Core.Sync; using Umbraco.Cms.Tests.Common.Attributes; using Umbraco.Cms.Tests.Common.Builders; using Umbraco.Cms.Tests.Common.Testing; @@ -35,6 +39,10 @@ protected override void CustomTestSetup(IUmbracoBuilder builder) { builder.AddNotificationHandler(); builder.AddNotificationHandler(); + + builder.AddNotificationHandler(); + builder.AddNotificationHandler(); + builder.Services.AddUnique(); } [Test] @@ -2026,6 +2034,298 @@ public void Can_Create_Property_Type_Based_On_PropertyEditorAlias() Assert.That(ctBase.PropertyTypes.First().PropertyEditorAlias, Is.EqualTo(dtdYesNo.EditorAlias)); } + [Test] + public void Adding_ContentType_Composition_Yields_RefreshOther() + { + var cts = ContentTypeService; + + // Arrange + IContentType component = CreateComponent(); + cts.Save(component); + IContentType site = CreateSite(); + cts.Save(site); + + // re-fetch before acting + site = cts.Get(site.Id)!; + + // Act + ContentTypeCacheRefresher.JsonPayload[] refreshedPayloads = null; + ContentTypeCacheRefreshedNotificationHandler.ContentTypeCacheRefreshed = payloads + => refreshedPayloads = payloads; + + Assert.IsTrue(site.AddContentType(component)); + Assert.IsTrue(site.ContentTypeCompositionExists(component.Alias)); + cts.Save(site); + + // Assert; expect RefreshOther when adding a compostion + AssertContentTypeRefreshPayload(refreshedPayloads, site.Id, ContentTypeChangeTypes.RefreshOther); + } + + [Test] + public void Adding_PropertyType_Yields_RefreshOther() + { + var cts = ContentTypeService; + + // Arrange + IContentType site = CreateSite(); + cts.Save(site); + + // re-fetch before acting + site = cts.Get(site.Id)!; + + // Act + ContentTypeCacheRefresher.JsonPayload[] refreshedPayloads = null; + ContentTypeCacheRefreshedNotificationHandler.ContentTypeCacheRefreshed = payloads + => refreshedPayloads = payloads; + + var propertyType = + new PropertyType(ShortStringHelper, Constants.PropertyEditors.Aliases.TextBox, ValueStorageType.Ntext, + "title") + { + Name = "Title", + Description = string.Empty, + Mandatory = false, + SortOrder = 1, + DataTypeId = -88 + }; + Assert.IsTrue(site.AddPropertyType(propertyType)); + cts.Save(site); + + // Assert; expect RefreshOther when adding a property + AssertContentTypeRefreshPayload(refreshedPayloads, site.Id, ContentTypeChangeTypes.RefreshOther); + } + + [Test] + public void Removing_ContentType_Composition_Yields_RefreshMain() + { + var cts = ContentTypeService; + + // Arrange + IContentType component = CreateComponent(); + cts.Save(component); + IContentType site = CreateSite(); + Assert.IsTrue(site.AddContentType(component)); + cts.Save(site); + + // re-fetch before acting + site = cts.Get(site.Id)!; + Assert.IsTrue(site.ContentTypeCompositionExists(component.Alias)); + + // Act + ContentTypeCacheRefresher.JsonPayload[] refreshedPayloads = null; + ContentTypeCacheRefreshedNotificationHandler.ContentTypeCacheRefreshed = payloads + => refreshedPayloads = payloads; + + Assert.IsTrue(site.RemoveContentType(component.Alias)); + cts.Save(site); + + // Assert; expect RefreshMain when removing a composition + AssertContentTypeRefreshPayload(refreshedPayloads, site.Id, ContentTypeChangeTypes.RefreshMain); + } + + [Test] + public void Removing_PropertyType_Yields_RefreshMain() + { + var cts = ContentTypeService; + + // Arrange + IContentType site = CreateSite(); + cts.Save(site); + + // re-fetch before acting + site = cts.Get(site.Id)!; + + // Act + ContentTypeCacheRefresher.JsonPayload[] refreshedPayloads = null; + ContentTypeCacheRefreshedNotificationHandler.ContentTypeCacheRefreshed = payloads + => refreshedPayloads = payloads; + + site.RemovePropertyType(site.PropertyTypes.First().Alias); + cts.Save(site); + + // Assert; expect RefreshMain when removing a property + AssertContentTypeRefreshPayload(refreshedPayloads, site.Id, ContentTypeChangeTypes.RefreshMain); + } + + [Test] + public void Removing_PropertyTypeGroup_Yields_RefreshOther() + { + var cts = ContentTypeService; + + // Arrange + IContentType site = CreateSite(); + cts.Save(site); + + // re-fetch before acting + site = cts.Get(site.Id)!; + + // Act + ContentTypeCacheRefresher.JsonPayload[] refreshedPayloads = null; + ContentTypeCacheRefreshedNotificationHandler.ContentTypeCacheRefreshed = payloads + => refreshedPayloads = payloads; + + site.RemovePropertyGroup(site.PropertyTypes.First().Alias); + cts.Save(site); + + // Assert; removing a group does not cause the contained properties to be removed, so expect RefreshOther + AssertContentTypeRefreshPayload(refreshedPayloads, site.Id, ContentTypeChangeTypes.RefreshOther); + } + + [Test] + public void Changing_PropertyType_Alias_Yields_RefreshMain() + { + var cts = ContentTypeService; + + // Arrange + IContentType site = CreateSite(); + cts.Save(site); + + // re-fetch before acting + site = cts.Get(site.Id)!; + + // Act + ContentTypeCacheRefresher.JsonPayload[] refreshedPayloads = null; + ContentTypeCacheRefreshedNotificationHandler.ContentTypeCacheRefreshed = payloads + => refreshedPayloads = payloads; + + site.PropertyTypes.First().Alias += "_updated"; + cts.Save(site); + + // Assert; expect RefreshMain when changing the alias of a property (it corresponds to removing the property) + AssertContentTypeRefreshPayload(refreshedPayloads, site.Id, ContentTypeChangeTypes.RefreshMain); + } + + [Test] + public void Changing_ContentType_Alias_Yields_RefreshMain() + { + var cts = ContentTypeService; + + // Arrange + IContentType site = CreateSite(); + cts.Save(site); + + // re-fetch before acting + site = cts.Get(site.Id)!; + + // Act + ContentTypeCacheRefresher.JsonPayload[] refreshedPayloads = null; + ContentTypeCacheRefreshedNotificationHandler.ContentTypeCacheRefreshed = payloads + => refreshedPayloads = payloads; + + site.Alias += "_updated"; + cts.Save(site); + + // Assert; expect RefreshMain when changing the alias of a content type + AssertContentTypeRefreshPayload(refreshedPayloads, site.Id, ContentTypeChangeTypes.RefreshMain); + } + + [Test] + public void Changing_PropertyType_Variance_Yields_RefreshMain() + { + var cts = ContentTypeService; + + // Arrange + IContentType site = CreateSite(); + site.Variations = ContentVariation.Culture; + cts.Save(site); + + // re-fetch before acting + site = cts.Get(site.Id)!; + + // Act + ContentTypeCacheRefresher.JsonPayload[] refreshedPayloads = null; + ContentTypeCacheRefreshedNotificationHandler.ContentTypeCacheRefreshed = payloads + => refreshedPayloads = payloads; + + site.PropertyTypes.First().Variations = ContentVariation.Culture; + cts.Save(site); + + // Assert; expect RefreshMain when changing the variance of a property + AssertContentTypeRefreshPayload(refreshedPayloads, site.Id, ContentTypeChangeTypes.RefreshMain); + } + + [Test] + public void Changing_ContentType_Variance_Yields_RefreshMain() + { + var cts = ContentTypeService; + + // Arrange + IContentType site = CreateSite(); + cts.Save(site); + + // re-fetch before acting + site = cts.Get(site.Id)!; + + // Act + ContentTypeCacheRefresher.JsonPayload[] refreshedPayloads = null; + ContentTypeCacheRefreshedNotificationHandler.ContentTypeCacheRefreshed = payloads + => refreshedPayloads = payloads; + + site.Variations = ContentVariation.Culture; + cts.Save(site); + + // Assert; expect RefreshMain when changing the variance of a content type + AssertContentTypeRefreshPayload(refreshedPayloads, site.Id, ContentTypeChangeTypes.RefreshMain); + } + + [Test] + public void Changing_User_Interface_Settings_Yields_RefreshMain() + { + var cts = ContentTypeService; + + // Arrange + IContentType site = CreateSite(); + cts.Save(site); + + // re-fetch before acting + site = cts.Get(site.Id)!; + + // Act + ContentTypeCacheRefresher.JsonPayload[] refreshedPayloads = null; + ContentTypeCacheRefreshedNotificationHandler.ContentTypeCacheRefreshed = payloads + => refreshedPayloads = payloads; + + site.Name += "_updated"; + site.Description += "_updated"; + site.Icon += "_updated"; + site.PropertyTypes.First().Name += "_updated"; + cts.Save(site); + + // Assert; expect RefreshOther when making UI changes only (names, icon, description etc.) + AssertContentTypeRefreshPayload(refreshedPayloads, site.Id, ContentTypeChangeTypes.RefreshOther); + } + + [Test] + public void Changing_History_Cleanup_And_Basic_Structure_Settings_Yields_RefreshMain() + { + var cts = ContentTypeService; + + // Arrange + IContentType site = CreateSite(); + cts.Save(site); + + // re-fetch before acting + site = cts.Get(site.Id)!; + + // Act + ContentTypeCacheRefresher.JsonPayload[] refreshedPayloads = null; + ContentTypeCacheRefreshedNotificationHandler.ContentTypeCacheRefreshed = payloads + => refreshedPayloads = payloads; + + site.HistoryCleanup = new HistoryCleanup + { + KeepAllVersionsNewerThanDays = 12, + KeepLatestVersionPerDayForDays = 32, + PreventCleanup = false, + }; + site.AllowedAsRoot = !site.AllowedAsRoot; + site.AllowedContentTypes = [new ContentTypeSort(site.Id, 1)]; + cts.Save(site); + + // Assert; expect RefreshOther when making UI changes only (names, icon, description etc.) + AssertContentTypeRefreshPayload(refreshedPayloads, site.Id, ContentTypeChangeTypes.RefreshOther); + } + private ContentType CreateComponent() { var component = new ContentType(ShortStringHelper, -1) @@ -2169,4 +2469,32 @@ public class ContentTypeNotificationHandler : INotificationHandler Deleted { get; set; } public void Handle(ContentTypeDeletedNotification notification) => Deleted?.Invoke(notification); } + + public class ContentTypeCacheRefreshedNotificationHandler : INotificationHandler + { + public static Action? ContentTypeCacheRefreshed { get; set; } + + public void Handle(ContentTypeCacheRefresherNotification notification) + { + if (notification.MessageType != MessageType.RefreshByPayload || notification.MessageObject is not ContentTypeCacheRefresher.JsonPayload[] payloads) + { + throw new NotSupportedException(); + } + + ContentTypeCacheRefreshed?.Invoke(payloads); + } + } + + private static void AssertContentTypeRefreshPayload(ContentTypeCacheRefresher.JsonPayload[]? refreshedPayloads, int expectedContentTypeId, ContentTypeChangeTypes expectedChangeTypes) + { + Assert.IsNotNull(refreshedPayloads); + Assert.AreEqual(1, refreshedPayloads.Length); + Assert.Multiple(() => + { + var payload = refreshedPayloads.First(); + Assert.AreEqual(expectedContentTypeId, payload.Id); + Assert.AreEqual(expectedChangeTypes, payload.ChangeTypes); + Assert.AreEqual(nameof(IContentType), payload.ItemType); + }); + } } From c99ee53800d118cc5737e7fa6c5abc0297b3902d Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Wed, 1 Apr 2026 12:30:31 +0200 Subject: [PATCH 4/4] Fix issue where removing a property type wasn't detecting and returning the expected change type. --- src/Umbraco.Core/Models/ContentTypeBase.cs | 25 ++++++++++++ src/Umbraco.Core/Models/PropertyGroup.cs | 39 +++++++++++++++++++ ...peServiceBaseOfTRepositoryTItemTService.cs | 5 ++- .../Services/ContentTypeServiceTests.cs | 30 ++++++++++++++ 4 files changed, 98 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Core/Models/ContentTypeBase.cs b/src/Umbraco.Core/Models/ContentTypeBase.cs index 61e07bd790bd..6efa3fb644e5 100644 --- a/src/Umbraco.Core/Models/ContentTypeBase.cs +++ b/src/Umbraco.Core/Models/ContentTypeBase.cs @@ -132,6 +132,30 @@ private set } } + /// + /// Detects if any persisted property types have been removed by comparing old and new collections, + /// and sets accordingly. + /// + private void DetectPropertyTypeRemovals(IEnumerable oldPropertyTypes, IEnumerable newPropertyTypes) + { + if (HasPropertyTypeBeenRemoved) + { + return; + } + + var oldIds = new HashSet(oldPropertyTypes.Select(pt => pt.Id).Where(id => id > 0)); + if (oldIds.Count == 0) + { + return; + } + + var newIds = new HashSet(newPropertyTypes.Select(pt => pt.Id).Where(id => id > 0)); + if (oldIds.Any(id => !newIds.Contains(id))) + { + HasPropertyTypeBeenRemoved = true; + } + } + /// /// PropertyTypes that are not part of a PropertyGroup /// @@ -273,6 +297,7 @@ public IEnumerable NoGroupPropertyTypes { if (PropertyTypeCollection != null) { + DetectPropertyTypeRemovals(PropertyTypeCollection, value); PropertyTypeCollection.ClearCollectionChangedEvents(); } diff --git a/src/Umbraco.Core/Models/PropertyGroup.cs b/src/Umbraco.Core/Models/PropertyGroup.cs index 9d23c85a9b93..34fbae3b02f6 100644 --- a/src/Umbraco.Core/Models/PropertyGroup.cs +++ b/src/Umbraco.Core/Models/PropertyGroup.cs @@ -20,6 +20,7 @@ public class PropertyGroup : EntityBase, IEquatable private string _alias; private string? _name; + private bool _hasPropertyTypeBeenRemoved; private PropertyTypeCollection? _propertyTypes; private int _sortOrder; @@ -94,6 +95,20 @@ public int SortOrder set => SetPropertyValueAndDetectChanges(value, ref _sortOrder, nameof(SortOrder)); } + /// + /// A boolean flag indicating if a property type has been removed from this group. + /// + [IgnoreDataMember] + internal bool HasPropertyTypeBeenRemoved + { + get => _hasPropertyTypeBeenRemoved; + private set + { + _hasPropertyTypeBeenRemoved = value; + OnPropertyChanged(nameof(HasPropertyTypeBeenRemoved)); + } + } + /// /// Gets or sets a collection of property types for the group. /// @@ -112,6 +127,7 @@ public PropertyTypeCollection? PropertyTypes { if (_propertyTypes != null) { + DetectPropertyTypeRemovals(_propertyTypes, value); _propertyTypes.ClearCollectionChangedEvents(); } @@ -155,4 +171,27 @@ protected override void PerformDeepClone(object clone) private void PropertyTypesChanged(object? sender, NotifyCollectionChangedEventArgs e) => OnPropertyChanged(nameof(PropertyTypes)); + + private void DetectPropertyTypeRemovals(PropertyTypeCollection oldPropertyTypes, PropertyTypeCollection? newPropertyTypes) + { + if (HasPropertyTypeBeenRemoved) + { + return; + } + + var oldIds = new HashSet(oldPropertyTypes.Select(pt => pt.Id).Where(id => id > 0)); + if (oldIds.Count == 0) + { + return; + } + + var newIds = newPropertyTypes != null + ? new HashSet(newPropertyTypes.Select(pt => pt.Id).Where(id => id > 0)) + : new HashSet(); + + if (oldIds.Any(id => !newIds.Contains(id))) + { + HasPropertyTypeBeenRemoved = true; + } + } } diff --git a/src/Umbraco.Core/Services/ContentTypeServiceBaseOfTRepositoryTItemTService.cs b/src/Umbraco.Core/Services/ContentTypeServiceBaseOfTRepositoryTItemTService.cs index 512fad06742e..f473c1409bd8 100644 --- a/src/Umbraco.Core/Services/ContentTypeServiceBaseOfTRepositoryTItemTService.cs +++ b/src/Umbraco.Core/Services/ContentTypeServiceBaseOfTRepositoryTItemTService.cs @@ -226,7 +226,10 @@ internal IEnumerable> ComposeContentTypeChanges(params }); // removed properties? - var hasAnyPropertyBeenRemoved = dirty.WasPropertyDirty("HasPropertyTypeBeenRemoved"); + // check both the content type level flag (set by RemovePropertyType) and + // individual property group flags (set when PropertyTypes collection is replaced, e.g. by the mapper) + var hasAnyPropertyBeenRemoved = dirty.WasPropertyDirty("HasPropertyTypeBeenRemoved") + || contentType.PropertyGroups.Any(g => g.WasPropertyDirty("HasPropertyTypeBeenRemoved")); // removed compositions? var hasAnyCompositionBeenRemoved = dirty.WasPropertyDirty("HasCompositionTypeBeenRemoved"); diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentTypeServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentTypeServiceTests.cs index ac8cb924a6ce..5c8996777dfb 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentTypeServiceTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentTypeServiceTests.cs @@ -2147,6 +2147,36 @@ public void Removing_PropertyType_Yields_RefreshMain() AssertContentTypeRefreshPayload(refreshedPayloads, site.Id, ContentTypeChangeTypes.RefreshMain); } + [Test] + public void Removing_PropertyType_By_Replacing_Collection_Yields_RefreshMain() + { + // This test simulates how the backoffice mapper removes a property: rather than calling + // RemovePropertyType(), it rebuilds the property collection without the removed property + // and assigns it to the group. This is the code path in ContentTypeMapDefinition.MapSaveToTypeBase(). + var cts = ContentTypeService; + + // Arrange + IContentType site = CreateSite(); + cts.Save(site); + + // re-fetch before acting + site = cts.Get(site.Id)!; + + // Act + ContentTypeCacheRefresher.JsonPayload[] refreshedPayloads = null; + ContentTypeCacheRefreshedNotificationHandler.ContentTypeCacheRefreshed = payloads + => refreshedPayloads = payloads; + + // Simulate the mapper path: replace the group's PropertyTypes with an empty collection + // (as if the property was removed via the backoffice UI) + PropertyGroup group = site.PropertyGroups.First(); + group.PropertyTypes = new PropertyTypeCollection(true); + cts.Save(site); + + // Assert; expect RefreshMain when removing a property, regardless of how it was removed + AssertContentTypeRefreshPayload(refreshedPayloads, site.Id, ContentTypeChangeTypes.RefreshMain); + } + [Test] public void Removing_PropertyTypeGroup_Yields_RefreshOther() {