(bug 37963) Fixed loading process for user options.

The bug has actually already been fixed, so this
patch just removes extraneous function calls and code in
User::getOption() and User::setOption(). It also adds
unit tests for user options (including a test for the
case provided in the bug report).

Change-Id: Idd8af9cf1a26a4adbde3ca71dde64539ecd0a207

diff --git a/RELEASE-NOTES-1.21 b/RELEASE-NOTES-1.21
index 3357686..7111254 100644 (file)
--- a/RELEASE-NOTES-1.21
+++ b/RELEASE-NOTES-1.21
@@ -56,6 +56,7 @@ production.
   current revision.
 * (bug 41494) Honor $wgLogExceptionBacktrace when logging non-API exceptions
   caught during API execution.
+* (bug 37963) Fixed loading process for user options
 
 === API changes in 1.21 ===
 * prop=revisions can now report the contentmodel and contentformat, see docs/contenthandler.txt

diff --git a/includes/User.php b/includes/User.php
index 770b28e..1efdf6b 100644 (file)
--- a/includes/User.php
+++ b/includes/User.php
@@ -2215,13 +2215,6 @@ class User {
                global $wgHiddenPrefs;
                $this->loadOptions();
 
-               if ( is_null( $this->mOptions ) ) {
-                       if($defaultOverride != '') {
-                               return $defaultOverride;
-                       }
-                       $this->mOptions = User::getDefaultOptions();
-               }
-
                # We want 'disabled' preferences to always behave as the default value for
                # users, even if they have set the option explicitly in their settings (ie they
                # set it, and then it was disabled removing their ability to change it).  But
@@ -2297,15 +2290,11 @@ class User {
         * @param $val mixed New value to set
         */
        public function setOption( $oname, $val ) {
-               $this->load();
                $this->loadOptions();
 
                // Explicitly NULL values should refer to defaults
                if( is_null( $val ) ) {
-                       $defaultOption = self::getDefaultOption( $oname );
-                       if( !is_null( $defaultOption ) ) {
-                               $val = $defaultOption;
-                       }
+                       $val = self::getDefaultOption( $oname );
                }
 
                $this->mOptions[$oname] = $val;

diff --git a/tests/phpunit/includes/UserTest.php b/tests/phpunit/includes/UserTest.php
index 8c92f36..bf9468a 100644 (file)
--- a/tests/phpunit/includes/UserTest.php
+++ b/tests/phpunit/includes/UserTest.php
@@ -187,4 +187,31 @@ class UserTest extends MediaWikiTestCase {
                $user->clearInstanceCache();
                $this->assertEquals( 4, $user->getEditCount(), 'After increasing the edit count manually, the user edit count should be 4' );
        }
+
+       /**
+        * Test changing user options.
+        */
+       public function testOptions() {
+               $user = User::newFromName( 'UnitTestUser' );
+               $user->addToDatabase();
+
+               $user->setOption( 'someoption', 'test' );
+               $user->setOption( 'cols', 200 );
+               $user->saveSettings();
+
+               $user = User::newFromName( 'UnitTestUser' );
+               $this->assertEquals( 'test', $user->getOption( 'someoption' ) );
+               $this->assertEquals( 200, $user->getOption( 'cols' ) );
+       }
+
+       /**
+        * Bug 37963
+        * Make sure defaults are loaded when setOption is called.
+        */
+       public function testAnonOptions() {
+               global $wgDefaultUserOptions;
+               $this->user->setOption( 'someoption', 'test' );
+               $this->assertEquals( $wgDefaultUserOptions['cols'], $this->user->getOption( 'cols' ) );
+               $this->assertEquals( 'test', $this->user->getOption( 'someoption' ) );
+       }
 }