diff --git a/src/Umbraco.Tests/Web/Validation/ContentModelValidatorTests.cs b/src/Umbraco.Tests/Web/Validation/ContentModelValidatorTests.cs index 49a18574fe..f385fbcba6 100644 --- a/src/Umbraco.Tests/Web/Validation/ContentModelValidatorTests.cs +++ b/src/Umbraco.Tests/Web/Validation/ContentModelValidatorTests.cs @@ -95,35 +95,84 @@ namespace Umbraco.Tests.Web.Validation } [Test] - public void TestSerializer() + public void Test_Serializer() { var nestedLevel2 = new ComplexEditorValidationResult(); - var elementTypeResult2 = new ComplexEditorElementTypeValidationResult("type2"); - var propertyTypeResult2 = new ComplexEditorPropertyTypeValidationResult("prop2"); - propertyTypeResult2.ValidationResults.Add(new ValidationResult("error2-1", new[] { "level2" })); - propertyTypeResult2.ValidationResults.Add(new ValidationResult("error2-2", new[] { "level2" })); - elementTypeResult2.ValidationResults.Add(propertyTypeResult2); - nestedLevel2.ValidationResults.Add(elementTypeResult2); + var id1 = Guid.NewGuid(); + var addressInfoElementTypeResult = new ComplexEditorElementTypeValidationResult("addressInfo", id1); + var cityPropertyTypeResult = new ComplexEditorPropertyTypeValidationResult("city"); + cityPropertyTypeResult.AddValidationResult(new ValidationResult("City is invalid")); + cityPropertyTypeResult.AddValidationResult(new ValidationResult("City cannot be empty")); + cityPropertyTypeResult.AddValidationResult(new ValidationResult("City is not in Australia", new[] { "country" })); + cityPropertyTypeResult.AddValidationResult(new ValidationResult("Not a capital city", new[] { "capital" })); + addressInfoElementTypeResult.ValidationResults.Add(cityPropertyTypeResult); + nestedLevel2.ValidationResults.Add(addressInfoElementTypeResult); var nestedLevel1 = new ComplexEditorValidationResult(); - var elementTypeResult1 = new ComplexEditorElementTypeValidationResult("type1"); - var propertyTypeResult1 = new ComplexEditorPropertyTypeValidationResult("prop1"); - propertyTypeResult1.ValidationResults.Add(new ValidationResult("error1-1", new[] { "level1" })); - propertyTypeResult1.ValidationResults.Add(nestedLevel2); // This is a nested result within the level 1 - elementTypeResult1.ValidationResults.Add(propertyTypeResult1); - nestedLevel1.ValidationResults.Add(elementTypeResult1); + var id2 = Guid.NewGuid(); + var addressBookElementTypeResult = new ComplexEditorElementTypeValidationResult("addressBook", id2); + var addressesPropertyTypeResult = new ComplexEditorPropertyTypeValidationResult("addresses"); + addressesPropertyTypeResult.AddValidationResult(new ValidationResult("Must have at least 3 addresses", new[] { "counter" })); + addressesPropertyTypeResult.AddValidationResult(nestedLevel2); // This is a nested result within the level 1 + addressBookElementTypeResult.ValidationResults.Add(addressesPropertyTypeResult); + var bookNamePropertyTypeResult = new ComplexEditorPropertyTypeValidationResult("bookName"); + bookNamePropertyTypeResult.AddValidationResult(new ValidationResult("Invalid address book name", new[] { "book" })); + addressBookElementTypeResult.ValidationResults.Add(bookNamePropertyTypeResult); + nestedLevel1.ValidationResults.Add(addressBookElementTypeResult); + + var id3 = Guid.NewGuid(); + var addressBookElementTypeResult2 = new ComplexEditorElementTypeValidationResult("addressBook", id3); + var addressesPropertyTypeResult2 = new ComplexEditorPropertyTypeValidationResult("addresses"); + addressesPropertyTypeResult2.AddValidationResult(new ValidationResult("Must have at least 2 addresses", new[] { "counter" })); + addressBookElementTypeResult2.ValidationResults.Add(addressesPropertyTypeResult); + var bookNamePropertyTypeResult2 = new ComplexEditorPropertyTypeValidationResult("bookName"); + bookNamePropertyTypeResult2.AddValidationResult(new ValidationResult("Name is too long")); + addressBookElementTypeResult2.ValidationResults.Add(bookNamePropertyTypeResult2); + nestedLevel1.ValidationResults.Add(addressBookElementTypeResult2); var serialized = JsonConvert.SerializeObject(nestedLevel1, Formatting.Indented, new ValidationResultConverter()); Console.WriteLine(serialized); - var jsonNestedError = JsonConvert.DeserializeObject(serialized); - Assert.AreEqual(JTokenType.Array, jsonNestedError["nestedValidation"].Type); - var nestedValidation = (JArray)jsonNestedError["nestedValidation"]; - AssertNestedValidation(nestedValidation); + var jsonError = JsonConvert.DeserializeObject(serialized); + + Assert.IsNotNull(jsonError.SelectToken("$[0]")); + Assert.AreEqual(id2.ToString(), jsonError.SelectToken("$[0].$id").Value()); + Assert.AreEqual("addressBook", jsonError.SelectToken("$[0].$elementTypeAlias").Value()); + Assert.IsNotNull(jsonError.SelectToken("$[0].ModelState")); + var error1 = jsonError.SelectToken("$[0].ModelState['_Properties.addresses.invariant.null.counter']") as JArray; + Assert.IsNotNull(error1); + Assert.AreEqual(1, error1.Count); + var error2 = jsonError.SelectToken("$[0].ModelState['_Properties.bookName.invariant.null.book']") as JArray; + Assert.IsNotNull(error2); + Assert.AreEqual(1, error2.Count); + + Assert.AreEqual(id3.ToString(), jsonError.SelectToken("$[1].$id").Value()); + Assert.AreEqual("addressBook", jsonError.SelectToken("$[1].$elementTypeAlias").Value()); + Assert.IsNotNull(jsonError.SelectToken("$[1].ModelState")); + var error6 = jsonError.SelectToken("$[1].ModelState['_Properties.addresses.invariant.null.counter']") as JArray; + Assert.IsNotNull(error6); + Assert.AreEqual(1, error6.Count); + var error7 = jsonError.SelectToken("$[1].ModelState['_Properties.bookName.invariant.null']") as JArray; + Assert.IsNotNull(error7); + Assert.AreEqual(1, error7.Count); + + Assert.IsNotNull(jsonError.SelectToken("$[0].addresses")); + Assert.AreEqual(id1.ToString(), jsonError.SelectToken("$[0].addresses[0].$id").Value()); + Assert.AreEqual("addressInfo", jsonError.SelectToken("$[0].addresses[0].$elementTypeAlias").Value()); + Assert.IsNotNull(jsonError.SelectToken("$[0].addresses[0].ModelState")); + var error3 = jsonError.SelectToken("$[0].addresses[0].ModelState['_Properties.city.invariant.null.country']") as JArray; + Assert.IsNotNull(error3); + Assert.AreEqual(1, error3.Count); + var error4 = jsonError.SelectToken("$[0].addresses[0].ModelState['_Properties.city.invariant.null.capital']") as JArray; + Assert.IsNotNull(error4); + Assert.AreEqual(1, error4.Count); + var error5 = jsonError.SelectToken("$[0].addresses[0].ModelState['_Properties.city.invariant.null']") as JArray; + Assert.IsNotNull(error5); + Assert.AreEqual(2, error5.Count); } [Test] - public void Test() + public void Validating_ContentItemSave() { var validator = new ContentSaveModelValidator( Factory.GetInstance(), @@ -132,22 +181,26 @@ namespace Umbraco.Tests.Web.Validation var content = MockedContent.CreateTextpageContent(_contentType, "test", -1); + var id1 = new Guid("c8df5136-d606-41f0-9134-dea6ae0c2fd9"); + var id2 = new Guid("f916104a-4082-48b2-a515-5c4bf2230f38"); + var id3 = new Guid("77E15DE9-1C79-47B2-BC60-4913BC4D4C6A"); + // TODO: Ok now test with a 4th level complex nested editor - const string complexValue = @"[{ - ""key"": ""c8df5136-d606-41f0-9134-dea6ae0c2fd9"", + var complexValue = @"[{ + ""key"": """ + id1.ToString() + @""", ""name"": ""Hello world"", ""ncContentTypeAlias"": """ + ContentTypeAlias + @""", ""title"": ""Hello world"", ""bodyText"": ""The world is round"" }, { - ""key"": ""f916104a-4082-48b2-a515-5c4bf2230f38"", + ""key"": """ + id2.ToString() + @""", ""name"": ""Super nested"", ""ncContentTypeAlias"": """ + ContentTypeAlias + @""", ""title"": ""Hi there!"", ""bodyText"": ""Well hello there"", ""complex"" : [{ - ""key"": ""77E15DE9-1C79-47B2-BC60-4913BC4D4C6A"", + ""key"": """ + id3.ToString() + @""", ""name"": ""I am a sub nested content"", ""ncContentTypeAlias"": """ + ContentTypeAlias + @""", ""title"": ""Hello up there :)"", @@ -224,50 +277,31 @@ namespace Umbraco.Tests.Web.Validation } var complexEditorErrors = modelState.Single(x => x.Key == complexPropertyKey).Value.Errors; Assert.AreEqual(1, complexEditorErrors.Count); - var nestedError = complexEditorErrors.Single(x => x.ErrorMessage.Contains("nestedValidation")); - var jsonNestedError = JsonConvert.DeserializeObject(nestedError.ErrorMessage); - Assert.AreEqual(JTokenType.Array, jsonNestedError["nestedValidation"].Type); - var nestedValidation = (JArray)jsonNestedError["nestedValidation"]; - AssertNestedValidation(nestedValidation); + var nestedError = complexEditorErrors[0]; + var jsonError = JsonConvert.DeserializeObject(nestedError.ErrorMessage); + + var modelStateKeys = new[] { "_Properties.title.invariant.null.innerFieldId", "_Properties.title.invariant.null.value", "_Properties.bodyText.invariant.null.innerFieldId", "_Properties.bodyText.invariant.null.value" }; + AssertNestedValidation(jsonError, 0, id1, modelStateKeys); + AssertNestedValidation(jsonError, 1, id2, modelStateKeys.Concat(new[] { "_Properties.complex.invariant.null.innerFieldId", "_Properties.complex.invariant.null.value" }).ToArray()); + var nestedJsonError = jsonError.SelectToken("$[1].complex") as JArray; + Assert.IsNotNull(nestedJsonError); + AssertNestedValidation(nestedJsonError, 0, id3, modelStateKeys); } - private void AssertNestedValidation(JArray nestedValidation) + private void AssertNestedValidation(JArray jsonError, int index, Guid id, string[] modelStateKeys) { - Assert.Greater(nestedValidation.Count, 0); - foreach (var rowErrors in nestedValidation) + Assert.IsNotNull(jsonError.SelectToken("$[" + index + "]")); + Assert.AreEqual(id.ToString(), jsonError.SelectToken("$[" + index + "].$id").Value()); + Assert.AreEqual("textPage", jsonError.SelectToken("$[" + index + "].$elementTypeAlias").Value()); + Assert.IsNotNull(jsonError.SelectToken("$[" + index + "].ModelState")); + foreach (var key in modelStateKeys) { - Assert.AreEqual(JTokenType.Object, rowErrors.Type); - var elementTypeErrors = (JObject)rowErrors; // this is a dictionary of element type alias -> dictionary of errors -> prop alias -> array errors - Assert.AreEqual(1, elementTypeErrors.Count); // there is 1 element type in error - foreach (var elementTypeAliasToErrors in elementTypeErrors) - { - Assert.IsNotEmpty(elementTypeAliasToErrors.Key); - var propErrors = (JObject)elementTypeAliasToErrors.Value; - - foreach (var propAliasToErrors in propErrors) - { - Assert.AreEqual(JTokenType.Array, propAliasToErrors.Value.Type); - var propTypeErrors = (JArray)propAliasToErrors.Value; - - foreach (var propError in propTypeErrors) - { - var nested = propError["nestedValidation"]; - if (nested != null) - { - // recurse - AssertNestedValidation((JArray)nested); - continue; - } - - Assert.IsNotEmpty(propError["errorMessage"].Value()); - Assert.AreEqual(1, propError["memberNames"].Value().Count); - } - } - } + var error = jsonError.SelectToken("$[" + index + "].ModelState['" + key + "']") as JArray; + Assert.IsNotNull(error); + Assert.AreEqual(1, error.Count); } } - [HideFromTypeFinder] [DataEditor("complexTest", "test", "test")] public class ComplexTestEditor : NestedContentPropertyEditor diff --git a/src/Umbraco.Web/ModelStateExtensions.cs b/src/Umbraco.Web/ModelStateExtensions.cs index c135c41657..6bbf58fbec 100644 --- a/src/Umbraco.Web/ModelStateExtensions.cs +++ b/src/Umbraco.Web/ModelStateExtensions.cs @@ -42,9 +42,9 @@ namespace Umbraco.Web { return state.Where(v => v.Key.StartsWith(prefix + ".")).All(v => !v.Value.Errors.Any()); } - + /// - /// Adds the error to model state correctly for a property so we can use it on the client side. + /// Adds an error to model state for a property so we can use it on the client side. /// /// /// @@ -52,9 +52,23 @@ namespace Umbraco.Web /// The culture for the property, if the property is invariant than this is empty internal static void AddPropertyError(this System.Web.Http.ModelBinding.ModelStateDictionary modelState, ValidationResult result, string propertyAlias, string culture = "", string segment = "") + { + modelState.AddPropertyValidationError(new ContentPropertyValidationResult(result), propertyAlias, culture, segment); + } + + /// + /// Adds the to the model state with the appropriate keys for property errors + /// + /// + /// + /// + /// + /// + internal static void AddPropertyValidationError(this System.Web.Http.ModelBinding.ModelStateDictionary modelState, + ValidationResult result, string propertyAlias, string culture = "", string segment = "") { modelState.AddValidationError( - new ContentPropertyValidationResult(result), + result, "_Properties", propertyAlias, //if the culture is null, we'll add the term 'invariant' as part of the key diff --git a/src/Umbraco.Web/PropertyEditors/BlockEditorPropertyEditor.cs b/src/Umbraco.Web/PropertyEditors/BlockEditorPropertyEditor.cs index 0c768bfa88..5aad81ab84 100644 --- a/src/Umbraco.Web/PropertyEditors/BlockEditorPropertyEditor.cs +++ b/src/Umbraco.Web/PropertyEditors/BlockEditorPropertyEditor.cs @@ -98,7 +98,7 @@ namespace Umbraco.Web.PropertyEditors { foreach (var row in _blockEditorValues.GetPropertyValues(value)) { - var elementValidation = new ElementTypeValidationModel(row.ContentTypeAlias); + var elementValidation = new ElementTypeValidationModel(row.ContentTypeAlias, row.Id); foreach (var prop in row.PropertyValues) { elementValidation.AddPropertyTypeValidation( @@ -174,7 +174,8 @@ namespace Umbraco.Web.PropertyEditors result.Add(new BlockValue { ContentTypeAlias = contentType.Alias, - PropertyValues = propValues + PropertyValues = propValues, + Id = ((GuidUdi)block.Udi).Guid }); } @@ -195,6 +196,7 @@ namespace Umbraco.Web.PropertyEditors /// internal class BlockValue { + public Guid Id { get; set; } public string ContentTypeAlias { get; set; } public IDictionary PropertyValues { get; set; } = new Dictionary(); } diff --git a/src/Umbraco.Web/PropertyEditors/ComplexEditorValidator.cs b/src/Umbraco.Web/PropertyEditors/ComplexEditorValidator.cs index 86f7ab42ad..365a3fff6b 100644 --- a/src/Umbraco.Web/PropertyEditors/ComplexEditorValidator.cs +++ b/src/Umbraco.Web/PropertyEditors/ComplexEditorValidator.cs @@ -59,7 +59,7 @@ namespace Umbraco.Web.PropertyEditors { foreach (var row in elements) { - var elementTypeValidationResult = new ComplexEditorElementTypeValidationResult(row.ElementTypeAlias); + var elementTypeValidationResult = new ComplexEditorElementTypeValidationResult(row.ElementTypeAlias, row.Id); foreach (var prop in row.PropertyTypeValidation) { @@ -68,7 +68,7 @@ namespace Umbraco.Web.PropertyEditors foreach (var validationResult in _propertyValidationService.ValidatePropertyValue(prop.PropertyType, prop.PostedValue)) { // add the result to the property results - propValidationResult.ValidationResults.Add(validationResult); + propValidationResult.AddValidationResult(validationResult); } // add the property results to the element type results @@ -97,9 +97,10 @@ namespace Umbraco.Web.PropertyEditors public class ElementTypeValidationModel { - public ElementTypeValidationModel(string elementTypeAlias) + public ElementTypeValidationModel(string elementTypeAlias, Guid id) { ElementTypeAlias = elementTypeAlias; + Id = id; } private List _list = new List(); @@ -107,6 +108,7 @@ namespace Umbraco.Web.PropertyEditors public IEnumerable PropertyTypeValidation => _list; public string ElementTypeAlias { get; } + public Guid Id { get; } public void AddPropertyTypeValidation(PropertyTypeValidationModel propValidation) => _list.Add(propValidation); } diff --git a/src/Umbraco.Web/PropertyEditors/NestedContentPropertyEditor.cs b/src/Umbraco.Web/PropertyEditors/NestedContentPropertyEditor.cs index 4a07f09cd4..4767dc19cd 100644 --- a/src/Umbraco.Web/PropertyEditors/NestedContentPropertyEditor.cs +++ b/src/Umbraco.Web/PropertyEditors/NestedContentPropertyEditor.cs @@ -272,7 +272,7 @@ namespace Umbraco.Web.PropertyEditors { foreach (var row in _nestedContentValues.GetPropertyValues(value)) { - var elementValidation = new ElementTypeValidationModel(row.ContentTypeAlias); + var elementValidation = new ElementTypeValidationModel(row.ContentTypeAlias, row.Id); foreach (var prop in row.PropertyValues) { elementValidation.AddPropertyTypeValidation( @@ -373,6 +373,9 @@ namespace Umbraco.Web.PropertyEditors /// internal class NestedContentRowValue { + [JsonProperty("key")] + public Guid Id{ get; set; } + [JsonProperty("name")] public string Name { get; set; } diff --git a/src/Umbraco.Web/PropertyEditors/Validation/ComplexEditorElementTypeValidationResult.cs b/src/Umbraco.Web/PropertyEditors/Validation/ComplexEditorElementTypeValidationResult.cs index d43c3ed13d..a7d69b1a40 100644 --- a/src/Umbraco.Web/PropertyEditors/Validation/ComplexEditorElementTypeValidationResult.cs +++ b/src/Umbraco.Web/PropertyEditors/Validation/ComplexEditorElementTypeValidationResult.cs @@ -1,4 +1,5 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using System.ComponentModel.DataAnnotations; namespace Umbraco.Web.PropertyEditors.Validation @@ -8,13 +9,15 @@ namespace Umbraco.Web.PropertyEditors.Validation /// public class ComplexEditorElementTypeValidationResult : ValidationResult { - public ComplexEditorElementTypeValidationResult(string elementTypeAlias) + public ComplexEditorElementTypeValidationResult(string elementTypeAlias, Guid blockId) : base(string.Empty) { ElementTypeAlias = elementTypeAlias; + BlockId = blockId; } public IList ValidationResults { get; } = new List(); public string ElementTypeAlias { get; } + public Guid BlockId { get; } } } diff --git a/src/Umbraco.Web/PropertyEditors/Validation/ComplexEditorPropertyTypeValidationResult.cs b/src/Umbraco.Web/PropertyEditors/Validation/ComplexEditorPropertyTypeValidationResult.cs index 20ba3a27d3..1872648f54 100644 --- a/src/Umbraco.Web/PropertyEditors/Validation/ComplexEditorPropertyTypeValidationResult.cs +++ b/src/Umbraco.Web/PropertyEditors/Validation/ComplexEditorPropertyTypeValidationResult.cs @@ -1,5 +1,7 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using System.ComponentModel.DataAnnotations; +using System.Linq; namespace Umbraco.Web.PropertyEditors.Validation { @@ -14,7 +16,17 @@ namespace Umbraco.Web.PropertyEditors.Validation PropertyTypeAlias = propertyTypeAlias; } - public IList ValidationResults { get; } = new List(); + private readonly List _validationResults = new List(); + + public void AddValidationResult(ValidationResult validationResult) + { + if (validationResult is ComplexEditorValidationResult && _validationResults.Any(x => x is ComplexEditorValidationResult)) + throw new InvalidOperationException($"Cannot add more than one {typeof(ComplexEditorValidationResult)}"); + + _validationResults.Add(validationResult); + } + + public IReadOnlyList ValidationResults => _validationResults; public string PropertyTypeAlias { get; } } } diff --git a/src/Umbraco.Web/PropertyEditors/Validation/ValidationResultConverter.cs b/src/Umbraco.Web/PropertyEditors/Validation/ValidationResultConverter.cs index ef3778bb75..af915569c0 100644 --- a/src/Umbraco.Web/PropertyEditors/Validation/ValidationResultConverter.cs +++ b/src/Umbraco.Web/PropertyEditors/Validation/ValidationResultConverter.cs @@ -4,6 +4,7 @@ using Newtonsoft.Json.Serialization; using System; using System.ComponentModel.DataAnnotations; using System.Linq; +using System.Web.Http.ModelBinding; using Umbraco.Core; namespace Umbraco.Web.PropertyEditors.Validation @@ -38,36 +39,64 @@ namespace Umbraco.Web.PropertyEditors.Validation if (validationResult is ComplexEditorValidationResult nestedResult && nestedResult.ValidationResults.Count > 0) { - // TODO: Change to the new validation structure - - - var jo = new JObject(); - // recurse to write out an array of ValidationResultCollection - var obj = JArray.FromObject(nestedResult.ValidationResults, camelCaseSerializer); - jo.Add("nestedValidation", obj); - jo.WriteTo(writer); + var ja = new JArray(); + foreach(var nested in nestedResult.ValidationResults) + { + // recurse to write out the ComplexEditorElementTypeValidationResult + var block = JObject.FromObject(nested, camelCaseSerializer); + ja.Add(block); + } + if (nestedResult.ValidationResults.Count > 0) + { + ja.WriteTo(writer); + } } else if (validationResult is ComplexEditorElementTypeValidationResult elementTypeValidationResult && elementTypeValidationResult.ValidationResults.Count > 0) { - var joElementType = new JObject(); - var joPropertyType = new JObject(); + var joElementType = new JObject + { + { "$id", elementTypeValidationResult.BlockId }, + { "$elementTypeAlias", elementTypeValidationResult.ElementTypeAlias } + }; + + var modelState = new ModelStateDictionary(); + // loop over property validations foreach (var propTypeResult in elementTypeValidationResult.ValidationResults) { - - // TODO: I think here we could do the ModelState thing? instead of recursing? We'd just have to - // not recurse if it was the exact type of the base class ValidationResult and build up the ModelState values - var ja = new JArray(); - foreach (var result in propTypeResult.ValidationResults) + // group the results by their type and iterate the groups + foreach (var result in propTypeResult.ValidationResults.GroupBy(x => x.GetType())) { - // recurse to get the validation result object and add to the array - var obj = JObject.FromObject(result, camelCaseSerializer); - ja.Add(obj); + // if the group's type isn't ComplexEditorValidationResult then it will in 99% of cases be + // just ValidationResult for whcih we want to create the sub "ModelState" data. If it's not a normal + // ValidationResult it will still just be converted to normal ModelState. + + if (result.Key == typeof(ComplexEditorValidationResult)) + { + // if it's ComplexEditorValidationResult then there can only be one which is validated so just get the single + if (result.Any()) + { + var complexResult = result.Single(); + // recurse to get the validation result object + var obj = JToken.FromObject(complexResult, camelCaseSerializer); + joElementType.Add(propTypeResult.PropertyTypeAlias, obj); + } + } + else + { + foreach (var v in result) + { + modelState.AddPropertyValidationError(v, propTypeResult.PropertyTypeAlias); + } + } } - // create a dictionary entry - joPropertyType.Add(propTypeResult.PropertyTypeAlias, ja); } - joElementType.Add(elementTypeValidationResult.ElementTypeAlias, joPropertyType); + + if (modelState.Count > 0) + { + joElementType.Add("ModelState", JObject.FromObject(modelState.ToErrorDictionary())); + } + joElementType.WriteTo(writer); } else