Skip to content

Commit 4e39185

Browse files
kesselbkesselb
committed
fix(settings): show the groups display name for non-loaded groups
By default only 25 group objects are loaded. If a user is assigend to a group, that isn't loaded yet, we only show the gid instead of the displayname. Assisted-by: Copilot:gpt-5.4 Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
1 parent bbbdf47 commit 4e39185

8 files changed

Lines changed: 163 additions & 9 deletions

File tree

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
namespace OCA\Provisioning_API\Controller;
1111

12+
use OC\Group\DisplayNameCache as GroupDisplayNameCache;
1213
use OC\Group\Manager as GroupManager;
1314
use OC\User\Backend;
1415
use OC\User\NoUserException;
@@ -36,6 +37,7 @@
3637

3738
/**
3839
* @psalm-import-type Provisioning_APIUserDetails from ResponseDefinitions
40+
* @psalm-import-type Provisioning_APIUserDetailsGroupDisplayname from ResponseDefinitions
3941
* @psalm-import-type Provisioning_APIUserDetailsQuota from ResponseDefinitions
4042
*/
4143
abstract class AUserDataOCSController extends OCSController {
@@ -62,6 +64,7 @@ public function __construct(
6264
protected ISubAdmin $subAdminManager,
6365
protected IFactory $l10nFactory,
6466
protected IRootFolder $rootFolder,
67+
private GroupDisplayNameCache $groupDisplayNameCache,
6568
) {
6669
parent::__construct($appName, $request);
6770
}
@@ -105,6 +108,7 @@ protected function getUserData(string $userId, bool $includeScopes = false): ?ar
105108
$userAccount = $this->accountManager->getAccount($targetUserObject);
106109
$groups = $this->groupManager->getUserGroups($targetUserObject);
107110
$gids = [];
111+
$gidsDisplayName = [];
108112
foreach ($groups as $group) {
109113
$gids[] = $group->getGID();
110114
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
namespace OCA\Provisioning_API\Controller;
1111

12+
use OC\Group\DisplayNameCache as GroupDisplayNameCache;
1213
use OCA\Provisioning_API\ResponseDefinitions;
1314
use OCA\Settings\Settings\Admin\Sharing;
1415
use OCA\Settings\Settings\Admin\Users;
@@ -52,6 +53,7 @@ public function __construct(
5253
IFactory $l10nFactory,
5354
IRootFolder $rootFolder,
5455
private LoggerInterface $logger,
56+
protected GroupDisplayNameCache $groupDisplayNameCache,
5557
) {
5658
parent::__construct($appName,
5759
$request,
@@ -63,6 +65,7 @@ public function __construct(
6365
$subAdminManager,
6466
$l10nFactory,
6567
$rootFolder,
68+
$this->groupDisplayNameCache,
6669
);
6770
}
6871

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

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
use InvalidArgumentException;
1414
use OC\Authentication\Token\RemoteWipe;
15+
use OC\Group\DisplayNameCache as GroupDisplayNameCache;
1516
use OC\Group\Group;
1617
use OC\KnownUser\KnownUserService;
1718
use OC\User\Backend;
@@ -83,6 +84,7 @@ public function __construct(
8384
private IPhoneNumberUtil $phoneNumberUtil,
8485
private IAppManager $appManager,
8586
private IAppConfig $appConfig,
87+
protected GroupDisplayNameCache $groupDisplayNameCache,
8688
) {
8789
parent::__construct(
8890
$appName,
@@ -95,6 +97,7 @@ public function __construct(
9597
$subAdminManager,
9698
$l10nFactory,
9799
$rootFolder,
100+
$groupDisplayNameCache,
98101
);
99102

100103
$this->l10n = $l10nFactory->get($appName);
@@ -148,7 +151,7 @@ public function getUsers(string $search = '', ?int $limit = null, int $offset =
148151
* @param string $search Text to search for
149152
* @param int|null $limit Limit the amount of groups returned
150153
* @param int $offset Offset for searching for groups
151-
* @return DataResponse<Http::STATUS_OK, array{users: array<string, Provisioning_APIUserDetails|array{id: string}>}, array{}>
154+
* @return DataResponse<Http::STATUS_OK, array{users: array<string, Provisioning_APIUserDetails|array{id: string}>, groups: array<string, Provisioning_APIUserDetailsGroupDisplayname}, array{}>
152155
*
153156
* 200: Users details returned
154157
*/
@@ -200,10 +203,29 @@ public function getUsersDetails(string $search = '', ?int $limit = null, int $of
200203
}
201204

202205
return new DataResponse([
203-
'users' => $usersDetails
206+
'users' => $usersDetails,
207+
'groups' => $this->findGroupsWithDisplayname($usersDetails),
204208
]);
205209
}
206210

211+
private function findGroupsWithDisplayname(array $userDetails): array {
212+
$groupIds = [];
213+
214+
foreach ($userDetails as $userDetail) {
215+
if (isset($userDetail['groups'])) {
216+
array_push($groupIds, ...array_values($userDetail['groups']));
217+
}
218+
}
219+
220+
$groupIds = array_unique($groupIds);
221+
sort($groupIds);
222+
223+
return array_map(function ($groupId) {
224+
$displayname = $this->groupDisplayNameCache->getDisplayName($groupId) ?? $groupId;
225+
return ['id' => $groupId, 'name' => $displayname];
226+
}, $groupIds);
227+
}
228+
207229
/**
208230
* Get the list of disabled users and their details
209231
*

‎apps/provisioning_api/lib/ResponseDefinitions.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,11 @@
2020
*
2121
* @psalm-type Provisioning_APIUserDetailsScope = 'v2-private'|'v2-local'|'v2-federated'|'v2-published'
2222
*
23+
* @psalm-type Provisioning_APIUserDetailsGroupDisplayname = array{
24+
* id: string,
25+
* name: string,
26+
* }
27+
*
2328
* @psalm-type Provisioning_APIUserDetails = array{
2429
* additional_mail: list<string>,
2530
* additional_mailScope?: list<Provisioning_APIUserDetailsScope>,

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
namespace OCA\Provisioning_API\Tests\Controller;
1010

11+
use OC\Group\DisplayNameCache as GroupDisplayNameCache;
1112
use OC\Group\Manager;
1213
use OC\User\NoUserException;
1314
use OCA\Provisioning_API\Controller\GroupsController;
@@ -37,6 +38,7 @@ class GroupsControllerTest extends \Test\TestCase {
3738
protected IFactory&MockObject $l10nFactory;
3839
protected LoggerInterface&MockObject $logger;
3940
protected GroupsController&MockObject $api;
41+
private GroupDisplayNameCache&MockObject $groupDisplayNameCache;
4042

4143
private IRootFolder $rootFolder;
4244

@@ -53,6 +55,7 @@ protected function setUp(): void {
5355
$this->l10nFactory = $this->createMock(IFactory::class);
5456
$this->logger = $this->createMock(LoggerInterface::class);
5557
$this->rootFolder = $this->createMock(IRootFolder::class);
58+
$this->groupDisplayNameCache = $this->createMock(GroupDisplayNameCache::class);
5659

5760
$this->groupManager
5861
->method('getSubAdmin')
@@ -70,7 +73,8 @@ protected function setUp(): void {
7073
$this->subAdminManager,
7174
$this->l10nFactory,
7275
$this->rootFolder,
73-
$this->logger
76+
$this->logger,
77+
$this->groupDisplayNameCache,
7478
])
7579
->onlyMethods(['fillStorageInfo'])
7680
->getMock();

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

Lines changed: 103 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
use Exception;
1212
use OC\Authentication\Token\RemoteWipe;
13+
use OC\Group\DisplayNameCache as GroupDisplayNameCache;
1314
use OC\Group\Manager;
1415
use OC\KnownUser\KnownUserService;
1516
use OC\PhoneNumberUtil;
@@ -70,6 +71,7 @@ class UsersControllerTest extends TestCase {
7071
private IPhoneNumberUtil $phoneNumberUtil;
7172
private IAppManager $appManager;
7273
private IAppConfig&MockObject $appConfig;
74+
private GroupDisplayNameCache&MockObject $groupDisplayNameCache;
7375

7476
protected function setUp(): void {
7577
parent::setUp();
@@ -93,6 +95,7 @@ protected function setUp(): void {
9395
$this->appManager = $this->createMock(IAppManager::class);
9496
$this->appConfig = $this->createMock(IAppConfig::class);
9597
$this->rootFolder = $this->createMock(IRootFolder::class);
98+
$this->groupDisplayNameCache = $this->createMock(GroupDisplayNameCache::class);
9699

97100
$l10n = $this->createMock(IL10N::class);
98101
$l10n->method('t')->willReturnCallback(fn (string $txt, array $replacement = []) => sprintf($txt, ...$replacement));
@@ -120,6 +123,7 @@ protected function setUp(): void {
120123
$this->phoneNumberUtil,
121124
$this->appManager,
122125
$this->appConfig,
126+
$this->groupDisplayNameCache,
123127
])
124128
->onlyMethods(['fillStorageInfo'])
125129
->getMock();
@@ -216,6 +220,87 @@ public function testGetUsersAsSubAdmin(): void {
216220
$this->assertEquals($expected, $this->api->getUsers('MyCustomSearch')->getData());
217221
}
218222

223+
public function testGetUsersDetailsReturnsEmptyGroupsList(): void {
224+
$loggedInUser = $this->getMockBuilder(IUser::class)
225+
->disableOriginalConstructor()
226+
->getMock();
227+
$loggedInUser
228+
->expects($this->once())
229+
->method('getUID')
230+
->willReturn('admin');
231+
232+
$this->userSession
233+
->expects($this->once())
234+
->method('getUser')
235+
->willReturn($loggedInUser);
236+
237+
$this->groupManager
238+
->expects($this->once())
239+
->method('getSubAdmin')
240+
->willReturn($this->subAdminManager);
241+
$this->groupManager
242+
->expects($this->once())
243+
->method('isAdmin')
244+
->with('admin')
245+
->willReturn(true);
246+
$this->groupManager
247+
->expects($this->once())
248+
->method('isDelegatedAdmin')
249+
->with('admin')
250+
->willReturn(false);
251+
252+
$this->userManager
253+
->expects($this->once())
254+
->method('search')
255+
->with('MyCustomSearch', 3, 0)
256+
->willReturn(['UID' => []]);
257+
258+
$api = $this->getMockBuilder(UsersController::class)
259+
->setConstructorArgs([
260+
'provisioning_api',
261+
$this->request,
262+
$this->userManager,
263+
$this->config,
264+
$this->groupManager,
265+
$this->userSession,
266+
$this->accountManager,
267+
$this->subAdminManager,
268+
$this->l10nFactory,
269+
$this->rootFolder,
270+
$this->urlGenerator,
271+
$this->logger,
272+
$this->newUserMailHelper,
273+
$this->secureRandom,
274+
$this->remoteWipe,
275+
$this->knownUserService,
276+
$this->eventDispatcher,
277+
$this->phoneNumberUtil,
278+
$this->appManager,
279+
$this->appConfig,
280+
$this->groupDisplayNameCache,
281+
])
282+
->onlyMethods(['getUserData'])
283+
->getMock();
284+
285+
$api->expects($this->once())
286+
->method('getUserData')
287+
->with('UID')
288+
->willReturn([
289+
'id' => 'UID',
290+
'groups' => [],
291+
]);
292+
293+
$this->assertEquals([
294+
'users' => [
295+
'UID' => [
296+
'id' => 'UID',
297+
'groups' => [],
298+
],
299+
],
300+
'groups' => [],
301+
], $api->getUsersDetails('MyCustomSearch', 3)->getData());
302+
}
303+
219304
private function createUserMock(string $uid, bool $enabled): MockObject&IUser {
220305
$mockUser = $this->getMockBuilder(IUser::class)
221306
->disableOriginalConstructor()
@@ -506,6 +591,7 @@ public function testAddUserSuccessfulWithDisplayName(): void {
506591
$this->phoneNumberUtil,
507592
$this->appManager,
508593
$this->appConfig,
594+
$this->groupDisplayNameCache,
509595
])
510596
->onlyMethods(['editUser'])
511597
->getMock();
@@ -1120,18 +1206,23 @@ public function testGetUserDataAsAdmin(): void {
11201206
->expects($this->once())
11211207
->method('getSubAdminsGroups')
11221208
->willReturn([$group3]);
1123-
$group0->expects($this->once())
1209+
$group0->expects($this->exactly(3))
11241210
->method('getGID')
11251211
->willReturn('group0');
1126-
$group1->expects($this->once())
1212+
$group1->expects($this->exactly(3))
11271213
->method('getGID')
11281214
->willReturn('group1');
1129-
$group2->expects($this->once())
1215+
$group2->expects($this->exactly(3))
11301216
->method('getGID')
11311217
->willReturn('group2');
11321218
$group3->expects($this->once())
11331219
->method('getGID')
11341220
->willReturn('group3');
1221+
$this->groupDisplayNameCache
1222+
->method('getDisplayName')
1223+
->willReturnCallback(function (string $gid): string {
1224+
return ucfirst($gid);
1225+
});
11351226

11361227
$this->mockAccount($targetUser, [
11371228
IAccountManager::PROPERTY_ADDRESS => ['value' => 'address'],
@@ -1233,6 +1324,11 @@ public function testGetUserDataAsAdmin(): void {
12331324
'notify_email' => null,
12341325
'manager' => '',
12351326
'pronouns' => 'they/them',
1327+
'groupsWithDisplayname' => [
1328+
['id' => 'group0', 'name' => 'Group0'],
1329+
['id' => 'group1', 'name' => 'Group1'],
1330+
['id' => 'group2', 'name' => 'Group2'],
1331+
],
12361332
];
12371333
$this->assertEquals($expected, $this->invokePrivate($this->api, 'getUserData', ['UID']));
12381334
}
@@ -1381,6 +1477,7 @@ public function testGetUserDataAsSubAdminAndUserIsAccessible(): void {
13811477
'notify_email' => null,
13821478
'manager' => '',
13831479
'pronouns' => 'they/them',
1480+
'groupsWithDisplayname' => [],
13841481
];
13851482
$this->assertEquals($expected, $this->invokePrivate($this->api, 'getUserData', ['UID']));
13861483
}
@@ -1565,6 +1662,7 @@ public function testGetUserDataAsSubAdminSelfLookup(): void {
15651662
'notify_email' => null,
15661663
'manager' => '',
15671664
'pronouns' => 'they/them',
1665+
'groupsWithDisplayname' => [],
15681666
];
15691667
$this->assertEquals($expected, $this->invokePrivate($this->api, 'getUserData', ['UID']));
15701668
}
@@ -4101,6 +4199,7 @@ public function testGetCurrentUserLoggedIn(): void {
41014199
$this->phoneNumberUtil,
41024200
$this->appManager,
41034201
$this->appConfig,
4202+
$this->groupDisplayNameCache,
41044203
])
41054204
->onlyMethods(['getUserData'])
41064205
->getMock();
@@ -4194,6 +4293,7 @@ public function testGetUser(): void {
41944293
$this->phoneNumberUtil,
41954294
$this->appManager,
41964295
$this->appConfig,
4296+
$this->groupDisplayNameCache,
41974297
])
41984298
->onlyMethods(['getUserData'])
41994299
->getMock();

‎apps/settings/src/components/Users/userFormUtils.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ export function userToFormData(user, allGroups, quotaOptions, serverLanguages) {
8787
displayName: user.displayname ?? '',
8888
password: '',
8989
email: user.email ?? '',
90-
groups,
90+
groups: user.groupsWithDisplayname,
9191
subadminGroups,
9292
quota,
9393
language: resolveLanguage(user, serverLanguages),

0 commit comments

Comments
 (0)