test(attachments): pin the both-dialects signed-URL readers (objectstack#3689) (#2913)

`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.

Co-authored-by: Claude <noreply@anthropic.com>

Author: os-zhuang

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.

packages/app-shell/src/views/RecordAttachmentsPanel.tsx

@@ -228,6 +228,10 @@ export const RecordAttachmentsPanel: React.FC<RecordAttachmentsPanelProps> = ({
           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}`;

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();
+  });
 });