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
Assisted-by: ClaudeCode:claude-sonnet-5

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
pull/56533/head
Daniel Kesselberg 9 months ago
parent fdfd320eff
commit 3cabc7a9ef
No known key found for this signature in database
GPG Key ID: 4A81C29F63464E8F
  1. 32
      apps/provisioning_api/lib/Controller/AUserDataOCSController.php
  2. 11
      apps/provisioning_api/lib/Controller/GroupsController.php
  3. 9
      apps/provisioning_api/lib/Controller/UsersController.php
  4. 5
      apps/provisioning_api/lib/ResponseDefinitions.php
  5. 33
      apps/provisioning_api/openapi-full.json
  6. 33
      apps/provisioning_api/openapi.json
  7. 32
      apps/provisioning_api/tests/Controller/GroupsControllerTest.php
  8. 99
      apps/provisioning_api/tests/Controller/UsersControllerTest.php
  9. 26
      apps/settings/src/store/users.js
  10. 62
      apps/settings/src/store/users.spec.ts
  11. 33
      openapi.json

@ -9,6 +9,7 @@ declare(strict_types=1);
namespace OCA\Provisioning_API\Controller;
use OC\Group\DisplayNameCache as GroupDisplayNameCache;
use OC\Group\Manager as GroupManager;
use OC\User\Backend;
use OCA\Provisioning_API\ResponseDefinitions;
@ -36,6 +37,7 @@ use OCP\Util;
/**
* @psalm-import-type Provisioning_APIUserDetails from ResponseDefinitions
* @psalm-import-type Provisioning_APIUserDetailsGroupDisplayname from ResponseDefinitions
* @psalm-import-type Provisioning_APIUserDetailsQuota from ResponseDefinitions
*/
abstract class AUserDataOCSController extends OCSController {
@ -62,6 +64,7 @@ abstract class AUserDataOCSController extends OCSController {
protected ISubAdmin $subAdminManager,
protected IFactory $l10nFactory,
protected IRootFolder $rootFolder,
private GroupDisplayNameCache $groupDisplayNameCache,
) {
parent::__construct($appName, $request);
}
@ -253,6 +256,35 @@ abstract class AUserDataOCSController extends OCSController {
return $groups;
}
/**
* A full group has id, name, usercount, disabled, canAdd and canRemove. Only
* the displayname is cached; usercount/disabled are not cached. So this only
* returns an {id, displayname} skeleton instead of the full group.
*
* @param array<string, Provisioning_APIUserDetails|array{id: string}> $userDetails
* @return list<Provisioning_APIUserDetailsGroupDisplayname>
*/
protected function findGroupsWithDisplayname(array $userDetails): array {
$groupIds = [];
foreach ($userDetails as $userDetail) {
if (isset($userDetail['groups'])) {
array_push($groupIds, ...array_values($userDetail['groups']));
}
if (isset($userDetail['subadmin'])) {
array_push($groupIds, ...array_values($userDetail['subadmin']));
}
}
$groupIds = array_unique($groupIds);
sort($groupIds);
return array_map(function ($groupId) {
$displayname = $this->groupDisplayNameCache->getDisplayName($groupId) ?? $groupId;
return ['id' => $groupId, 'displayname' => $displayname];
}, $groupIds);
}
/**
* @param IUser $user
* @return Provisioning_APIUserDetailsQuota

@ -9,6 +9,7 @@ declare(strict_types=1);
namespace OCA\Provisioning_API\Controller;
use OC\Group\DisplayNameCache as GroupDisplayNameCache;
use OCA\Provisioning_API\ResponseDefinitions;
use OCA\Settings\Settings\Admin\Sharing;
use OCA\Settings\Settings\Admin\Users;
@ -37,6 +38,7 @@ use Psr\Log\LoggerInterface;
/**
* @psalm-import-type Provisioning_APIGroupDetails from ResponseDefinitions
* @psalm-import-type Provisioning_APIUserDetails from ResponseDefinitions
* @psalm-import-type Provisioning_APIUserDetailsGroupDisplayname from ResponseDefinitions
*/
class GroupsController extends AUserDataOCSController {
@ -52,6 +54,7 @@ class GroupsController extends AUserDataOCSController {
IFactory $l10nFactory,
IRootFolder $rootFolder,
private LoggerInterface $logger,
GroupDisplayNameCache $groupDisplayNameCache,
) {
parent::__construct($appName,
$request,
@ -63,6 +66,7 @@ class GroupsController extends AUserDataOCSController {
$subAdminManager,
$l10nFactory,
$rootFolder,
$groupDisplayNameCache,
);
}
@ -187,7 +191,7 @@ class GroupsController extends AUserDataOCSController {
* @param int|null $limit Limit the amount of groups returned
* @param int $offset Offset for searching for groups
*
* @return DataResponse<Http::STATUS_OK, array{users: array<string, Provisioning_APIUserDetails|array{id: string}>}, array{}>
* @return DataResponse<Http::STATUS_OK, array{users: array<string, Provisioning_APIUserDetails|array{id: string}>, groups: list<Provisioning_APIUserDetailsGroupDisplayname>}, array{}>
* @throws OCSException
*
* 200: Group users details returned
@ -230,7 +234,10 @@ class GroupsController extends AUserDataOCSController {
// continue if a users ceased to exist.
}
}
return new DataResponse(['users' => $usersDetails]);
return new DataResponse([
'users' => $usersDetails,
'groups' => $this->findGroupsWithDisplayname($usersDetails),
]);
}
throw new OCSException('The requested group could not be found', OCSController::RESPOND_NOT_FOUND);

@ -12,6 +12,7 @@ namespace OCA\Provisioning_API\Controller;
use InvalidArgumentException;
use OC\Authentication\Token\RemoteWipe;
use OC\Group\DisplayNameCache as GroupDisplayNameCache;
use OC\Group\Group;
use OC\KnownUser\KnownUserService;
use OC\User\Backend;
@ -57,6 +58,7 @@ use Psr\Log\LoggerInterface;
/**
* @psalm-import-type Provisioning_APIGroupDetails from ResponseDefinitions
* @psalm-import-type Provisioning_APIUserDetails from ResponseDefinitions
* @psalm-import-type Provisioning_APIUserDetailsGroupDisplayname from ResponseDefinitions
*/
class UsersController extends AUserDataOCSController {
@ -83,6 +85,7 @@ class UsersController extends AUserDataOCSController {
private IPhoneNumberUtil $phoneNumberUtil,
private IAppManager $appManager,
private IAppConfig $appConfig,
GroupDisplayNameCache $groupDisplayNameCache,
) {
parent::__construct(
$appName,
@ -95,6 +98,7 @@ class UsersController extends AUserDataOCSController {
$subAdminManager,
$l10nFactory,
$rootFolder,
$groupDisplayNameCache,
);
$this->l10n = $l10nFactory->get($appName);
@ -148,7 +152,7 @@ class UsersController extends AUserDataOCSController {
* @param string $search Text to search for
* @param int|null $limit Limit the amount of groups returned
* @param int $offset Offset for searching for groups
* @return DataResponse<Http::STATUS_OK, array{users: array<string, Provisioning_APIUserDetails|array{id: string}>}, array{}>
* @return DataResponse<Http::STATUS_OK, array{users: array<string, Provisioning_APIUserDetails|array{id: string}>, groups: list<Provisioning_APIUserDetailsGroupDisplayname>}, array{}>
*
* 200: Users details returned
*/
@ -200,7 +204,8 @@ class UsersController extends AUserDataOCSController {
}
return new DataResponse([
'users' => $usersDetails
'users' => $usersDetails,
'groups' => $this->findGroupsWithDisplayname($usersDetails),
]);
}

@ -20,6 +20,11 @@ namespace OCA\Provisioning_API;
*
* @psalm-type Provisioning_APIUserDetailsScope = 'v2-private'|'v2-local'|'v2-federated'|'v2-published'
*
* @psalm-type Provisioning_APIUserDetailsGroupDisplayname = array{
* id: string,
* displayname: string,
* }
*
* @psalm-type Provisioning_APIUserDetails = array{
* additional_mail: list<string>,
* additional_mailScope?: list<Provisioning_APIUserDetailsScope>,

@ -333,6 +333,21 @@
}
}
},
"UserDetailsGroupDisplayname": {
"type": "object",
"required": [
"id",
"displayname"
],
"properties": {
"id": {
"type": "string"
},
"displayname": {
"type": "string"
}
}
},
"UserDetailsQuota": {
"type": "object",
"properties": {
@ -3530,7 +3545,8 @@
"data": {
"type": "object",
"required": [
"users"
"users",
"groups"
],
"properties": {
"users": {
@ -3553,6 +3569,12 @@
}
]
}
},
"groups": {
"type": "array",
"items": {
"$ref": "#/components/schemas/UserDetailsGroupDisplayname"
}
}
}
}
@ -3995,7 +4017,8 @@
"data": {
"type": "object",
"required": [
"users"
"users",
"groups"
],
"properties": {
"users": {
@ -4018,6 +4041,12 @@
}
]
}
},
"groups": {
"type": "array",
"items": {
"$ref": "#/components/schemas/UserDetailsGroupDisplayname"
}
}
}
}

@ -333,6 +333,21 @@
}
}
},
"UserDetailsGroupDisplayname": {
"type": "object",
"required": [
"id",
"displayname"
],
"properties": {
"id": {
"type": "string"
},
"displayname": {
"type": "string"
}
}
},
"UserDetailsQuota": {
"type": "object",
"properties": {
@ -930,7 +945,8 @@
"data": {
"type": "object",
"required": [
"users"
"users",
"groups"
],
"properties": {
"users": {
@ -953,6 +969,12 @@
}
]
}
},
"groups": {
"type": "array",
"items": {
"$ref": "#/components/schemas/UserDetailsGroupDisplayname"
}
}
}
}
@ -1506,7 +1528,8 @@
"data": {
"type": "object",
"required": [
"users"
"users",
"groups"
],
"properties": {
"users": {
@ -1529,6 +1552,12 @@
}
]
}
},
"groups": {
"type": "array",
"items": {
"$ref": "#/components/schemas/UserDetailsGroupDisplayname"
}
}
}
}

@ -8,6 +8,7 @@
namespace OCA\Provisioning_API\Tests\Controller;
use OC\Group\DisplayNameCache as GroupDisplayNameCache;
use OC\Group\Manager;
use OCA\Provisioning_API\Controller\GroupsController;
use OCP\Accounts\IAccountManager;
@ -37,6 +38,7 @@ class GroupsControllerTest extends \Test\TestCase {
protected IFactory&MockObject $l10nFactory;
protected LoggerInterface&MockObject $logger;
protected GroupsController&MockObject $api;
private GroupDisplayNameCache&MockObject $groupDisplayNameCache;
private IRootFolder $rootFolder;
@ -53,6 +55,7 @@ class GroupsControllerTest extends \Test\TestCase {
$this->l10nFactory = $this->createMock(IFactory::class);
$this->logger = $this->createMock(LoggerInterface::class);
$this->rootFolder = $this->createMock(IRootFolder::class);
$this->groupDisplayNameCache = $this->createMock(GroupDisplayNameCache::class);
$this->groupManager
->method('getSubAdmin')
@ -70,7 +73,8 @@ class GroupsControllerTest extends \Test\TestCase {
$this->subAdminManager,
$this->l10nFactory,
$this->rootFolder,
$this->logger
$this->logger,
$this->groupDisplayNameCache,
])
->onlyMethods(['fillStorageInfo'])
->getMock();
@ -490,7 +494,18 @@ class GroupsControllerTest extends \Test\TestCase {
->method('getSubAdminsGroups')
->willReturn([]);
$this->api->getGroupUsersDetails($gid);
$this->groupDisplayNameCache
->method('getDisplayName')
->with('ncg1')
->willReturn('Group One');
$result = $this->api->getGroupUsersDetails($gid);
$data = $result->getData();
$this->assertSame(['ncu1'], array_keys($data['users']));
$this->assertEquals([
['id' => 'ncg1', 'displayname' => 'Group One'],
], $data['groups']);
}
public function testGetGroupUsersDetailsEncoded(): void {
@ -534,6 +549,17 @@ class GroupsControllerTest extends \Test\TestCase {
->method('getSubAdminsGroups')
->willReturn([]);
$this->api->getGroupUsersDetails(urlencode($gid));
$this->groupDisplayNameCache
->method('getDisplayName')
->with('Department A/B C/D')
->willReturn('Department A/B C/D-name');
$result = $this->api->getGroupUsersDetails(urlencode($gid));
$data = $result->getData();
$this->assertSame(['ncu1'], array_keys($data['users']));
$this->assertEquals([
['id' => 'Department A/B C/D', 'displayname' => 'Department A/B C/D-name'],
], $data['groups']);
}
}

@ -10,6 +10,7 @@ namespace OCA\Provisioning_API\Tests\Controller;
use Exception;
use OC\Authentication\Token\RemoteWipe;
use OC\Group\DisplayNameCache as GroupDisplayNameCache;
use OC\Group\Manager;
use OC\KnownUser\KnownUserService;
use OC\PhoneNumberUtil;
@ -70,6 +71,7 @@ class UsersControllerTest extends TestCase {
private IPhoneNumberUtil $phoneNumberUtil;
private IAppManager $appManager;
private IAppConfig&MockObject $appConfig;
private GroupDisplayNameCache&MockObject $groupDisplayNameCache;
protected function setUp(): void {
parent::setUp();
@ -93,6 +95,7 @@ class UsersControllerTest extends TestCase {
$this->appManager = $this->createMock(IAppManager::class);
$this->appConfig = $this->createMock(IAppConfig::class);
$this->rootFolder = $this->createMock(IRootFolder::class);
$this->groupDisplayNameCache = $this->createMock(GroupDisplayNameCache::class);
$l10n = $this->createMock(IL10N::class);
$l10n->method('t')->willReturnCallback(fn (string $txt, array $replacement = []) => sprintf($txt, ...$replacement));
@ -120,6 +123,7 @@ class UsersControllerTest extends TestCase {
$this->phoneNumberUtil,
$this->appManager,
$this->appConfig,
$this->groupDisplayNameCache,
])
->onlyMethods(['fillStorageInfo'])
->getMock();
@ -216,6 +220,87 @@ class UsersControllerTest extends TestCase {
$this->assertEquals($expected, $this->api->getUsers('MyCustomSearch')->getData());
}
public function testGetUsersDetailsReturnsEmptyGroupsList(): void {
$loggedInUser = $this->getMockBuilder(IUser::class)
->disableOriginalConstructor()
->getMock();
$loggedInUser
->expects($this->once())
->method('getUID')
->willReturn('admin');
$this->userSession
->expects($this->once())
->method('getUser')
->willReturn($loggedInUser);
$this->groupManager
->expects($this->once())
->method('getSubAdmin')
->willReturn($this->subAdminManager);
$this->groupManager
->expects($this->once())
->method('isAdmin')
->with('admin')
->willReturn(true);
$this->groupManager
->expects($this->once())
->method('isDelegatedAdmin')
->with('admin')
->willReturn(false);
$this->userManager
->expects($this->once())
->method('search')
->with('MyCustomSearch', 3, 0)
->willReturn(['UID' => []]);
$api = $this->getMockBuilder(UsersController::class)
->setConstructorArgs([
'provisioning_api',
$this->request,
$this->userManager,
$this->config,
$this->groupManager,
$this->userSession,
$this->accountManager,
$this->subAdminManager,
$this->l10nFactory,
$this->rootFolder,
$this->urlGenerator,
$this->logger,
$this->newUserMailHelper,
$this->secureRandom,
$this->remoteWipe,
$this->knownUserService,
$this->eventDispatcher,
$this->phoneNumberUtil,
$this->appManager,
$this->appConfig,
$this->groupDisplayNameCache,
])
->onlyMethods(['getUserData'])
->getMock();
$api->expects($this->once())
->method('getUserData')
->with('UID')
->willReturn([
'id' => 'UID',
'groups' => [],
]);
$this->assertEquals([
'users' => [
'UID' => [
'id' => 'UID',
'groups' => [],
],
],
'groups' => [],
], $api->getUsersDetails('MyCustomSearch', 3)->getData());
}
private function createUserMock(string $uid, bool $enabled): MockObject&IUser {
$mockUser = $this->getMockBuilder(IUser::class)
->disableOriginalConstructor()
@ -506,6 +591,7 @@ class UsersControllerTest extends TestCase {
$this->phoneNumberUtil,
$this->appManager,
$this->appConfig,
$this->groupDisplayNameCache,
])
->onlyMethods(['editUser'])
->getMock();
@ -1120,18 +1206,23 @@ class UsersControllerTest extends TestCase {
->expects($this->once())
->method('getSubAdminsGroups')
->willReturn([$group3]);
$group0->expects($this->once())
$group0->expects($this->exactly(1))
->method('getGID')
->willReturn('group0');
$group1->expects($this->once())
$group1->expects($this->exactly(1))
->method('getGID')
->willReturn('group1');
$group2->expects($this->once())
$group2->expects($this->exactly(1))
->method('getGID')
->willReturn('group2');
$group3->expects($this->once())
->method('getGID')
->willReturn('group3');
$this->groupDisplayNameCache
->method('getDisplayName')
->willReturnCallback(function (string $gid): string {
return ucfirst($gid);
});
$this->mockAccount($targetUser, [
IAccountManager::PROPERTY_ADDRESS => ['value' => 'address'],
@ -4101,6 +4192,7 @@ class UsersControllerTest extends TestCase {
$this->phoneNumberUtil,
$this->appManager,
$this->appConfig,
$this->groupDisplayNameCache,
])
->onlyMethods(['getUserData'])
->getMock();
@ -4194,6 +4286,7 @@ class UsersControllerTest extends TestCase {
$this->phoneNumberUtil,
$this->appManager,
$this->appConfig,
$this->groupDisplayNameCache,
])
->onlyMethods(['getUserData'])
->getMock();

@ -76,7 +76,10 @@ const mutations = {
*/
addGroup(state, newGroup) {
try {
if (typeof state.groups.find((group) => group.id === newGroup.id) !== 'undefined') {
const existingGroup = state.groups.find((group) => group.id === newGroup.id)
if (existingGroup) {
// merge in whatever is provided, e.g. to upgrade a stub group with full details
Object.assign(existingGroup, newGroup)
return
}
// extend group to default values
@ -347,6 +350,20 @@ const getters = {
const CancelToken = axios.CancelToken
let searchRequestCancelSource = null
function commitGroupsFromUsersResponse(context, response) {
const groups = response.data.ocs.data.groups ?? []
if (groups.length === 0) {
return
}
// The response only carries {id, displayname} (see AUserDataOCSController::findGroupsWithDisplayname),
// so addGroup fills usercount/disabled/canAdd/canRemove with defaults. Trade-off: a group
// only known through this path shows a wrong usercount until the sidebar loads full details.
groups.forEach((group) => {
context.commit('addGroup', { id: group.id, name: group.displayname })
})
}
const actions = {
/**
@ -411,6 +428,7 @@ const actions = {
if (usersCount > 0) {
context.commit('appendUsers', response.data.ocs.data.users)
}
commitGroupsFromUsersResponse(context, response)
return usersCount
})
.catch((error) => {
@ -428,6 +446,7 @@ const actions = {
if (usersCount > 0) {
context.commit('appendUsers', response.data.ocs.data.users)
}
commitGroupsFromUsersResponse(context, response)
return usersCount
})
.catch((error) => {
@ -491,8 +510,9 @@ const actions = {
const limitParam = limit === -1 ? '' : `&limit=${limit}`
return api.get(generateOcsUrl('cloud/groups?offset={offset}&search={search}', { offset, search }) + limitParam)
.then((response) => {
if (Object.keys(response.data.ocs.data.groups).length > 0) {
response.data.ocs.data.groups.forEach(function(group) {
const groups = response.data.ocs.data.groups ?? []
if (groups.length > 0) {
groups.forEach(function(group) {
context.commit('addGroup', { id: group, name: group })
})
return true

@ -0,0 +1,62 @@
/**
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
import { describe, expect, it } from 'vitest'
import usersStore from './users.js'
const { mutations } = usersStore
describe('store:users addGroup', () => {
it('inserts a new group filled up with defaults', () => {
const state = { groups: [] }
mutations.addGroup(state, { id: 'group1', name: 'Group One' })
expect(state.groups).toEqual([
{ id: 'group1', name: 'Group One', usercount: 0, disabled: 0, canAdd: true, canRemove: true },
])
})
it('does not duplicate an existing group', () => {
const state = {
groups: [
{ id: 'group1', name: 'Group One', usercount: 5, disabled: 1, canAdd: false, canRemove: false },
],
}
mutations.addGroup(state, { id: 'group1', name: 'Group One' })
expect(state.groups).toHaveLength(1)
})
it('upgrades a stub group once full details are known', () => {
// e.g. committed first with only {id, name} from a users-details response
const state = {
groups: [
{ id: 'group1', name: 'group1', usercount: 0, disabled: 0, canAdd: true, canRemove: true },
],
}
mutations.addGroup(state, { id: 'group1', name: 'Group One', usercount: 5, disabled: 1, canAdd: false, canRemove: false })
expect(state.groups).toEqual([
{ id: 'group1', name: 'Group One', usercount: 5, disabled: 1, canAdd: false, canRemove: false },
])
})
it('does not clobber known details with a later stub commit', () => {
// e.g. the sidebar already loaded full details, then a users-details
// response commits the same group again with only {id, name}
const state = {
groups: [
{ id: 'group1', name: 'Group One', usercount: 5, disabled: 1, canAdd: false, canRemove: false },
],
}
mutations.addGroup(state, { id: 'group1', name: 'Group One' })
expect(state.groups).toEqual([
{ id: 'group1', name: 'Group One', usercount: 5, disabled: 1, canAdd: false, canRemove: false },
])
})
})

@ -3880,6 +3880,21 @@
}
}
},
"ProvisioningApiUserDetailsGroupDisplayname": {
"type": "object",
"required": [
"id",
"displayname"
],
"properties": {
"id": {
"type": "string"
},
"displayname": {
"type": "string"
}
}
},
"ProvisioningApiUserDetailsQuota": {
"type": "object",
"properties": {
@ -32235,7 +32250,8 @@
"data": {
"type": "object",
"required": [
"users"
"users",
"groups"
],
"properties": {
"users": {
@ -32258,6 +32274,12 @@
}
]
}
},
"groups": {
"type": "array",
"items": {
"$ref": "#/components/schemas/ProvisioningApiUserDetailsGroupDisplayname"
}
}
}
}
@ -32700,7 +32722,8 @@
"data": {
"type": "object",
"required": [
"users"
"users",
"groups"
],
"properties": {
"users": {
@ -32723,6 +32746,12 @@
}
]
}
},
"groups": {
"type": "array",
"items": {
"$ref": "#/components/schemas/ProvisioningApiUserDetailsGroupDisplayname"
}
}
}
}

Loading…
Cancel
Save