From 5c854ba13291c4eecb87ac7a5235304711e5cbbe Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Tue, 23 Mar 2021 16:59:05 +0100 Subject: [PATCH] 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 --- .../lib/Controller/UsersController.php | 31 +++++------------- .../tests/Controller/UsersControllerTest.php | 32 ++----------------- .../lib/Controller/UsersController.php | 15 +++------ .../settings/personal/personal.info.php | 22 ++----------- .../tests/Controller/UsersControllerTest.php | 1 + 5 files changed, 19 insertions(+), 82 deletions(-) diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index db4138054e..1c9b63a01c 100644 --- 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; @@ -84,8 +83,6 @@ class UsersController extends AUserData { protected $l10nFactory; /** @var NewUserMailHelper */ private $newUserMailHelper; - /** @var FederatedShareProviderFactory */ - private $federatedShareProviderFactory; /** @var ISecureRandom */ private $secureRandom; /** @var RemoteWipe */ @@ -107,7 +104,6 @@ class UsersController extends AUserData { ILogger $logger, IFactory $l10nFactory, NewUserMailHelper $newUserMailHelper, - FederatedShareProviderFactory $federatedShareProviderFactory, ISecureRandom $secureRandom, RemoteWipe $remoteWipe, KnownUserService $knownUserService, @@ -126,7 +122,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; @@ -519,15 +514,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); } @@ -573,15 +563,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 39743579b7..8f4e8395f2 100644 --- 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, @@ -3246,7 +3239,6 @@ class UsersControllerTest extends TestCase { $this->logger, $this->l10nFactory, $this->newUserMailHelper, - $this->federatedShareProviderFactory, $this->secureRandom, $this->remoteWipe, $this->knownUserService, @@ -3313,7 +3305,6 @@ class UsersControllerTest extends TestCase { $this->logger, $this->l10nFactory, $this->newUserMailHelper, - $this->federatedShareProviderFactory, $this->secureRandom, $this->remoteWipe, $this->knownUserService, @@ -3638,18 +3629,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, @@ -3664,27 +3650,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 02a0cda139..c59c16666f 100644 --- 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 60db8c8533..f2e3a51aad 100644 --- a/apps/settings/templates/settings/personal/personal.info.php +++ b/apps/settings/templates/settings/personal/personal.info.php @@ -177,7 +177,6 @@ script('settings', [ -

@@ -188,9 +187,7 @@ script('settings', [

- + @@ -199,8 +196,6 @@ script('settings', [ - -

@@ -211,9 +206,7 @@ script('settings', [

- + @@ -222,8 +215,6 @@ script('settings', [ - -

@@ -267,17 +258,12 @@ script('settings', [ />

- -

@@ -321,16 +307,12 @@ script('settings', [ />

-
diff --git a/apps/settings/tests/Controller/UsersControllerTest.php b/apps/settings/tests/Controller/UsersControllerTest.php index 2daf383410..f9652053de 100644 --- 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);