From 9d5d5160648adcb34f1dd24ce8eb47d38ce67ac8 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 13 Aug 2018 11:25:55 +1000 Subject: [PATCH] Fixes up invariant property validation since we were only allowing one callback per alias, but we need to allow multiple --- src/Umbraco.Core/Events/EventMessages.cs | 5 +- .../imaging/umbimagegravity.directive.js | 2 +- .../showvalidationonsubmit.directive.js | 1 + .../validation/valpropertymsg.directive.js | 44 ++++++++-------- .../validation/valserver.directive.js | 44 ++++++++-------- .../validation/valserverfield.directive.js | 43 ++++++++-------- .../services/servervalidationmgr.service.js | 50 +++++++++++++------ .../services/umbrequesthelper.service.js | 2 +- 8 files changed, 102 insertions(+), 89 deletions(-) diff --git a/src/Umbraco.Core/Events/EventMessages.cs b/src/Umbraco.Core/Events/EventMessages.cs index 2df1911249..6f2bf5b736 100644 --- a/src/Umbraco.Core/Events/EventMessages.cs +++ b/src/Umbraco.Core/Events/EventMessages.cs @@ -14,10 +14,7 @@ namespace Umbraco.Core.Events _msgs.Add(msg); } - public int Count - { - get { return _msgs.Count; } - } + public int Count => _msgs.Count; public IEnumerable GetAll() { diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/imaging/umbimagegravity.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/imaging/umbimagegravity.directive.js index 4759b59d45..4f783ac9a9 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/imaging/umbimagegravity.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/imaging/umbimagegravity.directive.js @@ -148,7 +148,7 @@ /** Sets the width/height/left/top dimentions based on the image size and the "center" value */ function setDimensions() { - if (htmlImage) { + if (htmlImage && vm.center) { vm.dimensions.width = htmlImage.width(); vm.dimensions.height = htmlImage.height(); vm.dimensions.left = vm.center.left * vm.dimensions.width - 10; diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/validation/showvalidationonsubmit.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/validation/showvalidationonsubmit.directive.js index 0b48c4e1b3..c46a3a9f9a 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/validation/showvalidationonsubmit.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/validation/showvalidationonsubmit.directive.js @@ -27,6 +27,7 @@ element.hide(); })); + //no isolate scope to listen to element destroy element.bind('$destroy', function () { for (var u in unsubscribe) { unsubscribe[u](); diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/validation/valpropertymsg.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/validation/valpropertymsg.directive.js index e7614d5ea5..5568b4b276 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/validation/valpropertymsg.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/validation/valpropertymsg.directive.js @@ -176,35 +176,31 @@ function valPropertyMsg(serverValidationManager) { // the correct field validation in their property editors. if (scope.currentProperty) { //this can be null if no property was assigned - serverValidationManager.subscribe(scope.currentProperty.alias, currentCulture, "", function (isValid, propertyErrors, allErrors) { - hasError = !isValid; - if (hasError) { - //set the error message to the server message - scope.errorMsg = propertyErrors[0].errorMsg; - //flag that the current validator is invalid - formCtrl.$setValidity('valPropertyMsg', false); - startWatch(); - } - else { - scope.errorMsg = ""; - //flag that the current validator is valid - formCtrl.$setValidity('valPropertyMsg', true); - stopWatch(); - } - }); - - //when the element is disposed we need to unsubscribe! - // NOTE: this is very important otherwise when this controller re-binds the previous subscriptsion will remain - // but they are a different callback instance than the above. - element.bind('$destroy', function () { - stopWatch(); - serverValidationManager.unsubscribe(scope.currentProperty.alias, currentCulture, ""); - }); + unsubscribe.push(serverValidationManager.subscribe(scope.currentProperty.alias, + currentCulture, + "", + function(isValid, propertyErrors, allErrors) { + hasError = !isValid; + if (hasError) { + //set the error message to the server message + scope.errorMsg = propertyErrors[0].errorMsg; + //flag that the current validator is invalid + formCtrl.$setValidity('valPropertyMsg', false); + startWatch(); + } + else { + scope.errorMsg = ""; + //flag that the current validator is valid + formCtrl.$setValidity('valPropertyMsg', true); + stopWatch(); + } + })); } //when the scope is disposed we need to unsubscribe scope.$on('$destroy', function () { + stopWatch(); for (var u in unsubscribe) { unsubscribe[u](); } diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/validation/valserver.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/validation/valserver.directive.js index 2d8734d804..eb522fe783 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/validation/valserver.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/validation/valserver.directive.js @@ -9,6 +9,7 @@ function valServer(serverValidationManager) { return { require: ['ngModel', '?^^umbProperty'], restrict: "A", + scope: {}, link: function (scope, element, attr, ctrls) { var modelCtrl = ctrls[0]; @@ -21,6 +22,7 @@ function valServer(serverValidationManager) { var currentProperty = umbPropCtrl.property; var currentCulture = currentProperty.culture; var watcher = null; + var unsubscribe = []; //default to 'value' if nothing is set var fieldName = "value"; @@ -68,27 +70,29 @@ function valServer(serverValidationManager) { } //subscribe to the server validation changes - serverValidationManager.subscribe(currentProperty.alias, currentCulture, fieldName, function (isValid, propertyErrors, allErrors) { - if (!isValid) { - modelCtrl.$setValidity('valServer', false); - //assign an error msg property to the current validator - modelCtrl.errorMsg = propertyErrors[0].errorMsg; - startWatch(); - } - else { - modelCtrl.$setValidity('valServer', true); - //reset the error message - modelCtrl.errorMsg = ""; - stopWatch(); - } - }); - - //when the element is disposed we need to unsubscribe! - // NOTE: this is very important otherwise when this controller re-binds the previous subscriptsion will remain - // but they are a different callback instance than the above. - element.bind('$destroy', function () { + unsubscribe.push(serverValidationManager.subscribe(currentProperty.alias, + currentCulture, + fieldName, + function(isValid, propertyErrors, allErrors) { + if (!isValid) { + modelCtrl.$setValidity('valServer', false); + //assign an error msg property to the current validator + modelCtrl.errorMsg = propertyErrors[0].errorMsg; + startWatch(); + } + else { + modelCtrl.$setValidity('valServer', true); + //reset the error message + modelCtrl.errorMsg = ""; + stopWatch(); + } + })); + + scope.$on('$destroy', function () { stopWatch(); - serverValidationManager.unsubscribe(currentProperty.alias, currentCulture, fieldName); + for (var u in unsubscribe) { + unsubscribe[u](); + } }); } }; diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/validation/valserverfield.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/validation/valserverfield.directive.js index d170670d15..7f5427da8b 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/validation/valserverfield.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/validation/valserverfield.directive.js @@ -10,10 +10,11 @@ function valServerField(serverValidationManager) { return { require: 'ngModel', restrict: "A", + scope: {}, link: function (scope, element, attr, ngModel) { var fieldName = null; - var eventBindings = []; + var unsubscribe = []; attr.$observe("valServerField", function (newVal) { if (newVal && fieldName === null) { @@ -24,7 +25,7 @@ function valServerField(serverValidationManager) { // resubmitted. So once a field is changed that has a server error assigned to it // we need to re-validate it for the server side validator so the user can resubmit // the form. Of course normal client-side validators will continue to execute. - eventBindings.push(scope.$watch(function() { + unsubscribe.push(scope.$watch(function() { return ngModel.$modelValue; }, function(newValue){ if (ngModel.$invalid) { @@ -33,32 +34,28 @@ function valServerField(serverValidationManager) { })); //subscribe to the server validation changes - serverValidationManager.subscribe(null, null, fieldName, function (isValid, fieldErrors, allErrors) { - if (!isValid) { - ngModel.$setValidity('valServerField', false); - //assign an error msg property to the current validator - ngModel.errorMsg = fieldErrors[0].errorMsg; - } - else { - ngModel.$setValidity('valServerField', true); - //reset the error message - ngModel.errorMsg = ""; - } - }); - - //when the element is disposed we need to unsubscribe! - // NOTE: this is very important otherwise when this controller re-binds the previous subscriptsion will remain - // but they are a different callback instance than the above. - element.bind('$destroy', function () { - serverValidationManager.unsubscribe(null, null, fieldName); - }); + unsubscribe.push(serverValidationManager.subscribe(null, + null, + fieldName, + function(isValid, fieldErrors, allErrors) { + if (!isValid) { + ngModel.$setValidity('valServerField', false); + //assign an error msg property to the current validator + ngModel.errorMsg = fieldErrors[0].errorMsg; + } + else { + ngModel.$setValidity('valServerField', true); + //reset the error message + ngModel.errorMsg = ""; + } + })); } }); scope.$on('$destroy', function(){ // unbind watchers - for(var e in eventBindings) { - eventBindings[e](); + for(var e in unsubscribe) { + unsubscribe[e](); } }); diff --git a/src/Umbraco.Web.UI.Client/src/common/services/servervalidationmgr.service.js b/src/Umbraco.Web.UI.Client/src/common/services/servervalidationmgr.service.js index 26317490a0..11e54c0fdb 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/servervalidationmgr.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/servervalidationmgr.service.js @@ -64,7 +64,7 @@ function serverValidationManager($timeout) { } } } - + return { /** @@ -116,7 +116,7 @@ function serverValidationManager($timeout) { * @name subscribe * @methodOf umbraco.services.serverValidationManager * @function - * + * @returns {} a method to unsubscribe this callback * @description * Adds a callback method that is executed whenever validation changes for the field name + property specified. * This is generally used for server side validation in order to match up a server side validation error with @@ -129,31 +129,49 @@ function serverValidationManager($timeout) { if (!callback) { return; } - + + var id = String.CreateGuid(); + if (propertyAlias === null) { - //don't add it if it already exists - var exists1 = _.find(callbacks, function (item) { - return item.propertyAlias === null && item.fieldName === fieldName; + callbacks.push({ + propertyAlias: null, + culture: null, + fieldName: fieldName, + callback: callback, + id: id }); - if (!exists1) { - callbacks.push({ propertyAlias: null, culture: null, fieldName: fieldName, callback: callback }); - } } else if (propertyAlias !== undefined) { //normalize culture to null if (!culture) { culture = null; } - //don't add it if it already exists - var exists2 = _.find(callbacks, function (item) { - return item.propertyAlias === propertyAlias && item.culture === culture && item.fieldName === fieldName; + callbacks.push({ + propertyAlias: propertyAlias, + culture: culture, fieldName: fieldName, + callback: callback, + id: id }); - if (!exists2) { - callbacks.push({ propertyAlias: propertyAlias, culture: culture, fieldName: fieldName, callback: callback }); - } } + + function unsubscribeId() { + //remove all callbacks for the content field + callbacks = _.reject(callbacks, function (item) { + return item.id === id; + }); + } + + //return a function to unsubscribe this subscription by uniqueId + return unsubscribeId; }, - + + /** + * Removes all callbacks registered for the propertyALias, culture and fieldName combination + * @param {} propertyAlias + * @param {} culture + * @param {} fieldName + * @returns {} + */ unsubscribe: function (propertyAlias, culture, fieldName) { if (propertyAlias === null) { diff --git a/src/Umbraco.Web.UI.Client/src/common/services/umbrequesthelper.service.js b/src/Umbraco.Web.UI.Client/src/common/services/umbrequesthelper.service.js index 24fb11d656..228c885529 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/umbrequesthelper.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/umbrequesthelper.service.js @@ -248,7 +248,7 @@ function umbRequestHelper($http, $q, umbDataFormatter, angularHelper, dialogServ formHelper.showNotifications(response.data); - //TODO: We need to pass the result through umbDataFormatter.formatContentGetData! + //TODO: Do we need to pass the result through umbDataFormatter.formatContentGetData? Right now things work so not sure but we should check //the data returned is the up-to-date data so the UI will refresh return $q.resolve(response.data);