From f25ad2e404ab70c2b62ebc43212293a6d6ac014f Mon Sep 17 00:00:00 2001
From: Bjoern Schiessle <bjoern@schiessle.org>
Date: Fri, 2 Dec 2016 16:54:17 +0100
Subject: [PATCH] make sure that we only update the email address if it really
 changed

Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
---
 settings/Controller/UsersController.php       |  2 ++
 .../Controller/UsersControllerTest.php        | 33 +++++++++++++++----
 2 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/settings/Controller/UsersController.php b/settings/Controller/UsersController.php
index 20440e6d39..28b8d2648d 100644
--- a/settings/Controller/UsersController.php
+++ b/settings/Controller/UsersController.php
@@ -605,6 +605,7 @@ class UsersController extends Controller {
 		// keep the user back-end up-to-date with the latest display name and email
 		// address
 		$oldDisplayName = $user->getDisplayName();
+		$oldDisplayName = is_null($oldDisplayName) ? '' : $oldDisplayName;
 		if (isset($data[AccountManager::PROPERTY_DISPLAYNAME]['value'])
 			&& $oldDisplayName !== $data[AccountManager::PROPERTY_DISPLAYNAME]['value']
 		) {
@@ -615,6 +616,7 @@ class UsersController extends Controller {
 		}
 
 		$oldEmailAddress = $user->getEMailAddress();
+		$oldEmailAddress = is_null($oldEmailAddress) ? '' : $oldEmailAddress;
 		if (isset($data[AccountManager::PROPERTY_EMAIL]['value'])
 			&& $oldEmailAddress !== $data[AccountManager::PROPERTY_EMAIL]['value']
 		) {
diff --git a/tests/Settings/Controller/UsersControllerTest.php b/tests/Settings/Controller/UsersControllerTest.php
index 69082a7929..b85667740f 100644
--- a/tests/Settings/Controller/UsersControllerTest.php
+++ b/tests/Settings/Controller/UsersControllerTest.php
@@ -2107,20 +2107,22 @@ class UsersControllerTest extends \Test\TestCase {
 		$user->method('getEMailAddress')->willReturn($oldEmailAddress);
 		$user->method('canChangeDisplayName')->willReturn(true);
 
-		if ($data[AccountManager::PROPERTY_EMAIL]['value'] !== $oldEmailAddress) {
+		if ($data[AccountManager::PROPERTY_EMAIL]['value'] === $oldEmailAddress ||
+		$oldEmailAddress === null && $data[AccountManager::PROPERTY_EMAIL]['value'] === '') {
+			$user->expects($this->never())->method('setEMailAddress');
+		} else {
 			$user->expects($this->once())->method('setEMailAddress')
 				->with($data[AccountManager::PROPERTY_EMAIL]['value'])
 				->willReturn(true);
-		} else {
-			$user->expects($this->never())->method('setEMailAddress');
 		}
 
-		if ($data[AccountManager::PROPERTY_DISPLAYNAME]['value'] !== $oldDisplayName) {
+		if ($data[AccountManager::PROPERTY_DISPLAYNAME]['value'] === $oldDisplayName ||
+			$oldDisplayName === null && $data[AccountManager::PROPERTY_DISPLAYNAME]['value'] === '') {
+			$user->expects($this->never())->method('setDisplayName');
+		} else {
 			$user->expects($this->once())->method('setDisplayName')
 				->with($data[AccountManager::PROPERTY_DISPLAYNAME]['value'])
 				->willReturn(true);
-		} else {
-			$user->expects($this->never())->method('setDisplayName');
 		}
 
 		$this->accountManager->expects($this->once())->method('updateUser')
@@ -2162,7 +2164,23 @@ class UsersControllerTest extends \Test\TestCase {
 				],
 				'john@example.com',
 				'john New doe'
-			]
+			],
+			[
+				[
+					AccountManager::PROPERTY_EMAIL => ['value' => ''],
+					AccountManager::PROPERTY_DISPLAYNAME => ['value' => 'john doe'],
+				],
+				null,
+				'john New doe'
+			],
+			[
+				[
+					AccountManager::PROPERTY_EMAIL => ['value' => 'john@example.com'],
+					AccountManager::PROPERTY_DISPLAYNAME => ['value' => 'john doe'],
+				],
+				'john@example.com',
+				null
+			],
 
 		];
 	}
@@ -2274,6 +2292,7 @@ class UsersControllerTest extends \Test\TestCase {
 			->with(
 				$this->equalTo($mailAddress)
 			);
+		$user->method('getEMailAddress')->willReturn('oldEmailAddress');
 		$this->mailer
 			->expects($this->any())
 			->method('validateMailAddress')