From b9d0bca1b6d0e2abc2fea7f66a0e41b046733cd9 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 22 Jul 2013 17:13:38 +1000 Subject: [PATCH] Fixed up validation messages when we are not redirecting, fixed up how we re-bind the content values to make sure we only set the values if they have been changed by the server and added unit tests for that. Added more properties to our display model including a new INotificationModel to put localized notifications in. --- .../src/common/services/util.service.js | 122 +++++++++++++++--- .../views/content/contentedit.controller.js | 36 +++--- .../services/content-editing-helper.spec.js | 34 ++++- src/Umbraco.Web/Editors/ContentController.cs | 37 +++++- .../ContentEditing/ContentItemDisplay.cs | 14 +- 5 files changed, 202 insertions(+), 41 deletions(-) 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 7781dc83eb..e760873c72 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 @@ -678,11 +678,67 @@ angular.module('umbraco.services').factory('iconHelper', iconHelper); function contentEditingHelper($location, $routeParams, notificationsService, serverValidationManager) { return { - + + /** + * @ngdoc function + * @name getAllProps + * @methodOf contentEditingHelper + * @function + * + * @description + * Returns all propertes contained for the content item (since the normal model has properties contained inside of tabs) + */ + getAllProps: function(content) { + var allProps = []; + + for (var i = 0; i < content.tabs.length; i++) { + for (var p = 0; p < content.tabs[i].properties.length; p++) { + allProps.push(content.tabs[i].properties[p]); + } + } + + return allProps; + }, + + /** + * @ngdoc function + * @name reBindChangedProperties + * @methodOf contentEditingHelper + * @function + * + * @description + * re-binds all changed property values to the origContent object from the newContent object and returns an array of changed properties. + */ + reBindChangedProperties: function(origContent, newContent) { + + var changed = []; + + //get a list of properties since they are contained in tabs + var allOrigProps = this.getAllProps(origContent); + var allNewProps = this.getAllProps(newContent); + + function getNewProp(alias) { + return _.find(allNewProps, function(item) { + return item.alias === alias; + }); + } + + for (var p in allOrigProps) { + var newProp = getNewProp(allOrigProps[p].alias); + if (!_.isEqual(allOrigProps[p].value, newProp.value)) { + //they have changed so set the origContent prop's value to the new value + allOrigProps[p].value = newProp.value; + changed.push(allOrigProps[p]); + } + } + + return changed; + }, + /** * @ngdoc function * @name handleValidationErrors - * @methodOf ContentEditController + * @methodOf contentEditingHelper * @function * * @description @@ -692,13 +748,7 @@ function contentEditingHelper($location, $routeParams, notificationsService, ser */ handleValidationErrors: function (content, modelState) { //get a list of properties since they are contained in tabs - var allProps = []; - - for (var i = 0; i < content.tabs.length; i++) { - for (var p = 0; p < content.tabs[i].properties.length; p++) { - allProps.push(content.tabs[i].properties[p]); - } - } + var allProps = this.getAllProps(content); //find the content property for the current error, for use in the loop below function findContentProp(props, propAlias) { @@ -744,13 +794,13 @@ function contentEditingHelper($location, $routeParams, notificationsService, ser /** * @ngdoc function * @name handleSaveError - * @methodOf ContentEditController + * @methodOf contentEditingHelper * @function * * @description * A function to handle what happens when we have validation issues from the server side */ - handleSaveError: function (err) { + handleSaveError: function (err, scope) { //When the status is a 403 status, we have validation errors. //Otherwise the error is probably due to invalid data (i.e. someone mucking around with the ids or something). //Or, some strange server error @@ -762,9 +812,10 @@ function contentEditingHelper($location, $routeParams, notificationsService, ser if (!this.redirectToCreatedContent(err.data.id, err.data.ModelState)) { //we are not redirecting because this is not new content, it is existing content. In this case - // we need to clear the server validation items. When we are creating new content we cannot clear - // the server validation items because we redirect and they need to persist until the validation is re-bound. - serverValidationManager.clear(); + // we need to detect what properties have changed and re-bind them with the server data. Then we need + // to re-bind any server validation errors after the digest takes place. + this.reBindChangedProperties(scope.content, err.data); + serverValidationManager.executeAndClearAllSubscriptions(); } //indicates we've handled the server result @@ -783,10 +834,51 @@ function contentEditingHelper($location, $routeParams, notificationsService, ser return false; }, + /** + * @ngdoc function + * @name handleSaveError + * @methodOf handleSuccessfulSave + * @function + * + * @description + * A function to handle when saving a content item is successful. This will rebind the values of the model that have changed + * ensure the notifications are displayed and that the appropriate events are fired. This will also check if we need to redirect + * when we're creating new content. + */ + handleSuccessfulSave: function (args) { + + if (!args) { + throw "args cannot be null"; + } + if (!args.scope) { + throw "args.scope cannot be null"; + } + if (!args.scope.content) { + throw "args.scope.content cannot be null"; + } + if (!args.newContent) { + throw "args.newContent cannot be null"; + } + if (!args.notifyHeader) { + throw "args.notifyHeader cannot be null"; + } + if (!args.notifyMsg) { + throw "args.notifyMsg cannot be null"; + } + + notificationsService.success(args.notifyHeader, args.notifyMsg); + args.scope.$broadcast("saved", { scope: args.scope }); + if (!this.redirectToCreatedContent(args.scope.content.id)) { + //we are not redirecting because this is not new content, it is existing content. In this case + // we need to detect what properties have changed and re-bind them with the server data + this.reBindChangedProperties(args.scope.content, args.newContent); + } + }, + /** * @ngdoc function * @name redirectToCreatedContent - * @methodOf ContentEditController + * @methodOf contentEditingHelper * @function * * @description diff --git a/src/Umbraco.Web.UI.Client/src/views/content/contentedit.controller.js b/src/Umbraco.Web.UI.Client/src/views/content/contentedit.controller.js index b24c35e0db..bc7fc67cde 100644 --- a/src/Umbraco.Web.UI.Client/src/views/content/contentedit.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/content/contentedit.controller.js @@ -60,17 +60,14 @@ function ContentEditController($scope, $routeParams, $location, contentResource, contentResource.publishContent(cnt, $routeParams.create, $scope.files) .then(function (data) { - //TODO: only update the content that has changed! - $scope.content = data; - - notificationsService.success("Published", "Content has been saved and published"); - $scope.$broadcast("saved", { scope: $scope }); - - contentEditingHelper.redirectToCreatedContent($scope.content.id); - }, function (err) { - //TODO: only update the content that has changed! - //$scope.content = err.data; - contentEditingHelper.handleSaveError(err); + contentEditingHelper.handleSuccessfulSave({ + scope: $scope, + newContent: data, + notifyHeader: "Published", + notifyMsg: "Content has been saved and published" + }); + }, function (err) { + contentEditingHelper.handleSaveError(err, $scope); }); }; @@ -85,17 +82,14 @@ function ContentEditController($scope, $routeParams, $location, contentResource, contentResource.saveContent(cnt, $routeParams.create, $scope.files) .then(function (data) { - //TODO: only update the content that has changed! - $scope.content = data; - - notificationsService.success("Saved", "Content has been saved"); - $scope.$broadcast("saved", { scope: $scope }); - - contentEditingHelper.redirectToCreatedContent($scope.content.id); + contentEditingHelper.handleSuccessfulSave({ + scope: $scope, + newContent: data, + notifyHeader: "Saved", + notifyMsg: "Content has been saved" + }); }, function (err) { - //TODO: only update the content that has changed! - //$scope.content = err.data; - contentEditingHelper.handleSaveError(err); + contentEditingHelper.handleSaveError(err, $scope); }); }; diff --git a/src/Umbraco.Web.UI.Client/test/unit/common/services/content-editing-helper.spec.js b/src/Umbraco.Web.UI.Client/test/unit/common/services/content-editing-helper.spec.js index e52bdd3f7e..8b35033e2a 100644 --- a/src/Umbraco.Web.UI.Client/test/unit/common/services/content-editing-helper.spec.js +++ b/src/Umbraco.Web.UI.Client/test/unit/common/services/content-editing-helper.spec.js @@ -25,7 +25,7 @@ describe('contentEditingHelper tests', function () { err.data.ModelState = {}; //act - var handled = contentEditingHelper.handleSaveError(err); + var handled = contentEditingHelper.handleSaveError(err, {content: content}); //assert expect(handled).toBe(true); @@ -39,7 +39,7 @@ describe('contentEditingHelper tests', function () { }; //act - var handled = contentEditingHelper.handleSaveError(err); + var handled = contentEditingHelper.handleSaveError(err, null); //assert expect(handled).toBe(false); @@ -55,7 +55,7 @@ describe('contentEditingHelper tests', function () { }; //act - var handled = contentEditingHelper.handleSaveError(err); + var handled = contentEditingHelper.handleSaveError(err, { content: content }); //assert expect(handled).toBe(false); @@ -183,4 +183,32 @@ describe('contentEditingHelper tests', function () { }); }); + + describe('wires up property values after saving', function () { + + it('does not update un-changed values', function () { + + //arrange + var origContent = mocksUtils.getMockContent(1234); + origContent.tabs[0].properties[0].value = { complex1: "origValue1a", complex2: "origValue1b" }; + origContent.tabs[1].properties[0].value = "origValue2"; + origContent.tabs[1].properties[1].value = "origValue3"; + origContent.tabs[1].properties[2].value = "origValue4"; + + var newContent = mocksUtils.getMockContent(1234); + newContent.tabs[0].properties[0].value = { complex1: "origValue1a", complex2: "newValue1b" }; + newContent.tabs[1].properties[0].value = "origValue2"; + newContent.tabs[1].properties[1].value = "newValue3"; + newContent.tabs[1].properties[2].value = "origValue4"; + + //act + var changed = contentEditingHelper.reBindChangedProperties(origContent, newContent); + + //assert + expect(changed.length).toBe(2); + expect(changed[0].alias).toBe("list"); + expect(changed[1].alias).toBe("textarea"); + }); + + }); }); \ No newline at end of file diff --git a/src/Umbraco.Web/Editors/ContentController.cs b/src/Umbraco.Web/Editors/ContentController.cs index 2671ab66fe..6a8807e886 100644 --- a/src/Umbraco.Web/Editors/ContentController.cs +++ b/src/Umbraco.Web/Editors/ContentController.cs @@ -18,6 +18,7 @@ using Umbraco.Web.Security; using Umbraco.Web.WebApi; using Umbraco.Web.WebApi.Binders; using Umbraco.Web.WebApi.Filters; +using umbraco; namespace Umbraco.Web.Editors { @@ -113,6 +114,8 @@ namespace Umbraco.Web.Editors contentItem.PersistedContent.Name = contentItem.Name; } + //TODO: We need to support 'send to publish' + //TODO: We'll need to save the new template, publishat, etc... values here //Map the property values @@ -173,6 +176,7 @@ namespace Umbraco.Web.Editors } } + bool isPublishSuccess = false; if (contentItem.Action == ContentSaveAction.Save || contentItem.Action == ContentSaveAction.SaveNew) { //save the item @@ -181,7 +185,7 @@ namespace Umbraco.Web.Editors else { //publish the item - Services.ContentService.SaveAndPublish(contentItem.PersistedContent); + isPublishSuccess = Services.ContentService.SaveAndPublish(contentItem.PersistedContent); } @@ -194,6 +198,37 @@ namespace Umbraco.Web.Editors throw new HttpResponseException(Request.CreateResponse(HttpStatusCode.Forbidden, display)); } + //put the correct msgs in + switch (contentItem.Action) + { + case ContentSaveAction.Save: + case ContentSaveAction.SaveNew: + display.AddSuccessNotification(ui.Text("speechBubbles", "editContentSavedHeader"), ui.Text("speechBubbles", "editContentSavedText")); + break; + case ContentSaveAction.Publish: + case ContentSaveAction.PublishNew: + + //If the document is at a level deeper than the root but it's ancestor's path is not published, + //it means that we cannot actually publish this document because one of it's parent's is not published. + //So, we still need to save the document but we'll show a different notification. + if (contentItem.PersistedContent.Level > 1 && !Services.ContentService.IsPublishable(contentItem.PersistedContent)) + { + display.AddWarningNotification(ui.Text("publish"), ui.Text("speechBubbles", "editContentPublishedFailedByParent")); + } + else + { + if (isPublishSuccess) + { + display.AddSuccessNotification(ui.Text("speechBubbles", "editContentPublishedHeader"), ui.Text("speechBubbles", "editContentPublishedText")); + } + else + { + display.AddWarningNotification(ui.Text("publish"), ui.Text("speechBubbles", "contentPublishedFailedByEvent")); + } + } + break; + } + return display; } diff --git a/src/Umbraco.Web/Models/ContentEditing/ContentItemDisplay.cs b/src/Umbraco.Web/Models/ContentEditing/ContentItemDisplay.cs index a5856a1763..b179072677 100644 --- a/src/Umbraco.Web/Models/ContentEditing/ContentItemDisplay.cs +++ b/src/Umbraco.Web/Models/ContentEditing/ContentItemDisplay.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.ComponentModel.DataAnnotations; +using System.Linq; using System.Runtime.Serialization; using System.Web.Http; using System.Web.Http.ModelBinding; @@ -12,8 +13,13 @@ namespace Umbraco.Web.Models.ContentEditing /// A model representing a content item to be displayed in the back office /// [DataContract(Name = "content", Namespace = "")] - public class ContentItemDisplay : TabbedContentItem + public class ContentItemDisplay : TabbedContentItem, INotificationModel { + public ContentItemDisplay() + { + Notifications = new List(); + } + [DataMember(Name = "publishDate")] public DateTime? PublishDate { get; set; } @@ -29,5 +35,11 @@ namespace Umbraco.Web.Models.ContentEditing [DataMember(Name = "ModelState")] public IDictionary Errors { get; set; } + /// + /// This is used to add custom localized messages/strings to the response for the app to use for localized UI purposes. + /// + [DataMember(Name = "notifications")] + public List Notifications { get; private set; } + } } \ No newline at end of file