diff --git a/app/components/ListPlusCell.tsx b/app/components/ListPlusCell.tsx new file mode 100644 index 0000000000..a5e1e80914 --- /dev/null +++ b/app/components/ListPlusCell.tsx @@ -0,0 +1,39 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * Copyright Oxide Computer Company + */ + +import React from 'react' + +import { Tooltip } from '~/ui/lib/Tooltip' + +type ListPlusCellProps = { + tooltipTitle: string + children: React.ReactNode +} + +/** + * Gives a count with a tooltip that expands to show details when the user hovers over it + */ +export const ListPlusCell = ({ tooltipTitle, children }: ListPlusCellProps) => { + const [first, ...rest] = React.Children.toArray(children) + const content = ( +
+
{tooltipTitle}
+ {...rest} +
+ ) + return ( +
+ {first} + {rest.length > 0 && ( + +
+{rest.length}
+ + )} +
+ ) +} diff --git a/app/components/RoleBadgeCell.tsx b/app/components/RoleBadgeCell.tsx deleted file mode 100644 index 67cc858bc1..0000000000 --- a/app/components/RoleBadgeCell.tsx +++ /dev/null @@ -1,30 +0,0 @@ -/* - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, you can obtain one at https://mozilla.org/MPL/2.0/. - * - * Copyright Oxide Computer Company - */ -import type { CellContext } from '@tanstack/react-table' - -import type { RoleKey } from '@oxide/api' - -import { Badge } from '~/ui/lib/Badge' - -/** - * Highlight the "effective" role in green, others gray. - * - * Example: User has collab on org and viewer on project. Collab supersedes - * because it is the "stronger" role, i.e., it strictly includes the perms on - * viewer. So collab is highlighted as the "effective" role. - */ -export const RoleBadgeCell = ( - info: CellContext -) => { - const cellRole = info.getValue() - if (!cellRole) return null - const effectiveRole = info.row.original.effectiveRole - return ( - {cellRole} - ) -} diff --git a/app/forms/project-access.tsx b/app/forms/project-access.tsx index 9ac407e749..1f02b7c1f0 100644 --- a/app/forms/project-access.tsx +++ b/app/forms/project-access.tsx @@ -107,7 +107,7 @@ export function ProjectAccessEditUserSideModal({ form={form} formType="edit" resourceName="role" - title={`Change role for ${name}`} + title={`Change project role for ${name}`} onSubmit={({ roleName }) => { updatePolicy.mutate({ path: { project }, diff --git a/app/forms/silo-access.tsx b/app/forms/silo-access.tsx index d1bd8c3f0f..1001d9d5ab 100644 --- a/app/forms/silo-access.tsx +++ b/app/forms/silo-access.tsx @@ -102,7 +102,7 @@ export function SiloAccessEditUserSideModal({ form={form} formType="edit" resourceName="role" - title={`Change role for ${name}`} + title={`Change silo role for ${name}`} onSubmit={({ roleName }) => { updatePolicy.mutate({ body: updateRole({ identityId, identityType, roleName }, policy), diff --git a/app/pages/SiloAccessPage.tsx b/app/pages/SiloAccessPage.tsx index f331a3d877..a9b14f0d09 100644 --- a/app/pages/SiloAccessPage.tsx +++ b/app/pages/SiloAccessPage.tsx @@ -23,7 +23,6 @@ import { import { Access24Icon } from '@oxide/design-system/icons/react' import { HL } from '~/components/HL' -import { RoleBadgeCell } from '~/components/RoleBadgeCell' import { SiloAccessAddUserSideModal, SiloAccessEditUserSideModal, @@ -31,11 +30,12 @@ import { import { confirmDelete } from '~/stores/confirm-delete' import { getActionsCol } from '~/table/columns/action-col' import { Table } from '~/table/Table' +import { Badge } from '~/ui/lib/Badge' import { Button } from '~/ui/lib/Button' import { EmptyMessage } from '~/ui/lib/EmptyMessage' import { PageHeader, PageTitle } from '~/ui/lib/PageHeader' import { TableActions, TableEmptyBox } from '~/ui/lib/Table' -import { accessTypeLabel } from '~/util/access' +import { accessTypeLabel, getBadgeColor } from '~/util/access' import { groupBy, isTruthy } from '~/util/array' const EmptyState = ({ onClick }: { onClick: () => void }) => ( @@ -117,8 +117,11 @@ export function SiloAccessPage() { cell: (props) => accessTypeLabel(props.getValue()), }), colHelper.accessor('siloRole', { - header: 'Silo role', - cell: RoleBadgeCell, + header: 'Role', + cell: (props) => { + const role = props.getValue() + return role ? silo.{role} : null + }, }), // TODO: tooltips on disabled elements explaining why getActionsCol((row: UserRow) => [ diff --git a/app/pages/project/access/ProjectAccessPage.tsx b/app/pages/project/access/ProjectAccessPage.tsx index e81c245d40..3cffabd7ce 100644 --- a/app/pages/project/access/ProjectAccessPage.tsx +++ b/app/pages/project/access/ProjectAccessPage.tsx @@ -14,7 +14,7 @@ import { apiQueryClient, byGroupThenName, deleteRole, - getEffectiveRole, + roleOrder, useApiMutation, useApiQueryClient, usePrefetchedApiQuery, @@ -25,7 +25,7 @@ import { import { Access24Icon } from '@oxide/design-system/icons/react' import { HL } from '~/components/HL' -import { RoleBadgeCell } from '~/components/RoleBadgeCell' +import { ListPlusCell } from '~/components/ListPlusCell' import { ProjectAccessAddUserSideModal, ProjectAccessEditUserSideModal, @@ -34,12 +34,14 @@ import { getProjectSelector, useProjectSelector } from '~/hooks' import { confirmDelete } from '~/stores/confirm-delete' import { getActionsCol } from '~/table/columns/action-col' import { Table } from '~/table/Table' +import { Badge } from '~/ui/lib/Badge' import { Button } from '~/ui/lib/Button' import { EmptyMessage } from '~/ui/lib/EmptyMessage' import { PageHeader, PageTitle } from '~/ui/lib/PageHeader' import { TableActions, TableEmptyBox } from '~/ui/lib/Table' -import { accessTypeLabel } from '~/util/access' -import { groupBy, isTruthy } from '~/util/array' +import { TipIcon } from '~/ui/lib/TipIcon' +import { accessTypeLabel, getBadgeColor } from '~/util/access' +import { groupBy, isTruthy, sortBy } from '~/util/array' const EmptyState = ({ onClick }: { onClick: () => void }) => ( @@ -69,9 +71,8 @@ type UserRow = { id: string identityType: IdentityType name: string - siloRole: RoleKey | undefined projectRole: RoleKey | undefined - effectiveRole: RoleKey + roleBadges: { roleSource: string; roleName: RoleKey }[] } const colHelper = createColumnHelper() @@ -92,26 +93,23 @@ export function ProjectAccessPage() { const rows = useMemo(() => { return groupBy(siloRows.concat(projectRows), (u) => u.id) .map(([userId, userAssignments]) => { - const siloRole = userAssignments.find((a) => a.roleSource === 'silo')?.roleName - const projectRole = userAssignments.find( - (a) => a.roleSource === 'project' - )?.roleName + const { name, identityType } = userAssignments[0] - const roles = [siloRole, projectRole].filter(isTruthy) + const siloAccessRow = userAssignments.find((a) => a.roleSource === 'silo') + const projectAccessRow = userAssignments.find((a) => a.roleSource === 'project') - const { name, identityType } = userAssignments[0] + const roleBadges = sortBy( + [siloAccessRow, projectAccessRow].filter(isTruthy), + (r) => roleOrder[r.roleName] // sorts strongest role first + ) - const row: UserRow = { + return { id: userId, identityType, name, - siloRole, - projectRole, - // we know there has to be at least one - effectiveRole: getEffectiveRole(roles)!, - } - - return row + projectRole: projectAccessRow?.roleName, + roleBadges, + } satisfies UserRow }) .sort(byGroupThenName) }, [siloRows, projectRows]) @@ -132,14 +130,27 @@ export function ProjectAccessPage() { header: 'Type', cell: (props) => accessTypeLabel(props.getValue()), }), - colHelper.accessor('siloRole', { - header: 'Silo role', - cell: RoleBadgeCell, - }), - colHelper.accessor('projectRole', { - header: 'Project role', - cell: RoleBadgeCell, + colHelper.accessor('roleBadges', { + header: () => ( + + Role + + A user or group's effective role for this project is the strongest role + on either the silo or project. + + + ), + cell: (props) => ( + + {props.getValue().map(({ roleName, roleSource }) => ( + + {roleSource}.{roleName} + + ))} + + ), }), + // TODO: tooltips on disabled elements explaining why getActionsCol((row: UserRow) => [ { diff --git a/app/ui/lib/FieldLabel.tsx b/app/ui/lib/FieldLabel.tsx index 2ae97ae5e6..5fb298051c 100644 --- a/app/ui/lib/FieldLabel.tsx +++ b/app/ui/lib/FieldLabel.tsx @@ -8,9 +8,7 @@ import cn from 'classnames' import type { ElementType, PropsWithChildren } from 'react' -import { Question12Icon } from '@oxide/design-system/icons/react' - -import { Tooltip } from '~/ui/lib/Tooltip' +import { TipIcon } from './TipIcon' interface FieldLabelProps { id: string @@ -43,13 +41,7 @@ export const FieldLabel = ({ )} - {tip && ( - - - - )} + {tip && {tip}} ) } diff --git a/app/ui/lib/TipIcon.tsx b/app/ui/lib/TipIcon.tsx new file mode 100644 index 0000000000..c704d54073 --- /dev/null +++ b/app/ui/lib/TipIcon.tsx @@ -0,0 +1,29 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * Copyright Oxide Computer Company + */ +import cn from 'classnames' + +import { Question12Icon } from '@oxide/design-system/icons/react' + +import { Tooltip } from './Tooltip' + +type TipIconProps = { + children: React.ReactNode + className?: string +} +export function TipIcon({ children, className }: TipIconProps) { + return ( + + + + ) +} diff --git a/app/util/access.spec.tsx b/app/util/access.spec.tsx index 2f0ed47884..b819a13625 100644 --- a/app/util/access.spec.tsx +++ b/app/util/access.spec.tsx @@ -7,9 +7,15 @@ */ import { expect, test } from 'vitest' -import { accessTypeLabel } from './access' +import { accessTypeLabel, getBadgeColor } from './access' test('accessTypeLabel', () => { expect(accessTypeLabel('silo_group')).toEqual('Group') expect(accessTypeLabel('silo_user')).toEqual('User') }) + +test('getBadgeColor', () => { + expect(getBadgeColor('admin')).toEqual('default') + expect(getBadgeColor('collaborator')).toEqual('purple') + expect(getBadgeColor('viewer')).toEqual('blue') +}) diff --git a/app/util/access.ts b/app/util/access.ts index 0816af6136..63d9a75d71 100644 --- a/app/util/access.ts +++ b/app/util/access.ts @@ -5,7 +5,18 @@ * * Copyright Oxide Computer Company */ + import type { IdentityType } from '~/api' +import type { BadgeColor } from '~/ui/lib/Badge' export const accessTypeLabel = (identityType: IdentityType) => identityType === 'silo_group' ? 'Group' : 'User' + +export const getBadgeColor = (role: 'admin' | 'collaborator' | 'viewer'): BadgeColor => { + const badgeColor = { + admin: 'default', + collaborator: 'purple', + viewer: 'blue', + } + return badgeColor[role] as BadgeColor +} diff --git a/test/e2e/project-access.e2e.ts b/test/e2e/project-access.e2e.ts index cc177ced09..c08d675b25 100644 --- a/test/e2e/project-access.e2e.ts +++ b/test/e2e/project-access.e2e.ts @@ -13,36 +13,34 @@ test('Click through project access page', async ({ page }) => { await page.goto('/projects/mock-project') await page.click('role=link[name*="Access & IAM"]') - // page is there, we see user 1-3 but not 4 + // page is there, we see user 1 and 3 but not 2 or 4 await expectVisible(page, ['role=heading[name*="Access & IAM"]']) const table = page.locator('table') await expectRowVisible(table, { Name: 'Hannah Arendt', Type: 'User', - 'Silo role': 'admin', - 'Project role': '', + Role: 'silo.admin', }) await expectRowVisible(table, { Name: 'Jacob Klein', Type: 'User', - 'Silo role': '', - 'Project role': 'collaborator', + Role: 'project.collaborator', }) await expectRowVisible(table, { - // no space because expectRowVisible uses textContent, not accessible name Name: 'real-estate-devs', Type: 'Group', - 'Silo role': 'collaborator', + Role: 'silo.collaborator', }) await expectRowVisible(table, { - // no space because expectRowVisible uses textContent, not accessible name Name: 'kernel-devs', Type: 'Group', - 'Silo role': '', - 'Project role': 'viewer', + Role: 'project.viewer', }) - await expectNotVisible(page, [`role=cell[name="${user4.display_name}"]`]) + await expectNotVisible(page, [ + `role=cell[name="Hans Jonas"]`, + `role=cell[name="Simone de Beauvoir"]`, + ]) // Add user 4 as collab await page.click('role=button[name="Add user or group"]') @@ -51,6 +49,7 @@ test('Click through project access page', async ({ page }) => { await page.click('role=button[name*="User or group"]') // only users not already on the project should be visible await expectNotVisible(page, ['role=option[name="Jacob Klein"]']) + await expectVisible(page, [ 'role=option[name="Hannah Arendt"]', 'role=option[name="Hans Jonas"]', @@ -72,7 +71,8 @@ test('Click through project access page', async ({ page }) => { // User 4 shows up in the table await expectRowVisible(table, { Name: 'Simone de Beauvoir', - 'Project role': 'collaborator', + Type: 'User', + Role: 'project.collaborator', }) // now change user 4 role from collab to viewer @@ -82,14 +82,16 @@ test('Click through project access page', async ({ page }) => { .click() await page.click('role=menuitem[name="Change role"]') - await expectVisible(page, ['role=heading[name*="Change role for Simone de Beauvoir"]']) + await expectVisible(page, [ + 'role=heading[name*="Change project role for Simone de Beauvoir"]', + ]) await expectVisible(page, ['button:has-text("Collaborator")']) await page.click('role=button[name*="Role"]') await page.click('role=option[name="Viewer"]') await page.click('role=button[name="Update role"]') - await expectRowVisible(table, { Name: user4.display_name, 'Project role': 'viewer' }) + await expectRowVisible(table, { Name: user4.display_name, Role: 'project.viewer' }) // now delete user 3. has to be 3 or 4 because they're the only ones that come // from the project policy @@ -107,9 +109,10 @@ test('Click through project access page', async ({ page }) => { await page.click('role=button[name*="Role"]') await page.click('role=option[name="Viewer"]') await page.click('role=button[name="Assign role"]') + // because we only show the "effective" role, we should still see the silo admin role, but should now have an additional count value await expectRowVisible(table, { Name: 'Hannah Arendt', - 'Silo role': 'admin', - 'Project role': 'viewer', + Type: 'User', + Role: 'silo.admin+1', }) }) diff --git a/test/e2e/silo-access.e2e.ts b/test/e2e/silo-access.e2e.ts index ec45ede7b7..5db838b65b 100644 --- a/test/e2e/silo-access.e2e.ts +++ b/test/e2e/silo-access.e2e.ts @@ -19,15 +19,14 @@ test('Click through silo access page', async ({ page }) => { await expectVisible(page, ['role=heading[name*="Access & IAM"]']) await expectRowVisible(table, { - // no space because expectRowVisible uses textContent, not accessible name Name: 'real-estate-devs', Type: 'Group', - 'Silo role': 'collaborator', + Role: 'silo.collaborator', }) await expectRowVisible(table, { Name: 'Hannah Arendt', Type: 'User', - 'Silo role': 'admin', + Role: 'silo.admin', }) await expectNotVisible(page, [`role=cell[name="${user4.display_name}"]`]) @@ -59,7 +58,8 @@ test('Click through silo access page', async ({ page }) => { // User 3 shows up in the table await expectRowVisible(table, { Name: 'Jacob Klein', - 'Silo role': 'collaborator', + Role: 'silo.collaborator', + Type: 'User', }) // now change user 3's role from collab to viewer @@ -69,14 +69,14 @@ test('Click through silo access page', async ({ page }) => { .click() await page.click('role=menuitem[name="Change role"]') - await expectVisible(page, ['role=heading[name*="Change role for Jacob Klein"]']) + await expectVisible(page, ['role=heading[name*="Change silo role for Jacob Klein"]']) await expectVisible(page, ['button:has-text("Collaborator")']) await page.click('role=button[name*="Role"]') await page.click('role=option[name="Viewer"]') await page.click('role=button[name="Update role"]') - await expectRowVisible(table, { Name: user3.display_name, 'Silo role': 'viewer' }) + await expectRowVisible(table, { Name: user3.display_name, Role: 'silo.viewer' }) // now delete user 3 const user3Row = page.getByRole('row', { name: user3.display_name, exact: false })