diff --git a/src/Umbraco.Core/Models/ContentTypeBase.cs b/src/Umbraco.Core/Models/ContentTypeBase.cs index 4d0bcb0e29..afc291c09d 100644 --- a/src/Umbraco.Core/Models/ContentTypeBase.cs +++ b/src/Umbraco.Core/Models/ContentTypeBase.cs @@ -298,23 +298,15 @@ namespace Umbraco.Cms.Core.Models /// /// Checks whether a PropertyType with a given alias already exists /// - /// Alias of the PropertyType + /// Alias of the PropertyType /// Returns True if a PropertyType with the passed in alias exists, otherwise False - public abstract bool PropertyTypeExists(string propertyTypeAlias); + public abstract bool PropertyTypeExists(string alias); /// - [Obsolete("Use AddPropertyGroup(name, alias) instead to explicitly set the alias.")] - public virtual bool AddPropertyGroup(string groupName) => AddPropertyGroup(groupName, groupName.ToSafeAlias(_shortStringHelper, true)); + public abstract bool AddPropertyGroup(string alias, string name); /// - public abstract bool AddPropertyGroup(string name, string alias); - - /// - [Obsolete("Use AddPropertyType(propertyType, groupAlias, groupName) instead to explicitly set the alias of the group (note the slighty different parameter order).")] - public virtual bool AddPropertyType(IPropertyType propertyType, string propertyGroupName) => AddPropertyType(propertyType, propertyGroupName.ToSafeAlias(_shortStringHelper, true), propertyGroupName); - - /// - public abstract bool AddPropertyType(IPropertyType propertyType, string groupAlias, string groupName); + public abstract bool AddPropertyType(IPropertyType propertyType, string propertyGroupAlias, string propertyGroupName = null); /// /// Adds a PropertyType, which does not belong to a PropertyGroup. @@ -336,11 +328,11 @@ namespace Umbraco.Cms.Core.Models /// Moves a PropertyType to a specified PropertyGroup /// /// Alias of the PropertyType to move - /// Name of the PropertyGroup to move the PropertyType to + /// Alias of the PropertyGroup to move the PropertyType to /// - /// If is null then the property is moved back to + /// If is null then the property is moved back to /// "generic properties" ie does not have a tab anymore. - public bool MovePropertyType(string propertyTypeAlias, string propertyGroupName) + public bool MovePropertyType(string propertyTypeAlias, string propertyGroupAlias) { // get property, ensure it exists var propertyType = PropertyTypes.FirstOrDefault(x => x.Alias == propertyTypeAlias); @@ -348,9 +340,9 @@ namespace Umbraco.Cms.Core.Models // get new group, if required, and ensure it exists PropertyGroup newPropertyGroup = null; - if (propertyGroupName != null) + if (propertyGroupAlias != null) { - var index = PropertyGroups.IndexOfKey(propertyGroupName); + var index = PropertyGroups.IndexOfKey(propertyGroupAlias); if (index == -1) return false; newPropertyGroup = PropertyGroups[index]; @@ -373,13 +365,13 @@ namespace Umbraco.Cms.Core.Models /// /// Removes a PropertyType from the current ContentType /// - /// Alias of the to remove - public void RemovePropertyType(string propertyTypeAlias) + /// Alias of the to remove + public void RemovePropertyType(string alias) { //check through each property group to see if we can remove the property type by alias from it foreach (var propertyGroup in PropertyGroups) { - if (propertyGroup.PropertyTypes.RemoveItem(propertyTypeAlias)) + if (propertyGroup.PropertyTypes.RemoveItem(alias)) { if (!HasPropertyTypeBeenRemoved) { @@ -391,7 +383,7 @@ namespace Umbraco.Cms.Core.Models } //check through each local property type collection (not assigned to a tab) - if (_noGroupPropertyTypes.RemoveItem(propertyTypeAlias)) + if (_noGroupPropertyTypes.RemoveItem(alias)) { if (!HasPropertyTypeBeenRemoved) { @@ -404,11 +396,11 @@ namespace Umbraco.Cms.Core.Models /// /// Removes a PropertyGroup from the current ContentType /// - /// Name of the to remove - public void RemovePropertyGroup(string propertyGroupName) + /// Alias of the to remove + public void RemovePropertyGroup(string alias) { - // if no group exists with that name, do nothing - var index = PropertyGroups.IndexOfKey(propertyGroupName); + // if no group exists with that alias, do nothing + var index = PropertyGroups.IndexOfKey(alias); if (index == -1) return; var group = PropertyGroups[index]; diff --git a/src/Umbraco.Core/Models/ContentTypeCompositionBase.cs b/src/Umbraco.Core/Models/ContentTypeCompositionBase.cs index 31093077bc..596dbe7a63 100644 --- a/src/Umbraco.Core/Models/ContentTypeCompositionBase.cs +++ b/src/Umbraco.Core/Models/ContentTypeCompositionBase.cs @@ -17,7 +17,8 @@ namespace Umbraco.Cms.Core.Models private List _contentTypeComposition = new List(); private List _removedContentTypeKeyTracker = new List(); - protected ContentTypeCompositionBase(IShortStringHelper shortStringHelper, int parentId) : base(shortStringHelper, parentId) + protected ContentTypeCompositionBase(IShortStringHelper shortStringHelper, int parentId) + : base(shortStringHelper, parentId) { } protected ContentTypeCompositionBase(IShortStringHelper shortStringHelper,IContentTypeComposition parent) @@ -61,7 +62,7 @@ namespace Umbraco.Cms.Core.Models void AcquireProperty(IPropertyType propertyType) { - propertyType.Variations = propertyType.Variations & Variations; + propertyType.Variations &= Variations; propertyType.ResetDirtyProperties(false); } @@ -90,7 +91,7 @@ namespace Umbraco.Cms.Core.Models IPropertyType AcquireProperty(IPropertyType propertyType) { propertyType = (IPropertyType) propertyType.DeepClone(); - propertyType.Variations = propertyType.Variations & Variations; + propertyType.Variations &= Variations; propertyType.ResetDirtyProperties(false); return propertyType; } @@ -132,8 +133,8 @@ namespace Umbraco.Cms.Core.Models if (ContentTypeCompositionExists(contentType.Alias) == false) { - //Before we actually go ahead and add the ContentType as a Composition we ensure that we don't - //end up with duplicate PropertyType aliases - in which case we throw an exception. + // Before we actually go ahead and add the ContentType as a Composition we ensure that we don't + // end up with duplicate PropertyType aliases - in which case we throw an exception. var conflictingPropertyTypeAliases = CompositionPropertyTypes.SelectMany( x => contentType.CompositionPropertyTypes .Where(y => y.Alias.Equals(x.Alias, StringComparison.InvariantCultureIgnoreCase)) @@ -143,9 +144,12 @@ namespace Umbraco.Cms.Core.Models throw new InvalidCompositionException(Alias, contentType.Alias, conflictingPropertyTypeAliases.ToArray()); _contentTypeComposition.Add(contentType); + OnPropertyChanged(nameof(ContentTypeComposition)); + return true; } + return false; } @@ -159,19 +163,21 @@ namespace Umbraco.Cms.Core.Models if (ContentTypeCompositionExists(alias)) { var contentTypeComposition = ContentTypeComposition.FirstOrDefault(x => x.Alias == alias); - if (contentTypeComposition == null)//You can't remove a composition from another composition + if (contentTypeComposition == null) // You can't remove a composition from another composition return false; _removedContentTypeKeyTracker.Add(contentTypeComposition.Id); - //If the ContentType we are removing has Compositions of its own these needs to be removed as well + // If the ContentType we are removing has Compositions of its own these needs to be removed as well var compositionIdsToRemove = contentTypeComposition.CompositionIds().ToList(); if (compositionIdsToRemove.Any()) _removedContentTypeKeyTracker.AddRange(compositionIdsToRemove); OnPropertyChanged(nameof(ContentTypeComposition)); + return _contentTypeComposition.Remove(contentTypeComposition); } + return false; } @@ -194,20 +200,14 @@ namespace Umbraco.Cms.Core.Models /// /// Checks whether a PropertyType with a given alias already exists /// - /// Alias of the PropertyType + /// Alias of the PropertyType /// Returns True if a PropertyType with the passed in alias exists, otherwise False - public override bool PropertyTypeExists(string propertyTypeAlias) - { - return CompositionPropertyTypes.Any(x => x.Alias == propertyTypeAlias); - } + public override bool PropertyTypeExists(string alias) => CompositionPropertyTypes.Any(x => x.Alias == alias); /// - public override bool AddPropertyGroup(string name, string alias) - { - return AddAndReturnPropertyGroup(name, alias) != null; - } + public override bool AddPropertyGroup(string alias, string name) => AddAndReturnPropertyGroup(alias, name) != null; - private PropertyGroup AddAndReturnPropertyGroup(string name, string alias) + private PropertyGroup AddAndReturnPropertyGroup(string alias, string name) { // Ensure we don't have it already if (PropertyGroups.Contains(alias)) @@ -216,8 +216,8 @@ namespace Umbraco.Cms.Core.Models // Add new group var group = new PropertyGroup(SupportsPublishing) { - Name = name, - Alias = alias + Alias = alias, + Name = name }; // check if it is inherited - there might be more than 1 but we want the 1st, to @@ -244,7 +244,7 @@ namespace Umbraco.Cms.Core.Models } /// - public override bool AddPropertyType(IPropertyType propertyType, string groupAlias, string groupName) + public override bool AddPropertyType(IPropertyType propertyType, string propertyGroupAlias, string propertyGroupName = null) { // ensure no duplicate alias - over all composition properties if (PropertyTypeExists(propertyType.Alias)) @@ -252,15 +252,16 @@ namespace Umbraco.Cms.Core.Models // get and ensure a group local to this content type PropertyGroup group; - var index = PropertyGroups.IndexOfKey(groupAlias); + var index = PropertyGroups.IndexOfKey(propertyGroupAlias); if (index != -1) { group = PropertyGroups[index]; } - else if (!string.IsNullOrEmpty(groupName)) + else if (!string.IsNullOrEmpty(propertyGroupName)) { - group = AddAndReturnPropertyGroup(groupName, groupAlias); - if (group == null) return false; + group = AddAndReturnPropertyGroup(propertyGroupAlias, propertyGroupName); + if (group == null) + return false; } else { @@ -281,11 +282,9 @@ namespace Umbraco.Cms.Core.Models /// An enumerable list of string aliases /// Does not contain the alias of the Current ContentType public IEnumerable CompositionAliases() - { - return ContentTypeComposition + => ContentTypeComposition .Select(x => x.Alias) .Union(ContentTypeComposition.SelectMany(x => x.CompositionAliases())); - } /// /// Gets a list of ContentType Ids from the current composition @@ -293,11 +292,9 @@ namespace Umbraco.Cms.Core.Models /// An enumerable list of integer ids /// Does not contain the Id of the Current ContentType public IEnumerable CompositionIds() - { - return ContentTypeComposition + => ContentTypeComposition .Select(x => x.Id) .Union(ContentTypeComposition.SelectMany(x => x.CompositionIds())); - } protected override void PerformDeepClone(object clone) { @@ -305,7 +302,7 @@ namespace Umbraco.Cms.Core.Models var clonedEntity = (ContentTypeCompositionBase)clone; - //need to manually assign since this is an internal field and will not be automatically mapped + // need to manually assign since this is an internal field and will not be automatically mapped clonedEntity._removedContentTypeKeyTracker = new List(); clonedEntity._contentTypeComposition = ContentTypeComposition.Select(x => (IContentTypeComposition)x.DeepClone()).ToList(); } diff --git a/src/Umbraco.Core/Models/IContentTypeBase.cs b/src/Umbraco.Core/Models/IContentTypeBase.cs index 6b55464a2c..bb5bd1182f 100644 --- a/src/Umbraco.Core/Models/IContentTypeBase.cs +++ b/src/Umbraco.Core/Models/IContentTypeBase.cs @@ -1,4 +1,3 @@ -using System; using System.Collections.Generic; using Umbraco.Cms.Core.Models.Entities; @@ -113,43 +112,32 @@ namespace Umbraco.Cms.Core.Models /// /// Removes a PropertyType from the current ContentType /// - /// Alias of the to remove - void RemovePropertyType(string propertyTypeAlias); + /// Alias of the to remove + void RemovePropertyType(string alias); /// /// Removes a property group from the current content type. /// - /// Name of the to remove - void RemovePropertyGroup(string propertyGroupName); // TODO Rename to propertyGroupAlias + /// Alias of the to remove + void RemovePropertyGroup(string alias); /// /// Checks whether a PropertyType with a given alias already exists /// - /// Alias of the PropertyType + /// Alias of the PropertyType /// Returns True if a PropertyType with the passed in alias exists, otherwise False - bool PropertyTypeExists(string propertyTypeAlias); - - /// - /// Adds the property type to the specified property group (creates a new group if not found). - /// - /// The property type to add. - /// The name of the property group to add the property type to. - /// - /// Returns true if the property type was added; otherwise, false. - /// - [Obsolete("Use AddPropertyType(propertyType, groupAlias, groupName) instead to explicitly set the alias of the group (note the slighty different parameter order).")] - bool AddPropertyType(IPropertyType propertyType, string propertyGroupName); + bool PropertyTypeExists(string alias); /// /// Adds the property type to the specified property group (creates a new group if not found and a name is specified). /// /// The property type to add. - /// The alias of the property group to add the property type to. - /// The name of the property group to create when not found. + /// The alias of the property group to add the property type to. + /// The name of the property group to create when not found. /// /// Returns true if the property type was added; otherwise, false. /// - bool AddPropertyType(IPropertyType propertyType, string groupAlias, string groupName); // TODO Make groupName optional (add null as default value) after removing obsolete overload + bool AddPropertyType(IPropertyType propertyType, string propertyGroupAlias, string propertyGroupName = null); /// /// Adds a PropertyType, which does not belong to a PropertyGroup. @@ -158,39 +146,26 @@ namespace Umbraco.Cms.Core.Models /// Returns True if PropertyType was added, otherwise False bool AddPropertyType(IPropertyType propertyType); - /// - /// Adds a property group with the alias based on the specified . - /// - /// Name of the group. - /// - /// Returns true if a property group with specified was added; otherwise, false. - /// - /// - /// This method will also check if a group already exists with the same alias. - /// - [Obsolete("Use AddPropertyGroup(name, alias) instead to explicitly set the alias.")] - bool AddPropertyGroup(string groupName); - /// /// Adds a property group with the specified and . /// - /// Name of the group. /// The alias. + /// Name of the group. /// /// Returns true if a property group with specified was added; otherwise, false. /// /// /// This method will also check if a group already exists with the same alias. /// - bool AddPropertyGroup(string name, string alias); + bool AddPropertyGroup(string alias, string name); /// /// Moves a PropertyType to a specified PropertyGroup /// /// Alias of the PropertyType to move - /// Name of the PropertyGroup to move the PropertyType to + /// Alias of the PropertyGroup to move the PropertyType to /// - bool MovePropertyType(string propertyTypeAlias, string propertyGroupName); // TODO Rename to propertyGroupAlias + bool MovePropertyType(string propertyTypeAlias, string propertyGroupAlias); /// /// Gets an corresponding to this content type. diff --git a/src/Umbraco.Infrastructure/Packaging/PackageDataInstallation.cs b/src/Umbraco.Infrastructure/Packaging/PackageDataInstallation.cs index d0f39af493..05ceef6697 100644 --- a/src/Umbraco.Infrastructure/Packaging/PackageDataInstallation.cs +++ b/src/Umbraco.Infrastructure/Packaging/PackageDataInstallation.cs @@ -824,7 +824,7 @@ namespace Umbraco.Cms.Infrastructure.Packaging alias = name.ToSafeAlias(_shortStringHelper, true); } - contentType.AddPropertyGroup(name, alias); + contentType.AddPropertyGroup(alias, name); var propertyGroup = contentType.PropertyGroups[alias]; if (Guid.TryParse(propertyGroupElement.Element("Key")?.Value, out var key)) diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentTypeCommonRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentTypeCommonRepository.cs index 50411ab5d8..e171a3a54f 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentTypeCommonRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentTypeCommonRepository.cs @@ -255,7 +255,7 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement if (contentType is IMemberType memberType) { // ensure that the group exists (ok if it already exists) - memberType.AddPropertyGroup(Cms.Core.Constants.Conventions.Member.StandardPropertiesGroupName, Cms.Core.Constants.Conventions.Member.StandardPropertiesGroupAlias); + memberType.AddPropertyGroup(Cms.Core.Constants.Conventions.Member.StandardPropertiesGroupAlias, Cms.Core.Constants.Conventions.Member.StandardPropertiesGroupName); // ensure that property types exist (ok if they already exist) foreach (var (alias, propertyType) in builtinProperties) diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberTypeRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberTypeRepository.cs index b8cfe29dc8..e355964de6 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberTypeRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberTypeRepository.cs @@ -146,7 +146,7 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement } //By Convention we add 9 standard PropertyTypes to an Umbraco MemberType - entity.AddPropertyGroup(Cms.Core.Constants.Conventions.Member.StandardPropertiesGroupName, Cms.Core.Constants.Conventions.Member.StandardPropertiesGroupAlias); + entity.AddPropertyGroup(Cms.Core.Constants.Conventions.Member.StandardPropertiesGroupAlias, Cms.Core.Constants.Conventions.Member.StandardPropertiesGroupName); var standardPropertyTypes = ConventionsHelper.GetStandardPropertyTypeStubs(_shortStringHelper); foreach (var standardPropertyType in standardPropertyTypes) { diff --git a/src/Umbraco.TestData/UmbracoTestDataController.cs b/src/Umbraco.TestData/UmbracoTestDataController.cs index 4fb3a1a890..9214d179b0 100644 --- a/src/Umbraco.TestData/UmbracoTestDataController.cs +++ b/src/Umbraco.TestData/UmbracoTestDataController.cs @@ -269,7 +269,7 @@ namespace Umbraco.TestData Name = "Umbraco Test Data Content", Icon = "icon-science color-green" }; - docType.AddPropertyGroup("Content", "content"); + docType.AddPropertyGroup("content", "Content"); docType.AddPropertyType(new PropertyType(_shortStringHelper, GetOrCreateRichText(), "review") { Name = "Review" diff --git a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentTypeServiceTests.cs b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentTypeServiceTests.cs index e809992607..1a514a401a 100644 --- a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentTypeServiceTests.cs +++ b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentTypeServiceTests.cs @@ -1251,7 +1251,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services Assert.AreEqual(2, meta.PropertyTypes.Count()); Assert.AreEqual("Meta Keywords", meta.PropertyTypes.First().Name); Assert.AreEqual("Meta Description", meta.PropertyTypes.Skip(1).First().Name); - meta.AddPropertyGroup("Content", "content"); + meta.AddPropertyGroup("content", "Content"); Assert.AreEqual(2, meta.PropertyTypes.Count()); ContentTypeService.Save(meta); @@ -1488,8 +1488,8 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services public void Can_Remove_PropertyGroup_Without_Removing_Property_Types() { var basePage = (IContentType)ContentTypeBuilder.CreateBasicContentType(); - basePage.AddPropertyGroup("Content", "content"); - basePage.AddPropertyGroup("Meta", "meta"); + basePage.AddPropertyGroup("content", "Content"); + basePage.AddPropertyGroup("meta", "Meta"); ContentTypeService.Save(basePage); var authorPropertyType = new PropertyType(ShortStringHelper, Constants.PropertyEditors.Aliases.TextBox, ValueStorageType.Ntext, "author")