Skip to content

Commit 919e148

Browse files
committed
refactor(settings): use IUserConfig and IAppConfig in UsersController
Signed-off-by: Peter Ringelmann <[email protected]>
1 parent 9b513c1 commit 919e148

File tree

2 files changed

+55
-24
lines changed

2 files changed

+55
-24
lines changed

apps/settings/lib/Controller/UsersController.php

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,11 @@
4141
use OCP\Encryption\IManager;
4242
use OCP\EventDispatcher\IEventDispatcher;
4343
use OCP\Group\ISubAdmin;
44+
use OCP\IAppConfig;
4445
use OCP\IConfig;
4546
use OCP\IGroup;
4647
use OCP\IGroupManager;
48+
use OCP\Config\IUserConfig;
4749
use OCP\IL10N;
4850
use OCP\INavigationManager;
4951
use OCP\IRequest;
@@ -75,6 +77,8 @@ public function __construct(
7577
private IGroupManager $groupManager,
7678
private IUserSession $userSession,
7779
private IConfig $config,
80+
private IAppConfig $appConfig,
81+
private IUserConfig $userConfig,
7882
private IL10N $l10n,
7983
private IMailer $mailer,
8084
private IFactory $l10nFactory,
@@ -200,12 +204,12 @@ public function usersList(INavigationManager $navigationManager, ISubAdmin $subA
200204
}
201205

202206
/* QUOTAS PRESETS */
203-
$quotaPreset = $this->parseQuotaPreset($this->config->getAppValue('files', 'quota_preset', '1 GB, 5 GB, 10 GB'));
204-
$allowUnlimitedQuota = $this->config->getAppValue('files', 'allow_unlimited_quota', '1') === '1';
207+
$quotaPreset = $this->parseQuotaPreset($this->appConfig->getValueString('files', 'quota_preset', '1 GB, 5 GB, 10 GB'));
208+
$allowUnlimitedQuota = $this->appConfig->getValueBool('files', 'allow_unlimited_quota', true);
205209
if (!$allowUnlimitedQuota && count($quotaPreset) > 0) {
206-
$defaultQuota = $this->config->getAppValue('files', 'default_quota', $quotaPreset[0]);
210+
$defaultQuota = $this->appConfig->getValueString('files', 'default_quota', $quotaPreset[0]);
207211
} else {
208-
$defaultQuota = $this->config->getAppValue('files', 'default_quota', 'none');
212+
$defaultQuota = $this->appConfig->getValueString('files', 'default_quota', 'none');
209213
}
210214

211215
$event = new BeforeTemplateRenderedEvent();
@@ -228,7 +232,7 @@ public function usersList(INavigationManager $navigationManager, ISubAdmin $subA
228232
$serverData['isDelegatedAdmin'] = $isDelegatedAdmin;
229233
$serverData['sortGroups'] = $forceSortGroupByName
230234
? MetaData::SORT_GROUPNAME
231-
: (int)$this->config->getAppValue('core', 'group.sortBy', (string)MetaData::SORT_USERCOUNT);
235+
: (int)$this->appConfig->getValueString('core', 'group.sortBy', (string)MetaData::SORT_USERCOUNT);
232236
$serverData['forceSortGroupByName'] = $forceSortGroupByName;
233237
$serverData['quotaPreset'] = $quotaPreset;
234238
$serverData['allowUnlimitedQuota'] = $allowUnlimitedQuota;
@@ -239,12 +243,12 @@ public function usersList(INavigationManager $navigationManager, ISubAdmin $subA
239243
// Settings
240244
$serverData['defaultQuota'] = $defaultQuota;
241245
$serverData['canChangePassword'] = $canChangePassword;
242-
$serverData['newUserGenerateUserID'] = $this->config->getAppValue('core', 'newUser.generateUserID', 'no') === 'yes';
243-
$serverData['newUserRequireEmail'] = $this->config->getAppValue('core', 'newUser.requireEmail', 'no') === 'yes';
244-
$serverData['newUserSendEmail'] = $this->config->getAppValue('core', 'newUser.sendEmail', 'yes') === 'yes';
246+
$serverData['newUserGenerateUserID'] = $this->appConfig->getValueBool('core', 'newUser.generateUserID', false);
247+
$serverData['newUserRequireEmail'] = $this->appConfig->getValueBool('core', 'newUser.requireEmail', false);
248+
$serverData['newUserSendEmail'] = $this->appConfig->getValueBool('core', 'newUser.sendEmail', true);
245249
$serverData['showConfig'] = [];
246250
foreach (self::ALLOWED_USER_PREFERENCES as $key) {
247-
$serverData['showConfig'][$key] = $this->config->getUserValue($uid, $this->appName, $key, 'false') === 'true';
251+
$serverData['showConfig'][$key] = $this->userConfig->getValueBool($uid, $this->appName, $key, false);
248252
}
249253

250254
$this->initialState->provideInitialState('usersSettings', $serverData);
@@ -263,12 +267,19 @@ public function usersList(INavigationManager $navigationManager, ISubAdmin $subA
263267
*/
264268
#[AuthorizedAdminSetting(settings:Users::class)]
265269
public function setPreference(string $key, string $value): JSONResponse {
266-
$allowedAppValues = ['newUser.sendEmail', 'group.sortBy'];
270+
$allowedAppValues = [
271+
'newUser.sendEmail',
272+
'group.sortBy',
273+
];
267274

268275
if (in_array($key, $allowedAppValues, true)) {
269-
$this->config->setAppValue('core', $key, $value);
276+
if ($key === 'newUser.sendEmail') {
277+
$this->appConfig->setValueBool('core', $key, $value === 'yes');
278+
} else {
279+
$this->appConfig->setValueString('core', $key, $value);
280+
}
270281
} elseif (in_array($key, self::ALLOWED_USER_PREFERENCES, true)) {
271-
$this->config->setUserValue($this->userSession->getUser()->getUID(), $this->appName, $key, $value);
282+
$this->userConfig->setValueBool($this->userSession->getUser()->getUID(), $this->appName, $key, $value === 'true');
272283
} else {
273284
return new JSONResponse([], Http::STATUS_FORBIDDEN);
274285
}

apps/settings/tests/Controller/UsersControllerTest.php

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,11 @@
2323
use OCP\AppFramework\Http;
2424
use OCP\AppFramework\Services\IInitialState;
2525
use OCP\BackgroundJob\IJobList;
26+
use OCP\Config\IUserConfig;
2627
use OCP\Encryption\IEncryptionModule;
2728
use OCP\Encryption\IManager;
2829
use OCP\EventDispatcher\IEventDispatcher;
30+
use OCP\IAppConfig;
2931
use OCP\IConfig;
3032
use OCP\IGroupManager;
3133
use OCP\IL10N;
@@ -45,6 +47,8 @@ class UsersControllerTest extends \Test\TestCase {
4547
private UserManager&MockObject $userManager;
4648
private IUserSession&MockObject $userSession;
4749
private IConfig&MockObject $config;
50+
private IAppConfig&MockObject $appConfig;
51+
private IUserConfig&MockObject $userConfig;
4852
private IMailer&MockObject $mailer;
4953
private IFactory&MockObject $l10nFactory;
5054
private IAppManager&MockObject $appManager;
@@ -65,6 +69,8 @@ protected function setUp(): void {
6569
$this->groupManager = $this->createMock(Manager::class);
6670
$this->userSession = $this->createMock(IUserSession::class);
6771
$this->config = $this->createMock(IConfig::class);
72+
$this->appConfig = $this->createMock(IAppConfig::class);
73+
$this->userConfig = $this->createMock(IUserConfig::class);
6874
$this->l = $this->createMock(IL10N::class);
6975
$this->mailer = $this->createMock(IMailer::class);
7076
$this->l10nFactory = $this->createMock(IFactory::class);
@@ -106,6 +112,8 @@ protected function getController(bool $isAdmin = false, array $mockedMethods = [
106112
$this->groupManager,
107113
$this->userSession,
108114
$this->config,
115+
$this->appConfig,
116+
$this->userConfig,
109117
$this->l,
110118
$this->mailer,
111119
$this->l10nFactory,
@@ -128,6 +136,8 @@ protected function getController(bool $isAdmin = false, array $mockedMethods = [
128136
$this->groupManager,
129137
$this->userSession,
130138
$this->config,
139+
$this->appConfig,
140+
$this->userConfig,
131141
$this->l,
132142
$this->mailer,
133143
$this->l10nFactory,
@@ -1000,20 +1010,30 @@ public function testSetPreference(string $key, string $value, bool $isUserValue,
10001010
$this->userSession->method('getUser')->willReturn($user);
10011011

10021012
if ($isAppValue) {
1003-
$this->config->expects($this->once())
1004-
->method('setAppValue')
1005-
->with('core', $key, $value);
1006-
$this->config->expects($this->never())
1007-
->method('setUserValue');
1013+
if ($value === 'true' || $value === 'false' || $value === 'yes' || $value === 'no') {
1014+
$this->appConfig->expects($this->once())
1015+
->method('setValueBool')
1016+
->with('core', $key, $value === 'yes' || $value === 'true');
1017+
} else {
1018+
$this->appConfig->expects($this->once())
1019+
->method('setValueString')
1020+
->with('core', $key, $value);
1021+
}
1022+
$this->userConfig->expects($this->never())
1023+
->method('setValueBool');
10081024
} elseif ($isUserValue) {
1009-
$this->config->expects($this->once())
1010-
->method('setUserValue')
1011-
->with('testUser', 'settings', $key, $value);
1012-
$this->config->expects($this->never())
1013-
->method('setAppValue');
1025+
$this->userConfig->expects($this->once())
1026+
->method('setValueBool')
1027+
->with('testUser', 'settings', $key, $value === 'true');
1028+
$this->appConfig->expects($this->never())
1029+
->method('setValueString');
1030+
$this->appConfig->expects($this->never())
1031+
->method('setValueBool');
10141032
} else {
1015-
$this->config->expects($this->never())->method('setAppValue');
1016-
$this->config->expects($this->never())->method('setUserValue');
1033+
$this->appConfig->expects($this->never())->method('setValueString');
1034+
$this->appConfig->expects($this->never())->method('setValueBool');
1035+
$this->userConfig->expects($this->never())->method('setValueString');
1036+
$this->userConfig->expects($this->never())->method('setValueBool');
10171037
}
10181038

10191039
$response = $controller->setPreference($key, $value);

0 commit comments

Comments
 (0)