diff --git a/src/Umbraco.Core/Exceptions/InvalidCompositionException.cs b/src/Umbraco.Core/Exceptions/InvalidCompositionException.cs index fcf7a44677..bb9becf058 100644 --- a/src/Umbraco.Core/Exceptions/InvalidCompositionException.cs +++ b/src/Umbraco.Core/Exceptions/InvalidCompositionException.cs @@ -3,22 +3,41 @@ namespace Umbraco.Core.Exceptions { public class InvalidCompositionException : Exception - { - public string ContentTypeAlias { get; set; } + { + public InvalidCompositionException(string contentTypeAlias, string addedCompositionAlias, string[] propertyTypeAliass) + { + ContentTypeAlias = contentTypeAlias; + AddedCompositionAlias = addedCompositionAlias; + PropertyTypeAliases = propertyTypeAliass; + } - public string AddedCompositionAlias { get; set; } + public InvalidCompositionException(string contentTypeAlias, string[] propertyTypeAliass) + { + ContentTypeAlias = contentTypeAlias; + PropertyTypeAliases = propertyTypeAliass; + } - public string PropertyTypeAlias { get; set; } + public string ContentTypeAlias { get; private set; } + + public string AddedCompositionAlias { get; private set; } + + public string[] PropertyTypeAliases { get; private set; } public override string Message { get { - return string.Format( - "InvalidCompositionException - ContentType with alias '{0}' was added as a Compsition to ContentType with alias '{1}', " + - "but there was a conflict on the PropertyType alias '{2}'. " + + return AddedCompositionAlias.IsNullOrWhiteSpace() + ? string.Format( + "ContentType with alias '{0}' has an invalid composition " + + "and there was a conflict on the following PropertyTypes: '{1}'. " + "PropertyTypes must have a unique alias across all Compositions in order to compose a valid ContentType Composition.", - AddedCompositionAlias, ContentTypeAlias, PropertyTypeAlias); + ContentTypeAlias, string.Join(", ", PropertyTypeAliases)) + : string.Format( + "ContentType with alias '{0}' was added as a Composition to ContentType with alias '{1}', " + + "but there was a conflict on the following PropertyTypes: '{2}'. " + + "PropertyTypes must have a unique alias across all Compositions in order to compose a valid ContentType Composition.", + AddedCompositionAlias, ContentTypeAlias, string.Join(", ", PropertyTypeAliases)); } } } diff --git a/src/Umbraco.Core/Models/ContentTypeCompositionBase.cs b/src/Umbraco.Core/Models/ContentTypeCompositionBase.cs index d102f06483..ea5a8fbcbf 100644 --- a/src/Umbraco.Core/Models/ContentTypeCompositionBase.cs +++ b/src/Umbraco.Core/Models/ContentTypeCompositionBase.cs @@ -94,13 +94,7 @@ namespace Umbraco.Core.Models .Select(p => p.Alias)).ToList(); if (conflictingPropertyTypeAliases.Any()) - throw new InvalidCompositionException - { - AddedCompositionAlias = contentType.Alias, - ContentTypeAlias = Alias, - PropertyTypeAlias = - string.Join(", ", conflictingPropertyTypeAliases) - }; + throw new InvalidCompositionException(Alias, contentType.Alias, conflictingPropertyTypeAliases.ToArray()); _contentTypeComposition.Add(contentType); OnPropertyChanged(ContentTypeCompositionSelector); diff --git a/src/Umbraco.Core/Services/ContentTypeService.cs b/src/Umbraco.Core/Services/ContentTypeService.cs index f144fa51e4..fde2b06d8d 100644 --- a/src/Umbraco.Core/Services/ContentTypeService.cs +++ b/src/Umbraco.Core/Services/ContentTypeService.cs @@ -308,15 +308,28 @@ namespace Umbraco.Core.Services } - public void Validate(IContentTypeComposition compo) + /// + /// Validates the composition, if its invalid a list of property type aliases that were duplicated is returned + /// + /// + /// + public Attempt ValidateComposition(IContentTypeComposition compo) { using (new WriteLock(Locker)) { - ValidateLocked(compo); + try + { + ValidateLocked(compo); + return Attempt.Succeed(); + } + catch (InvalidCompositionException ex) + { + return Attempt.Fail(ex.PropertyTypeAliases, ex); + } } } - private void ValidateLocked(IContentTypeComposition compositionContentType) + protected void ValidateLocked(IContentTypeComposition compositionContentType) { // performs business-level validation of the composition // should ensure that it is absolutely safe to save the composition @@ -369,10 +382,8 @@ namespace Umbraco.Core.Services if (contentTypeDependency == null) continue; var intersect = contentTypeDependency.PropertyTypes.Select(x => x.Alias.ToLowerInvariant()).Intersect(propertyTypeAliases).ToArray(); if (intersect.Length == 0) continue; - - var message = string.Format("The following PropertyType aliases from the current ContentType conflict with existing PropertyType aliases: {0}.", - string.Join(", ", intersect)); - throw new Exception(message); + + throw new InvalidCompositionException(compositionContentType.Alias, intersect.ToArray()); } } diff --git a/src/Umbraco.Core/Services/IContentTypeService.cs b/src/Umbraco.Core/Services/IContentTypeService.cs index 1eb0b8695c..a68a5f7a6f 100644 --- a/src/Umbraco.Core/Services/IContentTypeService.cs +++ b/src/Umbraco.Core/Services/IContentTypeService.cs @@ -10,6 +10,13 @@ namespace Umbraco.Core.Services /// public interface IContentTypeService : IService { + /// + /// Validates the composition, if its invalid a list of property type aliases that were duplicated is returned + /// + /// + /// + Attempt ValidateComposition(IContentTypeComposition compo); + Attempt CreateFolder(int parentId, string name, int userId = 0); /// diff --git a/src/Umbraco.Web.UI.Client/src/common/services/util.service.js b/src/Umbraco.Web.UI.Client/src/common/services/util.service.js index 805ffd33ea..8d94a66793 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/util.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/util.service.js @@ -508,7 +508,7 @@ function umbDataFormatter() { var realProperties = _.reject(g.properties, function (p) { //do not include these properties - return p.propertyState === "init"; + return p.propertyState === "init" || p.inherited === true; }); var saveProperties = _.map(realProperties, function (p) { @@ -518,9 +518,19 @@ function umbDataFormatter() { saveGroup.properties = saveProperties; + //if this is an inherited group and there are not non-inherited properties on it, then don't send up the data + if (saveGroup.inherited === true && saveProperties.length === 0) { + return null; + } + return saveGroup; }); + //we don't want any null groups + saveModel.groups = _.reject(saveModel.groups, function(g) { + return !g; + }); + return saveModel; }, diff --git a/src/Umbraco.Web.UI.Client/src/views/components/umb-groups-builder.html b/src/Umbraco.Web.UI.Client/src/views/components/umb-groups-builder.html index c7fbcb9308..f628367cb3 100644 --- a/src/Umbraco.Web.UI.Client/src/views/components/umb-groups-builder.html +++ b/src/Umbraco.Web.UI.Client/src/views/components/umb-groups-builder.html @@ -96,10 +96,12 @@
- {{ property.alias }}
+ + server-validation-field="{{'Groups[' + $parent.$parent.$parent.$parent.$index + '].Properties[' + $index + '].Alias'}}">
@@ -107,7 +109,7 @@ name="groupName" umb-auto-resize required - val-server-field="{{'Groups[' + $parent.$index + '].Properties[' + $index + '].Label'}}"> + val-server-field="{{'Groups[' + $parent.$parent.$parent.$parent.$index + '].Properties[' + $index + '].Label'}}">
diff --git a/src/Umbraco.Web/Editors/ContentTypeController.cs b/src/Umbraco.Web/Editors/ContentTypeController.cs index dc47d15234..561f929706 100644 --- a/src/Umbraco.Web/Editors/ContentTypeController.cs +++ b/src/Umbraco.Web/Editors/ContentTypeController.cs @@ -89,7 +89,42 @@ namespace Umbraco.Web.Editors ? Request.CreateResponse(HttpStatusCode.OK, result.Result) //return the id : Request.CreateValidationErrorResponse(result.Exception.Message); } - + + /// + /// Validates the composition and adds errors to the model state if any are found then throws an error response if there are errors + /// + /// + /// + /// + private void ValidateComposition(ContentTypeSave contentTypeSave, IContentTypeComposition composition) + { + var validateAttempt = Services.ContentTypeService.ValidateComposition(composition); + if (validateAttempt == false) + { + //if it's not successful then we need to return some model state for the property aliases that + // are duplicated + var propertyAliases = validateAttempt.Result.Distinct(); + foreach (var propertyAlias in propertyAliases) + { + //find the property relating to these + var prop = contentTypeSave.Groups.SelectMany(x => x.Properties).Single(x => x.Alias == propertyAlias); + var group = contentTypeSave.Groups.Single(x => x.Properties.Contains(prop)); + var propIndex = group.Properties.IndexOf(prop); + var groupIndex = contentTypeSave.Groups.IndexOf(group); + + var key = string.Format("Groups[{0}].Properties[{1}].Alias", groupIndex, propIndex); + ModelState.AddModelError(key, "Duplicate property aliases not allowed between compositions"); + } + + var display = Mapper.Map(composition); + //map the 'save' data on top + display = Mapper.Map(contentTypeSave, display); + display.Errors = ModelState.ToErrorDictionary(); + throw new HttpResponseException(Request.CreateValidationErrorResponse(display)); + } + + } + public ContentTypeDisplay PostSave(ContentTypeSave contentTypeSave) { var ctId = Convert.ToInt32(contentTypeSave.Id); @@ -106,8 +141,6 @@ namespace Umbraco.Web.Editors forDisplay.Errors = ModelState.ToErrorDictionary(); throw new HttpResponseException(Request.CreateValidationErrorResponse(forDisplay)); } - - //TODO: Deal with validation for composition with property and group names/aliases //filter out empty properties contentTypeSave.Groups = contentTypeSave.Groups.Where(x => x.Name.IsNullOrWhiteSpace() == false).ToList(); @@ -129,8 +162,11 @@ namespace Umbraco.Web.Editors throw new HttpResponseException(HttpStatusCode.NotFound); Mapper.Map(contentTypeSave, found); - ctService.Save(found); + //NOTE: this throws an error response if it is not valid + ValidateComposition(contentTypeSave, found); + + ctService.Save(found); display = Mapper.Map(found); } else @@ -166,11 +202,16 @@ namespace Umbraco.Web.Editors //save as new var newCt = Mapper.Map(contentTypeSave); + + //NOTE: this throws an error response if it is not valid + ValidateComposition(contentTypeSave, newCt); + ctService.Save(newCt); //we need to save it twice to allow itself under itself. if (allowItselfAsChild) { + //NOTE: This will throw if the composition isn't right... but it shouldn't be at this stage newCt.AddContentType(newCt); ctService.Save(newCt); }