From 13f0f58f76b372bca215f03d5c6c006d0fee9ec8 Mon Sep 17 00:00:00 2001
From: Brad Jorsch <bjorsch@wikimedia.org>
Date: Sun, 2 Dec 2012 22:48:57 -0500
Subject: [PATCH] (bug 42639) Fix API action=options for multiselect prefs

Preferences options using HTMLForm's "multiselect" type are stored in
the user preferences table as one key with a boolean value for each
option in the multiselect. The validation code added in change I98df55f2
does not take this into account, and therefore considers all of these
option keys invalid.

This changeset fixes that, and adds a unit test to verify correct
behavior.

Change-Id: I137c74a6045c7b39e2119a8edde2705738879bc9
---
 includes/api/ApiOptions.php                   | 29 +++++++++--
 tests/phpunit/includes/api/ApiOptionsTest.php | 48 +++++++++++++++++++
 2 files changed, 74 insertions(+), 3 deletions(-)

diff --git a/includes/api/ApiOptions.php b/includes/api/ApiOptions.php
index 265c2ccb67..01a376073f 100644
--- a/includes/api/ApiOptions.php
+++ b/includes/api/ApiOptions.php
@@ -74,13 +74,36 @@ class ApiOptions extends ApiBase {
 		}
 
 		$prefs = Preferences::getPreferences( $user, $this->getContext() );
+
+		// Multiselect options are stored in the database with one key per
+		// option, each having a boolean value. Extract those keys.
+		$multiselectOptions = array();
+		foreach ( $prefs as $name => $info ) {
+			if ( ( isset( $info['type'] ) && $info['type'] == 'multiselect' ) ||
+					( isset( $info['class'] ) && $info['class'] == 'HTMLMultiSelectField' ) ) {
+				$options = HTMLFormField::flattenOptions( $info['options'] );
+				$prefix = isset( $info['prefix'] ) ? $info['prefix'] : $name;
+
+				foreach ( $options as $value ) {
+					$multiselectOptions["$prefix$value"] = true;
+				}
+
+				unset( $prefs[$name] );
+			}
+		}
+
 		foreach ( $changes as $key => $value ) {
-			if ( !isset( $prefs[$key] ) ) {
+			if ( isset( $prefs[$key] ) ) {
+				$field = HTMLForm::loadInputFromParameters( $key, $prefs[$key] );
+				$validation = $field->validate( $value, $user->getOptions() );
+			} elseif( isset( $multiselectOptions[$key] ) ) {
+				// A key for a multiselect option.
+				$validation = true;
+				$value = (bool)$value;
+			} else {
 				$this->setWarning( "Not a valid preference: $key" );
 				continue;
 			}
-			$field = HTMLForm::loadInputFromParameters( $key, $prefs[$key] );
-			$validation = $field->validate( $value, $user->getOptions() );
 			if ( $validation === true ) {
 				$user->setOption( $key, $value );
 				$changed = true;
diff --git a/tests/phpunit/includes/api/ApiOptionsTest.php b/tests/phpunit/includes/api/ApiOptionsTest.php
index ac18afe7aa..b30b356e46 100644
--- a/tests/phpunit/includes/api/ApiOptionsTest.php
+++ b/tests/phpunit/includes/api/ApiOptionsTest.php
@@ -63,6 +63,22 @@ class ApiOptionsTest extends MediaWikiLangTestCase {
 			);
 		}
 
+		$preferences['testmultiselect'] = array(
+			'type' => 'multiselect',
+			'options' => array(
+				'Test' => array(
+					'<span dir="auto">Some HTML here for option 1</span>' => 'opt1',
+					'<span dir="auto">Some HTML here for option 2</span>' => 'opt2',
+					'<span dir="auto">Some HTML here for option 3</span>' => 'opt3',
+					'<span dir="auto">Some HTML here for option 4</span>' => 'opt4',
+				),
+			),
+			'section' => 'test',
+			'label' => '&#160;',
+			'prefix' => 'testmultiselect-',
+			'default' => array(),
+		);
+
 		return true;
 	}
 
@@ -262,4 +278,36 @@ class ApiOptionsTest extends MediaWikiLangTestCase {
 
 		$this->assertEquals( self::$Success, $response );
 	}
+
+	public function testMultiSelect() {
+		$this->mUserMock->expects( $this->never() )
+			->method( 'resetOptions' );
+
+		$this->mUserMock->expects( $this->at( 1 ) )
+			->method( 'setOption' )
+			->with( $this->equalTo( 'testmultiselect-opt1' ), $this->equalTo( true ) );
+
+		$this->mUserMock->expects( $this->at( 2 ) )
+			->method( 'setOption' )
+			->with( $this->equalTo( 'testmultiselect-opt2' ), $this->equalTo( false ) );
+
+		$this->mUserMock->expects( $this->at( 3 ) )
+			->method( 'setOption' )
+			->with( $this->equalTo( 'testmultiselect-opt3' ), $this->equalTo( false ) );
+
+		$this->mUserMock->expects( $this->at( 4 ) )
+			->method( 'setOption' )
+			->with( $this->equalTo( 'testmultiselect-opt4' ), $this->equalTo( false ) );
+
+		$this->mUserMock->expects( $this->once() )
+			->method( 'saveSettings' );
+
+		$request = $this->getSampleRequest( array(
+			'change' => 'testmultiselect-opt1=1|testmultiselect-opt2|testmultiselect-opt3=|testmultiselect-opt4=0'
+		) );
+
+		$response = $this->executeQuery( $request );
+
+		$this->assertEquals( self::$Success, $response );
+	}
 }
-- 
2.20.1