From ec8050361e3dd6fa4df34a777258f6b5d280ec40 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Mon, 11 May 2026 16:50:53 -0400 Subject: [PATCH 01/10] Add source cell for disks, with side modal --- app/components/ImageDetailSideModal.tsx | 53 +++++++++++ app/components/SnapshotDetailSideModal.tsx | 80 ++++++++++++++++ .../project/disks/DiskDetailSideModal.tsx | 6 +- app/pages/project/disks/DisksPage.tsx | 44 +++++++++ app/pages/project/instances/StorageTab.tsx | 40 ++++++++ app/table/cells/DiskSourceCell.tsx | 93 +++++++++++++++++++ mock-api/disk.ts | 9 ++ test/e2e/disks.e2e.ts | 51 ++++++++-- 8 files changed, 368 insertions(+), 8 deletions(-) create mode 100644 app/components/ImageDetailSideModal.tsx create mode 100644 app/components/SnapshotDetailSideModal.tsx create mode 100644 app/table/cells/DiskSourceCell.tsx diff --git a/app/components/ImageDetailSideModal.tsx b/app/components/ImageDetailSideModal.tsx new file mode 100644 index 000000000..0047808bb --- /dev/null +++ b/app/components/ImageDetailSideModal.tsx @@ -0,0 +1,53 @@ +/* + * 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 Image } from '@oxide/api' +import { Images16Icon } from '@oxide/design-system/icons/react' + +import { ReadOnlySideModalForm } from '~/components/form/ReadOnlySideModalForm' +import { SideModalFormDocs } from '~/ui/lib/ModalLinks' +import { PropertiesTable } from '~/ui/lib/PropertiesTable' +import { ResourceLabel } from '~/ui/lib/SideModal' +import { docLinks } from '~/util/links' +import { bytesToGiB } from '~/util/units' + +type ImageDetailSideModalProps = { + image: Image + onDismiss: () => void +} + +export function ImageDetailSideModal({ image, onDismiss }: ImageDetailSideModalProps) { + // projectId is only set on project images; silo images leave it null + const visibility = image.projectId ? 'Project' : 'Silo' + return ( + + {image.name} + + } + > + + + + {visibility} + {image.os} + {image.version} + {bytesToGiB(image.size)} GiB + + {image.blockSize.toLocaleString()} bytes + + + + + + + ) +} diff --git a/app/components/SnapshotDetailSideModal.tsx b/app/components/SnapshotDetailSideModal.tsx new file mode 100644 index 000000000..d0057930c --- /dev/null +++ b/app/components/SnapshotDetailSideModal.tsx @@ -0,0 +1,80 @@ +/* + * 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 { useQuery } from '@tanstack/react-query' + +import { api, qErrorsAllowed, type Snapshot } from '@oxide/api' +import { Snapshots16Icon } from '@oxide/design-system/icons/react' +import { Badge } from '@oxide/design-system/ui' + +import { ReadOnlySideModalForm } from '~/components/form/ReadOnlySideModalForm' +import { SnapshotStateBadge } from '~/components/StateBadge' +import { SkeletonCell } from '~/table/cells/EmptyCell' +import { SideModalFormDocs } from '~/ui/lib/ModalLinks' +import { PropertiesTable } from '~/ui/lib/PropertiesTable' +import { ResourceLabel } from '~/ui/lib/SideModal' +import { docLinks } from '~/util/links' +import { bytesToGiB } from '~/util/units' + +const sourceDiskQ = (disk: string) => + qErrorsAllowed( + api.diskView, + { path: { disk } }, + { + errorsExpected: { + explanation: 'the source disk may have been deleted.', + statusCode: 404, + }, + } + ) + +const DiskNameFromId = ({ diskId }: { diskId: string }) => { + const { data } = useQuery(sourceDiskQ(diskId)) + if (!data) return + if (data.type === 'error') return Deleted + return <>{data.data.name} +} + +type SnapshotDetailSideModalProps = { + snapshot: Snapshot + onDismiss: () => void +} + +export function SnapshotDetailSideModal({ + snapshot, + onDismiss, +}: SnapshotDetailSideModalProps) { + return ( + + {snapshot.name} + + } + > + + + + + + + + {bytesToGiB(snapshot.size)} GiB + + + + + + + + + + ) +} diff --git a/app/pages/project/disks/DiskDetailSideModal.tsx b/app/pages/project/disks/DiskDetailSideModal.tsx index 990695f98..912c5af61 100644 --- a/app/pages/project/disks/DiskDetailSideModal.tsx +++ b/app/pages/project/disks/DiskDetailSideModal.tsx @@ -15,6 +15,7 @@ import { ReadOnlySideModalForm } from '~/components/form/ReadOnlySideModalForm' import { DiskStateBadge, DiskTypeBadge } from '~/components/StateBadge' import { titleCrumb } from '~/hooks/use-crumbs' import { getDiskSelector, useDiskSelector } from '~/hooks/use-params' +import { DiskSourceName } from '~/table/cells/DiskSourceCell' import { SideModalFormDocs } from '~/ui/lib/ModalLinks' import { PropertiesTable } from '~/ui/lib/PropertiesTable' import { ResourceLabel } from '~/ui/lib/SideModal' @@ -83,8 +84,9 @@ export function DiskDetailSideModal({ {/* TODO: show attached instance by name like the table does? */} - - + + + {disk.readOnly ? 'True' : 'False'} diff --git a/app/pages/project/disks/DisksPage.tsx b/app/pages/project/disks/DisksPage.tsx index 469c4c8e9..5e82f0faf 100644 --- a/app/pages/project/disks/DisksPage.tsx +++ b/app/pages/project/disks/DisksPage.tsx @@ -30,6 +30,7 @@ import { getProjectSelector, useProjectSelector } from '~/hooks/use-params' import { useQuickActions } from '~/hooks/use-quick-actions' import { confirmDelete } from '~/stores/confirm-delete' import { addToast } from '~/stores/toast' +import { DiskSourceName, sourceImageQ, sourceSnapshotQ } from '~/table/cells/DiskSourceCell' import { InstanceLink } from '~/table/cells/InstanceLinkCell' import { LinkCell } from '~/table/cells/LinkCell' import { useColsWithActions, type MenuAction } from '~/table/columns/action-col' @@ -78,6 +79,41 @@ export async function clientLoader({ params }: LoaderFunctionArgs) { queryClient.setQueryData(queryKey, instance) } }), + + // Prime per-id image and snapshot lookups used by the Source column. + // Disks may be sourced from a project image, a silo image, or a + // project-scoped snapshot, so we fetch all three lists. Deleted sources + // simply miss the cache and fall back to the qErrorsAllowed 404 path. + queryClient + .fetchQuery(q(api.imageList, { query: { project, limit: ALL_ISH } })) + .then((images) => { + for (const image of images.items) { + queryClient.setQueryData(sourceImageQ(image.id).queryKey, { + type: 'success', + data: image, + }) + } + }), + queryClient + .fetchQuery(q(api.imageList, { query: { limit: ALL_ISH } })) + .then((images) => { + for (const image of images.items) { + queryClient.setQueryData(sourceImageQ(image.id).queryKey, { + type: 'success', + data: image, + }) + } + }), + queryClient + .fetchQuery(q(api.snapshotList, { query: { project, limit: ALL_ISH } })) + .then((snapshots) => { + for (const snapshot of snapshots.items) { + queryClient.setQueryData(sourceSnapshotQ(snapshot.id).queryKey, { + type: 'success', + data: snapshot, + }) + } + }), ]) return null } @@ -176,6 +212,14 @@ export default function DisksPage() { cell: (info) => , }), colHelper.accessor('size', Columns.size), + colHelper.accessor( + (row) => ({ imageId: row.imageId, snapshotId: row.snapshotId }), + { + id: 'source', + header: 'Source', + cell: (info) => , + } + ), colHelper.accessor('state.state', { header: 'state', cell: (info) => , diff --git a/app/pages/project/instances/StorageTab.tsx b/app/pages/project/instances/StorageTab.tsx index 86bb123e2..a0d314bef 100644 --- a/app/pages/project/instances/StorageTab.tsx +++ b/app/pages/project/instances/StorageTab.tsx @@ -31,6 +31,7 @@ import { getInstanceSelector, useInstanceSelector } from '~/hooks/use-params' import { DiskDetailSideModal } from '~/pages/project/disks/DiskDetailSideModal' import { confirmAction } from '~/stores/confirm-action' import { addToast } from '~/stores/toast' +import { DiskSourceName, sourceImageQ, sourceSnapshotQ } from '~/table/cells/DiskSourceCell' import { ButtonCell } from '~/table/cells/LinkCell' import { useColsWithActions, type MenuAction } from '~/table/columns/action-col' import { Columns } from '~/table/columns/common' @@ -39,6 +40,7 @@ import { Button } from '~/ui/lib/Button' import { CardBlock } from '~/ui/lib/CardBlock' import { EMBody, EmptyMessage } from '~/ui/lib/EmptyMessage' import { TableEmptyBox } from '~/ui/lib/Table' +import { ALL_ISH } from '~/util/consts' import { links } from '~/util/links' import { capitalize } from '~/util/str' @@ -55,6 +57,39 @@ export async function clientLoader({ params }: LoaderFunctionArgs) { // This is covered by the InstancePage loader but there's no downside to // being redundant. If it were removed there, we'd still want it here. queryClient.prefetchQuery(q(api.instanceView, selector)), + + // Prime per-id image and snapshot lookups used by the Source column. + // Sources may be project images, silo images, or project snapshots. + queryClient + .fetchQuery(q(api.imageList, { query: { project, limit: ALL_ISH } })) + .then((images) => { + for (const image of images.items) { + queryClient.setQueryData(sourceImageQ(image.id).queryKey, { + type: 'success', + data: image, + }) + } + }), + queryClient + .fetchQuery(q(api.imageList, { query: { limit: ALL_ISH } })) + .then((images) => { + for (const image of images.items) { + queryClient.setQueryData(sourceImageQ(image.id).queryKey, { + type: 'success', + data: image, + }) + } + }), + queryClient + .fetchQuery(q(api.snapshotList, { query: { project, limit: ALL_ISH } })) + .then((snapshots) => { + for (const snapshot of snapshots.items) { + queryClient.setQueryData(sourceSnapshotQ(snapshot.id).queryKey, { + type: 'success', + data: snapshot, + }) + } + }), ]) return null } @@ -99,6 +134,11 @@ export default function StorageTab() { cell: (info) => , }), colHelper.accessor('size', Columns.size), + colHelper.accessor((row) => ({ imageId: row.imageId, snapshotId: row.snapshotId }), { + id: 'source', + header: 'Source', + cell: (info) => , + }), colHelper.accessor((row) => row.state.state, { header: 'state', cell: (info) => , diff --git a/app/table/cells/DiskSourceCell.tsx b/app/table/cells/DiskSourceCell.tsx new file mode 100644 index 000000000..0aefe44a3 --- /dev/null +++ b/app/table/cells/DiskSourceCell.tsx @@ -0,0 +1,93 @@ +/* + * 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 { useQuery } from '@tanstack/react-query' +import { useState } from 'react' + +import { api, qErrorsAllowed } from '@oxide/api' +import { Badge } from '@oxide/design-system/ui' + +import { ImageDetailSideModal } from '~/components/ImageDetailSideModal' +import { SnapshotDetailSideModal } from '~/components/SnapshotDetailSideModal' +import { useIsInSideModal } from '~/ui/lib/modal-context' + +import { EmptyCell, SkeletonCell } from './EmptyCell' +import { ButtonCell } from './LinkCell' + +// Use qErrorsAllowed so deletion of the source resource is a cacheable result +// rather than an error that blows up the page. Tables and the disk detail +// modal both render a "Deleted" badge in that case. + +export const sourceImageQ = (image: string) => + qErrorsAllowed( + api.imageView, + { path: { image } }, + { + errorsExpected: { + explanation: 'the source image may have been deleted.', + statusCode: 404, + }, + } + ) + +export const sourceSnapshotQ = (snapshot: string) => + qErrorsAllowed( + api.snapshotView, + { path: { snapshot } }, + { + errorsExpected: { + explanation: 'the source snapshot may have been deleted.', + statusCode: 404, + }, + } + ) + +type Props = { + imageId?: string | null + snapshotId?: string | null +} + +/** + * Renders the source resource's name. In a table cell the name is a + * `ButtonCell` that opens a detail side modal; inside a side modal it falls + * back to plain text to avoid stacking modals. Falls back to a skeleton while + * loading and a "Deleted" badge when the source no longer exists. + */ +export const DiskSourceName = ({ imageId, snapshotId }: Props) => { + const inSideModal = useIsInSideModal() + const [showDetail, setShowDetail] = useState(false) + const image = useQuery({ ...sourceImageQ(imageId!), enabled: !!imageId }) + const snapshot = useQuery({ ...sourceSnapshotQ(snapshotId!), enabled: !!snapshotId }) + + if (!imageId && !snapshotId) return + + // imageId wins if somehow both are set + const result = imageId ? image.data : snapshot.data + if (!result) return + if (result.type === 'error') return Deleted + + const name = result.data.name + if (inSideModal) return <>{name} + return ( + <> + setShowDetail(true)}>{name} + {showDetail && + (imageId && image.data?.type === 'success' ? ( + setShowDetail(false)} + /> + ) : snapshotId && snapshot.data?.type === 'success' ? ( + setShowDetail(false)} + /> + ) : null)} + + ) +} diff --git a/mock-api/disk.ts b/mock-api/disk.ts index e496474b4..2dffdae01 100644 --- a/mock-api/disk.ts +++ b/mock-api/disk.ts @@ -81,6 +81,8 @@ export const disk2: Json = { block_size: 2048, disk_type: 'distributed', read_only: false, + // ubuntu-22-04 silo image (see ./image.ts) — exercises Source column + image_id: 'ae46ddf5-a8d5-40fa-bcda-fcac606e3f9b', } export const stoppedBootDisk: Json = { @@ -132,6 +134,8 @@ export const disks: Json[] = [ block_size: 2048, disk_type: 'distributed', read_only: false, + // snapshot-1 (see ./snapshot.ts) — exercises Source column + snapshot_id: 'ab805e59-b6b8-4c73-8081-6a224b6b0698', }, { id: '5695b16d-e1d6-44b0-a75c-7b4299831540', @@ -217,6 +221,9 @@ export const disks: Json[] = [ block_size: 2048, disk_type: 'distributed', read_only: false, + // intentionally references an image that doesn't exist so the Source + // column renders the "Deleted" badge for missing source resources + image_id: '2a5412c2-d109-45d9-8cc2-e0868cced259', }, { id: 'a028160f-603c-4562-bb71-d2d76f1ac2a8', @@ -273,6 +280,8 @@ export const disks: Json[] = [ block_size: 4096, disk_type: 'distributed', read_only: true, + // snapshot-2 (see ./snapshot.ts) + snapshot_id: '9a29813d-e94b-4c6a-82a0-672af3f78a6f', }, // put a ton of disks in project 2 so we can use it to test comboboxes ...Array.from({ length: 1010 }).map((_, i) => { diff --git a/test/e2e/disks.e2e.ts b/test/e2e/disks.e2e.ts index 9e8b0b9d4..888bbc36f 100644 --- a/test/e2e/disks.e2e.ts +++ b/test/e2e/disks.e2e.ts @@ -29,6 +29,41 @@ test('Disk detail side modal', async ({ page }) => { await expect(propertiesTableValue(modal, 'Read only')).toHaveText('False') }) +test('Source links open detail side modals from disk list', async ({ page }) => { + await page.goto('/projects/mock-project/disks') + + const table = page.getByRole('table') + + // Snapshot source: clicking snapshot-1 opens the snapshot side modal + const disk3 = table.getByRole('row', { name: /disk-3/ }) + await disk3.getByRole('button', { name: 'snapshot-1' }).click() + const snapshotModal = page.getByRole('dialog', { name: 'Snapshot details' }) + await expect(snapshotModal).toBeVisible() + await expect(propertiesTableValue(snapshotModal, 'Source disk')).toHaveText('disk-1') + await snapshotModal.getByRole('button', { name: 'Close' }).first().click() + await expect(snapshotModal).toBeHidden() + + // Image source: clicking ubuntu-22-04 opens the image side modal as silo image + const disk2 = table.getByRole('row', { name: /disk-2/ }) + await disk2.getByRole('button', { name: 'ubuntu-22-04' }).click() + const imageModal = page.getByRole('dialog', { name: 'Image details' }) + await expect(imageModal).toBeVisible() + await expect(propertiesTableValue(imageModal, 'Visibility')).toHaveText('Silo') + await expect(propertiesTableValue(imageModal, 'OS')).toHaveText('ubuntu') +}) + +test('Source name in disk side modal is plain text, not a link', async ({ page }) => { + await page.goto('/projects/mock-project/disks') + + // Open disk-3, which has a snapshot source. Inside the side modal the source + // name should not be a clickable button (no nested modal stacking). + await page.getByRole('link', { name: 'disk-3', exact: true }).click() + const modal = page.getByRole('dialog', { name: 'Disk details' }) + await expect(modal).toBeVisible() + await expect(propertiesTableValue(modal, 'Source')).toHaveText('snapshot-1') + await expect(modal.getByRole('button', { name: 'snapshot-1' })).toBeHidden() +}) + test('Read-only disk shows badge in table and detail', async ({ page }) => { await page.goto('/projects/mock-project/disks') @@ -60,13 +95,19 @@ test('List disks and snapshot', async ({ page }) => { name: 'disk-1', size: '2 GiB', state: 'attached', + Source: '—', }) await expectRowVisible(table, { Instance: '—', name: 'disk-3', size: '6 GiB', state: 'detached', + Source: 'snapshot-1', }) + // disk-2 is sourced from the ubuntu-22-04 silo image + await expectRowVisible(table, { name: 'disk-2', Source: 'ubuntu-22-04' }) + // disk-9 references an image that does not exist, so we render "Deleted" + await expectRowVisible(table, { name: 'disk-9', Source: 'Deleted' }) await clickRowAction(page, 'disk-1 db1', 'Snapshot') await expectToast(page, 'Creating snapshot of disk disk-1') @@ -252,11 +293,10 @@ test('Create disk from snapshot with read-only', async ({ page }) => { const row = page.getByRole('row', { name: /a-new-disk/ }) await expect(row.getByText('Read only', { exact: true })).toBeVisible() - // Verify snapshot ID in detail modal (now truncated) + // Verify the resolved source name appears in the detail modal await page.getByRole('link', { name: 'a-new-disk' }).click() const modal = page.getByRole('dialog', { name: 'Disk details' }) - // The ID is truncated to 32 chars, but full ID is in aria-label - await expect(modal.getByLabel('e6c58826-62fb-4205-820e-620407cd04e7')).toBeVisible() + await expect(propertiesTableValue(modal, 'Source')).toHaveText('delete-500') }) test('Create disk from image with read-only', async ({ page }) => { @@ -273,9 +313,8 @@ test('Create disk from image with read-only', async ({ page }) => { const row = page.getByRole('row', { name: /a-new-disk/ }) await expect(row.getByText('Read only', { exact: true })).toBeVisible() - // Verify image ID in detail modal (now truncated) + // Verify the resolved source name appears in the detail modal await page.getByRole('link', { name: 'a-new-disk' }).click() const modal = page.getByRole('dialog', { name: 'Disk details' }) - // The ID is truncated to 32 chars, but full ID is in aria-label - await expect(modal.getByLabel('4700ecf1-8f48-4ecf-b78e-816ddb76aaca')).toBeVisible() + await expect(propertiesTableValue(modal, 'Source')).toHaveText('image-3') }) From 32cef5b8d31f37bc62a9b1bde2b9fd24fea24252 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Wed, 13 May 2026 11:38:08 -0400 Subject: [PATCH 02/10] defer loading, with skeleton; add badge to sidemodal source cell --- app/forms/image-upload.tsx | 8 +++- app/pages/SiloImagesPage.tsx | 6 +++ app/pages/project/disks/DisksPage.tsx | 37 +------------------ app/pages/project/images/ImagesPage.tsx | 5 +++ app/pages/project/instances/StorageTab.tsx | 36 +----------------- app/pages/project/snapshots/SnapshotsPage.tsx | 2 + app/table/cells/DiskSourceCell.tsx | 17 +++++++-- test/e2e/disks.e2e.ts | 6 +-- test/e2e/images.e2e.ts | 17 +++++++-- 9 files changed, 52 insertions(+), 82 deletions(-) diff --git a/app/forms/image-upload.tsx b/app/forms/image-upload.tsx index 93e92573d..09b64d920 100644 --- a/app/forms/image-upload.tsx +++ b/app/forms/image-upload.tsx @@ -228,7 +228,12 @@ export default function ImageCreate() { const finalizeDisk = useApiMutation(api.diskFinalizeImport) const createImage = useApiMutation(api.imageCreate) const deleteDisk = useApiMutation(api.diskDelete) - const deleteSnapshot = useApiMutation(api.snapshotDelete) + const deleteSnapshot = useApiMutation(api.snapshotDelete, { + onSuccess() { + queryClient.invalidateEndpoint('snapshotList') + queryClient.invalidateEndpoint('snapshotView') + }, + }) // TODO: Distinguish cleanup mutations being called after successful run vs. // due to error. In the former case, they have their own steps to highlight as @@ -259,6 +264,7 @@ export default function ImageCreate() { const deleteSnapshotCleanup = useApiMutation(api.snapshotDelete, { onSuccess() { queryClient.invalidateEndpoint('snapshotList') + queryClient.invalidateEndpoint('snapshotView') }, }) diff --git a/app/pages/SiloImagesPage.tsx b/app/pages/SiloImagesPage.tsx index 6a830e377..d274f29cd 100644 --- a/app/pages/SiloImagesPage.tsx +++ b/app/pages/SiloImagesPage.tsx @@ -73,6 +73,8 @@ export default function SiloImagesPage() { // prettier-ignore addToast(<>Image {variables.path.image} deleted) queryClient.invalidateEndpoint('imageList') + // also drops per-id imageView entries seeded for Source-column lookups + queryClient.invalidateEndpoint('imageView') }, }) @@ -152,6 +154,8 @@ const PromoteImageModal = ({ onDismiss }: { onDismiss: () => void }) => { // prettier-ignore addToast(<>Image {data.name} promoted) queryClient.invalidateEndpoint('imageList') + // promotion flips projectId; refetch the per-id view + queryClient.invalidateEndpoint('imageView') onDismiss() }, onError: (err) => { @@ -248,6 +252,8 @@ const DemoteImageModal = ({ }) queryClient.invalidateEndpoint('imageList') + // demotion flips projectId; refetch the per-id view + queryClient.invalidateEndpoint('imageView') onDismiss() }, onError: (err) => { diff --git a/app/pages/project/disks/DisksPage.tsx b/app/pages/project/disks/DisksPage.tsx index 5e82f0faf..21cf28c66 100644 --- a/app/pages/project/disks/DisksPage.tsx +++ b/app/pages/project/disks/DisksPage.tsx @@ -30,7 +30,7 @@ import { getProjectSelector, useProjectSelector } from '~/hooks/use-params' import { useQuickActions } from '~/hooks/use-quick-actions' import { confirmDelete } from '~/stores/confirm-delete' import { addToast } from '~/stores/toast' -import { DiskSourceName, sourceImageQ, sourceSnapshotQ } from '~/table/cells/DiskSourceCell' +import { DiskSourceName } from '~/table/cells/DiskSourceCell' import { InstanceLink } from '~/table/cells/InstanceLinkCell' import { LinkCell } from '~/table/cells/LinkCell' import { useColsWithActions, type MenuAction } from '~/table/columns/action-col' @@ -79,41 +79,6 @@ export async function clientLoader({ params }: LoaderFunctionArgs) { queryClient.setQueryData(queryKey, instance) } }), - - // Prime per-id image and snapshot lookups used by the Source column. - // Disks may be sourced from a project image, a silo image, or a - // project-scoped snapshot, so we fetch all three lists. Deleted sources - // simply miss the cache and fall back to the qErrorsAllowed 404 path. - queryClient - .fetchQuery(q(api.imageList, { query: { project, limit: ALL_ISH } })) - .then((images) => { - for (const image of images.items) { - queryClient.setQueryData(sourceImageQ(image.id).queryKey, { - type: 'success', - data: image, - }) - } - }), - queryClient - .fetchQuery(q(api.imageList, { query: { limit: ALL_ISH } })) - .then((images) => { - for (const image of images.items) { - queryClient.setQueryData(sourceImageQ(image.id).queryKey, { - type: 'success', - data: image, - }) - } - }), - queryClient - .fetchQuery(q(api.snapshotList, { query: { project, limit: ALL_ISH } })) - .then((snapshots) => { - for (const snapshot of snapshots.items) { - queryClient.setQueryData(sourceSnapshotQ(snapshot.id).queryKey, { - type: 'success', - data: snapshot, - }) - } - }), ]) return null } diff --git a/app/pages/project/images/ImagesPage.tsx b/app/pages/project/images/ImagesPage.tsx index f3e788007..8390722d6 100644 --- a/app/pages/project/images/ImagesPage.tsx +++ b/app/pages/project/images/ImagesPage.tsx @@ -67,6 +67,8 @@ export default function ImagesPage() { // prettier-ignore addToast(<>Image {variables.path.image} deleted) queryClient.invalidateEndpoint('imageList') + // also drops per-id imageView entries seeded for Source-column lookups + queryClient.invalidateEndpoint('imageView') }, }) @@ -175,6 +177,9 @@ const PromoteImageModal = ({ onDismiss, imageName }: PromoteModalProps) => { }, }) queryClient.invalidateEndpoint('imageList') + // promotion flips projectId; refetch the per-id view so cached entries + // reflect the new visibility + queryClient.invalidateEndpoint('imageView') onDismiss() }, onError: (err) => { diff --git a/app/pages/project/instances/StorageTab.tsx b/app/pages/project/instances/StorageTab.tsx index a0d314bef..ab42021b3 100644 --- a/app/pages/project/instances/StorageTab.tsx +++ b/app/pages/project/instances/StorageTab.tsx @@ -31,7 +31,7 @@ import { getInstanceSelector, useInstanceSelector } from '~/hooks/use-params' import { DiskDetailSideModal } from '~/pages/project/disks/DiskDetailSideModal' import { confirmAction } from '~/stores/confirm-action' import { addToast } from '~/stores/toast' -import { DiskSourceName, sourceImageQ, sourceSnapshotQ } from '~/table/cells/DiskSourceCell' +import { DiskSourceName } from '~/table/cells/DiskSourceCell' import { ButtonCell } from '~/table/cells/LinkCell' import { useColsWithActions, type MenuAction } from '~/table/columns/action-col' import { Columns } from '~/table/columns/common' @@ -40,7 +40,6 @@ import { Button } from '~/ui/lib/Button' import { CardBlock } from '~/ui/lib/CardBlock' import { EMBody, EmptyMessage } from '~/ui/lib/EmptyMessage' import { TableEmptyBox } from '~/ui/lib/Table' -import { ALL_ISH } from '~/util/consts' import { links } from '~/util/links' import { capitalize } from '~/util/str' @@ -57,39 +56,6 @@ export async function clientLoader({ params }: LoaderFunctionArgs) { // This is covered by the InstancePage loader but there's no downside to // being redundant. If it were removed there, we'd still want it here. queryClient.prefetchQuery(q(api.instanceView, selector)), - - // Prime per-id image and snapshot lookups used by the Source column. - // Sources may be project images, silo images, or project snapshots. - queryClient - .fetchQuery(q(api.imageList, { query: { project, limit: ALL_ISH } })) - .then((images) => { - for (const image of images.items) { - queryClient.setQueryData(sourceImageQ(image.id).queryKey, { - type: 'success', - data: image, - }) - } - }), - queryClient - .fetchQuery(q(api.imageList, { query: { limit: ALL_ISH } })) - .then((images) => { - for (const image of images.items) { - queryClient.setQueryData(sourceImageQ(image.id).queryKey, { - type: 'success', - data: image, - }) - } - }), - queryClient - .fetchQuery(q(api.snapshotList, { query: { project, limit: ALL_ISH } })) - .then((snapshots) => { - for (const snapshot of snapshots.items) { - queryClient.setQueryData(sourceSnapshotQ(snapshot.id).queryKey, { - type: 'success', - data: snapshot, - }) - } - }), ]) return null } diff --git a/app/pages/project/snapshots/SnapshotsPage.tsx b/app/pages/project/snapshots/SnapshotsPage.tsx index 3e2f29ce4..693f64b5a 100644 --- a/app/pages/project/snapshots/SnapshotsPage.tsx +++ b/app/pages/project/snapshots/SnapshotsPage.tsx @@ -134,6 +134,8 @@ export default function SnapshotsPage() { const { mutateAsync: deleteSnapshot } = useApiMutation(api.snapshotDelete, { onSuccess() { queryClient.invalidateEndpoint('snapshotList') + // also drops per-id snapshotView entries seeded for Source-column lookups + queryClient.invalidateEndpoint('snapshotView') }, }) diff --git a/app/table/cells/DiskSourceCell.tsx b/app/table/cells/DiskSourceCell.tsx index 0aefe44a3..433ed6910 100644 --- a/app/table/cells/DiskSourceCell.tsx +++ b/app/table/cells/DiskSourceCell.tsx @@ -23,7 +23,7 @@ import { ButtonCell } from './LinkCell' // rather than an error that blows up the page. Tables and the disk detail // modal both render a "Deleted" badge in that case. -export const sourceImageQ = (image: string) => +const sourceImageQ = (image: string) => qErrorsAllowed( api.imageView, { path: { image } }, @@ -35,7 +35,7 @@ export const sourceImageQ = (image: string) => } ) -export const sourceSnapshotQ = (snapshot: string) => +const sourceSnapshotQ = (snapshot: string) => qErrorsAllowed( api.snapshotView, { path: { snapshot } }, @@ -66,13 +66,22 @@ export const DiskSourceName = ({ imageId, snapshotId }: Props) => { if (!imageId && !snapshotId) return - // imageId wins if somehow both are set + // Nexus populates exactly one of imageId/snapshotId per disk, so a disk won't have both, + // though the Disk type in the API just lists both as optional + // https://github.com/oxidecomputer/omicron/blob/254a0c5/nexus/db-model/src/disk_type_crucible.rs#L49-L78 const result = imageId ? image.data : snapshot.data if (!result) return if (result.type === 'error') return Deleted const name = result.data.name - if (inSideModal) return <>{name} + if (inSideModal) { + return ( + + {imageId ? 'Image' : 'Snapshot'} + {name} + + ) + } return ( <> setShowDetail(true)}>{name} diff --git a/test/e2e/disks.e2e.ts b/test/e2e/disks.e2e.ts index 888bbc36f..8305ff919 100644 --- a/test/e2e/disks.e2e.ts +++ b/test/e2e/disks.e2e.ts @@ -60,7 +60,7 @@ test('Source name in disk side modal is plain text, not a link', async ({ page } await page.getByRole('link', { name: 'disk-3', exact: true }).click() const modal = page.getByRole('dialog', { name: 'Disk details' }) await expect(modal).toBeVisible() - await expect(propertiesTableValue(modal, 'Source')).toHaveText('snapshot-1') + await expect(propertiesTableValue(modal, 'Source')).toHaveText('Snapshotsnapshot-1') await expect(modal.getByRole('button', { name: 'snapshot-1' })).toBeHidden() }) @@ -296,7 +296,7 @@ test('Create disk from snapshot with read-only', async ({ page }) => { // Verify the resolved source name appears in the detail modal await page.getByRole('link', { name: 'a-new-disk' }).click() const modal = page.getByRole('dialog', { name: 'Disk details' }) - await expect(propertiesTableValue(modal, 'Source')).toHaveText('delete-500') + await expect(propertiesTableValue(modal, 'Source')).toHaveText('Snapshotdelete-500') }) test('Create disk from image with read-only', async ({ page }) => { @@ -316,5 +316,5 @@ test('Create disk from image with read-only', async ({ page }) => { // Verify the resolved source name appears in the detail modal await page.getByRole('link', { name: 'a-new-disk' }).click() const modal = page.getByRole('dialog', { name: 'Disk details' }) - await expect(propertiesTableValue(modal, 'Source')).toHaveText('image-3') + await expect(propertiesTableValue(modal, 'Source')).toHaveText('Imageimage-3') }) diff --git a/test/e2e/images.e2e.ts b/test/e2e/images.e2e.ts index 1c3ef736c..98fd15e93 100644 --- a/test/e2e/images.e2e.ts +++ b/test/e2e/images.e2e.ts @@ -12,6 +12,7 @@ import { clipboardText, expect, expectNotVisible, + expectRowVisible, expectToast, expectVisible, getPageAsUser, @@ -143,19 +144,29 @@ test('can delete an image from a project', async ({ page }) => { test('can delete an image from a silo', async ({ page }) => { await page.goto('/images') - const cell = page.getByRole('cell', { name: 'ubuntu-20-04' }) + // ubuntu-22-04 is the silo image referenced by mock-project/disks/disk-2, so + // we use it here to also verify the disk's Source cell flips to "Deleted" + // after the source image is removed. + const cell = page.getByRole('cell', { name: 'ubuntu-22-04' }) await expect(cell).toBeVisible() - await clickRowAction(page, 'ubuntu-20-04', 'Delete') + await clickRowAction(page, 'ubuntu-22-04', 'Delete') const spinner = page.getByRole('dialog').getByLabel('Spinner') await expect(spinner).toBeHidden() await page.getByRole('button', { name: 'Confirm' }).click() await expect(spinner).toBeVisible() // Check deletion was successful - await expectToast(page, 'Image ubuntu-20-04 deleted') + await expectToast(page, 'Image ubuntu-22-04 deleted') await expect(cell).toBeHidden() await expect(spinner).toBeHidden() + + // Navigate client-side (preserves MSW db) to disk-2's row and verify the + // Source column now shows "Deleted" instead of the image name. + await page.getByRole('link', { name: 'Projects', exact: true }).click() + await page.getByRole('table').getByRole('link', { name: 'mock-project' }).click() + await page.getByRole('link', { name: 'Disks' }).click() + await expectRowVisible(page.getByRole('table'), { name: 'disk-2', Source: 'Deleted' }) }) // this is to some extent a test of our mock server implementation, but I want From 509331f66830345782636bb044bdadf6659c9dc7 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Thu, 4 Jun 2026 13:08:22 -0700 Subject: [PATCH 03/10] add PropertiesTable.SizeRow --- app/components/ImageDetailSideModal.tsx | 3 +-- app/components/SnapshotDetailSideModal.tsx | 5 +---- app/pages/SiloImagesPage.tsx | 1 - .../project/disks/DiskDetailSideModal.tsx | 3 +-- app/pages/project/images/ImagesPage.tsx | 1 - app/pages/project/snapshots/SnapshotsPage.tsx | 1 - app/ui/lib/PropertiesTable.tsx | 21 +++++++++++++++++++ 7 files changed, 24 insertions(+), 11 deletions(-) diff --git a/app/components/ImageDetailSideModal.tsx b/app/components/ImageDetailSideModal.tsx index 0047808bb..362d84c70 100644 --- a/app/components/ImageDetailSideModal.tsx +++ b/app/components/ImageDetailSideModal.tsx @@ -13,7 +13,6 @@ import { SideModalFormDocs } from '~/ui/lib/ModalLinks' import { PropertiesTable } from '~/ui/lib/PropertiesTable' import { ResourceLabel } from '~/ui/lib/SideModal' import { docLinks } from '~/util/links' -import { bytesToGiB } from '~/util/units' type ImageDetailSideModalProps = { image: Image @@ -40,7 +39,7 @@ export function ImageDetailSideModal({ image, onDismiss }: ImageDetailSideModalP {visibility} {image.os} {image.version} - {bytesToGiB(image.size)} GiB + {image.blockSize.toLocaleString()} bytes diff --git a/app/components/SnapshotDetailSideModal.tsx b/app/components/SnapshotDetailSideModal.tsx index d0057930c..a808630d4 100644 --- a/app/components/SnapshotDetailSideModal.tsx +++ b/app/components/SnapshotDetailSideModal.tsx @@ -18,7 +18,6 @@ import { SideModalFormDocs } from '~/ui/lib/ModalLinks' import { PropertiesTable } from '~/ui/lib/PropertiesTable' import { ResourceLabel } from '~/ui/lib/SideModal' import { docLinks } from '~/util/links' -import { bytesToGiB } from '~/util/units' const sourceDiskQ = (disk: string) => qErrorsAllowed( @@ -65,9 +64,7 @@ export function SnapshotDetailSideModal({ - - {bytesToGiB(snapshot.size)} GiB - + diff --git a/app/pages/SiloImagesPage.tsx b/app/pages/SiloImagesPage.tsx index d274f29cd..8d9757c44 100644 --- a/app/pages/SiloImagesPage.tsx +++ b/app/pages/SiloImagesPage.tsx @@ -73,7 +73,6 @@ export default function SiloImagesPage() { // prettier-ignore addToast(<>Image {variables.path.image} deleted) queryClient.invalidateEndpoint('imageList') - // also drops per-id imageView entries seeded for Source-column lookups queryClient.invalidateEndpoint('imageView') }, }) diff --git a/app/pages/project/disks/DiskDetailSideModal.tsx b/app/pages/project/disks/DiskDetailSideModal.tsx index 912c5af61..4441fa10c 100644 --- a/app/pages/project/disks/DiskDetailSideModal.tsx +++ b/app/pages/project/disks/DiskDetailSideModal.tsx @@ -22,7 +22,6 @@ import { ResourceLabel } from '~/ui/lib/SideModal' import { docLinks } from '~/util/links' import { pb } from '~/util/path-builder' import type * as PP from '~/util/path-params' -import { bytesToGiB } from '~/util/units' const diskView = ({ disk, project }: PP.Disk) => q(api.diskView, { path: { disk }, query: { project } }) @@ -76,7 +75,7 @@ export function DiskDetailSideModal({ - {bytesToGiB(disk.size)} GiB + diff --git a/app/pages/project/images/ImagesPage.tsx b/app/pages/project/images/ImagesPage.tsx index 8390722d6..6260511d6 100644 --- a/app/pages/project/images/ImagesPage.tsx +++ b/app/pages/project/images/ImagesPage.tsx @@ -67,7 +67,6 @@ export default function ImagesPage() { // prettier-ignore addToast(<>Image {variables.path.image} deleted) queryClient.invalidateEndpoint('imageList') - // also drops per-id imageView entries seeded for Source-column lookups queryClient.invalidateEndpoint('imageView') }, }) diff --git a/app/pages/project/snapshots/SnapshotsPage.tsx b/app/pages/project/snapshots/SnapshotsPage.tsx index 693f64b5a..47bda59cb 100644 --- a/app/pages/project/snapshots/SnapshotsPage.tsx +++ b/app/pages/project/snapshots/SnapshotsPage.tsx @@ -134,7 +134,6 @@ export default function SnapshotsPage() { const { mutateAsync: deleteSnapshot } = useApiMutation(api.snapshotDelete, { onSuccess() { queryClient.invalidateEndpoint('snapshotList') - // also drops per-id snapshotView entries seeded for Source-column lookups queryClient.invalidateEndpoint('snapshotView') }, }) diff --git a/app/ui/lib/PropertiesTable.tsx b/app/ui/lib/PropertiesTable.tsx index 8315bff80..2bfb2c7be 100644 --- a/app/ui/lib/PropertiesTable.tsx +++ b/app/ui/lib/PropertiesTable.tsx @@ -12,6 +12,7 @@ import { DescriptionCell } from '~/table/cells/DescriptionCell' import { EmptyCell } from '~/table/cells/EmptyCell' import { isOneOf } from '~/util/children' import { invariant } from '~/util/invariant' +import { formatBytes } from '~/util/units' import { DateTime } from './DateTime' import { Truncate } from './Truncate' @@ -33,6 +34,7 @@ export function PropertiesTable({ PropertiesTable.IdRow, PropertiesTable.DescriptionRow, PropertiesTable.DateRow, + PropertiesTable.SizeRow, ]), 'PropertiesTable only accepts specific Row components as children' ) @@ -99,3 +101,22 @@ PropertiesTable.DateRow = ({ ) + +PropertiesTable.SizeRow = ({ + bytes, + label = 'Size', +}: { + bytes: number + label?: string +}) => { + const size = formatBytes(bytes) + // wrap in a span so flex treats value+unit as one item; otherwise the browser + // collapses the trailing space at the flex-item boundary, rendering "1GiB" + return ( + + + {size.value} {size.unit} + + + ) +} From 625d2278c1e8f1f5a95341f4757c937862506788 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Wed, 1 Jul 2026 15:02:54 -0700 Subject: [PATCH 04/10] Use existing sizeCellInner function --- app/table/cells/DiskSourceCell.tsx | 1 + app/ui/lib/PropertiesTable.tsx | 15 ++------------- 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/app/table/cells/DiskSourceCell.tsx b/app/table/cells/DiskSourceCell.tsx index 433ed6910..65ab97170 100644 --- a/app/table/cells/DiskSourceCell.tsx +++ b/app/table/cells/DiskSourceCell.tsx @@ -61,6 +61,7 @@ type Props = { export const DiskSourceName = ({ imageId, snapshotId }: Props) => { const inSideModal = useIsInSideModal() const [showDetail, setShowDetail] = useState(false) + // the `!` is safe because the query only runs when the id is present (enabled) const image = useQuery({ ...sourceImageQ(imageId!), enabled: !!imageId }) const snapshot = useQuery({ ...sourceSnapshotQ(snapshotId!), enabled: !!snapshotId }) diff --git a/app/ui/lib/PropertiesTable.tsx b/app/ui/lib/PropertiesTable.tsx index 202695f4d..3322900d3 100644 --- a/app/ui/lib/PropertiesTable.tsx +++ b/app/ui/lib/PropertiesTable.tsx @@ -10,9 +10,9 @@ import type { ReactNode } from 'react' import { DescriptionCell } from '~/table/cells/DescriptionCell' import { EmptyCell } from '~/table/cells/EmptyCell' +import { sizeCellInner } from '~/table/columns/common' import { isOneOf } from '~/util/children' import { invariant } from '~/util/invariant' -import { formatBytes } from '~/util/units' import { CopyToClipboard } from './CopyToClipboard' import { DateTime } from './DateTime' @@ -110,18 +110,7 @@ PropertiesTable.SizeRow = ({ }: { bytes: number label?: string -}) => { - const size = formatBytes(bytes) - // wrap in a span so flex treats value+unit as one item; otherwise the browser - // collapses the trailing space at the flex-item boundary, rendering "1GiB" - return ( - - - {size.value} {size.unit} - - - ) -} +}) => {sizeCellInner(bytes)} PropertiesTable.CopyableRow = ({ label, text }: { label: string; text: string }) => ( From d388ee240f1960af89d1dbb7de7ecd01d652c41c Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 1 Jul 2026 16:45:50 -0500 Subject: [PATCH 05/10] increase gap next to image badge --- app/table/cells/DiskSourceCell.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/table/cells/DiskSourceCell.tsx b/app/table/cells/DiskSourceCell.tsx index 65ab97170..4e5960241 100644 --- a/app/table/cells/DiskSourceCell.tsx +++ b/app/table/cells/DiskSourceCell.tsx @@ -77,7 +77,7 @@ export const DiskSourceName = ({ imageId, snapshotId }: Props) => { const name = result.data.name if (inSideModal) { return ( - + {imageId ? 'Image' : 'Snapshot'} {name} From 3203d809e01a97b2e261f25cf131f424cb6c6f3f Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 1 Jul 2026 16:47:49 -0500 Subject: [PATCH 06/10] invalidate diskView on disk delete --- app/pages/project/disks/DisksPage.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/pages/project/disks/DisksPage.tsx b/app/pages/project/disks/DisksPage.tsx index 21cf28c66..956c29182 100644 --- a/app/pages/project/disks/DisksPage.tsx +++ b/app/pages/project/disks/DisksPage.tsx @@ -91,6 +91,8 @@ export default function DisksPage() { const { mutateAsync: deleteDisk } = useApiMutation(api.diskDelete, { onSuccess(_data, variables) { queryClient.invalidateEndpoint('diskList') + // deleted disk may be a snapshot's source, shown in the snapshot detail modal + queryClient.invalidateEndpoint('diskView') // prettier-ignore addToast(<>Disk {variables.path.disk} deleted) }, From 6a4416bc9e4ad8522373541660f42b25bb6e6b87 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 1 Jul 2026 16:50:45 -0500 Subject: [PATCH 07/10] include source type in Deleted badge --- app/table/cells/DiskSourceCell.tsx | 6 +++++- test/e2e/disks.e2e.ts | 4 ++-- test/e2e/images.e2e.ts | 7 +++++-- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/app/table/cells/DiskSourceCell.tsx b/app/table/cells/DiskSourceCell.tsx index 4e5960241..5325ce3b9 100644 --- a/app/table/cells/DiskSourceCell.tsx +++ b/app/table/cells/DiskSourceCell.tsx @@ -72,7 +72,11 @@ export const DiskSourceName = ({ imageId, snapshotId }: Props) => { // https://github.com/oxidecomputer/omicron/blob/254a0c5/nexus/db-model/src/disk_type_crucible.rs#L49-L78 const result = imageId ? image.data : snapshot.data if (!result) return - if (result.type === 'error') return Deleted + // include the source type, which comes from the disk itself, so it survives + // deletion of the source resource + if (result.type === 'error') { + return {imageId ? 'Image' : 'Snapshot'} deleted + } const name = result.data.name if (inSideModal) { diff --git a/test/e2e/disks.e2e.ts b/test/e2e/disks.e2e.ts index 8305ff919..b5fc0e2fa 100644 --- a/test/e2e/disks.e2e.ts +++ b/test/e2e/disks.e2e.ts @@ -106,8 +106,8 @@ test('List disks and snapshot', async ({ page }) => { }) // disk-2 is sourced from the ubuntu-22-04 silo image await expectRowVisible(table, { name: 'disk-2', Source: 'ubuntu-22-04' }) - // disk-9 references an image that does not exist, so we render "Deleted" - await expectRowVisible(table, { name: 'disk-9', Source: 'Deleted' }) + // disk-9 references an image that does not exist, so we render "Image deleted" + await expectRowVisible(table, { name: 'disk-9', Source: 'Image deleted' }) await clickRowAction(page, 'disk-1 db1', 'Snapshot') await expectToast(page, 'Creating snapshot of disk disk-1') diff --git a/test/e2e/images.e2e.ts b/test/e2e/images.e2e.ts index 706f79218..710010413 100644 --- a/test/e2e/images.e2e.ts +++ b/test/e2e/images.e2e.ts @@ -180,11 +180,14 @@ test('can delete an image from a silo', async ({ page }) => { await expect(spinner).toBeHidden() // Navigate client-side (preserves MSW db) to disk-2's row and verify the - // Source column now shows "Deleted" instead of the image name. + // Source column now shows "Image deleted" instead of the image name. await page.getByRole('link', { name: 'Projects', exact: true }).click() await page.getByRole('table').getByRole('link', { name: 'mock-project' }).click() await page.getByRole('link', { name: 'Disks' }).click() - await expectRowVisible(page.getByRole('table'), { name: 'disk-2', Source: 'Deleted' }) + await expectRowVisible(page.getByRole('table'), { + name: 'disk-2', + Source: 'Image deleted', + }) }) // this is to some extent a test of our mock server implementation, but I want From d63b3d17e4deadc32aca60e898001e93e32f8e97 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 1 Jul 2026 17:06:37 -0500 Subject: [PATCH 08/10] dedupe source name resolution --- app/components/SnapshotDetailSideModal.tsx | 26 +-------- app/pages/project/snapshots/SnapshotsPage.tsx | 36 +----------- app/table/cells/DiskSourceCell.tsx | 35 ++---------- app/table/cells/SourceNameCell.tsx | 56 +++++++++++++++++++ 4 files changed, 66 insertions(+), 87 deletions(-) create mode 100644 app/table/cells/SourceNameCell.tsx diff --git a/app/components/SnapshotDetailSideModal.tsx b/app/components/SnapshotDetailSideModal.tsx index a808630d4..317f8b005 100644 --- a/app/components/SnapshotDetailSideModal.tsx +++ b/app/components/SnapshotDetailSideModal.tsx @@ -5,39 +5,17 @@ * * Copyright Oxide Computer Company */ -import { useQuery } from '@tanstack/react-query' - -import { api, qErrorsAllowed, type Snapshot } from '@oxide/api' +import { type Snapshot } from '@oxide/api' import { Snapshots16Icon } from '@oxide/design-system/icons/react' -import { Badge } from '@oxide/design-system/ui' import { ReadOnlySideModalForm } from '~/components/form/ReadOnlySideModalForm' import { SnapshotStateBadge } from '~/components/StateBadge' -import { SkeletonCell } from '~/table/cells/EmptyCell' +import { DiskNameFromId } from '~/table/cells/SourceNameCell' import { SideModalFormDocs } from '~/ui/lib/ModalLinks' import { PropertiesTable } from '~/ui/lib/PropertiesTable' import { ResourceLabel } from '~/ui/lib/SideModal' import { docLinks } from '~/util/links' -const sourceDiskQ = (disk: string) => - qErrorsAllowed( - api.diskView, - { path: { disk } }, - { - errorsExpected: { - explanation: 'the source disk may have been deleted.', - statusCode: 404, - }, - } - ) - -const DiskNameFromId = ({ diskId }: { diskId: string }) => { - const { data } = useQuery(sourceDiskQ(diskId)) - if (!data) return - if (data.type === 'error') return Deleted - return <>{data.data.name} -} - type SnapshotDetailSideModalProps = { snapshot: Snapshot onDismiss: () => void diff --git a/app/pages/project/snapshots/SnapshotsPage.tsx b/app/pages/project/snapshots/SnapshotsPage.tsx index 47bda59cb..53177fac4 100644 --- a/app/pages/project/snapshots/SnapshotsPage.tsx +++ b/app/pages/project/snapshots/SnapshotsPage.tsx @@ -5,7 +5,6 @@ * * Copyright Oxide Computer Company */ -import { useQuery } from '@tanstack/react-query' import { createColumnHelper } from '@tanstack/react-table' import { useCallback, useState } from 'react' import { Outlet, useNavigate, type LoaderFunctionArgs } from 'react-router' @@ -14,14 +13,12 @@ import { api, getListQFn, q, - qErrorsAllowed, queryClient, useApiMutation, type Disk, type Snapshot, } from '@oxide/api' import { Snapshots16Icon, Snapshots24Icon } from '@oxide/design-system/icons/react' -import { Badge } from '@oxide/design-system/ui' import { DocsPopover } from '~/components/DocsPopover' import { SnapshotStateBadge } from '~/components/StateBadge' @@ -30,8 +27,7 @@ import { getProjectSelector, useProjectSelector } from '~/hooks/use-params' import { useQuickActions } from '~/hooks/use-quick-actions' import { DiskDetailSideModal } from '~/pages/project/disks/DiskDetailSideModal' import { confirmDelete } from '~/stores/confirm-delete' -import { SkeletonCell } from '~/table/cells/EmptyCell' -import { ButtonCell } from '~/table/cells/LinkCell' +import { DiskNameFromId, sourceDiskQ } from '~/table/cells/SourceNameCell' import { useColsWithActions, type MenuAction } from '~/table/columns/action-col' import { Columns } from '~/table/columns/common' import { useQueryTable } from '~/table/QueryTable' @@ -42,32 +38,6 @@ import { TableActions } from '~/ui/lib/Table' import { docLinks } from '~/util/links' import { pb } from '~/util/path-builder' -const diskViewErrorsAllowedQ = (disk: string) => - qErrorsAllowed( - api.diskView, - { path: { disk } }, - { - errorsExpected: { - explanation: 'the source disk may have been deleted.', - statusCode: 404, - }, - } - ) - -const DiskNameFromId = ({ - value, - onClick, -}: { - value: string - onClick: (disk: Disk) => void -}) => { - const { data } = useQuery(diskViewErrorsAllowedQ(value)) - - if (!data) return - if (data.type === 'error') return Deleted - return onClick(data.data)}>{data.data.name} -} - const EmptyState = () => ( } @@ -98,7 +68,7 @@ export async function clientLoader({ params }: LoaderFunctionArgs) { .fetchQuery(q(api.diskList, { query: { project, limit: 200 } })) .then((disks) => { for (const disk of disks.items) { - queryClient.setQueryData(diskViewErrorsAllowedQ(disk.id).queryKey, { + queryClient.setQueryData(sourceDiskQ(disk.id).queryKey, { type: 'success', data: disk, }) @@ -122,7 +92,7 @@ export default function SnapshotsPage() { colHelper.accessor('description', Columns.description), colHelper.accessor('diskId', { header: 'disk', - cell: (info) => , + cell: (info) => , }), colHelper.accessor('state', { cell: (info) => , diff --git a/app/table/cells/DiskSourceCell.tsx b/app/table/cells/DiskSourceCell.tsx index 5325ce3b9..26f2bb5e8 100644 --- a/app/table/cells/DiskSourceCell.tsx +++ b/app/table/cells/DiskSourceCell.tsx @@ -9,7 +9,6 @@ import { useQuery } from '@tanstack/react-query' import { useState } from 'react' -import { api, qErrorsAllowed } from '@oxide/api' import { Badge } from '@oxide/design-system/ui' import { ImageDetailSideModal } from '~/components/ImageDetailSideModal' @@ -18,34 +17,7 @@ import { useIsInSideModal } from '~/ui/lib/modal-context' import { EmptyCell, SkeletonCell } from './EmptyCell' import { ButtonCell } from './LinkCell' - -// Use qErrorsAllowed so deletion of the source resource is a cacheable result -// rather than an error that blows up the page. Tables and the disk detail -// modal both render a "Deleted" badge in that case. - -const sourceImageQ = (image: string) => - qErrorsAllowed( - api.imageView, - { path: { image } }, - { - errorsExpected: { - explanation: 'the source image may have been deleted.', - statusCode: 404, - }, - } - ) - -const sourceSnapshotQ = (snapshot: string) => - qErrorsAllowed( - api.snapshotView, - { path: { snapshot } }, - { - errorsExpected: { - explanation: 'the source snapshot may have been deleted.', - statusCode: 404, - }, - } - ) +import { sourceImageQ, sourceSnapshotQ } from './SourceNameCell' type Props = { imageId?: string | null @@ -63,7 +35,10 @@ export const DiskSourceName = ({ imageId, snapshotId }: Props) => { const [showDetail, setShowDetail] = useState(false) // the `!` is safe because the query only runs when the id is present (enabled) const image = useQuery({ ...sourceImageQ(imageId!), enabled: !!imageId }) - const snapshot = useQuery({ ...sourceSnapshotQ(snapshotId!), enabled: !!snapshotId }) + const snapshot = useQuery({ + ...sourceSnapshotQ(snapshotId!), + enabled: !!snapshotId, + }) if (!imageId && !snapshotId) return diff --git a/app/table/cells/SourceNameCell.tsx b/app/table/cells/SourceNameCell.tsx new file mode 100644 index 000000000..967e2b5c2 --- /dev/null +++ b/app/table/cells/SourceNameCell.tsx @@ -0,0 +1,56 @@ +/* + * 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 { useQuery } from '@tanstack/react-query' + +import { api, qErrorsAllowed, type Disk } from '@oxide/api' +import { Badge } from '@oxide/design-system/ui' + +import { SkeletonCell } from './EmptyCell' +import { ButtonCell } from './LinkCell' + +// Views of a resource another resource was created from. Use qErrorsAllowed so +// deletion of the source is a cacheable result rather than an error that blows +// up the page; consumers render a "Deleted" badge in that case. + +const deletedOk = (resource: string) => ({ + errorsExpected: { + explanation: `the source ${resource} may have been deleted.`, + statusCode: 404, + }, +}) + +export const sourceDiskQ = (disk: string) => + qErrorsAllowed(api.diskView, { path: { disk } }, deletedOk('disk')) + +export const sourceImageQ = (image: string) => + qErrorsAllowed(api.imageView, { path: { image } }, deletedOk('image')) + +export const sourceSnapshotQ = (snapshot: string) => + qErrorsAllowed(api.snapshotView, { path: { snapshot } }, deletedOk('snapshot')) + +type DiskNameFromIdProps = { + diskId: string + /** When present, the name is a button. Otherwise it's plain text. */ + onClick?: (disk: Disk) => void +} + +/** + * Disk name resolved from ID. Renders a skeleton while loading and a "Deleted" + * badge if the disk no longer exists. + */ +export const DiskNameFromId = ({ diskId, onClick }: DiskNameFromIdProps) => { + const { data } = useQuery(sourceDiskQ(diskId)) + + if (!data) return + if (data.type === 'error') return Deleted + + const disk = data.data + if (!onClick) return <>{disk.name} + return onClick(disk)}>{disk.name} +} From 99dfb39b454d363865e2aa1e5e6b106f97806127 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 1 Jul 2026 17:12:22 -0500 Subject: [PATCH 09/10] remove no-op snapshot invalidations in image upload --- app/forms/image-upload.tsx | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/app/forms/image-upload.tsx b/app/forms/image-upload.tsx index a7e407558..f5c398266 100644 --- a/app/forms/image-upload.tsx +++ b/app/forms/image-upload.tsx @@ -246,12 +246,9 @@ export default function ImageCreate() { const finalizeDisk = useApiMutation(api.diskFinalizeImport) const createImage = useApiMutation(api.imageCreate) const deleteDisk = useApiMutation(api.diskDelete) - const deleteSnapshot = useApiMutation(api.snapshotDelete, { - onSuccess() { - queryClient.invalidateEndpoint('snapshotList') - queryClient.invalidateEndpoint('snapshotView') - }, - }) + // no invalidation needed: the deleted snapshot is the transient one created + // by this flow, so nothing can be displaying it + const deleteSnapshot = useApiMutation(api.snapshotDelete) // TODO: Distinguish cleanup mutations being called after successful run vs. // due to error. In the former case, they have their own steps to highlight as @@ -282,7 +279,6 @@ export default function ImageCreate() { const deleteSnapshotCleanup = useApiMutation(api.snapshotDelete, { onSuccess() { queryClient.invalidateEndpoint('snapshotList') - queryClient.invalidateEndpoint('snapshotView') }, }) From d3aac53f4a62f9012ea7f4313604650d93c148e4 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 1 Jul 2026 17:19:21 -0500 Subject: [PATCH 10/10] restore IdRow e2e coverage in disk detail modal --- test/e2e/disks.e2e.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/e2e/disks.e2e.ts b/test/e2e/disks.e2e.ts index b5fc0e2fa..2ad7c48c8 100644 --- a/test/e2e/disks.e2e.ts +++ b/test/e2e/disks.e2e.ts @@ -27,6 +27,12 @@ test('Disk detail side modal', async ({ page }) => { await expect(modal.getByText('2 GiB')).toBeVisible() await expect(modal.getByText('2,048 bytes')).toBeVisible() // block size await expect(propertiesTableValue(modal, 'Read only')).toHaveText('False') + + // the ID is truncated for display, but the full ID is in the aria-label, + // next to a copy button + const idCell = propertiesTableValue(modal, 'ID') + await expect(idCell.getByLabel('7f2309a5-13e3-47e0-8a4c-2a3b3bc992fd')).toBeVisible() + await expect(idCell.getByRole('button', { name: 'Click to copy' })).toBeVisible() }) test('Source links open detail side modals from disk list', async ({ page }) => {