From 25c2ad91f29aeeef0fecd51a4385e1a7acc19724 Mon Sep 17 00:00:00 2001 From: Moriel Schottlender Date: Tue, 2 May 2017 14:17:45 -0700 Subject: [PATCH] RCFilter UI: allow getParametersFromFilters to accept filter list Up until now we assumed that this method should output the parameters from the state of the system, which meant that even if it accepted an external list, it required that it be the format of this.groups. Instead, allow the method to accept a "flat" list of filter name/values and translate that to a parameter state without changing the model state. Change-Id: Ib41da620a16d874326fe73220ae7a0114c555639 --- .../dm/mw.rcfilters.dm.FilterGroup.js | 56 ++++++--- .../dm/mw.rcfilters.dm.FiltersViewModel.js | 34 ++++-- .../dm.FiltersViewModel.test.js | 106 +++++++++++++++++- 3 files changed, 175 insertions(+), 21 deletions(-) diff --git a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterGroup.js b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterGroup.js index ca7c4e67df..63e13fdd8a 100644 --- a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterGroup.js +++ b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterGroup.js @@ -253,33 +253,63 @@ /** * Get the parameter representation from this group * + * @param {Object} [filterRepresentation] An object defining the state + * of the filters in this group, keyed by their name and current selected + * state value. * @return {Object} Parameter representation */ - mw.rcfilters.dm.FilterGroup.prototype.getParamRepresentation = function () { - var i, values, + mw.rcfilters.dm.FilterGroup.prototype.getParamRepresentation = function ( filterRepresentation ) { + var values, + areAnySelected = false, + buildFromCurrentState = !filterRepresentation, result = {}, - filterItems = this.getItems(); + filterParamNames = {}; + + filterRepresentation = filterRepresentation || {}; + + // Create or complete the filterRepresentation definition + this.getItems().forEach( function ( item ) { + // Map filter names to their parameter names + filterParamNames[ item.getName() ] = item.getParamName(); + + if ( buildFromCurrentState ) { + // This means we have not been given a filter representation + // so we are building one based on current state + filterRepresentation[ item.getName() ] = item.isSelected(); + } else if ( !filterRepresentation[ item.getName() ] ) { + // We are given a filter representation, but we have to make + // sure that we fill in the missing filters if there are any + // we will assume they are all falsey + filterRepresentation[ item.getName() ] = false; + } + if ( filterRepresentation[ item.getName() ] ) { + areAnySelected = true; + } + } ); + + // Build result if ( this.getType() === 'send_unselected_if_any' ) { // First, check if any of the items are selected at all. // If none is selected, we're treating it as if they are // all false // Go over the items and define the correct values - for ( i = 0; i < filterItems.length; i++ ) { - result[ filterItems[ i ].getParamName() ] = this.areAnySelected() ? - Number( !filterItems[ i ].isSelected() ) : 0; - } - + $.each( filterRepresentation, function ( name, value ) { + result[ filterParamNames[ name ] ] = areAnySelected ? + Number( !value ) : 0; + } ); } else if ( this.getType() === 'string_options' ) { values = []; - for ( i = 0; i < filterItems.length; i++ ) { - if ( filterItems[ i ].isSelected() ) { - values.push( filterItems[ i ].getParamName() ); + + $.each( filterRepresentation, function ( name, value ) { + // Collect values + if ( value ) { + values.push( filterParamNames[ name ] ); } - } + } ); - result[ this.getName() ] = ( values.length === filterItems.length ) ? + result[ this.getName() ] = ( values.length === Object.keys( filterRepresentation ).length ) ? 'all' : values.join( this.getSeparator() ); } diff --git a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FiltersViewModel.js b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FiltersViewModel.js index 69210be19f..18a9094770 100644 --- a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FiltersViewModel.js +++ b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FiltersViewModel.js @@ -472,17 +472,37 @@ * Analyze the groups and their filters and output an object representing * the state of the parameters they represent. * - * @param {Object} [filterGroups] An object defining the filter groups to - * translate to parameters. Its structure must follow that of this.groups - * see #getFilterGroups + * @param {Object} [filterDefinition] An object defining the filter values, + * keyed by filter names. * @return {Object} Parameter state object */ - mw.rcfilters.dm.FiltersViewModel.prototype.getParametersFromFilters = function ( filterGroups ) { - var result = {}, - groupItems = filterGroups || this.getFilterGroups(); + mw.rcfilters.dm.FiltersViewModel.prototype.getParametersFromFilters = function ( filterDefinition ) { + var groupItemDefinition, + result = {}, + groupItems = this.getFilterGroups(); + + if ( filterDefinition ) { + groupItemDefinition = {}; + // Filter definition is "flat", but in effect + // each group needs to tell us its result based + // on the values in it. We need to split this list + // back into groupings so we can "feed" it to the + // loop below, and we need to expand it so it includes + // all filters (set to false) + this.getItems().forEach( function ( filterItem ) { + groupItemDefinition[ filterItem.getGroupName() ] = groupItemDefinition[ filterItem.getGroupName() ] || {}; + groupItemDefinition[ filterItem.getGroupName() ][ filterItem.getName() ] = !!filterDefinition[ filterItem.getName() ]; + } ); + } $.each( groupItems, function ( group, model ) { - $.extend( result, model.getParamRepresentation() ); + $.extend( + result, + model.getParamRepresentation( + groupItemDefinition ? + groupItemDefinition[ group ] : null + ) + ); } ); return result; diff --git a/tests/qunit/suites/resources/mediawiki.rcfilters/dm.FiltersViewModel.test.js b/tests/qunit/suites/resources/mediawiki.rcfilters/dm.FiltersViewModel.test.js index 27d38258ef..8071d6e864 100644 --- a/tests/qunit/suites/resources/mediawiki.rcfilters/dm.FiltersViewModel.test.js +++ b/tests/qunit/suites/resources/mediawiki.rcfilters/dm.FiltersViewModel.test.js @@ -333,7 +333,7 @@ hidefilter6: 0, group3: '' }, - 'One filters in one "send_unselected_if_any" group returns the other parameters truthy.' + 'Two filters in one "send_unselected_if_any" group returns the other parameters truthy.' ); // Select 3 filters @@ -433,6 +433,110 @@ } ); + QUnit.test( 'getParametersFromFilters (custom object)', function ( assert ) { + var originalState, + model = new mw.rcfilters.dm.FiltersViewModel(), + definition = [ { + name: 'group1', + title: 'Group 1', + type: 'send_unselected_if_any', + filters: [ + { name: 'hidefilter1', label: 'Hide filter 1', description: '' }, + { name: 'hidefilter2', label: 'Hide filter 2', description: '' }, + { name: 'hidefilter3', label: 'Hide filter 3', description: '' } + ] + }, { + name: 'group2', + title: 'Group 2', + type: 'send_unselected_if_any', + filters: [ + { name: 'hidefilter4', label: 'Hide filter 4', description: '' }, + { name: 'hidefilter5', label: 'Hide filter 5', description: '' }, + { name: 'hidefilter6', label: 'Hide filter 6', description: '' } + ] + }, { + name: 'group3', + title: 'Group 3', + type: 'string_options', + separator: ',', + filters: [ + { name: 'filter7', label: 'Hide filter 7', description: '' }, + { name: 'filter8', label: 'Hide filter 8', description: '' }, + { name: 'filter9', label: 'Hide filter 9', description: '' } + ] + } ], + cases = [ + { + // This is mocking the cases above, both + // - 'Two filters in one "send_unselected_if_any" group returns the other parameters truthy.' + // - 'Two filters selected in "string_option" group returns those filters in the value.' + input: { + group1__hidefilter1: true, + group1__hidefilter2: true, + group1__hidefilter3: false, + group2__hidefilter4: false, + group2__hidefilter5: false, + group2__hidefilter6: false, + group3__filter7: true, + group3__filter8: true, + group3__filter9: false + }, + expected: { + // Group 1 (two selected, the others are true) + hidefilter1: 0, + hidefilter2: 0, + hidefilter3: 1, + // Group 2 (nothing is selected, all false) + hidefilter4: 0, + hidefilter5: 0, + hidefilter6: 0, + group3: 'filter7,filter8' + }, + msg: 'Given an explicit (complete) filter state object, the result is the same as if the object given represented the model state.' + }, + { + // This is mocking case above + // - 'One filters in one "send_unselected_if_any" group returns the other parameters truthy.' + input: { + group1__hidefilter1: 1 + }, + expected: { + // Group 1 (one selected, the others are true) + hidefilter1: 0, + hidefilter2: 1, + hidefilter3: 1, + // Group 2 (nothing is selected, all false) + hidefilter4: 0, + hidefilter5: 0, + hidefilter6: 0, + group3: '' + }, + msg: 'Given an explicit (incomplete) filter state object, the result is the same as if the object give represented the model state.' + } + ]; + + model.initializeFilters( definition ); + // Store original state + originalState = model.getSelectedState(); + + // Test each case + cases.forEach( function ( test ) { + assert.deepEqual( + model.getParametersFromFilters( test.input ), + test.expected, + test.msg + ); + } ); + + // After doing the above tests, make sure the actual state + // of the filter stayed the same + assert.deepEqual( + model.getSelectedState(), + originalState, + 'Running the method with external definition to parse does not actually change the state of the model' + ); + } ); + QUnit.test( 'getFiltersFromParameters', function ( assert ) { var definition = [ { name: 'group1', -- 2.20.1