fix(browse): refresh the table once an import's job completes

src/features/instance/databases/DESIGN.md

@@ -90,3 +90,27 @@ so it is judged by that table's attribute. Typed filters and deep-linked ones bo
 [`DatabaseTableView`](components/DatabaseTableView.tsx), which asks first (#1285). The request then
 carries `allowFullScan: true`, and so does a CSV export of that filter. A new way to apply a filter
 has to go through that function.
+
+## An import refreshes Browse from the mutation, not from the modal
+
+Every Browse read has a 60s `staleTime`, so an import into the open table shows nothing new until
+something invalidates `[entityId, database, table]`. Navigating to a table that is already on screen
+doesn't invalidate it. That refresh lives in `useImportDataMutation`'s own `onSettled`
+([`importData.ts`](../../../integrations/api/instance/database/importData.ts)), because React Query
+skips a `mutate` call's callbacks once the component that made it has unmounted. It used to live in
+the table view, and #1471 lost it when the modal moved into `DatabaseActionModals`.
+
+It is not awaited. The import settles at the job's verdict, and the modal closes then. The modal's
+`onImported` waits for the `describe_all` refetch before it navigates
+(`awaitDatabaseMapRefresh`, which joins the refetch rather than starting another), or
+`resolveDatabasesRedirect` would send a table the import just created back to the overview. If that
+refetch fails, the old map stays and the redirect still happens, as it did before.
+
+A failed import refreshes too, because any outcome can have written. `csv_url_load` inserts chunk by
+chunk (harper `dataLayer/bulkLoad.ts`, `insertChunk`), so a load that fails partway keeps its earlier
+chunks, and a refused load can follow the `create_table` that made its table. A submission with no
+verdict is the strongest case: its toast says "Outcome unknown" and asks the user to check whether the
+import took effect (`isUnansweredWrite`). A central-manager 504 on a long insert that Harper went on to apply is one
+example, and only a re-read shows the answer. When the instance is really down, the re-reads fail
+too and add their own error toasts. That noise is cheaper than a stale grid inviting a retry that
+duplicates every record without an id.

src/features/instance/databases/components/DatabaseActionModals.test.tsx

@@ -3,14 +3,14 @@
  */
 import { InstanceDatabaseMap, InstanceTable } from '@/integrations/api/api.patch';
 import { setWatchedValue, useWatchedValue } from '@/lib/events/watcher';
-import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
+import { QueryClient, QueryClientProvider, useQuery } from '@tanstack/react-query';
 import { act, cleanup, render, screen } from '@testing-library/react';
 import { afterEach, describe, expect, it, vi } from 'vitest';
 import { DatabaseActionModals } from './DatabaseActionModals';
 
+const navigate = vi.hoisted(() => vi.fn());
 vi.mock('@tanstack/react-router', () => {
 	const params = {};
-	const navigate = () => undefined;
 	const router = { invalidate: async () => undefined };
 	return { useParams: () => params, useNavigate: () => navigate, useRouter: () => router };
 });
@@ -43,7 +43,15 @@ vi.mock('@/features/instance/databases/modals/AddTableRowModal', () => ({
 	),
 }));
 vi.mock('@/features/instance/databases/modals/CreateNewTableModal', () => ({ CreateNewTableModal: () => null }));
-vi.mock('@/features/instance/databases/modals/ImportDataModal', () => ({ ImportDataModal: () => null }));
+const importModal = vi.hoisted(() => ({
+	onImported: undefined as undefined | ((databaseName: string, tableName: string) => Promise<void>),
+}));
+vi.mock('@/features/instance/databases/modals/ImportDataModal', () => ({
+	ImportDataModal: ({ onImported }: { onImported: typeof importModal.onImported }) => {
+		importModal.onImported = onImported;
+		return null;
+	},
+}));
 vi.mock('@/features/instance/databases/modals/DeleteTableModal', () => ({ DeleteTableModal: () => null }));
 vi.mock('@/features/instance/databases/modals/DeleteDatabaseModal', () => ({ DeleteDatabaseModal: () => null }));
 
@@ -162,3 +170,37 @@ describe('DatabaseActionModals opening Add Records', () => {
 		expect(screen.getByRole('status', { name: 'add target' }).textContent).toContain('"Dog"');
 	});
 });
+
+describe('DatabaseActionModals after an import', () => {
+	afterEach(() => navigate.mockClear());
+
+	it('goes to the imported table only once describe_all is back, so a table the import created is in the map', async () => {
+		const refetched = deferred<object>();
+		const tree = vi.fn<() => Promise<object>>().mockResolvedValueOnce(instanceDatabaseMap).mockReturnValueOnce(
+			refetched.promise,
+		);
+		const treeKey = ['instance-1', 'describe_all', true, true];
+		function DatabasesPage() {
+			useQuery({ queryKey: treeKey, queryFn: tree, staleTime: 60_000 });
+			return null;
+		}
+		const client = new QueryClient();
+		render(
+			<QueryClientProvider client={client}>
+				<DatabasesPage />
+				<DatabaseActionModals instanceDatabaseMap={instanceDatabaseMap} />
+			</QueryClientProvider>,
+		);
+		await vi.waitFor(() => expect(client.getQueryData(treeKey)).toBe(instanceDatabaseMap));
+
+		const imported = importModal.onImported!('data', 'Cat');
+		await vi.waitFor(() => expect(tree).toHaveBeenCalledTimes(2));
+		expect(navigate).not.toHaveBeenCalled();
+
+		await act(async () => {
+			refetched.resolve({ data: { ...instanceDatabaseMap.data, Cat: table('Cat', ['id']) } });
+			await imported;
+		});
+		expect(navigate).toHaveBeenCalledTimes(1);
+	});
+});

src/features/instance/databases/components/DatabaseActionModals.tsx

@@ -7,6 +7,7 @@ import { DeleteTableModal } from '@/features/instance/databases/modals/DeleteTab
 import { ImportDataModal } from '@/features/instance/databases/modals/ImportDataModal';
 import { InstanceDatabaseMap, InstanceTable } from '@/integrations/api/api.patch';
 import { getDescribeTable } from '@/integrations/api/instance/database/getDescribeTable';
+import { awaitDatabaseMapRefresh } from '@/integrations/api/instance/database/importData';
 import { setWatchedValue, useWatchedValue } from '@/lib/events/watcher';
 import { WatchedValuesTypeMap } from '@/lib/storage/watchedValueKeys';
 import { buildAbsoluteLinkToDatabasePage } from '@/lib/urls/buildAbsoluteLinkToDatabasePage';
@@ -91,11 +92,7 @@ export function DatabaseActionModals({ instanceDatabaseMap }: { instanceDatabase
 	}, [addTarget, addTableFromMap, instanceParams]);
 
 	const onImported = useCallback(async (databaseName: string, tableName: string) => {
-		// The import may have created a new table (or even database), so refresh the tree too.
-		await queryClient.invalidateQueries({
-			queryKey: [instanceParams.entityId, 'describe_all'],
-			refetchType: 'all',
-		});
+		await awaitDatabaseMapRefresh(queryClient, instanceParams.entityId);
 		goToTable(databaseName, tableName);
 		await router.invalidate();
 	}, [queryClient, instanceParams.entityId, goToTable, router]);

src/integrations/api/instance/database/importData.test.tsx

@@ -1,10 +1,22 @@
 // @vitest-environment jsdom
-import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
+import { QueryClient, QueryClientProvider, useQuery } from '@tanstack/react-query';
 import { act, renderHook } from '@testing-library/react';
-import type { AxiosInstance } from 'axios';
+import { AxiosError, type AxiosInstance, type AxiosResponse } from 'axios';
 import type { ReactNode } from 'react';
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
-import { importData, type ImportDataParams, type ImportProgressListener, useImportDataMutation } from './importData';
+import { getDescribeAllQueryOptions } from './getDescribeAll';
+import { getDescribeTableQueryOptions } from './getDescribeTable';
+import { getSearchByConditionsOptions } from './getSearchByConditions';
+import { getSearchByIdOptions } from './getSearchById';
+import { getSearchByValueOptions } from './getSearchByValue';
+import { getTableRecordCountQueryOptions } from './getTableRecordCount';
+import {
+	awaitDatabaseMapRefresh,
+	importData,
+	type ImportDataParams,
+	type ImportProgressListener,
+	useImportDataMutation,
+} from './importData';
 
 function progressListener() {
 	return {
@@ -14,21 +26,22 @@ function progressListener() {
 	};
 }
 
-function csvClient(statuses: string[]) {
+function csvClient(statuses: string[], completeMessage = 'loaded 5 of 5') {
 	let poll = 0;
 	const post = vi.fn(async (_url: string, body: { operation: string }) => {
 		if (body.operation === 'csv_url_load' || body.operation === 'csv_data_load') {
 			return { data: { message: 'Starting job', job_id: 'job-1' } };
 		}
 		const status = statuses[Math.min(poll, statuses.length - 1)];
 		poll += 1;
-		return { data: [{ id: 'job-1', status, message: status === 'COMPLETE' ? 'loaded 5 of 5' : undefined }] };
+		return { data: [{ id: 'job-1', status, message: status === 'COMPLETE' ? completeMessage : undefined }] };
 	});
 	return { post } as unknown as AxiosInstance;
 }
 
 function params(instanceClient: AxiosInstance, progress: ImportProgressListener): ImportDataParams {
 	return {
+		entityId: 'instance-1',
 		database: 'data',
 		table: 'dog',
 		tableExists: true,
@@ -112,3 +125,213 @@ describe('useImportDataMutation', () => {
 		expect(progress.onSuccess).not.toHaveBeenCalled();
 	});
 });
+
+describe('useImportDataMutation refreshing Browse', () => {
+	afterEach(() => vi.restoreAllMocks());
+
+	const browse = { entityId: 'instance-1', instanceClient: {} as AxiosInstance };
+	const page = { sort: { attribute: 'id', descending: false }, pageIndex: 0, pageSize: 20, onlyIfCached: true };
+	// The real builders, so the keys refreshed here are the keys Browse reads.
+	const keys = (tableName: string) => ({
+		page: getSearchByValueOptions({
+			...browse,
+			...page,
+			enabled: true,
+			databaseName: 'data',
+			tableName,
+			searchAttribute: 'id',
+		}).queryKey,
+		filtered: getSearchByConditionsOptions({
+			...browse,
+			...page,
+			enabled: true,
+			databaseName: 'data',
+			tableName,
+			conditions: [{ search_attribute: 'name', search_type: 'equals', search_value: 'Rex' }],
+		}).queryKey,
+		count: getTableRecordCountQueryOptions({ ...browse, enabled: true, databaseName: 'data', tableName }).queryKey,
+		describe: getDescribeTableQueryOptions({ ...browse, databaseName: 'data', tableName }).queryKey,
+		record: getSearchByIdOptions({ ...browse, enabled: true, databaseName: 'data', tableName, ids: ['rex'] })
+			.queryKey,
+	});
+	const treeKey = getDescribeAllQueryOptions({ ...browse, skipRecordCount: true, includeComputed: true }).queryKey;
+
+	function wrapperFor(client: QueryClient) {
+		return ({ children }: { children: ReactNode }) => (
+			<QueryClientProvider client={client}>{children}</QueryClientProvider>
+		);
+	}
+
+	async function openBrowse(client: QueryClient, tree = vi.fn(async (): Promise<object> => ({ dog: {} }))) {
+		const reads = {
+			page: vi.fn(async () => ({ data: [] })),
+			describe: vi.fn(async () => ({ record_count: 0 })),
+			otherPage: vi.fn(async () => ({ data: [] })),
+			tree,
+		};
+		// Browse's own options, so within the test only an invalidation refetches, and once.
+		const browseOptions = { staleTime: 60_000, retry: false } as const;
+		renderHook(() => {
+			useQuery({ queryKey: [...keys('dog').page], queryFn: reads.page, ...browseOptions });
+			useQuery({ queryKey: [...keys('dog').describe], queryFn: reads.describe, ...browseOptions });
+			useQuery({ queryKey: [...keys('cat').page], queryFn: reads.otherPage, ...browseOptions });
+			useQuery({ queryKey: [...treeKey], queryFn: reads.tree, ...browseOptions });
+		}, { wrapper: wrapperFor(client) });
+		// Cached but not on screen, so an invalidation only marks them stale.
+		client.setQueryData([...keys('dog').filtered], { data: [] });
+		client.setQueryData([...keys('dog').count], 5);
+		client.setQueryData([...keys('dog').record], { data: [] });
+		await vi.advanceTimersByTimeAsync(0);
+		for (const read of Object.values(reads)) {
+			expect(read).toHaveBeenCalledTimes(1);
+		}
+		return reads;
+	}
+
+	function startImport(client: QueryClient, instanceClient: AxiosInstance, overrides: Partial<ImportDataParams> = {}) {
+		const modal = renderHook(() => useImportDataMutation(), { wrapper: wrapperFor(client) });
+		act(() => modal.result.current.mutate({ ...params(instanceClient, progressListener()), ...overrides }));
+		return modal;
+	}
+
+	const rejection = (status?: number) =>
+		new AxiosError(
+			status ? `Request failed with status code ${status}` : 'Network Error',
+			status ? AxiosError.ERR_BAD_REQUEST : AxiosError.ERR_NETWORK,
+			undefined,
+			undefined,
+			status ? ({ status, data: 'refused' } as AxiosResponse) : undefined,
+		);
+
+	function createsTableThenRefuses() {
+		const post = vi.fn(async (_url: string, body: { operation: string }) => {
+			if (body.operation === 'create_table') {
+				return { data: { message: 'table created' } };
+			}
+			throw rejection(400);
+		});
+		return { post } as unknown as AxiosInstance;
+	}
+
+	function pollsStopAnswering() {
+		const post = vi.fn(async (_url: string, body: { operation: string }) => {
+			if (body.operation === 'csv_url_load') {
+				return { data: { message: 'Starting job', job_id: 'job-1' } };
+			}
+			throw rejection();
+		});
+		return { post } as unknown as AxiosInstance;
+	}
+
+	it('refetches the imported table and the tree once the job completes, rows skipped and the modal unmounted', async () => {
+		const client = new QueryClient();
+		const reads = await openBrowse(client);
+
+		startImport(client, csvClient(['IN_PROGRESS', 'COMPLETE'], 'successfully loaded 3 of 5 records')).unmount();
+		await vi.advanceTimersByTimeAsync(5_000);
+
+		expect(reads.page).toHaveBeenCalledTimes(2);
+		expect(reads.describe).toHaveBeenCalledTimes(2);
+		expect(reads.tree).toHaveBeenCalledTimes(2);
+		expect(client.getQueryState([...keys('dog').filtered])?.isInvalidated).toBe(true);
+		expect(client.getQueryState([...keys('dog').count])?.isInvalidated).toBe(true);
+		expect(client.getQueryState([...keys('dog').record])?.isInvalidated).toBe(true);
+		expect(reads.otherPage).toHaveBeenCalledTimes(1);
+	});
+
+	const rejectAll = (status?: number) => () =>
+		({
+			post: vi.fn(async () => {
+				throw rejection(status);
+			}),
+		}) as unknown as AxiosInstance;
+	const failures: [string, () => AxiosInstance, Partial<ImportDataParams>, number][] = [
+		['the job ended in ERROR', () => csvClient(['IN_PROGRESS', 'ERROR']), {}, 5_000],
+		['the wait timed out on a load still running', () => csvClient(['IN_PROGRESS']), {}, 11 * 60_000],
+		[
+			'Harper refused the records for a table the import had just created',
+			createsTableThenRefuses,
+			{ tableExists: false, source: { kind: 'json-records', records: [{ name: 'Rex' }] } },
+			5_000,
+		],
+		['the submission got no answer', rejectAll(), {}, 5_000],
+		['a gateway answered 504', rejectAll(504), {}, 5_000],
+		['the polls stopped answering', pollsStopAnswering, {}, 10_000],
+	];
+	it.each(failures)('refetches too when %s', async (_case, instanceClient, overrides, waitMs) => {
+		const client = new QueryClient();
+		const reads = await openBrowse(client);
+
+		const modal = startImport(client, instanceClient(), overrides);
+		await vi.advanceTimersByTimeAsync(waitMs);
+
+		expect(modal.result.current.isError).toBe(true);
+		expect(reads.page).toHaveBeenCalledTimes(2);
+		expect(reads.tree).toHaveBeenCalledTimes(2);
+	});
+
+	it('keeps re-reads that fail with the instance from escaping as unhandled rejections', async () => {
+		const client = new QueryClient();
+		const reads = await openBrowse(client);
+		reads.page.mockRejectedValue(rejection());
+		reads.tree.mockRejectedValue(rejection());
+
+		const modal = startImport(client, pollsStopAnswering());
+		await vi.advanceTimersByTimeAsync(10_000);
+
+		expect(modal.result.current.isError).toBe(true);
+		expect(client.getQueryState([...keys('dog').page])?.status).toBe('error');
+		expect(client.getQueryState([...treeKey])?.status).toBe('error');
+	});
+
+	it('settles at the job verdict, and a caller waiting for the map joins the refetch already in flight', async () => {
+		const client = new QueryClient();
+		let landRefetch!: (map: object) => void;
+		const tree = vi.fn<() => Promise<object>>()
+			.mockResolvedValueOnce({})
+			.mockImplementationOnce(() =>
+				new Promise((resolve) => {
+					landRefetch = resolve;
+				})
+			);
+		await openBrowse(client, tree);
+
+		let mapAtNavigation: unknown;
+		const modal = renderHook(() => useImportDataMutation(), { wrapper: wrapperFor(client) });
+		act(() =>
+			modal.result.current.mutate(params(csvClient(['COMPLETE']), progressListener()), {
+				onSuccess: async () => {
+					await awaitDatabaseMapRefresh(client, 'instance-1');
+					mapAtNavigation = client.getQueryData([...treeKey]);
+				},
+			})
+		);
+		await vi.advanceTimersByTimeAsync(0);
+
+		expect(modal.result.current.isSuccess).toBe(true);
+		expect(mapAtNavigation).toBeUndefined();
+
+		landRefetch({ dog: {} });
+		await vi.advanceTimersByTimeAsync(0);
+
+		expect(mapAtNavigation).toEqual({ dog: {} });
+		expect(tree).toHaveBeenCalledTimes(2);
+	});
+
+	it('keeps a completed import successful when the refresh throws', async () => {
+		const debug = vi.spyOn(console, 'debug').mockImplementation(() => {});
+		const client = new QueryClient();
+		vi.spyOn(client, 'invalidateQueries').mockImplementation(() => {
+			throw new Error('cache exploded');
+		});
+		const progress = progressListener();
+		const modal = renderHook(() => useImportDataMutation(), { wrapper: wrapperFor(client) });
+
+		act(() => modal.result.current.mutate(params(csvClient(['COMPLETE']), progress)));
+		await vi.advanceTimersByTimeAsync(5_000);
+
+		expect(modal.result.current.isSuccess).toBe(true);
+		expect(progress.onError).not.toHaveBeenCalled();
+		expect(debug).toHaveBeenCalledWith('Refreshing the imported table failed', expect.any(Error));
+	});
+});