From c4f4fdd794d3c29d3f4255104e7d5e8091894504 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sun, 26 Jul 2026 11:37:47 +0200 Subject: [PATCH] fix(booking-import): cover the AI retry and keep its source files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The widget's new healthApi.features() call left BackgroundTasksWidget.test.tsx red — the api/client mock never exported healthApi, so both existing tests threw on mount. The mock now carries it, pending by default like the poll backstop, and the retry itself finally has tests. Writing them turned up two gaps. The retry never called saveImportFiles, so the files only lived in memory: reloading during the retry parse left the review with no source document to attach to the bookings it creates, unlike every import started from the modal. And the button stayed live while the request was in flight, so a second click started a second force-ai job on the same files. --- .../BackgroundTasksWidget.test.tsx | 95 ++++++++++++++++++- .../BackgroundTasks/BackgroundTasksWidget.tsx | 31 ++++-- 2 files changed, 114 insertions(+), 12 deletions(-) diff --git a/client/src/components/BackgroundTasks/BackgroundTasksWidget.test.tsx b/client/src/components/BackgroundTasks/BackgroundTasksWidget.test.tsx index 3883a5f5e..6bfbab1ca 100644 --- a/client/src/components/BackgroundTasks/BackgroundTasksWidget.test.tsx +++ b/client/src/components/BackgroundTasks/BackgroundTasksWidget.test.tsx @@ -1,14 +1,19 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' -import { screen } from '@testing-library/react' +import { screen, waitFor } from '@testing-library/react' +import userEvent from '@testing-library/user-event' import { render } from '../../../tests/helpers/render' +import { reservationsApi, healthApi } from '../../api/client' +import { saveImportFiles } from '../../db/offlineDb' import { useBackgroundTasksStore, type BackgroundImportTask } from '../../store/backgroundTasksStore' import BackgroundTasksWidget from './BackgroundTasksWidget' vi.mock('../../api/websocket', () => ({ addListener: vi.fn(), removeListener: vi.fn() })) vi.mock('../../api/client', () => ({ // Keep the rehydrate/poll backstops pending so the seeded state is what renders. - reservationsApi: { importJobStatus: vi.fn(() => new Promise(() => {})) }, + reservationsApi: { importJobStatus: vi.fn(() => new Promise(() => {})), importBookingAsync: vi.fn() }, + healthApi: { features: vi.fn() }, })) +vi.mock('../../db/offlineDb', () => ({ saveImportFiles: vi.fn(() => Promise.resolve()) })) const task = (overrides: Partial = {}): BackgroundImportTask => ({ id: 'j1', @@ -22,8 +27,14 @@ const task = (overrides: Partial = {}): BackgroundImportTa ...overrides, }) +const pdf = () => new File(['%PDF'], 'voucher.pdf', { type: 'application/pdf' }) + beforeEach(() => { vi.clearAllMocks() + // Like the poll backstop above: leave the feature probe pending so tests that don't care + // about the AI retry render the same widget they did before the button existed. + vi.mocked(healthApi.features).mockReturnValue(new Promise(() => {})) + vi.mocked(saveImportFiles).mockResolvedValue(undefined) useBackgroundTasksStore.setState({ tasks: [] }) }) @@ -42,4 +53,84 @@ describe('BackgroundTasksWidget', () => { expect(screen.getByText('No reservations could be extracted from the uploaded files.')).toBeInTheDocument() expect(screen.queryByText(/AI parsing failed/)).not.toBeInTheDocument() }) + + describe('AI retry', () => { + it('offers the retry on an empty result once the addon reports AI parsing', async () => { + vi.mocked(healthApi.features).mockResolvedValue({ bookingImport: true, aiParsing: true }) + useBackgroundTasksStore.setState({ tasks: [task({ sourceFiles: [pdf()] })] }) + render() + expect(await screen.findByRole('button', { name: 'Try AI parsing' })).toBeInTheDocument() + }) + + it('stays hidden when the addon is off, the files are gone, or the run was already force-ai', async () => { + vi.mocked(healthApi.features).mockResolvedValue({ bookingImport: true, aiParsing: false }) + useBackgroundTasksStore.setState({ tasks: [task({ sourceFiles: [pdf()] })] }) + const { unmount } = render() + await waitFor(() => expect(healthApi.features).toHaveBeenCalled()) + expect(screen.queryByRole('button', { name: 'Try AI parsing' })).not.toBeInTheDocument() + unmount() + + vi.mocked(healthApi.features).mockResolvedValue({ bookingImport: true, aiParsing: true }) + // Rehydrated from storage: sourceFiles can't survive a reload, so there is nothing to resend. + useBackgroundTasksStore.setState({ tasks: [task()] }) + const withoutFiles = render() + await waitFor(() => expect(healthApi.features).toHaveBeenCalledTimes(2)) + expect(screen.queryByRole('button', { name: 'Try AI parsing' })).not.toBeInTheDocument() + withoutFiles.unmount() + + useBackgroundTasksStore.setState({ tasks: [task({ sourceFiles: [pdf()], mode: 'force-ai' })] }) + render() + await waitFor(() => expect(healthApi.features).toHaveBeenCalledTimes(3)) + expect(screen.queryByRole('button', { name: 'Try AI parsing' })).not.toBeInTheDocument() + }) + + it('re-submits the files with force-ai, keeps them for the review and replaces the task', async () => { + const file = pdf() + vi.mocked(healthApi.features).mockResolvedValue({ bookingImport: true, aiParsing: true }) + vi.mocked(reservationsApi.importBookingAsync).mockResolvedValue({ jobId: 'j2' }) + useBackgroundTasksStore.setState({ tasks: [task({ sourceFiles: [file] })] }) + render() + + await userEvent.click(await screen.findByRole('button', { name: 'Try AI parsing' })) + + expect(reservationsApi.importBookingAsync).toHaveBeenCalledWith('t1', [file], 'force-ai') + // Without this the reviewed bookings lose their source document after a reload. + await waitFor(() => expect(saveImportFiles).toHaveBeenCalledWith('j2', [file])) + await waitFor(() => { + const tasks = useBackgroundTasksStore.getState().tasks + expect(tasks).toHaveLength(1) + expect(tasks[0]).toMatchObject({ id: 'j2', tripId: 't1', status: 'running', mode: 'force-ai' }) + }) + }) + + it('keeps the task and surfaces the server error when the retry is rejected', async () => { + vi.mocked(healthApi.features).mockResolvedValue({ bookingImport: true, aiParsing: true }) + vi.mocked(reservationsApi.importBookingAsync).mockRejectedValue({ response: { data: { error: 'No AI model configured' } } }) + useBackgroundTasksStore.setState({ tasks: [task({ sourceFiles: [pdf()] })] }) + render() + + await userEvent.click(await screen.findByRole('button', { name: 'Try AI parsing' })) + + expect(await screen.findByText('No AI model configured')).toBeInTheDocument() + const tasks = useBackgroundTasksStore.getState().tasks + expect(tasks).toHaveLength(1) + expect(tasks[0]).toMatchObject({ id: 'j1', status: 'error' }) + expect(saveImportFiles).not.toHaveBeenCalled() + }) + + it('ignores a second click while the first retry is still in flight', async () => { + vi.mocked(healthApi.features).mockResolvedValue({ bookingImport: true, aiParsing: true }) + // Never settles: the retry stays in flight for the whole test. + vi.mocked(reservationsApi.importBookingAsync).mockReturnValue(new Promise<{ jobId: string }>(() => {})) + useBackgroundTasksStore.setState({ tasks: [task({ sourceFiles: [pdf()] })] }) + render() + + const button = await screen.findByRole('button', { name: 'Try AI parsing' }) + await userEvent.click(button) + await waitFor(() => expect(button).toBeDisabled()) + await userEvent.click(button) + + expect(reservationsApi.importBookingAsync).toHaveBeenCalledTimes(1) + }) + }) }) diff --git a/client/src/components/BackgroundTasks/BackgroundTasksWidget.tsx b/client/src/components/BackgroundTasks/BackgroundTasksWidget.tsx index 5600fc7e7..980fd633c 100644 --- a/client/src/components/BackgroundTasks/BackgroundTasksWidget.tsx +++ b/client/src/components/BackgroundTasks/BackgroundTasksWidget.tsx @@ -5,6 +5,7 @@ import { Loader2, CheckCircle2, AlertCircle, X } from 'lucide-react' import { useTranslation } from '../../i18n' import { addListener, removeListener } from '../../api/websocket' import { reservationsApi, healthApi } from '../../api/client' +import { saveImportFiles } from '../../db/offlineDb' import { useBackgroundTasksStore, type BackgroundImportTask } from '../../store/backgroundTasksStore' /** @@ -31,16 +32,25 @@ export default function BackgroundTasksWidget() { }, []) // Re-runs the same files with force-ai: the LLM sees every file, kitinerary is skipped. - const retryWithAi = (task: BackgroundImportTask) => { - if (!task.sourceFiles || task.sourceFiles.length === 0) return - reservationsApi - .importBookingAsync(task.tripId, task.sourceFiles, 'force-ai') - .then(({ jobId }) => { - dismiss(task.id) - addTask({ id: jobId, tripId: task.tripId, label: task.label, total: task.sourceFiles!.length, files: task.sourceFiles, mode: 'force-ai' }) - }) + const [retrying, setRetrying] = useState(null) + const retryWithAi = async (task: BackgroundImportTask) => { + const files = task.sourceFiles + if (!files || files.length === 0 || retrying === task.id) return + setRetrying(task.id) + try { + const { jobId } = await reservationsApi.importBookingAsync(task.tripId, files, 'force-ai') + // Same as the modal's first submit: the review attaches each source document to the + // booking it created, and only IndexedDB survives a reload mid-parse. + await saveImportFiles(jobId, files) + dismiss(task.id) + addTask({ id: jobId, tripId: task.tripId, label: task.label, total: files.length, files, mode: 'force-ai' }) + } catch (err) { // 409 when the addon is enabled but this user has no model configured. - .catch((err) => setError(task.id, task.tripId, err?.response?.data?.error ?? t('reservations.import.error'))) + const message = (err as { response?: { data?: { error?: string } } })?.response?.data?.error + setError(task.id, task.tripId, message ?? t('reservations.import.error')) + } finally { + setRetrying(null) + } } // On (re)load, reconcile tasks restored from localStorage with the server: a parse @@ -167,8 +177,9 @@ export default function BackgroundTasksWidget() { {aiParsing && task.mode !== 'force-ai' && task.sourceFiles && task.sourceFiles.length > 0 && (