From 4f6624451aa979a49840a10071cac75a1a5aae80 Mon Sep 17 00:00:00 2001
From: Moriel Schottlender <moriel@gmail.com>
Date: Thu, 17 Aug 2017 11:27:44 -0700
Subject: [PATCH] RCFilters: Fix validation for single_option groups

The single_option groups have a base definition that means they
always must have a value, and must never have more than one value.
The previous way we attempted to do that was confusing and missed
a case where after resetting filters, two values were selected before
one could be unselected, which then broke the behavior in the
entire group.

This fix reorganizes the validation when an item in the group is
selected or unselected to make sure the group retains its promised
behavior.

Bug: T173303
Change-Id: I5758ec324a26c0e5e6f5c473d206e818a1d22523
---
 .../dm/mw.rcfilters.dm.FilterGroup.js         | 61 ++++++++++---------
 1 file changed, 33 insertions(+), 28 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 6acc44dc5b..4dc86f6f65 100644
--- a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterGroup.js
+++ b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterGroup.js
@@ -222,34 +222,39 @@
 	mw.rcfilters.dm.FilterGroup.prototype.onFilterItemUpdate = function ( item ) {
 		// Update state
 		var changed = false,
-			active = this.areAnySelected();
-
-		if (
-			item.isSelected() &&
-			this.getType() === 'single_option' &&
-			this.currSelected &&
-			this.currSelected !== item
-		) {
-			this.currSelected.toggleSelected( false );
-		}
-
-		// For 'single_option' groups, check if we just unselected all
-		// items. This should never be the result. If we did unselect
-		// all (like resetting all filters to false) then this group
-		// must choose its default item or the first item in the group
-		if (
-			this.getType() === 'single_option' &&
-			!this.getItems().some( function ( filterItem ) {
-				return filterItem.isSelected();
-			} )
-		) {
-			// Single option means there must be a single option
-			// selected, so we have to either select the default
-			// or select the first option
-			this.currSelected = this.getItemByParamName( this.defaultParams[ this.getName() ] ) ||
-				this.getItems()[ 0 ];
-			this.currSelected.toggleSelected( true );
-			changed = true;
+			active = this.areAnySelected(),
+			model = this;
+
+		if ( this.getType() === 'single_option' ) {
+			// This group must have one item selected always
+			// and must never have more than one item selected at a time
+			if ( this.getSelectedItems().length === 0 ) {
+				// Nothing is selected anymore
+				// Select the default or the first item
+				this.currSelected = this.getItemByParamName( this.defaultParams[ this.getName() ] ) ||
+					this.getItems()[ 0 ];
+				this.currSelected.toggleSelected( true );
+				changed = true;
+			} else if ( this.getSelectedItems().length > 1 ) {
+				// There is more than one item selected
+				// This should only happen if the item given
+				// is the one that is selected, so unselect
+				// all items that is not it
+				this.getSelectedItems().forEach( function ( itemModel ) {
+					// Note that in case the given item is actually
+					// not selected, this loop will end up unselecting
+					// all items, which would trigger the case above
+					// when the last item is unselected anyways
+					var selected = itemModel.getName() === item.getName() &&
+						item.isSelected();
+
+					itemModel.toggleSelected( selected );
+					if ( selected ) {
+						model.currSelected = itemModel;
+					}
+				} );
+				changed = true;
+			}
 		}
 
 		if (
-- 
2.20.1