From e116a2d561b7a2367f1c9e43dba4ced677fc82bc Mon Sep 17 00:00:00 2001 From: Chris Barber Date: Tue, 8 Sep 2026 18:26:15 -0500 Subject: [PATCH 1/5] refactor(databases): move Import Data and Export CSV into the table's "..." menu The browse-data toolbar led with three primary-looking buttons, but only Add New Record(s) is a common action -- Import Data and Export CSV are occasional, and they crowded out the row for no gain. Both move into the existing "..." menu alongside the other per-table actions, keeping their permission gate (`canImportData`) and the in-flight `isExportingCSV` disabled state. Separators now group the menu: one after Export CSV to split the data-transfer pair from Only If Cached / Cleanup Orphan Blobs, and one before Drop Table / Drop Database so the destructive items are visibly set apart. The second is gated on `canManageBrowseInstance` so a user without those items doesn't get a trailing separator. The `accessKey="i"` / `"e"` shortcuts are dropped with the buttons -- access keys only fire on rendered elements, and the menu's content isn't in the DOM while it's closed. `n` (Add New Record(s)) and `f` (filters) are unchanged. Co-Authored-By: Claude Opus 5 (1M context) --- .../components/DatabaseTableView.tsx | 43 ++++++++----------- 1 file changed, 19 insertions(+), 24 deletions(-) diff --git a/src/features/instance/databases/components/DatabaseTableView.tsx b/src/features/instance/databases/components/DatabaseTableView.tsx index a11f46441..669a63ba5 100644 --- a/src/features/instance/databases/components/DatabaseTableView.tsx +++ b/src/features/instance/databases/components/DatabaseTableView.tsx @@ -1,5 +1,11 @@ import { Button } from '@/components/ui/button'; -import { DropdownMenu, DropdownMenuContent, DropdownMenuItem, DropdownMenuTrigger } from '@/components/ui/dropdownMenu'; +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuSeparator, + DropdownMenuTrigger, +} from '@/components/ui/dropdownMenu'; import { useInstanceClientIdParams } from '@/config/useInstanceClient'; import { formatBrowseDataTableHeader } from '@/features/instance/databases/functions/formatBrowseDataTableHeader'; import { @@ -534,29 +540,6 @@ export function DatabaseTableView({ instanceDatabaseMap, databaseName, tableName )} - {canImportData && ( - - )} -
@@ -613,6 +596,17 @@ export function DatabaseTableView({ instanceDatabaseMap, databaseName, tableName + {canImportData && ( + + + Import Data + + )} + + + Export CSV + + {onlyIfCached ? : } Only If Cached @@ -637,6 +631,7 @@ export function DatabaseTableView({ instanceDatabaseMap, databaseName, tableName Cleanup Orphan Blobs )} + {canManageBrowseInstance && } {canManageBrowseInstance && !isLastTableInDatabase && ( Date: Tue, 8 Sep 2026 23:32:38 -0500 Subject: [PATCH 2/5] fix(databases): stop the table menu's Export CSV inheriting a describe_all gate The "..." trigger carried `disabled={!instanceDatabaseMap}`, added back when the menu held nothing but Drop Table / Drop Database -- the split between those two turns on how many tables the database has, which only `describe_all` can answer. Moving Export CSV into the menu made it inherit that gate, though export needs only the separately fetched table schema and the search operation. An operation allowlist can grant `describe_table` and search without `describe_all`, and both describes use `retry: false`, so such a role gets a table that reads normally with its only export control permanently dead. The gate moves to where the dependency actually is: the trigger and its button lose `disabled`, and Drop Table alone waits on the map. Drop Database keeps just the manage-permission gate -- it acts on the database named by the route. DatabaseTableView had no tests; this adds its first, covering the reported case (map absent, table schema present). Co-Authored-By: Claude Opus 5 (1M context) --- .../components/DatabaseTableView.test.tsx | 106 ++++++++++++++++++ .../components/DatabaseTableView.tsx | 6 +- 2 files changed, 109 insertions(+), 3 deletions(-) create mode 100644 src/features/instance/databases/components/DatabaseTableView.test.tsx diff --git a/src/features/instance/databases/components/DatabaseTableView.test.tsx b/src/features/instance/databases/components/DatabaseTableView.test.tsx new file mode 100644 index 000000000..62f3079e6 --- /dev/null +++ b/src/features/instance/databases/components/DatabaseTableView.test.tsx @@ -0,0 +1,106 @@ +/** + * @vitest-environment jsdom + */ +import { cleanup, fireEvent, render, screen } from '@testing-library/react'; +import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest'; +import { DatabaseTableView } from './DatabaseTableView'; + +const stableParams = vi.hoisted(() => ({ + instance: { instanceId: 'instance-1' }, + client: { instanceClient: {}, entityId: 'instance-1', entityType: 'instance' }, + search: {}, +})); + +vi.mock('@tanstack/react-router', () => ({ + Link: ({ children }: { children: React.ReactNode }) => {children}, + useParams: () => stableParams.instance, + useSearch: () => stableParams.search, +})); + +vi.mock('@tanstack/react-query', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + useQuery: (options: { queryKey?: readonly unknown[] }) => ({ + data: options.queryKey?.at(-1) === 'describe_table' + ? { primary_key: 'id', attributes: [{ attribute: 'id', type: 'String', is_primary_key: true }] } + : undefined, + isError: false, + isFetching: false, + refetch: vi.fn(), + }), + useQueryClient: () => ({ invalidateQueries: vi.fn() }), + }; +}); + +vi.mock('@/config/useInstanceClient', () => ({ + useInstanceClientIdParams: () => stableParams.client, +})); + +vi.mock('@/hooks/useAuth', () => ({ useStaffPermission: () => false })); + +vi.mock('@/hooks/usePermissions', () => ({ + useInstanceBrowseManagePermission: () => true, + useInstanceImportDataPermission: () => true, + useInstanceSchemaTablePermission: () => true, + useInstanceTablePutPermission: () => true, +})); + +vi.mock('@/features/instance/databases/hooks/useExportTableCsv', () => ({ + useExportTableCsv: () => ({ exportCsv: vi.fn(), isExporting: false }), +})); + +vi.mock('@/integrations/api/instance/database/cleanupOrphanBlobs', () => ({ + useCleanupOrphanBlobsMutation: () => ({ mutate: vi.fn(), isPending: false }), +})); + +vi.mock('@/integrations/api/instance/database/deleteTableRecords', () => ({ + useDeleteTableRecords: () => ({ mutate: vi.fn(), isPending: false }), +})); + +vi.mock('@/integrations/api/instance/database/putTableRecords', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + usePutTableRecords: () => ({ mutate: vi.fn(), isPending: false }), + }; +}); + +vi.mock('@/integrations/api/instance/database/updateTableRecords', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + useUpdateTableRecords: () => ({ mutate: vi.fn(), isPending: false }), + }; +}); + +vi.mock('./PickColumnsDropdown', () => ({ PickColumnsDropdown: () => null })); +vi.mock('./TableView', () => ({ TableView: () => null })); +vi.mock('@/features/instance/databases/modals/EditTableRowModal', () => ({ EditTableRowModal: () => null })); + +beforeAll(() => { + Element.prototype.hasPointerCapture ??= () => false; + Element.prototype.setPointerCapture ??= () => undefined; + Element.prototype.releasePointerCapture ??= () => undefined; + Element.prototype.scrollIntoView ??= () => undefined; + if (typeof window.PointerEvent === 'undefined') { + window.PointerEvent = class extends MouseEvent {} as typeof PointerEvent; + } +}); + +afterEach(() => cleanup()); + +describe('DatabaseTableView table options', () => { + it('keeps schema-backed actions available when the database map is unavailable', () => { + render(); + + const trigger = screen.getByRole('button', { name: 'Table options' }); + expect(trigger.hasAttribute('disabled')).toBe(false); + fireEvent.pointerDown(trigger, { button: 0, ctrlKey: false }); + + expect(screen.getByRole('menuitem', { name: 'Import Data' })).toBeTruthy(); + expect(screen.getByRole('menuitem', { name: 'Export CSV' })).toBeTruthy(); + expect(screen.queryByRole('menuitem', { name: 'Drop Table' })).toBeNull(); + expect(screen.getByRole('menuitem', { name: 'Drop Database' })).toBeTruthy(); + }); +}); diff --git a/src/features/instance/databases/components/DatabaseTableView.tsx b/src/features/instance/databases/components/DatabaseTableView.tsx index 669a63ba5..7216c0301 100644 --- a/src/features/instance/databases/components/DatabaseTableView.tsx +++ b/src/features/instance/databases/components/DatabaseTableView.tsx @@ -590,8 +590,8 @@ export function DatabaseTableView({ instanceDatabaseMap, databaseName, tableName /> - - @@ -632,7 +632,7 @@ export function DatabaseTableView({ instanceDatabaseMap, databaseName, tableName )} {canManageBrowseInstance && } - {canManageBrowseInstance && !isLastTableInDatabase && ( + {canManageBrowseInstance && !!databaseTables && !isLastTableInDatabase && ( setWatchedValue('ShowDeleteTable', { databaseName, tableName })} From 699a0293ee4f8709de1158178785f21117715193 Mon Sep 17 00:00:00 2001 From: Chris Barber Date: Tue, 8 Sep 2026 23:38:17 -0500 Subject: [PATCH 3/5] refactor(databases): make Drop Table's map dependency the condition it gates on Builds on the previous commit's fix. `isLastTableInDatabase` answered `false` both when another table remains and when `describe_all` simply hasn't said -- so the call site had to carry `!!databaseTables &&` alongside it to stay honest, and any future reader of the flag alone would get the unsafe reading. Phrasing it as the decision instead (`canDropTable`: manage permission plus positive evidence that another table remains) leaves no unsafe reading to pick. Also widens the new test file: Drop Database stays available with the map absent (it acts on the database named by the route and never needed the map), Drop Table is withheld both for an absent map and for a resolved map that doesn't list this database, the single-table and cannot-manage cases are pinned, and the absent-map case now asserts the grid really did come up from describe_table alone -- so it tests a readable table whose export is dead, not a half-loaded view. Co-Authored-By: Claude Opus 5 (1M context) --- .../components/DatabaseTableView.test.tsx | 205 +++++++++++++----- .../components/DatabaseTableView.tsx | 15 +- 2 files changed, 155 insertions(+), 65 deletions(-) diff --git a/src/features/instance/databases/components/DatabaseTableView.test.tsx b/src/features/instance/databases/components/DatabaseTableView.test.tsx index 62f3079e6..473ca788e 100644 --- a/src/features/instance/databases/components/DatabaseTableView.test.tsx +++ b/src/features/instance/databases/components/DatabaseTableView.test.tsx @@ -1,83 +1,75 @@ /** * @vitest-environment jsdom */ +import { InstanceDatabaseMap, InstanceTable } from '@/integrations/api/api.patch'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; import { cleanup, fireEvent, render, screen } from '@testing-library/react'; import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest'; import { DatabaseTableView } from './DatabaseTableView'; -const stableParams = vi.hoisted(() => ({ - instance: { instanceId: 'instance-1' }, - client: { instanceClient: {}, entityId: 'instance-1', entityType: 'instance' }, - search: {}, -})); - -vi.mock('@tanstack/react-router', () => ({ - Link: ({ children }: { children: React.ReactNode }) => {children}, - useParams: () => stableParams.instance, - useSearch: () => stableParams.search, -})); +// The dropdown's own permission gate is the thing under test; every other permission hook just +// needs a fixed answer so the toolbar around it renders without pulling in the auth store/router. +const permissionState = vi.hoisted(() => ({ canManageBrowseInstance: true })); -vi.mock('@tanstack/react-query', async (importOriginal) => { - const actual = await importOriginal(); +vi.mock('@tanstack/react-router', () => { + // Stable references: the component keys effects off these objects' identity, and the real + // router hooks only produce a new one when params/search actually change. + const params = {}; + const search = {}; return { - ...actual, - useQuery: (options: { queryKey?: readonly unknown[] }) => ({ - data: options.queryKey?.at(-1) === 'describe_table' - ? { primary_key: 'id', attributes: [{ attribute: 'id', type: 'String', is_primary_key: true }] } - : undefined, - isError: false, - isFetching: false, - refetch: vi.fn(), - }), - useQueryClient: () => ({ invalidateQueries: vi.fn() }), + useParams: () => params, + useSearch: () => search, + Link: ({ children }: { children?: React.ReactNode }) => <>{children}, }; }); vi.mock('@/config/useInstanceClient', () => ({ - useInstanceClientIdParams: () => stableParams.client, + useInstanceClientIdParams: () => ({ entityId: 'instance-1', instanceClient: {}, entityType: 'instance' }), })); -vi.mock('@/hooks/useAuth', () => ({ useStaffPermission: () => false })); +vi.mock('@/hooks/useAuth', () => ({ + useStaffPermission: () => false, +})); vi.mock('@/hooks/usePermissions', () => ({ - useInstanceBrowseManagePermission: () => true, + useInstanceBrowseManagePermission: () => permissionState.canManageBrowseInstance, useInstanceImportDataPermission: () => true, useInstanceSchemaTablePermission: () => true, useInstanceTablePutPermission: () => true, })); -vi.mock('@/features/instance/databases/hooks/useExportTableCsv', () => ({ - useExportTableCsv: () => ({ exportCsv: vi.fn(), isExporting: false }), -})); +// The grid and row editor aren't what this file pins -- swap them for stubs so a render doesn't +// need real table data or a Radix Dialog. TableView's stub keeps its props, so a test can still +// show the grid was built from the schema this render actually had. +const tableViewColumns = vi.hoisted(() => ({ current: [] as { accessorKey?: string }[] })); -vi.mock('@/integrations/api/instance/database/cleanupOrphanBlobs', () => ({ - useCleanupOrphanBlobsMutation: () => ({ mutate: vi.fn(), isPending: false }), -})); - -vi.mock('@/integrations/api/instance/database/deleteTableRecords', () => ({ - useDeleteTableRecords: () => ({ mutate: vi.fn(), isPending: false }), +vi.mock('./TableView', () => ({ + TableView: ({ columns }: { columns: { accessorKey?: string }[] }) => { + tableViewColumns.current = columns; + return null; + }, })); +vi.mock('./PickColumnsDropdown', () => ({ PickColumnsDropdown: () => null })); +vi.mock('../modals/EditTableRowModal', () => ({ EditTableRowModal: () => null })); -vi.mock('@/integrations/api/instance/database/putTableRecords', async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - usePutTableRecords: () => ({ mutate: vi.fn(), isPending: false }), - }; -}); +// describe_table is the table's own schema fetch; every other query this component reads is +// irrelevant to the menu and comes back empty. Matching by queryKey (rather than mocking each +// `get*QueryOptions` builder) keeps the real gating logic -- which reads `instanceDatabaseMap` +// straight from props -- exercised as written. +const describeTableData = vi.hoisted(() => ({ current: undefined as InstanceTable | undefined })); -vi.mock('@/integrations/api/instance/database/updateTableRecords', async (importOriginal) => { - const actual = await importOriginal(); +vi.mock('@tanstack/react-query', async (importOriginal) => { + const actual = await importOriginal(); return { ...actual, - useUpdateTableRecords: () => ({ mutate: vi.fn(), isPending: false }), + useQuery: (options: { queryKey: readonly unknown[] }) => + options.queryKey.includes('describe_table') + ? { data: describeTableData.current, isFetching: false, isError: false } + : { data: undefined, isFetching: false, isError: false, refetch: vi.fn() }, }; }); -vi.mock('./PickColumnsDropdown', () => ({ PickColumnsDropdown: () => null })); -vi.mock('./TableView', () => ({ TableView: () => null })); -vi.mock('@/features/instance/databases/modals/EditTableRowModal', () => ({ EditTableRowModal: () => null })); - +// Radix's dropdown opens on pointerdown and probes pointer-capture APIs jsdom doesn't implement. beforeAll(() => { Element.prototype.hasPointerCapture ??= () => false; Element.prototype.setPointerCapture ??= () => undefined; @@ -88,19 +80,112 @@ beforeAll(() => { } }); -afterEach(() => cleanup()); - -describe('DatabaseTableView table options', () => { - it('keeps schema-backed actions available when the database map is unavailable', () => { - render(); +afterEach(() => { + cleanup(); + permissionState.canManageBrowseInstance = true; + tableViewColumns.current = []; +}); - const trigger = screen.getByRole('button', { name: 'Table options' }); +const dogTable = { + attributes: [{ attribute: 'id', type: 'string', is_primary_key: true, indexed: true }], + primary_key: 'id', +} as unknown as InstanceTable; + +function renderView( + { instanceDatabaseMap }: { instanceDatabaseMap?: InstanceDatabaseMap } = {}, +) { + describeTableData.current = dogTable; + const queryClient = new QueryClient(); + return render( + + + , + ); +} + +function openTableOptions() { + fireEvent.pointerDown(screen.getByRole('button', { name: /table options/i }), { button: 0, ctrlKey: false }); +} + +const exportCsvItem = () => screen.queryByText('Export CSV'); +const importDataItem = () => screen.queryByText('Import Data'); +const dropTableItem = () => screen.queryByText('Drop Table'); +const dropDatabaseItem = () => screen.queryByText('Drop Database'); + +function isDisabled(el: HTMLElement) { + return el.getAttribute('aria-disabled') === 'true' || el.hasAttribute('data-disabled'); +} + +describe('DatabaseTableView table options menu', () => { + // `describe_all` (the map) can be slower or unreachable for a role whose allowlist grants + // describe_table + search but not describe_all -- the trigger must not gate on it, or Export + // CSV becomes unreachable for that role. + it('is not disabled and offers Export CSV and Import Data while the database map is absent', () => { + renderView({ instanceDatabaseMap: undefined }); + + const trigger = screen.getByRole('button', { name: /table options/i }); expect(trigger.hasAttribute('disabled')).toBe(false); - fireEvent.pointerDown(trigger, { button: 0, ctrlKey: false }); - expect(screen.getByRole('menuitem', { name: 'Import Data' })).toBeTruthy(); - expect(screen.getByRole('menuitem', { name: 'Export CSV' })).toBeTruthy(); - expect(screen.queryByRole('menuitem', { name: 'Drop Table' })).toBeNull(); - expect(screen.getByRole('menuitem', { name: 'Drop Database' })).toBeTruthy(); + openTableOptions(); + + expect(exportCsvItem()).not.toBeNull(); + expect(isDisabled(exportCsvItem()!.closest('[role="menuitem"]')!)).toBe(false); + expect(importDataItem()).not.toBeNull(); + + // The grid came up from describe_table on its own, so this is a table the user can read and + // therefore expects to be able to export -- not a half-loaded view. + expect(tableViewColumns.current.map(({ accessorKey }) => accessorKey)).toContain('id'); + }); + + // Whether another table would remain is something only the map can answer, so Drop Table waits + // for it. Drop Database never needed the map -- it acts on the database named by the route. + it('withholds Drop Table but keeps Drop Database while the database map is absent', () => { + renderView({ instanceDatabaseMap: undefined }); + + openTableOptions(); + + expect(dropTableItem()).toBeNull(); + expect(dropDatabaseItem()).not.toBeNull(); + }); + + // A resolved map that doesn't list this database is just as uninformative as no map at all -- + // counting its (zero) tables would otherwise read as "not the last one". + it('withholds Drop Table when the map resolved without this database', () => { + renderView({ instanceDatabaseMap: { other: { cat: {} } } as never }); + + openTableOptions(); + + expect(dropTableItem()).toBeNull(); + }); + + it('offers Drop Table and Drop Database once the map resolves, for a role that can manage', () => { + renderView({ instanceDatabaseMap: { data: { dog: {}, cat: {} } } as never }); + + openTableOptions(); + + expect(dropTableItem()).not.toBeNull(); + expect(dropDatabaseItem()).not.toBeNull(); + }); + + it('offers neither drop entry to a role that cannot manage', () => { + permissionState.canManageBrowseInstance = false; + renderView({ instanceDatabaseMap: { data: { dog: {}, cat: {} } } as never }); + + openTableOptions(); + + expect(dropTableItem()).toBeNull(); + expect(dropDatabaseItem()).toBeNull(); + expect(exportCsvItem()).not.toBeNull(); + }); + + // Dropping the last table in a database is really dropping the database, so that entry alone + // covers it -- Drop Table would leave nothing to drop. + it('offers only Drop Database when this is the only table in the database', () => { + renderView({ instanceDatabaseMap: { data: { dog: {} } } as never }); + + openTableOptions(); + + expect(dropTableItem()).toBeNull(); + expect(dropDatabaseItem()).not.toBeNull(); }); }); diff --git a/src/features/instance/databases/components/DatabaseTableView.tsx b/src/features/instance/databases/components/DatabaseTableView.tsx index 7216c0301..aa8c07ca6 100644 --- a/src/features/instance/databases/components/DatabaseTableView.tsx +++ b/src/features/instance/databases/components/DatabaseTableView.tsx @@ -155,10 +155,15 @@ export function DatabaseTableView({ instanceDatabaseMap, databaseName, tableName // happen when a table's primary key was changed after rows existed; see #1199). const [clickedRow, setClickedRow] = useEffectedState | null>(null, allParams); - const isLastTableInDatabase = useMemo(() => { - const tableNames = databaseName ? Object.keys(instanceDatabaseMap?.[databaseName] || []).sort() : []; - return tableNames.length === 1; - }, [instanceDatabaseMap, databaseName]); + // Only `describe_all` (`instanceDatabaseMap`) knows how many tables the database has, and an + // allowlist can grant `describe_table` + search without it -- so a table can render fine while + // this stays unanswered. Phrased as the decision rather than the fact ("is this the last table?" + // answers `false` when it simply doesn't know, which would offer an irreversible action on a + // guess): dropping a table needs positive evidence that another one remains. + const canDropTable = useMemo( + () => canManageBrowseInstance && !!databaseTables && Object.keys(databaseTables).length > 1, + [canManageBrowseInstance, databaseTables], + ); const { toggled: filtersToggled, toggleOn: showFilters, toggleOff: hideFilters } = useToggler(false); const columnFiltersForm = useForm({ @@ -632,7 +637,7 @@ export function DatabaseTableView({ instanceDatabaseMap, databaseName, tableName )} {canManageBrowseInstance && } - {canManageBrowseInstance && !!databaseTables && !isLastTableInDatabase && ( + {canDropTable && ( setWatchedValue('ShowDeleteTable', { databaseName, tableName })} From ec9e09069704f53b38d31b5f5dabced6b40b4f54 Mon Sep 17 00:00:00 2001 From: Chris Barber Date: Wed, 9 Sep 2026 09:37:19 -0500 Subject: [PATCH 4/5] feat(databases): keep Import Data and Export CSV visible until the toolbar runs out of room Moving both into the "..." menu unconditionally cost more than it bought. Import Data especially is how a new user seeds their first data, and burying it adds onboarding friction for the sake of a tidier row; Export CSV follows it for symmetry. What actually needed fixing was their prominence, not their presence -- they led the toolbar in the same bright green as Add New Record(s), which read as three equally common actions. So both keep a toolbar button at `xl` and up, in `defaultOutline` (muted purple) rather than `positiveOutline` (green), leaving Add New Record(s) as the only green call to action. Below `xl` the buttons drop out and the menu entries take over, so the pair stays reachable at every width without ever appearing twice: the buttons are `hidden xl:inline-flex`, and the menu entries plus their separator are `xl:hidden`. The `accessKey="i"` / `"e"` shortcuts stay dropped. They only fire on rendered elements, so they would work at `xl` and up and silently do nothing below it -- a shortcut that depends on window width is worse than no shortcut. Co-Authored-By: Claude Opus 5 (1M context) --- .../components/DatabaseTableView.test.tsx | 24 ++++++++++++--- .../components/DatabaseTableView.tsx | 29 +++++++++++++++++-- 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/src/features/instance/databases/components/DatabaseTableView.test.tsx b/src/features/instance/databases/components/DatabaseTableView.test.tsx index 473ca788e..038730397 100644 --- a/src/features/instance/databases/components/DatabaseTableView.test.tsx +++ b/src/features/instance/databases/components/DatabaseTableView.test.tsx @@ -107,16 +107,32 @@ function openTableOptions() { fireEvent.pointerDown(screen.getByRole('button', { name: /table options/i }), { button: 0, ctrlKey: false }); } -const exportCsvItem = () => screen.queryByText('Export CSV'); -const importDataItem = () => screen.queryByText('Import Data'); -const dropTableItem = () => screen.queryByText('Drop Table'); -const dropDatabaseItem = () => screen.queryByText('Drop Database'); +const exportCsvItem = () => screen.queryByRole('menuitem', { name: 'Export CSV' }); +const importDataItem = () => screen.queryByRole('menuitem', { name: 'Import Data' }); +const dropTableItem = () => screen.queryByRole('menuitem', { name: 'Drop Table' }); +const dropDatabaseItem = () => screen.queryByRole('menuitem', { name: 'Drop Database' }); function isDisabled(el: HTMLElement) { return el.getAttribute('aria-disabled') === 'true' || el.hasAttribute('data-disabled'); } describe('DatabaseTableView table options menu', () => { + it('keeps Import Data and Export CSV discoverable until the toolbar is space-constrained', () => { + renderView(); + + const importButton = screen.getByRole('button', { name: 'Import Data' }); + const exportButton = screen.getByRole('button', { name: 'Export CSV' }); + expect(importButton.className).toContain('border-primary'); + expect(importButton.className).toContain('hidden xl:inline-flex'); + expect(exportButton.className).toContain('border-primary'); + expect(exportButton.className).toContain('hidden xl:inline-flex'); + + openTableOptions(); + + expect(importDataItem()!.className).toContain('xl:hidden'); + expect(exportCsvItem()!.className).toContain('xl:hidden'); + }); + // `describe_all` (the map) can be slower or unreachable for a role whose allowlist grants // describe_table + search but not describe_all -- the trigger must not gate on it, or Export // CSV becomes unreachable for that role. diff --git a/src/features/instance/databases/components/DatabaseTableView.tsx b/src/features/instance/databases/components/DatabaseTableView.tsx index aa8c07ca6..7abd71623 100644 --- a/src/features/instance/databases/components/DatabaseTableView.tsx +++ b/src/features/instance/databases/components/DatabaseTableView.tsx @@ -545,6 +545,25 @@ export function DatabaseTableView({ instanceDatabaseMap, databaseName, tableName )} + {canImportData && ( + + )} +
@@ -602,16 +621,20 @@ export function DatabaseTableView({ instanceDatabaseMap, databaseName, tableName {canImportData && ( - + Import Data )} - + Export CSV - + {onlyIfCached ? : } Only If Cached From 69f94a9900f29c38fe0798812128a5f063f68478 Mon Sep 17 00:00:00 2001 From: Chris Barber Date: Wed, 9 Sep 2026 11:12:08 -0500 Subject: [PATCH 5/5] test(databases): assert the responsive toolbar classes by name, not by substring The new breakpoint test matched `className` against the literal `'hidden xl:inline-flex'`, which only holds while `cn()` happens to emit those two classes adjacently in that order. A tailwind-merge bump or a reordered variant would break it with a failure that points at the string rather than at the behavior. `classList.contains` asks the question directly, per class. Also says why these assertions are the ones available: jsdom runs no Tailwind, so visibility itself can't be observed -- only the complementary pair that produces it, which is what actually keeps each action reachable at every width and duplicated at none. Co-Authored-By: Claude Opus 5 (1M context) --- .../components/DatabaseTableView.test.tsx | 26 ++++++++++++------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/src/features/instance/databases/components/DatabaseTableView.test.tsx b/src/features/instance/databases/components/DatabaseTableView.test.tsx index 038730397..7b3f32c64 100644 --- a/src/features/instance/databases/components/DatabaseTableView.test.tsx +++ b/src/features/instance/databases/components/DatabaseTableView.test.tsx @@ -117,20 +117,28 @@ function isDisabled(el: HTMLElement) { } describe('DatabaseTableView table options menu', () => { - it('keeps Import Data and Export CSV discoverable until the toolbar is space-constrained', () => { + // No Tailwind runs under jsdom, so actual visibility can't be asserted -- what can be pinned is + // the complementary pair that produces it, since that's what keeps each action reachable at every + // width and duplicated at none. `border-primary` pins the muted purple variant: under + // tailwind-merge it can only be there if the button isn't the green `positiveOutline` any more. + it('shows Import Data and Export CSV as buttons from xl up, and as menu entries below it', () => { renderView(); - const importButton = screen.getByRole('button', { name: 'Import Data' }); - const exportButton = screen.getByRole('button', { name: 'Export CSV' }); - expect(importButton.className).toContain('border-primary'); - expect(importButton.className).toContain('hidden xl:inline-flex'); - expect(exportButton.className).toContain('border-primary'); - expect(exportButton.className).toContain('hidden xl:inline-flex'); + for ( + const button of [ + screen.getByRole('button', { name: 'Import Data' }), + screen.getByRole('button', { name: 'Export CSV' }), + ] + ) { + expect(button.classList.contains('hidden')).toBe(true); + expect(button.classList.contains('xl:inline-flex')).toBe(true); + expect(button.classList.contains('border-primary')).toBe(true); + } openTableOptions(); - expect(importDataItem()!.className).toContain('xl:hidden'); - expect(exportCsvItem()!.className).toContain('xl:hidden'); + expect(importDataItem()!.classList.contains('xl:hidden')).toBe(true); + expect(exportCsvItem()!.classList.contains('xl:hidden')).toBe(true); }); // `describe_all` (the map) can be slower or unreachable for a role whose allowlist grants