jenkins-bot has submitted this change and it was merged. ( 
https://gerrit.wikimedia.org/r/352984 )

Change subject: Refactor filter-specific logic into group models
......................................................................


Refactor filter-specific logic into group models

Make the view model more generic and ready to accept other types
of filtering, like namespaces and tags, by handing off the responsibility
and logic of the parameterizing and filter relationships to the groups.

Change-Id: I9c333719e89088d96d044d72ddb9c39feeeb68ca
---
M resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterGroup.js
M resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FiltersViewModel.js
M resources/src/mediawiki.rcfilters/mw.rcfilters.js
M tests/qunit/suites/resources/mediawiki.rcfilters/dm.FiltersViewModel.test.js
4 files changed, 456 insertions(+), 239 deletions(-)

Approvals:
  Sbisson: Looks good to me, approved
  jenkins-bot: Verified



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 63e13fd..22b2619 100644
--- a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterGroup.js
+++ b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterGroup.js
@@ -38,6 +38,7 @@
                this.whatsThis = config.whatsThis || {};
 
                this.conflicts = config.conflicts || {};
+               this.defaultParams = {};
 
                this.aggregate( { update: 'filterItemUpdate' } );
                this.connect( this, { filterItemUpdate: 'onFilterItemUpdate' } 
);
@@ -57,6 +58,96 @@
         */
 
        /* Methods */
+
+       /**
+        * Initialize the group and create its filter items
+        *
+        * @param {Object} filterDefinition Filter definition for this group
+        * @param {string|Object} [groupDefault] Definition of the group default
+        */
+       mw.rcfilters.dm.FilterGroup.prototype.initializeFilters = function ( 
filterDefinition, groupDefault ) {
+               var supersetMap = {},
+                       model = this,
+                       items = [];
+
+               filterDefinition.forEach( function ( filter ) {
+                       // Instantiate an item
+                       var subsetNames = [],
+                               filterItem = new mw.rcfilters.dm.FilterItem( 
filter.name, model, {
+                                       group: model.getName(),
+                                       label: mw.msg( filter.label ),
+                                       description: mw.msg( filter.description 
),
+                                       cssClass: filter.cssClass
+                               } );
+
+                       filter.subset = filter.subset || [];
+                       filter.subset = filter.subset.map( function ( el ) {
+                               return el.filter;
+                       } );
+
+                       if ( filter.subset ) {
+                               subsetNames = [];
+                               filter.subset.forEach( function ( 
subsetFilterName ) { // eslint-disable-line no-loop-func
+                                       // Subsets (unlike conflicts) are 
always inside the same group
+                                       // We can re-map the names of the 
filters we are getting from
+                                       // the subsets with the group prefix
+                                       var subsetName = model.getPrefixedName( 
subsetFilterName );
+                                       // For convenience, we should store 
each filter's "supersets" -- these are
+                                       // the filters that have that item in 
their subset list. This will just
+                                       // make it easier to go through whether 
the item has any other items
+                                       // that affect it (and are selected) at 
any given time
+                                       supersetMap[ subsetName ] = 
supersetMap[ subsetName ] || [];
+                                       
mw.rcfilters.utils.addArrayElementsUnique(
+                                               supersetMap[ subsetName ],
+                                               filterItem.getName()
+                                       );
+
+                                       // Translate subset param name to add 
the group name, so we
+                                       // get consistent naming. We know that 
subsets are only within
+                                       // the same group
+                                       subsetNames.push( subsetName );
+                               } );
+
+                               // Set translated subset
+                               filterItem.setSubset( subsetNames );
+                       }
+
+                       items.push( filterItem );
+
+                       // Store default parameter state; in this case, default 
is defined per filter
+                       if ( model.getType() === 'send_unselected_if_any' ) {
+                               // Store the default parameter state
+                               // For this group type, parameter values are 
direct
+                               model.defaultParams[ filter.name ] = Number( 
!!filter.default );
+                       }
+               } );
+
+               // Add items
+               this.addItems( items );
+
+               // Now that we have all items, we can apply the superset map
+               this.getItems().forEach( function ( filterItem ) {
+                       filterItem.setSuperset( supersetMap[ 
filterItem.getName() ] );
+               } );
+
+               // Store default parameter state; in this case, default is 
defined per the
+               // entire group, given by groupDefault method parameter
+               if ( this.getType() === 'string_options' ) {
+                       // Store the default parameter group state
+                       // For this group, the parameter is group name and 
value is the names
+                       // of selected items
+                       this.defaultParams[ this.getName() ] = 
mw.rcfilters.utils.normalizeParamOptions(
+                               // Current values
+                               groupDefault ?
+                                       groupDefault.split( this.getSeparator() 
) :
+                                       [],
+                               // Legal values
+                               this.getItems().map( function ( item ) {
+                                       return item.getParamName();
+                               } )
+                       ).join( this.getSeparator() );
+               }
+       };
 
        /**
         * Respond to filterItem update event
@@ -89,6 +180,15 @@
         */
        mw.rcfilters.dm.FilterGroup.prototype.getName = function () {
                return this.name;
+       };
+
+       /**
+        * Get the default param state of this group
+        *
+        * @return {Object} Default param state
+        */
+       mw.rcfilters.dm.FilterGroup.prototype.getDefaultParams = function () {
+               return this.defaultParams;
        };
 
        /**
@@ -141,6 +241,21 @@
         */
        mw.rcfilters.dm.FilterGroup.prototype.setConflicts = function ( 
conflicts ) {
                this.conflicts = conflicts;
+       };
+
+       /**
+        * Set conflicts for each filter item in the group based on the
+        * given conflict map
+        *
+        * @param {Object} conflicts Object representing the conflict map,
+        *  keyed by the item name, where its value is an object for all its 
conflicts
+        */
+       mw.rcfilters.dm.FilterGroup.prototype.setFilterConflicts = function ( 
conflicts ) {
+               this.getItems().forEach( function ( filterItem ) {
+                       if ( conflicts[ filterItem.getName() ] ) {
+                               filterItem.setConflicts( conflicts[ 
filterItem.getName() ] );
+                       }
+               } );
        };
 
        /**
@@ -317,6 +432,97 @@
        };
 
        /**
+        * Get the filter representation this group would provide
+        * based on given parameter states.
+        *
+        * @param {Object|string} [paramRepresentation] An object defining a 
parameter
+        *  state to translate the filter state from. If not given, an object
+        *  representing all filters as falsey is returned; same as if the 
parameter
+        *  given were an empty object, or had some of the filters missing.
+        * @return {Object} Filter representation
+        */
+       mw.rcfilters.dm.FilterGroup.prototype.getFilterRepresentation = 
function ( paramRepresentation ) {
+               var areAnySelected, paramValues,
+                       model = this,
+                       paramToFilterMap = {},
+                       result = {};
+
+               if ( this.getType() === 'send_unselected_if_any' ) {
+                       paramRepresentation = paramRepresentation || {};
+                       // Expand param representation to include all filters 
in the group
+                       this.getItems().forEach( function ( filterItem ) {
+                               paramRepresentation[ filterItem.getParamName() 
] = !!paramRepresentation[ filterItem.getParamName() ];
+                               paramToFilterMap[ filterItem.getParamName() ] = 
filterItem;
+
+                               if ( paramRepresentation[ 
filterItem.getParamName() ] ) {
+                                       areAnySelected = true;
+                               }
+                       } );
+
+                       $.each( paramRepresentation, function ( paramName, 
paramValue ) {
+                               var filterItem = paramToFilterMap[ paramName ];
+
+                               result[ filterItem.getName() ] = areAnySelected 
?
+                                       // Flip the definition between the 
parameter
+                                       // state and the filter state
+                                       // This is what the 'toggleSelected' 
value of the filter is
+                                       !Number( paramValue ) :
+                                       // Otherwise, there are no selected 
items in the
+                                       // group, which means the state is false
+                                       false;
+                       } );
+               } else if ( this.getType() === 'string_options' ) {
+                       paramRepresentation = paramRepresentation || '';
+
+                       // Normalize the given parameter values
+                       paramValues = mw.rcfilters.utils.normalizeParamOptions(
+                               // Given
+                               paramRepresentation.split(
+                                       this.getSeparator()
+                               ),
+                               // Allowed values
+                               this.getItems().map( function ( filterItem ) {
+                                       return filterItem.getParamName();
+                               } )
+                       );
+                       // Translate the parameter values into a filter 
selection state
+                       this.getItems().forEach( function ( filterItem ) {
+                               result[ filterItem.getName() ] = (
+                                               // If it is the word 'all'
+                                               paramValues.length === 1 && 
paramValues[ 0 ] === 'all' ||
+                                               // All values are written
+                                               paramValues.length === 
model.getItemCount()
+                                       ) ?
+                                       // All true (either because all values 
are written or the term 'all' is written)
+                                       // is the same as all filters set to 
true
+                                       true :
+                                       // Otherwise, the filter is selected 
only if it appears in the parameter values
+                                       paramValues.indexOf( 
filterItem.getParamName() ) > -1;
+                       } );
+               }
+
+               // Go over result and make sure all filters are represented.
+               // If any filters are missing, they will get a falsey value
+               this.getItems().forEach( function ( filterItem ) {
+                       result[ filterItem.getName() ] = !!result[ 
filterItem.getName() ];
+               } );
+
+               return result;
+       };
+
+       /**
+        * Get item by its parameter name
+        *
+        * @param {string} paramName Parameter name
+        * @return {mw.rcfilters.dm.FilterItem} Filter item
+        */
+       mw.rcfilters.dm.FilterGroup.prototype.getItemByParamName = function ( 
paramName ) {
+               return this.getItems().filter( function ( item ) {
+                       return item.getParamName() === paramName;
+               } )[ 0 ];
+       };
+
+       /**
         * Get group type
         *
         * @return {string} Group type
@@ -326,8 +532,9 @@
        };
 
        /**
-        * Get the prefix used for the filter names inside this group
+        * Get the prefix used for the filter names inside this group.
         *
+        * @param {string} [name] Filter name to prefix
         * @return {string} Group prefix
         */
        mw.rcfilters.dm.FilterGroup.prototype.getNamePrefix = function () {
@@ -335,6 +542,16 @@
        };
 
        /**
+        * Get a filter name with the prefix used for the filter names inside 
this group.
+        *
+        * @param {string} name Filter name to prefix
+        * @return {string} Group prefix
+        */
+       mw.rcfilters.dm.FilterGroup.prototype.getPrefixedName = function ( name 
) {
+               return this.getNamePrefix() + name;
+       };
+
+       /**
         * Get group's title
         *
         * @return {string} Title
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 9054fe4..88ce33c 100644
--- a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FiltersViewModel.js
+++ b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FiltersViewModel.js
@@ -192,23 +192,21 @@
         * @param {Array} filters Filter group definition
         */
        mw.rcfilters.dm.FiltersViewModel.prototype.initializeFilters = function 
( filters ) {
-               var i, filterItem, filterConflictResult, groupConflictResult, 
subsetNames,
+               var filterItem, filterConflictResult, groupConflictResult,
                        model = this,
                        items = [],
-                       supersetMap = {},
                        groupConflictMap = {},
                        filterConflictMap = {},
-                       addArrayElementsUnique = function ( arr, elements ) {
-                               elements = Array.isArray( elements ) ? elements 
: [ elements ];
-
-                               elements.forEach( function ( element ) {
-                                       if ( arr.indexOf( element ) === -1 ) {
-                                               arr.push( element );
-                                       }
-                               } );
-
-                               return arr;
-                       },
+                       /*!
+                        * Expand a conflict definition from group name to
+                        * the list of all included filters in that group.
+                        * We do this so that the direct relationship in the
+                        * models are consistently item->items rather than
+                        * mixing item->group with item->item.
+                        *
+                        * @param {Object} obj Conflict definition
+                        * @return {Object} Expanded conflict definition
+                        */
                        expandConflictDefinitions = function ( obj ) {
                                var result = {};
 
@@ -220,7 +218,7 @@
                                                var filter;
 
                                                if ( conflict.filter ) {
-                                                       filterName = 
model.groups[ conflict.group ].getNamePrefix() + conflict.filter;
+                                                       filterName = 
model.groups[ conflict.group ].getPrefixedName( conflict.filter );
                                                        filter = 
model.getItemByName( filterName );
 
                                                        // Rename
@@ -262,7 +260,8 @@
                this.groups = {};
 
                filters.forEach( function ( data ) {
-                       var group = data.name;
+                       var i,
+                               group = data.name;
 
                        if ( !model.groups[ group ] ) {
                                model.groups[ group ] = new 
mw.rcfilters.dm.FilterGroup( group, {
@@ -278,77 +277,25 @@
                                        }
                                } );
                        }
+                       model.groups[ group ].initializeFilters( data.filters, 
data.default );
+                       items = items.concat( model.groups[ group ].getItems() 
);
 
+                       // Prepare conflicts
                        if ( data.conflicts ) {
+                               // Group conflicts
                                groupConflictMap[ group ] = data.conflicts;
                        }
 
                        for ( i = 0; i < data.filters.length; i++ ) {
-                               data.filters[ i ].subset = data.filters[ i 
].subset || [];
-                               data.filters[ i ].subset = data.filters[ i 
].subset.map( function ( el ) {
-                                       return el.filter;
-                               } );
-
-                               filterItem = new mw.rcfilters.dm.FilterItem( 
data.filters[ i ].name, model.groups[ group ], {
-                                       group: group,
-                                       label: mw.msg( data.filters[ i ].label 
),
-                                       description: mw.msg( data.filters[ i 
].description ),
-                                       cssClass: data.filters[ i ].cssClass
-                               } );
-
-                               if ( data.filters[ i ].subset ) {
-                                       subsetNames = [];
-                                       data.filters[ i ].subset.forEach( 
function ( subsetFilterName ) { // eslint-disable-line no-loop-func
-                                               var subsetName = model.groups[ 
group ].getNamePrefix() + subsetFilterName;
-                                               // For convenience, we should 
store each filter's "supersets" -- these are
-                                               // the filters that have that 
item in their subset list. This will just
-                                               // make it easier to go through 
whether the item has any other items
-                                               // that affect it (and are 
selected) at any given time
-                                               supersetMap[ subsetName ] = 
supersetMap[ subsetName ] || [];
-                                               addArrayElementsUnique(
-                                                       supersetMap[ subsetName 
],
-                                                       filterItem.getName()
-                                               );
-
-                                               // Translate subset param name 
to add the group name, so we
-                                               // get consistent naming. We 
know that subsets are only within
-                                               // the same group
-                                               subsetNames.push( subsetName );
-                                       } );
-
-                                       // Set translated subset
-                                       filterItem.setSubset( subsetNames );
-                               }
-
-                               // Store conflicts
+                               // Filter conflicts
                                if ( data.filters[ i ].conflicts ) {
+                                       filterItem = model.groups[ group 
].getItemByParamName( data.filters[ i ].name );
                                        filterConflictMap[ filterItem.getName() 
] = data.filters[ i ].conflicts;
                                }
-
-                               if ( data.type === 'send_unselected_if_any' ) {
-                                       // Store the default parameter state
-                                       // For this group type, parameter 
values are direct
-                                       model.defaultParams[ data.filters[ i 
].name ] = Number( !!data.filters[ i ].default );
-                               }
-
-                               model.groups[ group ].addItems( filterItem );
-                               items.push( filterItem );
-                       }
-
-                       if ( data.type === 'string_options' ) {
-                               // Store the default parameter group state
-                               // For this group, the parameter is group name 
and value is the names
-                               // of selected items
-                               model.defaultParams[ group ] = 
model.sanitizeStringOptionGroup(
-                                       group,
-                                       data.default ?
-                                               data.default.split( 
model.groups[ group ].getSeparator() ) :
-                                               []
-                               ).join( model.groups[ group ].getSeparator() );
                        }
                } );
 
-               // Add items to the model
+               // Add item references to the model, for lookup
                this.addItems( items );
 
                // Expand conflicts
@@ -360,14 +307,11 @@
                        model.groups[ group ].setConflicts( conflicts );
                } );
 
-               items.forEach( function ( filterItem ) {
-                       // Apply the superset map
-                       filterItem.setSuperset( supersetMap[ 
filterItem.getName() ] );
-
-                       // set conflicts for item
-                       if ( filterConflictResult[ filterItem.getName() ] ) {
-                               filterItem.setConflicts( filterConflictResult[ 
filterItem.getName() ] );
-                       }
+               // Set conflicts for items
+               $.each( filterConflictResult, function ( filterName, conflicts 
) {
+                       var filterItem = model.getItemByName( filterName );
+                       // set conflicts for items in the group
+                       filterItem.setConflicts( conflicts );
                } );
 
                // Create a map between known parameters and their models
@@ -383,6 +327,7 @@
                        }
                } );
 
+               // Finish initialization
                this.emit( 'initialize' );
        };
 
@@ -453,12 +398,18 @@
        };
 
        /**
-        * Get the default parameters object
+        * Get an object representing default parameters state
         *
         * @return {Object} Default parameter values
         */
        mw.rcfilters.dm.FiltersViewModel.prototype.getDefaultParams = function 
() {
-               return this.defaultParams;
+               var result = {};
+
+               $.each( this.groups, function ( name, model ) {
+                       result = $.extend( true, {}, result, 
model.getDefaultParams() );
+               } );
+
+               return result;
        };
 
        /**
@@ -502,6 +453,52 @@
        };
 
        /**
+        * This is the opposite of the #getParametersFromFilters method; this 
goes over
+        * the given parameters and translates into a selected/unselected value 
in the filters.
+        *
+        * @param {Object} params Parameters query object
+        * @return {Object} Filter state object
+        */
+       mw.rcfilters.dm.FiltersViewModel.prototype.getFiltersFromParameters = 
function ( params ) {
+               var groupMap = {},
+                       model = this,
+                       result = {};
+
+               // Go over the given parameters, break apart to groupings
+               // The resulting object represents the group with its parameter
+               // values. For example:
+               // {
+               //    group1: {
+               //       param1: "1",
+               //       param2: "0",
+               //       param3: "1"
+               //    },
+               //    group2: "param4|param5"
+               // }
+               $.each( params, function ( paramName, paramValue ) {
+                       var itemOrGroup = model.parameterMap[ paramName ];
+
+                       if ( itemOrGroup instanceof mw.rcfilters.dm.FilterItem 
) {
+                               groupMap[ itemOrGroup.getGroupName() ] = 
groupMap[ itemOrGroup.getGroupName() ] || {};
+                               groupMap[ itemOrGroup.getGroupName() ][ 
itemOrGroup.getParamName() ] = paramValue;
+                       } else if ( itemOrGroup instanceof 
mw.rcfilters.dm.FilterGroup ) {
+                               // This parameter represents a group (values 
are the filters)
+                               // this is equivalent to checking if the group 
is 'string_options'
+                               groupMap[ itemOrGroup.getName() ] = groupMap[ 
itemOrGroup.getName() ] || {};
+                               groupMap[ itemOrGroup.getName() ] = paramValue;
+                       }
+               } );
+
+               // Go over all groups, so we make sure we get the complete 
output
+               // even if the parameters don't include a certain group
+               $.each( this.groups, function ( groupName, groupModel ) {
+                       result = $.extend( true, {}, result, 
groupModel.getFilterRepresentation( groupMap[ groupName ] ) );
+               } );
+
+               return result;
+       };
+
+       /**
         * Get the highlight parameters based on current filter configuration
         *
         * @return {object} Object where keys are "<filter name>_color" and 
values
@@ -527,33 +524,11 @@
         * @return {string[]} Array of valid values
         */
        mw.rcfilters.dm.FiltersViewModel.prototype.sanitizeStringOptionGroup = 
function ( groupName, valueArray ) {
-               var result = [],
-                       validNames = this.getGroupFilters( groupName ).map( 
function ( filterItem ) {
-                               return filterItem.getParamName();
-                       } );
-
-               if ( valueArray.indexOf( 'all' ) > -1 ) {
-                       // If anywhere in the values there's 'all', we
-                       // treat it as if only 'all' was selected.
-                       // Example: param=valid1,valid2,all
-                       // Result: param=all
-                       return [ 'all' ];
-               }
-
-               // Get rid of any dupe and invalid parameter, only output
-               // valid ones
-               // Example: param=valid1,valid2,invalid1,valid1
-               // Result: param=valid1,valid2
-               valueArray.forEach( function ( value ) {
-                       if (
-                               validNames.indexOf( value ) > -1 &&
-                               result.indexOf( value ) === -1
-                       ) {
-                               result.push( value );
-                       }
+               var validNames = this.getGroupFilters( groupName ).map( 
function ( filterItem ) {
+                       return filterItem.getParamName();
                } );
 
-               return result;
+               return mw.rcfilters.utils.normalizeParamOptions( valueArray, 
validNames );
        };
 
        /**
@@ -580,100 +555,13 @@
                if ( this.defaultFiltersEmpty !== null ) {
                        // We only need to do this test once,
                        // because defaults are set once per session
-                       defaultFilters = this.getFiltersFromParameters();
+                       defaultFilters = this.getFiltersFromParameters( 
this.getDefaultParams() );
                        this.defaultFiltersEmpty = Object.keys( defaultFilters 
).every( function ( filterName ) {
                                return !defaultFilters[ filterName ];
                        } );
                }
 
                return this.defaultFiltersEmpty;
-       };
-
-       /**
-        * This is the opposite of the #getParametersFromFilters method; this 
goes over
-        * the given parameters and translates into a selected/unselected value 
in the filters.
-        *
-        * @param {Object} params Parameters query object
-        * @return {Object} Filter state object
-        */
-       mw.rcfilters.dm.FiltersViewModel.prototype.getFiltersFromParameters = 
function ( params ) {
-               var i,
-                       groupMap = {},
-                       model = this,
-                       base = this.getDefaultParams(),
-                       result = {};
-
-               params = $.extend( {}, base, params );
-
-               // Go over the given parameters
-               $.each( params, function ( paramName, paramValue ) {
-                       var itemOrGroup = model.parameterMap[ paramName ];
-
-                       if ( itemOrGroup instanceof mw.rcfilters.dm.FilterItem 
) {
-                               // Mark the group if it has any items that are 
selected
-                               groupMap[ itemOrGroup.getGroupName() ] = 
groupMap[ itemOrGroup.getGroupName() ] || {};
-                               groupMap[ itemOrGroup.getGroupName() 
].hasSelected = (
-                                       groupMap[ itemOrGroup.getGroupName() 
].hasSelected ||
-                                       !!Number( paramValue )
-                               );
-
-                               // Add filters
-                               groupMap[ itemOrGroup.getGroupName() ].filters 
= groupMap[ itemOrGroup.getGroupName() ].filters || [];
-                               groupMap[ itemOrGroup.getGroupName() 
].filters.push( itemOrGroup );
-                       } else if ( itemOrGroup instanceof 
mw.rcfilters.dm.FilterGroup ) {
-                               groupMap[ itemOrGroup.getName() ] = groupMap[ 
itemOrGroup.getName() ] || {};
-                               // This parameter represents a group (values 
are the filters)
-                               // this is equivalent to checking if the group 
is 'string_options'
-                               groupMap[ itemOrGroup.getName() ].filters = 
itemOrGroup.getItems();
-                       }
-               } );
-
-               // Now that we know the groups' selection states, we need to go 
over
-               // the filters in the groups and mark their selected states 
appropriately
-               $.each( groupMap, function ( group, data ) {
-                       var paramValues, filterItem,
-                               allItemsInGroup = data.filters;
-
-                       if ( model.groups[ group ].getType() === 
'send_unselected_if_any' ) {
-                               for ( i = 0; i < allItemsInGroup.length; i++ ) {
-                                       filterItem = allItemsInGroup[ i ];
-
-                                       result[ filterItem.getName() ] = 
groupMap[ filterItem.getGroupName() ].hasSelected ?
-                                               // Flip the definition between 
the parameter
-                                               // state and the filter state
-                                               // This is what the 
'toggleSelected' value of the filter is
-                                               !Number( params[ 
filterItem.getParamName() ] ) :
-                                               // Otherwise, there are no 
selected items in the
-                                               // group, which means the state 
is false
-                                               false;
-                               }
-                       } else if ( model.groups[ group ].getType() === 
'string_options' ) {
-                               paramValues = model.sanitizeStringOptionGroup(
-                                       group,
-                                       params[ group ].split(
-                                               model.groups[ group 
].getSeparator()
-                                       )
-                               );
-
-                               for ( i = 0; i < allItemsInGroup.length; i++ ) {
-                                       filterItem = allItemsInGroup[ i ];
-
-                                       result[ filterItem.getName() ] = (
-                                                       // If it is the word 
'all'
-                                                       paramValues.length === 
1 && paramValues[ 0 ] === 'all' ||
-                                                       // All values are 
written
-                                                       paramValues.length === 
model.groups[ group ].getItemCount()
-                                               ) ?
-                                               // All true (either because all 
values are written or the term 'all' is written)
-                                               // is the same as all filters 
set to true
-                                               true :
-                                               // Otherwise, the filter is 
selected only if it appears in the parameter values
-                                               paramValues.indexOf( 
filterItem.getParamName() ) > -1;
-                               }
-                       }
-               } );
-
-               return result;
        };
 
        /**
diff --git a/resources/src/mediawiki.rcfilters/mw.rcfilters.js 
b/resources/src/mediawiki.rcfilters/mw.rcfilters.js
index 3ddb5a0..8cea27e 100644
--- a/resources/src/mediawiki.rcfilters/mw.rcfilters.js
+++ b/resources/src/mediawiki.rcfilters/mw.rcfilters.js
@@ -1,3 +1,45 @@
 ( function ( mw ) {
-       mw.rcfilters = { dm: {}, ui: {} };
+       mw.rcfilters = {
+               dm: {},
+               ui: {},
+               utils: {
+                       addArrayElementsUnique: function ( arr, elements ) {
+                               elements = Array.isArray( elements ) ? elements 
: [ elements ];
+
+                               elements.forEach( function ( element ) {
+                                       if ( arr.indexOf( element ) === -1 ) {
+                                               arr.push( element );
+                                       }
+                               } );
+
+                               return arr;
+                       },
+                       normalizeParamOptions: function ( givenOptions, 
legalOptions ) {
+                               var result = [];
+
+                               if ( givenOptions.indexOf( 'all' ) > -1 ) {
+                                       // If anywhere in the values there's 
'all', we
+                                       // treat it as if only 'all' was 
selected.
+                                       // Example: param=valid1,valid2,all
+                                       // Result: param=all
+                                       return [ 'all' ];
+                               }
+
+                               // Get rid of any dupe and invalid parameter, 
only output
+                               // valid ones
+                               // Example: param=valid1,valid2,invalid1,valid1
+                               // Result: param=valid1,valid2
+                               givenOptions.forEach( function ( value ) {
+                                       if (
+                                               legalOptions.indexOf( value ) > 
-1 &&
+                                               result.indexOf( value ) === -1
+                                       ) {
+                                               result.push( value );
+                                       }
+                               } );
+
+                               return result;
+                       }
+               }
+       };
 }( mediaWiki ) );
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 bc266fb..c61c288 100644
--- 
a/tests/qunit/suites/resources/mediawiki.rcfilters/dm.FiltersViewModel.test.js
+++ 
b/tests/qunit/suites/resources/mediawiki.rcfilters/dm.FiltersViewModel.test.js
@@ -109,6 +109,97 @@
                );
        } );
 
+       QUnit.test( 'Default filters', function ( assert ) {
+               var definition = [ {
+                               name: 'group1',
+                               title: 'Group 1',
+                               type: 'send_unselected_if_any',
+                               filters: [
+                                       {
+                                               name: 'hidefilter1',
+                                               label: 'Show filter 1',
+                                               description: 'Description of 
Filter 1 in Group 1',
+                                               default: true
+                                       },
+                                       {
+                                               name: 'hidefilter2',
+                                               label: 'Show filter 2',
+                                               description: 'Description of 
Filter 2 in Group 1'
+                                       },
+                                       {
+                                               name: 'hidefilter3',
+                                               label: 'Show filter 3',
+                                               description: 'Description of 
Filter 3 in Group 1',
+                                               default: true
+                                       }
+                               ]
+                       }, {
+                               name: 'group2',
+                               title: 'Group 2',
+                               type: 'send_unselected_if_any',
+                               filters: [
+                                       {
+                                               name: 'hidefilter4',
+                                               label: 'Show filter 4',
+                                               description: 'Description of 
Filter 1 in Group 2'
+                                       },
+                                       {
+                                               name: 'hidefilter5',
+                                               label: 'Show filter 5',
+                                               description: 'Description of 
Filter 2 in Group 2',
+                                               default: true
+                                       },
+                                       {
+                                               name: 'hidefilter6',
+                                               label: 'Show filter 6',
+                                               description: 'Description of 
Filter 3 in Group 2'
+                                       }
+                               ]
+                       }, {
+
+                               name: 'group3',
+                               title: 'Group 3',
+                               type: 'string_options',
+                               separator: ',',
+                               default: 'filter8',
+                               filters: [
+                                       {
+                                               name: 'filter7',
+                                               label: 'Group 3: Filter 1',
+                                               description: 'Description of 
Filter 1 in Group 3'
+                                       },
+                                       {
+                                               name: 'filter8',
+                                               label: 'Group 3: Filter 2',
+                                               description: 'Description of 
Filter 2 in Group 3'
+                                       },
+                                       {
+                                               name: 'filter9',
+                                               label: 'Group 3: Filter 3',
+                                               description: 'Description of 
Filter 3 in Group 3'
+                                       }
+                               ]
+                       } ],
+                       model = new mw.rcfilters.dm.FiltersViewModel();
+
+               model.initializeFilters( definition );
+
+               // Empty query = only default values
+               assert.deepEqual(
+                       model.getDefaultParams(),
+                       {
+                               hidefilter1: 1,
+                               hidefilter2: 0,
+                               hidefilter3: 1,
+                               hidefilter4: 0,
+                               hidefilter5: 1,
+                               hidefilter6: 0,
+                               group3: 'filter8'
+                       },
+                       'Default parameters are stored properly per filter and 
group'
+               );
+       } );
+
        QUnit.test( 'Finding matching filters', function ( assert ) {
                var matches,
                        definition = [ {
@@ -608,17 +699,15 @@
                                        }
                                ]
                        } ],
-                       defaultFilterRepresentation = {
-                               // Group 1 and 2, "send_unselected_if_any", the 
values of the filters are "flipped" from the values of the parameters
+                       baseFilterRepresentation = {
                                group1__hidefilter1: false,
-                               group1__hidefilter2: true,
+                               group1__hidefilter2: false,
                                group1__hidefilter3: false,
-                               group2__hidefilter4: true,
+                               group2__hidefilter4: false,
                                group2__hidefilter5: false,
-                               group2__hidefilter6: true,
-                               // Group 3, "string_options", default values 
correspond to parameters and filters
+                               group2__hidefilter6: false,
                                group3__filter7: false,
-                               group3__filter8: true,
+                               group3__filter8: false,
                                group3__filter9: false
                        },
                        model = new mw.rcfilters.dm.FiltersViewModel();
@@ -628,18 +717,18 @@
                // Empty query = only default values
                assert.deepEqual(
                        model.getFiltersFromParameters( {} ),
-                       defaultFilterRepresentation,
-                       'Empty parameter query results in filters in initial 
default state'
+                       baseFilterRepresentation,
+                       'Empty parameter query results in an object 
representing all filters set to false'
                );
 
                assert.deepEqual(
                        model.getFiltersFromParameters( {
                                hidefilter2: '1'
                        } ),
-                       $.extend( {}, defaultFilterRepresentation, {
-                               group1__hidefilter1: false, // The text is 
"show filter 1"
+                       $.extend( {}, baseFilterRepresentation, {
+                               group1__hidefilter1: true, // The text is "show 
filter 1"
                                group1__hidefilter2: false, // The text is 
"show filter 2"
-                               group1__hidefilter3: false // The text is "show 
filter 3"
+                               group1__hidefilter3: true // The text is "show 
filter 3"
                        } ),
                        'One truthy parameter in a group whose other parameters 
are true by default makes the rest of the filters in the group false 
(unchecked)'
                );
@@ -650,7 +739,7 @@
                                hidefilter2: '1',
                                hidefilter3: '1'
                        } ),
-                       $.extend( {}, defaultFilterRepresentation, {
+                       $.extend( {}, baseFilterRepresentation, {
                                group1__hidefilter1: false, // The text is 
"show filter 1"
                                group1__hidefilter2: false, // The text is 
"show filter 2"
                                group1__hidefilter3: false // The text is "show 
filter 3"
@@ -680,11 +769,11 @@
                );
 
                // The result here is ignoring the first toggleFiltersSelected 
call
-               // We should receive default values + hidefilter6 as false
                assert.deepEqual(
                        model.getSelectedState(),
-                       $.extend( {}, defaultFilterRepresentation, {
-                               group2__hidefilter5: false,
+                       $.extend( {}, baseFilterRepresentation, {
+                               group2__hidefilter4: true,
+                               group2__hidefilter5: true,
                                group2__hidefilter6: false
                        } ),
                        'getFiltersFromParameters does not care about previous 
or existing state.'
@@ -696,31 +785,12 @@
 
                model.toggleFiltersSelected(
                        model.getFiltersFromParameters( {
-                               hidefilter1: '0'
-                       } )
-               );
-               model.toggleFiltersSelected(
-                       model.getFiltersFromParameters( {
-                               hidefilter1: '1'
-                       } )
-               );
-
-               // Simulates minor edits being hidden in preferences, then 
unhidden via URL
-               // override.
-               assert.deepEqual(
-                       model.getSelectedState(),
-                       defaultFilterRepresentation,
-                       'After checking and then unchecking a 
\'send_unselected_if_any\' filter (without touching other filters in that 
group), results are default'
-               );
-
-               model.toggleFiltersSelected(
-                       model.getFiltersFromParameters( {
                                group3: 'filter7'
                        } )
                );
                assert.deepEqual(
                        model.getSelectedState(),
-                       $.extend( {}, defaultFilterRepresentation, {
+                       $.extend( {}, baseFilterRepresentation, {
                                group3__filter7: true,
                                group3__filter8: false,
                                group3__filter9: false
@@ -735,7 +805,7 @@
                );
                assert.deepEqual(
                        model.getSelectedState(),
-                       $.extend( {}, defaultFilterRepresentation, {
+                       $.extend( {}, baseFilterRepresentation, {
                                group3__filter7: true,
                                group3__filter8: true,
                                group3__filter9: false
@@ -750,7 +820,7 @@
                );
                assert.deepEqual(
                        model.getSelectedState(),
-                       $.extend( {}, defaultFilterRepresentation, {
+                       $.extend( {}, baseFilterRepresentation, {
                                group3__filter7: true,
                                group3__filter8: true,
                                group3__filter9: true
@@ -765,7 +835,7 @@
                );
                assert.deepEqual(
                        model.getSelectedState(),
-                       $.extend( {}, defaultFilterRepresentation, {
+                       $.extend( {}, baseFilterRepresentation, {
                                group3__filter7: true,
                                group3__filter8: true,
                                group3__filter9: true
@@ -780,7 +850,7 @@
                );
                assert.deepEqual(
                        model.getSelectedState(),
-                       $.extend( {}, defaultFilterRepresentation, {
+                       $.extend( {}, baseFilterRepresentation, {
                                group3__filter7: true,
                                group3__filter8: false,
                                group3__filter9: true

-- 
To view, visit https://gerrit.wikimedia.org/r/352984
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I9c333719e89088d96d044d72ddb9c39feeeb68ca
Gerrit-PatchSet: 5
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Mooeypoo <[email protected]>
Gerrit-Reviewer: Sbisson <[email protected]>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to