Make extra user profile fields always editable

The fields for phone number, address, website and twitter are now
editable regardless whether federated sharing and the lookup server
are enabled or not.

Signed-off-by: Vincent Petry <vincent@nextcloud.com>

diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php
index d2f9b9e91c227f51cc6c289dbe10745d78b296d1..3a1dcd4f43a1d844069e0c8480367b408b55d9b5 100644 (file)
--- a/apps/provisioning_api/lib/Controller/UsersController.php
+++ b/apps/provisioning_api/lib/Controller/UsersController.php
@@ -50,7 +50,6 @@ use OC\Accounts\AccountManager;
 use OC\Authentication\Token\RemoteWipe;
 use OC\HintException;
 use OC\KnownUser\KnownUserService;
-use OCA\Provisioning_API\FederatedShareProviderFactory;
 use OCA\Settings\Mailer\NewUserMailHelper;
 use OCP\Accounts\IAccountManager;
 use OCP\App\IAppManager;
@@ -85,8 +84,6 @@ class UsersController extends AUserData {
        protected $l10nFactory;
        /** @var NewUserMailHelper */
        private $newUserMailHelper;
-       /** @var FederatedShareProviderFactory */
-       private $federatedShareProviderFactory;
        /** @var ISecureRandom */
        private $secureRandom;
        /** @var RemoteWipe */
@@ -108,7 +105,6 @@ class UsersController extends AUserData {
                                                                LoggerInterface $logger,
                                                                IFactory $l10nFactory,
                                                                NewUserMailHelper $newUserMailHelper,
-                                                               FederatedShareProviderFactory $federatedShareProviderFactory,
                                                                ISecureRandom $secureRandom,
                                                                RemoteWipe $remoteWipe,
                                                                KnownUserService $knownUserService,
@@ -127,7 +123,6 @@ class UsersController extends AUserData {
                $this->logger = $logger;
                $this->l10nFactory = $l10nFactory;
                $this->newUserMailHelper = $newUserMailHelper;
-               $this->federatedShareProviderFactory = $federatedShareProviderFactory;
                $this->secureRandom = $secureRandom;
                $this->remoteWipe = $remoteWipe;
                $this->knownUserService = $knownUserService;
@@ -532,15 +527,10 @@ class UsersController extends AUserData {
                        $permittedFields[] = IAccountManager::PROPERTY_EMAIL;
                }
 
-               if ($this->appManager->isEnabledForUser('federatedfilesharing')) {
-                       $shareProvider = $this->federatedShareProviderFactory->get();
-                       if ($shareProvider->isLookupServerUploadEnabled()) {
-                               $permittedFields[] = IAccountManager::PROPERTY_PHONE;
-                               $permittedFields[] = IAccountManager::PROPERTY_ADDRESS;
-                               $permittedFields[] = IAccountManager::PROPERTY_WEBSITE;
-                               $permittedFields[] = IAccountManager::PROPERTY_TWITTER;
-                       }
-               }
+               $permittedFields[] = IAccountManager::PROPERTY_PHONE;
+               $permittedFields[] = IAccountManager::PROPERTY_ADDRESS;
+               $permittedFields[] = IAccountManager::PROPERTY_WEBSITE;
+               $permittedFields[] = IAccountManager::PROPERTY_TWITTER;
 
                return new DataResponse($permittedFields);
        }
@@ -586,15 +576,10 @@ class UsersController extends AUserData {
                                $permittedFields[] = 'locale';
                        }
 
-                       if ($this->appManager->isEnabledForUser('federatedfilesharing')) {
-                               $shareProvider = $this->federatedShareProviderFactory->get();
-                               if ($shareProvider->isLookupServerUploadEnabled()) {
-                                       $permittedFields[] = IAccountManager::PROPERTY_PHONE;
-                                       $permittedFields[] = IAccountManager::PROPERTY_ADDRESS;
-                                       $permittedFields[] = IAccountManager::PROPERTY_WEBSITE;
-                                       $permittedFields[] = IAccountManager::PROPERTY_TWITTER;
-                               }
-                       }
+                       $permittedFields[] = IAccountManager::PROPERTY_PHONE;
+                       $permittedFields[] = IAccountManager::PROPERTY_ADDRESS;
+                       $permittedFields[] = IAccountManager::PROPERTY_WEBSITE;
+                       $permittedFields[] = IAccountManager::PROPERTY_TWITTER;
 
                        // If admin they can edit their own quota
                        if ($this->groupManager->isAdmin($currentLoggedInUser->getUID())) {

diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php
index d65e3d079138666736fa88708f891b1b61a52907..720eaaec9cf94d58208932a6b0e7154c01738c02 100644 (file)
--- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php
+++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php
@@ -46,9 +46,7 @@ use OC\Authentication\Token\RemoteWipe;
 use OC\Group\Manager;
 use OC\KnownUser\KnownUserService;
 use OC\SubAdmin;
-use OCA\FederatedFileSharing\FederatedShareProvider;
 use OCA\Provisioning_API\Controller\UsersController;
-use OCA\Provisioning_API\FederatedShareProviderFactory;
 use OCA\Settings\Mailer\NewUserMailHelper;
 use OCP\Accounts\IAccountManager;
 use OCP\App\IAppManager;
@@ -97,8 +95,6 @@ class UsersControllerTest extends TestCase {
        private $l10nFactory;
        /** @var NewUserMailHelper|MockObject */
        private $newUserMailHelper;
-       /** @var FederatedShareProviderFactory|MockObject */
-       private $federatedShareProviderFactory;
        /** @var ISecureRandom|MockObject */
        private $secureRandom;
        /** @var RemoteWipe|MockObject */
@@ -122,7 +118,6 @@ class UsersControllerTest extends TestCase {
                $this->urlGenerator = $this->createMock(IURLGenerator::class);
                $this->l10nFactory = $this->createMock(IFactory::class);
                $this->newUserMailHelper = $this->createMock(NewUserMailHelper::class);
-               $this->federatedShareProviderFactory = $this->createMock(FederatedShareProviderFactory::class);
                $this->secureRandom = $this->createMock(ISecureRandom::class);
                $this->remoteWipe = $this->createMock(RemoteWipe::class);
                $this->knownUserService = $this->createMock(KnownUserService::class);
@@ -142,7 +137,6 @@ class UsersControllerTest extends TestCase {
                                $this->logger,
                                $this->l10nFactory,
                                $this->newUserMailHelper,
-                               $this->federatedShareProviderFactory,
                                $this->secureRandom,
                                $this->remoteWipe,
                                $this->knownUserService,
@@ -407,7 +401,6 @@ class UsersControllerTest extends TestCase {
                                $this->logger,
                                $this->l10nFactory,
                                $this->newUserMailHelper,
-                               $this->federatedShareProviderFactory,
                                $this->secureRandom,
                                $this->remoteWipe,
                                $this->knownUserService,
@@ -3247,7 +3240,6 @@ class UsersControllerTest extends TestCase {
                                $this->logger,
                                $this->l10nFactory,
                                $this->newUserMailHelper,
-                               $this->federatedShareProviderFactory,
                                $this->secureRandom,
                                $this->remoteWipe,
                                $this->knownUserService,
@@ -3314,7 +3306,6 @@ class UsersControllerTest extends TestCase {
                                $this->logger,
                                $this->l10nFactory,
                                $this->newUserMailHelper,
-                               $this->federatedShareProviderFactory,
                                $this->secureRandom,
                                $this->remoteWipe,
                                $this->knownUserService,
@@ -3639,18 +3630,13 @@ class UsersControllerTest extends TestCase {
 
        public function dataGetEditableFields() {
                return [
-                       [false, false, []],
-                       [false,  true, [
+                       [false, [
                                IAccountManager::PROPERTY_PHONE,
                                IAccountManager::PROPERTY_ADDRESS,
                                IAccountManager::PROPERTY_WEBSITE,
                                IAccountManager::PROPERTY_TWITTER,
                        ]],
-                       [ true, false, [
-                               IAccountManager::PROPERTY_DISPLAYNAME,
-                               IAccountManager::PROPERTY_EMAIL,
-                       ]],
-                       [ true,  true ,[
+                       [ true, [
                                IAccountManager::PROPERTY_DISPLAYNAME,
                                IAccountManager::PROPERTY_EMAIL,
                                IAccountManager::PROPERTY_PHONE,
@@ -3665,27 +3651,15 @@ class UsersControllerTest extends TestCase {
         * @dataProvider dataGetEditableFields
         *
         * @param bool $allowedToChangeDisplayName
-        * @param bool $federatedSharingEnabled
         * @param array $expected
         */
-       public function testGetEditableFields(bool $allowedToChangeDisplayName, bool $federatedSharingEnabled, array $expected) {
+       public function testGetEditableFields(bool $allowedToChangeDisplayName, array $expected) {
                $this->config
                        ->method('getSystemValue')
                        ->with(
                                $this->equalTo('allow_user_to_change_display_name'),
                                $this->anything()
                        )->willReturn($allowedToChangeDisplayName);
-               $this->appManager
-                       ->method('isEnabledForUser')
-                       ->with($this->equalTo('federatedfilesharing'))
-                       ->willReturn($federatedSharingEnabled);
-
-               $shareprovider = $this->createMock(FederatedShareProvider::class);
-               $shareprovider->method('isLookupServerUploadEnabled')->willReturn(true);
-
-               $this->federatedShareProviderFactory
-                       ->method('get')
-                       ->willReturn($shareprovider);
 
                $expectedResp = new DataResponse($expected);
                $this->assertEquals($expectedResp, $this->api->getEditableFields());

diff --git a/apps/settings/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php
index 46de0b4cd96a4005093fe4210850138824ab1c60..a9b72571de6be5e13bb62ec0888f8d7fab8b9502 100644 (file)
--- a/apps/settings/lib/Controller/UsersController.php
+++ b/apps/settings/lib/Controller/UsersController.php
@@ -46,7 +46,6 @@ use OC\KnownUser\KnownUserService;
 use OC\L10N\Factory;
 use OC\Security\IdentityProof\Manager;
 use OC\User\Manager as UserManager;
-use OCA\FederatedFileSharing\FederatedShareProvider;
 use OCA\Settings\BackgroundJobs\VerifyUserData;
 use OCA\Settings\Events\BeforeTemplateRenderedEvent;
 use OCA\User_LDAP\User_Proxy;
@@ -401,15 +400,11 @@ class UsersController extends Controller {
                        $data[IAccountManager::PROPERTY_DISPLAYNAME] = ['value' => $displayname, 'scope' => $displaynameScope];
                        $data[IAccountManager::PROPERTY_EMAIL] = ['value' => $email, 'scope' => $emailScope];
                }
-               if ($this->appManager->isEnabledForUser('federatedfilesharing')) {
-                       $shareProvider = \OC::$server->query(FederatedShareProvider::class);
-                       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, '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];
+
                try {
                        $data = $this->saveUserSettings($user, $data);
                        if ($beforeData[IAccountManager::PROPERTY_PHONE]['value'] !== $data[IAccountManager::PROPERTY_PHONE]['value']) {

diff --git a/apps/settings/templates/settings/personal/personal.info.php b/apps/settings/templates/settings/personal/personal.info.php
index 60db8c8533343c84be41e3ca44479116f97f7ffd..f2e3a51aad70a7245ca6a67be4486e154820f5eb 100644 (file)
--- a/apps/settings/templates/settings/personal/personal.info.php
+++ b/apps/settings/templates/settings/personal/personal.info.php
@@ -177,7 +177,6 @@ script('settings', [
                                <?php } ?>
                        </form>
                </div>
-               <?php if (!empty($_['phone']) || $_['lookupServerUploadEnabled']) { ?>
                <div class="personal-settings-setting-box">
                        <form id="phoneform" class="section">
                                <h3>
@@ -188,9 +187,7 @@ script('settings', [
                                                </span>
                                        </div>
                                </h3>
-                               <input type="tel" id="phone" name="phone" <?php if (!$_['lookupServerUploadEnabled']) {
-                                               print_unescaped('disabled="1"');
-                                       } ?>
+                               <input type="tel" id="phone" name="phone"
                                           value="<?php p($_['phone']) ?>"
                                           placeholder="<?php p($l->t('Your phone number')); ?>"
                                       autocomplete="on" autocapitalize="none" autocorrect="off" />
@@ -199,8 +196,6 @@ script('settings', [
                                <input type="hidden" id="phonescope" value="<?php p($_['phoneScope']) ?>">
                        </form>
                </div>
-               <?php } ?>
-               <?php if (!empty($_['address']) || $_['lookupServerUploadEnabled']) { ?>
                <div class="personal-settings-setting-box">
                        <form id="addressform" class="section">
                                <h3>
@@ -211,9 +206,7 @@ script('settings', [
                                                </span>
                                        </div>
                                </h3>
-                               <input type="text" id="address" name="address" <?php if (!$_['lookupServerUploadEnabled']) {
-                                               print_unescaped('disabled="1"');
-                                       }  ?>
+                               <input type="text" id="address" name="address"
                                           placeholder="<?php p($l->t('Your postal address')); ?>"
                                           value="<?php p($_['address']) ?>"
                                           autocomplete="on" autocapitalize="none" autocorrect="off" />
@@ -222,8 +215,6 @@ script('settings', [
                                <input type="hidden" id="addressscope" value="<?php p($_['addressScope']) ?>">
                        </form>
                </div>
-               <?php } ?>
-               <?php if (!empty($_['website']) || $_['lookupServerUploadEnabled']) { ?>
                <div class="personal-settings-setting-box">
                        <form id="websiteform" class="section">
                                <h3>
@@ -267,17 +258,12 @@ script('settings', [
                                <input type="url" name="website" id="website" value="<?php p($_['website']); ?>"
                                       placeholder="<?php p($l->t('Link https://…')); ?>"
                                       autocomplete="on" autocapitalize="none" autocorrect="off"
-                                          <?php if (!$_['lookupServerUploadEnabled']) {
-                                               print_unescaped('disabled="1"');
-                                       }  ?>
                                />
                                <span class="icon-checkmark hidden"></span>
                                <span class="icon-error hidden" ></span>
                                <input type="hidden" id="websitescope" value="<?php p($_['websiteScope']) ?>">
                        </form>
                </div>
-               <?php } ?>
-               <?php if (!empty($_['twitter']) || $_['lookupServerUploadEnabled']) { ?>
                <div class="personal-settings-setting-box">
                        <form id="twitterform" class="section">
                                <h3>
@@ -321,16 +307,12 @@ script('settings', [
                                <input type="text" name="twitter" id="twitter" value="<?php p($_['twitter']); ?>"
                                           placeholder="<?php p($l->t('Twitter handle @…')); ?>"
                                           autocomplete="on" autocapitalize="none" autocorrect="off"
-                                          <?php if (!$_['lookupServerUploadEnabled']) {
-                                               print_unescaped('disabled="1"');
-                                       }  ?>
                                />
                                <span class="icon-checkmark hidden"></span>
                                <span class="icon-error hidden" ></span>
                                <input type="hidden" id="twitterscope" value="<?php p($_['twitterScope']) ?>">
                        </form>
                </div>
-               <?php } ?>
        </div>
 
        <div class="profile-settings-container">

diff --git a/apps/settings/tests/Controller/UsersControllerTest.php b/apps/settings/tests/Controller/UsersControllerTest.php
index 2daf383410eba05dfb8c179e639604c382154e4b..f9652053de8917d5963f0f65496fea73d4ffb16c 100644 (file)
--- a/apps/settings/tests/Controller/UsersControllerTest.php
+++ b/apps/settings/tests/Controller/UsersControllerTest.php
@@ -190,6 +190,7 @@ class UsersControllerTest extends \Test\TestCase {
        public function testSetUserSettings($email, $validEmail, $expectedStatus) {
                $controller = $this->getController(false, ['saveUserSettings']);
                $user = $this->createMock(IUser::class);
+               $user->method('getUID')->willReturn('johndoe');
 
                $this->userSession->method('getUser')->willReturn($user);