From c8ee579b5f3737b1573e74fb4d48b919ca3d2d1c Mon Sep 17 00:00:00 2001 From: Allison Truhlar Date: Wed, 19 Aug 2026 10:27:19 -0400 Subject: [PATCH 01/16] fix(views): cart rows expand without a Data Link Metadata + channels are fetched from the internal /api/content URL (credentialed) instead of the proxied-path URL, so every cart dataset can be inspected before a View is created - matching the behavior users already get on the browse page. --- .../componentTests/CartList.test.tsx | 3 - .../__tests__/componentTests/CartTab.test.tsx | 52 ++++++------ .../components/ui/Views/CartDatasetRow.tsx | 82 +++++++++---------- frontend/src/components/ui/Views/CartList.tsx | 12 --- 4 files changed, 62 insertions(+), 87 deletions(-) diff --git a/frontend/src/__tests__/componentTests/CartList.test.tsx b/frontend/src/__tests__/componentTests/CartList.test.tsx index f8ff6e6f..bd6512da 100644 --- a/frontend/src/__tests__/componentTests/CartList.test.tsx +++ b/frontend/src/__tests__/componentTests/CartList.test.tsx @@ -28,9 +28,6 @@ vi.mock('@/contexts/CartContext', () => ({ clearCart }) })); -vi.mock('@/queries/proxiedPathQueries', () => ({ - useAllProxiedPathsQuery: () => ({ data: [] }) -})); vi.mock('@/components/ui/Views/CartDatasetRow', () => ({ default: ({ label }: { label: string }) => (
{label}
diff --git a/frontend/src/__tests__/componentTests/CartTab.test.tsx b/frontend/src/__tests__/componentTests/CartTab.test.tsx index 408f0fa1..672c66b4 100644 --- a/frontend/src/__tests__/componentTests/CartTab.test.tsx +++ b/frontend/src/__tests__/componentTests/CartTab.test.tsx @@ -18,10 +18,11 @@ const view: View = { layers: [] }; -// Dataset A has an existing Data Link (channel expansion enabled) and TWO -// cart entries (a base entry + an already-checked "GFP" channel entry), to -// exercise the multi-entry "Remove" batch path. -// Dataset B has no Data Link (channel expansion disabled + hint). +// Dataset A has TWO cart entries (a base entry + an already-checked "GFP" +// channel entry), to exercise the multi-entry "Remove" batch path. +// Dataset B is a plain single-entry dataset - both expand identically now +// that metadata is fetched from the internal /api/content URL rather than +// a Data Link. const cartABase: CartItem = { fsp_name: 'fsp1', path: '/a', @@ -92,20 +93,6 @@ vi.mock('@/omezarr-helper', () => ({ getResolvedScales: () => [1, 0.65, 0.65], translateUnitToNeuroglancer: (unit?: string) => unit ?? '' })); -vi.mock('@/queries/proxiedPathQueries', () => ({ - useAllProxiedPathsQuery: () => ({ - data: [ - { - fsp_name: 'fsp1', - path: '/a', - url: 'https://data.example/a', - sharing_key: 'k1' - } - ], - error: null, - isPending: false - }) -})); vi.mock('@/components/ui/Views/CreateViewButton', () => ({ default: ({ label }: { label?: string }) => ( @@ -140,18 +127,20 @@ describe('Layer Cart tab', () => { expect(screen.getByText('Dataset B')).toBeInTheDocument(); }); - it('lazy-loads and shows channels when expanding a dataset with a Data Link', async () => { + it('lazy-loads and shows channels when expanding a dataset', async () => { const user = await renderCartTab(); await user.click(screen.getByRole('button', { name: 'Dataset A' })); await waitFor(() => { - expect(getOmeZarrChannels).toHaveBeenCalledWith('https://data.example/a'); + expect(getOmeZarrChannels).toHaveBeenCalledWith( + expect.stringContaining('/api/content/fsp1/a') + ); }); expect(await screen.findByText('DAPI')).toBeInTheDocument(); expect(screen.getByText('GFP')).toBeInTheDocument(); }); - it('lazy-loads and shows the axis table when expanding a dataset with a Data Link', async () => { + it('lazy-loads and shows the axis table when expanding a dataset', async () => { getOmeZarrMetadata.mockResolvedValueOnce({ shapes: [[3, 2048, 2048]], arr: { chunks: [1, 512, 512] }, @@ -172,7 +161,9 @@ describe('Layer Cart tab', () => { await user.click(screen.getByRole('button', { name: /Dataset A/ })); await waitFor(() => { - expect(getOmeZarrMetadata).toHaveBeenCalledWith('https://data.example/a'); + expect(getOmeZarrMetadata).toHaveBeenCalledWith( + expect.stringContaining('/api/content/fsp1/a') + ); }); expect(await screen.findByText('Chunk Size')).toBeInTheDocument(); }); @@ -193,14 +184,17 @@ describe('Layer Cart tab', () => { expect(screen.queryByText(/×/)).not.toBeInTheDocument(); }); - it('disables expansion and shows a hint for a dataset with no Data Link', async () => { - await renderCartTab(); + it('expands a dataset that has no Data Link (metadata fetched via /api/content)', async () => { + const user = await renderCartTab(); const expandButton = screen.getByRole('button', { name: 'Dataset B' }); - expect(expandButton).toBeDisabled(); - expect( - screen.getByText(/channels load after the view is created/i) - ).toBeInTheDocument(); - expect(getOmeZarrChannels).not.toHaveBeenCalled(); + expect(expandButton).not.toBeDisabled(); + await user.click(expandButton); + + await waitFor(() => { + expect(getOmeZarrChannels).toHaveBeenCalledWith( + expect.stringContaining('/api/content/fsp2/b') + ); + }); }); it('toggling a channel checkbox adds a channel-specific CartItem', async () => { diff --git a/frontend/src/components/ui/Views/CartDatasetRow.tsx b/frontend/src/components/ui/Views/CartDatasetRow.tsx index 043007d3..7c3a79d4 100644 --- a/frontend/src/components/ui/Views/CartDatasetRow.tsx +++ b/frontend/src/components/ui/Views/CartDatasetRow.tsx @@ -11,6 +11,7 @@ import { useCartContext } from '@/contexts/CartContext'; import { getOmeZarrChannels, getOmeZarrMetadata } from '@/omezarr-helper'; import type { Metadata } from '@/omezarr-helper'; import { makeBrowseLink } from '@/utils'; +import { getFileURL } from '@/utils/pathHandling'; import type { CartItem } from '@/contexts/CartContext'; interface CartDatasetRowProps { @@ -18,20 +19,18 @@ interface CartDatasetRowProps { readonly path: string; readonly label: string; readonly items: CartItem[]; - readonly dataLinkUrl: string | undefined; } // ponytail: two-level dataset->channel tree via MT Collapse (no generic -// TreeView exists). Channel URL comes from an existing Data Link; if a -// dataset has no link yet, expansion is disabled with a hint rather than -// creating a link just to browse channels. Non-Zarr/N5 datasets simply fail -// getOmeZarrChannels gracefully (toast) instead of a hard pre-check. +// TreeView exists). Metadata/channels are fetched from the internal +// /api/content URL (credentialed), so no Data Link is required to inspect +// dimensions or pick channels. Non-Zarr/N5 datasets fail gracefully in +// getOmeZarrChannels (toast) instead of a hard pre-check. export default function CartDatasetRow({ fsp_name, path, label, - items, - dataLinkUrl + items }: CartDatasetRowProps) { const { addToCart, removeFromCart, removeManyFromCart } = useCartContext(); const [isOpen, setIsOpen] = useState(false); @@ -47,10 +46,14 @@ export default function CartDatasetRow({ const handleToggleOpen = async () => { const nextOpen = !isOpen; setIsOpen(nextOpen); - if (nextOpen && channels === undefined && !loadingChannels && dataLinkUrl) { + if (!nextOpen) { + return; + } + const dataUrl = getFileURL(fsp_name, path); + if (channels === undefined && !loadingChannels) { setLoadingChannels(true); try { - setChannels(await getOmeZarrChannels(dataLinkUrl)); + setChannels(await getOmeZarrChannels(dataUrl)); } catch (error) { toast.error( error instanceof Error ? error.message : 'Failed to load channels' @@ -60,10 +63,10 @@ export default function CartDatasetRow({ setLoadingChannels(false); } } - if (nextOpen && metadata === null && !loadingMeta && dataLinkUrl) { + if (metadata === null && !loadingMeta) { setLoadingMeta(true); try { - setMetadata(await getOmeZarrMetadata(dataLinkUrl)); + setMetadata(await getOmeZarrMetadata(dataUrl)); } catch { // Metadata is a nice-to-have here; ignore fetch failures. } finally { @@ -108,8 +111,7 @@ export default function CartDatasetRow({