From 8521dfb0d7320489d8216cd83709e28b95e4bfae Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 13 Jul 2015 15:29:00 +0200 Subject: [PATCH] Fixes issue that you could not remove a property group, fixes issue that the inherited flag during mapping wasn't set on a standalone parent property group, updated/created tests to support. --- .../Mapping/ContentTypeModelMappingTests.cs | 147 +++++++++++++++++- .../Models/Mapping/ContentTypeModelMapper.cs | 21 ++- .../Mapping/PropertyTypeGroupResolver.cs | 10 +- 3 files changed, 166 insertions(+), 12 deletions(-) diff --git a/src/Umbraco.Tests/Models/Mapping/ContentTypeModelMappingTests.cs b/src/Umbraco.Tests/Models/Mapping/ContentTypeModelMappingTests.cs index f59a167e07..ad47bafb80 100644 --- a/src/Umbraco.Tests/Models/Mapping/ContentTypeModelMappingTests.cs +++ b/src/Umbraco.Tests/Models/Mapping/ContentTypeModelMappingTests.cs @@ -69,7 +69,7 @@ namespace Umbraco.Tests.Models.Mapping } [Test] - public void ContentTypeDisplay_To_PropertyType() + public void PropertyTypeDisplay_To_PropertyType() { // setup the mocks to return the data we want to test against... @@ -207,6 +207,33 @@ namespace Umbraco.Tests.Models.Mapping } } + [Test] + public void ContentTypeDisplay_With_Composition_To_IContentType() + { + //Arrange + + // setup the mocks to return the data we want to test against... + + _dataTypeService.Setup(x => x.GetDataTypeDefinitionById(It.IsAny())) + .Returns(Mock.Of( + definition => + definition.Id == 555 + && definition.PropertyEditorAlias == "myPropertyType" + && definition.DatabaseType == DataTypeDatabaseType.Nvarchar)); + + + var display = CreateCompositionContentTypeDisplay(); + + //Act + + var result = Mapper.Map(display); + + //Assert + + //TODO: Now we need to assert all of the more complicated parts + Assert.AreEqual(display.Groups.Count(x => x.Inherited == false), result.PropertyGroups.Count); + } + [Test] public void IContentType_To_ContentTypeDisplay() { @@ -306,6 +333,15 @@ namespace Umbraco.Tests.Models.Mapping }); MockedContentTypes.EnsureAllIds(ctMain, 8888); var ctChild1 = MockedContentTypes.CreateSimpleContentType("child1", "Child 1", ctMain, true); + ctChild1.AddPropertyType(new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "someProperty", + Name = "Some Property", + Description = "", + Mandatory = false, + SortOrder = 1, + DataTypeDefinitionId = -88 + }, "Another tab"); MockedContentTypes.EnsureAllIds(ctChild1, 7777); var contentType = MockedContentTypes.CreateSimpleContentType("child2", "Child 2", ctChild1, true, "CustomGroup"); //not assigned to tab @@ -349,7 +385,7 @@ namespace Umbraco.Tests.Models.Mapping Assert.AreEqual(allPropertyIdsMapped.Count(), allSourcePropertyIds.Count()); Assert.IsTrue(allPropertyIdsMapped.ContainsAll(allSourcePropertyIds)); - Assert.AreEqual(1, result.Groups.Count(x => x.ParentTabContentTypes.Any())); + Assert.AreEqual(2, result.Groups.Count(x => x.ParentTabContentTypes.Any())); Assert.IsTrue(result.Groups.SelectMany(x => x.ParentTabContentTypes).ContainsAll(new[] {ctMain.Id, ctChild1.Id})); Assert.AreEqual(contentType.AllowedTemplates.Count(), result.AllowedTemplates.Count()); @@ -437,6 +473,113 @@ namespace Umbraco.Tests.Models.Mapping } } } + }; + } + + private ContentTypeDisplay CreateCompositionContentTypeDisplay() + { + return new ContentTypeDisplay + { + Alias = "test", + AllowAsRoot = true, + AllowedTemplates = new List + { + new EntityBasic + { + Id = 555, + Alias = "template1", + Name = "Template1" + }, + new EntityBasic + { + Id = 556, + Alias = "template2", + Name = "Template2" + } + }, + AllowedContentTypes = new[] { 666, 667 }, + AvailableCompositeContentTypes = new List(), + DefaultTemplate = new EntityBasic() { Alias = "test" }, + Description = "hello world", + Icon = "tree-icon", + Id = 1234, + Key = new Guid("8A60656B-3866-46AB-824A-48AE85083070"), + Name = "My content type", + Path = "-1,1234", + ParentId = -1, + Thumbnail = "tree-thumb", + IsContainer = true, + Groups = new List() + { + new PropertyGroupDisplay + { + Id = 987, + Name = "Tab 1", + ParentGroupId = -1, + SortOrder = 0, + Inherited = false, + Properties = new List + { + new PropertyTypeDisplay + { + Alias = "property1", + Description = "this is property 1", + Inherited = false, + Label = "Property 1", + Validation = new PropertyTypeValidation + { + Mandatory = false, + Pattern = "" + }, + Editor = "myPropertyType", + Value = "value 1", + //View = ??? - isn't this the same as editor? + Config = new Dictionary + { + {"item1", "value1"}, + {"item2", "value2"} + }, + SortOrder = 0, + DataTypeId = 555, + View = "blah" + } + } + }, + new PropertyGroupDisplay + { + Id = 894, + Name = "Tab 2", + ParentGroupId = -1, + SortOrder = 0, + Inherited = true, + Properties = new List + { + new PropertyTypeDisplay + { + Alias = "parentProperty", + Description = "this is a property from the parent", + Inherited = true, + Label = "Parent property", + Validation = new PropertyTypeValidation + { + Mandatory = false, + Pattern = "" + }, + Editor = "myPropertyType", + Value = "parent value", + //View = ??? - isn't this the same as editor? + Config = new Dictionary + { + {"item1", "value1"} + }, + SortOrder = 0, + DataTypeId = 555, + View = "blah" + } + } + + } + } }; } diff --git a/src/Umbraco.Web/Models/Mapping/ContentTypeModelMapper.cs b/src/Umbraco.Web/Models/Mapping/ContentTypeModelMapper.cs index 529327d694..4df94c27ce 100644 --- a/src/Umbraco.Web/Models/Mapping/ContentTypeModelMapper.cs +++ b/src/Umbraco.Web/Models/Mapping/ContentTypeModelMapper.cs @@ -49,15 +49,17 @@ namespace Umbraco.Web.Models.Mapping //mapped in aftermap .ForMember(dto => dto.AllowedContentTypes, expression => expression.Ignore()) .ForMember(dto => dto.AllowedTemplates, expression => expression.Ignore()) - - + //ignore, we'll do this in after map .ForMember(dto => dto.PropertyGroups, expression => expression.Ignore()) .AfterMap((source, dest) => { var addedProperties = new List(); - //get all properties from groups that are not generic properties (-666 id) - foreach (var groupDisplay in source.Groups.Where(x => x.Id != -666)) + + //get all properties from groups that are not generic properties or inhertied (-666 id) + var selfNonGenericGroups = source.Groups.Where(x => x.Inherited == false && x.Id != -666).ToArray(); + + foreach (var groupDisplay in selfNonGenericGroups) { //use underlying logic to add the property group which should wire most things up for us dest.AddPropertyGroup(groupDisplay.Name); @@ -70,12 +72,19 @@ namespace Umbraco.Web.Models.Mapping dest.AddPropertyType(Mapper.Map(propertyTypeDisplay), groupDisplay.Name); addedProperties.Add(propertyTypeDisplay.Alias); } - + } + + //Groups to remove + var groupsToRemove = dest.PropertyGroups.Select(x => x.Name).Except(selfNonGenericGroups.Select(x => x.Name)).ToArray(); + foreach (var toRemove in groupsToRemove) + { + dest.RemovePropertyGroup(toRemove); } //add generic properties var genericProperties = source.Groups.FirstOrDefault(x => x.Id == -666); - if(genericProperties != null){ + if(genericProperties != null) + { foreach (var propertyTypeDisplay in genericProperties.Properties.Where(x => x.Inherited == false)) { dest.AddPropertyType(Mapper.Map(propertyTypeDisplay)); diff --git a/src/Umbraco.Web/Models/Mapping/PropertyTypeGroupResolver.cs b/src/Umbraco.Web/Models/Mapping/PropertyTypeGroupResolver.cs index ce3b9b082f..ae300a824e 100644 --- a/src/Umbraco.Web/Models/Mapping/PropertyTypeGroupResolver.cs +++ b/src/Umbraco.Web/Models/Mapping/PropertyTypeGroupResolver.cs @@ -58,6 +58,10 @@ namespace Umbraco.Web.Models.Mapping { Id = tab.Id, Inherited = false, Name = tab.Name, SortOrder = tab.SortOrder, ContentTypeId = source.Id }; + + if (tab.ParentId.HasValue) + group.ParentGroupId = tab.ParentId.Value; + group.Properties = MapProperties(tab.PropertyTypes, source, tab.Id, false); groups.Add(tab.Id, group); } @@ -73,7 +77,7 @@ namespace Umbraco.Web.Models.Mapping var group = new PropertyGroupDisplay() { - Id = tab.Id, Inherited = false, Name = tab.Name, SortOrder = tab.SortOrder, ContentTypeId = composition.Id, + Id = tab.Id, Inherited = true, Name = tab.Name, SortOrder = tab.SortOrder, ContentTypeId = composition.Id, ParentTabContentTypes = new[] {composition.Id}, ParentTabContentTypeNames = new[] {composition.Name} }; @@ -87,9 +91,7 @@ namespace Umbraco.Web.Models.Mapping //process generic properties assigned to this content item (without a group) - //NOTE: -666 is just a thing that is checked for on the front-end... I'm not a fan of this for the mapping - // since this is just for front-end, this could probably be updated to be -666 in the controller which is associated - // with giving the front-end it's data + //NOTE: -666 is just a thing that is checked during mapping the other direction, it's a 'special' id var entityGenericProperties = source.PropertyTypes.Where(x => x.PropertyGroupId == null); genericProperties.AddRange(MapProperties(entityGenericProperties, source, -666, false));