Skip to content

Commit 8648e4e

Browse files
kesselbkesselb
authored
Merge pull request #56533 from nextcloud/bug/56532/display-in-users-list
Show group display name when the group object is not loaded yet
2 parents 1dcf94e + a0f9134 commit 8648e4e

13 files changed

Lines changed: 359 additions & 22 deletions

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

Lines changed: 32 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 OCA\Provisioning_API\ResponseDefinitions;
@@ -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
}
@@ -253,6 +256,35 @@ protected function getUserSubAdminGroupsData(string $userId): array {
253256
return $groups;
254257
}
255258

259+
/**
260+
* A full group has id, name, usercount, disabled, canAdd and canRemove. Only
261+
* the displayname is cached; usercount/disabled are not cached. So this only
262+
* returns an {id, displayname} skeleton instead of the full group.
263+
*
264+
* @param array<string, Provisioning_APIUserDetails|array{id: string}> $userDetails
265+
* @return list<Provisioning_APIUserDetailsGroupDisplayname>
266+
*/
267+
protected function findGroupsWithDisplayname(array $userDetails): array {
268+
$groupIds = [];
269+
270+
foreach ($userDetails as $userDetail) {
271+
if (isset($userDetail['groups'])) {
272+
array_push($groupIds, ...array_values($userDetail['groups']));
273+
}
274+
if (isset($userDetail['subadmin'])) {
275+
array_push($groupIds, ...array_values($userDetail['subadmin']));
276+
}
277+
}
278+
279+
$groupIds = array_unique($groupIds);
280+
sort($groupIds);
281+
282+
return array_map(function ($groupId) {
283+
$displayname = $this->groupDisplayNameCache->getDisplayName($groupId) ?? $groupId;
284+
return ['id' => $groupId, 'displayname' => $displayname];
285+
}, $groupIds);
286+
}
287+
256288
/**
257289
* @param IUser $user
258290
* @return Provisioning_APIUserDetailsQuota

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

Lines changed: 9 additions & 2 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;
@@ -37,6 +38,7 @@
3738
/**
3839
* @psalm-import-type Provisioning_APIGroupDetails from ResponseDefinitions
3940
* @psalm-import-type Provisioning_APIUserDetails from ResponseDefinitions
41+
* @psalm-import-type Provisioning_APIUserDetailsGroupDisplayname from ResponseDefinitions
4042
*/
4143
class GroupsController extends AUserDataOCSController {
4244

@@ -52,6 +54,7 @@ public function __construct(
5254
IFactory $l10nFactory,
5355
IRootFolder $rootFolder,
5456
private LoggerInterface $logger,
57+
GroupDisplayNameCache $groupDisplayNameCache,
5558
) {
5659
parent::__construct($appName,
5760
$request,
@@ -63,6 +66,7 @@ public function __construct(
6366
$subAdminManager,
6467
$l10nFactory,
6568
$rootFolder,
69+
$groupDisplayNameCache,
6670
);
6771
}
6872

@@ -187,7 +191,7 @@ public function getGroupUsers(string $groupId): DataResponse {
187191
* @param int|null $limit Limit the amount of groups returned
188192
* @param int $offset Offset for searching for groups
189193
*
190-
* @return DataResponse<Http::STATUS_OK, array{users: array<string, Provisioning_APIUserDetails|array{id: string}>}, array{}>
194+
* @return DataResponse<Http::STATUS_OK, array{users: array<string, Provisioning_APIUserDetails|array{id: string}>, groups: list<Provisioning_APIUserDetailsGroupDisplayname>}, array{}>
191195
* @throws OCSException
192196
*
193197
* 200: Group users details returned
@@ -230,7 +234,10 @@ public function getGroupUsersDetails(string $groupId, string $search = '', ?int
230234
// continue if a users ceased to exist.
231235
}
232236
}
233-
return new DataResponse(['users' => $usersDetails]);
237+
return new DataResponse([
238+
'users' => $usersDetails,
239+
'groups' => $this->findGroupsWithDisplayname($usersDetails),
240+
]);
234241
}
235242

236243
throw new OCSException('The requested group could not be found', OCSController::RESPOND_NOT_FOUND);

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

Lines changed: 7 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;
@@ -57,6 +58,7 @@
5758
/**
5859
* @psalm-import-type Provisioning_APIGroupDetails from ResponseDefinitions
5960
* @psalm-import-type Provisioning_APIUserDetails from ResponseDefinitions
61+
* @psalm-import-type Provisioning_APIUserDetailsGroupDisplayname from ResponseDefinitions
6062
*/
6163
class UsersController extends AUserDataOCSController {
6264

@@ -83,6 +85,7 @@ public function __construct(
8385
private IPhoneNumberUtil $phoneNumberUtil,
8486
private IAppManager $appManager,
8587
private IAppConfig $appConfig,
88+
GroupDisplayNameCache $groupDisplayNameCache,
8689
) {
8790
parent::__construct(
8891
$appName,
@@ -95,6 +98,7 @@ public function __construct(
9598
$subAdminManager,
9699
$l10nFactory,
97100
$rootFolder,
101+
$groupDisplayNameCache,
98102
);
99103

100104
$this->l10n = $l10nFactory->get($appName);
@@ -148,7 +152,7 @@ public function getUsers(string $search = '', ?int $limit = null, int $offset =
148152
* @param string $search Text to search for
149153
* @param int|null $limit Limit the amount of groups returned
150154
* @param int $offset Offset for searching for groups
151-
* @return DataResponse<Http::STATUS_OK, array{users: array<string, Provisioning_APIUserDetails|array{id: string}>}, array{}>
155+
* @return DataResponse<Http::STATUS_OK, array{users: array<string, Provisioning_APIUserDetails|array{id: string}>, groups: list<Provisioning_APIUserDetailsGroupDisplayname>}, array{}>
152156
*
153157
* 200: Users details returned
154158
*/
@@ -200,7 +204,8 @@ public function getUsersDetails(string $search = '', ?int $limit = null, int $of
200204
}
201205

202206
return new DataResponse([
203-
'users' => $usersDetails
207+
'users' => $usersDetails,
208+
'groups' => $this->findGroupsWithDisplayname($usersDetails),
204209
]);
205210
}
206211

‎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+
* displayname: string,
26+
* }
27+
*
2328
* @psalm-type Provisioning_APIUserDetails = array{
2429
* additional_mail: list<string>,
2530
* additional_mailScope?: list<Provisioning_APIUserDetailsScope>,

‎apps/provisioning_api/openapi-full.json‎

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,21 @@
333333
}
334334
}
335335
},
336+
"UserDetailsGroupDisplayname": {
337+
"type": "object",
338+
"required": [
339+
"id",
340+
"displayname"
341+
],
342+
"properties": {
343+
"id": {
344+
"type": "string"
345+
},
346+
"displayname": {
347+
"type": "string"
348+
}
349+
}
350+
},
336351
"UserDetailsQuota": {
337352
"type": "object",
338353
"properties": {
@@ -3530,7 +3545,8 @@
35303545
"data": {
35313546
"type": "object",
35323547
"required": [
3533-
"users"
3548+
"users",
3549+
"groups"
35343550
],
35353551
"properties": {
35363552
"users": {
@@ -3553,6 +3569,12 @@
35533569
}
35543570
]
35553571
}
3572+
},
3573+
"groups": {
3574+
"type": "array",
3575+
"items": {
3576+
"$ref": "#/components/schemas/UserDetailsGroupDisplayname"
3577+
}
35563578
}
35573579
}
35583580
}
@@ -3995,7 +4017,8 @@
39954017
"data": {
39964018
"type": "object",
39974019
"required": [
3998-
"users"
4020+
"users",
4021+
"groups"
39994022
],
40004023
"properties": {
40014024
"users": {
@@ -4018,6 +4041,12 @@
40184041
}
40194042
]
40204043
}
4044+
},
4045+
"groups": {
4046+
"type": "array",
4047+
"items": {
4048+
"$ref": "#/components/schemas/UserDetailsGroupDisplayname"
4049+
}
40214050
}
40224051
}
40234052
}

‎apps/provisioning_api/openapi.json‎

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,21 @@
333333
}
334334
}
335335
},
336+
"UserDetailsGroupDisplayname": {
337+
"type": "object",
338+
"required": [
339+
"id",
340+
"displayname"
341+
],
342+
"properties": {
343+
"id": {
344+
"type": "string"
345+
},
346+
"displayname": {
347+
"type": "string"
348+
}
349+
}
350+
},
336351
"UserDetailsQuota": {
337352
"type": "object",
338353
"properties": {
@@ -930,7 +945,8 @@
930945
"data": {
931946
"type": "object",
932947
"required": [
933-
"users"
948+
"users",
949+
"groups"
934950
],
935951
"properties": {
936952
"users": {
@@ -953,6 +969,12 @@
953969
}
954970
]
955971
}
972+
},
973+
"groups": {
974+
"type": "array",
975+
"items": {
976+
"$ref": "#/components/schemas/UserDetailsGroupDisplayname"
977+
}
956978
}
957979
}
958980
}
@@ -1506,7 +1528,8 @@
15061528
"data": {
15071529
"type": "object",
15081530
"required": [
1509-
"users"
1531+
"users",
1532+
"groups"
15101533
],
15111534
"properties": {
15121535
"users": {
@@ -1529,6 +1552,12 @@
15291552
}
15301553
]
15311554
}
1555+
},
1556+
"groups": {
1557+
"type": "array",
1558+
"items": {
1559+
"$ref": "#/components/schemas/UserDetailsGroupDisplayname"
1560+
}
15321561
}
15331562
}
15341563
}

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

Lines changed: 29 additions & 3 deletions
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 OCA\Provisioning_API\Controller\GroupsController;
1314
use OCP\Accounts\IAccountManager;
@@ -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();
@@ -490,7 +494,18 @@ public function testGetGroupUsersDetails(): void {
490494
->method('getSubAdminsGroups')
491495
->willReturn([]);
492496

493-
$this->api->getGroupUsersDetails($gid);
497+
$this->groupDisplayNameCache
498+
->method('getDisplayName')
499+
->with('ncg1')
500+
->willReturn('Group One');
501+
502+
$result = $this->api->getGroupUsersDetails($gid);
503+
504+
$data = $result->getData();
505+
$this->assertSame(['ncu1'], array_keys($data['users']));
506+
$this->assertEquals([
507+
['id' => 'ncg1', 'displayname' => 'Group One'],
508+
], $data['groups']);
494509
}
495510

496511
public function testGetGroupUsersDetailsEncoded(): void {
@@ -534,6 +549,17 @@ public function testGetGroupUsersDetailsEncoded(): void {
534549
->method('getSubAdminsGroups')
535550
->willReturn([]);
536551

537-
$this->api->getGroupUsersDetails(urlencode($gid));
552+
$this->groupDisplayNameCache
553+
->method('getDisplayName')
554+
->with('Department A/B C/D')
555+
->willReturn('Department A/B C/D-name');
556+
557+
$result = $this->api->getGroupUsersDetails(urlencode($gid));
558+
559+
$data = $result->getData();
560+
$this->assertSame(['ncu1'], array_keys($data['users']));
561+
$this->assertEquals([
562+
['id' => 'Department A/B C/D', 'displayname' => 'Department A/B C/D-name'],
563+
], $data['groups']);
538564
}
539565
}

0 commit comments

Comments
 (0)