From 9555925a1271b54523c13283999cd762d7a65d75 Mon Sep 17 00:00:00 2001 From: Naomi Gilbert Date: Fri, 31 Jul 2026 20:16:15 -0400 Subject: [PATCH] fix(reports): export what is on screen instead of the last saved report Both export handlers built a query AST and layout from the live editor state, then discarded them whenever a saved report was selected and called GET /reports/{id}/export/... instead, which re-runs the stored report. The on-screen preview meanwhile posts the live state to /reports/preview, so the two disagreed: changing columns, sorting or grouping updated the preview but produced a byte-identical export, and the configuration had to be saved before it could be exported at all. Exports now always post the live state to the preview endpoints, so the file matches the preview it was requested from. The client keeps the report name for the downloaded filename, since those endpoints answer with a generic one. The query AST and layout are now built once and shared by the preview, the save payload and both exports, so they cannot drift apart again. --- .../__tests__/reports-live-exports.test.tsx | 130 ++++++++++ .../routes/reports.$databaseId.$tableId.tsx | 233 ++++++------------ 2 files changed, 201 insertions(+), 162 deletions(-) create mode 100644 apps/client/src/routes/__tests__/reports-live-exports.test.tsx diff --git a/apps/client/src/routes/__tests__/reports-live-exports.test.tsx b/apps/client/src/routes/__tests__/reports-live-exports.test.tsx new file mode 100644 index 0000000..ba6c8eb --- /dev/null +++ b/apps/client/src/routes/__tests__/reports-live-exports.test.tsx @@ -0,0 +1,130 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { apiClient } from '../../lib/apiClient'; +import { render } from '../../test/render'; + +vi.mock('@tanstack/react-router', () => ({ + createFileRoute: () => () => ({}), + useParams: () => ({ databaseId: 'db-1', tableId: 'tbl-1' }), + useNavigate: () => vi.fn(), + Link: ({ children, to, ...props }: any) => { + delete props.params; + delete props.search; + return ( + + {children} + + ); + }, +})); + +import { ReportBuilderPage } from '../reports.$databaseId.$tableId'; + +const savedReport = { + id: 'rep-1', + table_id: 'tbl-1', + name: 'Rapport sauvegarde', + query: { select: ['Titre'], group_by: 'Ville', sort: [] }, + layout: { + fields: [{ name: 'Titre', visible: true, order: 1 }], + view_id: 'view-1', + per_page: 10, + }, +}; + +const mockFields = [ + { id: 'f1', name: 'Titre', type: 'title', position: 0, options: {}, validation: {} }, + { id: 'f2', name: 'Ville', type: 'text', position: 1, options: {}, validation: {} }, +]; + +function renderPage() { + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + return render( + + + + ); +} + +describe('report exports follow the on-screen configuration', () => { + let fetchMock: ReturnType; + + beforeEach(() => { + vi.spyOn(apiClient, 'get').mockImplementation((url: string) => { + if (url.startsWith('/databases/')) + return Promise.resolve({ id: 'db-1', name: 'Base' }) as any; + if (url.startsWith('/tables/')) return Promise.resolve({ id: 'tbl-1', name: 'Table' }) as any; + if (url.startsWith('/fields')) return Promise.resolve(mockFields) as any; + if (url.startsWith('/reports')) return Promise.resolve([savedReport]) as any; + if (url.startsWith('/views')) return Promise.resolve([]) as any; + return Promise.resolve([]) as any; + }); + vi.spyOn(apiClient, 'post').mockResolvedValue({ columns: ['Titre'], groups: [] } as any); + + fetchMock = vi.fn().mockResolvedValue({ + ok: true, + headers: { get: () => 'attachment; filename="rapport_apercu.csv"' }, + blob: async () => new Blob(['col']), + }); + vi.stubGlobal('fetch', fetchMock); + // The print tab queries this during render; jsdom does not provide it. + window.matchMedia = vi.fn().mockReturnValue({ matches: false }) as any; + vi.stubGlobal('URL', { + ...window.URL, + createObjectURL: vi.fn(() => 'blob:mock'), + revokeObjectURL: vi.fn(), + }); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + }); + + it('posts the live configuration instead of re-exporting the saved report', async () => { + const user = userEvent.setup(); + renderPage(); + + // Select the saved report: this is the case that used to bypass the live state. + await user.click(await screen.findByRole('button', { name: /Rapport sauvegarde/ })); + + await waitFor(() => { + expect(screen.getByRole('button', { name: /CSV/ })).toBeEnabled(); + }); + + await user.click(screen.getByRole('button', { name: /CSV/ })); + + await waitFor(() => expect(fetchMock).toHaveBeenCalled()); + + const [url, init] = fetchMock.mock.calls[0]; + // Previously this was GET /reports/rep-1/export/csv, which re-ran the *stored* + // report and ignored anything edited since the last save. + expect(url).toContain('/reports/preview/csv'); + expect(init.method).toBe('POST'); + + const body = JSON.parse(init.body); + expect(body.table_id).toBe('tbl-1'); + expect(body.query.select).toEqual(['Titre']); + expect(body.layout.view_id).toBe('view-1'); + }); + + it('sends the PDF export through the same live path', async () => { + const user = userEvent.setup(); + renderPage(); + + await user.click(await screen.findByRole('button', { name: /Rapport sauvegarde/ })); + await waitFor(() => { + expect(screen.getByRole('button', { name: /PDF/ })).toBeEnabled(); + }); + + await user.click(screen.getByRole('button', { name: /PDF/ })); + await waitFor(() => expect(fetchMock).toHaveBeenCalled()); + + const [url, init] = fetchMock.mock.calls[0]; + expect(url).toContain('/reports/preview/pdf'); + expect(init.method).toBe('POST'); + expect(JSON.parse(init.body).name).toBe('Rapport sauvegarde'); + }); +}); diff --git a/apps/client/src/routes/reports.$databaseId.$tableId.tsx b/apps/client/src/routes/reports.$databaseId.$tableId.tsx index 207dcdc..fb67e10 100644 --- a/apps/client/src/routes/reports.$databaseId.$tableId.tsx +++ b/apps/client/src/routes/reports.$databaseId.$tableId.tsx @@ -67,7 +67,7 @@ interface SortRule { direction: 'asc' | 'desc'; } -function ReportBuilderPage() { +export function ReportBuilderPage() { const { databaseId, tableId } = useParams({ from: '/reports/$databaseId/$tableId' }); const { t } = useI18n(); const queryClient = useQueryClient(); @@ -300,42 +300,44 @@ function ReportBuilderPage() { } }, [selectedReportId, reportsQuery.data, fieldsQuery.data]); + // Single description of the configuration on screen, shared by the preview, the + // save payload and both exports so they cannot drift apart. + const buildQueryAST = () => ({ + select: selectedColumns, + group_by: groupField || undefined, + sort: sorts.map((s) => ({ field: s.field, direction: s.direction })), + where: + conditions.length > 0 + ? { + logic, + conditions: conditions.map((c) => ({ + field: c.field, + operator: c.operator, + value: c.value, + })), + } + : undefined, + }); + + const buildLayout = () => ({ + fields: selectedColumns.map((col, idx) => ({ + name: col, + visible: true, + order: idx + 1, + })), + show_headers_only: showHeadersOnly, + view_id: selectedViewId || undefined, + per_page: perPage, + }); + // Mutators for Saving, Updating and Deleting Reports const saveReportMutation = useMutation({ mutationFn: async () => { - const queryAST = { - select: selectedColumns, - group_by: groupField || undefined, - sort: sorts.map((s) => ({ field: s.field, direction: s.direction })), - where: - conditions.length > 0 - ? { - logic, - conditions: conditions.map((c) => ({ - field: c.field, - operator: c.operator, - value: c.value, - })), - } - : undefined, - }; - - const layoutObj = { - fields: selectedColumns.map((col, idx) => ({ - name: col, - visible: true, - order: idx + 1, - })), - show_headers_only: showHeadersOnly, - view_id: selectedViewId || undefined, - per_page: perPage, - }; - const payload = { table_id: tableId, name: reportName || t('reports.newReport'), - query: queryAST, - layout: layoutObj, + query: buildQueryAST(), + layout: buildLayout(), }; if (selectedReportId) { @@ -378,43 +380,14 @@ function ReportBuilderPage() { currentPage, perPage, ], - queryFn: () => { - const queryAST = { - select: selectedColumns, - group_by: groupField || undefined, - sort: sorts.map((s) => ({ field: s.field, direction: s.direction })), - where: - conditions.length > 0 - ? { - logic, - conditions: conditions.map((c) => ({ - field: c.field, - operator: c.operator, - value: c.value, - })), - } - : undefined, - }; - - const layoutObj = { - fields: selectedColumns.map((col, idx) => ({ - name: col, - visible: true, - order: idx + 1, - })), - show_headers_only: showHeadersOnly, - view_id: selectedViewId || undefined, - per_page: perPage, - }; - - return apiClient.post('/reports/preview', { + queryFn: () => + apiClient.post('/reports/preview', { table_id: tableId, - query: queryAST, - layout: layoutObj, + query: buildQueryAST(), + layout: buildLayout(), per_page: showHeadersOnly ? undefined : perPage, page: showHeadersOnly ? undefined : currentPage, - }); - }, + }), enabled: selectedColumns.length > 0, }); @@ -468,7 +441,10 @@ function ReportBuilderPage() { url: string, method: 'GET' | 'POST', body?: any, - defaultFilename: string = 'export' + defaultFilename: string = 'export', + // The preview endpoints always answer with a generic filename; when the report + // has a name, prefer it over what the server suggests. + forceFilename = false ) => { try { const response = await fetch(url.startsWith('/api') ? url : `/api/v1${url}`, { @@ -486,7 +462,7 @@ function ReportBuilderPage() { const disposition = response.headers.get('Content-Disposition'); let filename = defaultFilename; - if (disposition && disposition.indexOf('attachment') !== -1) { + if (!forceFilename && disposition && disposition.indexOf('attachment') !== -1) { const filenameRegex = /filename[^;=\n]*=((['"]).*?\2|[^;\n]*)/; const matches = filenameRegex.exec(disposition); if (matches != null && matches[1]) { @@ -509,105 +485,38 @@ function ReportBuilderPage() { } }; + /** + * Exports go through the preview endpoints with the configuration currently on + * screen. Routing a selected report to /reports/{id}/export instead would export + * the last *saved* state, so exports would silently ignore unsaved edits. + */ const handleExportCsv = () => { - const queryAST = { - select: selectedColumns, - group_by: groupField || undefined, - sort: sorts.map((s) => ({ field: s.field, direction: s.direction })), - where: - conditions.length > 0 - ? { - logic, - conditions: conditions.map((c) => ({ - field: c.field, - operator: c.operator, - value: c.value, - })), - } - : undefined, - }; - - const layoutObj = { - fields: selectedColumns.map((col, idx) => ({ - name: col, - visible: true, - order: idx + 1, - })), - show_headers_only: showHeadersOnly, - view_id: selectedViewId || undefined, - per_page: perPage, - }; - - if (selectedReportId) { - void downloadFile( - `/reports/${selectedReportId}/export/csv`, - 'GET', - undefined, - `${reportName || 'rapport'}.csv` - ); - } else { - void downloadFile( - '/reports/preview/csv', - 'POST', - { - table_id: tableId, - query: queryAST, - layout: layoutObj, - }, - 'rapport_apercu.csv' - ); - } + void downloadFile( + '/reports/preview/csv', + 'POST', + { + table_id: tableId, + query: buildQueryAST(), + layout: buildLayout(), + }, + `${reportName || 'rapport'}.csv`, + true + ); }; const handleExportPdf = () => { - const queryAST = { - select: selectedColumns, - group_by: groupField || undefined, - sort: sorts.map((s) => ({ field: s.field, direction: s.direction })), - where: - conditions.length > 0 - ? { - logic, - conditions: conditions.map((c) => ({ - field: c.field, - operator: c.operator, - value: c.value, - })), - } - : undefined, - }; - - const layoutObj = { - fields: selectedColumns.map((col, idx) => ({ - name: col, - visible: true, - order: idx + 1, - })), - show_headers_only: showHeadersOnly, - view_id: selectedViewId || undefined, - per_page: perPage, - }; - - if (selectedReportId) { - void downloadFile( - `/reports/${selectedReportId}/export/pdf`, - 'GET', - undefined, - `${reportName || 'rapport'}.pdf` - ); - } else { - void downloadFile( - '/reports/preview/pdf', - 'POST', - { - table_id: tableId, - name: reportName || 'Rapport temporaire', - query: queryAST, - layout: layoutObj, - }, - 'rapport_apercu.pdf' - ); - } + void downloadFile( + '/reports/preview/pdf', + 'POST', + { + table_id: tableId, + name: reportName || t('reports.newReport'), + query: buildQueryAST(), + layout: buildLayout(), + }, + `${reportName || 'rapport'}.pdf`, + true + ); }; const handleDeleteReport = () => {