From b7e25eed21e29ea449918eb8b7d7efac676aa316 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 16 Jul 2020 17:31:46 +1000 Subject: [PATCH] Yay! gets it all working for the block editor, refactored counting form errors recursively to actually work and be readable, updates valPropertyMsg with all of the odd edge cases. --- .../components/content/edit.controller.js | 23 ++++-- .../validation/valformmanager.directive.js | 52 ++++--------- .../validation/valpropertymsg.directive.js | 58 ++++++++------- .../common/services/angularhelper.service.js | 74 ++++++++++++++----- .../services/servervalidationmgr.service.js | 4 +- 5 files changed, 116 insertions(+), 95 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/content/edit.controller.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/content/edit.controller.js index 7f6d1cc728..68b19abf08 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/content/edit.controller.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/content/edit.controller.js @@ -516,6 +516,12 @@ } } + function handleHttpException(err) { + if (!err.status) { + $exceptionHandler(err); + } + } + /** Just shows a simple notification that there are client side validation issues to be fixed */ function showValidationNotification() { //TODO: We need to make the validation UI much better, there's a lot of inconsistencies in v8 including colors, issues with the property groups and validation errors between variants @@ -576,6 +582,7 @@ overlayService.close(); }, function (err) { $scope.page.buttonGroupState = 'error'; + handleHttpException(err); }); @@ -621,7 +628,7 @@ model.submitButtonState = "error"; //re-map the dialog model since we've re-bound the properties dialog.variants = $scope.content.variants; - $exceptionHandler(err); + handleHttpException(err); }); }, close: function () { @@ -644,7 +651,7 @@ $scope.page.buttonGroupState = "success"; }, function (err) { $scope.page.buttonGroupState = "error"; - $exceptionHandler(err); + handleHttpException(err); });; } }; @@ -680,7 +687,7 @@ model.submitButtonState = "error"; //re-map the dialog model since we've re-bound the properties dialog.variants = $scope.content.variants; - $exceptionHandler(err); + handleHttpException(err); }); }, close: function () { @@ -705,7 +712,7 @@ $scope.page.buttonGroupState = "success"; }, function (err) { $scope.page.buttonGroupState = "error"; - $exceptionHandler(err); + handleHttpException(err); }); } }; @@ -743,7 +750,7 @@ model.submitButtonState = "error"; //re-map the dialog model since we've re-bound the properties dialog.variants = $scope.content.variants; - $exceptionHandler(err); + handleHttpException(err); }); }, close: function (oldModel) { @@ -768,7 +775,7 @@ $scope.page.saveButtonState = "success"; }, function (err) { $scope.page.saveButtonState = "error"; - $exceptionHandler(err); + handleHttpException(err); }); } @@ -821,7 +828,7 @@ model.submitButtonState = "error"; //re-map the dialog model since we've re-bound the properties dialog.variants = Utilities.copy($scope.content.variants); - $exceptionHandler(err); + handleHttpException(err); }); }, @@ -880,7 +887,7 @@ model.submitButtonState = "error"; //re-map the dialog model since we've re-bound the properties dialog.variants = $scope.content.variants; - $exceptionHandler(err); + handleHttpException(err); }); }, diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/validation/valformmanager.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/validation/valformmanager.directive.js index 86ea94914a..d91234e583 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/validation/valformmanager.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/validation/valformmanager.directive.js @@ -112,46 +112,22 @@ function valFormManager(serverValidationManager, $rootScope, $timeout, $location }); //watch the list of validation errors to notify the application of any validation changes - scope.$watch(function () { - //the validators are in the $error collection: https://docs.angularjs.org/api/ng/type/form.FormController#$error - //since each key is the validator name (i.e. 'required') we can't just watch the number of keys, we need to watch - //the sum of the items inside of each key + scope.$watch(() => angularHelper.countAllFormErrors(formCtrl), + function (e) { + + notify(scope); + + notifySubView(); + + //find all invalid elements' .control-group's and apply the error class + var inError = element.find(".control-group .ng-invalid").closest(".control-group"); + inError.addClass("error"); + + //find all control group's that have no error and ensure the class is removed + var noInError = element.find(".control-group .ng-valid").closest(".control-group").not(inError); + noInError.removeClass("error"); - //get the lengths of each array for each key in the $error collection - var validatorLengths = _.map(formCtrl.$error, function (val, key) { - // if there are child ng-forms, include the $error collections in those as well - var innerErrorCount = _.reduce( - _.map(val, v => - _.reduce( - _.map(v.$error, e => e.length), - (m, n) => m + n - ) - ), - (memo, num) => memo + num - ); - return val.length + innerErrorCount; }); - //sum up all numbers in the resulting array - var sum = _.reduce(validatorLengths, function (memo, num) { - return memo + num; - }, 0); - //this is the value we watch to notify of any validation changes on the form - return sum; - }, function (e) { - - notify(scope); - - notifySubView(); - - //find all invalid elements' .control-group's and apply the error class - var inError = element.find(".control-group .ng-invalid").closest(".control-group"); - inError.addClass("error"); - - //find all control group's that have no error and ensure the class is removed - var noInError = element.find(".control-group .ng-valid").closest(".control-group").not(inError); - noInError.removeClass("error"); - - }); //This tracks if the user is currently saving a new item, we use this to determine // if we should display the warning dialog that they are leaving the page - if a new item 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 ec95baffb2..ccb8e2e6aa 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 @@ -84,26 +84,36 @@ function valPropertyMsg(serverValidationManager, localizationService, angularHel } // return true if there is only a single error left on the property form of either valPropertyMsg or valServer - function shouldClearError() { - var errCount = 0; + function checkAndClearError() { - for (var e in formCtrl.$error) { - if (Utilities.isArray(formCtrl.$error[e])) { - errCount++; - } + var errCount = angularHelper.countAllFormErrors(formCtrl); + + if (errCount === 0) { + resetError(); + return true; } - //we are explicitly checking for valServer errors here, since we shouldn't auto clear - // based on other errors. We'll also check if there's no other validation errors apart from valPropertyMsg, if valPropertyMsg - // is the only one, then we'll clear. - - if (errCount === 0 - || (errCount === 1 && hasExplicitError()) - || (formCtrl.$invalid && Utilities.isArray(formCtrl.$error.valServer))) { + if (errCount > 2) { + return false; + } + var hasValServer = Utilities.isArray(formCtrl.$error.valServer); + if (errCount === 1 && hasValServer) { return true; } + var hasOwnErr = hasExplicitError(); + if ((errCount === 1 && hasOwnErr) || (errCount === 2 && hasOwnErr && hasValServer)) { + + var propertyValidationPath = umbPropCtrl.getValidationPath(); + // check if we can clear it based on child server errors, if we are the only explicit one remaining we can clear ourselves + if (isLastServerError(propertyValidationPath)) { + serverValidationManager.removePropertyError(propertyValidationPath, currentCulture, "", currentSegment); + return true; + } + return false; + } + return false; } @@ -113,14 +123,13 @@ function valPropertyMsg(serverValidationManager, localizationService, angularHel } // returns true if there is only a single server validation error for this property validation key in it's validation path - function isLastServerError(propertyValidationKey) { + function isLastServerError(propertyValidationPath) { var nestedErrs = serverValidationManager.getPropertyErrorsByValidationPath( - propertyValidationKey, + propertyValidationPath, currentCulture, - "", currentSegment, - true); - if (nestedErrs.length === 1 && nestedErrs[0].propertyAlias === propertyValidationKey) { + { matchType: "prefix" }); + if (nestedErrs.length === 0 || (nestedErrs.length === 1 && nestedErrs[0].propertyAlias === propertyValidationPath)) { return true; } @@ -144,13 +153,7 @@ function valPropertyMsg(serverValidationManager, localizationService, angularHel return; } - if (shouldClearError()) { - var propertyValidationKey = umbPropCtrl.getValidationPath(); - // check if we can clear it based on child server errors, if we are the only explicit one remaining we can clear ourselves - if (isLastServerError(propertyValidationKey)) { - serverValidationManager.removePropertyError(propertyValidationKey, currentCulture, "", currentSegment); - } - + if (checkAndClearError()) { resetError(); } else if (showValidation && scope.errorMsg === "") { @@ -170,10 +173,11 @@ function valPropertyMsg(serverValidationManager, localizationService, angularHel } function resetError() { + stopWatch(); hasError = false; formCtrl.$setValidity('valPropertyMsg', true, formCtrl); scope.errorMsg = ""; - stopWatch(); + } function checkValidationStatus() { @@ -270,7 +274,7 @@ function valPropertyMsg(serverValidationManager, localizationService, angularHel hasError = !isValid; if (hasError) { //set the error message to the server message - scope.errorMsg = propertyErrors.length > 0 ? labels.propertyHasErrors : propertyErrors[0].errorMsg || labels.propertyHasErrors; + scope.errorMsg = propertyErrors.length > 1 ? labels.propertyHasErrors : propertyErrors[0].errorMsg || labels.propertyHasErrors; //flag that the current validator is invalid formCtrl.$setValidity('valPropertyMsg', false, formCtrl); startWatch(); diff --git a/src/Umbraco.Web.UI.Client/src/common/services/angularhelper.service.js b/src/Umbraco.Web.UI.Client/src/common/services/angularhelper.service.js index 12247f15b5..8c384455bb 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/angularhelper.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/angularhelper.service.js @@ -10,8 +10,61 @@ function angularHelper($q) { var requiredFormProps = ["$error", "$name", "$dirty", "$pristine", "$valid", "$submitted", "$pending"]; + function collectAllFormErrorsRecursively(formCtrl, allErrors) { + // loop over the error dictionary (see https://docs.angularjs.org/api/ng/type/form.FormController#$error) + var keys = Object.keys(formCtrl.$error); + if (keys.length === 0) { + return; + } + keys.forEach(validationKey => { + var ctrls = formCtrl.$error[validationKey]; + ctrls.forEach(ctrl => { + if (isForm(ctrl)) { + // sometimes the control in error is the same form so we cannot recurse else we'll cause an infinite loop + // and in this case it means the error is assigned directly to the form, not a control + if (ctrl === formCtrl) { + allErrors.push(ctrl); // add the error + return; + } + // recurse with the sub form + collectAllFormErrorsRecursively(ctrl, allErrors); + } + else { + // it's a normal control + allErrors.push(ctrl); // add the error + } + }); + }); + } + + function isForm(obj) { + // a method to check that the collection of object prop names contains the property name expected + function allPropertiesExist(objectPropNames) { + //ensure that every required property name exists on the current object + return _.every(requiredFormProps, function (item) { + return _.contains(objectPropNames, item); + }); + } + + //get the keys of the property names for the current object + var props = _.keys(obj); + //if the length isn't correct, try the next prop + if (props.length < requiredFormProps.length) { + return false; + } + + //ensure that every required property name exists on the current scope property + return allPropertiesExist(props); + } + return { + countAllFormErrors: function (formCtrl) { + var allErrors = []; + collectAllFormErrorsRecursively(formCtrl, allErrors); + return allErrors.length; + }, + /** * Will traverse up the $scope chain to all ancestors until the predicate matches for the current scope or until it's at the root. * @param {any} scope @@ -104,26 +157,7 @@ function angularHelper($q) { }, - isForm: function (obj) { - - // a method to check that the collection of object prop names contains the property name expected - function allPropertiesExist(objectPropNames) { - //ensure that every required property name exists on the current object - return _.every(requiredFormProps, function (item) { - return _.contains(objectPropNames, item); - }); - } - - //get the keys of the property names for the current object - var props = _.keys(obj); - //if the length isn't correct, try the next prop - if (props.length < requiredFormProps.length) { - return false; - } - - //ensure that every required property name exists on the current scope property - return allPropertiesExist(props); - }, + isForm: isForm, /** * @ngdoc function 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 a5392d16ee..53309d63f5 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 @@ -776,8 +776,8 @@ function serverValidationManager($timeout) { return undefined; }, - getPropertyErrorsByValidationPath: function (propertyAlias, culture, segment, options) { - return getPropertyErrors(propertyAlias, culture, segment, "", options); + getPropertyErrorsByValidationPath: function (propertyValidationPath, culture, segment, options) { + return getPropertyErrors(propertyValidationPath, culture, segment, "", options); }, /**