From 5e9aae778489d8cc749159101d41d492dba3c426 Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Wed, 29 Jul 2026 04:03:45 +0200 Subject: [PATCH] test(import): cover drag and drop, button wiring, and fixture structures Three gaps where the implementation could break with a green suite. The drag listeners and their enter/leave depth counter had no test at all, so replacing the counter with a boolean passed everything. The Confirm and Cancel buttons were only asserted on for labels and disabled state, so the panel could have called mutate with a stale id or nothing at all. And the fixtures' own specs only checked archive layout, so deleting the U+E202 citation marker - which renders as nothing in an editor and in a diff - would have left every citation assertion downstream passing vacuously. --- .../Data/Import/__tests__/Dropzone.spec.tsx | 107 ++++++++++++++++ .../Data/Import/__tests__/Import.spec.tsx | 117 +++++++++++++++++- .../api/src/import/__data__/fixture.spec.ts | 66 ++++++++++ 3 files changed, 286 insertions(+), 4 deletions(-) create mode 100644 client/src/components/Nav/SettingsTabs/Data/Import/__tests__/Dropzone.spec.tsx diff --git a/client/src/components/Nav/SettingsTabs/Data/Import/__tests__/Dropzone.spec.tsx b/client/src/components/Nav/SettingsTabs/Data/Import/__tests__/Dropzone.spec.tsx new file mode 100644 index 0000000000..06784d28b1 --- /dev/null +++ b/client/src/components/Nav/SettingsTabs/Data/Import/__tests__/Dropzone.spec.tsx @@ -0,0 +1,107 @@ +import { render, screen, fireEvent } from 'test/layout-test-utils'; +import Dropzone from '../Dropzone'; + +function transfer(files: File[] = [], types: string[] = ['Files']) { + return { types, files }; +} + +const zip = () => new File(['zip-bytes'], 'export.zip', { type: 'application/zip' }); + +/** The tint that appears over the settings row while a file is dragged across + * it. Selected on `pointer-events-none` rather than `aria-hidden` alone: the + * button's decorative icon is also aria-hidden, and an SVG's `className` is an + * `SVGAnimatedString` rather than a string. */ +function overlayOf(container: HTMLElement): HTMLElement { + const overlay = container.querySelector('div[aria-hidden="true"].pointer-events-none'); + if (!overlay) { + throw new Error('drag overlay not found'); + } + return overlay as HTMLElement; +} + +describe('Dropzone drag and drop', () => { + it('imports a dropped export', () => { + const onFile = jest.fn(); + const file = zip(); + const { container } = render(); + const zone = container.firstChild as HTMLElement; + + fireEvent.drop(zone, { dataTransfer: transfer([file]) }); + + expect(onFile).toHaveBeenCalledWith(file); + }); + + /** + * The counter is load-bearing: dragenter and dragleave both fire for every + * child element crossed, so a plain boolean flickers the overlay off as soon + * as the pointer moves between the label and the button. Replacing the + * counter with a boolean passes every other test in this file. + */ + it('keeps the overlay up while the pointer crosses a child element', () => { + const { container } = render(); + const zone = container.firstChild as HTMLElement; + const child = screen.getByRole('button'); + const overlay = () => overlayOf(container); + + fireEvent.dragEnter(zone, { dataTransfer: transfer() }); + expect(overlay().className).toContain('opacity-100'); + + fireEvent.dragEnter(child, { dataTransfer: transfer() }); + fireEvent.dragLeave(child, { dataTransfer: transfer() }); + expect(overlay().className).toContain('opacity-100'); + + fireEvent.dragLeave(zone, { dataTransfer: transfer() }); + expect(overlay().className).toContain('opacity-0'); + }); + + it('ignores a drag that carries no files', () => { + const onFile = jest.fn(); + const { container } = render(); + const zone = container.firstChild as HTMLElement; + const overlay = () => overlayOf(container); + + fireEvent.dragEnter(zone, { dataTransfer: transfer([], ['text/plain']) }); + expect(overlay().className).toContain('opacity-0'); + + fireEvent.drop(zone, { dataTransfer: transfer([], ['text/plain']) }); + expect(onFile).not.toHaveBeenCalled(); + }); + + /** A drop accepted mid-upload would start a second, concurrent import of a + * different archive over the top of the first. */ + it('accepts no drop while an upload is already in flight', () => { + const onFile = jest.fn(); + const { container } = render(); + const zone = container.firstChild as HTMLElement; + + fireEvent.drop(zone, { dataTransfer: transfer([zip()]) }); + + expect(onFile).not.toHaveBeenCalled(); + }); + + it('imports a file chosen through the button', () => { + const onFile = jest.fn(); + const file = zip(); + const { container } = render(); + const input = container.querySelector('input[type="file"]') as HTMLInputElement; + + fireEvent.change(input, { target: { files: [file] } }); + + expect(onFile).toHaveBeenCalledWith(file); + expect(input.value).toBe(''); + }); + + it('recovers after a drag that ends outside the row', () => { + const onFile = jest.fn(); + const { container } = render(); + const zone = container.firstChild as HTMLElement; + const overlay = () => overlayOf(container); + + fireEvent.dragEnter(zone, { dataTransfer: transfer() }); + fireEvent.dragLeave(zone, { dataTransfer: transfer() }); + expect(overlay().className).toContain('opacity-0'); + + fireEvent.dragEnter(zone, { dataTransfer: transfer() }); + expect(overlay().className).toContain('opacity-100'); + }); +}); diff --git a/client/src/components/Nav/SettingsTabs/Data/Import/__tests__/Import.spec.tsx b/client/src/components/Nav/SettingsTabs/Data/Import/__tests__/Import.spec.tsx index 732c20ae6b..42841d7b77 100644 --- a/client/src/components/Nav/SettingsTabs/Data/Import/__tests__/Import.spec.tsx +++ b/client/src/components/Nav/SettingsTabs/Data/Import/__tests__/Import.spec.tsx @@ -62,6 +62,8 @@ describe('Import panel', () => { let capturedStartOptions: { onError?: (error: unknown) => void } = {}; let showToast: jest.Mock; let uploadMutate: jest.Mock; + let startMutate: jest.Mock; + let cancelMutate: jest.Mock; beforeEach(() => { capturedUploadOptions = {}; @@ -77,13 +79,18 @@ describe('Import panel', () => { return { mutate: uploadMutate, isLoading: false }; }, ); + startMutate = jest.fn(); + cancelMutate = jest.fn(); dataProvider.useStartImportMutation.mockImplementation( (options: typeof capturedStartOptions) => { capturedStartOptions = options ?? {}; - return { mutate: jest.fn(), isLoading: false }; + return { mutate: startMutate, isLoading: false }; }, ); - dataProvider.useCancelImportMutation.mockReturnValue({ mutate: jest.fn(), isLoading: false }); + dataProvider.useCancelImportMutation.mockReturnValue({ + mutate: cancelMutate, + isLoading: false, + }); dataProvider.useImportJobQuery.mockReturnValue({ data: undefined }); window.localStorage.clear(); }); @@ -374,12 +381,54 @@ describe('Import panel', () => { render(); /* Exactly once: the count is the
summary. It used to also be - repeated in the status block above, which this assertion enshrined. */ - expect(screen.getAllByText(/1 items could not be imported/i)).toHaveLength(1); + repeated in the status block above, which this assertion enshrined. + Singular, because a single failure is one item, not "1 items". */ + expect(screen.getAllByText(/^1 item could not be imported$/i)).toHaveLength(1); expect(screen.getByText(/conversation 11 malformed/i)).toBeInTheDocument(); expect(screen.getByText(/archive truncated/i)).toBeInTheDocument(); }); + it('pluralizes the error count for more than one failure', () => { + dataProvider.useImportJobQuery.mockReturnValue({ + data: job({ + phase: 'failed', + status: 'failed', + report: { + imported: 10, + skipped: 0, + assetsImported: 0, + assetsUnavailable: 0, + errors: ['conversation 11 malformed', 'conversation 12 malformed'], + }, + }), + }); + + render(); + + expect(screen.getByText(/^2 items could not be imported$/i)).toBeInTheDocument(); + }); + + it('pluralizes the report counts, so a single conversation is not "1 conversations"', () => { + dataProvider.useImportJobQuery.mockReturnValue({ + data: job({ + phase: 'completed', + status: 'completed', + report: { + imported: 1, + skipped: 0, + assetsImported: 1, + assetsUnavailable: 0, + errors: [], + }, + }), + }); + + render(); + + expect(screen.getByText('1 conversation imported, 0 skipped')).toBeInTheDocument(); + expect(screen.getByText('1 attachment imported, 0 unavailable')).toBeInTheDocument(); + }); + it('moves focus back to the import control after starting another import', () => { dataProvider.useImportJobQuery.mockReturnValue({ data: job({ @@ -549,6 +598,66 @@ describe('Import panel', () => { expect(window.localStorage.getItem('importJobId')).toBeNull(); }); + /** + * The buttons were only ever asserted on for their labels and disabled + * state, so the panel could have called mutate with undefined, an empty + * string, or a stale id and every test still passed. On a multi-minute + * import that is a Cancel button that silently does nothing. + */ + it('starts the job the panel is currently tracking', () => { + window.localStorage.setItem('importJobId', 'job-99'); + dataProvider.useImportJobQuery.mockReturnValue({ data: job() }); + + render(); + fireEvent.click(screen.getByRole('button', { name: /^import$/i })); + + expect(startMutate).toHaveBeenCalledWith('job-99'); + }); + + it('cancels the job it is tracking, from the confirmation screen', () => { + window.localStorage.setItem('importJobId', 'job-99'); + dataProvider.useImportJobQuery.mockReturnValue({ data: job() }); + + render(); + fireEvent.click(screen.getByRole('button', { name: /cancel import/i })); + + expect(cancelMutate).toHaveBeenCalledWith('job-99'); + }); + + it('cancels the job it is tracking, mid-run', () => { + window.localStorage.setItem('importJobId', 'job-99'); + dataProvider.useImportJobQuery.mockReturnValue({ + data: job({ + phase: 'conversations', + status: 'active', + progress: { + conversations: { done: 10, total: 100 }, + messages: { done: 0, total: 0 }, + assets: { done: 0, total: 0 }, + }, + }), + }); + + render(); + fireEvent.click(screen.getByRole('button', { name: /cancel import/i })); + + expect(cancelMutate).toHaveBeenCalledWith('job-99'); + }); + + it('starts the job returned by the upload, not a stale one', () => { + render(); + + /* Queued before the state change so the re-render the upload triggers + already shows the confirmation screen. */ + dataProvider.useImportJobQuery.mockReturnValue({ data: job() }); + act(() => { + capturedUploadOptions.onSuccess?.({ jobId: 'job-42', summary: summary() }); + }); + fireEvent.click(screen.getByRole('button', { name: /^import$/i })); + + expect(startMutate).toHaveBeenCalledWith('job-42'); + }); + it('shows a distinct toast for an unsupported file type', () => { render(); diff --git a/packages/api/src/import/__data__/fixture.spec.ts b/packages/api/src/import/__data__/fixture.spec.ts index 5e82adfa83..cdf776b808 100644 --- a/packages/api/src/import/__data__/fixture.spec.ts +++ b/packages/api/src/import/__data__/fixture.spec.ts @@ -37,3 +37,69 @@ describe('buildFixtureExport', () => { archive.close(); }); }); + +/** + * The fixture's structures, not its layout. + * + * `CITE` is U+E202, which renders as nothing in an editor and in a diff, so + * "tidying" it away is a plausible accident rather than a hypothetical one — + * and every citation assertion downstream would still pass, because they match + * on the text before the marker. These guards fail loudly instead. + */ +describe('fixture structures every downstream test depends on', () => { + async function readShard(name: string): Promise>> { + const filepath = await buildFixtureExport(); + const archive = await openArchive(filepath); + try { + return JSON.parse((await archive.read(name)).toString('utf8')); + } finally { + archive.close(); + } + } + + it('keeps a real citation anchor with its content reference', async () => { + const shard = await readShard('conversations-000.json'); + const mapping = (shard[0] as unknown as { mapping: Record }) + .mapping; + const cited = (mapping.a1 as { message: { content: { parts: string[] }; metadata: unknown } }) + .message; + + expect(cited.content.parts[0]).toBe('Stay in Positano.turn0search0'); + expect(cited.content.parts[0]).toContain(''); + expect((cited.metadata as { content_references: unknown[] }).content_references).toHaveLength( + 1, + ); + }); + + it('keeps the thoughts to reasoning_recap chain the thinking part is built from', async () => { + const shard = await readShard('conversations-000.json'); + const mapping = ( + shard[0] as unknown as { + mapping: Record; + } + ).mapping; + + expect(mapping.t1.message.content.content_type).toBe('thoughts'); + expect(mapping.r1.message.content.content_type).toBe('reasoning_recap'); + }); + + it('keeps the multimodal parts the asset pipeline resolves', async () => { + const shard = await readShard('conversations-001.json'); + const mapping = ( + shard[0] as unknown as { + mapping: Record< + string, + { message: { content: { parts: Array<{ content_type?: string }> } } } + >; + } + ).mapping; + + const types = Object.values(mapping) + .flatMap((node) => node.message?.content?.parts ?? []) + .map((part) => (typeof part === 'string' ? 'text' : part.content_type)); + + expect(types).toContain('image_asset_pointer'); + expect(types).toContain('audio_transcription'); + expect(types).toContain('audio_asset_pointer'); + }); +});