From d8101d8ba148fb2a9e2d0fcb3373417623dab587 Mon Sep 17 00:00:00 2001
From: Piotr Miazga <piotr@polishdeveloper.pl>
Date: Tue, 4 Jul 2017 20:30:45 +0200
Subject: [PATCH] Pass old user options in PreferencesFormPreSave hook

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                             |  1 +
 includes/Preferences.php                   |  7 ++-
 tests/phpunit/includes/PreferencesTest.php | 72 ++++++++++++++++++++++
 3 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/docs/hooks.txt b/docs/hooks.txt
index 62e5bdb217..fcbca0f6a3 100644
--- a/docs/hooks.txt
+++ b/docs/hooks.txt
@@ -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.
diff --git a/includes/Preferences.php b/includes/Preferences.php
index 40176197b5..008963b5a7 100644
--- a/includes/Preferences.php
+++ b/includes/Preferences.php
@@ -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 ] );
diff --git a/tests/phpunit/includes/PreferencesTest.php b/tests/phpunit/includes/PreferencesTest.php
index 90b6396e28..ada516d554 100644
--- a/tests/phpunit/includes/PreferencesTest.php
+++ b/tests/phpunit/includes/PreferencesTest.php
@@ -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 = [];
-- 
2.20.1