From ed217079200766837306ccb8d13abf49fa91da58 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Fri, 3 Feb 2017 16:50:40 -0500 Subject: [PATCH 1/3] DataValidator, ValidStateMonitor: Add #clearFailures argument This allows clearing only the specified failures. * src/validate/ValidStateMonitor.js (clearFailures): Add `fields' argument. Make method more concise. (_fixFailure): Handle clearing `_failures' record. * src/validate/DataValidator.js (clearFailures): Add `fields' argument. * test/validate/ValidStateMonitorTest.js: Add test. * test/validate/DataValidatorTest.js: Add test. --- src/validate/DataValidator.js | 14 ++++-- src/validate/ValidStateMonitor.js | 59 +++++++++++++++----------- test/validate/DataValidatorTest.js | 11 +++-- test/validate/ValidStateMonitorTest.js | 50 +++++++++++++++++++++- 4 files changed, 101 insertions(+), 33 deletions(-) diff --git a/src/validate/DataValidator.js b/src/validate/DataValidator.js index 4651fcb..12e663f 100644 --- a/src/validate/DataValidator.js +++ b/src/validate/DataValidator.js @@ -159,13 +159,21 @@ module.exports = Class( 'DataValidator', /** - * Clear all recorded failures + * Clear specified failures, or otherwise all recorded failures + * + * `fields` must be a key-value map with the field name as the key and + * an array of indexes as the value. Any field in `fields` that has no + * failure is ignored. + * + * See `ValidStateMonitor#clearFailures` for more information. + * + * @param {Object} fields key-value names of fields/indexes to clear * * @return {DataValidator} self */ - 'public clearFailures'() + 'public clearFailures'( failures ) { - this._field_monitor.clearFailures(); + this._field_monitor.clearFailures( failures ); return this; }, diff --git a/src/validate/ValidStateMonitor.js b/src/validate/ValidStateMonitor.js index afe8545..a25a79a 100644 --- a/src/validate/ValidStateMonitor.js +++ b/src/validate/ValidStateMonitor.js @@ -263,7 +263,6 @@ module.exports = Class( 'ValidStateMonitor' ) // looks like it has been resolved this._fixFailure( fixed, name, fail_i, result ); - delete past_fail[ fail_i ]; return true; } ); } ) ).then( fixes => fixes.some( fix => fix === true ) ); @@ -328,48 +327,58 @@ module.exports = Class( 'ValidStateMonitor' ) 'private _fixFailure'( fixed, name, index, value ) { ( fixed[ name ] = fixed[ name ] || [] )[ index ] = value; + + // caller is expected to have ensured that this exists + delete this._failures[ name ][ index ]; + return fixed; }, /** - * Clear all recorded failures + * Clear specified failures, or otherwise all recorded failures * - * For each recorded failure, a `fix` even is emitted. All failure - * records are then cleared. + * `fields` must be a key-value map with the field name as the key and + * an array of indexes as the value. Any field in `fields` that has no + * failure is ignored. + * + * For each specified failure, a `fix` event is emitted. If no failures + * are specified by `fields`, all recorded failures are marked as + * fixed. If a field in `fields` is not known, it is ignored. * * Normally the resulting fix object contains the values that triggered - * the fix. Instead, each fixed index will contain `undefined`. + * the fix. Instead, each fixed index will contain `null`. * * This process is synchronous, and only a single `fix` event is emitted * after all failures have been cleared. * + * @param {Object} fields key-value names of fields/indexes to clear + * * @return {ValidStateMonitor} self */ - 'public clearFailures'() + 'public clearFailures'( fields ) { + const failures = this._failures; + let fixed = {}; - for ( let name in this._failures ) - { - const failure = this._failures[ name ]; + const isRequestedIndex = ( fields ) + ? field => ( fields[ field.getName() ] || [] ).indexOf( + field.getIndex() + ) !== -1 + : () => true; - for ( let cause_i in failure ) - { - const cause = failure[ cause_i ]; - - for ( let cause_i in cause ) - { - let fail_i = cause.getField().getIndex(); - - this._fixFailure( fixed, name, fail_i, undefined ); - } - } - } - - // clear _before_ emitting the fixes (listeners might trigger - // additional failures, for example, or call `#hasFailures`) - this._failures = {}; + Object.keys( failures ) + .reduce( + ( all_fields, name ) => all_fields.concat( + failures[ name ].map( cause => cause.getField() ) + ), + [] + ) + .filter( isRequestedIndex ) + .forEach( field => this._fixFailure( + fixed, field.getName(), field.getIndex(), null + ) ); this.emit( 'fix', fixed ); diff --git a/test/validate/DataValidatorTest.js b/test/validate/DataValidatorTest.js index 8ff4af9..b31a93e 100644 --- a/test/validate/DataValidatorTest.js +++ b/test/validate/DataValidatorTest.js @@ -217,7 +217,7 @@ describe( 'DataValidator', () => describe( '#clearFailures', () => { - it( 'marks all failures as fixed', () => + it( 'proxies to validator', () => { const bvalidator = createMockBucketValidator(); const vmonitor = ValidStateMonitor(); @@ -229,9 +229,14 @@ describe( 'DataValidator', () => bvalidator, vmonitor, dep_factory, createStubStore() ); - mock_vmonitor.expects( 'clearFailures' ).once(); + const failures = [ 'foo', 'bar' ]; - expect( sut.clearFailures() ) + mock_vmonitor + .expects( 'clearFailures' ) + .once() + .withExactArgs( failures ); + + expect( sut.clearFailures( failures ) ) .to.equal( sut ); mock_vmonitor.verify(); diff --git a/test/validate/ValidStateMonitorTest.js b/test/validate/ValidStateMonitorTest.js index 5c44cb4..b57ff07 100644 --- a/test/validate/ValidStateMonitorTest.js +++ b/test/validate/ValidStateMonitorTest.js @@ -596,7 +596,7 @@ describe( 'ValidStateMonitor', function() describe( '#clearFailures', () => { - it( 'clears all failures', () => + it( 'clears all failures when provided no arguments', () => { return new Promise( ( accept, reject ) => { @@ -608,7 +608,7 @@ describe( 'ValidStateMonitor', function() .on( 'fix', fixed => { expect( fixed ) - .to.deep.equal( { foo: [ undefined ] } ); + .to.deep.equal( { foo: [ null ] } ); expect( sut.hasFailures() ).to.be.false; @@ -620,6 +620,52 @@ describe( 'ValidStateMonitor', function() .catch( e => reject( e ) ); } ); } ); + + + it( 'clears only provided failures when provided array argument', () => + { + return new Promise( ( accept, reject ) => + { + mkstore( {} ).then( empty => + { + const sut = Sut(); + + return sut + .on( 'fix', fixed => + { + debugger; + // `bar' not cleared + expect( fixed ) + .to.deep.equal( { + foo: [ null ], + baz: [ , null ], + } ); + + // still has `bar' + expect( sut.hasFailures() ).to.be.true; + + accept( true ); + } ) + .update( empty, { + foo: mkfail( 'foo', [ 'bar1', 'bar2' ] ), + bar: mkfail( 'bar', [ 'baz' ] ), + baz: mkfail( 'baz', [ 'quux', 'quuux' ] ), + } ) + .then( sut => sut.clearFailures( { + foo: [ 0 ], + baz: [ 1 ], + } ) ); + } ) + .catch( e => reject( e ) ); + } ); + } ); + + + it( 'does not error on non-existent failure', () => + { + expect( () => Sut().clearFailures( [ 'foo', 'baz' ] ) ) + .to.not.throw( Error ); + } ); } ); } ); From e26a7c3cac34ae498800e8835f9312d4e717e51f Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Tue, 7 Feb 2017 13:56:52 -0500 Subject: [PATCH 2/3] FieldVisibilityEventHandler: Add class This extracts code from internal lovullo repo rating-fw (originally Client#_hideFields), 5d4019f1973f9271f4b1bd24ce1f55b56ccda09e. * src/event/FieldVisibilityEventHandler.js: Add class. * test/event/FieldVisibilityEventHandlerTest.js: Add test case. --- src/event/FieldVisibilityEventHandler.js | 111 ++++++++++++++ test/event/FieldVisibilityEventHandlerTest.js | 145 ++++++++++++++++++ 2 files changed, 256 insertions(+) create mode 100644 src/event/FieldVisibilityEventHandler.js create mode 100644 test/event/FieldVisibilityEventHandlerTest.js diff --git a/src/event/FieldVisibilityEventHandler.js b/src/event/FieldVisibilityEventHandler.js new file mode 100644 index 0000000..aa5b33d --- /dev/null +++ b/src/event/FieldVisibilityEventHandler.js @@ -0,0 +1,111 @@ +/** + * Field visibility event handler + * + * Copyright (C) 2017 LoVullo Associates, Inc. + * + * This file is part of the Liza Data Collection Framework + * + * Liza is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +const Class = require( 'easejs' ).Class; +const EventHandler = require( './EventHandler' ); +const UnknownEventError = require( './UnknownEventError' ); + + +/** + * Shows/hides fields according to event id + * + * @todo use something more appropriate than Ui + * @todo should not be concerned with data validators + */ +module.exports = Class( 'FieldVisibilityEventHandler' ) + .implement( EventHandler ) + .extend( +{ + /** + * Client UI + * @type {Ui} + */ + 'private _ui': null, + + /** + * Field data validator + * @type {DataValidator} + */ + 'private _data_validator': null, + + + /** + * Initialize with Client UI + * + * @param {Ui} stepui Client UI + * @param {DataValidator} data_validator field data validator + */ + __construct( stepui, data_validator ) + { + this._ui = stepui; + this._data_validator = data_validator; + }, + + + /** + * Show/hide specified fields + * + * If a given field is not known then it will be silently ignored; the + * callback `callback` will still be invoked. + * + * This relies on a poorly designed API that should change in the future. + * + * @param {string} event_id event id + * @param {function(*,Object)} callback continuation to invoke on completion + * + * @param {elementName:string, indexes:Array.} data + * + * @return {EventHandler} self + */ + 'public handle'( event_id, callback, { elementName: field_name, indexes } ) + { + // TODO: Law of Demeter! + const group = this._ui.getCurrentStep() + .getElementGroup( field_name ); + + // we probably should care, but we don't right now + if ( !group ) + { + callback(); + return; + } + + const action = ( () => + { + switch ( event_id ) + { + case 'show': + return group.showField.bind( group ); + + case 'hide': + return group.hideField.bind( group ); + + default: + throw UnknownEventError( `Unknown visibility event: ${event_id}` ); + } + } )(); + + this._data_validator.clearFailures( [ field_name ] ); + indexes.forEach( field_i => action( field_name, field_i ) ); + + callback(); + } +} ); diff --git a/test/event/FieldVisibilityEventHandlerTest.js b/test/event/FieldVisibilityEventHandlerTest.js new file mode 100644 index 0000000..005e0e7 --- /dev/null +++ b/test/event/FieldVisibilityEventHandlerTest.js @@ -0,0 +1,145 @@ +/** + * Test case for FieldVisibilityEventHandler + * + * Copyright (C) 2017 LoVullo Associates, Inc. + * + * This file is part of the Liza Data Collection Framework + * + * Liza is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +const event = require( '../../' ).event; +const expect = require( 'chai' ).expect; +const Class = require( 'easejs' ).Class; + +const { + FieldVisibilityEventHandler: Sut, + UnknownEventError +} = event; + + +describe( 'FieldVisibilityEventHandler', () => +{ + it( 'shows/hides each element index', done => + { + const name = 'field_name'; + const shown = { [name]: [] }; + const hidden = { [name]: [] }; + + const sut = Sut( + createMockStepUi( + name, + ( field, index ) => shown[ field ].push( index ), + ( field, index ) => hidden[ field ].push( index ) + ), + createStubDataProvider() + ); + + // purposefully sparse indexes + const show_indexes = [ 2, 4, ]; + const hide_indexes = [ 0, 3, ]; + + const show_data = { + elementName: name, + indexes: show_indexes, + }; + + const hide_data = { + elementName: name, + indexes: hide_indexes, + }; + + sut.handle( 'show', () => + { + // implicitly ensures proper name is passed + expect( shown[ name ] ).to.deep.equal( show_indexes ); + + sut.handle( 'hide', () => + { + expect( hidden[ name ] ).to.deep.equal( hide_indexes ); + done(); + }, hide_data ); + }, show_data ); + } ); + + + it( 'throws error given unknown event', () => + { + expect( () => + { + Sut( createMockStepUi() ).handle( 'unknown', () => {}, {} ); + } ).to.throw( UnknownEventError ); + } ); + + + it( 'ignores unknown groups', done => + { + expect( () => + { + Sut( { + getCurrentStep: () => ( { getElementGroup: () => null } ) + } ).handle( 'hide', done, {} ) + } ).to.not.throw( Error ); + } ); + + + it( 'clears failures on hidden fields', done => + { + const name = 'foo_bar'; + + const hide_data = { + elementName: name, + indexes: [ 0 ], + }; + + Sut( + createMockStepUi( name, () => {}, () => {} ), + createStubDataProvider( failures => + { + expect( failures ) + .to.deep.equal( [ name ] ) + + // we don't care about the rest of the processing at this + // point + done(); + } ) + ).handle( 'hide', () => {}, hide_data ); + } ); +} ); + + +function createMockStepUi( expected_name, showf, hidef ) +{ + return { + getCurrentStep: () => ( { + getElementGroup( field_name ) + { + expect( field_name ).to.equal( expected_name ); + + return { + showField: showf, + hideField: hidef, + }; + } + } ), + }; +} + + +function createStubDataProvider( fail_callback ) +{ + return { + clearFailures: fail_callback || () => {}, + }; +} From 028606242a2f8ebbd558cf4752309054fdee427e Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Wed, 8 Feb 2017 10:36:06 -0500 Subject: [PATCH 3/3] Integrate field visibility event handler * src/client/Client.js (_hideField): Remove method (extracted into FieldVisibilityEventHandler). (handleEvent): Remove show/hide handling. (_handleClassMatch): Adjust to new class/API. * src/client/ClientDependencyFactory.js (createClientEventHandler): Add show/hide event handlers. --- src/client/Client.js | 111 +++++--------------------- src/client/ClientDependencyFactory.js | 16 +++- 2 files changed, 35 insertions(+), 92 deletions(-) diff --git a/src/client/Client.js b/src/client/Client.js index b6e6cd5..ea8aa78 100644 --- a/src/client/Client.js +++ b/src/client/Client.js @@ -326,10 +326,6 @@ module.exports = Class( 'Client' ) // used to communicate with the server this.dataProxy = this._createDataProxy( jQuery ); - this._eventHandler = this._factory.createClientEventHandler( - this, this.elementStyler, this.dataProxy, jQuery - ); - this.uiDialog = this._factory.createUiDialog(); this.programId = this._getProgramId(); this.program = this._createProgram(); @@ -347,6 +343,10 @@ module.exports = Class( 'Client' ) this.ui = this._createUi( this.nav ); + this._eventHandler = this._factory.createClientEventHandler( + this, this._dataValidator, this.elementStyler, this.dataProxy, jQuery + ); + this._classMatcher = this._factory.createFieldClassMatcher( this.program.whens ); @@ -699,7 +699,7 @@ module.exports = Class( 'Client' ) .getExclusiveFieldNames(); - var showq = [], hideq = []; + var visq = []; for ( var field in cmatch ) { // ignore fields that are not on the current step @@ -761,13 +761,14 @@ module.exports = Class( 'Client' ) if ( show.length ) { - showq[ field ] = show; - _self._mergeCmatchHidden( field, show, false ); + visq[ field ] = { event_id: 'show', name: field, indexes: show }; + this._mergeCmatchHidden( field, show, false ); } + if ( hide.length ) { - hideq[ field ] = hide; - _self._mergeCmatchHidden( field, hide, true ); + visq[ field ] = { event_id: 'hide', name: field, indexes: hide }; + this._mergeCmatchHidden( field, hide, true ); } } @@ -780,10 +781,19 @@ module.exports = Class( 'Client' ) // manipulations on it (TODO: this is a workaround for group // show/hide issues; we need a better solution to guarantee // order - setTimeout( function() + setTimeout( () => { - _self._hideFields( showq, 'show' ); - _self._hideFields( hideq, 'hide' ); + Object.keys( visq ).forEach( field => + { + const { event_id, name, indexes } = visq[ field ]; + + this.handleEvent( event_id, { + elementName: name, + indexes: indexes, + } ); + + this._dapiTrigger( name ); + } ); }, 25 ); }, @@ -2598,16 +2608,6 @@ module.exports = Class( 'Client' ) // perform event (XXX: replace me; see above) switch ( event_name ) { - case 'enable': - case 'disable': - case 'hide': - case 'show': - var fdata = {}; - fdata[ data.elementName ] = data.indexes; - - this._hideFields( fdata, event_name ); - break; - case 'set': var setdata = {}; setdata[ data.elementName ] = []; @@ -2644,73 +2644,6 @@ module.exports = Class( 'Client' ) }, - 'private _hideFields': function( fields, event_name ) - { - var stepui = this.ui.getCurrentStep(); - - if ( !stepui ) - { - return; - } - - for ( var field in fields ) - { - var indexes = fields[ field ], - indexes_len = indexes.length; - - for ( var i = 0; i < indexes_len; i++ ) - { - var index = indexes[ i ]; - - if ( index === undefined ) - { - continue; - } - - var group = stepui.getElementGroup( field ); - if ( group === null ) - { - window.console && console.warn && console.warn( - 'No group found for %s event: %s[%s]', - event_name, - field, - index - ); - - continue; - } - - this._dapiTrigger( field ); - - if ( event_name === 'show' ) - { - group.showField( field, index ); - } - else if ( event_name === 'hide' ) - { - group.hideField( field, index ); - } - else - { - // locate the element within the group - var $element = group.getElementByName( - field, index - ); - - if ( event_name === 'enable' ) - { - $element.attr( 'readonly', false ); - } - else if ( event_name === 'disable' ) - { - $element.attr( 'readonly', true ); - } - } - } - } - }, - - /** * Trigger DataApi event for field FIELD * diff --git a/src/client/ClientDependencyFactory.js b/src/client/ClientDependencyFactory.js index 1a8c0da..bdf711d 100644 --- a/src/client/ClientDependencyFactory.js +++ b/src/client/ClientDependencyFactory.js @@ -103,12 +103,12 @@ var Step = require( '../step/Step' ), Class = require( 'easejs' ).Class; -var event = require( '../event' ); +var liza_event = require( '../event' ); function requireh( name ) { - return event[ name ]; + return liza_event[ name ]; } @@ -347,8 +347,15 @@ module.exports = Class( 'ClientDependencyFactory', createFieldClassMatcher: FieldClassMatcher, - createClientEventHandler: function( client, styler, data_proxy, jquery ) + createClientEventHandler: function( + client, data_validator, styler, data_proxy, jquery + ) { + const field_vis_handler = requireh( 'FieldVisibilityEventHandler' )( + client.getUi(), + data_validator + ); + return DelegateEventHandler( { 'indvRate': requireh( 'IndvRateEventHandler' )( client, data_proxy @@ -358,6 +365,9 @@ module.exports = Class( 'ClientDependencyFactory', 'kickBack': requireh( 'KickbackEventHandler' )( client ), 'status': requireh( 'StatusEventHandler' )( styler ), + 'show': field_vis_handler, + 'hide': field_vis_handler, + 'action$cvv2Dialog': requireh( 'Cvv2DialogEventHandler' )( jquery ) } ); }