From 77aba3df3c2693fc8c1f656096327fde365bd99f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Sun, 31 Jan 2021 22:53:48 +0100 Subject: [PATCH] Fix deleting properties of user settings when not given explicitly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The controller can receive an optional subset of the properties of the user settings; values not given are set to "null" by default. However, those null values overwrote the previously existing values, so in practice any value not given was deleted from the user settings. Now only non null values overwrite the previous values. Signed-off-by: Daniel Calviño Sánchez --- .../lib/Controller/UsersController.php | 52 ++++++-- .../tests/Controller/UsersControllerTest.php | 124 ++++++++++++++++++ 2 files changed, 163 insertions(+), 13 deletions(-) diff --git a/apps/settings/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php index 4dfc6e0d71..353db298c8 100644 --- a/apps/settings/lib/Controller/UsersController.php +++ b/apps/settings/lib/Controller/UsersController.php @@ -375,24 +375,50 @@ class UsersController extends Controller { } $user = $this->userSession->getUser(); $data = $this->accountManager->getUser($user); - $data[IAccountManager::PROPERTY_AVATAR]['scope'] = $avatarScope; + if (!is_null($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; - $data[IAccountManager::PROPERTY_DISPLAYNAME]['scope'] = $displaynameScope; - $data[IAccountManager::PROPERTY_EMAIL]['value'] = $email; - $data[IAccountManager::PROPERTY_EMAIL]['scope'] = $emailScope; + if (!is_null($displayname)) { + $data[IAccountManager::PROPERTY_DISPLAYNAME]['value'] = $displayname; + } + if (!is_null($displaynameScope)) { + $data[IAccountManager::PROPERTY_DISPLAYNAME]['scope'] = $displaynameScope; + } + if (!is_null($email)) { + $data[IAccountManager::PROPERTY_EMAIL]['value'] = $email; + } + if (!is_null($emailScope)) { + $data[IAccountManager::PROPERTY_EMAIL]['scope'] = $emailScope; + } } if ($this->appManager->isEnabledForUser('federatedfilesharing')) { $shareProvider = \OC::$server->query(FederatedShareProvider::class); if ($shareProvider->isLookupServerUploadEnabled()) { - $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; + if (!is_null($website)) { + $data[IAccountManager::PROPERTY_WEBSITE]['value'] = $website; + } + if (!is_null($websiteScope)) { + $data[IAccountManager::PROPERTY_WEBSITE]['scope'] = $websiteScope; + } + if (!is_null($address)) { + $data[IAccountManager::PROPERTY_ADDRESS]['value'] = $address; + } + if (!is_null($addressScope)) { + $data[IAccountManager::PROPERTY_ADDRESS]['scope'] = $addressScope; + } + if (!is_null($phone)) { + $data[IAccountManager::PROPERTY_PHONE]['value'] = $phone; + } + if (!is_null($phoneScope)) { + $data[IAccountManager::PROPERTY_PHONE]['scope'] = $phoneScope; + } + if (!is_null($twitter)) { + $data[IAccountManager::PROPERTY_TWITTER]['value'] = $twitter; + } + if (!is_null($twitterScope)) { + $data[IAccountManager::PROPERTY_TWITTER]['scope'] = $twitterScope; + } } } try { diff --git a/apps/settings/tests/Controller/UsersControllerTest.php b/apps/settings/tests/Controller/UsersControllerTest.php index 78ddcde9e3..a43ff375fe 100644 --- a/apps/settings/tests/Controller/UsersControllerTest.php +++ b/apps/settings/tests/Controller/UsersControllerTest.php @@ -415,6 +415,130 @@ class UsersControllerTest extends \Test\TestCase { ); } + /** + * @dataProvider dataTestSetUserSettingsSubset + * + * @param string $property + * @param string $propertyValue + */ + public function testSetUserSettingsSubset($property, $propertyValue) { + $controller = $this->getController(false, ['saveUserSettings']); + $user = $this->createMock(IUser::class); + + $this->userSession->method('getUser')->willReturn($user); + + $defaultProperties = $this->getDefaultAccountManagerUserData(); + + $this->accountManager->expects($this->once()) + ->method('getUser') + ->with($user) + ->willReturn($defaultProperties); + + $this->appManager->expects($this->once()) + ->method('isEnabledForUser') + ->with('federatedfilesharing') + ->willReturn(true); + + $avatarScope = ($property === 'avatarScope') ? $propertyValue : null; + $displayName = ($property === 'displayName') ? $propertyValue : null; + $displayNameScope = ($property === 'displayNameScope') ? $propertyValue : null; + $phone = ($property === 'phone') ? $propertyValue : null; + $phoneScope = ($property === 'phoneScope') ? $propertyValue : null; + $email = ($property === 'email') ? $propertyValue : null; + $emailScope = ($property === 'emailScope') ? $propertyValue : null; + $website = ($property === 'website') ? $propertyValue : null; + $websiteScope = ($property === 'websiteScope') ? $propertyValue : null; + $address = ($property === 'address') ? $propertyValue : null; + $addressScope = ($property === 'addressScope') ? $propertyValue : null; + $twitter = ($property === 'twitter') ? $propertyValue : null; + $twitterScope = ($property === 'twitterScope') ? $propertyValue : null; + + $expectedProperties = $defaultProperties; + if ($property === 'avatarScope') { + $expectedProperties[IAccountManager::PROPERTY_AVATAR]['scope'] = $propertyValue; + } + if ($property === 'displayName') { + $expectedProperties[IAccountManager::PROPERTY_DISPLAYNAME]['value'] = $propertyValue; + } + if ($property === 'displayNameScope') { + $expectedProperties[IAccountManager::PROPERTY_DISPLAYNAME]['scope'] = $propertyValue; + } + if ($property === 'phone') { + $expectedProperties[IAccountManager::PROPERTY_PHONE]['value'] = $propertyValue; + } + if ($property === 'phoneScope') { + $expectedProperties[IAccountManager::PROPERTY_PHONE]['scope'] = $propertyValue; + } + if ($property === 'email') { + $expectedProperties[IAccountManager::PROPERTY_EMAIL]['value'] = $propertyValue; + } + if ($property === 'emailScope') { + $expectedProperties[IAccountManager::PROPERTY_EMAIL]['scope'] = $propertyValue; + } + if ($property === 'website') { + $expectedProperties[IAccountManager::PROPERTY_WEBSITE]['value'] = $propertyValue; + } + if ($property === 'websiteScope') { + $expectedProperties[IAccountManager::PROPERTY_WEBSITE]['scope'] = $propertyValue; + } + if ($property === 'address') { + $expectedProperties[IAccountManager::PROPERTY_ADDRESS]['value'] = $propertyValue; + } + if ($property === 'addressScope') { + $expectedProperties[IAccountManager::PROPERTY_ADDRESS]['scope'] = $propertyValue; + } + if ($property === 'twitter') { + $expectedProperties[IAccountManager::PROPERTY_TWITTER]['value'] = $propertyValue; + } + if ($property === 'twitterScope') { + $expectedProperties[IAccountManager::PROPERTY_TWITTER]['scope'] = $propertyValue; + } + + if (!empty($email)) { + $this->mailer->expects($this->once())->method('validateMailAddress') + ->willReturn(true); + } + + $controller->expects($this->once()) + ->method('saveUserSettings') + ->with($user, $expectedProperties) + ->willReturnArgument(1); + + $result = $controller->setUserSettings( + $avatarScope, + $displayName, + $displayNameScope, + $phone, + $phoneScope, + $email, + $emailScope, + $website, + $websiteScope, + $address, + $addressScope, + $twitter, + $twitterScope + ); + } + + public function dataTestSetUserSettingsSubset() { + return [ + ['avatarScope', IAccountManager::VISIBILITY_PUBLIC], + ['displayName', 'Display name'], + ['displayNameScope', IAccountManager::VISIBILITY_PUBLIC], + ['phone', '47658468'], + ['phoneScope', IAccountManager::VISIBILITY_PUBLIC], + ['email', 'john@example.com'], + ['emailScope', IAccountManager::VISIBILITY_PUBLIC], + ['website', 'nextcloud.com'], + ['websiteScope', IAccountManager::VISIBILITY_PUBLIC], + ['address', 'street and city'], + ['addressScope', IAccountManager::VISIBILITY_PUBLIC], + ['twitter', '@nextclouders'], + ['twitterScope', IAccountManager::VISIBILITY_PUBLIC], + ]; + } + /** * @dataProvider dataTestSaveUserSettings *