From fdc94b9d1b423b54982e33fb576e17dd5b7c5bfe Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 5 Oct 2018 15:07:19 +0200 Subject: [PATCH] Writes tests to ensure that property values are updated in docs that are of a composed type of the one that variants are being changed in. --- .../Models/ContentTypeBaseExtensions.cs | 15 +++ .../Models/IContentTypeComposition.cs | 1 + .../IContentTypeRepositoryBase.cs | 6 + .../Implement/ContentTypeRepositoryBase.cs | 10 +- .../Services/ContentTypeServiceExtensions.cs | 18 +-- ...peServiceBaseOfTRepositoryTItemTService.cs | 3 + .../Services/ContentTypeServiceTests.cs | 113 ++++++++++++++++++ .../Editors/ContentTypeControllerBase.cs | 2 +- 8 files changed, 148 insertions(+), 20 deletions(-) diff --git a/src/Umbraco.Core/Models/ContentTypeBaseExtensions.cs b/src/Umbraco.Core/Models/ContentTypeBaseExtensions.cs index 8af48bb881..0d2f817660 100644 --- a/src/Umbraco.Core/Models/ContentTypeBaseExtensions.cs +++ b/src/Umbraco.Core/Models/ContentTypeBaseExtensions.cs @@ -63,5 +63,20 @@ namespace Umbraco.Core.Models aliases = a; return hasAnyPropertyVariationChanged; } + + /// + /// Returns the list of content types the composition is used in + /// + /// + /// + /// + internal static IEnumerable GetWhereCompositionIsUsedInContentTypes(this IContentTypeComposition source, + IContentTypeComposition[] allContentTypes) + { + var sourceId = source != null ? source.Id : 0; + + // find which content types are using this composition + return allContentTypes.Where(x => x.ContentTypeComposition.Any(y => y.Id == sourceId)).ToArray(); + } } } diff --git a/src/Umbraco.Core/Models/IContentTypeComposition.cs b/src/Umbraco.Core/Models/IContentTypeComposition.cs index b5277a23be..36ace19f0f 100644 --- a/src/Umbraco.Core/Models/IContentTypeComposition.cs +++ b/src/Umbraco.Core/Models/IContentTypeComposition.cs @@ -10,6 +10,7 @@ namespace Umbraco.Core.Models /// /// Gets or sets the content types that compose this content type. /// + //fixme: we should be storing key references, not the object else we are caching way too much IEnumerable ContentTypeComposition { get; set; } /// diff --git a/src/Umbraco.Core/Persistence/Repositories/IContentTypeRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/IContentTypeRepositoryBase.cs index f5e9013082..3bb1ac38ca 100644 --- a/src/Umbraco.Core/Persistence/Repositories/IContentTypeRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/IContentTypeRepositoryBase.cs @@ -10,6 +10,12 @@ namespace Umbraco.Core.Persistence.Repositories { TItem Get(string alias); IEnumerable> Move(TItem moving, EntityContainer container); + + /// + /// Returns the content types that are direct compositions of the content type + /// + /// The content type id + /// IEnumerable GetTypesDirectlyComposedOf(int id); /// diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs index d1737d495a..3f1ea3116e 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs @@ -415,7 +415,7 @@ AND umbracoNode.id <> @id", ClearScheduledPublishing(entity); } - //track any property types that are changing variation + //track any content type/property types that are changing variation which will require content updates var propertyTypeVariationChanges = new Dictionary(); // insert or update properties @@ -426,7 +426,9 @@ AND umbracoNode.id <> @id", if (!ctVariationChanging) { if (propertyType.IsPropertyDirty("Variations")) + { propertyTypeVariationChanges[propertyType.Id] = propertyType.Variations; + } } else { @@ -481,7 +483,7 @@ AND umbracoNode.id <> @id", .From() .WhereIn(x => x.Id, propertyTypeVariationChanges.Keys)); - foreach(var f in from) + foreach (var f in from) { changes[f.Key] = (propertyTypeVariationChanges[f.Key], (ContentVariation)f.Value); } @@ -490,6 +492,7 @@ AND umbracoNode.id <> @id", MoveVariantData(changes); } + // deal with orphan properties: those that were in a deleted tab, // and have not been re-mapped to another tab or to 'generic properties' if (orphanPropertyTypeIds != null) @@ -845,8 +848,11 @@ AND umbracoNode.id <> @id", } } + /// public IEnumerable GetTypesDirectlyComposedOf(int id) { + //fixme - this will probably be more efficient to simply load all content types and do the calculation, see GetWhereCompositionIsUsedInContentTypes + var sql = Sql() .SelectAll() .From() diff --git a/src/Umbraco.Core/Services/ContentTypeServiceExtensions.cs b/src/Umbraco.Core/Services/ContentTypeServiceExtensions.cs index 9e82213aa5..66b3982b49 100644 --- a/src/Umbraco.Core/Services/ContentTypeServiceExtensions.cs +++ b/src/Umbraco.Core/Services/ContentTypeServiceExtensions.cs @@ -109,23 +109,7 @@ namespace Umbraco.Core.Services return new ContentTypeAvailableCompositionsResults(ancestors, result); } - /// - /// Returns the list of content types the composition is used in - /// - /// - /// - /// - /// - internal static IEnumerable GetWhereCompositionIsUsedInContentTypes(this IContentTypeService ctService, - IContentTypeComposition source, - IContentTypeComposition[] allContentTypes) - { - - var sourceId = source != null ? source.Id : 0; - - // find which content types are using this composition - return allContentTypes.Where(x => x.ContentTypeComposition.Any(y => y.Id == sourceId)).ToArray(); - } + private static IContentTypeComposition[] GetAncestors(IContentTypeComposition ctype, IContentTypeComposition[] allContentTypes) { diff --git a/src/Umbraco.Core/Services/Implement/ContentTypeServiceBaseOfTRepositoryTItemTService.cs b/src/Umbraco.Core/Services/Implement/ContentTypeServiceBaseOfTRepositoryTItemTService.cs index a3bc87315b..a114f415cc 100644 --- a/src/Umbraco.Core/Services/Implement/ContentTypeServiceBaseOfTRepositoryTItemTService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentTypeServiceBaseOfTRepositoryTItemTService.cs @@ -346,6 +346,9 @@ namespace Umbraco.Core.Services.Implement public IEnumerable GetComposedOf(int id) { + //fixme: this is essentially the same as ContentTypeServiceExtensions.GetWhereCompositionIsUsedInContentTypes which loads + // all content types to figure this out, this instead makes quite a few queries so should be replaced + using (var scope = ScopeProvider.CreateScope(autoComplete: true)) { scope.ReadLock(ReadLockIds); diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs index 2d0471b9bd..f25382d557 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs @@ -266,6 +266,119 @@ namespace Umbraco.Tests.Services Assert.AreEqual("hello world", doc.GetValue("title", "en-US")); } + [Test] + public void Change_Property_Type_From_Variant_Invariant_On_A_Composition() + { + //create content type with a property type that varies by culture + var contentType = MockedContentTypes.CreateBasicContentType(); + contentType.Variations = ContentVariation.Culture; + var contentCollection = new PropertyTypeCollection(true); + contentCollection.Add(new PropertyType("test", ValueStorageType.Ntext) + { + Alias = "title", + Name = "Title", + Description = "", + Mandatory = false, + SortOrder = 1, + DataTypeId = -88, + Variations = ContentVariation.Culture + }); + contentType.PropertyGroups.Add(new PropertyGroup(contentCollection) { Name = "Content", SortOrder = 1 }); + ServiceContext.ContentTypeService.Save(contentType); + + //compose this from the other one + var contentType2 = MockedContentTypes.CreateBasicContentType("test"); + contentType2.Variations = ContentVariation.Culture; + contentType2.AddContentType(contentType); + ServiceContext.ContentTypeService.Save(contentType2); + + //create some content of this content type + IContent doc = MockedContent.CreateBasicContent(contentType); + doc.SetCultureName("Home", "en-US"); + doc.SetValue("title", "hello world", "en-US"); + ServiceContext.ContentService.Save(doc); + + IContent doc2 = MockedContent.CreateBasicContent(contentType2); + doc2.SetCultureName("Home", "en-US"); + doc2.SetValue("title", "hello world", "en-US"); + ServiceContext.ContentService.Save(doc2); + + //change the property type to be invariant + contentType.PropertyTypes.First().Variations = ContentVariation.Nothing; + ServiceContext.ContentTypeService.Save(contentType); + doc = ServiceContext.ContentService.GetById(doc.Id); //re-get + doc2 = ServiceContext.ContentService.GetById(doc2.Id); //re-get + + Assert.AreEqual("hello world", doc.GetValue("title")); + Assert.AreEqual("hello world", doc2.GetValue("title")); + + //change back property type to be variant + contentType.PropertyTypes.First().Variations = ContentVariation.Culture; + ServiceContext.ContentTypeService.Save(contentType); + doc = ServiceContext.ContentService.GetById(doc.Id); //re-get + doc2 = ServiceContext.ContentService.GetById(doc2.Id); //re-get + + Assert.AreEqual("hello world", doc.GetValue("title", "en-US")); + Assert.AreEqual("hello world", doc2.GetValue("title", "en-US")); + } + + [Test] + public void Change_Content_Type_From_Variant_Invariant_On_A_Composition() + { + //create content type with a property type that varies by culture + var contentType = MockedContentTypes.CreateBasicContentType(); + contentType.Variations = ContentVariation.Culture; + var contentCollection = new PropertyTypeCollection(true); + contentCollection.Add(new PropertyType("test", ValueStorageType.Ntext) + { + Alias = "title", + Name = "Title", + Description = "", + Mandatory = false, + SortOrder = 1, + DataTypeId = -88, + Variations = ContentVariation.Culture + }); + contentType.PropertyGroups.Add(new PropertyGroup(contentCollection) { Name = "Content", SortOrder = 1 }); + ServiceContext.ContentTypeService.Save(contentType); + + //compose this from the other one + var contentType2 = MockedContentTypes.CreateBasicContentType("test"); + contentType2.Variations = ContentVariation.Culture; + contentType2.AddContentType(contentType); + ServiceContext.ContentTypeService.Save(contentType2); + + //create some content of this content type + IContent doc = MockedContent.CreateBasicContent(contentType); + doc.SetCultureName("Home", "en-US"); + doc.SetValue("title", "hello world", "en-US"); + ServiceContext.ContentService.Save(doc); + + IContent doc2 = MockedContent.CreateBasicContent(contentType2); + doc2.SetCultureName("Home", "en-US"); + doc2.SetValue("title", "hello world", "en-US"); + ServiceContext.ContentService.Save(doc2); + + //change the content type to be invariant + contentType.Variations = ContentVariation.Nothing; + ServiceContext.ContentTypeService.Save(contentType); + doc = ServiceContext.ContentService.GetById(doc.Id); //re-get + doc2 = ServiceContext.ContentService.GetById(doc2.Id); //re-get + + Assert.AreEqual("hello world", doc.GetValue("title")); + Assert.AreEqual("hello world", doc2.GetValue("title")); + + //change back content type to be variant + contentType.Variations = ContentVariation.Culture; + ServiceContext.ContentTypeService.Save(contentType); + doc = ServiceContext.ContentService.GetById(doc.Id); //re-get + doc2 = ServiceContext.ContentService.GetById(doc2.Id); //re-get + + //this will be null because the doc type was changed back to variant but it's property types don't get changed back + Assert.IsNull(doc.GetValue("title", "en-US")); + Assert.IsNull(doc2.GetValue("title", "en-US")); + } + [Test] public void Deleting_Media_Type_With_Hierarchy_Of_Media_Items_Moves_Orphaned_Media_To_Recycle_Bin() { diff --git a/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs b/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs index fa1aaf7345..22d86631ca 100644 --- a/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs +++ b/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs @@ -161,7 +161,7 @@ namespace Umbraco.Web.Editors throw new ArgumentOutOfRangeException("The entity type was not a content type"); } - var contentTypesWhereCompositionIsUsed = Services.ContentTypeService.GetWhereCompositionIsUsedInContentTypes(source, allContentTypes); + var contentTypesWhereCompositionIsUsed = source.GetWhereCompositionIsUsedInContentTypes(allContentTypes); return contentTypesWhereCompositionIsUsed .Select(x => Mapper.Map(x)) .Select(x =>