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/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/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', () => { 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 new file mode 100644 index 000000000..d5ae584c6 --- /dev/null +++ b/testing/src/specs/admin-navigation.spec.ts @@ -0,0 +1,56 @@ +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' }); + 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(); + }); +}); 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);