Skip to content

Commit 0ee96fa

Browse files
PVince81PVince81
committed
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>
1 parent 99d1709 commit 0ee96fa

File tree

5 files changed

+19
-79
lines changed

5 files changed

+19
-79
lines changed

‎apps/provisioning_api/lib/Controller/UsersController.php‎

Lines changed: 8 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,6 @@ class UsersController extends AUserData {
8585
protected $l10nFactory;
8686
/** @var NewUserMailHelper */
8787
private $newUserMailHelper;
88-
/** @var FederatedShareProviderFactory */
89-
private $federatedShareProviderFactory;
9088
/** @var ISecureRandom */
9189
private $secureRandom;
9290
/** @var RemoteWipe */
@@ -108,7 +106,6 @@ public function __construct(string $appName,
108106
LoggerInterface $logger,
109107
IFactory $l10nFactory,
110108
NewUserMailHelper $newUserMailHelper,
111-
FederatedShareProviderFactory $federatedShareProviderFactory,
112109
ISecureRandom $secureRandom,
113110
RemoteWipe $remoteWipe,
114111
KnownUserService $knownUserService,
@@ -127,7 +124,6 @@ public function __construct(string $appName,
127124
$this->logger = $logger;
128125
$this->l10nFactory = $l10nFactory;
129126
$this->newUserMailHelper = $newUserMailHelper;
130-
$this->federatedShareProviderFactory = $federatedShareProviderFactory;
131127
$this->secureRandom = $secureRandom;
132128
$this->remoteWipe = $remoteWipe;
133129
$this->knownUserService = $knownUserService;
@@ -532,15 +528,10 @@ public function getEditableFields(): DataResponse {
532528
$permittedFields[] = IAccountManager::PROPERTY_EMAIL;
533529
}
534530

535-
if ($this->appManager->isEnabledForUser('federatedfilesharing')) {
536-
$shareProvider = $this->federatedShareProviderFactory->get();
537-
if ($shareProvider->isLookupServerUploadEnabled()) {
538-
$permittedFields[] = IAccountManager::PROPERTY_PHONE;
539-
$permittedFields[] = IAccountManager::PROPERTY_ADDRESS;
540-
$permittedFields[] = IAccountManager::PROPERTY_WEBSITE;
541-
$permittedFields[] = IAccountManager::PROPERTY_TWITTER;
542-
}
543-
}
531+
$permittedFields[] = IAccountManager::PROPERTY_PHONE;
532+
$permittedFields[] = IAccountManager::PROPERTY_ADDRESS;
533+
$permittedFields[] = IAccountManager::PROPERTY_WEBSITE;
534+
$permittedFields[] = IAccountManager::PROPERTY_TWITTER;
544535

545536
return new DataResponse($permittedFields);
546537
}
@@ -586,15 +577,10 @@ public function editUser(string $userId, string $key, string $value): DataRespon
586577
$permittedFields[] = 'locale';
587578
}
588579

589-
if ($this->appManager->isEnabledForUser('federatedfilesharing')) {
590-
$shareProvider = $this->federatedShareProviderFactory->get();
591-
if ($shareProvider->isLookupServerUploadEnabled()) {
592-
$permittedFields[] = IAccountManager::PROPERTY_PHONE;
593-
$permittedFields[] = IAccountManager::PROPERTY_ADDRESS;
594-
$permittedFields[] = IAccountManager::PROPERTY_WEBSITE;
595-
$permittedFields[] = IAccountManager::PROPERTY_TWITTER;
596-
}
597-
}
580+
$permittedFields[] = IAccountManager::PROPERTY_PHONE;
581+
$permittedFields[] = IAccountManager::PROPERTY_ADDRESS;
582+
$permittedFields[] = IAccountManager::PROPERTY_WEBSITE;
583+
$permittedFields[] = IAccountManager::PROPERTY_TWITTER;
598584

599585
// If admin they can edit their own quota
600586
if ($this->groupManager->isAdmin($currentLoggedInUser->getUID())) {

‎apps/provisioning_api/tests/Controller/UsersControllerTest.php‎

Lines changed: 3 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,6 @@ class UsersControllerTest extends TestCase {
9797
private $l10nFactory;
9898
/** @var NewUserMailHelper|MockObject */
9999
private $newUserMailHelper;
100-
/** @var FederatedShareProviderFactory|MockObject */
101-
private $federatedShareProviderFactory;
102100
/** @var ISecureRandom|MockObject */
103101
private $secureRandom;
104102
/** @var RemoteWipe|MockObject */
@@ -122,7 +120,6 @@ protected function setUp(): void {
122120
$this->urlGenerator = $this->createMock(IURLGenerator::class);
123121
$this->l10nFactory = $this->createMock(IFactory::class);
124122
$this->newUserMailHelper = $this->createMock(NewUserMailHelper::class);
125-
$this->federatedShareProviderFactory = $this->createMock(FederatedShareProviderFactory::class);
126123
$this->secureRandom = $this->createMock(ISecureRandom::class);
127124
$this->remoteWipe = $this->createMock(RemoteWipe::class);
128125
$this->knownUserService = $this->createMock(KnownUserService::class);
@@ -142,7 +139,6 @@ protected function setUp(): void {
142139
$this->logger,
143140
$this->l10nFactory,
144141
$this->newUserMailHelper,
145-
$this->federatedShareProviderFactory,
146142
$this->secureRandom,
147143
$this->remoteWipe,
148144
$this->knownUserService,
@@ -407,7 +403,6 @@ public function testAddUserSuccessfulWithDisplayName() {
407403
$this->logger,
408404
$this->l10nFactory,
409405
$this->newUserMailHelper,
410-
$this->federatedShareProviderFactory,
411406
$this->secureRandom,
412407
$this->remoteWipe,
413408
$this->knownUserService,
@@ -3247,7 +3242,6 @@ public function testGetCurrentUserLoggedIn() {
32473242
$this->logger,
32483243
$this->l10nFactory,
32493244
$this->newUserMailHelper,
3250-
$this->federatedShareProviderFactory,
32513245
$this->secureRandom,
32523246
$this->remoteWipe,
32533247
$this->knownUserService,
@@ -3314,7 +3308,6 @@ public function testGetUser() {
33143308
$this->logger,
33153309
$this->l10nFactory,
33163310
$this->newUserMailHelper,
3317-
$this->federatedShareProviderFactory,
33183311
$this->secureRandom,
33193312
$this->remoteWipe,
33203313
$this->knownUserService,
@@ -3639,18 +3632,13 @@ public function testResendWelcomeMessageFailed() {
36393632

36403633
public function dataGetEditableFields() {
36413634
return [
3642-
[false, false, []],
3643-
[false, true, [
3635+
[false, [
36443636
IAccountManager::PROPERTY_PHONE,
36453637
IAccountManager::PROPERTY_ADDRESS,
36463638
IAccountManager::PROPERTY_WEBSITE,
36473639
IAccountManager::PROPERTY_TWITTER,
36483640
]],
3649-
[ true, false, [
3650-
IAccountManager::PROPERTY_DISPLAYNAME,
3651-
IAccountManager::PROPERTY_EMAIL,
3652-
]],
3653-
[ true, true ,[
3641+
[ true, [
36543642
IAccountManager::PROPERTY_DISPLAYNAME,
36553643
IAccountManager::PROPERTY_EMAIL,
36563644
IAccountManager::PROPERTY_PHONE,
@@ -3665,27 +3653,15 @@ public function dataGetEditableFields() {
36653653
* @dataProvider dataGetEditableFields
36663654
*
36673655
* @param bool $allowedToChangeDisplayName
3668-
* @param bool $federatedSharingEnabled
36693656
* @param array $expected
36703657
*/
3671-
public function testGetEditableFields(bool $allowedToChangeDisplayName, bool $federatedSharingEnabled, array $expected) {
3658+
public function testGetEditableFields(bool $allowedToChangeDisplayName, array $expected) {
36723659
$this->config
36733660
->method('getSystemValue')
36743661
->with(
36753662
$this->equalTo('allow_user_to_change_display_name'),
36763663
$this->anything()
36773664
)->willReturn($allowedToChangeDisplayName);
3678-
$this->appManager
3679-
->method('isEnabledForUser')
3680-
->with($this->equalTo('federatedfilesharing'))
3681-
->willReturn($federatedSharingEnabled);
3682-
3683-
$shareprovider = $this->createMock(FederatedShareProvider::class);
3684-
$shareprovider->method('isLookupServerUploadEnabled')->willReturn(true);
3685-
3686-
$this->federatedShareProviderFactory
3687-
->method('get')
3688-
->willReturn($shareprovider);
36893665

36903666
$expectedResp = new DataResponse($expected);
36913667
$this->assertEquals($expectedResp, $this->api->getEditableFields());

‎apps/settings/lib/Controller/UsersController.php‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@
4646
use OC\L10N\Factory;
4747
use OC\Security\IdentityProof\Manager;
4848
use OC\User\Manager as UserManager;
49-
use OCA\FederatedFileSharing\FederatedShareProvider;
5049
use OCA\Settings\BackgroundJobs\VerifyUserData;
5150
use OCA\Settings\Events\BeforeTemplateRenderedEvent;
5251
use OCA\User_LDAP\User_Proxy;
@@ -401,15 +400,11 @@ public function setUserSettings(?string $avatarScope = null,
401400
$data[IAccountManager::PROPERTY_DISPLAYNAME] = ['value' => $displayname, 'scope' => $displaynameScope];
402401
$data[IAccountManager::PROPERTY_EMAIL] = ['value' => $email, 'scope' => $emailScope];
403402
}
404-
if ($this->appManager->isEnabledForUser('federatedfilesharing')) {
405-
$shareProvider = \OC::$server->query(FederatedShareProvider::class);
406-
if ($shareProvider->isLookupServerUploadEnabled()) {
407-
$data[IAccountManager::PROPERTY_WEBSITE] = ['value' => $website, 'scope' => $websiteScope];
408-
$data[IAccountManager::PROPERTY_ADDRESS] = ['value' => $address, 'scope' => $addressScope];
409-
$data[IAccountManager::PROPERTY_PHONE] = ['value' => $phone, 'scope' => $phoneScope];
410-
$data[IAccountManager::PROPERTY_TWITTER] = ['value' => $twitter, 'scope' => $twitterScope];
411-
}
412-
}
403+
$data[IAccountManager::PROPERTY_WEBSITE] = ['value' => $website, 'scope' => $websiteScope];
404+
$data[IAccountManager::PROPERTY_ADDRESS] = ['value' => $address, 'scope' => $addressScope];
405+
$data[IAccountManager::PROPERTY_PHONE] = ['value' => $phone, 'scope' => $phoneScope];
406+
$data[IAccountManager::PROPERTY_TWITTER] = ['value' => $twitter, 'scope' => $twitterScope];
407+
413408
try {
414409
$data = $this->saveUserSettings($user, $data);
415410
if ($beforeData[IAccountManager::PROPERTY_PHONE]['value'] !== $data[IAccountManager::PROPERTY_PHONE]['value']) {

‎apps/settings/templates/settings/personal/personal.info.php‎

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,6 @@
177177
<?php } ?>
178178
</form>
179179
</div>
180-
<?php if (!empty($_['phone']) || $_['lookupServerUploadEnabled']) { ?>
181180
<div class="personal-settings-setting-box">
182181
<form id="phoneform" class="section">
183182
<h3>
@@ -188,9 +187,7 @@
188187
</span>
189188
</div>
190189
</h3>
191-
<input type="tel" id="phone" name="phone" <?php if (!$_['lookupServerUploadEnabled']) {
192-
print_unescaped('disabled="1"');
193-
} ?>
190+
<input type="tel" id="phone" name="phone"
194191
value="<?php p($_['phone']) ?>"
195192
placeholder="<?php p($l->t('Your phone number')); ?>"
196193
autocomplete="on" autocapitalize="none" autocorrect="off" />
@@ -199,8 +196,6 @@
199196
<input type="hidden" id="phonescope" value="<?php p($_['phoneScope']) ?>">
200197
</form>
201198
</div>
202-
<?php } ?>
203-
<?php if (!empty($_['address']) || $_['lookupServerUploadEnabled']) { ?>
204199
<div class="personal-settings-setting-box">
205200
<form id="addressform" class="section">
206201
<h3>
@@ -211,9 +206,7 @@
211206
</span>
212207
</div>
213208
</h3>
214-
<input type="text" id="address" name="address" <?php if (!$_['lookupServerUploadEnabled']) {
215-
print_unescaped('disabled="1"');
216-
} ?>
209+
<input type="text" id="address" name="address"
217210
placeholder="<?php p($l->t('Your postal address')); ?>"
218211
value="<?php p($_['address']) ?>"
219212
autocomplete="on" autocapitalize="none" autocorrect="off" />
@@ -222,8 +215,6 @@
222215
<input type="hidden" id="addressscope" value="<?php p($_['addressScope']) ?>">
223216
</form>
224217
</div>
225-
<?php } ?>
226-
<?php if (!empty($_['website']) || $_['lookupServerUploadEnabled']) { ?>
227218
<div class="personal-settings-setting-box">
228219
<form id="websiteform" class="section">
229220
<h3>
@@ -267,17 +258,12 @@
267258
<input type="url" name="website" id="website" value="<?php p($_['website']); ?>"
268259
placeholder="<?php p($l->t('Link https://…')); ?>"
269260
autocomplete="on" autocapitalize="none" autocorrect="off"
270-
<?php if (!$_['lookupServerUploadEnabled']) {
271-
print_unescaped('disabled="1"');
272-
} ?>
273261
/>
274262
<span class="icon-checkmark hidden"></span>
275263
<span class="icon-error hidden" ></span>
276264
<input type="hidden" id="websitescope" value="<?php p($_['websiteScope']) ?>">
277265
</form>
278266
</div>
279-
<?php } ?>
280-
<?php if (!empty($_['twitter']) || $_['lookupServerUploadEnabled']) { ?>
281267
<div class="personal-settings-setting-box">
282268
<form id="twitterform" class="section">
283269
<h3>
@@ -321,16 +307,12 @@
321307
<input type="text" name="twitter" id="twitter" value="<?php p($_['twitter']); ?>"
322308
placeholder="<?php p($l->t('Twitter handle @…')); ?>"
323309
autocomplete="on" autocapitalize="none" autocorrect="off"
324-
<?php if (!$_['lookupServerUploadEnabled']) {
325-
print_unescaped('disabled="1"');
326-
} ?>
327310
/>
328311
<span class="icon-checkmark hidden"></span>
329312
<span class="icon-error hidden" ></span>
330313
<input type="hidden" id="twitterscope" value="<?php p($_['twitterScope']) ?>">
331314
</form>
332315
</div>
333-
<?php } ?>
334316
</div>
335317

336318
<div class="profile-settings-container">

‎apps/settings/tests/Controller/UsersControllerTest.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ protected function getController($isAdmin = false, $mockedMethods = []) {
190190
public function testSetUserSettings($email, $validEmail, $expectedStatus) {
191191
$controller = $this->getController(false, ['saveUserSettings']);
192192
$user = $this->createMock(IUser::class);
193+
$user->method('getUID')->willReturn('johndoe');
193194

194195
$this->userSession->method('getUser')->willReturn($user);
195196

0 commit comments

Comments
 (0)