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 <danxuliu@gmail.com>

diff --git a/apps/settings/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php
index 4dfc6e0d71e88a78203a503e10d71808e5e4c161..353db298c8350109567e5274dfced0ff32959bde 100644 (file)
--- 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 78ddcde9e3789b3103f3c1ace904d12b1820ab4f..a43ff375fe65ef63fa83f49aac5728ceed0af3c5 100644 (file)
--- 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
         *