From cec7f6da4094d99b10de38b24bfad1eb5aaea29f Mon Sep 17 00:00:00 2001 From: delixfe Date: Wed, 29 Aug 2012 17:34:37 +0200 Subject: [PATCH 01/12] added unit test 'Nested grouping finds items in observableArrays - observable' --- Tests/validation-tests.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Tests/validation-tests.js b/Tests/validation-tests.js index d1d1e04f..b3bac4b3 100644 --- a/Tests/validation-tests.js +++ b/Tests/validation-tests.js @@ -896,6 +896,15 @@ test('Nested Grouping works - Not Observable', function () { equals(errors().length, 3, 'Grouping correctly finds 3 invalid properties'); }); +test('Nested grouping finds items in observableArrays - observable', function () { + var vm = { array: ko.observableArray( [ { one: ko.observable().extend( { required: true } ) } ]) }; + + var errors = ko.validation.group(vm, { deep: true, observable: true }); + + equals(errors().length, 1, 'Grouping finds property on object in observableArray'); + +}); + test('Issue #31 - Recursively Show All Messages', function () { var vm = { one: ko.observable().extend({ required: true }), From afb5c1c831e55be2a52e57cf0b6a2dafa24994c3 Mon Sep 17 00:00:00 2001 From: delixfe Date: Thu, 30 Aug 2012 20:12:06 +0200 Subject: [PATCH 02/12] extended group with a new option live which enables group to retraverse the viewmodel if a change in an observableArray is detected --- Src/knockout.validation.js | 15 ++++++++++++++- Tests/validation-tests.js | 19 +++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/Src/knockout.validation.js b/Src/knockout.validation.js index 64001c7e..e3fef9ac 100644 --- a/Src/knockout.validation.js +++ b/Src/knockout.validation.js @@ -35,7 +35,8 @@ errorMessageClass: 'validationMessage', // class to decorate error message grouping: { deep: false, //by default grouping is shallow - observable: true //and using observables + observable: true, //and using observables + live: false //react to changes to observableArrays if observable === true } }; @@ -132,6 +133,10 @@ if (val === "") { return true; } + }, + //created issue to solve that in ko https://github.com/SteveSanderson/knockout/issues/619 + isObservableArray: function (obj) { + return ko.isObservable(obj) && !(obj.destroyAll === undefined); } }; } ()); @@ -229,6 +234,14 @@ traverse(obj); + if (options.live) { + ko.utils.arrayForEach(validatables(), function (observable) { + if (utils.isObservableArray(observable)) { + observable.subscribe(function () { traverse(obj); }) + } + }); + } + result = ko.computed(function () { var errors = []; ko.utils.arrayForEach(validatables(), function (observable) { diff --git a/Tests/validation-tests.js b/Tests/validation-tests.js index b3bac4b3..0649cafc 100644 --- a/Tests/validation-tests.js +++ b/Tests/validation-tests.js @@ -902,7 +902,26 @@ test('Nested grouping finds items in observableArrays - observable', function () var errors = ko.validation.group(vm, { deep: true, observable: true }); equals(errors().length, 1, 'Grouping finds property on object in observableArray'); +}); + +test('Nested grouping does not add newly items newly inserted into observableArrays to result - observable, not live', function () { + var vm = { array: ko.observableArray() }; + + var errors = ko.validation.group(vm, { deep: true, observable: true, live: false }); + + vm.array.push( { one: ko.observable().extend( { required: true } ) }); + + equals(errors().length, 0, 'grouping does not add newly items newly inserted into observableArrays to result'); +}); + +test('Nested grouping adds items newly inserted into observableArrays to result - observable, live', function () { + var vm = { array: ko.observableArray() }; + + var errors = ko.validation.group(vm, { deep: true, observable: true, live: true }); + + vm.array.push( { one: ko.observable().extend( { required: true } ) }); + equals(errors().length, 1, 'grouping adds newly items newly inserted into observableArrays to result'); }); test('Issue #31 - Recursively Show All Messages', function () { From 7e7157ce85de3213ad80cc8900259b50b592ddb0 Mon Sep 17 00:00:00 2001 From: delixfe Date: Sat, 1 Sep 2012 17:36:33 +0200 Subject: [PATCH 03/12] grouping ignores items nested in destroyed objects (currently only working for non observable groupings) --- Src/knockout.validation.js | 3 ++- Tests/validation-tests.js | 25 +++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/Src/knockout.validation.js b/Src/knockout.validation.js index cc56e3f1..32011bf5 100644 --- a/Src/knockout.validation.js +++ b/Src/knockout.validation.js @@ -214,7 +214,8 @@ } //get list of values either from array or object but ignore non-objects - if (val) { + // and destroyed objects + if (val && !val._destroy) { if (utils.isArray(val)) { objValues = val; } else if (utils.isObject(val)) { diff --git a/Tests/validation-tests.js b/Tests/validation-tests.js index 0649cafc..9544ff58 100644 --- a/Tests/validation-tests.js +++ b/Tests/validation-tests.js @@ -924,6 +924,31 @@ test('Nested grouping adds items newly inserted into observableArrays to result equals(errors().length, 1, 'grouping adds newly items newly inserted into observableArrays to result'); }); +test('Nested grouping ignores items nested in destroyed objects - not observable', function () { + var obj = { nested: ko.observable().extend({ required: true }) }; + + function getErrorCount() { + return errorsFn = ko.validation.group(obj, { deep: true, observable: false, live: false })().length; + } + + equal(getErrorCount(), 1, 'obj is not destroyed and should return nested\'s error'); + + obj._destroy = true; + + equal(getErrorCount(), 0, 'obj is destroyed and nested therefore ignored'); +}); + +test('Nested grouping ignores items nested in destroyed objects - observable, live', function () { + var obj = { nested: ko.observable().extend({ required: true }) }; + var array = ko.observableArray([obj]); + + var errors = ko.validation.group(array, { deep: true, observable: true, live: true }); + + equal(errors().length, 1, 'obj is not yet destroyed and nested therefore invalid'); + array.destroy(obj); + equal(errors().length, 0, 'obj is destroyed and nested therefore ignored'); +}); + test('Issue #31 - Recursively Show All Messages', function () { var vm = { one: ko.observable().extend({ required: true }), From ddab23b2fa9218ae578a8779fcd4c92d0f58886a Mon Sep 17 00:00:00 2001 From: delixfe Date: Sat, 1 Sep 2012 17:43:44 +0200 Subject: [PATCH 04/12] validateables were not cleared before retraversing the viewmodel --- Src/knockout.validation.js | 5 ++++- Tests/validation-tests.js | 11 +++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/Src/knockout.validation.js b/Src/knockout.validation.js index cc56e3f1..7c4db098 100644 --- a/Src/knockout.validation.js +++ b/Src/knockout.validation.js @@ -240,7 +240,10 @@ if (options.live) { ko.utils.arrayForEach(validatables(), function (observable) { if (utils.isObservableArray(observable)) { - observable.subscribe(function () { traverse(obj); }) + observable.subscribe(function () { + validatables([]); //clear validatables + traverse(obj); + }) } }); } diff --git a/Tests/validation-tests.js b/Tests/validation-tests.js index 0649cafc..25b03b2d 100644 --- a/Tests/validation-tests.js +++ b/Tests/validation-tests.js @@ -924,6 +924,17 @@ test('Nested grouping adds items newly inserted into observableArrays to result equals(errors().length, 1, 'grouping adds newly items newly inserted into observableArrays to result'); }); +test('Nested grouping adds items newly inserted into observableArrays to result - cleares validatables before traversing again - observable, live', function () { + var vm = { array: ko.observableArray() }; + + var errors = ko.validation.group(vm, { deep: true, observable: true, live: true }); + + vm.array.push({ one: ko.observable().extend({ required: true }) }); + vm.array.push({ one: ko.observable().extend({ required: true }) }); + + equals(errors().length, 2, 'validatables are added only once'); +}); + test('Issue #31 - Recursively Show All Messages', function () { var vm = { one: ko.observable().extend({ required: true }), From ae7b134c10d46f70061059cf624eeb96dbbcf024 Mon Sep 17 00:00:00 2001 From: delixfe Date: Thu, 22 Nov 2012 18:44:56 +0100 Subject: [PATCH 05/12] Added missing semicolon. --- Src/knockout.validation.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Src/knockout.validation.js b/Src/knockout.validation.js index e5b1f714..c9f92278 100644 --- a/Src/knockout.validation.js +++ b/Src/knockout.validation.js @@ -244,7 +244,7 @@ observable.subscribe(function () { validatables([]); //clear validatables traverse(obj); - }) + }); } }); } From ce20bd4166660ef4f2ba21cd2f2037271011678c Mon Sep 17 00:00:00 2001 From: delixfe Date: Fri, 23 Nov 2012 18:03:08 +0100 Subject: [PATCH 06/12] The traverse function of grouping stores the validatables in a temporary not observable array to avoid to cause the reevaluation of computeds depending on the errors for every added validatable. --- Src/knockout.validation.js | 14 +++++++++----- Tests/validation-tests.js | 21 +++++++++++++++++++-- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/Src/knockout.validation.js b/Src/knockout.validation.js index c9f92278..a811596a 100644 --- a/Src/knockout.validation.js +++ b/Src/knockout.validation.js @@ -194,6 +194,7 @@ group: function group(obj, options) { // array of observables or viewModel var options = ko.utils.extend(configuration.grouping, options), validatables = ko.observableArray([]), + validatablesTemp = [], result = null, //anonymous, immediate function to traverse objects hierarchically @@ -210,7 +211,7 @@ //make sure it is validatable object if (!obj.isValid) obj.extend({ validatable: true }); - validatables.push(obj); + validatablesTemp.push(obj); } //get list of values either from array or object but ignore non-objects @@ -237,13 +238,16 @@ if (options.observable) { traverse(obj); + validatables(validatablesTemp); if (options.live) { ko.utils.arrayForEach(validatables(), function (observable) { if (utils.isObservableArray(observable)) { observable.subscribe(function () { - validatables([]); //clear validatables + validatablesTemp = []; + //validatables([]); //clear validatables traverse(obj); + validatables(validatablesTemp); }); } }); @@ -262,9 +266,9 @@ } else { //if not using observables then every call to error() should traverse the structure result = function () { var errors = []; - validatables([]); //clear validatables + validatablesTemp = []; //clear validatables traverse(obj); // and traverse tree again - ko.utils.arrayForEach(validatables(), function (observable) { + ko.utils.arrayForEach(validatablesTemp, function (observable) { if (!observable.isValid()) { errors.push(observable.error); } @@ -282,7 +286,7 @@ // ensure we have latest changes result(); - ko.utils.arrayForEach(validatables(), function (observable) { + ko.utils.arrayForEach(validatablesTemp, function (observable) { observable.isModified(show); }); }; diff --git a/Tests/validation-tests.js b/Tests/validation-tests.js index d0088955..99efbc7c 100644 --- a/Tests/validation-tests.js +++ b/Tests/validation-tests.js @@ -904,7 +904,7 @@ test('Nested grouping finds items in observableArrays - observable', function () equals(errors().length, 1, 'Grouping finds property on object in observableArray'); }); -test('Nested grouping does not add newly items newly inserted into observableArrays to result - observable, not live', function () { +test('Nested grouping does not add items newly inserted into observableArrays to result - observable, not live', function () { var vm = { array: ko.observableArray() }; var errors = ko.validation.group(vm, { deep: true, observable: true, live: false }); @@ -941,14 +941,31 @@ test('Nested grouping ignores items nested in destroyed objects - not observable test('Nested grouping ignores items nested in destroyed objects - observable, live', function () { var obj = { nested: ko.observable().extend({ required: true }) }; var array = ko.observableArray([obj]); + var vm = { array: array}; - var errors = ko.validation.group(array, { deep: true, observable: true, live: true }); + var errors = ko.validation.group(vm, { deep: true, observable: true, live: true }); equal(errors().length, 1, 'obj is not yet destroyed and nested therefore invalid'); array.destroy(obj); equal(errors().length, 0, 'obj is destroyed and nested therefore ignored'); }); +test('Nested grouping does not cause the reevaluation of computeds depending on the result for every observable', function () { + var vm = { array: ko.observableArray() }; + var item = { one: ko.observable().extend( { required: true } ) }; + + var errors = ko.validation.group(vm, { deep: true, observable: true, live: true }); + + var computedHitCount = 0; + var computed = ko.computed(function () { + computedHitCount++; + errors(); + }); + + vm.array.push(item); + equals(computedHitCount, 2, ' first on while creating the computed, second one for adding the item'); +}); + test('Nested grouping adds items newly inserted into observableArrays to result - cleares validatables before traversing again - observable, live', function () { var vm = { array: ko.observableArray() }; From 6f79d9fb3a3ea203ade8a9e27b09440006f21f10 Mon Sep 17 00:00:00 2001 From: delixfe Date: Fri, 23 Nov 2012 18:05:08 +0100 Subject: [PATCH 07/12] Removed a comment. --- Src/knockout.validation.js | 1 - 1 file changed, 1 deletion(-) diff --git a/Src/knockout.validation.js b/Src/knockout.validation.js index a811596a..0cdf9d76 100644 --- a/Src/knockout.validation.js +++ b/Src/knockout.validation.js @@ -245,7 +245,6 @@ if (utils.isObservableArray(observable)) { observable.subscribe(function () { validatablesTemp = []; - //validatables([]); //clear validatables traverse(obj); validatables(validatablesTemp); }); From 4888a1c2ababadd5675d1903c94b3d2d406bc94d Mon Sep 17 00:00:00 2001 From: delixfe Date: Sun, 23 Dec 2012 17:25:46 +0100 Subject: [PATCH 08/12] Use validatablesTemp instead of the call to validatables() to subscribe to all observableArrays. --- Src/knockout.validation.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Src/knockout.validation.js b/Src/knockout.validation.js index 4ef8ce3b..109284f6 100644 --- a/Src/knockout.validation.js +++ b/Src/knockout.validation.js @@ -253,7 +253,7 @@ validatables(validatablesTemp); if (options.live) { - ko.utils.arrayForEach(validatables(), function (observable) { + ko.utils.arrayForEach(validatablesTemp, function (observable) { if (utils.isObservableArray(observable)) { observable.subscribe(function () { validatablesTemp = []; From 44bf5c256f33b395ec5dfac5ccaaa37c3bcacb3f Mon Sep 17 00:00:00 2001 From: delixfe Date: Sun, 23 Dec 2012 17:49:08 +0100 Subject: [PATCH 09/12] Live tracking of observableArrays did work only on the arrays existing at the creation of the validation group. --- Src/knockout.validation.js | 38 +++++++++++++++++++++++--------------- Tests/validation-tests.js | 11 +++++++++++ 2 files changed, 34 insertions(+), 15 deletions(-) diff --git a/Src/knockout.validation.js b/Src/knockout.validation.js index 109284f6..185da545 100644 --- a/Src/knockout.validation.js +++ b/Src/knockout.validation.js @@ -224,6 +224,10 @@ //make sure it is validatable object if (!obj.isValid) obj.extend({ validatable: true }); validatablesTemp.push(obj); + + if(options.live && utils.isObservableArray(obj)) { + subscribeToObservableArray(obj); + } } //get list of values either from array or object but ignore non-objects @@ -244,26 +248,30 @@ if (observable && !observable.nodeType) traverse(observable, level + 1); }); } + }, + + observableArraySubscriptions = [], + clearObservableArraySubscriptions = function () { + ko.utils.arrayForEach(observableArraySubscriptions, function (subscription) { + subscription.dispose(); + }); + observableArraySubscriptions = []; + }, + traverseAndStoreInValidatables = function() { + clearObservableArraySubscriptions(); + validatablesTemp = []; + traverse(obj); + validatables(validatablesTemp); + }, + subscribeToObservableArray = function(observableArray) { + observableArraySubscriptions.push(observableArray.subscribe(traverseAndStoreInValidatables)); }; //if using observables then traverse structure once and add observables if (options.observable) { - traverse(obj); - validatables(validatablesTemp); - - if (options.live) { - ko.utils.arrayForEach(validatablesTemp, function (observable) { - if (utils.isObservableArray(observable)) { - observable.subscribe(function () { - validatablesTemp = []; - traverse(obj); - validatables(validatablesTemp); - }); - } - }); - } - + traverseAndStoreInValidatables(); + result = ko.computed(function () { var errors = []; ko.utils.arrayForEach(validatables(), function (observable) { diff --git a/Tests/validation-tests.js b/Tests/validation-tests.js index 9306dc4b..b0fc9ef1 100644 --- a/Tests/validation-tests.js +++ b/Tests/validation-tests.js @@ -964,6 +964,17 @@ test('Nested grouping does not add items newly inserted into observableArrays to equals(errors().length, 0, 'grouping does not add newly items newly inserted into observableArrays to result'); }); +test('Nested grouping adds items newly inserted into an observableArrays nested in an object in an observableArray to result - observable, live', function () { + var vm = { array: ko.observableArray() }; + + var errors = ko.validation.group(vm, { deep: true, observable: true, live: true }); + + vm.array.push({ array: ko.observableArray() }); + vm.array()[0].array.push( { one: ko.observable().extend( { required: true } ) }); + + equals(errors().length, 1, 'grouping adds newly items newly inserted into observableArrays to result'); +}); + test('Nested grouping adds items newly inserted into observableArrays to result - observable, live', function () { var vm = { array: ko.observableArray() }; From 372b09cc626e2a5da535943e01171b64f874d655 Mon Sep 17 00:00:00 2001 From: delixfe Date: Sun, 23 Dec 2012 18:10:26 +0100 Subject: [PATCH 10/12] Added a todo for dispose handling. --- Src/knockout.validation.js | 1 + 1 file changed, 1 insertion(+) diff --git a/Src/knockout.validation.js b/Src/knockout.validation.js index 185da545..2a8a97ed 100644 --- a/Src/knockout.validation.js +++ b/Src/knockout.validation.js @@ -272,6 +272,7 @@ traverseAndStoreInValidatables(); + // TODO: call clearObservableArraySubscriptions on dispose of result -> but ko.computed has no disposeCallback result = ko.computed(function () { var errors = []; ko.utils.arrayForEach(validatables(), function (observable) { From 3d84253446d3f741390d33ed9605df787a182a18 Mon Sep 17 00:00:00 2001 From: Mauro Reverberi Date: Fri, 28 Jun 2013 10:48:18 +0200 Subject: [PATCH 11/12] Retraverse function added to observable group result --- Src/knockout.validation.js | 2 ++ Tests/validation-tests.js | 15 +++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/Src/knockout.validation.js b/Src/knockout.validation.js index 2a8a97ed..d802680d 100644 --- a/Src/knockout.validation.js +++ b/Src/knockout.validation.js @@ -283,6 +283,8 @@ return errors; }); + result.retraverse = traverseAndStoreInValidatables; + } else { //if not using observables then every call to error() should traverse the structure result = function () { var errors = []; diff --git a/Tests/validation-tests.js b/Tests/validation-tests.js index b0fc9ef1..0ebabbbb 100644 --- a/Tests/validation-tests.js +++ b/Tests/validation-tests.js @@ -1038,6 +1038,21 @@ test('Nested grouping adds items newly inserted into observableArrays to result equals(errors().length, 2, 'validatables are added only once'); }); +test('Retraverse all objects', function () { + var vm = { prop: ko.observable()}; + var errors = ko.validation.group(vm, { deep: true, observable: true, live: true }); + + var complexObject = { name: ko.observable().extend({ required: true }); }; + vm.prop(complexObject); + + errors.showAllMessages(); + equals(errors().length, 0, 'no validation error'); // complex object is not added to validation + + errors.retraverse(); + + equals(errors().length, 1, 'validatables are added only once'); +}); + test('Issue #31 - Recursively Show All Messages', function () { var vm = { one: ko.observable().extend({ required: true }), From 4e9ba1edb0faeb8f497abd3fe28328e1d1674d29 Mon Sep 17 00:00:00 2001 From: Mauro Reverberi Date: Fri, 28 Jun 2013 10:54:38 +0200 Subject: [PATCH 12/12] Retraverse Validation Test bugfix --- Tests/validation-tests.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/validation-tests.js b/Tests/validation-tests.js index 0ebabbbb..1e5c29cd 100644 --- a/Tests/validation-tests.js +++ b/Tests/validation-tests.js @@ -1042,7 +1042,7 @@ test('Retraverse all objects', function () { var vm = { prop: ko.observable()}; var errors = ko.validation.group(vm, { deep: true, observable: true, live: true }); - var complexObject = { name: ko.observable().extend({ required: true }); }; + var complexObject = { name: ko.observable().extend({ required: true }) }; vm.prop(complexObject); errors.showAllMessages();