Display full sled policy in table (#2624)

* make sled policy column display the right data

* use beautiful column grouping

* fix the e2es

* make display more like API data

Author: david-crespo

app/pages/system/UtilizationPage.tsx

@@ -162,7 +162,7 @@ function UsageTab() {
     <Table className="w-full">
       <Table.Header>
         <Table.HeaderRow>
-          <Table.HeadCell>Silo</Table.HeadCell>
+          <Table.HeadCell data-test-ignore></Table.HeadCell>
           {/* data-test-ignore makes the row asserts work in the e2e tests */}
           <Table.HeadCell colSpan={3} data-test-ignore>
             Provisioned / Quota
@@ -173,7 +173,7 @@ function UsageTab() {
           <Table.HeadCell data-test-ignore></Table.HeadCell>
         </Table.HeaderRow>
         <Table.HeaderRow>
-          <Table.HeadCell data-test-ignore></Table.HeadCell>
+          <Table.HeadCell>Silo</Table.HeadCell>
           <Table.HeadCell>CPU</Table.HeadCell>
           <Table.HeadCell>Memory</Table.HeadCell>
           <Table.HeadCell>Storage</Table.HeadCell>

app/pages/system/inventory/SledsTab.tsx

@@ -16,32 +16,18 @@ import {
 } from '@oxide/api'
 import { Servers24Icon } from '@oxide/design-system/icons/react'
 
+import { EmptyCell } from '~/table/cells/EmptyCell'
 import { makeLinkCell } from '~/table/cells/LinkCell'
 import { useQueryTable } from '~/table/QueryTable'
 import { Badge, type BadgeColor } from '~/ui/lib/Badge'
 import { EmptyMessage } from '~/ui/lib/EmptyMessage'
 import { pb } from '~/util/path-builder'
 
-const POLICY_KIND_BADGE_COLORS: Record<SledPolicy['kind'], BadgeColor> = {
-  in_service: 'default',
-  expunged: 'neutral',
-}
-
 const STATE_BADGE_COLORS: Record<SledState, BadgeColor> = {
   active: 'default',
   decommissioned: 'neutral',
 }
 
-const EmptyState = () => {
-  return (
-    <EmptyMessage
-      icon={<Servers24Icon />}
-      title="Something went wrong"
-      body="We expected some racks here, but none were found"
-    />
-  )
-}
-
 const sledList = getListQFn('sledList', {})
 
 export async function loader() {
@@ -55,16 +41,45 @@ const staticCols = [
     cell: makeLinkCell((sledId) => pb.sled({ sledId })),
   }),
   // TODO: colHelper.accessor('baseboard.serviceAddress', { header: 'service address' }),
-  colHelper.accessor('baseboard.part', { header: 'part number' }),
-  colHelper.accessor('baseboard.serial', { header: 'serial number' }),
-  colHelper.accessor('baseboard.revision', { header: 'revision' }),
-  colHelper.accessor('policy.kind', {
-    header: 'policy',
-    cell: (info) => (
-      <Badge color={POLICY_KIND_BADGE_COLORS[info.getValue()]}>
-        {info.getValue().replace(/_/g, ' ')}
-      </Badge>
-    ),
+  colHelper.group({
+    id: 'baseboard',
+    header: 'Baseboard',
+    columns: [
+      colHelper.accessor('baseboard.part', { header: 'part number' }),
+      colHelper.accessor('baseboard.serial', { header: 'serial number' }),
+      colHelper.accessor('baseboard.revision', { header: 'revision' }),
+    ],
+  }),
+  colHelper.group({
+    id: 'policy',
+    header: 'Policy',
+    columns: [
+      colHelper.accessor('policy', {
+        header: 'Kind',
+        cell: (info) => {
+          // need to cast because inference is broken inside groups
+          // https://github.com/TanStack/table/issues/5065
+          const policy: SledPolicy = info.getValue()
+          return policy.kind === 'expunged' ? (
+            <Badge color="neutral">Expunged</Badge>
+          ) : (
+            <Badge>In service</Badge>
+          )
+        },
+      }),
+      colHelper.accessor('policy', {
+        header: 'Provision policy',
+        cell: (info) => {
+          const policy: SledPolicy = info.getValue()
+          if (policy.kind === 'expunged') return <EmptyCell />
+          return policy.provisionPolicy === 'provisionable' ? (
+            <Badge>Provisionable</Badge>
+          ) : (
+            <Badge color="neutral">Not provisionable</Badge>
+          )
+        },
+      }),
+    ],
   }),
   colHelper.accessor('state', {
     cell: (info) => (
@@ -75,7 +90,7 @@ const staticCols = [
 
 Component.displayName = 'SledsTab'
 export function Component() {
-  const emptyState = <EmptyState />
+  const emptyState = <EmptyMessage icon={<Servers24Icon />} title="No sleds found" />
   const { table } = useQueryTable({ query: sledList, columns: staticCols, emptyState })
   return table
 }

app/table/Table.tsx

@@ -29,18 +29,30 @@ export const Table = <TData,>({
 }: TableProps<TData>) => (
   <UITable {...tableProps}>
     <UITable.Header>
-      {table.getHeaderGroups().map((headerGroup) => (
-        <UITable.HeaderRow key={headerGroup.id}>
-          {headerGroup.headers.map((header) => (
-            <UITable.HeadCell
-              key={header.id}
-              className={header.column.columnDef.meta?.thClassName}
-            >
-              {flexRender(header.column.columnDef.header, header.getContext())}
-            </UITable.HeadCell>
-          ))}
-        </UITable.HeaderRow>
-      ))}
+      {table.getHeaderGroups().map((headerGroup) => {
+        console.log(headerGroup)
+        return (
+          <UITable.HeaderRow key={headerGroup.id}>
+            {headerGroup.headers.map((header) => (
+              <UITable.HeadCell
+                key={header.id}
+                className={header.column.columnDef.meta?.thClassName}
+                colSpan={header.colSpan}
+              >
+                {
+                  // Placeholder concept is for when grouped columns are
+                  // combined with regular columns. The regular column only
+                  // needs one entry in the stack of header cells, so the others
+                  // have isPlacholder=true. See sleds table for an example.
+                  header.isPlaceholder
+                    ? null
+                    : flexRender(header.column.columnDef.header, header.getContext())
+                }
+              </UITable.HeadCell>
+            ))}
+          </UITable.HeaderRow>
+        )
+      })}
     </UITable.Header>
     <UITable.Body>
       {table.getRowModel().rows.map((row) => {

mock-api/sled.ts

@@ -35,7 +35,7 @@ export const sleds: Json<Sled[]> = [
     rack_id: '759a1c80-4bff-4d0b-97ce-b482ca936724',
     policy: {
       kind: 'in_service',
-      provision_policy: 'provisionable',
+      provision_policy: 'non_provisionable',
     },
     state: 'active',
     baseboard: {

test/e2e/instance-create.e2e.ts

@@ -474,12 +474,13 @@ test('attaches a floating IP; disables button when no IPs available', async ({ p
   await page.getByRole('tab', { name: 'Networking' }).click()
 
   // ensure External IPs table has rows for the Ephemeral IP and the Floating IP
-  await expectRowVisible(page.getByRole('table'), {
+  const ipsTable = page.getByRole('table', { name: 'External IPs' })
+  await expectRowVisible(ipsTable, {
     ip: '123.4.56.0',
     Kind: 'ephemeral',
     name: '—',
   })
-  await expectRowVisible(page.getByRole('table'), {
+  await expectRowVisible(ipsTable, {
     ip: floatingIp.ip,
     Kind: 'floating',
     name: floatingIp.name,

test/e2e/inventory.e2e.ts

@@ -20,29 +20,40 @@ test('Sled inventory page', async ({ page }) => {
   await expect(sledsTab).toHaveClass(/is-selected/)
 
   const sledsTable = page.getByRole('table')
+  // expectRowVisible currently only looks at the last header row in case of
+  // grouping, hence the slightly weird column names
+  await expectRowVisible(sledsTable, {
+    id: sleds[0].id,
+    'serial number': sleds[0].baseboard.serial,
+    Kind: 'In service',
+    'Provision policy': 'Provisionable',
+    state: 'active',
+  })
   await expectRowVisible(sledsTable, {
     id: sleds[1].id,
     'serial number': sleds[1].baseboard.serial,
-    policy: 'in service',
+    Kind: 'In service',
+    'Provision policy': 'Not provisionable',
     state: 'active',
   })
   await expectRowVisible(sledsTable, {
     id: sleds[2].id,
     'serial number': sleds[2].baseboard.serial,
-    policy: 'expunged',
+    Kind: 'Expunged',
+    'Provision policy': '—',
     state: 'active',
   })
   await expectRowVisible(sledsTable, {
     id: sleds[3].id,
     'serial number': sleds[3].baseboard.serial,
-    policy: 'expunged',
+    Kind: 'Expunged',
+    'Provision policy': '—',
     state: 'decommissioned',
   })
 
   // Visit the sled detail page of the first sled
   await sledsTable.getByRole('link').first().click()
 
-  // TODO: Once sled location is piped through this'll need to be dynamic
   await expectVisible(page, ['role=heading[name*="Sled"]'])
 
   const instancesTab = page.getByRole('tab', { name: 'Instances' })

test/e2e/utils.ts

@@ -69,10 +69,10 @@ export async function expectRowVisible(
 ) {
   // wait for header and rows to avoid flake town
   const headerLoc = table.locator('thead >> role=cell')
-  await headerLoc.locator('nth=0').waitFor() // nth=0 bc error if there's more than 1
+  await headerLoc.first().waitFor() // nth=0 bc error if there's more than 1
 
   const rowLoc = table.locator('tbody >> role=row')
-  await rowLoc.locator('nth=0').waitFor()
+  await rowLoc.first().waitFor()
 
   async function getRows() {
     // need to pull header keys every time because the whole page can change
@@ -81,7 +81,10 @@ export async function expectRowVisible(
     // filter out data-test-ignore is specifically for making the header cells
     // match up with the contents on the double-header utilization table
     const headerKeys = await table
-      .locator('thead >> th:not([data-test-ignore])')
+      .locator('thead')
+      .getByRole('row')
+      .last()
+      .locator('th:not([data-test-ignore])')
       .allTextContents()
 
     const rows = await map(table.locator('tbody >> role=row'), async (row) => {