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..7b3f32c64 --- /dev/null +++ b/src/features/instance/databases/components/DatabaseTableView.test.tsx @@ -0,0 +1,215 @@ +/** + * @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'; + +// 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-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 { + useParams: () => params, + useSearch: () => search, + Link: ({ children }: { children?: React.ReactNode }) => <>{children}, + }; +}); + +vi.mock('@/config/useInstanceClient', () => ({ + useInstanceClientIdParams: () => ({ entityId: 'instance-1', instanceClient: {}, entityType: 'instance' }), +})); + +vi.mock('@/hooks/useAuth', () => ({ + useStaffPermission: () => false, +})); + +vi.mock('@/hooks/usePermissions', () => ({ + useInstanceBrowseManagePermission: () => permissionState.canManageBrowseInstance, + useInstanceImportDataPermission: () => true, + useInstanceSchemaTablePermission: () => true, + useInstanceTablePutPermission: () => true, +})); + +// 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('./TableView', () => ({ + TableView: ({ columns }: { columns: { accessorKey?: string }[] }) => { + tableViewColumns.current = columns; + return null; + }, +})); +vi.mock('./PickColumnsDropdown', () => ({ PickColumnsDropdown: () => null })); +vi.mock('../modals/EditTableRowModal', () => ({ EditTableRowModal: () => null })); + +// 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('@tanstack/react-query', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + 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() }, + }; +}); + +// Radix's dropdown opens on pointerdown and probes pointer-capture APIs jsdom doesn't implement. +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(); + permissionState.canManageBrowseInstance = true; + tableViewColumns.current = []; +}); + +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.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', () => { + // 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(); + + 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()!.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 + // 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); + + 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 a11f46441..7abd71623 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 { @@ -149,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({ @@ -536,26 +547,22 @@ export function DatabaseTableView({ instanceDatabaseMap, databaseName, tableName )} {canImportData && ( )} @@ -607,12 +614,27 @@ export function DatabaseTableView({ instanceDatabaseMap, databaseName, tableName /> - - + {canImportData && ( + + + Import Data + + )} + + + Export CSV + + {onlyIfCached ? : } Only If Cached @@ -637,7 +659,8 @@ export function DatabaseTableView({ instanceDatabaseMap, databaseName, tableName Cleanup Orphan Blobs )} - {canManageBrowseInstance && !isLastTableInDatabase && ( + {canManageBrowseInstance && } + {canDropTable && ( setWatchedValue('ShowDeleteTable', { databaseName, tableName })}