(bug 42638) Fix API action=options&reset=1 & unit tests

Change I98df55f2 broke action=options&reset=1, causing it to return an
error "No changes were requested" rather than resetting the options as
it should. Unfortunately, that change also broke the unit test that
would have caught this regression.

This changeset fixes the bug and the unit tests.

Change-Id: I7fe63640d54efab4572538e9d08f5b75c61243a4

diff --git a/includes/api/ApiOptions.php b/includes/api/ApiOptions.php
index f21bbc0..265c2cc 100644 (file)
--- a/includes/api/ApiOptions.php
+++ b/includes/api/ApiOptions.php
@@ -69,7 +69,7 @@ class ApiOptions extends ApiBase {
                        $newValue = isset( $params['optionvalue'] ) ? $params['optionvalue'] : null;
                        $changes[$params['optionname']] = $newValue;
                }
-               if ( !count( $changes ) ) {
+               if ( !$changed && !count( $changes ) ) {
                        $this->dieUsage( 'No changes were requested', 'nochanges' );
                }
 

diff --git a/tests/phpunit/includes/api/ApiOptionsTest.php b/tests/phpunit/includes/api/ApiOptionsTest.php
index 5469ccf..ac18afe 100644 (file)
--- a/tests/phpunit/includes/api/ApiOptionsTest.php
+++ b/tests/phpunit/includes/api/ApiOptionsTest.php
@@ -2,11 +2,14 @@
 
 /**
  * @group API
+ * @group Database
  */
 class ApiOptionsTest extends MediaWikiLangTestCase {
 
        private $mTested, $mUserMock, $mContext, $mSession;
 
+       private $mOldGetPreferencesHooks = false;
+
        private static $Success = array( 'options' => 'success' );
 
        protected function setUp() {
@@ -16,8 +19,13 @@ class ApiOptionsTest extends MediaWikiLangTestCase {
                        ->disableOriginalConstructor()
                        ->getMock();
 
+               // Set up groups
+               $this->mUserMock->expects( $this->any() )
+                       ->method( 'getEffectiveGroups' )->will( $this->returnValue( array( '*', 'user')) );
+
                // Create a new context
                $this->mContext = new DerivativeContext( new RequestContext() );
+               $this->mContext->getContext()->setTitle( Title::newFromText( 'Test' ) );
                $this->mContext->setUser( $this->mUserMock );
 
                $main = new ApiMain( $this->mContext );
@@ -26,6 +34,36 @@ class ApiOptionsTest extends MediaWikiLangTestCase {
                $this->mSession = array();
 
                $this->mTested = new ApiOptions( $main, 'options' );
+
+               global $wgHooks;
+               if ( !isset( $wgHooks['GetPreferences'] ) ) {
+                       $wgHooks['GetPreferences'] = array();
+               }
+               $this->mOldGetPreferencesHooks = $wgHooks['GetPreferences'];
+               $wgHooks['GetPreferences'][] = array( $this, 'hookGetPreferences' );
+       }
+
+       protected function tearDown() {
+               global $wgHooks;
+
+               if ( $this->mOldGetPreferencesHooks !== false ) {
+                       $wgHooks['GetPreferences'] = $this->mOldGetPreferencesHooks;
+                       $this->mOldGetPreferencesHooks = false;
+               }
+
+               parent::tearDown();
+       }
+
+       public function hookGetPreferences( $user, &$preferences ) {
+               foreach ( array( 'name', 'willBeNull', 'willBeEmpty', 'willBeHappy' ) as $k ) {
+                       $preferences[$k] = array(
+                               'type' => 'text',
+                               'section' => 'test',
+                               'label' => ' ',
+                       );
+               }
+
+               return true;
        }
 
        private function getSampleRequest( $custom = array() ) {
@@ -105,9 +143,6 @@ class ApiOptionsTest extends MediaWikiLangTestCase {
                $this->fail( "UsageException was not thrown" );
        }
 
-       /**
-        * @group Broken
-        */
        public function testReset() {
                $this->mUserMock->expects( $this->once() )
                        ->method( 'resetOptions' );
@@ -125,9 +160,6 @@ class ApiOptionsTest extends MediaWikiLangTestCase {
                $this->assertEquals( self::$Success, $response );
        }
 
-       /**
-        * @group Broken
-        */
        public function testOptionWithValue() {
                $this->mUserMock->expects( $this->never() )
                        ->method( 'resetOptions' );
@@ -146,9 +178,6 @@ class ApiOptionsTest extends MediaWikiLangTestCase {
                $this->assertEquals( self::$Success, $response );
        }
 
-       /**
-        * @group Broken
-        */
        public function testOptionResetValue() {
                $this->mUserMock->expects( $this->never() )
                        ->method( 'resetOptions' );
@@ -166,22 +195,28 @@ class ApiOptionsTest extends MediaWikiLangTestCase {
                $this->assertEquals( self::$Success, $response );
        }
 
-       /**
-        * @group Broken
-        */
        public function testChange() {
                $this->mUserMock->expects( $this->never() )
                        ->method( 'resetOptions' );
 
                $this->mUserMock->expects( $this->at( 1 ) )
+                       ->method( 'getOptions' );
+
+               $this->mUserMock->expects( $this->at( 2 ) )
                        ->method( 'setOption' )
                        ->with( $this->equalTo( 'willBeNull' ), $this->equalTo( null ) );
 
-               $this->mUserMock->expects( $this->at( 2 ) )
+               $this->mUserMock->expects( $this->at( 3 ) )
+                       ->method( 'getOptions' );
+
+               $this->mUserMock->expects( $this->at( 4 ) )
                        ->method( 'setOption' )
                        ->with( $this->equalTo( 'willBeEmpty' ), $this->equalTo( '' ) );
 
-               $this->mUserMock->expects( $this->at( 3 ) )
+               $this->mUserMock->expects( $this->at( 5 ) )
+                       ->method( 'getOptions' );
+
+               $this->mUserMock->expects( $this->at( 6 ) )
                        ->method( 'setOption' )
                        ->with( $this->equalTo( 'willBeHappy' ), $this->equalTo( 'Happy' ) );
 
@@ -195,18 +230,21 @@ class ApiOptionsTest extends MediaWikiLangTestCase {
                $this->assertEquals( self::$Success, $response );
        }
 
-       /**
-        * @group Broken
-        */
        public function testResetChangeOption() {
                $this->mUserMock->expects( $this->once() )
                        ->method( 'resetOptions' );
 
                $this->mUserMock->expects( $this->at( 2 ) )
+                       ->method( 'getOptions' );
+
+               $this->mUserMock->expects( $this->at( 3 ) )
                        ->method( 'setOption' )
                        ->with( $this->equalTo( 'willBeHappy' ), $this->equalTo( 'Happy' ) );
 
-               $this->mUserMock->expects( $this->at( 3 ) )
+               $this->mUserMock->expects( $this->at( 4 ) )
+                       ->method( 'getOptions' );
+
+               $this->mUserMock->expects( $this->at( 5 ) )
                        ->method( 'setOption' )
                        ->with( $this->equalTo( 'name' ), $this->equalTo( 'value' ) );