Pass old user options in PreferencesFormPreSave hook
authorPiotr Miazga <piotr@polishdeveloper.pl>
Tue, 4 Jul 2017 18:30:45 +0000 (20:30 +0200)
committerPiotr Miazga <piotr@polishdeveloper.pl>
Wed, 5 Jul 2017 15:12:12 +0000 (17:12 +0200)
Changes:
 - added one argument to PreferencesFormPreSave hook,
   a $oldUserOptions array which contains set of all user
   options before save
 - updated documentation

Bug: T169365
Change-Id: I28003c5898d64031e1efb212cb0bec58ff44b958

docs/hooks.txt
includes/Preferences.php
tests/phpunit/includes/PreferencesTest.php

index 62e5bdb..fcbca0f 100644 (file)
@@ -2657,6 +2657,7 @@ $formData: array of user submitted data
 $form: PreferencesForm object, also a ContextSource
 $user: User object with preferences to be saved set
 &$result: boolean indicating success
+$oldUserOptions: array with user old options (before save)
 
 'PreferencesGetLegend': Override the text used for the <legend> of a
 preferences section.
index 4017619..008963b 100644 (file)
@@ -1485,6 +1485,8 @@ class Preferences {
                }
 
                if ( $user->isAllowed( 'editmyoptions' ) ) {
+                       $oldUserOptions = $user->getOptions();
+
                        foreach ( self::$saveBlacklist as $b ) {
                                unset( $formData[$b] );
                        }
@@ -1505,7 +1507,10 @@ class Preferences {
                                $user->setOption( $key, $value );
                        }
 
-                       Hooks::run( 'PreferencesFormPreSave', [ $formData, $form, $user, &$result ] );
+                       Hooks::run(
+                               'PreferencesFormPreSave',
+                               [ $formData, $form, $user, &$result, $oldUserOptions ]
+                       );
                }
 
                MediaWiki\Auth\AuthManager::callLegacyAuthPlugin( 'updateExternalDB', [ $user ] );
index 90b6396..ada516d 100644 (file)
@@ -77,6 +77,78 @@ class PreferencesTest extends MediaWikiTestCase {
                $this->assertEquals( 'mw-email-authenticated', $prefs['emailauthentication']['cssclass'] );
        }
 
+       /**
+        * Test that PreferencesFormPreSave hook has correct data:
+        *  - user Object is passed
+        *  - oldUserOptions contains previous user options (before save)
+        *  - formData and User object have set up new properties
+        *
+        * @see https://phabricator.wikimedia.org/T169365
+        * @covers Preferences::tryFormSubmit
+        */
+       public function testPreferencesFormPreSaveHookHasCorrectData() {
+               $oldOptions = [
+                       'test' => 'abc',
+                       'option' => 'old'
+               ];
+               $newOptions = [
+                       'test' => 'abc',
+                       'option' => 'new'
+               ];
+               $configMock = new HashConfig( [
+                       'HiddenPrefs' => []
+               ] );
+               $form = $this->getMockBuilder( PreferencesForm::class )
+                       ->disableOriginalConstructor()
+                       ->getMock();
+
+               $userMock = $this->getMockBuilder( User::class )
+                       ->disableOriginalConstructor()
+                       ->getMock();
+               $userMock->method( 'getOptions' )
+                       ->willReturn( $oldOptions );
+               $userMock->method( 'isAllowedAny' )
+                       ->willReturn( true );
+               $userMock->method( 'isAllowed' )
+                       ->willReturn( true );
+
+               $userMock->expects( $this->exactly( 2 ) )
+                       ->method( 'setOption' )
+                       ->withConsecutive(
+                               [ $this->equalTo( 'test' ), $this->equalTo( $newOptions[ 'test' ] ) ],
+                               [ $this->equalTo( 'option' ), $this->equalTo( $newOptions[ 'option' ] ) ]
+                       );
+
+               $form->expects( $this->any() )
+                       ->method( 'getModifiedUser' )
+                       ->willReturn( $userMock );
+
+               $form->expects( $this->any() )
+                       ->method( 'getContext' )
+                       ->willReturn( $this->context );
+
+               $form->expects( $this->any() )
+                       ->method( 'getConfig' )
+                       ->willReturn( $configMock );
+
+               $this->setTemporaryHook( 'PreferencesFormPreSave', function(
+                       $formData, $form, $user, &$result, $oldUserOptions )
+                       use ( $newOptions, $oldOptions, $userMock ) {
+
+                       $this->assertSame( $userMock, $user );
+                       foreach ( $newOptions as $option => $value ) {
+                               $this->assertSame( $value, $formData[ $option ] );
+                       }
+                       foreach ( $oldOptions as $option => $value ) {
+                               $this->assertSame( $value, $oldUserOptions[ $option ] );
+                       }
+                       $this->assertEquals( true, $result );
+               }
+               );
+
+               Preferences::tryFormSubmit( $newOptions, $form );
+       }
+
        /** Helper */
        protected function prefsFor( $user_key ) {
                $preferences = [];