diff --git a/src/Umbraco.Core/Models/ContentTypeBase.cs b/src/Umbraco.Core/Models/ContentTypeBase.cs index 7218a2421d..a0305d2cfb 100644 --- a/src/Umbraco.Core/Models/ContentTypeBase.cs +++ b/src/Umbraco.Core/Models/ContentTypeBase.cs @@ -487,11 +487,8 @@ namespace Umbraco.Core.Models var oldPropertyGroup = PropertyGroups.FirstOrDefault(x => x.PropertyTypes.Any(y => y.Alias == propertyTypeAlias)); - // reset PropertyGroupId, which will be re-evaluated when the content type - // is saved - what is important is group.PropertyTypes - see code in - // ContentTypeBaseRepository.PersistUpdatedBaseContentType - propertyType.PropertyGroupId = new Lazy(() => default(int)); - propertyType.ResetDirtyProperties(); // PropertyGroupId must not be dirty + // set new group + propertyType.PropertyGroupId = newPropertyGroup == null ? null : new Lazy(() => newPropertyGroup.Id, false); // remove from old group, if any - add to new group, if any if (oldPropertyGroup != null) @@ -540,7 +537,7 @@ namespace Umbraco.Core.Models // re-assign the group's properties to no group foreach (var property in group.PropertyTypes) { - property.PropertyGroupId = new Lazy(() => 0); + property.PropertyGroupId = null; _propertyTypes.Add(property); } diff --git a/src/Umbraco.Core/Models/PropertyType.cs b/src/Umbraco.Core/Models/PropertyType.cs index 7f22b65c8c..0649801a0c 100644 --- a/src/Umbraco.Core/Models/PropertyType.cs +++ b/src/Umbraco.Core/Models/PropertyType.cs @@ -221,8 +221,9 @@ namespace Umbraco.Core.Models } /// - /// Gets or Sets the PropertyGroup's Id for which this PropertyType belongs + /// Gets or sets the identifier of the PropertyGroup this PropertyType belongs to. /// + /// For generic properties, the value is null. [DataMember] internal Lazy PropertyGroupId { diff --git a/src/Umbraco.Core/Persistence/Factories/MemberTypeReadOnlyFactory.cs b/src/Umbraco.Core/Persistence/Factories/MemberTypeReadOnlyFactory.cs index 93ad99a5c1..eebbc34eda 100644 --- a/src/Umbraco.Core/Persistence/Factories/MemberTypeReadOnlyFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/MemberTypeReadOnlyFactory.cs @@ -164,7 +164,7 @@ namespace Umbraco.Core.Persistence.Factories Name = typeDto.Name, SortOrder = typeDto.SortOrder, ValidationRegExp = typeDto.ValidationRegExp, - PropertyGroupId = new Lazy(() => default(int)), + PropertyGroupId = null, CreateDate = dto.CreateDate, UpdateDate = dto.CreateDate }; diff --git a/src/Umbraco.Core/Services/EntityXmlSerializer.cs b/src/Umbraco.Core/Services/EntityXmlSerializer.cs index 00bc7c5a9b..1e7a95d6fd 100644 --- a/src/Umbraco.Core/Services/EntityXmlSerializer.cs +++ b/src/Umbraco.Core/Services/EntityXmlSerializer.cs @@ -267,11 +267,15 @@ namespace Umbraco.Core.Services structure.Add(new XElement("MediaType", allowedType.Alias)); } - var genericProperties = new XElement("GenericProperties"); + var genericProperties = new XElement("GenericProperties"); // actually, all of them foreach (var propertyType in mediaType.PropertyTypes) { var definition = dataTypeService.GetDataTypeDefinitionById(propertyType.DataTypeDefinitionId); - var propertyGroup = mediaType.PropertyGroups.FirstOrDefault(x => x.Id == propertyType.PropertyGroupId.Value); + + var propertyGroup = propertyType.PropertyGroupId == null // true generic property + ? null + : mediaType.PropertyGroups.FirstOrDefault(x => x.Id == propertyType.PropertyGroupId.Value); + var genericProperty = new XElement("GenericProperty", new XElement("Name", propertyType.Name), new XElement("Alias", propertyType.Alias), @@ -381,14 +385,14 @@ namespace Umbraco.Core.Services structure.Add(new XElement("DocumentType", allowedType.Alias)); } - var genericProperties = new XElement("GenericProperties"); + var genericProperties = new XElement("GenericProperties"); // actually, all of them foreach (var propertyType in contentType.PropertyTypes) { var definition = dataTypeService.GetDataTypeDefinitionById(propertyType.DataTypeDefinitionId); - var propertyGroup = propertyType.PropertyGroupId == null - ? null - : contentType.PropertyGroups.FirstOrDefault(x => x.Id == propertyType.PropertyGroupId.Value); + var propertyGroup = propertyType.PropertyGroupId == null // true generic property + ? null + : contentType.PropertyGroups.FirstOrDefault(x => x.Id == propertyType.PropertyGroupId.Value); var genericProperty = new XElement("GenericProperty", new XElement("Name", propertyType.Name), diff --git a/src/Umbraco.Tests/Persistence/Repositories/ContentTypeRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/ContentTypeRepositoryTest.cs index 053f71b7e0..b2fed80f1b 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/ContentTypeRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/ContentTypeRepositoryTest.cs @@ -90,7 +90,7 @@ namespace Umbraco.Tests.Persistence.Repositories templateRepo.AddOrUpdate(template); } unitOfWork.Commit(); - + var contentType = MockedContentTypes.CreateSimpleContentType(); contentType.AllowedTemplates = new[] {templates[0], templates[1]}; contentType.SetDefaultTemplate(templates[0]); @@ -127,7 +127,7 @@ namespace Umbraco.Tests.Persistence.Repositories repository.AddOrUpdate(contentType); unitOfWork.Commit(); - //create a + //create a var contentType2 = (IContentType)new ContentType(contentType, "hello") { Name = "Blahasdfsadf" @@ -293,7 +293,9 @@ namespace Umbraco.Tests.Persistence.Repositories using (var repository = CreateRepository(unitOfWork)) { // Act - var contentType = (IContentType)MockedContentTypes.CreateSimpleContentType("test", "Test", propertyGroupName: "testGroup"); + var contentType = (IContentType)MockedContentTypes.CreateSimpleContentType2("test", "Test", propertyGroupName: "testGroup"); + + Assert.AreEqual(4, contentType.PropertyTypes.Count()); // there is NO mapping from display to contentType, but only from save // to contentType, so if we want to test, let's to it properly! @@ -301,12 +303,18 @@ namespace Umbraco.Tests.Persistence.Repositories var save = MapToContentTypeSave(display); var mapped = Mapper.Map(save); + Assert.AreEqual(4, mapped.PropertyTypes.Count()); + repository.AddOrUpdate(mapped); unitOfWork.Commit(); + Assert.AreEqual(4, mapped.PropertyTypes.Count()); + //re-get contentType = repository.Get(mapped.Id); + Assert.AreEqual(4, contentType.PropertyTypes.Count()); + // Assert Assert.That(contentType.HasIdentity, Is.True); Assert.That(contentType.PropertyGroups.All(x => x.HasIdentity), Is.True); @@ -316,7 +324,11 @@ namespace Umbraco.Tests.Persistence.Repositories Assert.That(contentType.PropertyGroups.ElementAt(0).Name == "testGroup", Is.True); var groupId = contentType.PropertyGroups.ElementAt(0).Id; - Assert.That(contentType.PropertyTypes.All(x => x.PropertyGroupId.Value == groupId), Is.True); + + var propertyTypes = contentType.PropertyTypes.ToArray(); + Assert.AreEqual("gen", propertyTypes[0].Alias); // just to be sure + Assert.IsNull(propertyTypes[0].PropertyGroupId); + Assert.IsTrue(propertyTypes.Skip(1).All((x => x.PropertyGroupId.Value == groupId))); } } @@ -353,7 +365,7 @@ namespace Umbraco.Tests.Persistence.Repositories Assert.That(contentType.PropertyTypes.Any(x => x.Alias == "subtitle"), Is.True); } - + } // this is for tests only because it makes no sense at all to have such a @@ -491,16 +503,16 @@ namespace Umbraco.Tests.Persistence.Repositories var provider = new PetaPocoUnitOfWorkProvider(Logger); var unitOfWork = provider.GetUnitOfWork(); using (var repository = CreateRepository(unitOfWork)) - { + { var ctMain = MockedContentTypes.CreateSimpleContentType(); var ctChild1 = MockedContentTypes.CreateSimpleContentType("child1", "Child 1", ctMain, true); var ctChild2 = MockedContentTypes.CreateSimpleContentType("child2", "Child 2", ctChild1, true); - + repository.AddOrUpdate(ctMain); repository.AddOrUpdate(ctChild1); - repository.AddOrUpdate(ctChild2); + repository.AddOrUpdate(ctChild2); unitOfWork.Commit(); - + // Act var resolvedParent = repository.Get(ctMain.Id); @@ -528,7 +540,7 @@ namespace Umbraco.Tests.Persistence.Repositories var child3 = MockedContentTypes.CreateSimpleContentType("zyx", "zyx", contentType, randomizeAliases: true); repository.AddOrUpdate(child3); var child2 = MockedContentTypes.CreateSimpleContentType("a123", "a123", contentType, randomizeAliases: true); - repository.AddOrUpdate(child2); + repository.AddOrUpdate(child2); unitOfWork.Commit(); // Act @@ -540,7 +552,7 @@ namespace Umbraco.Tests.Persistence.Repositories Assert.AreEqual("aabc", contentTypes.ElementAt(1).Name); Assert.AreEqual("zyx", contentTypes.ElementAt(2).Name); } - + } [Test] @@ -671,7 +683,7 @@ namespace Umbraco.Tests.Persistence.Repositories { var contentType = repository.Get(NodeDto.NodeIdSeed + 1); - // Act + // Act contentType.PropertyGroups["Meta"].PropertyTypes.Remove("description"); repository.AddOrUpdate(contentType); unitOfWork.Commit(); @@ -841,7 +853,7 @@ namespace Umbraco.Tests.Persistence.Repositories Assert.That(contentType.PropertyTypes.Count(), Is.EqualTo(5)); Assert.That(contentType.PropertyTypes.Any(x => x.Alias == "metaAuthor"), Is.True); } - + } [Test] diff --git a/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs b/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs index 786053e680..e3585d0554 100644 --- a/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs +++ b/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs @@ -164,6 +164,24 @@ namespace Umbraco.Tests.TestHelpers.Entities return contentType; } + public static ContentType CreateSimpleContentType2(string alias, string name, IContentType parent = null, bool randomizeAliases = false, string propertyGroupName = "Content") + { + var contentType = CreateSimpleContentType(alias, name, parent, randomizeAliases, propertyGroupName); + + var propertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = RandomAlias("gen", randomizeAliases), + Name = "Gen", + Description = "", + Mandatory = false, + SortOrder = 1, + DataTypeDefinitionId = -88 + }; + contentType.AddPropertyType(propertyType); + + return contentType; + } + public static ContentType CreateSimpleContentType(string alias, string name, IContentType parent = null, bool randomizeAliases = false, string propertyGroupName = "Content") { var contentType = parent == null ? new ContentType(-1) : new ContentType(parent, alias); diff --git a/src/Umbraco.Web/Models/Mapping/PropertyTypeGroupResolver.cs b/src/Umbraco.Web/Models/Mapping/PropertyTypeGroupResolver.cs index 7921c3ae3c..a2273b07b2 100644 --- a/src/Umbraco.Web/Models/Mapping/PropertyTypeGroupResolver.cs +++ b/src/Umbraco.Web/Models/Mapping/PropertyTypeGroupResolver.cs @@ -113,7 +113,7 @@ namespace Umbraco.Web.Models.Mapping var genericProperties = new List(); // add generic properties local to this content type - var entityGenericProperties = source.PropertyTypes.Where(x => x.PropertyGroupId == null || x.PropertyGroupId.Value == 0); + var entityGenericProperties = source.PropertyTypes.Where(x => x.PropertyGroupId == null); genericProperties.AddRange(MapProperties(entityGenericProperties, source, PropertyGroupBasic.GenericPropertiesGroupId, false)); // add generic properties inherited through compositions