From e30271be4f1c7519a580948ed1d8e47830ff8960 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 29 Jan 2021 22:11:16 +0100 Subject: [PATCH] Respect additional user settings not covered by the controller MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "AccountManager::updateUser()" wipes previous user data with whichever user data is given (except for some adjustments, like resetting the verified status when needed). As the controller overrode the properties those properties would lose some of their attributes even if they are not affected by the changes made by the controller. Now the controller only modifies the attributes set ("value" and "scope") to prevent that. Note that with this change the controller no longer removes the "verified" status, but this is not a problem because, as mentioned, "AccountManager::updateUser()" resets them when needed (for example, when the value of the website property changes). This change is a previous step to fix overwritting properties with null values, and it will prevent the controller from making unexpected changes if more attributes are added in the future. Signed-off-by: Daniel Calviño Sánchez --- .../lib/Controller/UsersController.php | 20 ++++++++++++------- .../tests/Controller/UsersControllerTest.php | 6 ------ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/apps/settings/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php index 89840e6bcf..1aeb5f83c1 100644 --- a/apps/settings/lib/Controller/UsersController.php +++ b/apps/settings/lib/Controller/UsersController.php @@ -358,19 +358,25 @@ class UsersController extends Controller { } $user = $this->userSession->getUser(); $data = $this->accountManager->getUser($user); - $data[IAccountManager::PROPERTY_AVATAR] = ['scope' => $avatarScope]; + $data[IAccountManager::PROPERTY_AVATAR]['scope'] = $avatarScope; if ($this->config->getSystemValue('allow_user_to_change_display_name', true) !== false) { - $data[IAccountManager::PROPERTY_DISPLAYNAME] = ['value' => $displayname, 'scope' => $displaynameScope]; - $data[IAccountManager::PROPERTY_EMAIL] = ['value' => $email, 'scope' => $emailScope]; + $data[IAccountManager::PROPERTY_DISPLAYNAME]['value'] = $displayname; + $data[IAccountManager::PROPERTY_DISPLAYNAME]['scope'] = $displaynameScope; + $data[IAccountManager::PROPERTY_EMAIL]['value'] = $email; + $data[IAccountManager::PROPERTY_EMAIL]['scope'] = $emailScope; } if ($this->appManager->isEnabledForUser('federatedfilesharing')) { $federatedFileSharing = new \OCA\FederatedFileSharing\AppInfo\Application(); $shareProvider = $federatedFileSharing->getFederatedShareProvider(); if ($shareProvider->isLookupServerUploadEnabled()) { - $data[IAccountManager::PROPERTY_WEBSITE] = ['value' => $website, 'scope' => $websiteScope]; - $data[IAccountManager::PROPERTY_ADDRESS] = ['value' => $address, 'scope' => $addressScope]; - $data[IAccountManager::PROPERTY_PHONE] = ['value' => $phone, 'scope' => $phoneScope]; - $data[IAccountManager::PROPERTY_TWITTER] = ['value' => $twitter, 'scope' => $twitterScope]; + $data[IAccountManager::PROPERTY_WEBSITE]['value'] = $website; + $data[IAccountManager::PROPERTY_WEBSITE]['scope'] = $websiteScope; + $data[IAccountManager::PROPERTY_ADDRESS]['value'] = $address; + $data[IAccountManager::PROPERTY_ADDRESS]['scope'] = $addressScope; + $data[IAccountManager::PROPERTY_PHONE]['value'] = $phone; + $data[IAccountManager::PROPERTY_PHONE]['scope'] = $phoneScope; + $data[IAccountManager::PROPERTY_TWITTER]['value'] = $twitter; + $data[IAccountManager::PROPERTY_TWITTER]['scope'] = $twitterScope; } } try { diff --git a/apps/settings/tests/Controller/UsersControllerTest.php b/apps/settings/tests/Controller/UsersControllerTest.php index 83a620f151..0d4ca5fd3a 100644 --- a/apps/settings/tests/Controller/UsersControllerTest.php +++ b/apps/settings/tests/Controller/UsersControllerTest.php @@ -312,16 +312,12 @@ class UsersControllerTest extends \Test\TestCase { $expectedProperties[IAccountManager::PROPERTY_AVATAR]['scope'] = $avatarScope; $expectedProperties[IAccountManager::PROPERTY_PHONE]['value'] = $phone; $expectedProperties[IAccountManager::PROPERTY_PHONE]['scope'] = $phoneScope; - unset($expectedProperties[IAccountManager::PROPERTY_PHONE]['verified']); $expectedProperties[IAccountManager::PROPERTY_WEBSITE]['value'] = $website; $expectedProperties[IAccountManager::PROPERTY_WEBSITE]['scope'] = $websiteScope; - unset($expectedProperties[IAccountManager::PROPERTY_WEBSITE]['verified']); $expectedProperties[IAccountManager::PROPERTY_ADDRESS]['value'] = $address; $expectedProperties[IAccountManager::PROPERTY_ADDRESS]['scope'] = $addressScope; - unset($expectedProperties[IAccountManager::PROPERTY_ADDRESS]['verified']); $expectedProperties[IAccountManager::PROPERTY_TWITTER]['value'] = $twitter; $expectedProperties[IAccountManager::PROPERTY_TWITTER]['scope'] = $twitterScope; - unset($expectedProperties[IAccountManager::PROPERTY_TWITTER]['verified']); $this->mailer->expects($this->once())->method('validateMailAddress') ->willReturn(true); @@ -385,10 +381,8 @@ class UsersControllerTest extends \Test\TestCase { $expectedProperties[IAccountManager::PROPERTY_AVATAR]['scope'] = $avatarScope; $expectedProperties[IAccountManager::PROPERTY_DISPLAYNAME]['value'] = $displayName; $expectedProperties[IAccountManager::PROPERTY_DISPLAYNAME]['scope'] = $displayNameScope; - unset($expectedProperties[IAccountManager::PROPERTY_DISPLAYNAME]['verified']); $expectedProperties[IAccountManager::PROPERTY_EMAIL]['value'] = $email; $expectedProperties[IAccountManager::PROPERTY_EMAIL]['scope'] = $emailScope; - unset($expectedProperties[IAccountManager::PROPERTY_EMAIL]['verified']); $this->mailer->expects($this->once())->method('validateMailAddress') ->willReturn(true);