From 729177ffef69a061779edd048da89e4f6e9ed5b5 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 6 Sep 2018 20:33:32 +1000 Subject: [PATCH] Fixes issue with not showing validation errors in the dialogs when creating content and saving/publishing when the names haven't been filled out, adds unit tests to support. --- .../ControllerTesting/TestRunner.cs | 6 +- .../Web/Controllers/ContentControllerTests.cs | 111 ++++++++++++++++-- .../content/umbvariantcontent.directive.js | 1 + .../editor/umbeditorheader.directive.js | 11 +- .../content/umb-variant-content-editors.html | 1 + .../content/umb-variant-content.html | 3 +- .../components/editor/umb-editor-header.html | 4 +- src/Umbraco.Web/Editors/ContentController.cs | 24 +++- .../Models/Mapping/OwnerResolver.cs | 3 +- 9 files changed, 142 insertions(+), 22 deletions(-) diff --git a/src/Umbraco.Tests/TestHelpers/ControllerTesting/TestRunner.cs b/src/Umbraco.Tests/TestHelpers/ControllerTesting/TestRunner.cs index 64a22926a0..5834415568 100644 --- a/src/Umbraco.Tests/TestHelpers/ControllerTesting/TestRunner.cs +++ b/src/Umbraco.Tests/TestHelpers/ControllerTesting/TestRunner.cs @@ -58,14 +58,14 @@ namespace Umbraco.Tests.TestHelpers.ControllerTesting var response = await server.HttpClient.SendAsync(request); Console.WriteLine(response); - var json = ""; if (response.IsSuccessStatusCode == false) { WriteResponseError(response); } - else + + var json = (await ((StreamContent)response.Content).ReadAsStringAsync()).TrimStart(AngularJsonMediaTypeFormatter.XsrfPrefix); + if (!json.IsNullOrWhiteSpace()) { - json = (await ((StreamContent) response.Content).ReadAsStringAsync()).TrimStart(AngularJsonMediaTypeFormatter.XsrfPrefix); var deserialized = JsonConvert.DeserializeObject(json); Console.Write(JsonConvert.SerializeObject(deserialized, Formatting.Indented)); } diff --git a/src/Umbraco.Tests/Web/Controllers/ContentControllerTests.cs b/src/Umbraco.Tests/Web/Controllers/ContentControllerTests.cs index e74f034d22..f75371d203 100644 --- a/src/Umbraco.Tests/Web/Controllers/ContentControllerTests.cs +++ b/src/Umbraco.Tests/Web/Controllers/ContentControllerTests.cs @@ -29,6 +29,7 @@ using Umbraco.Web.PropertyEditors; using System; using Umbraco.Web.WebApi; using Umbraco.Web.Trees; +using System.Globalization; namespace Umbraco.Tests.Web.Controllers { @@ -45,6 +46,8 @@ namespace Umbraco.Tests.Web.Controllers var userServiceMock = new Mock(); userServiceMock.Setup(service => service.GetUserById(It.IsAny())) .Returns((int id) => id == 1234 ? new User(1234, "Test", "test@test.com", "test@test.com", "", new List(), new int[0], new int[0]) : null); + userServiceMock.Setup(x => x.GetProfileById(It.IsAny())) + .Returns((int id) => id == 1234 ? new User(1234, "Test", "test@test.com", "test@test.com", "", new List(), new int[0], new int[0]) : null); userServiceMock.Setup(service => service.GetPermissionsForPath(It.IsAny(), It.IsAny())) .Returns(new EntityPermissionSet(123, new EntityPermissionCollection(new[] { @@ -59,20 +62,30 @@ namespace Umbraco.Tests.Web.Controllers var entityService = new Mock(); entityService.Setup(x => x.GetAllPaths(UmbracoObjectTypes.Document, It.IsAny())) - .Returns((UmbracoObjectTypes objType, int[] ids) => ids.Select(x => new TreeEntityPath {Path = $"-1,{x}", Id = x}).ToList()); + .Returns((UmbracoObjectTypes objType, int[] ids) => ids.Select(x => new TreeEntityPath { Path = $"-1,{x}", Id = x }).ToList()); var dataTypeService = new Mock(); dataTypeService.Setup(service => service.GetDataType(It.IsAny())) .Returns(Mock.Of(type => type.Id == 9876 && type.Name == "text")); dataTypeService.Setup(service => service.GetDataType(-87)) //the RTE .Returns(Mock.Of(type => type.Id == -87 && type.Name == "Rich text" && type.Configuration == new RichTextConfiguration())); - - + + var langService = new Mock(); + langService.Setup(x => x.GetAllLanguages()).Returns(new[] { + Mock.Of(x => x.IsoCode == "en-US"), + Mock.Of(x => x.IsoCode == "es-ES"), + Mock.Of(x => x.IsoCode == "fr-FR") + }); + + var textService = new Mock(); + textService.Setup(x => x.Localize(It.IsAny(), It.IsAny(), It.IsAny>())).Returns(""); Container.RegisterSingleton(f => Mock.Of()); Container.RegisterSingleton(f => userServiceMock.Object); Container.RegisterSingleton(f => entityService.Object); Container.RegisterSingleton(f => dataTypeService.Object); + Container.RegisterSingleton(f => langService.Object); + Container.RegisterSingleton(f => textService.Object); Container.RegisterSingleton(f => Mock.Of()); Container.RegisterSingleton(f => new UmbracoApiControllerTypeCollection(new[] { typeof(ContentTreeController) })); } @@ -144,7 +157,7 @@ namespace Umbraco.Tests.Web.Controllers ""action"": ""save"", ""variants"": [ { - ""name"": null, + ""name"": ""asdf"", ""properties"": [ { ""id"": 1, @@ -152,10 +165,12 @@ namespace Umbraco.Tests.Web.Controllers ""value"": ""asdf"" } ], - ""culture"": ""en-US"" + ""culture"": ""en-US"", + ""save"": true, + ""publish"": true }, { - ""name"": null, + ""name"": ""asdf"", ""properties"": [ { ""id"": 1, @@ -163,7 +178,9 @@ namespace Umbraco.Tests.Web.Controllers ""value"": ""asdf"" } ], - ""culture"": ""fr-FR"" + ""culture"": ""fr-FR"", + ""save"": true, + ""publish"": true }, { ""name"": ""asdf"", @@ -279,7 +296,7 @@ namespace Umbraco.Tests.Web.Controllers Assert.AreEqual(HttpStatusCode.NotFound, response.Item1.StatusCode); } - + [Test] public async Task PostSave_Simple_Invariant() { @@ -287,7 +304,7 @@ namespace Umbraco.Tests.Web.Controllers ApiController Factory(HttpRequestMessage message, UmbracoHelper helper) { - + var contentServiceMock = Mock.Get(Current.Services.ContentService); contentServiceMock.Setup(x => x.GetById(123)).Returns(() => content); contentServiceMock.Setup(x => x.Save(It.IsAny(), It.IsAny(), It.IsAny())) @@ -313,7 +330,81 @@ namespace Umbraco.Tests.Web.Controllers Assert.AreEqual(content.PropertyTypes.Count(), display.Variants.ElementAt(0).Tabs.ElementAt(0).Properties.Count()); } + [Test] + public async Task PostSave_Validate_Empty_Name() + { + var content = GetMockedContent(); + + ApiController Factory(HttpRequestMessage message, UmbracoHelper helper) + { + + var contentServiceMock = Mock.Get(Current.Services.ContentService); + contentServiceMock.Setup(x => x.GetById(123)).Returns(() => content); + contentServiceMock.Setup(x => x.Save(It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(new OperationResult(OperationResultType.Success, new Core.Events.EventMessages())); //success + + var publishedSnapshot = Mock.Of(); + var propertyEditorCollection = new PropertyEditorCollection(new DataEditorCollection(Enumerable.Empty())); + var usersController = new ContentController(publishedSnapshot, propertyEditorCollection); + Container.InjectProperties(usersController); + return usersController; + } + + //clear out the name + var json = JsonConvert.DeserializeObject(PublishJsonInvariant); + json["variants"].ElementAt(0)["name"] = null; + + var runner = new TestRunner(Factory); + var response = await runner.Execute("Content", "PostSave", HttpMethod.Post, + content: GetMultiPartRequestContent(JsonConvert.SerializeObject(json)), + mediaTypeHeader: new MediaTypeWithQualityHeaderValue("multipart/form-data"), + assertOkResponse: false); + + Assert.AreEqual(HttpStatusCode.BadRequest, response.Item1.StatusCode); + var display = JsonConvert.DeserializeObject(response.Item2); + Assert.AreEqual(1, display.Errors.Count()); + Assert.IsTrue(display.Errors.ContainsKey("Variants[0].Name")); + //ModelState":{"Variants[0].Name":["Required"]} + } + + [Test] + public async Task PostSave_Validate_Variants_Empty_Name() + { + var content = GetMockedContent(); + + ApiController Factory(HttpRequestMessage message, UmbracoHelper helper) + { + + var contentServiceMock = Mock.Get(Current.Services.ContentService); + contentServiceMock.Setup(x => x.GetById(123)).Returns(() => content); + contentServiceMock.Setup(x => x.Save(It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(new OperationResult(OperationResultType.Success, new Core.Events.EventMessages())); //success + + var publishedSnapshot = Mock.Of(); + var propertyEditorCollection = new PropertyEditorCollection(new DataEditorCollection(Enumerable.Empty())); + var usersController = new ContentController(publishedSnapshot, propertyEditorCollection); + Container.InjectProperties(usersController); + return usersController; + } + + //clear out one of the names + var json = JsonConvert.DeserializeObject(PublishJsonVariant); + json["variants"].ElementAt(0)["name"] = null; + + var runner = new TestRunner(Factory); + var response = await runner.Execute("Content", "PostSave", HttpMethod.Post, + content: GetMultiPartRequestContent(JsonConvert.SerializeObject(json)), + mediaTypeHeader: new MediaTypeWithQualityHeaderValue("multipart/form-data"), + assertOkResponse: false); + + Assert.AreEqual(HttpStatusCode.BadRequest, response.Item1.StatusCode); + var display = JsonConvert.DeserializeObject(response.Item2); + Assert.AreEqual(2, display.Errors.Count()); + Assert.IsTrue(display.Errors.ContainsKey("Variants[0].Name")); + Assert.IsTrue(display.Errors.ContainsKey("_content_variant_en-US_")); + } + //TODO: There are SOOOOO many more tests we should write - a lot of them to do with validation - + } } diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/content/umbvariantcontent.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/content/umbvariantcontent.directive.js index 590d37d009..5b00b4a254 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/content/umbvariantcontent.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/content/umbvariantcontent.directive.js @@ -10,6 +10,7 @@ content: "<", page: "<", editor: "<", + editorIndex: "<", editorCount: "<", onCloseSplitView: "&", onSelectVariant: "&", diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/editor/umbeditorheader.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/editor/umbeditorheader.directive.js index 1d1c9d8f00..189bc8f997 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/editor/umbeditorheader.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/editor/umbeditorheader.directive.js @@ -209,6 +209,13 @@ Use this directive to construct a header inside the main editor window. function link(scope, el, attr, ctrl) { + if (!scope.serverValidationNameField) { + scope.serverValidationNameField = "Name"; + } + if (!scope.serverValidationAliasField) { + scope.serverValidationAliasField = "Alias"; + } + scope.vm = {}; scope.vm.dropdownOpen = false; scope.vm.currentVariant = ""; @@ -324,7 +331,9 @@ Use this directive to construct a header inside the main editor window. splitViewOpen: "=?", onOpenInSplitView: "&?", onCloseSplitView: "&?", - onSelectVariant: "&?" + onSelectVariant: "&?", + serverValidationNameField: "@?", + serverValidationAliasField: "@?" }, link: link }; diff --git a/src/Umbraco.Web.UI.Client/src/views/components/content/umb-variant-content-editors.html b/src/Umbraco.Web.UI.Client/src/views/components/content/umb-variant-content-editors.html index c4212adc58..c479cf6502 100644 --- a/src/Umbraco.Web.UI.Client/src/views/components/content/umb-variant-content-editors.html +++ b/src/Umbraco.Web.UI.Client/src/views/components/content/umb-variant-content-editors.html @@ -7,6 +7,7 @@ page="vm.page" content="vm.content" editor="editor" + editor-index="$index" editor-count="vm.editors.length" on-open-split-view="vm.openSplitView(variant)" on-close-split-view="vm.closeSplitView($index)" diff --git a/src/Umbraco.Web.UI.Client/src/views/components/content/umb-variant-content.html b/src/Umbraco.Web.UI.Client/src/views/components/content/umb-variant-content.html index 1f6a178c58..f268e28abf 100644 --- a/src/Umbraco.Web.UI.Client/src/views/components/content/umb-variant-content.html +++ b/src/Umbraco.Web.UI.Client/src/views/components/content/umb-variant-content.html @@ -19,7 +19,8 @@ split-view-open="vm.editorCount > 1" on-open-in-split-view="vm.openSplitView(variant)" on-close-split-view="vm.onCloseSplitView()" - on-select-variant="vm.selectVariant(variant)"> + on-select-variant="vm.selectVariant(variant)" + server-validation-name-field="{{'Variants[' + vm.editorIndex + '].Name'}}"> diff --git a/src/Umbraco.Web.UI.Client/src/views/components/editor/umb-editor-header.html b/src/Umbraco.Web.UI.Client/src/views/components/editor/umb-editor-header.html index d4a540892e..74f2c4be03 100644 --- a/src/Umbraco.Web.UI.Client/src/views/components/editor/umb-editor-header.html +++ b/src/Umbraco.Web.UI.Client/src/views/components/editor/umb-editor-header.html @@ -34,7 +34,7 @@ ng-model="name" ng-class="{'name-is-empty': $parent.name===null || $parent.name===''}" umb-auto-focus - val-server-field="Name" + val-server-field="{{serverValidationNameField}}" required /> @@ -44,7 +44,7 @@ alias="$parent.alias" alias-from="$parent.name" enable-lock="true" - server-validation-field="Alias"> + server-validation-field="{{serverValidationAliasField}}"> diff --git a/src/Umbraco.Web/Editors/ContentController.cs b/src/Umbraco.Web/Editors/ContentController.cs index 45a93724d2..09126b80ee 100644 --- a/src/Umbraco.Web/Editors/ContentController.cs +++ b/src/Umbraco.Web/Editors/ContentController.cs @@ -588,14 +588,20 @@ namespace Umbraco.Web.Editors // * Permissions are valid MapValuesForPersistence(contentItem); - //this a custom check for any variants not being flagged for Saving since we'll need to manually - //remove the ModelState validation for the Name + //This a custom check for any variants not being flagged for Saving since we'll need to manually + //remove the ModelState validation for the Name. + //We are also tracking which cultures have an invalid Name var variantCount = 0; + var variantNameErrors = new List(); foreach (var variant in contentItem.Variants) { - if (!variant.Save) + var msKey = $"Variants[{variantCount}].Name"; + if (ModelState.ContainsKey(msKey)) { - ModelState.Remove($"Variants[{variantCount}].Name"); + if (!variant.Save) + ModelState.Remove(msKey); + else + variantNameErrors.Add(variant.Culture); } variantCount++; } @@ -608,6 +614,16 @@ namespace Umbraco.Web.Editors // a message indicating this if (ModelState.IsValid == false) { + //another special case, if there's more than 1 variant, then we need to add the culture specific error + //messages based on the variants in error so that the messages show in the publish/save dialog + if (variantCount > 1) + { + foreach (var c in variantNameErrors) + { + AddCultureValidationError(c, "speechBubbles/contentCultureValidationError"); + } + } + if (IsCreatingAction(contentItem.Action)) { if (!RequiredForPersistenceAttribute.HasRequiredValuesForPersistence(contentItem) diff --git a/src/Umbraco.Web/Models/Mapping/OwnerResolver.cs b/src/Umbraco.Web/Models/Mapping/OwnerResolver.cs index 68e1a1ff91..3894fa373a 100644 --- a/src/Umbraco.Web/Models/Mapping/OwnerResolver.cs +++ b/src/Umbraco.Web/Models/Mapping/OwnerResolver.cs @@ -22,7 +22,8 @@ namespace Umbraco.Web.Models.Mapping public UserProfile Resolve(TPersisted source) { - return Mapper.Map(source.GetCreatorProfile(_userService)); + var profile = source.GetCreatorProfile(_userService); + return profile == null ? null : Mapper.Map(profile); } } }