From 4b4f571d143c584637d9da255d22a77910cb3582 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20Knippers?= Date: Fri, 23 Aug 2019 15:18:53 +0200 Subject: [PATCH 01/10] Support for Segments in ContentTypeRepositoryBase --- .../Implement/ContentTypeRepositoryBase.cs | 187 +++++++----------- 1 file changed, 75 insertions(+), 112 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs index f2efb03ba4..4428f35de7 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs @@ -1,4 +1,5 @@ -using System; + +using System; using System.Collections.Generic; using System.Data; using System.Globalization; @@ -410,24 +411,8 @@ AND umbracoNode.id <> @id", // note: this only deals with *local* property types, we're dealing w/compositions later below foreach (var propertyType in entity.PropertyTypes) { - if (contentTypeVariationChanging) - { - // content type is changing - switch (newContentTypeVariation) - { - case ContentVariation.Nothing: // changing to Nothing - // all property types must change to Nothing - propertyType.Variations = ContentVariation.Nothing; - break; - case ContentVariation.Culture: // changing to Culture - // all property types can remain Nothing - break; - case ContentVariation.CultureAndSegment: - case ContentVariation.Segment: - default: - throw new NotSupportedException(); // TODO: Support this - } - } + // Update property variations + propertyType.Variations = newContentTypeVariation & propertyType.Variations; // then, track each property individually if (propertyType.IsPropertyDirty("Variations")) @@ -455,23 +440,17 @@ AND umbracoNode.id <> @id", // via composition, with their original variations (ie not filtered by this // content type variations - we need this true value to make decisions. + propertyTypeVariationChanges = propertyTypeVariationChanges ?? new Dictionary(); + foreach (var propertyType in ((ContentTypeCompositionBase)entity).RawComposedPropertyTypes) { - if (propertyType.VariesBySegment() || newContentTypeVariation.VariesBySegment()) - throw new NotSupportedException(); // TODO: support this + if (propertyType.Variations == ContentVariation.Nothing) continue; - if (propertyType.Variations == ContentVariation.Culture) - { - if (propertyTypeVariationChanges == null) - propertyTypeVariationChanges = new Dictionary(); + var target = newContentTypeVariation & propertyType.Variations; - // if content type moves to Culture, property type becomes Culture here again - // if content type moves to Nothing, property type becomes Nothing here - if (newContentTypeVariation == ContentVariation.Culture) - propertyTypeVariationChanges[propertyType.Id] = (ContentVariation.Nothing, ContentVariation.Culture); - else if (newContentTypeVariation == ContentVariation.Nothing) - propertyTypeVariationChanges[propertyType.Id] = (ContentVariation.Culture, ContentVariation.Nothing); - } + // if content type moves to a different variant, property type becomes Culture here again + // if content type moves to Nothing, property type becomes Nothing here + propertyTypeVariationChanges[propertyType.Id] = (propertyType.Variations, target); } } @@ -512,7 +491,7 @@ AND umbracoNode.id <> @id", var impacted = GetImpactedContentTypes(entity, all); // if some property types have actually changed, move their variant data - if (propertyTypeVariationChanges != null) + if (propertyTypeVariationChanges?.Count > 0) MovePropertyTypeVariantData(propertyTypeVariationChanges, impacted); // deal with orphan properties: those that were in a deleted tab, @@ -661,27 +640,24 @@ AND umbracoNode.id <> @id", var impactedL = impacted.Select(x => x.Id).ToList(); //Group by the "To" variation so we can bulk update in the correct batches - foreach (var grouping in propertyTypeChanges.GroupBy(x => x.Value.ToVariation)) + foreach (var grouping in propertyTypeChanges.GroupBy(x => x.Value)) { var propertyTypeIds = grouping.Select(x => x.Key).ToList(); - var toVariation = grouping.Key; + var (FromVariation, ToVariation) = grouping.Key; - switch (toVariation) + if(!FromVariation.HasFlag(ContentVariation.Culture) && + ToVariation.HasFlag(ContentVariation.Culture)) { - case ContentVariation.Culture: - CopyPropertyData(null, defaultLanguageId, propertyTypeIds, impactedL); - CopyTagData(null, defaultLanguageId, propertyTypeIds, impactedL); - RenormalizeDocumentEditedFlags(propertyTypeIds, impactedL); - break; - case ContentVariation.Nothing: - CopyPropertyData(defaultLanguageId, null, propertyTypeIds, impactedL); - CopyTagData(defaultLanguageId, null, propertyTypeIds, impactedL); - RenormalizeDocumentEditedFlags(propertyTypeIds, impactedL); - break; - case ContentVariation.CultureAndSegment: - case ContentVariation.Segment: - default: - throw new NotSupportedException(); // TODO: Support this + CopyPropertyData(null, defaultLanguageId, propertyTypeIds, impactedL); + CopyTagData(null, defaultLanguageId, propertyTypeIds, impactedL); + RenormalizeDocumentEditedFlags(propertyTypeIds, impactedL); + } + else if (FromVariation.HasFlag(ContentVariation.Culture) && + !ToVariation.HasFlag(ContentVariation.Culture)) + { + CopyPropertyData(defaultLanguageId, null, propertyTypeIds, impactedL); + CopyTagData(defaultLanguageId, null, propertyTypeIds, impactedL); + RenormalizeDocumentEditedFlags(propertyTypeIds, impactedL); } } } @@ -693,78 +669,65 @@ AND umbracoNode.id <> @id", { var defaultLanguageId = GetDefaultLanguageId(); - switch (toVariation) + var cultureIsNotEnabled = !fromVariation.HasFlag(ContentVariation.Culture); + var cultureWillBeEnabled = toVariation.HasFlag(ContentVariation.Culture); + + if(cultureIsNotEnabled && cultureWillBeEnabled) { - case ContentVariation.Culture: + //move the names + //first clear out any existing names that might already exists under the default lang + //there's 2x tables to update - //move the names - //first clear out any existing names that might already exists under the default lang - //there's 2x tables to update + //clear out the versionCultureVariation table + var sqlSelect = Sql().Select(x => x.Id) + .From() + .InnerJoin().On(x => x.Id, x => x.VersionId) + .InnerJoin().On(x => x.NodeId, x => x.NodeId) + .Where(x => x.ContentTypeId == contentType.Id) + .Where(x => x.LanguageId == defaultLanguageId); + var sqlDelete = Sql() + .Delete() + .WhereIn(x => x.Id, sqlSelect); - //clear out the versionCultureVariation table - var sqlSelect = Sql().Select(x => x.Id) - .From() - .InnerJoin().On(x => x.Id, x => x.VersionId) - .InnerJoin().On(x => x.NodeId, x => x.NodeId) - .Where(x => x.ContentTypeId == contentType.Id) - .Where(x => x.LanguageId == defaultLanguageId); - var sqlDelete = Sql() - .Delete() - .WhereIn(x => x.Id, sqlSelect); + Database.Execute(sqlDelete); - Database.Execute(sqlDelete); + //clear out the documentCultureVariation table + sqlSelect = Sql().Select(x => x.Id) + .From() + .InnerJoin().On(x => x.NodeId, x => x.NodeId) + .Where(x => x.ContentTypeId == contentType.Id) + .Where(x => x.LanguageId == defaultLanguageId); + sqlDelete = Sql() + .Delete() + .WhereIn(x => x.Id, sqlSelect); - //clear out the documentCultureVariation table - sqlSelect = Sql().Select(x => x.Id) - .From() - .InnerJoin().On(x => x.NodeId, x => x.NodeId) - .Where(x => x.ContentTypeId == contentType.Id) - .Where(x => x.LanguageId == defaultLanguageId); - sqlDelete = Sql() - .Delete() - .WhereIn(x => x.Id, sqlSelect); + Database.Execute(sqlDelete); - Database.Execute(sqlDelete); + //now we need to insert names into these 2 tables based on the invariant data - //now we need to insert names into these 2 tables based on the invariant data + //insert rows into the versionCultureVariationDto table based on the data from contentVersionDto for the default lang + var cols = Sql().Columns(x => x.VersionId, x => x.Name, x => x.UpdateUserId, x => x.UpdateDate, x => x.LanguageId); + sqlSelect = Sql().Select(x => x.Id, x => x.Text, x => x.UserId, x => x.VersionDate) + .Append($", {defaultLanguageId}") //default language ID + .From() + .InnerJoin().On(x => x.NodeId, x => x.NodeId) + .Where(x => x.ContentTypeId == contentType.Id); + var sqlInsert = Sql($"INSERT INTO {ContentVersionCultureVariationDto.TableName} ({cols})").Append(sqlSelect); - //insert rows into the versionCultureVariationDto table based on the data from contentVersionDto for the default lang - var cols = Sql().Columns(x => x.VersionId, x => x.Name, x => x.UpdateUserId, x => x.UpdateDate, x => x.LanguageId); - sqlSelect = Sql().Select(x => x.Id, x => x.Text, x => x.UserId, x => x.VersionDate) - .Append($", {defaultLanguageId}") //default language ID - .From() - .InnerJoin().On(x => x.NodeId, x => x.NodeId) - .Where(x => x.ContentTypeId == contentType.Id); - var sqlInsert = Sql($"INSERT INTO {ContentVersionCultureVariationDto.TableName} ({cols})").Append(sqlSelect); + Database.Execute(sqlInsert); - Database.Execute(sqlInsert); + //insert rows into the documentCultureVariation table + cols = Sql().Columns(x => x.NodeId, x => x.Edited, x => x.Published, x => x.Name, x => x.Available, x => x.LanguageId); + sqlSelect = Sql().Select(x => x.NodeId, x => x.Edited, x => x.Published) + .AndSelect(x => x.Text) + .Append($", 1, {defaultLanguageId}") //make Available + default language ID + .From() + .InnerJoin().On(x => x.NodeId, x => x.NodeId) + .InnerJoin().On(x => x.NodeId, x => x.NodeId) + .Where(x => x.ContentTypeId == contentType.Id); + sqlInsert = Sql($"INSERT INTO {DocumentCultureVariationDto.TableName} ({cols})").Append(sqlSelect); - //insert rows into the documentCultureVariation table - cols = Sql().Columns(x => x.NodeId, x => x.Edited, x => x.Published, x => x.Name, x => x.Available, x => x.LanguageId); - sqlSelect = Sql().Select(x => x.NodeId, x => x.Edited, x => x.Published) - .AndSelect(x => x.Text) - .Append($", 1, {defaultLanguageId}") //make Available + default language ID - .From() - .InnerJoin().On(x => x.NodeId, x => x.NodeId) - .InnerJoin().On(x => x.NodeId, x => x.NodeId) - .Where(x => x.ContentTypeId == contentType.Id); - sqlInsert = Sql($"INSERT INTO {DocumentCultureVariationDto.TableName} ({cols})").Append(sqlSelect); - - Database.Execute(sqlInsert); - - break; - case ContentVariation.Nothing: - - //we don't need to move the names! this is because we always keep the invariant names with the name of the default language. - - //however, if we were to move names, we could do this: BUT this doesn't work with SQLCE, for that we'd have to update row by row :( - // if we want these SQL statements back, look into GIT history - - break; - case ContentVariation.CultureAndSegment: - case ContentVariation.Segment: - default: - throw new NotSupportedException(); // TODO: Support this + Database.Execute(sqlInsert); } } From fc717d1e3735b80d94a31a655c0c5cd69d4ea220 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20Knippers?= Date: Thu, 3 Oct 2019 14:01:00 +0200 Subject: [PATCH 02/10] Code comments --- .../Implement/ContentTypeRepositoryBase.cs | 39 +++++++++++++------ 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs index 4428f35de7..ab5c698b86 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs @@ -412,6 +412,11 @@ AND umbracoNode.id <> @id", foreach (var propertyType in entity.PropertyTypes) { // Update property variations + + // Determine target variation of the property type. + // The property is only considered culture variant when the base content type is also culture variant. + // The property is only considered segment variant when the base content type is also segment variant. + // Example: Culture variant content type with a Culture+Segment variant property type will become ContentVariation.Culture propertyType.Variations = newContentTypeVariation & propertyType.Variations; // then, track each property individually @@ -442,15 +447,17 @@ AND umbracoNode.id <> @id", propertyTypeVariationChanges = propertyTypeVariationChanges ?? new Dictionary(); - foreach (var propertyType in ((ContentTypeCompositionBase)entity).RawComposedPropertyTypes) + foreach (var composedPropertyType in ((ContentTypeCompositionBase)entity).RawComposedPropertyTypes) { - if (propertyType.Variations == ContentVariation.Nothing) continue; + if (composedPropertyType.Variations == ContentVariation.Nothing) continue; - var target = newContentTypeVariation & propertyType.Variations; + // Determine target variation of the composed property type. + // The composed property is only considered culture variant when the base content type is also culture variant. + // The composed property is only considered segment variant when the base content type is also segment variant. + // Example: Culture variant content type with a Culture+Segment variant property type will become ContentVariation.Culture + var target = newContentTypeVariation & composedPropertyType.Variations; - // if content type moves to a different variant, property type becomes Culture here again - // if content type moves to Nothing, property type becomes Nothing here - propertyTypeVariationChanges[propertyType.Id] = (propertyType.Variations, target); + propertyTypeVariationChanges[composedPropertyType.Id] = (composedPropertyType.Variations, target); } } @@ -645,16 +652,19 @@ AND umbracoNode.id <> @id", var propertyTypeIds = grouping.Select(x => x.Key).ToList(); var (FromVariation, ToVariation) = grouping.Key; - if(!FromVariation.HasFlag(ContentVariation.Culture) && - ToVariation.HasFlag(ContentVariation.Culture)) + var fromCultureEnabled = FromVariation.HasFlag(ContentVariation.Culture); + var toCultureEnabled = ToVariation.HasFlag(ContentVariation.Culture); + + if (!fromCultureEnabled && toCultureEnabled) { + // Culture has been enabled CopyPropertyData(null, defaultLanguageId, propertyTypeIds, impactedL); CopyTagData(null, defaultLanguageId, propertyTypeIds, impactedL); RenormalizeDocumentEditedFlags(propertyTypeIds, impactedL); } - else if (FromVariation.HasFlag(ContentVariation.Culture) && - !ToVariation.HasFlag(ContentVariation.Culture)) + else if (fromCultureEnabled && !toCultureEnabled) { + // Culture has been disabled CopyPropertyData(defaultLanguageId, null, propertyTypeIds, impactedL); CopyTagData(defaultLanguageId, null, propertyTypeIds, impactedL); RenormalizeDocumentEditedFlags(propertyTypeIds, impactedL); @@ -672,7 +682,7 @@ AND umbracoNode.id <> @id", var cultureIsNotEnabled = !fromVariation.HasFlag(ContentVariation.Culture); var cultureWillBeEnabled = toVariation.HasFlag(ContentVariation.Culture); - if(cultureIsNotEnabled && cultureWillBeEnabled) + if (cultureIsNotEnabled && cultureWillBeEnabled) { //move the names //first clear out any existing names that might already exists under the default lang @@ -729,6 +739,13 @@ AND umbracoNode.id <> @id", Database.Execute(sqlInsert); } + else + { + //we don't need to move the names! this is because we always keep the invariant names with the name of the default language. + + //however, if we were to move names, we could do this: BUT this doesn't work with SQLCE, for that we'd have to update row by row :( + // if we want these SQL statements back, look into GIT history + } } /// From d76ff8b9994f1ff040d88cbd8d3e579c4fc34623 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20Knippers?= Date: Thu, 3 Oct 2019 16:40:44 +0200 Subject: [PATCH 03/10] Updated a couple unit tests for segments --- .../ContentTypeServiceVariantsTests.cs | 113 ++++++++++++------ 1 file changed, 79 insertions(+), 34 deletions(-) diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs index 956de186be..6e55c16e64 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs @@ -107,12 +107,27 @@ namespace Umbraco.Tests.Services } } - [Test] - public void Change_Content_Type_Variation_Clears_Redirects() + [TestCase(ContentVariation.Nothing, ContentVariation.Nothing, false)] + [TestCase(ContentVariation.Nothing, ContentVariation.Culture, true)] + [TestCase(ContentVariation.Nothing, ContentVariation.CultureAndSegment, true)] + [TestCase(ContentVariation.Nothing, ContentVariation.Segment, true)] + [TestCase(ContentVariation.Culture, ContentVariation.Nothing, true)] + [TestCase(ContentVariation.Culture, ContentVariation.Culture, false)] + [TestCase(ContentVariation.Culture, ContentVariation.Segment, true)] + [TestCase(ContentVariation.Culture, ContentVariation.CultureAndSegment, true)] + [TestCase(ContentVariation.Segment, ContentVariation.Nothing, true)] + [TestCase(ContentVariation.Segment, ContentVariation.Culture, true)] + [TestCase(ContentVariation.Segment, ContentVariation.Segment, false)] + [TestCase(ContentVariation.Segment, ContentVariation.CultureAndSegment, true)] + [TestCase(ContentVariation.CultureAndSegment, ContentVariation.Nothing, true)] + [TestCase(ContentVariation.CultureAndSegment, ContentVariation.Culture, true)] + [TestCase(ContentVariation.CultureAndSegment, ContentVariation.Segment, true)] + [TestCase(ContentVariation.CultureAndSegment, ContentVariation.CultureAndSegment, false)] + public void Change_Content_Type_Variation_Clears_Redirects(ContentVariation startingContentTypeVariation, ContentVariation changedContentTypeVariation, bool shouldUrlRedirectsBeCleared) { - var contentType = MockedContentTypes.CreateBasicContentType(); - contentType.Variations = ContentVariation.Nothing; - var properties = CreatePropertyCollection(("title", ContentVariation.Nothing)); + var contentType = MockedContentTypes.CreateBasicContentType(); + contentType.Variations = startingContentTypeVariation; + var properties = CreatePropertyCollection(("title", startingContentTypeVariation)); contentType.PropertyGroups.Add(new PropertyGroup(properties) { Name = "Content" }); ServiceContext.ContentTypeService.Save(contentType); var contentType2 = MockedContentTypes.CreateBasicContentType("test"); @@ -121,6 +136,11 @@ namespace Umbraco.Tests.Services //create some content of this content type IContent doc = MockedContent.CreateBasicContent(contentType); doc.Name = "Hello1"; + if(startingContentTypeVariation.HasFlag(ContentVariation.Culture)) + { + doc.SetCultureName(doc.Name, "en-US"); + } + ServiceContext.ContentService.Save(doc); IContent doc2 = MockedContent.CreateBasicContent(contentType2); @@ -129,24 +149,27 @@ namespace Umbraco.Tests.Services ServiceContext.RedirectUrlService.Register("hello/world", doc.Key); ServiceContext.RedirectUrlService.Register("hello2/world2", doc2.Key); + // These 2 assertions should probably be moved to a test for the Register() method? Assert.AreEqual(1, ServiceContext.RedirectUrlService.GetContentRedirectUrls(doc.Key).Count()); Assert.AreEqual(1, ServiceContext.RedirectUrlService.GetContentRedirectUrls(doc2.Key).Count()); //change variation - contentType.Variations = ContentVariation.Culture; + contentType.Variations = changedContentTypeVariation; ServiceContext.ContentTypeService.Save(contentType); - - Assert.AreEqual(0, ServiceContext.RedirectUrlService.GetContentRedirectUrls(doc.Key).Count()); + var expectedRedirectUrlCount = shouldUrlRedirectsBeCleared ? 0 : 1; + Assert.AreEqual(expectedRedirectUrlCount, ServiceContext.RedirectUrlService.GetContentRedirectUrls(doc.Key).Count()); Assert.AreEqual(1, ServiceContext.RedirectUrlService.GetContentRedirectUrls(doc2.Key).Count()); - } - [Test] - public void Change_Content_Type_From_Invariant_Variant() - { + [TestCase(ContentVariation.Nothing, ContentVariation.Culture)] + [TestCase(ContentVariation.Nothing, ContentVariation.CultureAndSegment)] + [TestCase(ContentVariation.Segment, ContentVariation.Culture)] + [TestCase(ContentVariation.Segment, ContentVariation.CultureAndSegment)] + public void Change_Content_Type_From_No_Culture_To_Culture(ContentVariation from, ContentVariation to) + { var contentType = MockedContentTypes.CreateBasicContentType(); - contentType.Variations = ContentVariation.Nothing; - var properties = CreatePropertyCollection(("title", ContentVariation.Nothing)); + contentType.Variations = from; + var properties = CreatePropertyCollection(("title", from)); contentType.PropertyGroups.Add(new PropertyGroup(properties) { Name = "Content" }); ServiceContext.ContentTypeService.Save(contentType); @@ -161,12 +184,12 @@ namespace Umbraco.Tests.Services Assert.AreEqual("Hello1", doc.Name); Assert.AreEqual("hello world", doc.GetValue("title")); Assert.IsTrue(doc.Edited); - Assert.IsFalse (doc.IsCultureEdited("en-US")); + Assert.IsFalse(doc.IsCultureEdited("en-US")); //change the content type to be variant, we will also update the name here to detect the copy changes doc.Name = "Hello2"; ServiceContext.ContentService.Save(doc); - contentType.Variations = ContentVariation.Culture; + contentType.Variations = to; ServiceContext.ContentTypeService.Save(contentType); doc = ServiceContext.ContentService.GetById(doc.Id); //re-get @@ -178,7 +201,7 @@ namespace Umbraco.Tests.Services //change back property type to be invariant, we will also update the name here to detect the copy changes doc.SetCultureName("Hello3", "en-US"); ServiceContext.ContentService.Save(doc); - contentType.Variations = ContentVariation.Nothing; + contentType.Variations = from; ServiceContext.ContentTypeService.Save(contentType); doc = ServiceContext.ContentService.GetById(doc.Id); //re-get @@ -188,12 +211,15 @@ namespace Umbraco.Tests.Services Assert.IsFalse(doc.IsCultureEdited("en-US")); } - [Test] - public void Change_Content_Type_From_Variant_Invariant() + [TestCase(ContentVariation.Culture, ContentVariation.Nothing)] + [TestCase(ContentVariation.Culture, ContentVariation.Segment)] + [TestCase(ContentVariation.CultureAndSegment, ContentVariation.Nothing)] + [TestCase(ContentVariation.CultureAndSegment, ContentVariation.Segment)] + public void Change_Content_Type_From_Culture_To_No_Culture(ContentVariation startingContentTypeVariation, ContentVariation changeContentTypeVariationTo) { var contentType = MockedContentTypes.CreateBasicContentType(); - contentType.Variations = ContentVariation.Culture; - var properties = CreatePropertyCollection(("title", ContentVariation.Culture)); + contentType.Variations = startingContentTypeVariation; + var properties = CreatePropertyCollection(("title", startingContentTypeVariation)); contentType.PropertyGroups.Add(new PropertyGroup(properties) { Name = "Content" }); ServiceContext.ContentTypeService.Save(contentType); @@ -212,7 +238,7 @@ namespace Umbraco.Tests.Services //change the content type to be invariant, we will also update the name here to detect the copy changes doc.SetCultureName("Hello2", "en-US"); ServiceContext.ContentService.Save(doc); - contentType.Variations = ContentVariation.Nothing; + contentType.Variations = changeContentTypeVariationTo; ServiceContext.ContentTypeService.Save(contentType); doc = ServiceContext.ContentService.GetById(doc.Id); //re-get @@ -224,19 +250,20 @@ namespace Umbraco.Tests.Services //change back property type to be variant, we will also update the name here to detect the copy changes doc.Name = "Hello3"; ServiceContext.ContentService.Save(doc); - contentType.Variations = ContentVariation.Culture; + contentType.Variations = startingContentTypeVariation; ServiceContext.ContentTypeService.Save(contentType); doc = ServiceContext.ContentService.GetById(doc.Id); //re-get //at this stage all property types were switched to invariant so even though the variant value //exists it will not be returned because the property type is invariant, //so this check proves that null will be returned + Assert.AreEqual("Hello3", doc.Name); Assert.IsNull(doc.GetValue("title", "en-US")); Assert.IsTrue(doc.Edited); Assert.IsTrue(doc.IsCultureEdited("en-US")); // this is true because the name change is copied to the default language //we can now switch the property type to be variant and the value can be returned again - contentType.PropertyTypes.First().Variations = ContentVariation.Culture; + contentType.PropertyTypes.First().Variations = startingContentTypeVariation; ServiceContext.ContentTypeService.Save(contentType); doc = ServiceContext.ContentService.GetById(doc.Id); //re-get @@ -244,24 +271,42 @@ namespace Umbraco.Tests.Services Assert.AreEqual("hello world", doc.GetValue("title", "en-US")); Assert.IsTrue(doc.Edited); Assert.IsTrue(doc.IsCultureEdited("en-US")); - } - - [Test] - public void Change_Property_Type_From_To_Variant_On_Invariant_Content_Type() + [TestCase(ContentVariation.Nothing, ContentVariation.Nothing, true)] + [TestCase(ContentVariation.Nothing, ContentVariation.Culture, false)] + [TestCase(ContentVariation.Nothing, ContentVariation.Segment, false)] + [TestCase(ContentVariation.Nothing, ContentVariation.CultureAndSegment, false)] + [TestCase(ContentVariation.Culture, ContentVariation.Nothing, true)] + [TestCase(ContentVariation.Culture, ContentVariation.Culture, true)] + [TestCase(ContentVariation.Culture, ContentVariation.Segment, false)] + [TestCase(ContentVariation.Culture, ContentVariation.CultureAndSegment, false)] + [TestCase(ContentVariation.Segment, ContentVariation.Nothing, true)] + [TestCase(ContentVariation.Segment, ContentVariation.Culture, false)] + [TestCase(ContentVariation.Segment, ContentVariation.Segment, true)] + [TestCase(ContentVariation.Segment, ContentVariation.CultureAndSegment, false)] + [TestCase(ContentVariation.CultureAndSegment, ContentVariation.Nothing, true)] + [TestCase(ContentVariation.CultureAndSegment, ContentVariation.Culture, true)] + [TestCase(ContentVariation.CultureAndSegment, ContentVariation.Segment, true)] + [TestCase(ContentVariation.CultureAndSegment, ContentVariation.CultureAndSegment, true)] + public void Verify_If_Property_Type_Variation_Is_Allowed(ContentVariation contentTypeVariation, ContentVariation propertyTypeVariation, bool isAllowed) { var contentType = MockedContentTypes.CreateBasicContentType(); - contentType.Variations = ContentVariation.Nothing; - var properties = CreatePropertyCollection(("title", ContentVariation.Nothing)); + contentType.Variations = contentTypeVariation; + var properties = CreatePropertyCollection(("title", contentTypeVariation)); contentType.PropertyGroups.Add(new PropertyGroup(properties) { Name = "Content" }); ServiceContext.ContentTypeService.Save(contentType); - //change the property type to be variant - contentType.PropertyTypes.First().Variations = ContentVariation.Culture; + contentType.PropertyTypes.First().Variations = propertyTypeVariation; - //Cannot change a property type to be variant if the content type itself is not variant - Assert.Throws(() => ServiceContext.ContentTypeService.Save(contentType)); + // property may only vary by segment if the content type itself is also varied by segment + // property may only vary by culture if the content type itself is also varied by culture + // properties without variance should always be allowed + + if (isAllowed) + Assert.DoesNotThrow(() => ServiceContext.ContentTypeService.Save(contentType)); + else + Assert.Throws(() => ServiceContext.ContentTypeService.Save(contentType)); } [Test] From c7d00b2c0083d47d37d6706c626d613101b97282 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20Knippers?= Date: Fri, 4 Oct 2019 08:53:22 +0200 Subject: [PATCH 04/10] Corrected ValidateVariations method Triggered by a failing unit test that was updated for segments --- .../Implement/ContentTypeRepositoryBase.cs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs index ab5c698b86..2d70e89aa0 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs @@ -513,20 +513,18 @@ AND umbracoNode.id <> @id", /// /// Ensures that no property types are flagged for a variance that is not supported by the content type itself /// - /// + /// The entity for which the property types will be validated private void ValidateVariations(IContentTypeComposition entity) { - //if the entity does not vary at all, then the property cannot have a variance value greater than it - if (entity.Variations == ContentVariation.Nothing) + foreach (var prop in entity.PropertyTypes) { - foreach (var prop in entity.PropertyTypes) - { - if (prop.IsPropertyDirty(nameof(prop.Variations)) && prop.Variations > entity.Variations) - throw new InvalidOperationException($"The property {prop.Alias} cannot have variations of {prop.Variations} with the content type variations of {entity.Variations}"); - } - + // The variation of a property is only allowed if all its variation flags + // are also set on the entity itself. It cannot set anything that is not also set by the content type. + // For example, when entity.Variations is set to Culture a property cannot be set to Segment. + var isValid = entity.Variations.HasFlag(prop.Variations); + if (!isValid) + throw new InvalidOperationException($"The property {prop.Alias} cannot have variations of {prop.Variations} with the content type variations of {entity.Variations}"); } - } private IEnumerable GetImpactedContentTypes(IContentTypeComposition contentType, IEnumerable all) From 2e79a515c9ac83bee24b5354fe3bfb89d4d0819d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20Knippers?= Date: Fri, 4 Oct 2019 12:07:05 +0200 Subject: [PATCH 05/10] Updated tests --- .../ContentTypeServiceVariantsTests.cs | 82 ++++++++++++++++++- 1 file changed, 79 insertions(+), 3 deletions(-) diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs index 2a94ba971e..48bcf3c928 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs @@ -124,9 +124,7 @@ namespace Umbraco.Tests.Services public void Change_Content_Type_Variation_Clears_Redirects(ContentVariation startingContentTypeVariation, ContentVariation changedContentTypeVariation, bool shouldUrlRedirectsBeCleared) { var contentType = MockedContentTypes.CreateBasicContentType(); - contentType.Variations = startingContentTypeVariation; - var properties = CreatePropertyCollection(("title", startingContentTypeVariation)); - contentType.PropertyGroups.Add(new PropertyGroup(properties) { Name = "Content" }); + contentType.Variations = startingContentTypeVariation; ServiceContext.ContentTypeService.Save(contentType); var contentType2 = MockedContentTypes.CreateBasicContentType("test"); ServiceContext.ContentTypeService.Save(contentType2); @@ -271,6 +269,84 @@ namespace Umbraco.Tests.Services Assert.IsTrue(doc.IsCultureEdited("en-US")); } + [TestCase(ContentVariation.Nothing, ContentVariation.Nothing)] + [TestCase(ContentVariation.Nothing, ContentVariation.Culture)] + [TestCase(ContentVariation.Nothing, ContentVariation.Segment)] + [TestCase(ContentVariation.Nothing, ContentVariation.CultureAndSegment)] + [TestCase(ContentVariation.Culture, ContentVariation.Nothing)] + [TestCase(ContentVariation.Culture, ContentVariation.Culture)] + [TestCase(ContentVariation.Culture, ContentVariation.Segment)] + [TestCase(ContentVariation.Culture, ContentVariation.CultureAndSegment)] + [TestCase(ContentVariation.Segment, ContentVariation.Nothing)] + [TestCase(ContentVariation.Segment, ContentVariation.Culture)] + [TestCase(ContentVariation.Segment, ContentVariation.Segment)] + [TestCase(ContentVariation.Segment, ContentVariation.CultureAndSegment)] + [TestCase(ContentVariation.CultureAndSegment, ContentVariation.Nothing)] + [TestCase(ContentVariation.CultureAndSegment, ContentVariation.Culture)] + [TestCase(ContentVariation.CultureAndSegment, ContentVariation.Segment)] + [TestCase(ContentVariation.CultureAndSegment, ContentVariation.CultureAndSegment)] + public void Preserve_Content_Name_After_Content_Type_Variation_Change(ContentVariation contentTypeVariationFrom, ContentVariation contentTypeVariationTo) + { + var contentType = MockedContentTypes.CreateBasicContentType(); + contentType.Variations = contentTypeVariationFrom; + ServiceContext.ContentTypeService.Save(contentType); + + var invariantContentName = "Content Invariant"; + + var defaultCultureContentName = "Content en-US"; + var defaultCulture = "en-US"; + + var nlContentName = "Content nl-NL"; + var nlCulture = "nl-NL"; + + ServiceContext.LocalizationService.Save(new Language(nlCulture)); + + var includeCultureNames = contentType.Variations.HasFlag(ContentVariation.Culture); + + // Create some content of this content type + IContent doc = MockedContent.CreateBasicContent(contentType); + + doc.Name = invariantContentName; + if (includeCultureNames) + { + Assert.DoesNotThrow(() => doc.SetCultureName(defaultCultureContentName, defaultCulture)); + Assert.DoesNotThrow(() => doc.SetCultureName(nlContentName, nlCulture)); + } else + { + Assert.Throws(() => doc.SetCultureName(defaultCultureContentName, defaultCulture)); + Assert.Throws(() => doc.SetCultureName(nlContentName, nlCulture)); + } + + ServiceContext.ContentService.Save(doc); + doc = ServiceContext.ContentService.GetById(doc.Id); + + AssertAll(); + + // Change variation + contentType.Variations = contentTypeVariationTo; + ServiceContext.ContentService.Save(doc); + doc = ServiceContext.ContentService.GetById(doc.Id); + + AssertAll(); + + void AssertAll() + { + if (includeCultureNames) + { + // Invariant content name is not preserved when content type is set to culture + Assert.AreEqual(defaultCultureContentName, doc.Name); + Assert.AreEqual(doc.Name, doc.GetCultureName(defaultCulture)); + Assert.AreEqual(nlContentName, doc.GetCultureName(nlCulture)); + } + else + { + Assert.AreEqual(invariantContentName, doc.Name); + Assert.AreEqual(null, doc.GetCultureName(defaultCulture)); + Assert.AreEqual(null, doc.GetCultureName(nlCulture)); + } + } + } + [TestCase(ContentVariation.Nothing, ContentVariation.Nothing, true)] [TestCase(ContentVariation.Nothing, ContentVariation.Culture, false)] [TestCase(ContentVariation.Nothing, ContentVariation.Segment, false)] From 3858bd40adf5fc6c8181c1c8518d41b3e01fd824 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20Knippers?= Date: Fri, 4 Oct 2019 14:28:13 +0200 Subject: [PATCH 06/10] Moved property type variation auto correct. --- .../Repositories/Implement/ContentTypeRepository.cs | 10 ++++++++++ .../Implement/ContentTypeRepositoryBase.cs | 10 +--------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepository.cs index 359b967dab..7c7f39fa54 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepository.cs @@ -287,6 +287,16 @@ namespace Umbraco.Core.Persistence.Repositories.Implement entity.SortOrder = maxSortOrder + 1; } + // Update property variations based on the content type variation + foreach (var propertyType in entity.PropertyTypes) + { + // Determine variation for the property type. + // The property is only considered culture variant when the base content type is also culture variant. + // The property is only considered segment variant when the base content type is also segment variant. + // Example: Culture variant content type with a Culture+Segment variant property type will become ContentVariation.Culture + propertyType.Variations = entity.Variations & propertyType.Variations; + } + PersistUpdatedBaseContentType(entity); PersistTemplates(entity, true); diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs index 2d70e89aa0..dec5805cf2 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs @@ -411,15 +411,7 @@ AND umbracoNode.id <> @id", // note: this only deals with *local* property types, we're dealing w/compositions later below foreach (var propertyType in entity.PropertyTypes) { - // Update property variations - - // Determine target variation of the property type. - // The property is only considered culture variant when the base content type is also culture variant. - // The property is only considered segment variant when the base content type is also segment variant. - // Example: Culture variant content type with a Culture+Segment variant property type will become ContentVariation.Culture - propertyType.Variations = newContentTypeVariation & propertyType.Variations; - - // then, track each property individually + // track each property individually if (propertyType.IsPropertyDirty("Variations")) { // allocate the list only when needed From 4a3b3c41517541a62bc791f0a00e07e613e770f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20Knippers?= Date: Fri, 4 Oct 2019 14:19:51 +0200 Subject: [PATCH 07/10] Property type correction test --- .../ContentTypeServiceVariantsTests.cs | 53 +++++++++---------- 1 file changed, 25 insertions(+), 28 deletions(-) diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs index 48bcf3c928..7affe2de3f 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs @@ -347,40 +347,37 @@ namespace Umbraco.Tests.Services } } - [TestCase(ContentVariation.Nothing, ContentVariation.Nothing, true)] - [TestCase(ContentVariation.Nothing, ContentVariation.Culture, false)] - [TestCase(ContentVariation.Nothing, ContentVariation.Segment, false)] - [TestCase(ContentVariation.Nothing, ContentVariation.CultureAndSegment, false)] - [TestCase(ContentVariation.Culture, ContentVariation.Nothing, true)] - [TestCase(ContentVariation.Culture, ContentVariation.Culture, true)] - [TestCase(ContentVariation.Culture, ContentVariation.Segment, false)] - [TestCase(ContentVariation.Culture, ContentVariation.CultureAndSegment, false)] - [TestCase(ContentVariation.Segment, ContentVariation.Nothing, true)] - [TestCase(ContentVariation.Segment, ContentVariation.Culture, false)] - [TestCase(ContentVariation.Segment, ContentVariation.Segment, true)] - [TestCase(ContentVariation.Segment, ContentVariation.CultureAndSegment, false)] - [TestCase(ContentVariation.CultureAndSegment, ContentVariation.Nothing, true)] - [TestCase(ContentVariation.CultureAndSegment, ContentVariation.Culture, true)] - [TestCase(ContentVariation.CultureAndSegment, ContentVariation.Segment, true)] - [TestCase(ContentVariation.CultureAndSegment, ContentVariation.CultureAndSegment, true)] - public void Verify_If_Property_Type_Variation_Is_Allowed(ContentVariation contentTypeVariation, ContentVariation propertyTypeVariation, bool isAllowed) + [TestCase(ContentVariation.Nothing, ContentVariation.Nothing)] + [TestCase(ContentVariation.Nothing, ContentVariation.Culture)] + [TestCase(ContentVariation.Nothing, ContentVariation.Segment)] + [TestCase(ContentVariation.Nothing, ContentVariation.CultureAndSegment)] + [TestCase(ContentVariation.Culture, ContentVariation.Nothing)] + [TestCase(ContentVariation.Culture, ContentVariation.Culture)] + [TestCase(ContentVariation.Culture, ContentVariation.Segment)] + [TestCase(ContentVariation.Culture, ContentVariation.CultureAndSegment)] + [TestCase(ContentVariation.Segment, ContentVariation.Nothing)] + [TestCase(ContentVariation.Segment, ContentVariation.Culture)] + [TestCase(ContentVariation.Segment, ContentVariation.Segment)] + [TestCase(ContentVariation.Segment, ContentVariation.CultureAndSegment)] + [TestCase(ContentVariation.CultureAndSegment, ContentVariation.Nothing)] + [TestCase(ContentVariation.CultureAndSegment, ContentVariation.Culture)] + [TestCase(ContentVariation.CultureAndSegment, ContentVariation.Segment)] + [TestCase(ContentVariation.CultureAndSegment, ContentVariation.CultureAndSegment)] + public void Verify_If_Property_Type_Variation_Is_Correctly_Corrected_When_Content_Type_Is_Updated(ContentVariation contentTypeVariation, ContentVariation propertyTypeVariation) { var contentType = MockedContentTypes.CreateBasicContentType(); + + // We test an updated content type so it has to be saved first. + ServiceContext.ContentTypeService.Save(contentType); + + // Update it contentType.Variations = contentTypeVariation; - var properties = CreatePropertyCollection(("title", contentTypeVariation)); + var properties = CreatePropertyCollection(("title", propertyTypeVariation)); contentType.PropertyGroups.Add(new PropertyGroup(properties) { Name = "Content" }); ServiceContext.ContentTypeService.Save(contentType); - contentType.PropertyTypes.First().Variations = propertyTypeVariation; - - // property may only vary by segment if the content type itself is also varied by segment - // property may only vary by culture if the content type itself is also varied by culture - // properties without variance should always be allowed - - if (isAllowed) - Assert.DoesNotThrow(() => ServiceContext.ContentTypeService.Save(contentType)); - else - Assert.Throws(() => ServiceContext.ContentTypeService.Save(contentType)); + // Check if property type variations have been updated correctly + Assert.AreEqual(properties.First().Variations, contentTypeVariation & propertyTypeVariation); } [Test] From 2b14408693d03f3f909d27188b03700a42977b8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20Knippers?= Date: Tue, 15 Oct 2019 11:17:03 +0200 Subject: [PATCH 08/10] Revert "Moved property type variation auto correct." This reverts commit 3858bd40adf5fc6c8181c1c8518d41b3e01fd824. --- .../Repositories/Implement/ContentTypeRepository.cs | 10 ---------- .../Implement/ContentTypeRepositoryBase.cs | 10 +++++++++- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepository.cs index 7c7f39fa54..359b967dab 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepository.cs @@ -287,16 +287,6 @@ namespace Umbraco.Core.Persistence.Repositories.Implement entity.SortOrder = maxSortOrder + 1; } - // Update property variations based on the content type variation - foreach (var propertyType in entity.PropertyTypes) - { - // Determine variation for the property type. - // The property is only considered culture variant when the base content type is also culture variant. - // The property is only considered segment variant when the base content type is also segment variant. - // Example: Culture variant content type with a Culture+Segment variant property type will become ContentVariation.Culture - propertyType.Variations = entity.Variations & propertyType.Variations; - } - PersistUpdatedBaseContentType(entity); PersistTemplates(entity, true); diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs index dec5805cf2..2d70e89aa0 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs @@ -411,7 +411,15 @@ AND umbracoNode.id <> @id", // note: this only deals with *local* property types, we're dealing w/compositions later below foreach (var propertyType in entity.PropertyTypes) { - // track each property individually + // Update property variations + + // Determine target variation of the property type. + // The property is only considered culture variant when the base content type is also culture variant. + // The property is only considered segment variant when the base content type is also segment variant. + // Example: Culture variant content type with a Culture+Segment variant property type will become ContentVariation.Culture + propertyType.Variations = newContentTypeVariation & propertyType.Variations; + + // then, track each property individually if (propertyType.IsPropertyDirty("Variations")) { // allocate the list only when needed From c8a8d2e23508c3049c1fc7bd1594b0147856c47f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20Knippers?= Date: Tue, 15 Oct 2019 11:21:12 +0200 Subject: [PATCH 09/10] Moved property type variation "auto correction" into separate method and perform before validation. --- .../Implement/ContentTypeRepositoryBase.cs | 30 +++++++++++++------ 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs index 2d70e89aa0..6385482686 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs @@ -219,6 +219,7 @@ AND umbracoNode.nodeObjectType = @objectType", protected void PersistUpdatedBaseContentType(IContentTypeComposition entity) { + CorrectPropertyTypeVariations(entity); ValidateVariations(entity); var dto = ContentTypeFactory.BuildContentTypeDto(entity); @@ -411,15 +412,7 @@ AND umbracoNode.id <> @id", // note: this only deals with *local* property types, we're dealing w/compositions later below foreach (var propertyType in entity.PropertyTypes) { - // Update property variations - - // Determine target variation of the property type. - // The property is only considered culture variant when the base content type is also culture variant. - // The property is only considered segment variant when the base content type is also segment variant. - // Example: Culture variant content type with a Culture+Segment variant property type will become ContentVariation.Culture - propertyType.Variations = newContentTypeVariation & propertyType.Variations; - - // then, track each property individually + // track each property individually if (propertyType.IsPropertyDirty("Variations")) { // allocate the list only when needed @@ -510,6 +503,25 @@ AND umbracoNode.id <> @id", CommonRepository.ClearCache(); // always } + /// + /// Corrects the property type variations for the given entity + /// to make sure the property type variation is compatible with the + /// variation set on the entity itself. + /// + /// Entity to correct properties for + private void CorrectPropertyTypeVariations(IContentTypeComposition entity) + { + // Update property variations based on the content type variation + foreach (var propertyType in entity.PropertyTypes) + { + // Determine variation for the property type. + // The property is only considered culture variant when the base content type is also culture variant. + // The property is only considered segment variant when the base content type is also segment variant. + // Example: Culture variant content type with a Culture+Segment variant property type will become ContentVariation.Culture + propertyType.Variations = entity.Variations & propertyType.Variations; + } + } + /// /// Ensures that no property types are flagged for a variance that is not supported by the content type itself /// From 4495a4ab5a9a6f716c3a8c91bb526134de8173fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20Knippers?= Date: Tue, 15 Oct 2019 11:50:16 +0200 Subject: [PATCH 10/10] Updated various test cases to include the Segment flag. The original test method has been changed to a method with parameters for the ContentVariation "from" and "to" options and [TestCase] attributes are applied to run the different combinations. --- .../ContentTypeServiceVariantsTests.cs | 97 +++++++++++-------- 1 file changed, 59 insertions(+), 38 deletions(-) diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs index 7affe2de3f..0fe05f385f 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs @@ -380,12 +380,16 @@ namespace Umbraco.Tests.Services Assert.AreEqual(properties.First().Variations, contentTypeVariation & propertyTypeVariation); } - [Test] - public void Change_Property_Type_From_Invariant_Variant() + [TestCase(ContentVariation.Nothing, ContentVariation.Culture)] + [TestCase(ContentVariation.Nothing, ContentVariation.CultureAndSegment)] + [TestCase(ContentVariation.Segment, ContentVariation.Culture)] + [TestCase(ContentVariation.Segment, ContentVariation.CultureAndSegment)] + public void Change_Property_Type_From_Invariant_Variant(ContentVariation invariant, ContentVariation variant) { var contentType = MockedContentTypes.CreateBasicContentType(); - contentType.Variations = ContentVariation.Culture; - var properties = CreatePropertyCollection(("title", ContentVariation.Nothing)); + // content type supports all variations + contentType.Variations = ContentVariation.Culture | ContentVariation.Segment; + var properties = CreatePropertyCollection(("title", invariant)); contentType.PropertyGroups.Add(new PropertyGroup(properties) { Name = "Content" }); ServiceContext.ContentTypeService.Save(contentType); @@ -401,7 +405,7 @@ namespace Umbraco.Tests.Services Assert.IsTrue(doc.Edited); //change the property type to be variant - contentType.PropertyTypes.First().Variations = ContentVariation.Culture; + contentType.PropertyTypes.First().Variations = variant; ServiceContext.ContentTypeService.Save(contentType); doc = ServiceContext.ContentService.GetById(doc.Id); //re-get @@ -410,7 +414,7 @@ namespace Umbraco.Tests.Services Assert.IsTrue(doc.Edited); //change back property type to be invariant - contentType.PropertyTypes.First().Variations = ContentVariation.Nothing; + contentType.PropertyTypes.First().Variations = invariant; ServiceContext.ContentTypeService.Save(contentType); doc = ServiceContext.ContentService.GetById(doc.Id); //re-get @@ -419,13 +423,17 @@ namespace Umbraco.Tests.Services Assert.IsTrue(doc.Edited); } - [Test] - public void Change_Property_Type_From_Variant_Invariant() + [TestCase(ContentVariation.Culture, ContentVariation.Nothing)] + [TestCase(ContentVariation.Culture, ContentVariation.Segment)] + [TestCase(ContentVariation.CultureAndSegment, ContentVariation.Nothing)] + [TestCase(ContentVariation.CultureAndSegment, ContentVariation.Segment)] + public void Change_Property_Type_From_Variant_Invariant(ContentVariation variant, ContentVariation invariant) { //create content type with a property type that varies by culture var contentType = MockedContentTypes.CreateBasicContentType(); - contentType.Variations = ContentVariation.Culture; - var properties = CreatePropertyCollection(("title", ContentVariation.Culture)); + // content type supports all variations + contentType.Variations = ContentVariation.Culture | ContentVariation.Segment; + var properties = CreatePropertyCollection(("title", variant)); contentType.PropertyGroups.Add(new PropertyGroup(properties) { Name = "Content" }); ServiceContext.ContentTypeService.Save(contentType); @@ -438,33 +446,37 @@ namespace Umbraco.Tests.Services Assert.AreEqual("hello world", doc.GetValue("title", "en-US")); //change the property type to be invariant - contentType.PropertyTypes.First().Variations = ContentVariation.Nothing; + contentType.PropertyTypes.First().Variations = invariant; ServiceContext.ContentTypeService.Save(contentType); doc = ServiceContext.ContentService.GetById(doc.Id); //re-get Assert.AreEqual("hello world", doc.GetValue("title")); //change back property type to be variant - contentType.PropertyTypes.First().Variations = ContentVariation.Culture; + contentType.PropertyTypes.First().Variations = variant; ServiceContext.ContentTypeService.Save(contentType); doc = ServiceContext.ContentService.GetById(doc.Id); //re-get Assert.AreEqual("hello world", doc.GetValue("title", "en-US")); } - [Test] - public void Change_Property_Type_From_Variant_Invariant_On_A_Composition() + [TestCase(ContentVariation.Culture, ContentVariation.Nothing)] + [TestCase(ContentVariation.Culture, ContentVariation.Segment)] + [TestCase(ContentVariation.CultureAndSegment, ContentVariation.Nothing)] + [TestCase(ContentVariation.CultureAndSegment, ContentVariation.Segment)] + public void Change_Property_Type_From_Variant_Invariant_On_A_Composition(ContentVariation variant, ContentVariation invariant) { //create content type with a property type that varies by culture var contentType = MockedContentTypes.CreateBasicContentType(); - contentType.Variations = ContentVariation.Culture; - var properties = CreatePropertyCollection(("title", ContentVariation.Culture)); + // content type supports all variations + contentType.Variations = ContentVariation.Culture | ContentVariation.Segment; + var properties = CreatePropertyCollection(("title", variant)); contentType.PropertyGroups.Add(new PropertyGroup(properties) { Name = "Content" }); ServiceContext.ContentTypeService.Save(contentType); //compose this from the other one var contentType2 = MockedContentTypes.CreateBasicContentType("test"); - contentType2.Variations = ContentVariation.Culture; + contentType2.Variations = contentType.Variations; contentType2.AddContentType(contentType); ServiceContext.ContentTypeService.Save(contentType2); @@ -480,7 +492,7 @@ namespace Umbraco.Tests.Services ServiceContext.ContentService.Save(doc2); //change the property type to be invariant - contentType.PropertyTypes.First().Variations = ContentVariation.Nothing; + contentType.PropertyTypes.First().Variations = invariant; ServiceContext.ContentTypeService.Save(contentType); doc = ServiceContext.ContentService.GetById(doc.Id); //re-get doc2 = ServiceContext.ContentService.GetById(doc2.Id); //re-get @@ -489,7 +501,7 @@ namespace Umbraco.Tests.Services Assert.AreEqual("hello world", doc2.GetValue("title")); //change back property type to be variant - contentType.PropertyTypes.First().Variations = ContentVariation.Culture; + contentType.PropertyTypes.First().Variations = variant; ServiceContext.ContentTypeService.Save(contentType); doc = ServiceContext.ContentService.GetById(doc.Id); //re-get doc2 = ServiceContext.ContentService.GetById(doc2.Id); //re-get @@ -498,19 +510,22 @@ namespace Umbraco.Tests.Services Assert.AreEqual("hello world", doc2.GetValue("title", "en-US")); } - [Test] - public void Change_Content_Type_From_Variant_Invariant_On_A_Composition() + [TestCase(ContentVariation.Culture, ContentVariation.Nothing)] + [TestCase(ContentVariation.Culture, ContentVariation.Segment)] + [TestCase(ContentVariation.CultureAndSegment, ContentVariation.Nothing)] + [TestCase(ContentVariation.CultureAndSegment, ContentVariation.Segment)] + public void Change_Content_Type_From_Variant_Invariant_On_A_Composition(ContentVariation variant, ContentVariation invariant) { //create content type with a property type that varies by culture var contentType = MockedContentTypes.CreateBasicContentType(); - contentType.Variations = ContentVariation.Culture; + contentType.Variations = variant; var properties = CreatePropertyCollection(("title", ContentVariation.Culture)); contentType.PropertyGroups.Add(new PropertyGroup(properties) { Name = "Content" }); ServiceContext.ContentTypeService.Save(contentType); //compose this from the other one var contentType2 = MockedContentTypes.CreateBasicContentType("test"); - contentType2.Variations = ContentVariation.Culture; + contentType2.Variations = contentType.Variations; contentType2.AddContentType(contentType); ServiceContext.ContentTypeService.Save(contentType2); @@ -526,7 +541,7 @@ namespace Umbraco.Tests.Services ServiceContext.ContentService.Save(doc2); //change the content type to be invariant - contentType.Variations = ContentVariation.Nothing; + contentType.Variations = invariant; ServiceContext.ContentTypeService.Save(contentType); doc = ServiceContext.ContentService.GetById(doc.Id); //re-get doc2 = ServiceContext.ContentService.GetById(doc2.Id); //re-get @@ -535,7 +550,7 @@ namespace Umbraco.Tests.Services Assert.AreEqual("hello world", doc2.GetValue("title")); //change back content type to be variant - contentType.Variations = ContentVariation.Culture; + contentType.Variations = variant; ServiceContext.ContentTypeService.Save(contentType); doc = ServiceContext.ContentService.GetById(doc.Id); //re-get doc2 = ServiceContext.ContentService.GetById(doc2.Id); //re-get @@ -811,22 +826,25 @@ namespace Umbraco.Tests.Services "{'properties':{'value1':[{'culture':'en','seg':'','val':'v1en'},{'culture':'fr','seg':'','val':'v1fr'}],'value2':[{'culture':'en','seg':'','val':'v2'}]},'cultureData':"); } - [Test] - public void Change_Property_Variations_From_Variant_To_Invariant_And_Ensure_Edited_Values_Are_Renormalized() + [TestCase(ContentVariation.Culture, ContentVariation.Nothing)] + [TestCase(ContentVariation.Culture, ContentVariation.Segment)] + [TestCase(ContentVariation.CultureAndSegment, ContentVariation.Nothing)] + [TestCase(ContentVariation.CultureAndSegment, ContentVariation.Segment)] + public void Change_Property_Variations_From_Variant_To_Invariant_And_Ensure_Edited_Values_Are_Renormalized(ContentVariation variant, ContentVariation invariant) { // one simple content type, variant, with both variant and invariant properties // can change an invariant property to variant and back CreateFrenchAndEnglishLangs(); - var contentType = CreateContentType(ContentVariation.Culture); + var contentType = CreateContentType(ContentVariation.Culture | ContentVariation.Segment); - var properties = CreatePropertyCollection(("value1", ContentVariation.Culture)); + var properties = CreatePropertyCollection(("value1", variant)); contentType.PropertyGroups.Add(new PropertyGroup(properties) { Name = "Content" }); ServiceContext.ContentTypeService.Save(contentType); - var document = (IContent)new Content("document", -1, contentType); + IContent document = new Content("document", -1, contentType); document.SetCultureName("doc1en", "en"); document.SetCultureName("doc1fr", "fr"); document.SetValue("value1", "v1en-init", "en"); @@ -855,7 +873,7 @@ namespace Umbraco.Tests.Services Assert.IsTrue(document.Edited); // switch property type to Invariant - contentType.PropertyTypes.First(x => x.Alias == "value1").Variations = ContentVariation.Nothing; + contentType.PropertyTypes.First(x => x.Alias == "value1").Variations = invariant; ServiceContext.ContentTypeService.Save(contentType); //This is going to have to re-normalize the "Edited" flag document = ServiceContext.ContentService.GetById(document.Id); @@ -882,7 +900,7 @@ namespace Umbraco.Tests.Services Assert.IsFalse(document.Edited); // switch property back to Culture - contentType.PropertyTypes.First(x => x.Alias == "value1").Variations = ContentVariation.Culture; + contentType.PropertyTypes.First(x => x.Alias == "value1").Variations = variant; ServiceContext.ContentTypeService.Save(contentType); document = ServiceContext.ContentService.GetById(document.Id); @@ -911,17 +929,20 @@ namespace Umbraco.Tests.Services Assert.IsFalse(document.Edited); } - [Test] - public void Change_Property_Variations_From_Invariant_To_Variant_And_Ensure_Edited_Values_Are_Renormalized() + [TestCase(ContentVariation.Nothing, ContentVariation.Culture)] + [TestCase(ContentVariation.Nothing, ContentVariation.CultureAndSegment)] + [TestCase(ContentVariation.Segment, ContentVariation.Culture)] + [TestCase(ContentVariation.Segment, ContentVariation.CultureAndSegment)] + public void Change_Property_Variations_From_Invariant_To_Variant_And_Ensure_Edited_Values_Are_Renormalized(ContentVariation invariant, ContentVariation variant) { // one simple content type, variant, with both variant and invariant properties // can change an invariant property to variant and back CreateFrenchAndEnglishLangs(); - var contentType = CreateContentType(ContentVariation.Culture); + var contentType = CreateContentType(ContentVariation.Culture | ContentVariation.Segment); - var properties = CreatePropertyCollection(("value1", ContentVariation.Nothing)); + var properties = CreatePropertyCollection(("value1", invariant)); contentType.PropertyGroups.Add(new PropertyGroup(properties) { Name = "Content" }); ServiceContext.ContentTypeService.Save(contentType); @@ -951,7 +972,7 @@ namespace Umbraco.Tests.Services Assert.IsTrue(document.Edited); // switch property type to Culture - contentType.PropertyTypes.First(x => x.Alias == "value1").Variations = ContentVariation.Culture; + contentType.PropertyTypes.First(x => x.Alias == "value1").Variations = variant; ServiceContext.ContentTypeService.Save(contentType); //This is going to have to re-normalize the "Edited" flag document = ServiceContext.ContentService.GetById(document.Id); @@ -976,7 +997,7 @@ namespace Umbraco.Tests.Services Assert.IsFalse(document.Edited); // switch property back to Invariant - contentType.PropertyTypes.First(x => x.Alias == "value1").Variations = ContentVariation.Nothing; + contentType.PropertyTypes.First(x => x.Alias == "value1").Variations = invariant; ServiceContext.ContentTypeService.Save(contentType); document = ServiceContext.ContentService.GetById(document.Id);