From 7f9eb8782d75eb85a53d711d1b77a778a296aa16 Mon Sep 17 00:00:00 2001
From: Stephane Bisson <sbisson@wikimedia.org>
Date: Wed, 29 Mar 2017 16:29:48 -0400
Subject: [PATCH] RC Filters: correctly read default value for 'string_options'
 filters

Bug: T160092
Change-Id: If39f1b280952ce1cef91092416c8454b896f3484
---
 .../dm/mw.rcfilters.dm.FiltersViewModel.js    | 19 ++++++++----------
 .../dm.FiltersViewModel.test.js               | 20 +++++++++----------
 2 files changed, 18 insertions(+), 21 deletions(-)

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 f04cc856e4..8fd15531ae 100644
--- a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FiltersViewModel.js
+++ b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FiltersViewModel.js
@@ -192,7 +192,7 @@
 	 * @param {Array} filters Filter group definition
 	 */
 	mw.rcfilters.dm.FiltersViewModel.prototype.initializeFilters = function ( filters ) {
-		var i, filterItem, selectedFilterNames, filterConflictResult, groupConflictResult, subsetNames,
+		var i, filterItem, filterConflictResult, groupConflictResult, subsetNames,
 			model = this,
 			items = [],
 			supersetMap = {},
@@ -283,7 +283,6 @@
 				groupConflictMap[ group ] = data.conflicts;
 			}
 
-			selectedFilterNames = [];
 			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 ) {
@@ -330,22 +329,20 @@
 					// Store the default parameter state
 					// For this group type, parameter values are direct
 					model.defaultParams[ data.filters[ i ].name ] = Number( !!data.filters[ i ].default );
-				} else if (
-					data.type === 'string_options' &&
-					data.filters[ i ].default
-				) {
-					selectedFilterNames.push( data.filters[ i ].name );
 				}
 
 				model.groups[ group ].addItems( filterItem );
 				items.push( filterItem );
 			}
 
-			if ( data.type === 'string_options' ) {
+			if ( data.type === 'string_options' && data.default ) {
 				// 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, selectedFilterNames ).join( model.groups[ group ].getSeparator() );
+				model.defaultParams[ group ] = model.sanitizeStringOptionGroup(
+					group,
+					data.default.split( model.groups[ group ].getSeparator() )
+				).join( model.groups[ group ].getSeparator() );
 			}
 		} );
 
@@ -655,8 +652,8 @@
 							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 false
-						false :
+						// 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;
 				}
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 405fdcf1de..27d38258ef 100644
--- a/tests/qunit/suites/resources/mediawiki.rcfilters/dm.FiltersViewModel.test.js
+++ b/tests/qunit/suites/resources/mediawiki.rcfilters/dm.FiltersViewModel.test.js
@@ -485,6 +485,7 @@
 				title: 'Group 3',
 				type: 'string_options',
 				separator: ',',
+				default: 'filter8',
 				filters: [
 					{
 						name: 'filter7',
@@ -494,8 +495,7 @@
 					{
 						name: 'filter8',
 						label: 'Group 3: Filter 2',
-						description: 'Description of Filter 2 in Group 3',
-						default: true
+						description: 'Description of Filter 2 in Group 3'
 					},
 					{
 						name: 'filter9',
@@ -647,11 +647,11 @@
 		assert.deepEqual(
 			model.getSelectedState(),
 			$.extend( {}, defaultFilterRepresentation, {
-				group3__filter7: false,
-				group3__filter8: false,
-				group3__filter9: false
+				group3__filter7: true,
+				group3__filter8: true,
+				group3__filter9: true
 			} ),
-			'A \'string_options\' parameter containing all values, results in all filters of the group as unchecked.'
+			'A \'string_options\' parameter containing all values, results in all filters of the group as checked.'
 		);
 
 		model.toggleFiltersSelected(
@@ -662,11 +662,11 @@
 		assert.deepEqual(
 			model.getSelectedState(),
 			$.extend( {}, defaultFilterRepresentation, {
-				group3__filter7: false,
-				group3__filter8: false,
-				group3__filter9: false
+				group3__filter7: true,
+				group3__filter8: true,
+				group3__filter9: true
 			} ),
-			'A \'string_options\' parameter containing the value \'all\', results in all filters of the group as unchecked.'
+			'A \'string_options\' parameter containing the value \'all\', results in all filters of the group as checked.'
 		);
 
 		model.toggleFiltersSelected(
-- 
2.20.1