diff --git a/catalog/CHANGELOG.md b/catalog/CHANGELOG.md index 3bef9b40743..cf1a58c62f6 100644 --- a/catalog/CHANGELOG.md +++ b/catalog/CHANGELOG.md @@ -21,6 +21,7 @@ complete sentence without it. ## Changes +- [Fixed] Admin Users: the roles dialog and the Role column are read-only for service users, rather than offering a Save the registry refuses ([#5264](https://github.com/quiltdata/quilt/pull/5264)) - [Fixed] Admin Users: a disabled Enabled or Admin switch says why on hover and on keyboard focus — your own account, a service user managed by the stack, or admin capabilities managed by the SSO configuration — instead of rendering as the same unexplained dead control; the Admin switch now also refuses service users ([#5263](https://github.com/quiltdata/quilt/pull/5263)) - [Fixed] Search sidebar: the facet "Sort by" control no longer disappears while you type in "Find metadata" on stacks with truncated facet lists, and it is withheld when the query matches nothing rather than offering to sort an empty list ([#5262](https://github.com/quiltdata/quilt/pull/5262)) - [Fixed] Search sidebar: the facet "Sort by" control announces what it is — its label used to land on a hidden input, leaving assistive tech to read the control as its current ordering and nothing more ([#5261](https://github.com/quiltdata/quilt/pull/5261)) diff --git a/catalog/app/containers/Admin/UsersAndRoles/Users.spec.tsx b/catalog/app/containers/Admin/UsersAndRoles/Users.spec.tsx index 950a60654f7..18d0b0d626f 100644 --- a/catalog/app/containers/Admin/UsersAndRoles/Users.spec.tsx +++ b/catalog/app/containers/Admin/UsersAndRoles/Users.spec.tsx @@ -2,10 +2,21 @@ import * as React from 'react' import { render, cleanup, fireEvent, screen } from '@testing-library/react' import { describe, it, expect, vi, afterEach } from 'vitest' -import { EditableSwitch, columns } from './Users' +import { EditRoles, EditableSwitch, columns } from './Users' vi.mock('constants/config', () => ({ default: {} })) +// The dialog reaches for a notification channel and a GraphQL mutation on mount; +// stub only those, so the rest of each module still resolves for later tests. +vi.mock('containers/Notifications', async () => ({ + ...(await vi.importActual('containers/Notifications')), + use: () => ({ push: vi.fn() }), +})) +vi.mock('utils/GraphQL', async () => ({ + ...(await vi.importActual('utils/GraphQL')), + useMutation: () => vi.fn(), +})) + describe('containers/Admin/UsersAndRoles/Users', () => { describe('EditableSwitch', () => { afterEach(cleanup) @@ -196,4 +207,86 @@ describe('containers/Admin/UsersAndRoles/Users', () => { expect(container.querySelector('input')?.disabled).toBe(false) }) }) + describe('the Role column', () => { + afterEach(cleanup) + + const column = columns.find((c) => c.id === 'role')! + + function renderRole(user: object) { + return render( + <> + {column.getDisplay!( + undefined, + { extraRoles: [], ...user } as never, + { + roles: [], + defaultRole: null, + openDialog: vi.fn(), + } as never, + )} + , + ) + } + + // Gated on the same reason as the dialog it opens, so both flags matter here. + it.each([ + ['an SSO-managed user', { isRoleAssignmentDisabled: true, isService: false }], + ['a service user', { isRoleAssignmentDisabled: false, isService: true }], + ])('offers only viewing for %s', (_label, user) => { + const { container } = renderRole(user) + expect(container.querySelector('[title]')?.getAttribute('title')).toBe( + 'Click to view', + ) + }) + + it('offers editing for an ordinary user', () => { + const { container } = renderRole({ + isRoleAssignmentDisabled: false, + isService: false, + }) + expect(container.querySelector('[title]')?.getAttribute('title')).toBe( + 'Click to edit', + ) + }) + }) + + describe('EditRoles', () => { + afterEach(cleanup) + + function renderDialog(user: object) { + return render( + , + ) + } + + it.each([ + [ + 'an SSO-managed user', + { isRoleAssignmentDisabled: true, isService: false }, + 'Roles are assigned via role mapping and may be changed in config.', + ], + [ + 'a service user', + { isRoleAssignmentDisabled: false, isService: true }, + 'Roles for this service user are managed by the stack.', + ], + ])('offers no way to save for %s, and says why', (_label, user, why) => { + renderDialog(user) + expect(screen.getByText('Roles assigned to "u"')).toBeDefined() + expect(screen.queryByText('Save')).toBeNull() + expect(screen.getByText('Close')).toBeDefined() + expect(screen.getByText(why)).toBeDefined() + }) + + it('offers a save for an ordinary user', () => { + renderDialog({ isRoleAssignmentDisabled: false, isService: false }) + expect(screen.getByText('Assign roles to "u"')).toBeDefined() + expect(screen.getByText('Save')).toBeDefined() + }) + }) }) diff --git a/catalog/app/containers/Admin/UsersAndRoles/Users.tsx b/catalog/app/containers/Admin/UsersAndRoles/Users.tsx index b226104eded..564a28c8d77 100644 --- a/catalog/app/containers/Admin/UsersAndRoles/Users.tsx +++ b/catalog/app/containers/Admin/UsersAndRoles/Users.tsx @@ -594,7 +594,18 @@ interface EditRolesProps { user: User } -function EditRoles({ close, roles, defaultRole, user }: EditRolesProps) { +// One resolver for the dialog and the Role column that opens it: each derives +// `readOnly` from the reason, so the guard and the copy cannot drift apart. +function whyRoleReadOnly(user: User): 'service' | 'sso' | undefined { + // `isService` before the SSO flag: the registry couples them, but the guard + // must not depend on that. + if (user.isService) return 'service' + if (user.isRoleAssignmentDisabled) return 'sso' + return undefined +} + +// Exported for testing. +export function EditRoles({ close, roles, defaultRole, user }: EditRolesProps) { const { push } = Notifications.use() const setRole = GQL.useMutation(USER_SET_ROLE_MUTATION) @@ -654,6 +665,9 @@ function EditRoles({ close, roles, defaultRole, user }: EditRolesProps) { [user.extraRoles, user.role], ) + const nonAssignableReason = whyRoleReadOnly(user) + const readOnly = nonAssignableReason !== undefined + return ( onSubmit={onSubmit} @@ -672,7 +686,7 @@ function EditRoles({ close, roles, defaultRole, user }: EditRolesProps) { }) => ( <> - {user.isRoleAssignmentDisabled + {readOnly ? `Roles assigned to "${user.name}"` : `Assign roles to "${user.name}"`} @@ -683,8 +697,8 @@ function EditRoles({ close, roles, defaultRole, user }: EditRolesProps) { )} @@ -694,7 +708,7 @@ function EditRoles({ close, roles, defaultRole, user }: EditRolesProps) { - {user.isRoleAssignmentDisabled ? ( + {readOnly ? ( Close @@ -878,8 +892,10 @@ function RoleDisplay({ user, roles, defaultRole, openDialog }: RoleDisplayProps) fullWidth: true, }) + const readOnly = whyRoleReadOnly(user) !== undefined + return ( - + {user.role?.name ?? emptyRole} {user.extraRoles.length > 0 && +{user.extraRoles.length}}