From 2d7370468dcdb5b669142a330ff0896e90b42e2e Mon Sep 17 00:00:00 2001
From: Stephane Bisson <sbisson@wikimedia.org>
Date: Tue, 2 May 2017 13:47:17 -0400
Subject: [PATCH] RC Filters: support multiple namespaces

Make the namespace filter support multiple values separated by ','.

Bug: T164132
Change-Id: I6a83eafef69ea416dd9ba8cecc524efbd85db438
---
 .../specialpage/ChangesListSpecialPage.php    | 37 ++++++------
 .../ChangesListSpecialPageTest.php            | 60 ++++++++++---------
 2 files changed, 53 insertions(+), 44 deletions(-)

diff --git a/includes/specialpage/ChangesListSpecialPage.php b/includes/specialpage/ChangesListSpecialPage.php
index bc241e06dc..47d07329c0 100644
--- a/includes/specialpage/ChangesListSpecialPage.php
+++ b/includes/specialpage/ChangesListSpecialPage.php
@@ -803,7 +803,7 @@ abstract class ChangesListSpecialPage extends SpecialPage {
 			}
 		}
 
-		$opts->add( 'namespace', '', FormOptions::INTNULL );
+		$opts->add( 'namespace', '', FormOptions::STRING );
 		$opts->add( 'invert', false );
 		$opts->add( 'associated', false );
 
@@ -996,25 +996,28 @@ abstract class ChangesListSpecialPage extends SpecialPage {
 		}
 
 		// Namespace filtering
-		if ( $opts['namespace'] !== '' ) {
-			$selectedNS = $dbr->addQuotes( $opts['namespace'] );
-			$operator = $opts['invert'] ? '!=' : '=';
-			$boolean = $opts['invert'] ? 'AND' : 'OR';
-
-			// Namespace association (T4429)
-			if ( !$opts['associated'] ) {
-				$condition = "rc_namespace $operator $selectedNS";
-			} else {
-				// Also add the associated namespace
-				$associatedNS = $dbr->addQuotes(
-					MWNamespace::getAssociated( $opts['namespace'] )
+		if ( $opts[ 'namespace' ] !== '' ) {
+			$namespaces = explode( ',', $opts[ 'namespace' ] );
+
+			if ( $opts[ 'associated' ] ) {
+				$associatedNamespaces = array_map(
+					function ( $ns ) {
+						return MWNamespace::getAssociated( $ns );
+					},
+					$namespaces
 				);
-				$condition = "(rc_namespace $operator $selectedNS "
-					. $boolean
-					. " rc_namespace $operator $associatedNS)";
+				$namespaces = array_unique( array_merge( $namespaces, $associatedNamespaces ) );
 			}
 
-			$conds[] = $condition;
+			if ( count( $namespaces ) === 1 ) {
+				$operator = $opts[ 'invert' ] ? '!=' : '=';
+				$value = $dbr->addQuotes( reset( $namespaces ) );
+			} else {
+				$operator = $opts[ 'invert' ] ? 'NOT IN' : 'IN';
+				sort( $namespaces );
+				$value = '(' . $dbr->makeList( $namespaces ) . ')';
+			}
+			$conds[] = "rc_namespace $operator $value";
 		}
 	}
 
diff --git a/tests/phpunit/includes/specialpage/ChangesListSpecialPageTest.php b/tests/phpunit/includes/specialpage/ChangesListSpecialPageTest.php
index b536c22f2e..704b45e9ea 100644
--- a/tests/phpunit/includes/specialpage/ChangesListSpecialPageTest.php
+++ b/tests/phpunit/includes/specialpage/ChangesListSpecialPageTest.php
@@ -109,7 +109,7 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase
 			[
 				'namespace' => NS_MAIN,
 			],
-			"rc conditions with no options (aka default setting)"
+			"rc conditions with one namespace"
 		);
 	}
 
@@ -126,50 +126,56 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase
 		);
 	}
 
-	/**
-	 * T4429
-	 * @dataProvider provideNamespacesAssociations
-	 */
-	public function testRcNsFilterAssociation( $ns1, $ns2 ) {
+	public function testRcNsFilterMultiple() {
 		$this->assertConditions(
 			[ # expected
-				"(rc_namespace = '$ns1' OR rc_namespace = '$ns2')",
+				"rc_namespace IN ('1','2','3')",
 			],
 			[
-				'namespace' => $ns1,
+				'namespace' => '1,2,3',
+			],
+			"rc conditions with multiple namespaces"
+		);
+	}
+
+	public function testRcNsFilterMultipleAssociated() {
+		$this->assertConditions(
+			[ # expected
+				"rc_namespace IN ('0','1','4','5','6','7')",
+			],
+			[
+				'namespace' => '1,4,7',
 				'associated' => 1,
 			],
-			"rc conditions with namespace inverted"
+			"rc conditions with multiple namespaces and associated"
 		);
 	}
 
-	/**
-	 * T4429
-	 * @dataProvider provideNamespacesAssociations
-	 */
-	public function testRcNsFilterAssociationWithInversion( $ns1, $ns2 ) {
+	public function testRcNsFilterMultipleAssociatedInvert() {
 		$this->assertConditions(
 			[ # expected
-				"(rc_namespace != '$ns1' AND rc_namespace != '$ns2')",
+				"rc_namespace NOT IN ('2','3','8','9')",
 			],
 			[
-				'namespace' => $ns1,
+				'namespace' => '2,3,9',
 				'associated' => 1,
-				'invert' => 1,
+				'invert' => 1
 			],
-			"rc conditions with namespace inverted"
+			"rc conditions with multiple namespaces, associated and inverted"
 		);
 	}
 
-	/**
-	 * Provides associated namespaces to test recent changes
-	 * namespaces association filtering.
-	 */
-	public static function provideNamespacesAssociations() {
-		return [ # (NS => Associated_NS)
-			[ NS_MAIN, NS_TALK ],
-			[ NS_TALK, NS_MAIN ],
-		];
+	public function testRcNsFilterMultipleInvert() {
+		$this->assertConditions(
+			[ # expected
+				"rc_namespace NOT IN ('1','2','3')",
+			],
+			[
+				'namespace' => '1,2,3',
+				'invert' => 1,
+			],
+			"rc conditions with multiple namespaces inverted"
+		);
 	}
 
 	public function testRcHidemyselfFilter() {
-- 
2.20.1