From 58533f6d721e7df9f7ff43439ef9346a0068fea6 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 8 Feb 2018 16:10:39 -0500 Subject: [PATCH 1/5] Client: Remove unused `reset' event This event hasn't seen use in ...possibly ever. * src/client/Client.js (handleEvent): Remove `reset' event. Always return `this'. --- src/client/Client.js | 24 ++++++------------------ 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/src/client/Client.js b/src/client/Client.js index a047945..9df73ec 100644 --- a/src/client/Client.js +++ b/src/client/Client.js @@ -2585,7 +2585,7 @@ module.exports = Class( 'Client' ) if ( err ) { error_callback( err ); - return; + return this; } // XXX: move me @@ -2600,7 +2600,7 @@ module.exports = Class( 'Client' ) // we had no problem handling this event; no need to continue with // the old event handling system - return; + return this; } catch ( e ) { @@ -2609,25 +2609,13 @@ module.exports = Class( 'Client' ) { // ruh roh this._handleError( e ); - return; + return this; } } - // perform event (XXX: replace me; see above) - switch ( event_name ) - { - case 'reset': - var reset = {}; - reset[ data.elementName ] = data.indexes; - - this._resetFields( reset ); - break; - - default: - this._handleError( Error( - 'Unknown client-side event: ' + event_name - ) ); - } + this._handleError( Error( + 'Unknown client-side event: ' + event_name + ) ); // call the callback, if one was provided if ( callback instanceof Function ) From cc7370711da9f06d899a27f887d6791f9c76c3c0 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 8 Feb 2018 16:18:46 -0500 Subject: [PATCH 2/5] Client: Eliminate old event handling system All of the old events have been removed! * src/client/Client.js (handleEvent): Remove remainder of old system (after last commit, all that was left was error handling). Correct docblock. --- src/client/Client.js | 122 +++++++------------------------------------ 1 file changed, 19 insertions(+), 103 deletions(-) diff --git a/src/client/Client.js b/src/client/Client.js index 9df73ec..29fcf56 100644 --- a/src/client/Client.js +++ b/src/client/Client.js @@ -2561,14 +2561,12 @@ module.exports = Class( 'Client' ) /** * Handles client-side events * - * @param String event_name name of the event - * @param Object data data to pass to event - * @param Function callback function to call when event is done (if - * not asynchronous, it'll be called immediately) + * @param {string} event_name name of the event + * @param {Object} data data to pass to event + * @param {function(Object)} callback function to call when event is done + * @param {function(Error)} error_callback function to call if event fails * - * @param Function error_callback function to call if event fails - * - * @return Client self to allow for method chaining + * @return {Client} self to allow for method chaining */ handleEvent: function( event_name, data, callback, error_callback ) { @@ -2577,108 +2575,26 @@ module.exports = Class( 'Client' ) this.emit( this.__self.$('EVENT_TRIGGER'), event_name, data ); - try - { - this._eventHandler.handle( - event_name, function( err, data ) - { - if ( err ) - { - error_callback( err ); - return this; - } - - // XXX: move me - if ( event_name === 'rate' ) - { - _self.emit( _self.__self.$('EVENT_POST_RATE'), data ); - } - - callback && callback( data ); - }, data - ); - - // we had no problem handling this event; no need to continue with - // the old event handling system - return this; - } - catch ( e ) - { - // segue into the old event handling system - if ( !( Class.isA( UnknownEventError, e ) ) ) + this._eventHandler.handle( + event_name, function( err, data ) { - // ruh roh - this._handleError( e ); - return this; - } - } - - this._handleError( Error( - 'Unknown client-side event: ' + event_name - ) ); - - // call the callback, if one was provided - if ( callback instanceof Function ) - { - callback.call( this ); - } - - return this; - }, - - - /** - * Trigger DataApi event for field FIELD - * - * @param {string} field field name - * - * @return {undefined} - */ - 'private _dapiTrigger': function( field ) - { - var _self = this; - - this.getQuote().visitData( function( bucket ) - { - _self.program.dapi( - _self._currentStepId, - field, - bucket, - {}, - _self._cmatch, - null - ); - } ); - }, - - - 'private _resetFields': function( fields ) - { - var update = {}; - - for ( var field in fields ) - { - var cur = fields[ field ], - cdata = this._quote.getDataByName( field ), - val = this.elementStyler.getDefault( field ); - - var data = []; - for ( var i in cur ) - { - var index = cur[ i ]; - - if ( cdata[ index ] === val ) + if ( err ) { - continue; + error_callback && error_callback( err ); + return this; } - data[ index ] = val; - } + // XXX: move me + if ( event_name === 'rate' ) + { + _self.emit( _self.__self.$('EVENT_POST_RATE'), data ); + } - update[ field ] = data; - } + callback && callback( data ); + }, data + ); - this._quote.setData( update ); + return this; }, From 4b114d01379f0618a210388875a3d062f36771c4 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Fri, 9 Feb 2018 10:34:34 -0500 Subject: [PATCH 3/5] Client: Extract cmatch methods This makes the minimal number of changes necessary to ensure that all object references remain available. It is a mess. And despite moving all of this, Client is still a massive clusterfuck. * src/client/Client.js (system): Add import. (_cmatch): Now references new Cmatch class instance. (_cmatchHidden, _classMatcher): Remove fields. (_forceCmatchAction): Rename to `forceCmatchAction'. (_hookClassifier, _postProcessCmatch, _cmatchVisFromUi, _handleClassMatch, _mergeCmatchHidden, _clearCmatchFields): Extract methods. Update references as necessary. (getCmatchData): Remove unused method. (_handleError): Rename to `handleError' to make accessible to Cmatch. Update references. * src/client/ClientDependencyFactory.js (FieldClassMatcher): Remove import. (createFieldClassMatcher): Remove method. See `system/client'. * src/client/Cmatch.js: New class. * src/system/client.js (Cmatch, field): Add imports. (cmatch): Add export. --- src/client/Client.js | 401 ++------------------- src/client/ClientDependencyFactory.js | 11 +- src/client/Cmatch.js | 482 ++++++++++++++++++++++++++ src/system/client.js | 10 +- 4 files changed, 517 insertions(+), 387 deletions(-) create mode 100644 src/client/Cmatch.js diff --git a/src/client/Client.js b/src/client/Client.js index 29fcf56..2ed3748 100644 --- a/src/client/Client.js +++ b/src/client/Client.js @@ -23,6 +23,7 @@ const Class = require( 'easejs' ).Class; const EventEmitter = require( 'events' ).EventEmitter; const DomFieldNotFoundError = require( '../ui/field/DomFieldNotFoundError' ); const UnknownEventError = require( './event/UnknownEventError' ); +const system = require( '../system/client' ); /** @@ -198,30 +199,10 @@ module.exports = Class( 'Client' ) 'private _validatorFormatter': null, /** - * Contains classification match data per field - * - * TODO: Move this to somewhere more appropriate - * - * @type {Object} + * Classification match handler + * @type {Cmatch} */ - 'private _cmatch': {}, - - /** - * Performs classification matching on fields - * - * A field will have a positive match for a given index if all of its - * classes match - * - * @type {FieldClassMatcher} - */ - 'private _classMatcher': null, - - - /** - * Fields that were hidden (including indexes) since the last cmatch clear - * @type {Object} - */ - 'private _cmatchHidden': {}, + 'private _cmatch': null, /** * Automatically discards staging bucket contents @@ -347,9 +328,7 @@ module.exports = Class( 'Client' ) this, this._dataValidator, this.elementStyler, this.dataProxy, jQuery ); - this._classMatcher = this._factory.createFieldClassMatcher( - this.program.whens - ); + this._cmatch = system.cmatch( this.program, this.__inst ); this._validatorFormatter = this._factory.createValidatorFormatter( this.program.meta.qtypes @@ -428,7 +407,7 @@ module.exports = Class( 'Client' ) { client.ui.displayStep( step_id, function() { - client.forceCmatchAction(); + client._cmatch.forceCmatchAction(); } ); client._currentStepId = step_id; @@ -503,7 +482,7 @@ module.exports = Class( 'Client' ) client._quote.setQuickSaveData( data.content.quicksave || {} ); - client._hookClassifier(); + client._cmatch.hookClassifier( client._dataValidator ); // store internal status client._isInternal = client.program.isInternal = @@ -556,285 +535,6 @@ module.exports = Class( 'Client' ) }, - /** - * Force handling of the most recent cmatch data - * - * This can be used to refresh the UI to ensure that it is consistent with - * the cmatch data. - * - * @return {Client} self - */ - 'public forceCmatchAction': function() - { - if ( !( this._cmatch ) ) - { - return this; - } - - this._handleClassMatch( this._cmatch, true ); - - return this; - }, - - - 'private _hookClassifier': function() - { - var _self = this, - program = this.program; - - // clear/initialize cmatches - this._cmatch = {}; - - var cmatchprot = false; - - // set classifier - this._quote - .setClassifier( program.getClassifierKnownFields(), function() - { - return program.classify.apply( program, arguments ); - } ) - .on( 'classify', function( classes ) - { - if ( cmatchprot === true ) - { - _self._handleError( Error( 'cmatch recursion' ) ); - } - - cmatchprot = true; - - // handle field fixes - _self._dataValidator.validate( undefined, classes ) - .catch( e => _self.handleError( e ) ); - - _self._classMatcher.match( classes, function( cmatch ) - { - // it's important that we do this here so that everything - // that uses the cmatch data will consistently benefit - _self._postProcessCmatch( cmatch ); - - // if we're not on a current step, defer - if ( !( _self.ui.getCurrentStep() ) ) - { - _self._cmatch = cmatch; - cmatchprot = false; - return; - } - - _self._handleClassMatch( cmatch ); - cmatchprot = false; - } ); - } ); - }, - - - 'private _postProcessCmatch': function( cmatch ) - { - // for any matches that are scalars (they will have no indexes), loop - // through each field and set the index to the value of 'all' - for ( var field in cmatch ) - { - if ( field === '__classes' ) - { - continue; - } - - var cfield = cmatch[ field ]; - - if ( cfield.indexes.length === 0 ) - { - var data = this.getQuote().getDataByName( field ), - i = data.length; - - // this will do nothing if there is no data found - while ( i-- ) - { - cfield.indexes[ i ] = cfield.all; - } - } - } - - return cmatch; - }, - - - // from UI - 'private _cmatchVisFromUi': function( field, all ) - { - var step = this.getUi().getCurrentStep(); - - if ( !step ) - { - return []; - } - - var group = step.getElementGroup( field ); - if ( !group ) - { - return []; - } - - var i = group.getCurrentIndexCount(), - ret = []; - - while ( i-- ) - { - ret.push( all ); - } - - return ret; - }, - - - 'private _handleClassMatch': function( cmatch, force ) - { - force = !!force; - - this.ui.setCmatch( cmatch ); - - var _self = this, - quote = this.getQuote(), - - // oh dear god...(Demeter, specifically..) - fields = this.getUi().getCurrentStep().getStep() - .getExclusiveFieldNames(); - - - var visq = []; - for ( var field in cmatch ) - { - // ignore fields that are not on the current step - if ( !( fields[ field ] ) ) - { - continue; - } - - // if the match is still false, then we can rest assured - // that nothing has changed (and skip the overhead) - if ( !force - && ( cmatch[ field ] === false ) - && ( _self._cmatch[ field ] === false ) - ) - { - continue; - } - - var show = [], - hide = [], - - cfield = cmatch[ field ], - - vis = cfield.indexes, - cur = ( - ( _self._cmatch[ field ] || {} ).indexes - || [] - ); - - // the system was previously unable to determine the length, so - // let's attempt to get it from the UI - if ( vis.length === 0 ) - { - vis = this._cmatchVisFromUi( field, cfield.all ); - } - - // consider the number of indexes in the bucket first; - // otherwise, we might try to operate on fields that don't - // exist (bucket/class indexes not the same). the check for - // undefined in the first index is a workaround for the explicit - // setting of the length property of the bucket value when - // indexes are removed - var curdata = quote.getDataByName( field ), - fieldn = ( curdata.length > 0 && ( curdata[ 0 ] !== undefined ) ) - ? curdata.length - : vis.length; - - for ( var i = 0; i < fieldn; i++ ) - { - // do not record unchanged indexes as changed - // (avoiding the event overhead) - if ( !force && ( vis[ i ] === cur[ i ] ) ) - { - continue; - } - - ( ( vis[ i ] ) ? show : hide ).push( i ); - } - - if ( show.length ) - { - visq[ field ] = { event_id: 'show', name: field, indexes: show }; - this._mergeCmatchHidden( field, show, false ); - } - - if ( hide.length ) - { - visq[ field ] = { event_id: 'hide', name: field, indexes: hide }; - this._mergeCmatchHidden( field, hide, true ); - } - } - - // it's important to do this before showing/hiding fields, since - // those might trigger events that check the current cmatches - this._cmatch = cmatch; - - - // allow DOM operations to complete before we trigger - // manipulations on it (TODO: this is a workaround for group - // show/hide issues; we need a better solution to guarantee - // order - setTimeout( () => - { - Object.keys( visq ).forEach( field => - { - const { event_id, name, indexes } = visq[ field ]; - - this.handleEvent( event_id, { - elementName: name, - indexes: indexes, - } ); - - this._dapiTrigger( name ); - } ); - }, 25 ); - }, - - - 'private _mergeCmatchHidden': function( name, indexes, hidden ) - { - if ( !( this._cmatchHidden[ name ] ) ) - { - this._cmatchHidden[ name ] = {}; - } - - var cindexes = this._cmatchHidden[ name ]; - - for ( i in indexes ) - { - if ( hidden ) - { - cindexes[ indexes[ i ] ] = i; - } - else - { - delete cindexes[ indexes[ i ] ]; - } - } - - var some = false; - for ( var i in cindexes ) - { - some = true; - break; - } - - if ( !some ) - { - // v8 devs do not recomment delete as it progressively slows down - // property access on the object - this._cmatchHidden[ name ] = undefined; - } - }, - - /** * Hooks quote for performing validations on data change * @@ -862,7 +562,7 @@ module.exports = Class( 'Client' ) name, bucket, diff, - this._cmatch, + this._cmatch.getMatches(), function() { var args = arguments; @@ -937,7 +637,7 @@ module.exports = Class( 'Client' ) { // N.B.: We pass {} as the diff because nothing has actually changed _self.ui.invalidateForm( - validate_callback( bucket, {}, _self._cmatch ) + validate_callback( bucket, {}, _self._cmatch.getMatches() ) ); } ); }, @@ -1009,7 +709,7 @@ module.exports = Class( 'Client' ) // force UI cmatch update, since we may have fields that have been added // that need to be shown/hidden based on the current set of // classifications - this.forceCmatchAction(); + this._cmatch.forceCmatchAction(); }, @@ -1049,7 +749,7 @@ module.exports = Class( 'Client' ) catch ( e ) { // todo: better suited for brokers - this._handleError( Error( + this.handleError( Error( "Error loading program data: " + e.message ) ); @@ -1058,7 +758,7 @@ module.exports = Class( 'Client' ) program.on( 'error', function( e ) { - _self._handleError( e ); + _self.handleError( e ); } ); // handle field updates @@ -1158,7 +858,7 @@ module.exports = Class( 'Client' ) } ) .on( 'error', function( e ) { - _self._handleError( e ); + _self.handleError( e ); } ); return program; @@ -1412,7 +1112,7 @@ module.exports = Class( 'Client' ) // handle context errors root_context.on( 'error', function( e ) { - client._handleError( e ); + client.handleError( e ); } ); // must init after the Ui obj is available @@ -1472,7 +1172,7 @@ module.exports = Class( 'Client' ) } ) .on( 'error', function( e ) { - client._handleError( e ); + client.handleError( e ); } ); return ui.saveStep( function( stepui ) @@ -1929,7 +1629,7 @@ module.exports = Class( 'Client' ) var client = this; // if the step contains invalid data, they must correct it - if ( !( stepui.isValid( this._cmatch ) ) ) + if ( !( stepui.isValid( this._cmatch.getMatches() ) ) ) { // well we didn't get very far callback( false ); @@ -1947,7 +1647,7 @@ module.exports = Class( 'Client' ) // we want to do this before save so that we don't re-mark the bucket as // dirty by populating it with uncommitted data - client._clearCmatchFields(); + client._cmatch.clearCmatchFields(); // give devs the option to disable client-side submit events so we can // test server-side functionality @@ -1959,7 +1659,7 @@ module.exports = Class( 'Client' ) // shouldn't occurr, we should still throw an exception if one is // triggered var failures = this.program.submit( - step_id, bucket, this._cmatch + step_id, bucket, this._cmatch.getMatches() ); if ( failures !== null ) @@ -2072,7 +1772,7 @@ module.exports = Class( 'Client' ) { event.abort(); - _self._handleError( Error( + _self.handleError( Error( 'Save timeout; please try again' ) ); }, 15000 ); @@ -2371,57 +2071,6 @@ module.exports = Class( 'Client' ) }, - 'private _clearCmatchFields': function() - { - var step = this.getUi().getCurrentStep(), - program = this.program; - - // don't bother if we're not yet on a step - if ( !step ) - { - return; - } - - var reset = {}; - for ( var name in step.getStep().getExclusiveFieldNames() ) - { - var data = this._cmatchHidden[ name ]; - - // if there is no data or we have been asked to retain this field's - // value, then do not clear - if ( !data || program.cretain[ name ] ) - { - continue; - } - - // what state is the current data in? - var cur = this.getQuote().getDataByName( name ); - - // we could have done Array.join(',').split(','), but we're trying - // to keep performance sane here - var indexes = []; - for ( var i in data ) - { - // we do *not* want to reset fields that have been removed - if ( cur[ i ] === undefined ) - { - break; - } - - indexes.push( i ); - } - - reset[ name ] = indexes; - } - - // batch reset (limit the number of times events are kicked off) - this._resetFields( reset ); - - // we've done our deed; reset it for the next time around - this._cmatchHidden = {}; - }, - - /** * Perform `forward' validations if needed * @@ -2449,7 +2098,7 @@ module.exports = Class( 'Client' ) var failures = this.program.forward( cur_step_id, bucket, - this._cmatch, + this._cmatch.getMatches(), function( trigger_event, question_id, value ) { client.handleEvent( trigger_event, { stepId: +value } ); @@ -2830,7 +2479,7 @@ module.exports = Class( 'Client' ) // proxy errors this._fieldMonitor.on( 'error', function( e ) { - _self._handleError( e ); + _self.handleError( e ); } ); }, @@ -2864,7 +2513,7 @@ module.exports = Class( 'Client' ) * * @return {undefined} */ - 'private _handleError': function( e ) + 'public handleError': function( e ) { if ( !e ) { @@ -2928,10 +2577,4 @@ module.exports = Class( 'Client' ) { return this.program.id; }, - - - 'public getCmatchData': function() - { - return this._cmatch; - } } ); diff --git a/src/client/ClientDependencyFactory.js b/src/client/ClientDependencyFactory.js index 03b2682..2ca86eb 100644 --- a/src/client/ClientDependencyFactory.js +++ b/src/client/ClientDependencyFactory.js @@ -82,9 +82,8 @@ var Step = require( '../step/Step' ), NavStyler = require( '../ui/nav/NavStyler' ), Sidebar = require( '../ui/sidebar/Sidebar' ), - FieldClassMatcher = require( '../field/FieldClassMatcher' ), - DataApiFactory = require( '../dapi/DataApiFactory' ), - DataApiManager = require( '../dapi/DataApiManager' ), + DataApiFactory = require( '../dapi/DataApiFactory' ), + DataApiManager = require( '../dapi/DataApiManager' ), RootDomContext = require( '../ui/context/RootDomContext' ), DomFieldFactory = require( '../ui/field/DomFieldFactory' ), @@ -345,8 +344,6 @@ module.exports = Class( 'ClientDependencyFactory', createNotifyBar: UiNotifyBar, - createFieldClassMatcher: FieldClassMatcher, - createClientEventHandler: function( client, data_validator, styler, data_proxy, jquery @@ -373,5 +370,5 @@ module.exports = Class( 'ClientDependencyFactory', 'action$cvv2Dialog': requireh( 'Cvv2DialogEventHandler' )( jquery ) } ); - } -}); + }, +} ); diff --git a/src/client/Cmatch.js b/src/client/Cmatch.js new file mode 100644 index 0000000..5766da3 --- /dev/null +++ b/src/client/Cmatch.js @@ -0,0 +1,482 @@ +/** + * Liza classification match (cmatch) handling + * + * Copyright (C) 2018 R-T Specialty, LLC. + * + * 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 . + * + * TODO: This is code directly extracted from Client, modified to maintain + * references to necessary objects. It is coupled with far too many things, + * and the code is a mess. Getting this clean and well-tested is important, + * as this is one of the core systems and is both complicated and complex. + */ + +"use strict"; + +const { Class } = require( 'easejs' ); + + +module.exports = Class( 'Cmatch', +{ + /** + * Contains classification match data per field + * + * @type {Object} + */ + 'private _cmatch': {}, + + /** + * Fields that were hidden (including indexes) since the last cmatch + * clear + * + * @type {Object} + */ + 'private _cmatchHidden': {}, + + /** + * Performs classification matching on fields + * + * A field will have a positive match for a given index if all of its + * classes match + * + * @type {FieldClassMatcher} + */ + 'private _classMatcher': null, + + /** + * Program client + * @type {Client} + */ + 'private _client': null, + + + /** + * Initialize match handler + * + * This relies on too many objects; see header. + * + * @param {FieldClassMatcher} class_matcher class/field matcher + * @param {Program} program active program + * @param {Client} client active client + */ + constructor( class_matcher, program, client ) + { + this._classMatcher = class_matcher; + this._program = program; + this._client = client; + }, + + + 'private _cmatchVisFromUi': function( field, all ) + { + var step = this._client.getUi().getCurrentStep(); + + if ( !step ) + { + return []; + } + + var group = step.getElementGroup( field ); + if ( !group ) + { + return []; + } + + var i = group.getCurrentIndexCount(), + ret = []; + + while ( i-- ) + { + ret.push( all ); + } + + return ret; + }, + + + 'public hookClassifier': function( data_validator ) + { + var _self = this, + program = this._program; + + // clear/initialize cmatches + this._cmatch = {}; + + var cmatchprot = false; + + // set classifier + this._client.getQuote() + .setClassifier( program.getClassifierKnownFields(), function() + { + return program.classify.apply( program, arguments ); + } ) + .on( 'classify', function( classes ) + { + if ( cmatchprot === true ) + { + _self._client.handleError( Error( 'cmatch recursion' ) ); + } + + cmatchprot = true; + + // handle field fixes + data_validator.validate( undefined, classes ) + .catch( e => _self.client._handleError( e ) ); + + _self._classMatcher.match( classes, function( cmatch ) + { + // it's important that we do this here so that everything + // that uses the cmatch data will consistently benefit + _self._postProcessCmatch( cmatch ); + + // if we're not on a current step, defer + if ( !( _self._client.getUi().getCurrentStep() ) ) + { + _self._cmatch = cmatch; + cmatchprot = false; + return; + } + + _self._handleClassMatch( cmatch ); + cmatchprot = false; + } ); + } ); + }, + + + 'private _postProcessCmatch': function( cmatch ) + { + // for any matches that are scalars (they will have no indexes), loop + // through each field and set the index to the value of 'all' + for ( var field in cmatch ) + { + if ( field === '__classes' ) + { + continue; + } + + var cfield = cmatch[ field ]; + + if ( cfield.indexes.length === 0 ) + { + var data = this._client.getQuote().getDataByName( field ), + i = data.length; + + // this will do nothing if there is no data found + while ( i-- ) + { + cfield.indexes[ i ] = cfield.all; + } + } + } + + return cmatch; + }, + + + 'private _mergeCmatchHidden': function( name, indexes, hidden ) + { + if ( !( this._cmatchHidden[ name ] ) ) + { + this._cmatchHidden[ name ] = {}; + } + + var cindexes = this._cmatchHidden[ name ]; + + for ( i in indexes ) + { + if ( hidden ) + { + cindexes[ indexes[ i ] ] = i; + } + else + { + delete cindexes[ indexes[ i ] ]; + } + } + + var some = false; + for ( var i in cindexes ) + { + some = true; + break; + } + + if ( !some ) + { + // v8 devs do not recomment delete as it progressively slows down + // property access on the object + this._cmatchHidden[ name ] = undefined; + } + }, + + + 'private _handleClassMatch': function( cmatch, force ) + { + force = !!force; + + this._client.getUi().setCmatch( cmatch ); + + var _self = this, + quote = this._client.getQuote(), + + // oh dear god...(Demeter, specifically..) + fields = this._client.getUi().getCurrentStep().getStep() + .getExclusiveFieldNames(); + + + var visq = []; + for ( var field in cmatch ) + { + // ignore fields that are not on the current step + if ( !( fields[ field ] ) ) + { + continue; + } + + // if the match is still false, then we can rest assured + // that nothing has changed (and skip the overhead) + if ( !force + && ( cmatch[ field ] === false ) + && ( _self._cmatch[ field ] === false ) + ) + { + continue; + } + + var show = [], + hide = [], + + cfield = cmatch[ field ], + + vis = cfield.indexes, + cur = ( + ( _self._cmatch[ field ] || {} ).indexes + || [] + ); + + // TODO: Figure out something better here. This is currently + // needed for hiding statics---they are registered as exclusive + // fields (`fields', above), but aren't actually fields (they're + // not in the bucket). But we must show/hide them appropriately. + if ( vis.length === 0 ) + { + vis = this._cmatchVisFromUi( field, cfield.all ); + } + + // consider the number of indexes in the bucket first; + // otherwise, we might try to operate on fields that don't + // exist (bucket/class indexes not the same). the check for + // undefined in the first index is a workaround for the explicit + // setting of the length property of the bucket value when + // indexes are removed + var curdata = quote.getDataByName( field ), + fieldn = ( curdata.length > 0 && ( curdata[ 0 ] !== undefined ) ) + ? curdata.length + : vis.length; + + for ( var i = 0; i < fieldn; i++ ) + { + // do not record unchanged indexes as changed + // (avoiding the event overhead) + if ( !force && ( vis[ i ] === cur[ i ] ) ) + { + continue; + } + + ( ( vis[ i ] ) ? show : hide ).push( i ); + } + + if ( show.length ) + { + visq[ field ] = { event_id: 'show', name: field, indexes: show }; + this._mergeCmatchHidden( field, show, false ); + } + + if ( hide.length ) + { + visq[ field ] = { event_id: 'hide', name: field, indexes: hide }; + this._mergeCmatchHidden( field, hide, true ); + } + } + + // it's important to do this before showing/hiding fields, since + // those might trigger events that check the current cmatches + this._cmatch = cmatch; + + + // allow DOM operations to complete before we trigger + // manipulations on it (TODO: this is a workaround for group + // show/hide issues; we need a better solution to guarantee + // order + setTimeout( () => + { + Object.keys( visq ).forEach( field => + { + const { event_id, name, indexes } = visq[ field ]; + + this._client.handleEvent( event_id, { + elementName: name, + indexes: indexes, + } ); + + this._dapiTrigger( name ); + } ); + }, 25 ); + }, + + + /** + * Trigger DataApi event for field FIELD + * + * @param {string} field field name + * + * @return {undefined} + */ + 'private _dapiTrigger': function( field ) + { + const current_step_id = this._client.nav.getCurrentStepId(); + + this._client.getQuote().visitData( bucket => + { + this._program.dapi( + current_step_id, + field, + bucket, + {}, + this._cmatch, + null + ); + } ); + }, + + + 'public clearCmatchFields': function() + { + var step = this._client.getUi().getCurrentStep(), + program = this._program; + + // don't bother if we're not yet on a step + if ( !step ) + { + return; + } + + var reset = {}; + for ( var name in step.getStep().getExclusiveFieldNames() ) + { + var data = this._cmatchHidden[ name ]; + + // if there is no data or we have been asked to retain this field's + // value, then do not clear + if ( !data || program.cretain[ name ] ) + { + continue; + } + + // what state is the current data in? + var cur = this._client.getQuote().getDataByName( name ); + + // we could have done Array.join(',').split(','), but we're trying + // to keep performance sane here + var indexes = []; + for ( var i in data ) + { + // we do *not* want to reset fields that have been removed + if ( cur[ i ] === undefined ) + { + break; + } + + indexes.push( i ); + } + + reset[ name ] = indexes; + } + + // batch reset (limit the number of times events are kicked off) + this._resetFields( reset ); + + // we've done our deed; reset it for the next time around + this._cmatchHidden = {}; + }, + + + 'private _resetFields': function( fields ) + { + const quote = this._client.getQuote(); + const update = {}; + + for ( var field in fields ) + { + var cur = fields[ field ], + cdata = quote.getDataByName( field ), + val = this._client.elementStyler.getDefault( field ); + + var data = []; + for ( var i in cur ) + { + var index = cur[ i ]; + + if ( cdata[ index ] === val ) + { + continue; + } + + data[ index ] = val; + } + + update[ field ] = data; + } + + quote.setData( update ); + }, + + + /** + * Force handling of the most recent cmatch data + * + * This can be used to refresh the UI to ensure that it is consistent with + * the cmatch data. + * + * @return {Client} self + */ + 'public forceCmatchAction': function() + { + if ( !( this._cmatch ) ) + { + return this; + } + + this._handleClassMatch( this._cmatch, true ); + + return this; + }, + + + /** + * Get matches from last classifier application + * + * TODO: Remove me; breaks encapsulation. Intended for transition from + * mammoth Client. + * + * @return {Object} classification matches + */ + 'public getMatches'() + { + return this._cmatch; + }, +} ); diff --git a/src/system/client.js b/src/system/client.js index 18652b5..a1e109c 100644 --- a/src/system/client.js +++ b/src/system/client.js @@ -21,7 +21,9 @@ "use strict"; -const store = require( '../store' ); +const Cmatch = require( '../client/Cmatch' ); +const field = require( '../field' ); +const store = require( '../store' ); /** @@ -33,6 +35,12 @@ const store = require( '../store' ); * This is incomplete; it will be added to as code is ported to liza. */ module.exports = { + cmatch: ( program, client ) => Cmatch( + field.FieldClassMatcher( program.whens ), + program, + client + ), + data: { /** * Create a store suitable for comparing diffs From 0a50b22496fc619fe075c61fe618ffdb9816aa10 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Fri, 9 Feb 2018 11:03:21 -0500 Subject: [PATCH 4/5] Cmatch: Extract show/hide marking into own method This is to give us a fairly easy means of testing this logic for a bugfix. This refactoring also obviates a pretty nasty bug; see docblock. * src/client/Cmatch.js (_handleClassMatch): Extract show/hide marking. (markShowHide): New method. --- src/client/Cmatch.js | 47 +++++++++++++++++++++++++++++++++----------- 1 file changed, 36 insertions(+), 11 deletions(-) diff --git a/src/client/Cmatch.js b/src/client/Cmatch.js index 5766da3..62913c6 100644 --- a/src/client/Cmatch.js +++ b/src/client/Cmatch.js @@ -300,17 +300,7 @@ module.exports = Class( 'Cmatch', ( ( vis[ i ] ) ? show : hide ).push( i ); } - if ( show.length ) - { - visq[ field ] = { event_id: 'show', name: field, indexes: show }; - this._mergeCmatchHidden( field, show, false ); - } - - if ( hide.length ) - { - visq[ field ] = { event_id: 'hide', name: field, indexes: hide }; - this._mergeCmatchHidden( field, hide, true ); - } + this.markShowHide( field, visq, show, hide ); } // it's important to do this before showing/hiding fields, since @@ -339,6 +329,41 @@ module.exports = Class( 'Cmatch', }, + /** + * Mark fields to be shown/hidden + * + * This also updates the cached visibility of field FIELD. + * + * XXX: This method makes very obvious a nasty bug where hides override + * shows if both are set. + * + * @param {string} field field name + * @param {Array} show indexes to show + * @param {Array} hide indexes to hide + * + * @return {undefined} + */ + 'protected markShowHide'( field, visq, show, hide ) + { + if ( !( show.length || hide.length ) ) + { + return; + } + + if ( show.length ) + { + this._mergeCmatchHidden( field, show, false ); + visq[ field ] = { event_id: 'show', name: field, indexes: show }; + } + + if ( hide.length ) + { + this._mergeCmatchHidden( field, hide, true ); + visq[ field ] = { event_id: 'hide', name: field, indexes: hide }; + } + }, + + /** * Trigger DataApi event for field FIELD * From 3b303e50a957cd58597c1439cf653a0bdbcb9e1b Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Fri, 9 Feb 2018 11:43:59 -0500 Subject: [PATCH 5/5] Cmatch: Fix combined show/hide of same field, multi-index This is something that managed to slip by (but not unnoticed) for almost exactly one year to this day (028606242a2f8ebbd558cf4752309054fdee427e). It can only be reproduced by changing classes that result in visibility changes differing on the same field by index. The issue hides itself on first load (because all fields are shown by default) and on refresh. The problem is that, when one index shows a field but another hides it, the hide overrode the show indexes, so only the hide took place. * src/client/Cmatch.js (markShowHide): Make virtual. New implementation to support concurrent show/hide. (_handleClassMatch): Use it. * test/client/CmatchTest.js: New test. * npm-shrinkwrap.json: ease.js v0.2.{8=>9}. --- npm-shrinkwrap.json | 2 +- src/client/Cmatch.js | 32 ++++++++------ test/client/CmatchTest.js | 87 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 108 insertions(+), 13 deletions(-) create mode 100644 test/client/CmatchTest.js diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index 6b583ba..b93cc56 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -38,7 +38,7 @@ "version": "3.2.0" }, "easejs": { - "version": "0.2.8" + "version": "0.2.9" }, "escape-string-regexp": { "version": "1.0.5" diff --git a/src/client/Cmatch.js b/src/client/Cmatch.js index 62913c6..4705e0c 100644 --- a/src/client/Cmatch.js +++ b/src/client/Cmatch.js @@ -238,7 +238,7 @@ module.exports = Class( 'Cmatch', .getExclusiveFieldNames(); - var visq = []; + var visq = {}; for ( var field in cmatch ) { // ignore fields that are not on the current step @@ -316,11 +316,16 @@ module.exports = Class( 'Cmatch', { Object.keys( visq ).forEach( field => { - const { event_id, name, indexes } = visq[ field ]; + const field_vis = visq[ field ]; - this._client.handleEvent( event_id, { - elementName: name, - indexes: indexes, + Object.keys( field_vis ).forEach( event_id => + { + const indexes = field_vis[ event_id ]; + + this._client.handleEvent( event_id, { + elementName: field, + indexes: indexes, + } ); } ); this._dapiTrigger( name ); @@ -334,33 +339,36 @@ module.exports = Class( 'Cmatch', * * This also updates the cached visibility of field FIELD. * - * XXX: This method makes very obvious a nasty bug where hides override - * shows if both are set. - * * @param {string} field field name * @param {Array} show indexes to show * @param {Array} hide indexes to hide * * @return {undefined} */ - 'protected markShowHide'( field, visq, show, hide ) + 'virtual protected markShowHide'( field, visq, show, hide ) { if ( !( show.length || hide.length ) ) { - return; + return visq; } + const { [field]: result = {} } = visq; + if ( show.length ) { this._mergeCmatchHidden( field, show, false ); - visq[ field ] = { event_id: 'show', name: field, indexes: show }; + result.show = show; } if ( hide.length ) { this._mergeCmatchHidden( field, hide, true ); - visq[ field ] = { event_id: 'hide', name: field, indexes: hide }; + result.hide = hide; } + + visq[ field ] = result; + + return visq; }, diff --git a/test/client/CmatchTest.js b/test/client/CmatchTest.js new file mode 100644 index 0000000..cfbfb14 --- /dev/null +++ b/test/client/CmatchTest.js @@ -0,0 +1,87 @@ +/** + * Test case for Cmatch + * + * Copyright (C) 2018 R-T Specialty, LLC. + * + * 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( '../../' ).client; +const { expect } = require( 'chai' ); + +const Sut = require( '../../src/client/Cmatch' ) + .extend( +{ + 'override constructor'( _, __, ___ ) {}, + + // make public + 'override public markShowHide'( field, visq, show, hide ) + { + return this.__super( field, visq, show, hide ); + } +} ); + + +// these tests aren't terribly effective right now +describe( "Cmatch", () => +{ + it( "marks hidden fields on class change to show", () => + { + expect( + Sut().markShowHide( 'foo', {}, [ 1, 2 ], [] ) + ).to.deep.equal( { foo: { show: [ 1, 2 ] } } ); + } ); + + + it( "marks shown fields on class change to hide", () => + { + expect( + Sut().markShowHide( 'foo', {}, [], [ 3, 4, 5 ] ) + ).to.deep.equal( { foo: { hide: [ 3, 4, 5 ] } } ); + } ); + + + it( "marks combination show/hide on class change", () => + { + expect( + Sut().markShowHide( 'foo', {}, [ 2, 3 ], [ 4, 5, 6 ] ) + ).to.deep.equal( { + foo: { + show: [ 2, 3 ], + hide: [ 4, 5, 6 ], + } + } ); + } ); + + + it( "marks no fields with no show or hide", () => + { + expect( + Sut().markShowHide( 'foo', {}, [], [] ) + ).to.deep.equal( {} ); + } ); + + + it( "does not affect marking of other fields", () => + { + const barval = {}; + const visq = { bar: barval }; + + Sut().markShowHide( 'foo', {}, [ 1 ], [ 0 ] ); + + expect( visq.bar ).to.equal( barval ); + } ); +} );