From 8a148d85357021d37fb0abdad817430e2b9166c1 Mon Sep 17 00:00:00 2001 From: SteveSandersonMS Date: Mon, 14 Dec 2015 12:02:41 +0000 Subject: [PATCH] Move from ValidationErrorResult to HttpBadRequest, and support object-level errors too --- .../npm/src/Validation.ts | 13 ++++--- .../ValidationErrorResult.cs | 30 --------------- .../MusicStore/Apis/AlbumsApiController.cs | 23 +++++++++-- .../Apis/Models/SentimentAnalysis.cs | 38 +++++++++++++++++++ .../admin/album-edit/album-edit.html | 1 + .../components/admin/album-edit/album-edit.ts | 6 ++- 6 files changed, 72 insertions(+), 39 deletions(-) delete mode 100644 Microsoft.AspNet.SpaServices/ValidationErrorResult.cs create mode 100644 samples/angular/MusicStore/Apis/Models/SentimentAnalysis.cs diff --git a/Microsoft.AspNet.AngularServices/npm/src/Validation.ts b/Microsoft.AspNet.AngularServices/npm/src/Validation.ts index 5ed2beb..b6d7535 100644 --- a/Microsoft.AspNet.AngularServices/npm/src/Validation.ts +++ b/Microsoft.AspNet.AngularServices/npm/src/Validation.ts @@ -14,16 +14,19 @@ export class Validation { var errors = response; Object.keys(errors || {}).forEach(key => { errors[key].forEach(errorMessage => { - // This in particular is rough - if (!controlGroup.controls[key].errors) { - (controlGroup.controls[key])._errors = {}; + // If there's a specific control for this key, then use it. Otherwise associate the error + // with the whole control group. + var control = controlGroup.controls[key] || controlGroup; + + // This is rough. Need to find out if there's a better way, or if this is even supported. + if (!control.errors) { + (control)._errors = {}; } - controlGroup.controls[key].errors[errorMessage] = true; + control.errors[errorMessage] = true; }); }); } - } export interface ValidationErrorResult { diff --git a/Microsoft.AspNet.SpaServices/ValidationErrorResult.cs b/Microsoft.AspNet.SpaServices/ValidationErrorResult.cs deleted file mode 100644 index 9cd0ba1..0000000 --- a/Microsoft.AspNet.SpaServices/ValidationErrorResult.cs +++ /dev/null @@ -1,30 +0,0 @@ -using System.Collections.Generic; -using System.Linq; -using Microsoft.AspNet.Mvc; -using Microsoft.AspNet.Mvc.ModelBinding; - -namespace Microsoft.AspNet.SpaServices -{ - public class ValidationErrorResult : ObjectResult { - public const int DefaultStatusCode = 400; - - public ValidationErrorResult(ModelStateDictionary modelState, int errorStatusCode = DefaultStatusCode) - : base(CreateResultObject(modelState)) - { - if (!modelState.IsValid) { - this.StatusCode = errorStatusCode; - } - } - - private static IDictionary> CreateResultObject(ModelStateDictionary modelState) - { - if (modelState.IsValid) { - return null; - } else { - return modelState - .Where(m => m.Value.Errors.Any()) - .ToDictionary(m => m.Key, m => m.Value.Errors.Select(me => me.ErrorMessage)); - } - } - } -} diff --git a/samples/angular/MusicStore/Apis/AlbumsApiController.cs b/samples/angular/MusicStore/Apis/AlbumsApiController.cs index 3b57e0b..55c4518 100644 --- a/samples/angular/MusicStore/Apis/AlbumsApiController.cs +++ b/samples/angular/MusicStore/Apis/AlbumsApiController.cs @@ -1,4 +1,6 @@ -using System.Linq; +using System.ComponentModel.DataAnnotations; +using System.Collections.Generic; +using System.Linq; using System.Threading.Tasks; using Microsoft.AspNet.Authorization; using Microsoft.AspNet.Mvc; @@ -118,7 +120,7 @@ namespace MusicStore.Apis if (!ModelState.IsValid) { // Return the model errors - return new ValidationErrorResult(ModelState); + return HttpBadRequest(ModelState); } var dbAlbum = await _storeContext.Albums.SingleOrDefaultAsync(a => a.AlbumId == albumId); @@ -165,7 +167,7 @@ namespace MusicStore.Apis } [ModelMetadataType(typeof(Album))] - public class AlbumChangeDto + public class AlbumChangeDto : IValidatableObject { public int GenreId { get; set; } @@ -176,6 +178,21 @@ namespace MusicStore.Apis public decimal Price { get; set; } public string AlbumArtUrl { get; set; } + + public IEnumerable Validate(ValidationContext validationContext) + { + // An example of object-level (i.e., multi-property) validation + if (this.GenreId == 13 /* Indie */) { + switch (SentimentAnalysis.GetSentiment(Title)) { + case SentimentAnalysis.SentimentResult.Positive: + yield return new ValidationResult("Sounds too positive. Indie music requires more ambiguity."); + break; + case SentimentAnalysis.SentimentResult.Negative: + yield return new ValidationResult("Sounds too negative. Indie music requires more ambiguity."); + break; + } + } + } } public class AlbumResultDto : AlbumChangeDto diff --git a/samples/angular/MusicStore/Apis/Models/SentimentAnalysis.cs b/samples/angular/MusicStore/Apis/Models/SentimentAnalysis.cs new file mode 100644 index 0000000..e505d2e --- /dev/null +++ b/samples/angular/MusicStore/Apis/Models/SentimentAnalysis.cs @@ -0,0 +1,38 @@ +using System.Linq; +using System.Text.RegularExpressions; + +namespace MusicStore.Models +{ + // Obviously this is not a serious sentiment analyser. It is only here to provide an amusing demonstration of cross-property + // validation in AlbumsApiController. + public static class SentimentAnalysis + { + private static string[] positiveSentimentWords = new[] { "happy", "fun", "joy", "love", "delight", "bunny", "bunnies", "asp.net" }; + + private static string[] negativeSentimentWords = new[] { "sad", "pain", "despair", "hate", "scorn", "death", "package management" }; + + public static SentimentResult GetSentiment(string text) { + var numPositiveWords = CountWordOccurrences(text, positiveSentimentWords); + var numNegativeWords = CountWordOccurrences(text, negativeSentimentWords); + if (numPositiveWords > numNegativeWords) { + return SentimentResult.Positive; + } else if (numNegativeWords > numPositiveWords) { + return SentimentResult.Negative; + } else { + return SentimentResult.Neutral; + } + } + + private static int CountWordOccurrences(string text, string[] words) + { + // Very simplistic matching technique for this sample. Not scalable and not really even correct. + return new Regex(string.Join("|", words), RegexOptions.IgnoreCase).Matches(text).Count; + } + + public enum SentimentResult { + Negative, + Neutral, + Positive, + } + } +} \ No newline at end of file diff --git a/samples/angular/MusicStore/wwwroot/ng-app/components/admin/album-edit/album-edit.html b/samples/angular/MusicStore/wwwroot/ng-app/components/admin/album-edit/album-edit.html index f32b27a..5b26fbc 100644 --- a/samples/angular/MusicStore/wwwroot/ng-app/components/admin/album-edit/album-edit.html +++ b/samples/angular/MusicStore/wwwroot/ng-app/components/admin/album-edit/album-edit.html @@ -37,6 +37,7 @@
Done! Your changes were saved.
+
{{ errorMessage }}
Back to List diff --git a/samples/angular/MusicStore/wwwroot/ng-app/components/admin/album-edit/album-edit.ts b/samples/angular/MusicStore/wwwroot/ng-app/components/admin/album-edit/album-edit.ts index be2929a..7f1b4e0 100644 --- a/samples/angular/MusicStore/wwwroot/ng-app/components/admin/album-edit/album-edit.ts +++ b/samples/angular/MusicStore/wwwroot/ng-app/components/admin/album-edit/album-edit.ts @@ -69,7 +69,7 @@ export class AlbumEdit { var albumId = this.originalAlbum.AlbumId; this._putJson(`/api/albums/${ albumId }/update`, this.form.value).subscribe(response => { - if (response.ok) { + if (response.status === 200) { this.changesSaved = true; } else { AspNet.Validation.showValidationErrors(response, this.form); @@ -88,6 +88,10 @@ export class AlbumEdit { headers: new Headers({ 'Content-Type': 'application/json' }) }); } + + public get formErrors() { + return Object.keys(this.form.errors || {}); + } } // TODO: Figure out what type declaration is provided by Angular/RxJs and use that instead