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

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 6acc44d..4dc86f6 100644 (file)
--- 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 (