refactor(databases): move Import Data and Export CSV into the table's "..." menu

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<typeof import('@tanstack/react-query')>();
+	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(
+		<QueryClientProvider client={queryClient}>
+			<DatabaseTableView databaseName="data" tableName="dog" instanceDatabaseMap={instanceDatabaseMap} />
+		</QueryClientProvider>,
+	);
+}
+
+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();
+	});
+});

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<Record<string, unknown> | 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 && (
 						<Button
-							variant="positiveOutline"
+							variant="defaultOutline"
+							className="hidden xl:inline-flex"
 							onClick={onImportDataClicked}
-							accessKey="i"
 						>
 							<CloudUploadIcon />
-							<span>
-								<u>I</u>mport Data
-							</span>
+							<span>Import Data</span>
 						</Button>
 					)}
 					<Button
-						variant="positiveOutline"
+						variant="defaultOutline"
+						className="hidden xl:inline-flex"
 						onClick={onExportCSVClicked}
 						disabled={isExportingCSV}
-						accessKey="e"
 					>
 						<CloudDownloadIcon />
-						<span>
-							<u>E</u>xport CSV
-						</span>
+						<span>Export CSV</span>
 					</Button>
 				</div>
 
@@ -607,12 +614,27 @@ export function DatabaseTableView({ instanceDatabaseMap, databaseName, tableName
 					/>
 
 					<DropdownMenu>
-						<DropdownMenuTrigger asChild disabled={!instanceDatabaseMap}>
-							<Button variant="ghost" size="icon" disabled={!instanceDatabaseMap}>
+						<DropdownMenuTrigger asChild>
+							<Button variant="ghost" size="icon">
 								<EllipsisIcon aria-label="Table options" />
 							</Button>
 						</DropdownMenuTrigger>
 						<DropdownMenuContent side="bottom" align="end">
+							{canImportData && (
+								<DropdownMenuItem className="xl:hidden" onClick={onImportDataClicked}>
+									<CloudUploadIcon />
+									Import Data
+								</DropdownMenuItem>
+							)}
+							<DropdownMenuItem
+								className="xl:hidden"
+								onClick={onExportCSVClicked}
+								disabled={isExportingCSV}
+							>
+								<CloudDownloadIcon />
+								Export CSV
+							</DropdownMenuItem>
+							<DropdownMenuSeparator className="xl:hidden" />
 							<DropdownMenuItem className="focus:bg-primary/70 focus:text-white" onClick={toggleOnlyCached}>
 								{onlyIfCached ? <CircleCheckBigIcon className="text-green" /> : <CircleIcon />}
 								Only If Cached
@@ -637,7 +659,8 @@ export function DatabaseTableView({ instanceDatabaseMap, databaseName, tableName
 									Cleanup Orphan Blobs
 								</DropdownMenuItem>
 							)}
-							{canManageBrowseInstance && !isLastTableInDatabase && (
+							{canManageBrowseInstance && <DropdownMenuSeparator />}
+							{canDropTable && (
 								<DropdownMenuItem
 									className="focus:bg-red/70 focus:text-white"
 									onClick={() => setWatchedValue('ShowDeleteTable', { databaseName, tableName })}