From: Matthew Flaschen <mflaschen@wikimedia.org>
Date: Fri, 17 Mar 2017 22:33:12 +0000 (-0400)
Subject: Watchlist: Fix form and preference overriding
X-Git-Tag: 1.31.0-rc.0~3766^2
X-Git-Url: https://git.cyclocoop.org/%7B%24www_url%7Dadmin/compta/banques/ajouter.php?a=commitdiff_plain;h=bc18aa06773bbb874540569aef7f530ad51013bf;p=lhc%2Fweb%2Fwiklou.git

Watchlist: Fix form and preference overriding

Fix a regression caused by aa063f4c5a19.  Restore the behavior
where:

* Without action=submit: Boolean preferences apply to the displayed form and its
  results, but they can be overriden by the query string.
* With action=submit: Only boolean parameters in the query string apply;
  preferences are not considered.

  (However, boolean preferences not on the form, i.e. 'extended', still
  always apply.)

This behavior is a consequence of how checkboxes work in HTML; only
checked boxes are sent to the server.

Bug: T160734
Change-Id: Ic050dd1445ade9449496bb051f04ca0a87b8b5d7
---

diff --git a/includes/specials/SpecialWatchlist.php b/includes/specials/SpecialWatchlist.php
index 5d7fa5dbcd..4a17533d6d 100644
--- 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 6e702b637c..66d35a5195 100644
--- 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',
+				],
+			],
+		];
+	}
 }