From: Daniel Calviño Sánchez Date: Sun, 31 Jan 2021 21:53:48 +0000 (+0100) Subject: Fix deleting properties of user settings when not given explicitly X-Git-Tag: v20.0.10RC1~3^2~4 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=77aba3df3c2693fc8c1f656096327fde365bd99f;p=nextcloud-server.git Fix deleting properties of user settings when not given explicitly 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 --- diff --git a/apps/settings/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php index 4dfc6e0d71e..353db298c83 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 78ddcde9e37..a43ff375fe6 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 *