Merge "Watchlist: Fix form and preference overriding"

diff --git a/includes/specials/SpecialWatchlist.php b/includes/specials/SpecialWatchlist.php
index 5d7fa5d..4a17533 100644 (file)
--- a/includes/specials/SpecialWatchlist.php
+++ b/includes/specials/SpecialWatchlist.php
@@ -165,10 +165,6 @@ class SpecialWatchlist extends ChangesListSpecialPage {
 
                $opts->add( 'days', $user->getOption( 'watchlistdays' ), FormOptions::FLOAT );
                $opts->add( 'extended', $user->getBoolOption( 'extendwatchlist' ) );
-               if ( $this->getRequest()->getVal( 'action' ) == 'submit' ) {
-                       // The user has submitted the form, so we dont need the default values
-                       return $opts;
-               }
 
                return $opts;
        }
@@ -214,6 +210,26 @@ class SpecialWatchlist extends ChangesListSpecialPage {
                        }
                }
 
+               if ( $this->getRequest()->getVal( 'action' ) == 'submit' ) {
+                       $allBooleansFalse = [];
+
+                       // If the user submitted the form, start with a baseline of "all
+                       // booleans are false", then change the ones they checked.  This
+                       // means we ignore the defaults.
+
+                       // This is how we handle the fact that HTML forms don't submit
+                       // unchecked boxes.
+                       foreach ( $this->filterGroups as $filterGroup ) {
+                               if ( $filterGroup instanceof ChangesListBooleanFilterGroup ) {
+                                       foreach ( $filterGroup->getFilters() as $filter ) {
+                                               $allBooleansFalse[$filter->getName()] = false;
+                                       }
+                               }
+                       }
+
+                       $params = $params + $allBooleansFalse;
+               }
+
                // Not the prettiest way to achieve this… FormOptions internally depends on data sanitization
                // methods defined on WebRequest and removing this dependency would cause some code duplication.
                $request = new DerivativeRequest( $this->getRequest(), $params );

diff --git a/tests/phpunit/includes/specials/SpecialWatchlistTest.php b/tests/phpunit/includes/specials/SpecialWatchlistTest.php
index 6e702b6..66d35a5 100644 (file)
--- a/tests/phpunit/includes/specials/SpecialWatchlistTest.php
+++ b/tests/phpunit/includes/specials/SpecialWatchlistTest.php
@@ -8,6 +8,41 @@
  * @covers SpecialWatchlist
  */
 class SpecialWatchlistTest extends SpecialPageTestBase {
+       public function setUp() {
+               parent::setUp();
+
+               $this->setTemporaryHook(
+                       'ChangesListSpecialPageFilters',
+                       null
+               );
+
+               $this->setTemporaryHook(
+                       'SpecialWatchlistQuery',
+                       null
+               );
+
+               $this->setTemporaryHook(
+                       'ChangesListSpecialPageQuery',
+                       null
+               );
+
+               $this->setMwGlobals(
+                       'wgDefaultUserOptions',
+                       [
+                               'extendwatchlist' => 1,
+                               'watchlistdays' => 3.0,
+                               'watchlisthideanons' => 0,
+                               'watchlisthidebots' => 0,
+                               'watchlisthideliu' => 0,
+                               'watchlisthideminor' => 0,
+                               'watchlisthideown' => 0,
+                               'watchlisthidepatrolled' => 0,
+                               'watchlisthidecategorization' => 1,
+                               'watchlistreloadautomatically' => 0,
+                       ]
+               );
+
+       }
 
        /**
         * Returns a new instance of the special page under test.
@@ -29,4 +64,124 @@ class SpecialWatchlistTest extends SpecialPageTestBase {
                $this->assertContains( '(nowatchlist)', $html );
        }
 
+       /**
+        * @dataProvider provideFetchOptionsFromRequest
+        */
+       public function testFetchOptionsFromRequest( $expectedValues, $preferences, $inputParams ) {
+               $page = TestingAccessWrapper::newFromObject(
+                       $this->newSpecialPage()
+               );
+
+               $context = new DerivativeContext( $page->getContext() );
+
+               $fauxRequest = new FauxRequest( $inputParams, /* $wasPosted= */ false );
+               $user = $this->getTestUser()->getUser();
+
+               foreach ( $preferences as $key => $value ) {
+                       $user->setOption( $key, $value );
+               }
+
+               $context->setRequest( $fauxRequest );
+               $context->setUser( $user );
+               $page->setContext( $context );
+
+               $page->registerFilters();
+               $formOptions = $page->getDefaultOptions();
+               $page->fetchOptionsFromRequest( $formOptions );
+
+               $this->assertArrayEquals(
+                       $expectedValues,
+                       $formOptions->getAllValues(),
+                       /* $ordered= */ false,
+                       /* $named= */ true
+               );
+       }
+
+       public function provideFetchOptionsFromRequest() {
+               // $defaults and $allFalse are just to make the expected values below
+               // shorter by hiding the background.
+
+               $page = TestingAccessWrapper::newFromObject(
+                       $this->newSpecialPage()
+               );
+
+               $this->setTemporaryHook(
+                       'ChangesListSpecialPageFilters',
+                       null
+               );
+
+               $page->registerFilters();
+
+               // Does not consider $preferences, just wiki's defaults
+               $wikiDefaults = $page->getDefaultOptions()->getAllValues();
+
+               $allFalse = $wikiDefaults;
+
+               foreach ( $allFalse as $key => &$value ) {
+                       if ( $value === true ) {
+                               $value = false;
+                       }
+               }
+
+               // This is not exposed on the form (only in preferences) so it
+               // respects the preference.
+               $allFalse['extended'] = true;
+
+               return [
+                       [
+                               [
+                                       'hideminor' => true,
+                               ] + $wikiDefaults,
+                               [],
+                               [
+                                       'hideMinor' => 1,
+                               ],
+                       ],
+
+                       [
+                               [
+                                       // First two same as prefs
+                                       'hideminor' => true,
+                                       'hidebots' => false,
+
+                                       // Second two overriden
+                                       'hideanons' => false,
+                                       'hideliu' => true,
+                               ] + $wikiDefaults,
+                               [
+                                       'watchlisthideminor' => 1,
+                                       'watchlisthidebots' => 0,
+
+                                       'watchlisthideanons' => 1,
+                                       'watchlisthideliu' => 0,
+                               ],
+                               [
+                                       'hideanons' => 0,
+                                       'hideliu' => 1,
+                               ],
+                       ],
+
+                       // Defaults/preferences for form elements are entirely ignored for
+                       // action=submit and omitted elements become false
+                       [
+                               [
+                                       'hideminor' => false,
+                                       'hidebots' => true,
+                                       'hideanons' => false,
+                                       'hideliu' => true,
+                               ] + $allFalse,
+                               [
+                                       'watchlisthideminor' => 0,
+                                       'watchlisthidebots' => 1,
+                                       'watchlisthideanons' => 1,
+                                       'watchlisthideliu' => 0,
+                               ],
+                               [
+                                       'hidebots' => 1,
+                                       'hideliu' => 1,
+                                       'action' => 'submit',
+                               ],
+                       ],
+               ];
+       }
 }