From 17d2c4cab9ef4b6f665ffbca5e7fbfff4f323f8f Mon Sep 17 00:00:00 2001 From: Ronald Barendse Date: Tue, 22 Jun 2021 11:45:23 +0200 Subject: [PATCH] Convert level into group type and ensure names are unique again --- .../V_8_15_0/AddPropertyTypeGroupColumns.cs | 2 +- src/Umbraco.Core/Models/ContentTypeBase.cs | 10 ++-- .../Models/ContentTypeCompositionBase.cs | 8 +-- src/Umbraco.Core/Models/PropertyGroup.cs | 52 ++++++++++++++++--- .../Models/PropertyGroupCollection.cs | 21 ++++---- src/Umbraco.Core/Models/PropertyGroupType.cs | 19 +++++++ .../Persistence/Dtos/PropertyTypeGroupDto.cs | 6 +-- .../Factories/PropertyGroupFactory.cs | 4 +- .../Mappers/PropertyGroupMapper.cs | 2 +- .../Implement/ContentTypeCommonRepository.cs | 2 +- .../Implement/ContentTypeRepositoryBase.cs | 4 +- src/Umbraco.Core/Umbraco.Core.csproj | 1 + .../components/umbgroupsbuilder.directive.js | 8 +-- .../services/umbdataformatter.service.js | 4 +- .../Editors/DashboardController.cs | 2 +- .../Models/ContentEditing/ContentTypeSave.cs | 2 +- .../ContentEditing/PropertyGroupBasic.cs | 5 +- .../ContentEditing/PropertyGroupDisplay.cs | 42 ++++++++++++++- src/Umbraco.Web/Models/ContentEditing/Tab.cs | 5 +- .../Mapping/ContentPropertyMapDefinition.cs | 2 +- .../Mapping/ContentTypeMapDefinition.cs | 12 ++--- .../Models/Mapping/EntityMapDefinition.cs | 2 +- .../Models/Mapping/PropertyTypeGroupMapper.cs | 23 ++++---- .../Models/Mapping/TabsAndPropertiesMapper.cs | 14 +++-- 24 files changed, 172 insertions(+), 80 deletions(-) create mode 100644 src/Umbraco.Core/Models/PropertyGroupType.cs diff --git a/src/Umbraco.Core/Migrations/Upgrade/V_8_15_0/AddPropertyTypeGroupColumns.cs b/src/Umbraco.Core/Migrations/Upgrade/V_8_15_0/AddPropertyTypeGroupColumns.cs index 395981faf5..3e302f2d15 100644 --- a/src/Umbraco.Core/Migrations/Upgrade/V_8_15_0/AddPropertyTypeGroupColumns.cs +++ b/src/Umbraco.Core/Migrations/Upgrade/V_8_15_0/AddPropertyTypeGroupColumns.cs @@ -15,7 +15,7 @@ namespace Umbraco.Core.Migrations.Upgrade.V_8_15_0 // Add new columns AddColumnIfNotExists(columns, "parentKey"); - AddColumnIfNotExists(columns, "level"); + AddColumnIfNotExists(columns, "type"); AddColumnIfNotExists(columns, "icon"); // Create self-referencing foreign key diff --git a/src/Umbraco.Core/Models/ContentTypeBase.cs b/src/Umbraco.Core/Models/ContentTypeBase.cs index 5bff936652..733e41da9c 100644 --- a/src/Umbraco.Core/Models/ContentTypeBase.cs +++ b/src/Umbraco.Core/Models/ContentTypeBase.cs @@ -339,21 +339,19 @@ namespace Umbraco.Core.Models /// "generic properties" ie does not have a tab anymore. public bool MovePropertyType(string propertyTypeAlias, string propertyGroupName) { - // note: not dealing with alias casing at all here? - // get property, ensure it exists - var propertyType = PropertyTypes.FirstOrDefault(x => x.Alias == propertyTypeAlias); + var propertyType = PropertyTypes.FirstOrDefault(x => x.Alias.InvariantEquals(propertyTypeAlias)); if (propertyType == null) return false; // get new group, if required, and ensure it exists var newPropertyGroup = propertyGroupName == null ? null - : PropertyGroups.FirstOrDefault(x => x.Name.InvariantEquals(propertyGroupName) && x.ParentKey == null && x.Level == 1); + : PropertyGroups.FirstOrDefault(x => x.Name.InvariantEquals(propertyGroupName)); if (propertyGroupName != null && newPropertyGroup == null) return false; // get old group var oldPropertyGroup = PropertyGroups.FirstOrDefault(x => - x.PropertyTypes.Any(y => y.Alias == propertyTypeAlias)); + x.PropertyTypes.Any(y => y.Alias.InvariantEquals(propertyTypeAlias))); // set new group propertyType.PropertyGroupId = newPropertyGroup == null ? null : new Lazy(() => newPropertyGroup.Id, false); @@ -403,7 +401,7 @@ namespace Umbraco.Core.Models public void RemovePropertyGroup(string propertyGroupName) { // if no group exists with that name, do nothing - var group = PropertyGroups.FirstOrDefault(x => x.Name.InvariantEquals(propertyGroupName) && x.ParentKey == null && x.Level == 1); + var group = PropertyGroups.FirstOrDefault(x => x.Name.InvariantEquals(propertyGroupName)); if (group == null) return; // first remove the group diff --git a/src/Umbraco.Core/Models/ContentTypeCompositionBase.cs b/src/Umbraco.Core/Models/ContentTypeCompositionBase.cs index 514b307396..fe418d2307 100644 --- a/src/Umbraco.Core/Models/ContentTypeCompositionBase.cs +++ b/src/Umbraco.Core/Models/ContentTypeCompositionBase.cs @@ -218,7 +218,7 @@ namespace Umbraco.Core.Models private PropertyGroup AddAndReturnPropertyGroup(string name) { // ensure we don't have it already - if (PropertyGroups.Any(x => x.Name.InvariantEquals(name) && x.ParentKey == null && x.Level == 1)) + if (PropertyGroups.Any(x => x.Name.InvariantEquals(name))) return null; // create the new group @@ -227,7 +227,7 @@ namespace Umbraco.Core.Models // check if it is inherited - there might be more than 1 but we want the 1st, to // reuse its sort order - if there are more than 1 and they have different sort // orders... there isn't much we can do anyways - var inheritGroup = CompositionPropertyGroups.FirstOrDefault(x => x.Name.InvariantEquals(name) && x.ParentKey == null && x.Level == 1); + var inheritGroup = CompositionPropertyGroups.FirstOrDefault(x => x.Name.InvariantEquals(name)); if (inheritGroup == null) { // no, just local, set sort order @@ -260,8 +260,8 @@ namespace Umbraco.Core.Models return false; // get and ensure a group local to this content type - var group = PropertyGroups.FirstOrDefault(x => x.Name.InvariantEquals(propertyGroupName) && x.ParentKey == null && x.Level == 1) - ??AddAndReturnPropertyGroup(propertyGroupName); + var group = PropertyGroups.FirstOrDefault(x => x.Name.InvariantEquals(propertyGroupName)) + ?? AddAndReturnPropertyGroup(propertyGroupName); if (group == null) return false; diff --git a/src/Umbraco.Core/Models/PropertyGroup.cs b/src/Umbraco.Core/Models/PropertyGroup.cs index f4629bf98d..31351a46ad 100644 --- a/src/Umbraco.Core/Models/PropertyGroup.cs +++ b/src/Umbraco.Core/Models/PropertyGroup.cs @@ -1,6 +1,8 @@ using System; +using System.Collections.Generic; using System.Collections.Specialized; using System.Diagnostics; +using System.Linq; using System.Runtime.Serialization; using Umbraco.Core.Models.Entities; @@ -15,7 +17,7 @@ namespace Umbraco.Core.Models public class PropertyGroup : EntityBase, IEquatable { private Guid? _parentKey; - private short _level; + private PropertyGroupType _type; private string _name; private string _icon; private int _sortOrder; @@ -28,7 +30,6 @@ namespace Umbraco.Core.Models public PropertyGroup(PropertyTypeCollection propertyTypeCollection) { PropertyTypes = propertyTypeCollection; - Level = 1; // TODO We default to 1 (property group) for backwards compatibility, but should use zero/no default at some point. } private void PropertyTypesChanged(object sender, NotifyCollectionChangedEventArgs e) @@ -50,10 +51,10 @@ namespace Umbraco.Core.Models /// Gets or sets the Sort Order of the Group /// [DataMember] - public short Level + public PropertyGroupType Type { - get => _level; - set => SetPropertyValueAndDetectChanges(value, ref _level, nameof(Level)); + get => _type; + set => SetPropertyValueAndDetectChanges(value, ref _type, nameof(Type)); } /// @@ -116,9 +117,9 @@ namespace Umbraco.Core.Models } } - public bool Equals(PropertyGroup other) => other != null && base.Equals(other) && ParentKey == other.ParentKey && Level == other.Level && Icon == other.Icon && Name.InvariantEquals(other.Name); + public bool Equals(PropertyGroup other) => other != null && base.Equals(other) && ParentKey == other.ParentKey && Type == other.Type && Icon == other.Icon && Name.InvariantEquals(other.Name); - public override int GetHashCode() => (base.GetHashCode(), ParentKey, Level, Icon, Name.ToLowerInvariant()).GetHashCode(); + public override int GetHashCode() => (base.GetHashCode(), ParentKey, Type, Icon, Name?.ToLowerInvariant()).GetHashCode(); protected override void PerformDeepClone(object clone) { @@ -134,4 +135,41 @@ namespace Umbraco.Core.Models } } } + + internal static class PropertyGroupExtensions + { + /// + /// Orders the property groups by hierarchy (so child groups are after their parent group) and removes circular references. + /// + /// The property groups. + /// + /// The ordered property groups. + /// + public static IEnumerable OrderByHierarchy(this IEnumerable propertyGroups) + { + var groups = propertyGroups.ToList(); + var visitedParentKeys = new HashSet(groups.Count); + + IEnumerable OrderByHierarchy(Guid? parentKey) + { + if (parentKey.HasValue && visitedParentKeys.Add(parentKey.Value) == false) + { + // We already visited this parent key, stop to prevent a circular reference + yield break; + } + + foreach (var group in groups.Where(x => x.ParentKey == parentKey).OrderBy(x => x.Type).ThenBy(x => x.SortOrder)) + { + yield return group; + + foreach (var childGroup in OrderByHierarchy(group.Key)) + { + yield return childGroup; + } + } + } + + return OrderByHierarchy(null); + } + } } diff --git a/src/Umbraco.Core/Models/PropertyGroupCollection.cs b/src/Umbraco.Core/Models/PropertyGroupCollection.cs index 973147b3fb..eb91d26f63 100644 --- a/src/Umbraco.Core/Models/PropertyGroupCollection.cs +++ b/src/Umbraco.Core/Models/PropertyGroupCollection.cs @@ -77,32 +77,29 @@ namespace Umbraco.Core.Models //Note this is done to ensure existing groups can be renamed if (item.HasIdentity && item.Id > 0) { - var exists = Contains(item.Id); - if (exists) + var index = IndexOfKey(item.Id); + if (index != -1) { var keyExists = Contains(item.Name); if (keyExists) throw new Exception($"Naming conflict: Changing the name of PropertyGroup '{item.Name}' would result in duplicates"); //collection events will be raised in SetItem - SetItem(IndexOfKey(item.Id), item); + SetItem(index, item); return; } } else { - var key = GetKeyForItem(item); - if (key != null) + var index = IndexOfKey(item.Name); + if (index != -1) { - var exists = Contains(key); - if (exists) - { - //collection events will be raised in SetItem - SetItem(IndexOfKey(key), item); - return; - } + //collection events will be raised in SetItem + SetItem(index, item); + return; } } + //collection events will be raised in InsertItem base.Add(item); } diff --git a/src/Umbraco.Core/Models/PropertyGroupType.cs b/src/Umbraco.Core/Models/PropertyGroupType.cs new file mode 100644 index 0000000000..cb2a21ebc9 --- /dev/null +++ b/src/Umbraco.Core/Models/PropertyGroupType.cs @@ -0,0 +1,19 @@ +namespace Umbraco.Core.Models +{ + /// + /// Represents the type of a property group. + /// + public enum PropertyGroupType : short + { + /// + /// Display as a group (using a header). + /// + Group = 0, + /// + /// Display as an app (using a name and icon). + /// + App = 1, + //Tab = 2, + //Fieldset = 3 + } +} diff --git a/src/Umbraco.Core/Persistence/Dtos/PropertyTypeGroupDto.cs b/src/Umbraco.Core/Persistence/Dtos/PropertyTypeGroupDto.cs index aa9ea072ee..48d72a7a63 100644 --- a/src/Umbraco.Core/Persistence/Dtos/PropertyTypeGroupDto.cs +++ b/src/Umbraco.Core/Persistence/Dtos/PropertyTypeGroupDto.cs @@ -28,9 +28,9 @@ namespace Umbraco.Core.Persistence.Dtos [ForeignKey(typeof(PropertyTypeGroupDto), Column = "uniqueID", Name = "FK_" + TableName + "_parentKey")] public Guid? ParentKey { get; set; } - [Column("level")] - [Constraint(Default = 1)] // TODO We default to 1 (property group) for backwards compatibility, but should use zero/no default at some point. - public short Level { get; set; } = 1; + [Column("type")] + [Constraint(Default = 0)] + public short Type { get; set; } [Column("contenttypeNodeId")] [ForeignKey(typeof(ContentTypeDto), Column = "nodeId")] diff --git a/src/Umbraco.Core/Persistence/Factories/PropertyGroupFactory.cs b/src/Umbraco.Core/Persistence/Factories/PropertyGroupFactory.cs index 70a4c1f9be..437cb33a31 100644 --- a/src/Umbraco.Core/Persistence/Factories/PropertyGroupFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/PropertyGroupFactory.cs @@ -36,7 +36,7 @@ namespace Umbraco.Core.Persistence.Factories group.Key = groupDto.UniqueId; group.ParentKey = groupDto.ParentKey; - group.Level = groupDto.Level; + group.Type = (PropertyGroupType)groupDto.Type; group.Icon = groupDto.Icon; group.Name = groupDto.Text; group.SortOrder = groupDto.SortOrder; @@ -109,7 +109,7 @@ namespace Umbraco.Core.Persistence.Factories { UniqueId = propertyGroup.Key, ParentKey = propertyGroup.ParentKey, - Level = propertyGroup.Level, + Type = (short)propertyGroup.Type, ContentTypeNodeId = contentTypeId, Icon = propertyGroup.Icon, Text = propertyGroup.Name, diff --git a/src/Umbraco.Core/Persistence/Mappers/PropertyGroupMapper.cs b/src/Umbraco.Core/Persistence/Mappers/PropertyGroupMapper.cs index b88af3f76f..0668518cd0 100644 --- a/src/Umbraco.Core/Persistence/Mappers/PropertyGroupMapper.cs +++ b/src/Umbraco.Core/Persistence/Mappers/PropertyGroupMapper.cs @@ -21,7 +21,7 @@ namespace Umbraco.Core.Persistence.Mappers DefineMap(nameof(PropertyGroup.Id), nameof(PropertyTypeGroupDto.Id)); DefineMap(nameof(PropertyGroup.Key), nameof(PropertyTypeGroupDto.UniqueId)); DefineMap(nameof(PropertyGroup.ParentKey), nameof(PropertyTypeGroupDto.ParentKey)); - DefineMap(nameof(PropertyGroup.Level), nameof(PropertyTypeGroupDto.Level)); + DefineMap(nameof(PropertyGroup.Type), nameof(PropertyTypeGroupDto.Type)); DefineMap(nameof(PropertyGroup.Icon), nameof(PropertyTypeGroupDto.Icon)); DefineMap(nameof(PropertyGroup.Name), nameof(PropertyTypeGroupDto.Text)); DefineMap(nameof(PropertyGroup.SortOrder), nameof(PropertyTypeGroupDto.SortOrder)); diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeCommonRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeCommonRepository.cs index 8a949d384a..a03f61d54e 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeCommonRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeCommonRepository.cs @@ -274,7 +274,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement Id = dto.Id, Key = dto.UniqueId, ParentKey = dto.ParentKey, - Level = dto.Level, + Type = (PropertyGroupType)dto.Type, Icon = dto.Icon, Name = dto.Text, SortOrder = dto.SortOrder diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs index 560a8ffdad..831ce0ee2b 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs @@ -175,7 +175,7 @@ AND umbracoNode.nodeObjectType = @objectType", } //Insert Tabs - foreach (var propertyGroup in entity.PropertyGroups.OrderBy(x => x.Level).ToArray()) + foreach (var propertyGroup in entity.PropertyGroups.OrderByHierarchy()) { var tabDto = PropertyGroupFactory.BuildGroupDto(propertyGroup, nodeDto.NodeId); var primaryKey = Convert.ToInt32(Database.Insert(tabDto)); @@ -374,7 +374,7 @@ AND umbracoNode.id <> @id", } // insert or update groups, assign properties - foreach (var propertyGroup in entity.PropertyGroups.OrderBy(x => x.Level).ToArray()) + foreach (var propertyGroup in entity.PropertyGroups.OrderByHierarchy()) { // insert or update group var groupDto = PropertyGroupFactory.BuildGroupDto(propertyGroup, entity.Id); diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 9eeb41a099..bdb59b511a 100755 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -158,6 +158,7 @@ + diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/umbgroupsbuilder.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/umbgroupsbuilder.directive.js index b255619fea..28ce7ec022 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/umbgroupsbuilder.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/umbgroupsbuilder.directive.js @@ -23,12 +23,12 @@ if (newValue && newValue.length === 0) return; scope.tabs = $filter("filter")(scope.model.groups, (group) => { - return group.level === 0; + return group.type === 1; }); }); function getFirstTab () { - return scope.model.groups.find(group => group.level === 0); + return scope.model.groups.find(group => group.type === 1); } function activate() { @@ -402,7 +402,7 @@ key: String.CreateGuid(), name: "", parentKey: null, - level: 0, + type: 1, sortOrder, icon: "icon-document color-black" }; @@ -504,7 +504,7 @@ parentTabContentTypeNames: [], name: "", parentKey: tabKey || null, - level: 1, + type: 0, sortOrder: lastGroupSortOrder }; diff --git a/src/Umbraco.Web.UI.Client/src/common/services/umbdataformatter.service.js b/src/Umbraco.Web.UI.Client/src/common/services/umbdataformatter.service.js index d2eb660601..4926202559 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/umbdataformatter.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/umbdataformatter.service.js @@ -64,7 +64,7 @@ var saveModel = _.pick(displayModel, 'compositeContentTypes', 'isContainer', 'allowAsRoot', 'allowedTemplates', 'allowedContentTypes', 'alias', 'description', 'thumbnail', 'name', 'id', 'icon', 'trashed', - 'key', 'parentId', 'alias', 'path', 'allowCultureVariant', 'allowSegmentVariant', 'isElement', 'tabs'); + 'key', 'parentId', 'alias', 'path', 'allowCultureVariant', 'allowSegmentVariant', 'isElement'); // TODO: Map these saveModel.allowedTemplates = _.map(displayModel.allowedTemplates, function (t) { return t.alias; }); @@ -75,7 +75,7 @@ }); saveModel.groups = _.map(realGroups, function (g) { - var saveGroup = _.pick(g, 'inherited', 'id', 'sortOrder', 'name', 'key', 'parentKey', 'level', 'icon'); + var saveGroup = _.pick(g, 'inherited', 'id', 'sortOrder', 'name', 'key', 'parentKey', 'type', 'icon'); var realProperties = _.reject(g.properties, function (p) { //do not include these properties diff --git a/src/Umbraco.Web/Editors/DashboardController.cs b/src/Umbraco.Web/Editors/DashboardController.cs index 7d95b69270..41dc1b42b5 100644 --- a/src/Umbraco.Web/Editors/DashboardController.cs +++ b/src/Umbraco.Web/Editors/DashboardController.cs @@ -204,7 +204,7 @@ namespace Umbraco.Web.Editors Id = x.Id, Key = x.Key, ParentKey = x.ParentKey, - Level = x.Level, + Type = x.Type, Icon = x.Icon, Alias = x.Alias, Label = x.Label, diff --git a/src/Umbraco.Web/Models/ContentEditing/ContentTypeSave.cs b/src/Umbraco.Web/Models/ContentEditing/ContentTypeSave.cs index 4caef2373b..0579849d2e 100644 --- a/src/Umbraco.Web/Models/ContentEditing/ContentTypeSave.cs +++ b/src/Umbraco.Web/Models/ContentEditing/ContentTypeSave.cs @@ -88,7 +88,7 @@ namespace Umbraco.Web.Models.ContentEditing yield return validationResult; } - var duplicateGroups = Groups.GroupBy(x => (x.Name?.ToUpperInvariant(), x.Level)).Where(x => x.Count() > 1).ToArray(); + var duplicateGroups = Groups.GroupBy(x => x.Name?.ToUpperInvariant()).Where(x => x.Count() > 1).ToArray(); if (duplicateGroups.Any()) { //we need to return the field name with an index so it's wired up correctly diff --git a/src/Umbraco.Web/Models/ContentEditing/PropertyGroupBasic.cs b/src/Umbraco.Web/Models/ContentEditing/PropertyGroupBasic.cs index d10c767dd4..2d00e94b6a 100644 --- a/src/Umbraco.Web/Models/ContentEditing/PropertyGroupBasic.cs +++ b/src/Umbraco.Web/Models/ContentEditing/PropertyGroupBasic.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.ComponentModel.DataAnnotations; using System.Runtime.Serialization; +using Umbraco.Core.Models; namespace Umbraco.Web.Models.ContentEditing { @@ -39,8 +40,8 @@ namespace Umbraco.Web.Models.ContentEditing [DataMember(Name = "parentKey")] public Guid? ParentKey { get; set; } - [DataMember(Name = "level")] - public short Level { get; set; } + [DataMember(Name = "type")] + public PropertyGroupType Type { get; set; } [DataMember(Name = "icon")] public string Icon { get; set; } diff --git a/src/Umbraco.Web/Models/ContentEditing/PropertyGroupDisplay.cs b/src/Umbraco.Web/Models/ContentEditing/PropertyGroupDisplay.cs index 1523daacda..7b61c1ce18 100644 --- a/src/Umbraco.Web/Models/ContentEditing/PropertyGroupDisplay.cs +++ b/src/Umbraco.Web/Models/ContentEditing/PropertyGroupDisplay.cs @@ -1,5 +1,7 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using System.ComponentModel; +using System.Linq; using System.Runtime.Serialization; namespace Umbraco.Web.Models.ContentEditing @@ -36,4 +38,42 @@ namespace Umbraco.Web.Models.ContentEditing [ReadOnly(true)] public IEnumerable ParentTabContentTypeNames { get; set; } } + + internal static class PropertyGroupDisplayExtensions + { + /// + /// Orders the property groups by hierarchy (so child groups are after their parent group) and removes circular references. + /// + /// The property groups. + /// + /// The ordered property groups. + /// + public static IEnumerable> OrderByHierarchy(this IEnumerable> propertyGroups) + where T : PropertyTypeDisplay + { + var groups = propertyGroups.ToList(); + var visitedParentKeys = new HashSet(groups.Count); + + IEnumerable> OrderByHierarchy(Guid? parentKey) + { + if (parentKey.HasValue && visitedParentKeys.Add(parentKey.Value) == false) + { + // We already visited this parent key, stop to prevent a circular reference + yield break; + } + + foreach (var group in groups.Where(x => x.ParentKey == parentKey).OrderBy(x => x.Type).ThenBy(x => x.SortOrder)) + { + yield return group; + + foreach (var childGroup in OrderByHierarchy(group.Key)) + { + yield return childGroup; + } + } + } + + return OrderByHierarchy(null); + } + } } diff --git a/src/Umbraco.Web/Models/ContentEditing/Tab.cs b/src/Umbraco.Web/Models/ContentEditing/Tab.cs index 4536b80889..c122a18dff 100644 --- a/src/Umbraco.Web/Models/ContentEditing/Tab.cs +++ b/src/Umbraco.Web/Models/ContentEditing/Tab.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Runtime.Serialization; +using Umbraco.Core.Models; namespace Umbraco.Web.Models.ContentEditing { @@ -19,8 +20,8 @@ namespace Umbraco.Web.Models.ContentEditing [DataMember(Name = "parentKey")] public Guid? ParentKey { get; set; } - [DataMember(Name = "level")] - public int Level { get; set; } + [DataMember(Name = "type")] + public PropertyGroupType Type { get; set; } [DataMember(Name = "icon")] public string Icon { get; set; } diff --git a/src/Umbraco.Web/Models/Mapping/ContentPropertyMapDefinition.cs b/src/Umbraco.Web/Models/Mapping/ContentPropertyMapDefinition.cs index 981db18616..e911b44f46 100644 --- a/src/Umbraco.Web/Models/Mapping/ContentPropertyMapDefinition.cs +++ b/src/Umbraco.Web/Models/Mapping/ContentPropertyMapDefinition.cs @@ -38,7 +38,7 @@ namespace Umbraco.Web.Models.Mapping target.Id = source.Id; target.Key = source.Key; target.ParentKey = source.ParentKey; - target.Level = source.Level; + target.Type = source.Type; target.Icon = source.Icon; target.IsActive = true; target.Label = source.Name; diff --git a/src/Umbraco.Web/Models/Mapping/ContentTypeMapDefinition.cs b/src/Umbraco.Web/Models/Mapping/ContentTypeMapDefinition.cs index 1b596aa209..2bc7a7d456 100644 --- a/src/Umbraco.Web/Models/Mapping/ContentTypeMapDefinition.cs +++ b/src/Umbraco.Web/Models/Mapping/ContentTypeMapDefinition.cs @@ -303,7 +303,7 @@ namespace Umbraco.Web.Models.Mapping target.Key = source.Key; target.ParentKey = source.ParentKey; - target.Level = source.Level; + target.Type = source.Type; target.Icon = source.Icon; target.Name = source.Name; target.SortOrder = source.SortOrder; @@ -317,7 +317,7 @@ namespace Umbraco.Web.Models.Mapping target.Key = source.Key; target.ParentKey = source.ParentKey; - target.Level = source.Level; + target.Type = source.Type; target.Icon = source.Icon; target.Name = source.Name; target.SortOrder = source.SortOrder; @@ -331,7 +331,7 @@ namespace Umbraco.Web.Models.Mapping target.Key = source.Key; target.ParentKey = source.ParentKey; - target.Level = source.Level; + target.Type = source.Type; target.Icon = source.Icon; target.Name = source.Name; target.SortOrder = source.SortOrder; @@ -348,7 +348,7 @@ namespace Umbraco.Web.Models.Mapping target.Key = source.Key; target.ParentKey = source.ParentKey; - target.Level = source.Level; + target.Type = source.Type; target.Icon = source.Icon; target.Name = source.Name; target.SortOrder = source.SortOrder; @@ -670,7 +670,7 @@ namespace Umbraco.Web.Models.Mapping { var propertiesA = properties.ToArray(); var distinctProperties = propertiesA - .Select(x => x.Alias.ToUpperInvariant()) + .Select(x => x.Alias?.ToUpperInvariant()) .Distinct() .Count(); if (distinctProperties != propertiesA.Length) @@ -681,7 +681,7 @@ namespace Umbraco.Web.Models.Mapping { var groupsA = groups.ToArray(); var distinctProperties = groupsA - .Select(x => (x.Name?.ToUpperInvariant(), x.Level)) + .Select(x => x.Name?.ToUpperInvariant()) .Distinct() .Count(); if (distinctProperties != groupsA.Length) diff --git a/src/Umbraco.Web/Models/Mapping/EntityMapDefinition.cs b/src/Umbraco.Web/Models/Mapping/EntityMapDefinition.cs index 1c4ca6087c..15af391e6b 100644 --- a/src/Umbraco.Web/Models/Mapping/EntityMapDefinition.cs +++ b/src/Umbraco.Web/Models/Mapping/EntityMapDefinition.cs @@ -85,7 +85,7 @@ namespace Umbraco.Web.Models.Mapping // Umbraco.Code.MapAll -Udi -Trashed private static void Map(PropertyGroup source, EntityBasic target, MapperContext context) { - target.Alias = source.Name.ToLowerInvariant(); + target.Alias = source.Name?.ToLowerInvariant(); target.Icon = "icon-tab"; target.Id = source.Id; target.Key = source.Key; diff --git a/src/Umbraco.Web/Models/Mapping/PropertyTypeGroupMapper.cs b/src/Umbraco.Web/Models/Mapping/PropertyTypeGroupMapper.cs index 90ee756f01..2c3dec3061 100644 --- a/src/Umbraco.Web/Models/Mapping/PropertyTypeGroupMapper.cs +++ b/src/Umbraco.Web/Models/Mapping/PropertyTypeGroupMapper.cs @@ -77,7 +77,7 @@ namespace Umbraco.Web.Models.Mapping Id = tab.Id, Key = tab.Key, ParentKey = tab.ParentKey, - Level = tab.Level, + Type = tab.Type, Icon = tab.Icon, Name = tab.Name, SortOrder = tab.SortOrder, @@ -106,7 +106,7 @@ namespace Umbraco.Web.Models.Mapping Id = tab.Id, Key = tab.Key, ParentKey = tab.ParentKey, - Level = tab.Level, + Type = tab.Type, Icon = tab.Icon, Name = tab.Name, SortOrder = tab.SortOrder, @@ -146,7 +146,6 @@ namespace Umbraco.Web.Models.Mapping var genericTab = new PropertyGroupDisplay { Id = PropertyGroupBasic.GenericPropertiesGroupId, - Level = 1, Name = "Generic properties", ContentTypeId = source.Id, SortOrder = 999, @@ -171,36 +170,36 @@ namespace Umbraco.Web.Models.Mapping // now merge tabs based on names // as for one name, we might have one local tab, plus some inherited tabs - var groupsGroupsByNameAndLevel = groups.GroupBy(x => (x.Name, x.Level)).ToArray(); + var groupsGroupsByHierarchy = groups.GroupBy(x => (x.Name, x.ParentKey, x.Type)).ToArray(); groups = new List>(); // start with a fresh list - foreach (var groupsByNameAndLevel in groupsGroupsByNameAndLevel) + foreach (var groupsByHierarchy in groupsGroupsByHierarchy) { // single group, just use it - if (groupsByNameAndLevel.Count() == 1) + if (groupsByHierarchy.Count() == 1) { - groups.Add(groupsByNameAndLevel.First()); + groups.Add(groupsByHierarchy.First()); continue; } // multiple groups, merge - var group = groupsByNameAndLevel.FirstOrDefault(x => x.Inherited == false) // try local - ?? groupsByNameAndLevel.First(); // else pick one randomly + var group = groupsByHierarchy.FirstOrDefault(x => x.Inherited == false) // try local + ?? groupsByHierarchy.First(); // else pick one randomly groups.Add(group); // in case we use the local one, flag as inherited group.Inherited = true; // merge (and sort) properties - var properties = groupsByNameAndLevel.SelectMany(x => x.Properties).OrderBy(x => x.SortOrder).ToArray(); + var properties = groupsByHierarchy.SelectMany(x => x.Properties).OrderBy(x => x.SortOrder).ToArray(); group.Properties = properties; // collect parent group info - var parentGroups = groupsByNameAndLevel.Where(x => x.ContentTypeId != source.Id).ToArray(); + var parentGroups = groupsByHierarchy.Where(x => x.ContentTypeId != source.Id).ToArray(); group.ParentTabContentTypes = parentGroups.SelectMany(x => x.ParentTabContentTypes).ToArray(); group.ParentTabContentTypeNames = parentGroups.SelectMany(x => x.ParentTabContentTypeNames).ToArray(); } - return groups.OrderBy(x => x.Level).ThenBy(x => x.SortOrder).ToArray(); + return groups.OrderByHierarchy(); } private IEnumerable MapProperties(IEnumerable properties, IContentTypeBase contentType, int groupId, bool inherited) diff --git a/src/Umbraco.Web/Models/Mapping/TabsAndPropertiesMapper.cs b/src/Umbraco.Web/Models/Mapping/TabsAndPropertiesMapper.cs index 674e06c51b..97467bd06e 100644 --- a/src/Umbraco.Web/Models/Mapping/TabsAndPropertiesMapper.cs +++ b/src/Umbraco.Web/Models/Mapping/TabsAndPropertiesMapper.cs @@ -58,7 +58,6 @@ namespace Umbraco.Web.Models.Mapping tabs.Add(new Tab { Id = 0, - Level = 1, Label = LocalizedTextService.Localize("general/properties"), Alias = "Generic properties", Properties = genericproperties @@ -132,13 +131,12 @@ namespace Umbraco.Web.Models.Mapping // content.CompositionPropertyGroups). var groups = contentType.CompositionPropertyGroups.ToArray(); var parentKeys = groups.Where(x => x.ParentKey.HasValue).Select(x => x.ParentKey.Value).Distinct().ToArray(); - var groupsGroupsByNameAndLevel = groups.OrderBy(x => x.Level).ThenBy(x => x.SortOrder).GroupBy(x => (x.Name, x.Level)); - foreach (var groupsByNameAndLevel in groupsGroupsByNameAndLevel) + foreach (var groupsByHierarchy in groups.OrderByHierarchy().GroupBy(x => (x.Name, x.ParentKey))) { var properties = new List(); // merge properties for groups with the same name - foreach (var group in groupsByNameAndLevel) + foreach (var group in groupsByHierarchy) { var groupProperties = source.GetPropertiesForGroup(group) .Where(x => IgnoreProperties.Contains(x.Alias) == false); // skip ignored @@ -146,7 +144,7 @@ namespace Umbraco.Web.Models.Mapping properties.AddRange(groupProperties); } - if (properties.Count == 0 && groupsByNameAndLevel.All(x => !parentKeys.Contains(x.Key))) + if (properties.Count == 0 && groupsByHierarchy.All(x => !parentKeys.Contains(x.Key))) continue; //map the properties @@ -154,15 +152,15 @@ namespace Umbraco.Web.Models.Mapping // add the tab // we need to pick an identifier... there is no "right" way... - var g = groupsByNameAndLevel.FirstOrDefault(x => x.Id == source.ContentTypeId) // try local - ?? groupsByNameAndLevel.First(); // else pick one randomly + var g = groupsByHierarchy.FirstOrDefault(x => x.Id == source.ContentTypeId) // try local + ?? groupsByHierarchy.First(); // else pick one randomly tabs.Add(new Tab { Id = g.Id, Key = g.Key, ParentKey = g.ParentKey, - Level = g.Level, + Type = g.Type, Icon = g.Icon, Alias = g.Name, Label = LocalizedTextService.UmbracoDictionaryTranslate(g.Name),