From eee1a9fef45a446dd4ce855a5377870ba30698b2 Mon Sep 17 00:00:00 2001 From: Doug Bunting Date: Fri, 2 Sep 2016 23:51:53 -0700 Subject: [PATCH] Do not generate a validation summary when `excludePropertyErrors` unless specific model has an error - #5209 - update affected `HtmlHelperValiationSummaryTest` and functional tests - add `ValidationSummaryTagHelperTest` tests to cover related scenarios ##### Behaviour changes when no errors exist for the model: ###### Tag helper ``` html

Oopsie

``` previously generated ``` html

Oopsie

``` and now generates ``` html

Oopsie

``` ###### HTML helper ``` c# @Html.ValidationSummary(excludePropertyErrors: true, message: "Oopsie") ``` previously generated ``` html
Oopsie
``` and now generates nothing (`@HtmlString.Empty`). --- .../ViewFeatures/DefaultHtmlGenerator.cs | 19 +- .../HtmlHelperOptionsTest.cs | 8 +- ...WebSite.HtmlGeneration_Customer.Index.html | 3 +- .../ValidationSummaryTagHelperTest.cs | 203 ++++++++++++++---- .../HtmlHelperValidationSummaryTest.cs | 32 ++- .../HtmlHelperOptionsController.cs | 2 + 6 files changed, 201 insertions(+), 66 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/DefaultHtmlGenerator.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/DefaultHtmlGenerator.cs index b63e755699..6b820edd15 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/DefaultHtmlGenerator.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/DefaultHtmlGenerator.cs @@ -816,9 +816,19 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures throw new ArgumentNullException(nameof(viewContext)); } - if (viewContext.ViewData.ModelState.IsValid && (!viewContext.ClientValidationEnabled || excludePropertyErrors)) + var viewData = viewContext.ViewData; + if (!viewContext.ClientValidationEnabled && viewData.ModelState.IsValid) { - // No client side validation/updates + // Client-side validation is not enabled to add to the generated element and element will be empty. + return null; + } + + ModelStateEntry entryForModel; + if (excludePropertyErrors && + (!viewData.ModelState.TryGetValue(viewData.TemplateInfo.HtmlFieldPrefix, out entryForModel) || + entryForModel.Errors.Count == 0)) + { + // Client-side validation (if enabled) will not affect the generated element and element will be empty. return null; } @@ -829,6 +839,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures { headerTag = viewContext.ValidationSummaryMessageElement; } + var messageTag = new TagBuilder(headerTag); messageTag.InnerHtml.SetContent(message); wrappedMessage.AppendLine(messageTag); @@ -841,7 +852,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures // If excludePropertyErrors is true, describe any validation issue with the current model in a single item. // Otherwise, list individual property errors. var isHtmlSummaryModified = false; - var modelStates = ValidationHelpers.GetModelStateList(viewContext.ViewData, excludePropertyErrors); + var modelStates = ValidationHelpers.GetModelStateList(viewData, excludePropertyErrors); var htmlSummary = new TagBuilder("ul"); foreach (var modelState in modelStates) @@ -871,7 +882,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures var tagBuilder = new TagBuilder("div"); tagBuilder.MergeAttributes(GetHtmlAttributeDictionaryOrNull(htmlAttributes)); - if (viewContext.ViewData.ModelState.IsValid) + if (viewData.ModelState.IsValid) { tagBuilder.AddCssClass(HtmlHelper.ValidationSummaryValidCssClassName); } diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/HtmlHelperOptionsTest.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/HtmlHelperOptionsTest.cs index d2bab1e71f..8354547edf 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/HtmlHelperOptionsTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/HtmlHelperOptionsTest.cs @@ -23,7 +23,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests // Arrange var expected = @"
MySummary -
An error occurred. @@ -31,7 +31,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
MySummary -
An error occurred. @@ -54,7 +54,7 @@ False"; // Arrange var expected = @"
MySummary -
An error occurred. @@ -63,7 +63,7 @@ False"; True
MySummary -
An error occurred. diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Customer.Index.html b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Customer.Index.html index 9026b3bc0d..26cbd426c8 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Customer.Index.html +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Customer.Index.html @@ -33,8 +33,7 @@
-
+
diff --git a/test/Microsoft.AspNetCore.Mvc.TagHelpers.Test/ValidationSummaryTagHelperTest.cs b/test/Microsoft.AspNetCore.Mvc.TagHelpers.Test/ValidationSummaryTagHelperTest.cs index 82b0ab58ec..6767d6d051 100644 --- a/test/Microsoft.AspNetCore.Mvc.TagHelpers.Test/ValidationSummaryTagHelperTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.TagHelpers.Test/ValidationSummaryTagHelperTest.cs @@ -4,7 +4,6 @@ using System; using System.Collections.Generic; using System.IO; -using System.Linq; using System.Threading.Tasks; using Microsoft.AspNetCore.Html; using Microsoft.AspNetCore.Http; @@ -42,8 +41,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers [Theory] [MemberData(nameof(ProcessAsync_GeneratesExpectedOutput_WithNoErrorsData))] - public async Task ProcessAsync_GeneratesExpectedOutput_WithNoErrors( - ModelStateDictionary modelState) + public async Task ProcessAsync_GeneratesExpectedOutput_WithNoErrors(ModelStateDictionary modelState) { // Arrange var expectedTagName = "not-div"; @@ -53,8 +51,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers var expectedPreContent = "original pre-content"; var expectedContent = "original content"; var tagHelperContext = new TagHelperContext( - allAttributes: new TagHelperAttributeList( - Enumerable.Empty()), + allAttributes: new TagHelperAttributeList(), items: new Dictionary(), uniqueId: "test"); var output = new TagHelperOutput( @@ -65,9 +62,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers }, getChildContentAsync: (useCachedResult, encoder) => { - var tagHelperContent = new DefaultTagHelperContent(); - tagHelperContent.SetContent("Something"); - return Task.FromResult(tagHelperContent); + throw new InvalidOperationException("getChildContentAsync called unexpectedly"); }); output.PreContent.SetContent(expectedPreContent); output.Content.SetContent(expectedContent); @@ -85,11 +80,18 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers await validationSummaryTagHelper.ProcessAsync(tagHelperContext, output); // Assert - Assert.Equal(2, output.Attributes.Count); - var attribute = Assert.Single(output.Attributes, attr => attr.Name.Equals("class")); - Assert.Equal("form-control validation-summary-valid", attribute.Value); - attribute = Assert.Single(output.Attributes, attr => attr.Name.Equals("data-valmsg-summary")); - Assert.Equal("true", attribute.Value); + Assert.Collection( + output.Attributes, + attribute => + { + Assert.Equal("class", attribute.Name); + Assert.Equal("form-control validation-summary-valid", attribute.Value); + }, + attribute => + { + Assert.Equal("data-valmsg-summary", attribute.Name); + Assert.Equal("true", attribute.Value); + }); Assert.Equal(expectedPreContent, output.PreContent.GetContent()); Assert.Equal(expectedContent, output.Content.GetContent()); Assert.Equal( @@ -98,6 +100,120 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers Assert.Equal(expectedTagName, output.TagName); } + [Theory] + [MemberData(nameof(ProcessAsync_GeneratesExpectedOutput_WithNoErrorsData))] + public async Task ProcessAsync_DoesNothingIfClientSideValiationDisabled_WithNoErrorsData( + ModelStateDictionary modelStateDictionary) + { + // Arrange + var metadataProvider = new TestModelMetadataProvider(); + var htmlGenerator = new TestableHtmlGenerator(metadataProvider); + var viewContext = CreateViewContext(); + viewContext.ClientValidationEnabled = false; + + var validationSummaryTagHelper = new ValidationSummaryTagHelper(htmlGenerator) + { + ValidationSummary = ValidationSummary.All, + ViewContext = viewContext, + }; + + var output = new TagHelperOutput( + "div", + new TagHelperAttributeList(), + (useCachedResult, encoder) => + { + throw new InvalidOperationException("getChildContentAsync called unexpectedly."); + }); + + var context = new TagHelperContext( + new TagHelperAttributeList(), + items: new Dictionary(), + uniqueId: "test"); + + // Act + await validationSummaryTagHelper.ProcessAsync(context, output); + + // Assert + Assert.Equal("div", output.TagName); + Assert.Empty(output.Attributes); + Assert.False(output.IsContentModified); + Assert.False(output.PreContent.IsModified); + Assert.False(output.PreElement.IsModified); + Assert.False(output.PostContent.IsModified); + Assert.False(output.PostElement.IsModified); + } + + public static TheoryData ProcessAsync_DoesNothingIfModelOnly_WithNoModelErrorData + { + get + { + var emptyModelState = new ModelStateDictionary(); + + var modelState = new ModelStateDictionary(); + SetValidModelState(modelState); + + var invalidModelState = new ModelStateDictionary(); + SetValidModelState(invalidModelState); + invalidModelState.AddModelError($"{nameof(Model.Strings)}[1]", "This value is invalid."); + + return new TheoryData + { + { string.Empty, emptyModelState }, + { string.Empty, modelState }, + { nameof(Model.Text), modelState }, + { "not-a-key", modelState }, + { string.Empty, invalidModelState }, + { $"{nameof(Model.Strings)}[2]", invalidModelState }, + { nameof(Model.Text), invalidModelState }, + { "not-a-key", invalidModelState }, + }; + } + } + + [Theory] + [MemberData(nameof(ProcessAsync_DoesNothingIfModelOnly_WithNoModelErrorData))] + public async Task ProcessAsync_DoesNothingIfModelOnly_WithNoModelError( + string prefix, + ModelStateDictionary modelStateDictionary) + { + // Arrange + var metadataProvider = new TestModelMetadataProvider(); + var htmlGenerator = new TestableHtmlGenerator(metadataProvider); + var viewContext = CreateViewContext(); + viewContext.ViewData.TemplateInfo.HtmlFieldPrefix = prefix; + + var validationSummaryTagHelper = new ValidationSummaryTagHelper(htmlGenerator) + { + ValidationSummary = ValidationSummary.ModelOnly, + ViewContext = viewContext, + }; + + var output = new TagHelperOutput( + "div", + new TagHelperAttributeList(), + (useCachedResult, encoder) => + { + throw new InvalidOperationException("getChildContentAsync called unexpectedly."); + }); + + var context = new TagHelperContext( + new TagHelperAttributeList(), + items: new Dictionary(), + uniqueId: "test"); + + // Act + await validationSummaryTagHelper.ProcessAsync(context, output); + + // Assert + Assert.Equal("div", output.TagName); + Assert.Empty(output.Attributes); + Assert.False(output.IsContentModified); + Assert.False(output.PreContent.IsModified); + Assert.False(output.PreElement.IsModified); + Assert.False(output.PostContent.IsModified); + Assert.False(output.PostElement.IsModified); + } + [Theory] [InlineData(ValidationSummary.All)] [InlineData(ValidationSummary.ModelOnly)] @@ -117,8 +233,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers var expectedPreContent = "original pre-content"; var expectedContent = "original content"; var tagHelperContext = new TagHelperContext( - allAttributes: new TagHelperAttributeList( - Enumerable.Empty()), + allAttributes: new TagHelperAttributeList(), items: new Dictionary(), uniqueId: "test"); var output = new TagHelperOutput( @@ -178,8 +293,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers var expectedPreContent = "original pre-content"; var expectedContent = "original content"; var tagHelperContext = new TagHelperContext( - allAttributes: new TagHelperAttributeList( - Enumerable.Empty()), + allAttributes: new TagHelperAttributeList(), items: new Dictionary(), uniqueId: "test"); var output = new TagHelperOutput( @@ -211,11 +325,18 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers await validationSummaryTagHelper.ProcessAsync(tagHelperContext, output); // Assert - Assert.Equal(2, output.Attributes.Count); - var attribute = Assert.Single(output.Attributes, attr => attr.Name.Equals("class")); - Assert.Equal("form-control validation-summary-errors", attribute.Value); - attribute = Assert.Single(output.Attributes, attr => attr.Name.Equals("data-valmsg-summary")); - Assert.Equal("true", attribute.Value); + Assert.Collection( + output.Attributes, + attribute => + { + Assert.Equal("class", attribute.Name); + Assert.Equal("form-control validation-summary-errors", attribute.Value); + }, + attribute => + { + Assert.Equal("data-valmsg-summary", attribute.Name); + Assert.Equal("true", attribute.Value); + }); Assert.Equal(expectedPreContent, output.PreContent.GetContent()); Assert.Equal(expectedContent, output.Content.GetContent()); Assert.Equal( @@ -266,8 +387,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers validationSummaryTagHelper.ViewContext = expectedViewContext; var context = new TagHelperContext( - allAttributes: new TagHelperAttributeList( - Enumerable.Empty()), + allAttributes: new TagHelperAttributeList(), items: new Dictionary(), uniqueId: "test"); @@ -288,9 +408,9 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers // Arrange var tagBuilder = new TagBuilder("span2"); tagBuilder.InnerHtml.SetHtmlContent("New HTML"); + tagBuilder.Attributes.Add("anything", "something"); tagBuilder.Attributes.Add("data-foo", "bar"); tagBuilder.Attributes.Add("data-hello", "world"); - tagBuilder.Attributes.Add("anything", "something"); var generator = new Mock(MockBehavior.Strict); generator @@ -322,8 +442,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers validationSummaryTagHelper.ViewContext = viewContext; var context = new TagHelperContext( - allAttributes: new TagHelperAttributeList( - Enumerable.Empty()), + allAttributes: new TagHelperAttributeList(), items: new Dictionary(), uniqueId: "test"); @@ -332,13 +451,23 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers // Assert Assert.Equal("div", output.TagName); - Assert.Equal(3, output.Attributes.Count); - var attribute = Assert.Single(output.Attributes, attr => attr.Name.Equals("data-foo")); - Assert.Equal("bar", attribute.Value); - attribute = Assert.Single(output.Attributes, attr => attr.Name.Equals("data-hello")); - Assert.Equal("world", attribute.Value); - attribute = Assert.Single(output.Attributes, attr => attr.Name.Equals("anything")); - Assert.Equal("something", attribute.Value); + Assert.Collection( + output.Attributes, + attribute => + { + Assert.Equal("anything", attribute.Name); + Assert.Equal("something", attribute.Value); + }, + attribute => + { + Assert.Equal("data-foo", attribute.Name); + Assert.Equal("bar", attribute.Value); + }, + attribute => + { + Assert.Equal("data-hello", attribute.Name); + Assert.Equal("world", attribute.Value); + }); Assert.Equal(expectedPreContent, output.PreContent.GetContent()); Assert.Equal(expectedContent, output.Content.GetContent()); Assert.Equal("Content of validation summaryNew HTML", output.PostContent.GetContent()); @@ -371,8 +500,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers validationSummaryTagHelper.ViewContext = viewContext; var context = new TagHelperContext( - allAttributes: new TagHelperAttributeList( - Enumerable.Empty()), + allAttributes: new TagHelperAttributeList(), items: new Dictionary(), uniqueId: "test"); @@ -427,8 +555,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers validationSummaryTagHelper.ViewContext = viewContext; var context = new TagHelperContext( - allAttributes: new TagHelperAttributeList( - Enumerable.Empty()), + allAttributes: new TagHelperAttributeList(), items: new Dictionary(), uniqueId: "test"); diff --git a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Rendering/HtmlHelperValidationSummaryTest.cs b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Rendering/HtmlHelperValidationSummaryTest.cs index e89698eb2a..5531f4da05 100644 --- a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Rendering/HtmlHelperValidationSummaryTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Rendering/HtmlHelperValidationSummaryTest.cs @@ -75,9 +75,6 @@ namespace Microsoft.AspNetCore.Mvc.Rendering { get { - var basicDiv = "
    " + - "
  • " + Environment.NewLine + - "
"; var divWithError = "
    " + "
  • HtmlEncode[[This is my validation message]]
    • " + Environment.NewLine + "
"; @@ -89,8 +86,8 @@ namespace Microsoft.AspNetCore.Mvc.Rendering { { false, false, divWithError, divWithError }, { false, true, divWithErrorAndSummary, divWithErrorAndSummary }, - { true, false, divWithError, basicDiv }, - { true, true, divWithError, basicDiv }, + { true, false, divWithError, string.Empty }, + { true, true, divWithError, string.Empty }, }; } } @@ -100,9 +97,6 @@ namespace Microsoft.AspNetCore.Mvc.Rendering { get { - var basicDiv = "
    " + - "
  • " + Environment.NewLine + - "
"; var divWithRootError = "
    " + "
  • HtmlEncode[[This is an error for the model root.]]
    • " + Environment.NewLine + "
  • HtmlEncode[[This is another error for the model root.]]
    • " + Environment.NewLine + @@ -129,7 +123,7 @@ namespace Microsoft.AspNetCore.Mvc.Rendering { false, "some.unrelated.prefix", divWithAllErrors }, { true, string.Empty, divWithRootError }, { true, "Property3", divWithProperty3Error }, - { true, "some.unrelated.prefix", basicDiv }, + { true, "some.unrelated.prefix", string.Empty }, }; } } @@ -526,11 +520,7 @@ namespace Microsoft.AspNetCore.Mvc.Rendering var validationSummaryResult = helper.ValidationSummary(excludePropertyErrors: true); // Assert - Assert.Equal( - "
    • " + - Environment.NewLine + - "
    ", - HtmlContentUtilities.HtmlContentToString(validationSummaryResult)); + Assert.Equal(HtmlString.Empty, validationSummaryResult); } [Fact] @@ -576,6 +566,7 @@ namespace Microsoft.AspNetCore.Mvc.Rendering { // Arrange var helper = DefaultTemplatesUtilities.GetHtmlHelper(); + helper.ViewData.ModelState.AddModelError(string.Empty, "Error for root"); helper.ViewData.ModelState.AddModelError("Property1", "Error for Property1"); // Act @@ -585,7 +576,7 @@ namespace Microsoft.AspNetCore.Mvc.Rendering Assert.Equal( "
    HtmlEncode[[Custom Message]]" + Environment.NewLine + - "
    • " + Environment.NewLine + + "
      • HtmlEncode[[Error for root]]
        • " + Environment.NewLine + "
    ", HtmlContentUtilities.HtmlContentToString(validationSummaryResult)); } @@ -633,6 +624,7 @@ namespace Microsoft.AspNetCore.Mvc.Rendering { // Arrange var helper = DefaultTemplatesUtilities.GetHtmlHelper(); + helper.ViewData.ModelState.AddModelError(string.Empty, "Error for root"); helper.ViewData.ModelState.AddModelError("Property1", "Error for Property1"); // Act @@ -642,7 +634,7 @@ namespace Microsoft.AspNetCore.Mvc.Rendering Assert.Equal( "
    HtmlEncode[[Custom Message]]
    " + Environment.NewLine + - "
    • " + Environment.NewLine + + "
      • HtmlEncode[[Error for root]]
        • " + Environment.NewLine + "
    ", HtmlContentUtilities.HtmlContentToString(validationSummaryResult)); } @@ -652,16 +644,20 @@ namespace Microsoft.AspNetCore.Mvc.Rendering { // Arrange var helper = DefaultTemplatesUtilities.GetHtmlHelper(); + helper.ViewData.ModelState.AddModelError(string.Empty, "Error for root"); helper.ViewData.ModelState.AddModelError("Property1", "Error for Property1"); // Act - var validationSummaryResult = helper.ValidationSummary(excludePropertyErrors: true, message: "Custom Message", htmlAttributes: new { attr = "value" }); + var validationSummaryResult = helper.ValidationSummary( + excludePropertyErrors: true, + message: "Custom Message", + htmlAttributes: new { attr = "value" }); // Assert Assert.Equal( "
    HtmlEncode[[Custom Message]]" + Environment.NewLine + - "
    • " + Environment.NewLine + + "
      • HtmlEncode[[Error for root]]
        • " + Environment.NewLine + "
    ", HtmlContentUtilities.HtmlContentToString(validationSummaryResult)); } diff --git a/test/WebSites/RazorWebSite/Controllers/HtmlHelperOptionsController.cs b/test/WebSites/RazorWebSite/Controllers/HtmlHelperOptionsController.cs index 461f6c7326..91b20ea824 100644 --- a/test/WebSites/RazorWebSite/Controllers/HtmlHelperOptionsController.cs +++ b/test/WebSites/RazorWebSite/Controllers/HtmlHelperOptionsController.cs @@ -23,6 +23,7 @@ namespace RazorWebSite.Controllers offset: TimeSpan.FromHours(0)) }; + ModelState.AddModelError(string.Empty, "A model error occurred."); ModelState.AddModelError("Error", "An error occurred."); return View(model); } @@ -42,6 +43,7 @@ namespace RazorWebSite.Controllers offset: TimeSpan.FromHours(0)) }; + ModelState.AddModelError(string.Empty, "A model error occurred."); ModelState.AddModelError("Error", "An error occurred."); return View(model); }