From ecdfea5cdba883bcc76f20c906bc59c93cde9a7e Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Mon, 12 Nov 2018 14:53:58 -0500 Subject: [PATCH] ProgramInit: Do not initialize bucket values for undefined question types These denote fields that are generated but do not actually have any data associated with them. For example, select options with predicates have a field generated so that they contribute to the group field count (so that the group will automatically show/hide appropriately), but those should never have values associated with them in the bucket. This was manifesting as a nasty bug: The bucket contained a key for generated options. When the quote is loaded, the client "empties" the bucket. In doing so, it set the option value to the empty string, which had the effect of rendering the dropdown useless---every value was the empty string! * src/program/ProgramInit.js (_isKnownType): New method. (init): Use it and ignore fields with unknown types. * src/server/Server.js: Add note that we shouldn't have this logic duplicated between ProgramInit and ProgramQuoteCleaner. * src/server/quote/ProgramQuoteCleaner.js (_fixGroup): Ignore fields with unknown types. (_isKnownType): New method. * test/program/ProgramInitTest.js: Update existing tests. Add new. * test/server/quote/ProgramQuoteCleanerTest.js: Test this case. --- src/program/ProgramInit.js | 37 ++++++++++-- src/server/Server.js | 2 + src/server/quote/ProgramQuoteCleaner.js | 25 ++++++++ test/program/ProgramInitTest.js | 60 ++++++++++++++++---- test/server/quote/ProgramQuoteCleanerTest.js | 12 +++- 5 files changed, 119 insertions(+), 17 deletions(-) diff --git a/src/program/ProgramInit.js b/src/program/ProgramInit.js index d4db00f..658486f 100644 --- a/src/program/ProgramInit.js +++ b/src/program/ProgramInit.js @@ -54,9 +54,12 @@ module.exports = Class( 'ProgramInit', */ 'public init'( program, doc_data ) { - const defaults = program.defaults || {}; const data = doc_data || {}; - const groups = program.meta.groups; + + const { + defaults = {}, + meta: { groups = {}, qtypes = {} }, + } = program; Object.keys( program.groupExclusiveFields ).forEach( group => { @@ -64,10 +67,17 @@ module.exports = Class( 'ProgramInit', while ( i-- ) { - const field = program.groupExclusiveFields[ group ][ i ]; + const field = program.groupExclusiveFields[ group ][ i ]; const default_value = defaults[ field ]; - if ( !defaults.hasOwnProperty( field )) + // generated questions with no types should never be part of + // the bucket + if ( !this._isKnownType( qtypes[ field ] ) ) + { + continue; + } + + if ( !defaults.hasOwnProperty( field ) ) { continue; } @@ -96,8 +106,25 @@ module.exports = Class( 'ProgramInit', } } } - }); + } ); return Promise.resolve( data ); }, + + + /** + * Determine whether question type QTYPE is known + * + * This assumes that the type is known unless QTYPE.type is "undefined". + * + * @param {Object} qtype type data for question + * + * @return {boolean} whether type is known + */ + 'private _isKnownType'( qtype ) + { + return qtype + && ( typeof qtype.type === 'string' ) + && ( qtype.type !== 'undefined' ); + }, } ); diff --git a/src/server/Server.js b/src/server/Server.js index 6fd7127..60a965c 100644 --- a/src/server/Server.js +++ b/src/server/Server.js @@ -536,6 +536,8 @@ module.exports = Class( 'Server' ) */ 'private _getDefaultBucket': function( program, quote_data ) { + // TOOD: this duplicates some logic with ProgramQuoteCleaner; we + // probably do not need both of them return this._progInit.init( program, quote_data.data ); }, diff --git a/src/server/quote/ProgramQuoteCleaner.js b/src/server/quote/ProgramQuoteCleaner.js index 3eac650..001ca97 100644 --- a/src/server/quote/ProgramQuoteCleaner.js +++ b/src/server/quote/ProgramQuoteCleaner.js @@ -102,11 +102,19 @@ module.exports = Class( 'ProgramQuoteCleaner', const update = {}; const group_fields = this._program.groupExclusiveFields[ group_id ]; + const qtypes = this._program.meta.qtypes || {}; group_fields.forEach( field => { const flen = ( quote.getDataByName( field ) || [] ).length; + // generated questions with no types should never be part of + // the bucket + if ( !this._isKnownType( qtypes[ field ] ) ) + { + return; + } + if ( flen >= length ) { return; @@ -182,5 +190,22 @@ module.exports = Class( 'ProgramQuoteCleaner', metabucket.setValues( { [field_name]: [] } ); } ); }, + + + /** + * Determine whether question type QTYPE is known + * + * This assumes that the type is known unless QTYPE.type is "undefined". + * + * @param {Object} qtype type data for question + * + * @return {boolean} whether type is known + */ + 'private _isKnownType'( qtype ) + { + return qtype + && ( typeof qtype.type === 'string' ) + && ( qtype.type !== 'undefined' ); + }, } ); diff --git a/test/program/ProgramInitTest.js b/test/program/ProgramInitTest.js index 8d3ee90..02cea98 100644 --- a/test/program/ProgramInitTest.js +++ b/test/program/ProgramInitTest.js @@ -35,7 +35,11 @@ describe( 'ProgramInit', () => label: "initializes defaults", defaults: { a: "one", b: "two" }, meta: { - groups: {} + groups: {}, + qtypes: { + a: { type: "noyes" }, + b: { type: "noyes" }, + }, }, groupExclusiveFields: { Something: [ "a", "b" ] @@ -73,7 +77,11 @@ describe( 'ProgramInit', () => bar: "test" }, meta: { - groups: {} + groups: {}, + qtypes: { + foo: { type: "noyes" }, + bar: { type: "noyes" }, + }, }, groupExclusiveFields: { Something: [ "foo" ], @@ -89,7 +97,10 @@ describe( 'ProgramInit', () => label: "keeps existing data with no defaults", defaults: {}, meta: { - groups: {} + groups: {}, + qtypes: { + bar: { type: "text" }, + }, }, groupExclusiveFields: { SomethingElse: [ "bar" ] @@ -103,7 +114,10 @@ describe( 'ProgramInit', () => label: "does not overwrite existing data with defaults", defaults: { foo: "init" }, meta: { - groups: {} + groups: {}, + qtypes: { + foo: { type: "text" }, + }, }, groupExclusiveFields: { Something: [ "foo" ] @@ -121,7 +135,10 @@ describe( 'ProgramInit', () => Something: { min: 3 } - } + }, + qtypes: { + foo: { type: "text" }, + }, }, groupExclusiveFields: { Something: [ "foo" ] @@ -139,7 +156,10 @@ describe( 'ProgramInit', () => Something: { min: 5 } - } + }, + qtypes: { + foo: { type: "text" }, + }, }, groupExclusiveFields: { Something: [ "foo" ] @@ -157,7 +177,10 @@ describe( 'ProgramInit', () => Something: { min: 5 } - } + }, + qtypes: { + foo: { type: "text" }, + }, }, groupExclusiveFields: { Something: [ "foo" ] @@ -169,6 +192,21 @@ describe( 'ProgramInit', () => foo: [ "1", "2", "3", "4", "init" ], }, }, + { + label: "ignores undefined types", + defaults: { foo: "default" }, + meta: { + groups: {}, + qtypes: { + foo: { type: "undefined" }, + }, + }, + groupExclusiveFields: { + Something: [ "foo" ] + }, + doc_data: {}, + expected: {}, + }, ].forEach( ({ label, doc_data, id, defaults, meta, groupExclusiveFields, expected }) => { it( label, () => @@ -176,10 +214,10 @@ describe( 'ProgramInit', () => const sut = Sut( null ); const program = { - id: "foo", - defaults: defaults, - meta: meta, - groupExclusiveFields : groupExclusiveFields + id: "foo", + defaults: defaults, + meta: meta, + groupExclusiveFields: groupExclusiveFields }; return expect( sut.init( program, doc_data ) ) diff --git a/test/server/quote/ProgramQuoteCleanerTest.js b/test/server/quote/ProgramQuoteCleanerTest.js index cb09a4c..957f7ae 100644 --- a/test/server/quote/ProgramQuoteCleanerTest.js +++ b/test/server/quote/ProgramQuoteCleanerTest.js @@ -42,13 +42,22 @@ describe( 'ProgramQuoteCleaner', () => exclusive: { one: [ "field11", "field12" ], two: [ "field21", "field22" ], - three: [ "field31", "field32" ], + three: [ "field31", "field32", "unknown_ignore_me" ], }, defaults: { field12: "12default", }, + qtypes: { + "field11": { type: "text" }, + "field12": { type: "text" }, + "field21": { type: "text" }, + "field22": { type: "text" }, + "field31": { type: "text" }, + "field32": { type: "text" }, + }, + existing: { "field11": [ "1", "", "3" ], // leader one, two "field12": [ "a", "b" ], @@ -74,6 +83,7 @@ describe( 'ProgramQuoteCleaner', () => program.defaults = test.defaults; program.groupIndexField = test.group_index; program.groupExclusiveFields = test.exclusive; + program.meta.qtypes = test.qtypes; const updates = {};