diff --git a/src/Umbraco.Core/Models/ContentEditing/HistoryCleanup.cs b/src/Umbraco.Core/Models/ContentEditing/HistoryCleanup.cs index b7bfb32808..a0d9bbbcb3 100644 --- a/src/Umbraco.Core/Models/ContentEditing/HistoryCleanup.cs +++ b/src/Umbraco.Core/Models/ContentEditing/HistoryCleanup.cs @@ -1,17 +1,34 @@ using System.Runtime.Serialization; +using Umbraco.Cms.Core.Models.Entities; namespace Umbraco.Cms.Core.Models.ContentEditing { [DataContract(Name = "historyCleanup", Namespace = "")] - public class HistoryCleanup + public class HistoryCleanup : BeingDirtyBase { + private bool _preventCleanup; + private int? _keepAllVersionsNewerThanDays; + private int? _keepLatestVersionPerDayForDays; + [DataMember(Name = "preventCleanup")] - public bool PreventCleanup { get; set; } + public bool PreventCleanup + { + get => _preventCleanup; + set => SetPropertyValueAndDetectChanges(value, ref _preventCleanup, nameof(PreventCleanup)); + } [DataMember(Name = "keepAllVersionsNewerThanDays")] - public int? KeepAllVersionsNewerThanDays { get; set; } + public int? KeepAllVersionsNewerThanDays + { + get => _keepAllVersionsNewerThanDays; + set => SetPropertyValueAndDetectChanges(value, ref _keepAllVersionsNewerThanDays, nameof(KeepAllVersionsNewerThanDays)); + } [DataMember(Name = "keepLatestVersionPerDayForDays")] - public int? KeepLatestVersionPerDayForDays { get; set; } + public int? KeepLatestVersionPerDayForDays + { + get => _keepLatestVersionPerDayForDays; + set => SetPropertyValueAndDetectChanges(value, ref _keepLatestVersionPerDayForDays, nameof(KeepLatestVersionPerDayForDays)); + } } } diff --git a/src/Umbraco.Core/Models/ContentType.cs b/src/Umbraco.Core/Models/ContentType.cs index 6ff94f57f3..a252aa4723 100644 --- a/src/Umbraco.Core/Models/ContentType.cs +++ b/src/Umbraco.Core/Models/ContentType.cs @@ -96,7 +96,13 @@ namespace Umbraco.Cms.Core.Models } } - public HistoryCleanup HistoryCleanup { get; set; } + private HistoryCleanup _historyCleanup; + + public HistoryCleanup HistoryCleanup + { + get => _historyCleanup; + set => SetPropertyValueAndDetectChanges(value, ref _historyCleanup, nameof(HistoryCleanup)); + } /// /// Determines if AllowedTemplates contains templateId @@ -162,5 +168,8 @@ namespace Umbraco.Cms.Core.Models /// IContentType IContentType.DeepCloneWithResetIdentities(string newAlias) => (IContentType)DeepCloneWithResetIdentities(newAlias); + + /// + public override bool IsDirty() => base.IsDirty() || HistoryCleanup.IsDirty(); } } diff --git a/src/Umbraco.Core/Models/Mapping/ContentTypeMapDefinition.cs b/src/Umbraco.Core/Models/Mapping/ContentTypeMapDefinition.cs index 3625e90a14..4bb34017ec 100644 --- a/src/Umbraco.Core/Models/Mapping/ContentTypeMapDefinition.cs +++ b/src/Umbraco.Core/Models/Mapping/ContentTypeMapDefinition.cs @@ -133,7 +133,7 @@ namespace Umbraco.Cms.Core.Models.Mapping if (target is IContentTypeWithHistoryCleanup targetWithHistoryCleanup) { - targetWithHistoryCleanup.HistoryCleanup = source.HistoryCleanup; + MapHistoryCleanup(source, targetWithHistoryCleanup); } target.AllowedTemplates = source.AllowedTemplates @@ -147,6 +147,34 @@ namespace Umbraco.Cms.Core.Models.Mapping : _fileService.GetTemplate(source.DefaultTemplate)); } + private static void MapHistoryCleanup(DocumentTypeSave source, IContentTypeWithHistoryCleanup target) + { + // If source history cleanup is null we don't have to map all properties + if (source.HistoryCleanup is null) + { + target.HistoryCleanup = null; + return; + } + + // We need to reset the dirty properties, because it is otherwise true, just because the json serializer has set properties + target.HistoryCleanup.ResetDirtyProperties(false); + if (target.HistoryCleanup.PreventCleanup != source.HistoryCleanup.PreventCleanup) + { + target.HistoryCleanup.PreventCleanup = source.HistoryCleanup.PreventCleanup; + } + + if (target.HistoryCleanup.KeepAllVersionsNewerThanDays != source.HistoryCleanup.KeepAllVersionsNewerThanDays) + { + target.HistoryCleanup.KeepAllVersionsNewerThanDays = source.HistoryCleanup.KeepAllVersionsNewerThanDays; + } + + if (target.HistoryCleanup.KeepLatestVersionPerDayForDays != + source.HistoryCleanup.KeepLatestVersionPerDayForDays) + { + target.HistoryCleanup.KeepLatestVersionPerDayForDays = source.HistoryCleanup.KeepLatestVersionPerDayForDays; + } + } + // no MapAll - take care private void Map(MediaTypeSave source, IMediaType target, MapperContext context) { @@ -328,7 +356,10 @@ namespace Umbraco.Cms.Core.Models.Mapping if (source.GroupId > 0) { - target.PropertyGroupId = new Lazy(() => source.GroupId, false); + if (target.PropertyGroupId?.Value != source.GroupId) + { + target.PropertyGroupId = new Lazy(() => source.GroupId, false); + } } target.Alias = source.Alias; @@ -523,7 +554,15 @@ namespace Umbraco.Cms.Core.Models.Mapping target.Thumbnail = source.Thumbnail; target.AllowedAsRoot = source.AllowAsRoot; - target.AllowedContentTypes = source.AllowedContentTypes.Select((t, i) => new ContentTypeSort(t, i)); + + bool allowedContentTypesUnchanged = target.AllowedContentTypes.Select(x => x.Id.Value) + .SequenceEqual(source.AllowedContentTypes); + + if (allowedContentTypesUnchanged is false) + { + target.AllowedContentTypes = source.AllowedContentTypes.Select((t, i) => new ContentTypeSort(t, i)); + } + if (!(target is IMemberType)) { @@ -574,13 +613,21 @@ namespace Umbraco.Cms.Core.Models.Mapping // ensure no duplicate alias, then assign the group properties collection EnsureUniqueAliases(destProperties); - destGroup.PropertyTypes = new PropertyTypeCollection(isPublishing, destProperties); + if (destGroup.PropertyTypes.SupportsPublishing != isPublishing || destGroup.PropertyTypes.SequenceEqual(destProperties) is false) + { + destGroup.PropertyTypes = new PropertyTypeCollection(isPublishing, destProperties); + } + destGroups.Add(destGroup); } // ensure no duplicate name, then assign the groups collection EnsureUniqueAliases(destGroups); - target.PropertyGroups = new PropertyGroupCollection(destGroups); + + if (target.PropertyGroups.SequenceEqual(destGroups) is false) + { + target.PropertyGroups = new PropertyGroupCollection(destGroups); + } // because the property groups collection was rebuilt, there is no need to remove // the old groups - they are just gone and will be cleared by the repository diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Models/ContentTypeHistoryCleanupTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Models/ContentTypeHistoryCleanupTests.cs new file mode 100644 index 0000000000..1a9d293527 --- /dev/null +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Models/ContentTypeHistoryCleanupTests.cs @@ -0,0 +1,105 @@ +using System.Linq; +using NUnit.Framework; +using Umbraco.Cms.Core.Models.ContentEditing; +using Umbraco.Cms.Tests.Common.Builders; + +namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Models +{ + [TestFixture] + public class ContentTypeHistoryCleanupTests + { + [Test] + public void Changing_Keep_all_Makes_ContentType_Dirty() + { + var contentType = ContentTypeBuilder.CreateBasicContentType(); + + Assert.IsFalse(contentType.IsDirty()); + + var newValue = 2; + contentType.HistoryCleanup.KeepAllVersionsNewerThanDays = newValue; + Assert.IsTrue(contentType.IsDirty()); + Assert.AreEqual(newValue, contentType.HistoryCleanup.KeepAllVersionsNewerThanDays); + } + + [Test] + public void Changing_Keep_latest_Makes_ContentType_Dirty() + { + var contentType = ContentTypeBuilder.CreateBasicContentType(); + + Assert.IsFalse(contentType.IsDirty()); + + var newValue = 2; + contentType.HistoryCleanup.KeepLatestVersionPerDayForDays = newValue; + Assert.IsTrue(contentType.IsDirty()); + Assert.AreEqual(newValue, contentType.HistoryCleanup.KeepLatestVersionPerDayForDays); + } + + [Test] + public void Changing_Prevent_Cleanup_Makes_ContentType_Dirty() + { + var contentType = ContentTypeBuilder.CreateBasicContentType(); + + Assert.IsFalse(contentType.IsDirty()); + + var newValue = true; + contentType.HistoryCleanup.PreventCleanup = newValue; + Assert.IsTrue(contentType.IsDirty()); + Assert.AreEqual(newValue, contentType.HistoryCleanup.PreventCleanup); + } + + [Test] + public void Replacing_History_Cleanup_Registers_As_Dirty() + { + var contentType = ContentTypeBuilder.CreateBasicContentType(); + Assert.IsFalse(contentType.IsDirty()); + + contentType.HistoryCleanup = new HistoryCleanup(); + + Assert.IsTrue(contentType.IsDirty()); + Assert.IsTrue(contentType.IsPropertyDirty(nameof(contentType.HistoryCleanup))); + } + + [Test] + public void Replacing_History_Cleanup_Removes_Old_Dirty_History_Properties() + { + var contentType = ContentTypeBuilder.CreateBasicContentType(); + + contentType.Alias = "NewValue"; + contentType.HistoryCleanup.KeepAllVersionsNewerThanDays = 2; + + contentType.PropertyChanged += (sender, args) => + { + // Ensure that property changed is only invoked for history cleanup + Assert.AreEqual(nameof(contentType.HistoryCleanup), args.PropertyName); + }; + + // Since we're replacing the entire HistoryCleanup the changed property is no longer dirty, the entire HistoryCleanup is + contentType.HistoryCleanup = new HistoryCleanup(); + + Assert.Multiple(() => + { + Assert.IsTrue(contentType.IsDirty()); + Assert.IsFalse(contentType.WasDirty()); + Assert.AreEqual(2, contentType.GetDirtyProperties().Count()); + Assert.IsTrue(contentType.IsPropertyDirty(nameof(contentType.HistoryCleanup))); + Assert.IsTrue(contentType.IsPropertyDirty(nameof(contentType.Alias))); + }); + } + + [Test] + public void Old_History_Cleanup_Reference_Doesnt_Make_Content_Type_Dirty() + { + var contentType = ContentTypeBuilder.CreateBasicContentType(); + var oldHistoryCleanup = contentType.HistoryCleanup; + + contentType.HistoryCleanup = new HistoryCleanup(); + contentType.ResetDirtyProperties(); + contentType.ResetWereDirtyProperties(); + + oldHistoryCleanup.KeepAllVersionsNewerThanDays = 2; + + Assert.IsFalse(contentType.IsDirty()); + Assert.IsFalse(contentType.WasDirty()); + } + } +}