From a8c880a1c81e1dd9216f51de457fdb19a3b04908 Mon Sep 17 00:00:00 2001 From: joshunrau Date: Wed, 4 Nov 2026 16:41:23 -0500 Subject: [PATCH 1/3] fix(web): keep a nav group open on pages beneath its links A nav group collapsed whenever the path stopped exactly matching one of its links, so following a link inside a page, such as from Branding to its Login Page editor, closed the Admin Panel. A link now also counts as active on any path beneath it. Co-Authored-By: Claude Opus 5.5 --- apps/web/src/components/NavGroup/NavGroup.tsx | 4 +++- .../NavGroup/__tests__/NavGroup.test.tsx | 15 +++++++++++++++ testing/src/specs/admin-navigation.spec.ts | 16 ++++++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 testing/src/specs/admin-navigation.spec.ts diff --git a/apps/web/src/components/NavGroup/NavGroup.tsx b/apps/web/src/components/NavGroup/NavGroup.tsx index 2d02d30c3..c0a3f551a 100644 --- a/apps/web/src/components/NavGroup/NavGroup.tsx +++ b/apps/web/src/components/NavGroup/NavGroup.tsx @@ -18,11 +18,13 @@ type NavGroupProps = { onNavigate?: (url: string, search?: { [key: string]: string }) => void; }; +const isAtOrBeneath = (url: string, pathname: string) => pathname === url || pathname.startsWith(`${url}/`); + const isItemActive = (item: NavItem, pathname: string, searchParams: { [key: string]: unknown }): boolean => { if (item.children) { return item.children.some((child) => isItemActive(child, pathname, searchParams)); } - if (item.url !== pathname) { + if (!isAtOrBeneath(item.url!, pathname)) { return false; } if (!item.search) { diff --git a/apps/web/src/components/NavGroup/__tests__/NavGroup.test.tsx b/apps/web/src/components/NavGroup/__tests__/NavGroup.test.tsx index 3a9229eb7..546c782a7 100644 --- a/apps/web/src/components/NavGroup/__tests__/NavGroup.test.tsx +++ b/apps/web/src/components/NavGroup/__tests__/NavGroup.test.tsx @@ -83,6 +83,21 @@ describe('NavGroup', () => { expect(isExpanded()).toBe(false); }); + it('should stay expanded and keep the item active on a page beneath it, so the menu does not close mid-task', () => { + visit('/datahub', {}); + const { rerender } = renderGroup(); + visit('/datahub/subject-1', {}); + rerender(); + expect(isExpanded()).toBe(true); + expect([...navButton('/datahub').classList]).toContain('bg-slate-800'); + }); + + it('should not treat a path that only shares a prefix with an item as beneath it', () => { + visit('/datahub-archive', {}); + renderGroup(); + expect(isExpanded()).toBe(false); + }); + it('should mark an item active only when the current search params match its own', () => { visit('/logs', { status: 'active' }); renderGroup(); diff --git a/testing/src/specs/admin-navigation.spec.ts b/testing/src/specs/admin-navigation.spec.ts new file mode 100644 index 000000000..b9d7e5867 --- /dev/null +++ b/testing/src/specs/admin-navigation.spec.ts @@ -0,0 +1,16 @@ +import { expect, test } from '../support/fixtures'; + +test.describe('admin navigation', () => { + test.use({ actingRole: 'ADMIN' }); + + test('should keep the Admin Panel open on a page beneath one of its links', async ({ getPageModel }) => { + const brandingPage = await getPageModel('/admin/branding'); + const adminPanel = brandingPage.sidebar.getByRole('button', { exact: true, name: 'Admin Panel' }); + await expect(adminPanel).toHaveAttribute('aria-expanded', 'true'); + + await brandingPage.loginPageLink.click(); + await expect(brandingPage.$ref).toHaveURL('/admin/branding/login-page'); + await expect(adminPanel).toHaveAttribute('aria-expanded', 'true'); + await expect(brandingPage.sidebar.getByTestId('nav-button-/admin/branding')).toBeVisible(); + }); +}); From e85f323b95902c6f7327181fc85c83b9bff8ddb4 Mon Sep 17 00:00:00 2001 From: joshunrau Date: Wed, 4 Nov 2026 16:53:02 -0500 Subject: [PATCH 2/3] feat(web): group every admin link under the Admin Panel Manage Groups, Manage Users and Manage Permission Sets sat at the top level of the sidebar, right beside the group manager's Manage Group link and with the same icon. They now live in the Admin Panel, which opens with a direct link to the Audit Logs, followed by three groups: - Access: Groups, Users, Permission Sets - Instruments: Form & Interactive, Series, Repositories - Instance: Settings, Branding, Mail Server The links drop their "Manage" prefix, since the panel already says what they are. Co-Authored-By: Claude Opus 5.5 --- .../PermissionSetChoice.tsx | 6 +- .../src/hooks/__tests__/useNavItems.test.ts | 39 ++++++ apps/web/src/hooks/useNavItems.ts | 111 +++++++++--------- testing/src/pages/_app/route.page.ts | 2 +- testing/src/specs/admin-navigation.spec.ts | 40 +++++++ testing/src/specs/admin-settings.spec.ts | 7 +- 6 files changed, 140 insertions(+), 65 deletions(-) diff --git a/apps/web/src/components/UserPermissionsEditor/PermissionSetChoice.tsx b/apps/web/src/components/UserPermissionsEditor/PermissionSetChoice.tsx index c56c0916e..88f840235 100644 --- a/apps/web/src/components/UserPermissionsEditor/PermissionSetChoice.tsx +++ b/apps/web/src/components/UserPermissionsEditor/PermissionSetChoice.tsx @@ -32,9 +32,9 @@ export const PermissionSetChoice = ({ return (

{t({ - en: 'No permission sets exist yet. An administrator can create one under Manage Permission Sets.', - es: 'Todavía no existe ningún conjunto de permisos. Un administrador puede crear uno en Gestionar conjuntos de permisos.', - fr: "Aucun ensemble d'autorisations n'existe encore. Un administrateur peut en créer un sous Gérer les ensembles d'autorisations." + en: 'No permission sets exist yet. An administrator can create one under Admin Panel > Access > Permission Sets.', + es: 'Todavía no existe ningún conjunto de permisos. Un administrador puede crear uno en Panel de administración > Acceso > Conjuntos de permisos.', + fr: "Aucun ensemble d'autorisations n'existe encore. Un administrateur peut en créer un sous Panneau d'administration > Accès > Ensembles d'autorisations." })}

); diff --git a/apps/web/src/hooks/__tests__/useNavItems.test.ts b/apps/web/src/hooks/__tests__/useNavItems.test.ts index 498e4b43e..20a0e6151 100644 --- a/apps/web/src/hooks/__tests__/useNavItems.test.ts +++ b/apps/web/src/hooks/__tests__/useNavItems.test.ts @@ -206,6 +206,45 @@ describe('useNavItems', () => { expect(urls.indexOf('/admin/permission-sets')).toBe(urls.indexOf('/admin/users') + 1); }); + const adminPanel = () => { + return renderHook(() => useNavItems()) + .result.current.flat() + .find((item) => item.label === 'Admin Panel'); + }; + + it('should open the admin panel with the audit logs, ahead of the access, instruments and instance groups', () => { + expect(adminPanel()?.children?.map((child) => child.label)).toStrictEqual([ + 'Audit Logs', + 'Access', + 'Instruments', + 'Instance' + ]); + }); + + // The audit logs record activity on every entity, not only access changes, so they sit beside the + // groups rather than inside one, one click from the panel. + it('should link the audit logs directly from the admin panel rather than from inside a group', () => { + const auditLogs = adminPanel()?.children?.[0]; + expect(auditLogs?.url).toBe('/admin/audit/logs'); + expect(auditLogs?.children).toBeUndefined(); + }); + + it('should gather the pages that decide who can do what under access', () => { + const access = adminPanel()?.children?.find((child) => child.label === 'Access'); + expect(access?.children?.map((child) => child.url)).toStrictEqual([ + '/admin/groups', + '/admin/users', + '/admin/permission-sets' + ]); + }); + + it('should leave no admin page at the top level of the sidebar, so admin links never mix with clinical ones', () => { + const topLevelUrls = renderHook(() => useNavItems()) + .result.current.flat() + .map((item) => item.url); + expect(topLevelUrls.filter((url) => url?.startsWith('/admin/'))).toStrictEqual([]); + }); + it('should hide the permission sets from anyone short of an administrator', () => { mocks.can.mockImplementation((action, subject) => !(action === 'manage' && subject === 'all')); expect(navUrls()).not.toContain('/admin/permission-sets'); diff --git a/apps/web/src/hooks/useNavItems.ts b/apps/web/src/hooks/useNavItems.ts index 9c8c80080..e4d9abffe 100644 --- a/apps/web/src/hooks/useNavItems.ts +++ b/apps/web/src/hooks/useNavItems.ts @@ -12,12 +12,14 @@ import { FileTextIcon, KeyRoundIcon, LayersIcon, + LockIcon, LogsIcon, MailIcon, PackageIcon, PaletteIcon, PlusIcon, SendIcon, + ServerIcon, ShieldIcon, Trash2Icon, UploadIcon, @@ -166,71 +168,39 @@ export function useNavItems() { } } - if (ability?.can('manage', 'all')) { - globalItems.push({ - icon: UsersIcon, - label: t({ - en: 'Manage Groups', - es: 'Gestionar grupos', - fr: 'Gérer les groupes' - }), - url: '/admin/groups' - }); - globalItems.push({ - icon: UserCogIcon, - label: t({ - en: 'Manage Users', - es: 'Gestionar usuarios', - fr: 'Gérer les utilisateurs' - }), - url: '/admin/users' - }); - globalItems.push({ - icon: KeyRoundIcon, - label: t({ - en: 'Manage Permission Sets', - es: 'Gestionar conjuntos de permisos', - fr: "Gérer les ensembles d'autorisations" - }), - url: '/admin/permission-sets' - }); - } - const adminItems: NavItem[] = []; if (ability?.can('manage', 'all')) { adminItems.push({ children: [ - { - icon: CogIcon, - label: t({ - en: 'App Settings', - es: 'Configuración de la aplicación', - fr: "Paramètres de l'application" - }), - url: '/admin/settings' - }, { icon: LogsIcon, label: t('common.auditLogs'), url: '/admin/audit/logs' }, { - icon: PaletteIcon, - label: t({ - en: 'Branding', - es: 'Imagen de marca', - fr: 'Image de marque' - }), - url: '/admin/branding' - }, - { - icon: PackageIcon, - label: t({ - en: 'Instrument Repos', - es: 'Repositorios de instrumentos', - fr: "Dépôts d'instruments" - }), - url: '/admin/instrument-repos' + children: [ + { + icon: UsersIcon, + label: t({ en: 'Groups', es: 'Grupos', fr: 'Groupes' }), + url: '/admin/groups' + }, + { + icon: UserCogIcon, + label: t({ en: 'Users', es: 'Usuarios', fr: 'Utilisateurs' }), + url: '/admin/users' + }, + { + icon: KeyRoundIcon, + label: t({ + en: 'Permission Sets', + es: 'Conjuntos de permisos', + fr: "Ensembles d'autorisations" + }), + url: '/admin/permission-sets' + } + ], + icon: LockIcon, + label: t({ en: 'Access', es: 'Acceso', fr: 'Accès' }) }, { children: [ @@ -249,15 +219,40 @@ export function useNavItems() { label: t({ en: 'Series', es: 'Series', fr: 'Séries' }), search: { view: 'series' }, url: '/admin/instruments' + }, + { + icon: PackageIcon, + label: t({ en: 'Repositories', es: 'Repositorios', fr: 'Dépôts' }), + url: '/admin/instrument-repos' } ], icon: ClipboardListIcon, label: t({ en: 'Instruments', es: 'Instrumentos', fr: 'Instruments' }) }, { - icon: MailIcon, - label: t({ en: 'Mail', es: 'Correo', fr: 'Courriel' }), - url: '/admin/mail' + children: [ + { + icon: CogIcon, + label: t({ en: 'Settings', es: 'Configuración', fr: 'Paramètres' }), + url: '/admin/settings' + }, + { + icon: PaletteIcon, + label: t({ + en: 'Branding', + es: 'Imagen de marca', + fr: 'Image de marque' + }), + url: '/admin/branding' + }, + { + icon: MailIcon, + label: t({ en: 'Mail Server', es: 'Servidor de correo', fr: 'Serveur de courriel' }), + url: '/admin/mail' + } + ], + icon: ServerIcon, + label: t({ en: 'Instance', es: 'Instancia', fr: 'Instance' }) } ], icon: ShieldIcon, diff --git a/testing/src/pages/_app/route.page.ts b/testing/src/pages/_app/route.page.ts index 907470b26..69a7eb86b 100644 --- a/testing/src/pages/_app/route.page.ts +++ b/testing/src/pages/_app/route.page.ts @@ -19,7 +19,7 @@ export abstract class AppPage extends RootPage { * the setup-state query, and clicking an already-open group would collapse it. */ async expandNavGroup(label: string) { - const groupButton = this.sidebar.getByRole('button', { name: label }); + const groupButton = this.sidebar.getByRole('button', { exact: true, name: label }); await expect(groupButton).toBeVisible(); if ((await groupButton.getAttribute('aria-expanded')) !== 'true') { await groupButton.click(); diff --git a/testing/src/specs/admin-navigation.spec.ts b/testing/src/specs/admin-navigation.spec.ts index b9d7e5867..d5ae584c6 100644 --- a/testing/src/specs/admin-navigation.spec.ts +++ b/testing/src/specs/admin-navigation.spec.ts @@ -1,8 +1,48 @@ import { expect, test } from '../support/fixtures'; +const ADMIN_PANEL_GROUPS = { + Access: ['/admin/groups', '/admin/users', '/admin/permission-sets'], + Instance: ['/admin/settings', '/admin/branding', '/admin/mail'], + Instruments: ['/admin/instrument-repos'] +}; + test.describe('admin navigation', () => { test.use({ actingRole: 'ADMIN' }); + test('should file every admin page under a group in the Admin Panel, leaving none at the top level', async ({ + getPageModel + }) => { + const dashboardPage = await getPageModel('/dashboard'); + await expect(dashboardPage.sidebar.getByRole('button', { exact: true, name: 'Admin Panel' })).toBeVisible(); + await expect(dashboardPage.sidebar.locator('[data-nav-url^="/admin/"]')).toHaveCount(0); + + await dashboardPage.expandNavGroup('Admin Panel'); + for (const [group, urls] of Object.entries(ADMIN_PANEL_GROUPS)) { + await dashboardPage.expandNavGroup(group); + for (const url of urls) { + await expect(dashboardPage.sidebar.getByTestId(`nav-button-${url}`)).toBeVisible(); + } + } + }); + + test('should reach the audit logs straight from the Admin Panel, without opening a group', async ({ + getPageModel + }) => { + const dashboardPage = await getPageModel('/dashboard'); + await dashboardPage.expandNavGroup('Admin Panel'); + await dashboardPage.sidebar.getByTestId('nav-button-/admin/audit/logs').click(); + await expect(dashboardPage.$ref).toHaveURL('/admin/audit/logs'); + }); + + test('should reach the users list through Access in the Admin Panel', async ({ getPageModel }) => { + const dashboardPage = await getPageModel('/dashboard'); + await dashboardPage.expandNavGroup('Admin Panel'); + await dashboardPage.expandNavGroup('Access'); + await dashboardPage.sidebar.getByTestId('nav-button-/admin/users').click(); + await expect(dashboardPage.$ref).toHaveURL('/admin/users'); + await expect(dashboardPage.$ref.getByTestId('page-header')).toContainText('Manage Users'); + }); + test('should keep the Admin Panel open on a page beneath one of its links', async ({ getPageModel }) => { const brandingPage = await getPageModel('/admin/branding'); const adminPanel = brandingPage.sidebar.getByRole('button', { exact: true, name: 'Admin Panel' }); diff --git a/testing/src/specs/admin-settings.spec.ts b/testing/src/specs/admin-settings.spec.ts index c943741c6..36b593e93 100644 --- a/testing/src/specs/admin-settings.spec.ts +++ b/testing/src/specs/admin-settings.spec.ts @@ -137,10 +137,11 @@ test.describe('admin settings', () => { await settingsPage.selectLanguage('Español'); const sidebar = page.getByTestId('sidebar'); - // The group header is a `NavGroup`, not a `NavButton`, so it carries no testid and is matched - // by role; its submenu items are `NavButton`s and are matched by theirs. + // The group headers are `NavGroup`s, not `NavButton`s, so they carry no testid and are matched + // by role; their submenu items are `NavButton`s and are matched by theirs. await expect(sidebar.getByRole('button', { name: 'Panel de administración' })).toBeVisible(); - await expect(sidebar.getByTestId('nav-button-/admin/settings')).toContainText('Configuración de la aplicación'); + await expect(sidebar.getByRole('button', { exact: true, name: 'Instancia' })).toBeVisible(); + await expect(sidebar.getByTestId('nav-button-/admin/settings')).toHaveText('Configuración'); await settingsPage.selectLanguage('English'); const removed = waitForSetupPatch(page); From 4b6f039eb5dda96fd7e129104f2121435177ad64 Mon Sep 17 00:00:00 2001 From: joshunrau Date: Wed, 4 Nov 2026 16:53:20 -0500 Subject: [PATCH 3/3] test(web): match the walkthrough's Don't Show Again label 5fa1391a2 title-cased the walkthrough's dismiss button but left its tests looking for "Don't show again". One of them failed outright, which dropped WalkthroughProvider below the coverage gate on main; the other asserted the button's absence and so passed whatever rendered. Co-Authored-By: Claude Opus 5.5 --- apps/web/src/providers/__tests__/WalkthroughProvider.test.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/web/src/providers/__tests__/WalkthroughProvider.test.tsx b/apps/web/src/providers/__tests__/WalkthroughProvider.test.tsx index 5aedcffe5..8eac32b00 100644 --- a/apps/web/src/providers/__tests__/WalkthroughProvider.test.tsx +++ b/apps/web/src/providers/__tests__/WalkthroughProvider.test.tsx @@ -303,7 +303,7 @@ describe('WalkthroughProvider', () => { it('should mark the walkthrough complete and close when the user asks not to see it again', () => { renderProvider(); - fireEvent.click(screen.getByRole('button', { name: "Don't show again" })); + fireEvent.click(screen.getByRole('button', { name: "Don't Show Again" })); expect(mocks.store.setIsWalkthroughComplete).toHaveBeenCalledWith(true); expect(mocks.store.setIsWalkthroughOpen).toHaveBeenCalledWith(false); }); @@ -326,7 +326,7 @@ describe('WalkthroughProvider', () => { it('should hide the option not to show again on the last step, where done already completes it', () => { renderProvider(); clickNext(STEP_COUNT_WITHOUT_GATEWAY - 1); - expect(screen.queryByRole('button', { name: "Don't show again" })).toBeNull(); + expect(screen.queryByRole('button', { name: "Don't Show Again" })).toBeNull(); }); it('should mark the walkthrough complete and close when the user finishes the last step', () => {