Catalog: stop offering a refused Save for service users (5217 stack 6/9)

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))

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(
+        <EditRoles
+          close={vi.fn()}
+          roles={[]}
+          defaultRole={null}
+          user={{ name: 'u', extraRoles: [], role: null, ...user } as never}
+        />,
+      )
+    }
+
+    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()
+    })
+  })
 })

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 (
     <RF.Form<FormValues>
       onSubmit={onSubmit}
@@ -672,7 +686,7 @@ function EditRoles({ close, roles, defaultRole, user }: EditRolesProps) {
       }) => (
         <>
           <M.DialogTitle>
-            {user.isRoleAssignmentDisabled
+            {readOnly
               ? `Roles assigned to "${user.name}"`
               : `Assign roles to "${user.name}"`}
           </M.DialogTitle>
@@ -683,8 +697,8 @@ function EditRoles({ close, roles, defaultRole, user }: EditRolesProps) {
                   <RoleSelect.RoleSelect
                     roles={roles}
                     defaultRole={defaultRole}
-                    nonAssignable={user.isRoleAssignmentDisabled}
-                    nonAssignableReason={user.isService ? 'service' : 'sso'}
+                    nonAssignable={readOnly}
+                    nonAssignableReason={nonAssignableReason}
                     {...props}
                   />
                 )}
@@ -694,7 +708,7 @@ function EditRoles({ close, roles, defaultRole, user }: EditRolesProps) {
               </Form.FormErrorAuto>
             </DialogForm>
           </M.DialogContent>
-          {user.isRoleAssignmentDisabled ? (
+          {readOnly ? (
             <M.DialogActions>
               <M.Button color="primary" onClick={close} variant="contained">
                 Close
@@ -878,8 +892,10 @@ function RoleDisplay({ user, roles, defaultRole, openDialog }: RoleDisplayProps)
       fullWidth: true,
     })
 
+  const readOnly = whyRoleReadOnly(user) !== undefined
+
   return (
-    <M.Tooltip title={user.isRoleAssignmentDisabled ? 'Click to view' : 'Click to edit'}>
+    <M.Tooltip title={readOnly ? 'Click to view' : 'Click to edit'}>
       <Clickable onClick={edit}>
         {user.role?.name ?? emptyRole}
         {user.extraRoles.length > 0 && <Hint> +{user.extraRoles.length}</Hint>}