From 776e8cc1220c3e8f6d8ddea58ec3dae1da39593e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 08:58:20 +0000 Subject: [PATCH] test(attachments): pin the both-dialects signed-URL readers (objectstack#3689) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `GET /api/v1/storage/files/:fileId/url` answered a bare `{ url }` and now answers the declared `{ success: true, data: { url } }` envelope (objectstack#3689 — the success-path half of the #3675 error-path fix). Both console readers of that route — `RecordAttachmentsPanel.handleDownload` and `ApprovalsInboxPage.openAttachment` — already took either shape, which is what lets the two repos land in any order. Nothing to fix, then; what was missing is any statement that the tolerance is deliberate. So the panel's download suite now drives the enveloped shape in its main case and the legacy bare one in a dedicated case, mirroring how the same file already covers both error dialects, and both readers say in a comment why they read two. No behaviour change. --- .../src/pages/system/ApprovalsInboxPage.tsx | 3 ++ .../src/views/RecordAttachmentsPanel.tsx | 4 ++ .../__tests__/RecordAttachmentsPanel.test.tsx | 37 +++++++++++++++++-- 3 files changed, 40 insertions(+), 4 deletions(-) diff --git a/apps/console/src/pages/system/ApprovalsInboxPage.tsx b/apps/console/src/pages/system/ApprovalsInboxPage.tsx index 70eb041d72..c9e344f776 100644 --- a/apps/console/src/pages/system/ApprovalsInboxPage.tsx +++ b/apps/console/src/pages/system/ApprovalsInboxPage.tsx @@ -449,6 +449,9 @@ export function ApprovalsInboxPage() { const res = await authFetch(`${base}/api/v1/storage/files/${encodeURIComponent(att.id)}/url`); if (!res.ok) throw new Error(`HTTP_${res.status}`); const body = await res.json().catch(() => null); + // Both dialects: the declared `{ success: true, data: { url } }` envelope + // this route answers as of objectstack#3689, and the bare `{ url }` an + // older server still sends — the console deploys independently of it. const raw = body?.data?.url ?? body?.url; if (!raw) throw new Error('NO_URL'); // Signed URLs from the local adapter are relative; S3/GCS are absolute. diff --git a/packages/app-shell/src/views/RecordAttachmentsPanel.tsx b/packages/app-shell/src/views/RecordAttachmentsPanel.tsx index 54cbad5ef5..962e24106c 100644 --- a/packages/app-shell/src/views/RecordAttachmentsPanel.tsx +++ b/packages/app-shell/src/views/RecordAttachmentsPanel.tsx @@ -228,6 +228,10 @@ export const RecordAttachmentsPanel: React.FC = ({ throw Object.assign(new Error(code ?? `Download failed (${res.status})`), { code }); } const body = await res.json(); + // Both URL dialects, for the same independent-deploy reason as the + // error branch above: the route answered a bare `{ url }` until + // objectstack#3689 moved it into the declared + // `{ success: true, data: { url } }` envelope. const url: string | undefined = body?.url ?? body?.data?.url; if (!url) throw new Error('Download URL missing from response'); const target = /^https?:/i.test(url) ? url : `${baseUrl}${url}`; diff --git a/packages/app-shell/src/views/__tests__/RecordAttachmentsPanel.test.tsx b/packages/app-shell/src/views/__tests__/RecordAttachmentsPanel.test.tsx index 3e160c9141..3044024eb8 100644 --- a/packages/app-shell/src/views/__tests__/RecordAttachmentsPanel.test.tsx +++ b/packages/app-shell/src/views/__tests__/RecordAttachmentsPanel.test.tsx @@ -97,11 +97,13 @@ describe('RecordAttachmentsPanel — server-denial error mapping (#2755)', () => describe('RecordAttachmentsPanel — authenticated signed-URL download (#2970)', () => { it('fetches /files/:id/url with auth and opens the signed URL', async () => { const openSpy = vi.spyOn(window, 'open').mockImplementation(() => null); + // The declared envelope the route answers as of objectstack#3689 — the URL + // moved from the top level down under `data`. const fetchSpy = vi.spyOn(globalThis, 'fetch').mockResolvedValue( - new Response(JSON.stringify({ url: '/api/v1/storage/_local/raw/tok123' }), { - status: 200, - headers: { 'Content-Type': 'application/json' }, - }), + new Response( + JSON.stringify({ success: true, data: { url: '/api/v1/storage/_local/raw/tok123' } }), + { status: 200, headers: { 'Content-Type': 'application/json' } }, + ), ); setup(makeDataSource()); await waitFor(() => expect(screen.getByText('report.pdf')).toBeInTheDocument()); @@ -190,4 +192,31 @@ describe('RecordAttachmentsPanel — authenticated signed-URL download (#2970)', ); expect(openSpy).not.toHaveBeenCalled(); }); + + // Same reasoning on the SUCCESS path (objectstack#3689): the route used to + // answer a bare `{ url }` with no envelope at all. The reader takes both, so + // whichever repo lands first, downloads keep working — and this pins that + // tolerance as deliberate rather than incidental. + it('still opens the legacy bare `{ url }` shape (older server)', async () => { + const openSpy = vi.spyOn(window, 'open').mockImplementation(() => null); + vi.spyOn(globalThis, 'fetch').mockResolvedValue( + new Response(JSON.stringify({ url: '/api/v1/storage/_local/raw/legacy-tok' }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }), + ); + setup(makeDataSource()); + await waitFor(() => expect(screen.getByText('report.pdf')).toBeInTheDocument()); + + await userEvent.setup().click(screen.getByRole('button', { name: 'Download' })); + + await waitFor(() => + expect(openSpy).toHaveBeenCalledWith( + expect.stringContaining('/api/v1/storage/_local/raw/legacy-tok'), + '_blank', + 'noopener,noreferrer', + ), + ); + expect(screen.queryByRole('alert')).not.toBeInTheDocument(); + }); });