From c1cf9f94bb6fe421dad608853c8551c957304977 Mon Sep 17 00:00:00 2001 From: fs Date: Wed, 22 Apr 2026 15:49:10 +0200 Subject: [PATCH] feat(core): UserProfileViewService + admin/users detail drawer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extracts the user profile read-view into a core service + shared partial so modules no longer duplicate the assignment/scope logic, and migrates the admin/users list as the drawer's second consumer. Consolidation: - `core/Service/User/UserProfileViewService` owns the single read-path that resolves tenants/departments/roles for a user UUID, enforces scope-aware department visibility, and returns a consistent `{status, user}` shape. - `templates/partials/app-user-profile.phtml` is the shared render template; takes `$profileKey` for storage namespace + lightbox grouping. - `AddressBookService::buildViewContext()` shrinks to a one-line delegation and drops its `UserAssignmentService` dependency. The old `modules/addressbook/templates/address-book-profile.phtml` is removed. admin/users migration: - `pages/admin/users/view-fragment($id).php` + `(none).phtml` use the core service and enforce `ABILITY_ADMIN_USERS_VIEW`. - `admin-users-index.js` replaces the grid-native `linkColumn` on the first name with a formatter that emits `data-drawer-trigger`; `linkColumn` is disabled so the drawer click handler wins. `fullUrl` points at the edit form — drawer → edit is one click. - Drawer labels wired via page config. The new `DetailDrawerFragmentContractTest` validated the users fragment endpoint automatically — no manual test additions were needed. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Container/Registrars/UserRegistrar.php | 8 ++ core/Service/User/UserProfileViewService.php | 136 ++++++++++++++++++ .../AddressBookContainerRegistrar.php | 6 +- .../Service/AddressBookService.php | 102 +------------ .../pages/address-book/view(default).phtml | 3 +- .../address-book/view-fragment(none).phtml | 3 +- .../Service/AddressBookServiceTest.php | 8 +- pages/admin/users/index(default).phtml | 6 + pages/admin/users/view-fragment($id).php | 45 ++++++ pages/admin/users/view-fragment(none).phtml | 6 + .../partials/app-user-profile.phtml | 8 +- .../FrontendRuntimeHostContractTest.php | 2 +- web/js/pages/admin-users-index.js | 34 ++++- 13 files changed, 254 insertions(+), 113 deletions(-) create mode 100644 core/Service/User/UserProfileViewService.php create mode 100644 pages/admin/users/view-fragment($id).php create mode 100644 pages/admin/users/view-fragment(none).phtml rename modules/addressbook/templates/address-book-profile.phtml => templates/partials/app-user-profile.phtml (97%) diff --git a/core/App/Container/Registrars/UserRegistrar.php b/core/App/Container/Registrars/UserRegistrar.php index 02b9b9b..d3de2d6 100644 --- a/core/App/Container/Registrars/UserRegistrar.php +++ b/core/App/Container/Registrars/UserRegistrar.php @@ -22,9 +22,11 @@ use MintyPHP\Service\User\UserLifecycleRestoreService; use MintyPHP\Service\User\UserLifecycleService; use MintyPHP\Service\User\UserPasswordPolicyService; use MintyPHP\Service\User\UserPasswordService; +use MintyPHP\Service\User\UserProfileViewService; use MintyPHP\Service\User\UserRepositoryFactory; use MintyPHP\Service\User\UserServicesFactory; use MintyPHP\Service\User\UserTenantContextService; +use MintyPHP\Service\Tenant\TenantScopeService; final class UserRegistrar implements ContainerRegistrar { @@ -58,5 +60,11 @@ final class UserRegistrar implements ContainerRegistrar $container->set(UserTenantRepository::class, static fn (AppContainer $c): UserTenantRepository => $c->get(UserRepositoryFactory::class)->createUserTenantRepository()); $container->set(UserRoleRepository::class, static fn (AppContainer $c): UserRoleRepository => $c->get(UserRepositoryFactory::class)->createUserRoleRepository()); $container->set(UserDepartmentRepository::class, static fn (AppContainer $c): UserDepartmentRepository => $c->get(UserRepositoryFactory::class)->createUserDepartmentRepository()); + $container->set(UserProfileViewService::class, static fn (AppContainer $c): UserProfileViewService => new UserProfileViewService( + $c->get(UserAccountService::class), + $c->get(UserAssignmentService::class), + $c->get(TenantScopeService::class), + $c->get(UserAvatarService::class) + )); } } diff --git a/core/Service/User/UserProfileViewService.php b/core/Service/User/UserProfileViewService.php new file mode 100644 index 0000000..60ab37e --- /dev/null +++ b/core/Service/User/UserProfileViewService.php @@ -0,0 +1,136 @@ +} + */ + public function buildProfile(int $currentUserId, string $uuid): array + { + $uuid = trim($uuid); + if ($uuid === '') { + return ['status' => 'invalid_uuid']; + } + + $user = $this->userAccountService->findByUuid($uuid); + if (!$user || !isset($user['id'])) { + return ['status' => 'not_found']; + } + + $targetUserId = (int) $user['id']; + if ($targetUserId <= 0) { + return ['status' => 'not_found']; + } + + // Users can always view their own profile; others require scope access. + if ( + $currentUserId !== $targetUserId + && !$this->scopeGateway->canAccess('users', $targetUserId, $currentUserId) + ) { + return ['status' => 'forbidden']; + } + + $viewerTenantIds = $this->normalizeIds($this->scopeGateway->getUserTenantIds($currentUserId)); + $assignments = $this->userAssignmentService->buildAssignmentsForUser($targetUserId); + + $departmentMap = []; + foreach (($assignments['departments'] ?? []) as $department) { + if (empty($department['active'])) { + continue; + } + $tenantId = (int) ($department['tenant_id'] ?? 0); + $label = trim((string) ($department['description'] ?? '')); + // Only show departments in tenants the viewer also belongs to — prevents leaking org structure. + if ($tenantId <= 0 || $label === '' || !in_array($tenantId, $viewerTenantIds, true)) { + continue; + } + $departmentMap[$tenantId] ??= []; + $departmentMap[$tenantId][] = $label; + } + foreach ($departmentMap as $tenantId => $labels) { + $labels = array_values(array_unique($labels)); + natcasesort($labels); + $departmentMap[$tenantId] = array_values($labels); + } + + $primaryTenantId = (int) ($user['primary_tenant_id'] ?? 0); + $tenantGroups = []; + foreach (($assignments['tenants'] ?? []) as $tenant) { + $tenantId = (int) ($tenant['id'] ?? 0); + $tenantLabel = trim((string) ($tenant['description'] ?? '')); + $status = strtolower(trim((string) ($tenant['status'] ?? ''))); + if ( + $tenantId <= 0 + || $tenantLabel === '' + || $status !== 'active' + || !in_array($tenantId, $viewerTenantIds, true) + ) { + continue; + } + $tenantGroups[] = [ + 'label' => $tenantLabel, + 'is_primary' => $primaryTenantId > 0 && $tenantId === $primaryTenantId, + 'departments' => $departmentMap[$tenantId] ?? [], + ]; + } + + usort($tenantGroups, static fn (array $a, array $b): int => strnatcasecmp( + (string) $a['label'], + (string) $b['label'] + )); + + $roleLabels = []; + foreach (($assignments['roles'] ?? []) as $role) { + if (empty($role['active'])) { + continue; + } + $label = trim((string) ($role['description'] ?? '')); + if ($label !== '') { + $roleLabels[] = $label; + } + } + $roleLabels = array_values(array_unique($roleLabels)); + natcasesort($roleLabels); + $roleLabels = array_values($roleLabels); + + $avatarUuid = (string) ($user['uuid'] ?? ''); + $user['has_avatar'] = $avatarUuid !== '' && $this->avatarService->hasAvatar($avatarUuid); + $user['tenant_groups'] = $tenantGroups; + $user['role_labels'] = $roleLabels; + + return [ + 'status' => 'ok', + 'user' => $user, + ]; + } + + /** + * @param array $values + * @return list + */ + private function normalizeIds(array $values): array + { + $ids = array_values(array_unique(array_map('intval', $values))); + return array_values(array_filter($ids, static fn (int $id): bool => $id > 0)); + } +} diff --git a/modules/addressbook/lib/Module/AddressBook/AddressBookContainerRegistrar.php b/modules/addressbook/lib/Module/AddressBook/AddressBookContainerRegistrar.php index 07861b2..846e71d 100644 --- a/modules/addressbook/lib/Module/AddressBook/AddressBookContainerRegistrar.php +++ b/modules/addressbook/lib/Module/AddressBook/AddressBookContainerRegistrar.php @@ -12,8 +12,8 @@ use MintyPHP\Service\Org\DepartmentService; use MintyPHP\Service\Tenant\TenantScopeService; use MintyPHP\Service\Tenant\TenantService; use MintyPHP\Service\User\UserAccountService; -use MintyPHP\Service\User\UserAssignmentService; use MintyPHP\Service\User\UserAvatarService; +use MintyPHP\Service\User\UserProfileViewService; final class AddressBookContainerRegistrar implements ContainerRegistrar { @@ -25,13 +25,13 @@ final class AddressBookContainerRegistrar implements ContainerRegistrar $container->set(AddressBookService::class, static fn (AppContainer $c): AddressBookService => new AddressBookService( $c->get(UserAccountService::class), - $c->get(UserAssignmentService::class), $c->get(TenantService::class), $c->get(DepartmentService::class), $c->get(RoleService::class), $c->get(TenantScopeService::class), $c->get(UserCustomFieldValueService::class), - $c->get(UserAvatarService::class) + $c->get(UserAvatarService::class), + $c->get(UserProfileViewService::class) )); } } diff --git a/modules/addressbook/lib/Module/AddressBook/Service/AddressBookService.php b/modules/addressbook/lib/Module/AddressBook/Service/AddressBookService.php index 14d0c38..28931b4 100644 --- a/modules/addressbook/lib/Module/AddressBook/Service/AddressBookService.php +++ b/modules/addressbook/lib/Module/AddressBook/Service/AddressBookService.php @@ -9,20 +9,20 @@ use MintyPHP\Service\Org\DepartmentService; use MintyPHP\Service\Tenant\TenantScopeService; use MintyPHP\Service\Tenant\TenantService; use MintyPHP\Service\User\UserAccountService; -use MintyPHP\Service\User\UserAssignmentService; use MintyPHP\Service\User\UserAvatarService; +use MintyPHP\Service\User\UserProfileViewService; class AddressBookService { public function __construct( private readonly UserAccountService $userAccountService, - private readonly UserAssignmentService $userAssignmentService, private readonly TenantService $tenantService, private readonly DepartmentService $departmentService, private readonly RoleService $roleService, private readonly TenantScopeService $scopeGateway, private readonly UserCustomFieldValueService $customFieldValueService, - private readonly UserAvatarService $avatarService + private readonly UserAvatarService $avatarService, + private readonly UserProfileViewService $userProfileViewService ) { } @@ -240,101 +240,7 @@ class AddressBookService public function buildViewContext(int $currentUserId, string $uuid): array { - $uuid = trim($uuid); - if ($uuid === '') { - return ['status' => 'invalid_uuid']; - } - - $user = $this->userAccountService->findByUuid($uuid); - if (!$user || !isset($user['id'])) { - return ['status' => 'not_found']; - } - - $targetUserId = (int) $user['id']; - if ($targetUserId <= 0) { - return ['status' => 'not_found']; - } - - // Users can always view their own profile; others require scope access. - if ( - $currentUserId !== $targetUserId - && !$this->scopeGateway->canAccess('users', $targetUserId, $currentUserId) - ) { - return ['status' => 'forbidden']; - } - - $viewerTenantIds = $this->normalizeIds($this->scopeGateway->getUserTenantIds($currentUserId)); - $assignments = $this->userAssignmentService->buildAssignmentsForUser($targetUserId); - - $departmentMap = []; - foreach (($assignments['departments'] ?? []) as $department) { - if (empty($department['active'])) { - continue; - } - $tenantId = (int) ($department['tenant_id'] ?? 0); - $label = trim((string) ($department['description'] ?? '')); - // Only show departments in tenants the viewer also belongs to — prevents leaking org structure. - if ($tenantId <= 0 || $label === '' || !in_array($tenantId, $viewerTenantIds, true)) { - continue; - } - $departmentMap[$tenantId] ??= []; - $departmentMap[$tenantId][] = $label; - } - foreach ($departmentMap as $tenantId => $labels) { - $labels = array_values(array_unique($labels)); - natcasesort($labels); - $departmentMap[$tenantId] = array_values($labels); - } - - $primaryTenantId = (int) ($user['primary_tenant_id'] ?? 0); - $tenantGroups = []; - foreach (($assignments['tenants'] ?? []) as $tenant) { - $tenantId = (int) ($tenant['id'] ?? 0); - $tenantLabel = trim((string) ($tenant['description'] ?? '')); - $status = strtolower(trim((string) ($tenant['status'] ?? ''))); - if ( - $tenantId <= 0 - || $tenantLabel === '' - || $status !== 'active' - || !in_array($tenantId, $viewerTenantIds, true) - ) { - continue; - } - $tenantGroups[] = [ - 'label' => $tenantLabel, - 'is_primary' => $primaryTenantId > 0 && $tenantId === $primaryTenantId, - 'departments' => $departmentMap[$tenantId] ?? [], - ]; - } - - usort($tenantGroups, static fn (array $a, array $b): int => strnatcasecmp( - (string) $a['label'], - (string) $b['label'] - )); - - $roleLabels = []; - foreach (($assignments['roles'] ?? []) as $role) { - if (empty($role['active'])) { - continue; - } - $label = trim((string) ($role['description'] ?? '')); - if ($label !== '') { - $roleLabels[] = $label; - } - } - $roleLabels = array_values(array_unique($roleLabels)); - natcasesort($roleLabels); - $roleLabels = array_values($roleLabels); - - $avatarUuid = (string) ($user['uuid'] ?? ''); - $user['has_avatar'] = $avatarUuid !== '' && $this->avatarService->hasAvatar($avatarUuid); - $user['tenant_groups'] = $tenantGroups; - $user['role_labels'] = $roleLabels; - - return [ - 'status' => 'ok', - 'user' => $user, - ]; + return $this->userProfileViewService->buildProfile($currentUserId, $uuid); } /** diff --git a/modules/addressbook/pages/address-book/view(default).phtml b/modules/addressbook/pages/address-book/view(default).phtml index 6c89226..ed92f61 100644 --- a/modules/addressbook/pages/address-book/view(default).phtml +++ b/modules/addressbook/pages/address-book/view(default).phtml @@ -1,2 +1,3 @@ createMock(UserAccountService::class); - $userAssignmentService = $this->createMock(UserAssignmentService::class); $tenantService = $this->createMock(TenantService::class); $departmentService = $this->createMock(DepartmentService::class); $roleService = $this->createMock(RoleService::class); $scopeGateway = $this->createMock(TenantScopeService::class); $customFieldGateway = $this->createMock(UserCustomFieldValueService::class); $avatarService = $this->createMock(UserAvatarService::class); + $userProfileViewService = $this->createMock(UserProfileViewService::class); $scopeGateway ->expects($this->once()) @@ -67,13 +67,13 @@ class AddressBookServiceTest extends TestCase ]); $service = new AddressBookService( $userAccountService, - $userAssignmentService, $tenantService, $departmentService, $roleService, $scopeGateway, $customFieldGateway, - $avatarService + $avatarService, + $userProfileViewService ); $context = $service->buildIndexContext(7, []); diff --git a/pages/admin/users/index(default).phtml b/pages/admin/users/index(default).phtml index a50b308..4f3213e 100644 --- a/pages/admin/users/index(default).phtml +++ b/pages/admin/users/index(default).phtml @@ -141,6 +141,12 @@ $pageConfig = [ 'bulkSendAccessConfirm' => t('Send access emails to selected users?'), 'bulkAccessPdfConfirm' => t('Generate access PDFs for selected users?'), 'primaryTenant' => t('Primary tenant'), + 'drawerClose' => t('Close'), + 'drawerPrev' => t('Previous'), + 'drawerNext' => t('Next'), + 'drawerOpenFull' => t('Open full page'), + 'drawerLoading' => t('Loading'), + 'drawerError' => t('Failed to load'), 'bulkMessages' => [ 'activate' => [ 'success' => t('%d users activated'), diff --git a/pages/admin/users/view-fragment($id).php b/pages/admin/users/view-fragment($id).php new file mode 100644 index 0000000..724b781 --- /dev/null +++ b/pages/admin/users/view-fragment($id).php @@ -0,0 +1,45 @@ +all(); +Guard::requireLogin(); + +$currentUserId = (int) ($session['user']['id'] ?? 0); +$uuid = trim((string) ($id ?? '')); +if ($uuid === '') { + http_response_code(400); + return; +} + +$decision = app(AuthorizationService::class)->authorize(UserAuthorizationPolicy::ABILITY_ADMIN_USERS_VIEW, [ + 'actor_user_id' => $currentUserId, +]); +if (!$decision->isAllowed()) { + http_response_code(403); + $user = null; + return; +} + +$profile = app(UserProfileViewService::class)->buildProfile($currentUserId, $uuid); +$status = (string) ($profile['status'] ?? ''); +if ($status === 'not_found' || $status === 'invalid_uuid') { + http_response_code(404); + $user = null; + return; +} +if ($status === 'forbidden') { + http_response_code(403); + $user = null; + return; +} +$user = $profile['user'] ?? null; +if (!is_array($user)) { + http_response_code(404); + $user = null; + return; +} diff --git a/pages/admin/users/view-fragment(none).phtml b/pages/admin/users/view-fragment(none).phtml new file mode 100644 index 0000000..9db0481 --- /dev/null +++ b/pages/admin/users/view-fragment(none).phtml @@ -0,0 +1,6 @@ +
- @@ -91,7 +95,7 @@ if ($profileDescription !== '') {
-
+
diff --git a/tests/Architecture/FrontendRuntimeHostContractTest.php b/tests/Architecture/FrontendRuntimeHostContractTest.php index 236a04e..6776782 100644 --- a/tests/Architecture/FrontendRuntimeHostContractTest.php +++ b/tests/Architecture/FrontendRuntimeHostContractTest.php @@ -41,7 +41,7 @@ class FrontendRuntimeHostContractTest extends TestCase public function testTabsHostsUseDataAppComponentContract(): void { $files = [ - 'modules/addressbook/templates/address-book-profile.phtml', + 'templates/partials/app-user-profile.phtml', 'pages/admin/departments/_form.phtml', 'pages/admin/permissions/_form.phtml', 'pages/admin/roles/_form.phtml', diff --git a/web/js/pages/admin-users-index.js b/web/js/pages/admin-users-index.js index 783151b..409d53d 100644 --- a/web/js/pages/admin-users-index.js +++ b/web/js/pages/admin-users-index.js @@ -1,7 +1,8 @@ import { createListPageModule } from '../core/app-list-page-module.js'; import { initUsersListPage } from './app-users-list.js'; import { bindBulkVisibility } from '../components/app-bulk-selection.js'; -import { buildUrl, badgeHtml, escapeHtml, buildBadgeList } from './app-list-utils.js'; +import { initDetailDrawer } from '../components/app-detail-drawer.js'; +import { buildUrl, badgeHtml, escapeHtml, buildBadgeList, withCurrentListReturn } from './app-list-utils.js'; import { showAsyncFlash, withLoading } from '../components/app-async-flash.js'; createListPageModule({ @@ -61,7 +62,17 @@ createListPageModule({ }, }, { name: 'UUID', hidden: true }, - { name: labels.firstName || 'First name', sort: true }, + { + name: labels.firstName || 'First name', + sort: true, + formatter: (cell, row) => { + const name = escapeHtml(String(cell || '').trim() || '-'); + const uuid = row?.cells?.[uuidIndex]?.data; + if (!uuid) { return gridjs.html(`${name}`); } + const editUrl = escapeHtml(withCurrentListReturn(new URL(`admin/users/edit/${uuid}`, appBase).toString())); + return gridjs.html(`${name}`); + }, + }, { name: labels.lastName || 'Last name', sort: true }, { name: labels.email || 'Email', sort: true }, { @@ -140,7 +151,7 @@ createListPageModule({ rowDataset: (row) => ({ uuid: row?.cells?.[uuidIndex]?.data, }), - rowInteraction: { linkColumn: 2 }, + rowInteraction: { linkColumn: false }, rowDblClick: { getUrl: (rowData) => { if (!canEditRow(rowData)) { @@ -190,6 +201,23 @@ createListPageModule({ buttonSelector: '[data-users-bulk]', }); + initDetailDrawer({ + gridConfig, + triggerSelector: '[data-drawer-trigger]', + rowUuidAttr: 'uuid', + fetchUrl: (uuid) => new URL(`admin/users/view-fragment/${uuid}`, appBase).toString(), + fullUrl: (uuid) => withCurrentListReturn(new URL(`admin/users/edit/${uuid}`, appBase).toString()), + hashPrefix: 'user', + labels: { + close: labels.drawerClose || 'Close', + previous: labels.drawerPrev || 'Previous', + next: labels.drawerNext || 'Next', + openFull: labels.drawerOpenFull || 'Open full page', + loading: labels.drawerLoading || 'Loading', + error: labels.drawerError || 'Failed to load', + }, + }); + return usersGridConfig; }, });