From 3188a4a8e0ad15529f52487fc83f3ce011b726cb Mon Sep 17 00:00:00 2001 From: Daniel Kesselberg Date: Wed, 19 Nov 2025 15:32:44 +0100 Subject: [PATCH 1/2] 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 --- .../lib/Controller/AUserDataOCSController.php | 32 +++++++ .../lib/Controller/GroupsController.php | 11 ++- .../lib/Controller/UsersController.php | 8 +- .../lib/ResponseDefinitions.php | 5 + apps/provisioning_api/openapi-full.json | 33 ++++++- apps/provisioning_api/openapi.json | 33 ++++++- .../tests/Controller/GroupsControllerTest.php | 6 +- .../tests/Controller/UsersControllerTest.php | 94 ++++++++++++++++++- apps/settings/src/store/users.js | 26 ++++- apps/settings/src/store/users.spec.ts | 62 ++++++++++++ openapi.json | 33 ++++++- 11 files changed, 326 insertions(+), 17 deletions(-) create mode 100644 apps/settings/src/store/users.spec.ts diff --git a/apps/provisioning_api/lib/Controller/AUserDataOCSController.php b/apps/provisioning_api/lib/Controller/AUserDataOCSController.php index 5f3f474ec7b7b..465188fd9b9e4 100644 --- a/apps/provisioning_api/lib/Controller/AUserDataOCSController.php +++ b/apps/provisioning_api/lib/Controller/AUserDataOCSController.php @@ -8,6 +8,7 @@ */ namespace OCA\Provisioning_API\Controller; +use OC\Group\DisplayNameCache as GroupDisplayNameCache; use OC\Group\Manager as GroupManager; use OC\User\Backend; use OC\User\NoUserException; @@ -35,6 +36,7 @@ /** * @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 { @@ -61,6 +63,7 @@ public function __construct( protected ISubAdmin $subAdminManager, protected IFactory $l10nFactory, protected IRootFolder $rootFolder, + private GroupDisplayNameCache $groupDisplayNameCache, ) { parent::__construct($appName, $request); } @@ -252,6 +255,35 @@ protected function getUserSubAdminGroupsData(string $userId): array { 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 $userDetails + * @return list + */ + 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 diff --git a/apps/provisioning_api/lib/Controller/GroupsController.php b/apps/provisioning_api/lib/Controller/GroupsController.php index 62f639b48e4e4..bd87f56764c3b 100644 --- a/apps/provisioning_api/lib/Controller/GroupsController.php +++ b/apps/provisioning_api/lib/Controller/GroupsController.php @@ -8,6 +8,7 @@ */ 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; @@ -36,6 +37,7 @@ /** * @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 { @@ -51,6 +53,7 @@ public function __construct( IFactory $l10nFactory, IRootFolder $rootFolder, private LoggerInterface $logger, + GroupDisplayNameCache $groupDisplayNameCache, ) { parent::__construct($appName, $request, @@ -62,6 +65,7 @@ public function __construct( $subAdminManager, $l10nFactory, $rootFolder, + $groupDisplayNameCache, ); } @@ -186,7 +190,7 @@ public function getGroupUsers(string $groupId): DataResponse { * @param int|null $limit Limit the amount of groups returned * @param int $offset Offset for searching for groups * - * @return DataResponse}, array{}> + * @return DataResponse, groups: list}, array{}> * @throws OCSException * * 200: Group users details returned @@ -229,7 +233,10 @@ public function getGroupUsersDetails(string $groupId, string $search = '', ?int // 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); diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 12ada1c0213bf..a9974edbff0c1 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -12,6 +12,7 @@ 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; @@ -56,6 +57,7 @@ /** * @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 { @@ -93,6 +95,7 @@ public function __construct( $subAdminManager, $l10nFactory, $rootFolder, + $groupDisplayNameCache, ); $this->l10n = $l10nFactory->get($appName); @@ -146,7 +149,7 @@ public function getUsers(string $search = '', ?int $limit = null, int $offset = * @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}, array{}> + * @return DataResponse, groups: list}, array{}> * * 200: Users details returned */ @@ -198,7 +201,8 @@ public function getUsersDetails(string $search = '', ?int $limit = null, int $of } return new DataResponse([ - 'users' => $usersDetails + 'users' => $usersDetails, + 'groups' => $this->findGroupsWithDisplayname($usersDetails), ]); } diff --git a/apps/provisioning_api/lib/ResponseDefinitions.php b/apps/provisioning_api/lib/ResponseDefinitions.php index 3156da71aa30b..0593b281e695c 100644 --- a/apps/provisioning_api/lib/ResponseDefinitions.php +++ b/apps/provisioning_api/lib/ResponseDefinitions.php @@ -20,6 +20,11 @@ * * @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, * additional_mailScope?: list, diff --git a/apps/provisioning_api/openapi-full.json b/apps/provisioning_api/openapi-full.json index d08d94c653834..2560bf0e99006 100644 --- a/apps/provisioning_api/openapi-full.json +++ b/apps/provisioning_api/openapi-full.json @@ -333,6 +333,21 @@ } } }, + "UserDetailsGroupDisplayname": { + "type": "object", + "required": [ + "id", + "displayname" + ], + "properties": { + "id": { + "type": "string" + }, + "displayname": { + "type": "string" + } + } + }, "UserDetailsQuota": { "type": "object", "properties": { @@ -3526,7 +3541,8 @@ "data": { "type": "object", "required": [ - "users" + "users", + "groups" ], "properties": { "users": { @@ -3549,6 +3565,12 @@ } ] } + }, + "groups": { + "type": "array", + "items": { + "$ref": "#/components/schemas/UserDetailsGroupDisplayname" + } } } } @@ -3991,7 +4013,8 @@ "data": { "type": "object", "required": [ - "users" + "users", + "groups" ], "properties": { "users": { @@ -4014,6 +4037,12 @@ } ] } + }, + "groups": { + "type": "array", + "items": { + "$ref": "#/components/schemas/UserDetailsGroupDisplayname" + } } } } diff --git a/apps/provisioning_api/openapi.json b/apps/provisioning_api/openapi.json index eb973f1dd3d1a..0fa6365306120 100644 --- a/apps/provisioning_api/openapi.json +++ b/apps/provisioning_api/openapi.json @@ -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" + } } } } diff --git a/apps/provisioning_api/tests/Controller/GroupsControllerTest.php b/apps/provisioning_api/tests/Controller/GroupsControllerTest.php index 129853d0c9bd2..8d3c732db1542 100644 --- a/apps/provisioning_api/tests/Controller/GroupsControllerTest.php +++ b/apps/provisioning_api/tests/Controller/GroupsControllerTest.php @@ -7,6 +7,7 @@ */ namespace OCA\Provisioning_API\Tests\Controller; +use OC\Group\DisplayNameCache as GroupDisplayNameCache; use OC\Group\Manager; use OC\User\NoUserException; use OCA\Provisioning_API\Controller\GroupsController; @@ -36,6 +37,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 @@ protected function setUp(): void { $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 @@ protected function setUp(): void { $this->subAdminManager, $this->l10nFactory, $this->rootFolder, - $this->logger + $this->logger, + $this->groupDisplayNameCache, ]) ->onlyMethods(['fillStorageInfo']) ->getMock(); diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php index da3548375ab15..7b0fc4e57ba71 100644 --- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php +++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php @@ -10,6 +10,7 @@ use Exception; use OC\Authentication\Token\RemoteWipe; +use OC\Group\DisplayNameCache as GroupDisplayNameCache; use OC\Group\Manager; use OC\KnownUser\KnownUserService; use OC\PhoneNumberUtil; @@ -88,6 +89,7 @@ protected function setUp(): void { $this->phoneNumberUtil = new PhoneNumberUtil(); $this->appManager = $this->createMock(IAppManager::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)); @@ -210,6 +212,87 @@ public function testGetUsersAsSubAdmin(): void { $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() @@ -1123,18 +1206,23 @@ public function testGetUserDataAsAdmin(): void { ->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'], diff --git a/apps/settings/src/store/users.js b/apps/settings/src/store/users.js index bdbee01f79eb5..9639835543e0d 100644 --- a/apps/settings/src/store/users.js +++ b/apps/settings/src/store/users.js @@ -75,7 +75,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 @@ -311,6 +314,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 = { /** @@ -383,6 +400,7 @@ const actions = { if (usersCount > 0) { context.commit('appendUsers', response.data.ocs.data.users) } + commitGroupsFromUsersResponse(context, response) return usersCount }) .catch((error) => { @@ -400,6 +418,7 @@ const actions = { if (usersCount > 0) { context.commit('appendUsers', response.data.ocs.data.users) } + commitGroupsFromUsersResponse(context, response) return usersCount }) .catch((error) => { @@ -463,8 +482,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 diff --git a/apps/settings/src/store/users.spec.ts b/apps/settings/src/store/users.spec.ts new file mode 100644 index 0000000000000..128fa5c25445c --- /dev/null +++ b/apps/settings/src/store/users.spec.ts @@ -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 }, + ]) + }) +}) diff --git a/openapi.json b/openapi.json index 9a7657759eeee..bd3c0b3596d70 100644 --- a/openapi.json +++ b/openapi.json @@ -3853,6 +3853,21 @@ } } }, + "ProvisioningApiUserDetailsGroupDisplayname": { + "type": "object", + "required": [ + "id", + "displayname" + ], + "properties": { + "id": { + "type": "string" + }, + "displayname": { + "type": "string" + } + } + }, "ProvisioningApiUserDetailsQuota": { "type": "object", "properties": { @@ -30651,7 +30666,8 @@ "data": { "type": "object", "required": [ - "users" + "users", + "groups" ], "properties": { "users": { @@ -30674,6 +30690,12 @@ } ] } + }, + "groups": { + "type": "array", + "items": { + "$ref": "#/components/schemas/ProvisioningApiUserDetailsGroupDisplayname" + } } } } @@ -31116,7 +31138,8 @@ "data": { "type": "object", "required": [ - "users" + "users", + "groups" ], "properties": { "users": { @@ -31139,6 +31162,12 @@ } ] } + }, + "groups": { + "type": "array", + "items": { + "$ref": "#/components/schemas/ProvisioningApiUserDetailsGroupDisplayname" + } } } } From 2da8a1176b3fd222bb80aa68ae0328586c9c3b79 Mon Sep 17 00:00:00 2001 From: Daniel Kesselberg Date: Wed, 1 Jul 2026 17:36:17 +0200 Subject: [PATCH 2/2] chore(assets): Recompile assets Signed-off-by: Daniel Kesselberg