(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

diff --git a/includes/api/ApiOptions.php b/includes/api/ApiOptions.php
index 265c2cc..01a3760 100644 (file)
--- 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 ac18afe..b30b356 100644 (file)
--- 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 );
+       }
 }