Fix: Use Authenticated API for Frontend File Downloads

frontend/package.json

@@ -65,5 +65,10 @@
     "nth-check": ">=2.0.1",
     "postcss": ">=8.4.31",
     "webpack-dev-server": "4.15.2"
+  },
+  "jest": {
+    "transformIgnorePatterns": [
+      "/node_modules/(?!axios)/"
+    ]
   }
 }

frontend/src/components/collections/LightweightCollectionDetail.test.tsx

@@ -0,0 +1,121 @@
+import React from 'react';
+import { render, screen, fireEvent, waitFor } from '@testing-library/react';
+import { BrowserRouter as Router } from 'react-router-dom';
+import LightweightCollectionDetail from './LightweightCollectionDetail';
+import apiClient from '../../services/apiClient';
+import { useNotification } from '../../contexts/NotificationContext';
+
+// Mock the API client
+jest.mock('../../services/apiClient');
+const mockedApiClient = apiClient as jest.Mocked<typeof apiClient>;
+
+// Mock the useNotification hook
+jest.mock('../../contexts/NotificationContext', () => ({
+  useNotification: jest.fn(),
+}));
+const mockedUseNotification = useNotification as jest.Mock;
+
+// Mock react-router-dom hooks
+jest.mock('react-router-dom', () => ({
+  ...jest.requireActual('react-router-dom'),
+  useParams: () => ({ id: '123' }),
+  useNavigate: () => jest.fn(),
+}));
+
+// Mock window.URL.createObjectURL and revokeObjectURL
+const createObjectURL = jest.fn(() => 'mock-blob-url');
+const revokeObjectURL = jest.fn();
+window.URL.createObjectURL = createObjectURL;
+window.URL.revokeObjectURL = revokeObjectURL;
+
+describe('LightweightCollectionDetail', () => {
+  const mockCollection = {
+    id: '123',
+    name: 'Test Collection',
+    description: 'Test Description',
+    status: 'ready' as const,
+    documents: [
+      { id: 'doc1', name: 'test-file.pdf', type: 'application/pdf', size: 1024, uploadedAt: new Date(), status: 'ready' as const },
+    ],
+    createdAt: new Date(),
+    updatedAt: new Date(),
+    documentCount: 1,
+  };
+
+  const addNotification = jest.fn();
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+    mockedUseNotification.mockReturnValue({ addNotification });
+    mockedApiClient.getCollection.mockResolvedValue(mockCollection);
+  });
+
+  it('should handle document download correctly', async () => {
+    const mockBlob = new Blob(['test content'], { type: 'application/pdf' });
+    mockedApiClient.downloadDocument.mockResolvedValue(mockBlob);
+
+    render(
+      <Router>
+        <LightweightCollectionDetail />
+      </Router>
+    );
+
+    // Wait for the component to finish loading
+    await waitFor(() => expect(screen.getByRole('heading', { name: /test collection/i })).toBeInTheDocument());
+
+    // Find the download button for the document
+    const downloadButton = screen.getByTitle('Download document');
+    expect(downloadButton).toBeInTheDocument();
+
+    // Create a spy on document.createElement to check if the link is created
+    const linkSpy = jest.spyOn(document, 'createElement');
+
+    // Click the download button
+    fireEvent.click(downloadButton);
+
+    // Wait for the download logic to execute
+    await waitFor(() => {
+      // Check if the API was called correctly
+      expect(mockedApiClient.downloadDocument).toHaveBeenCalledWith('123', 'doc1');
+
+      // Check if a blob URL was created
+      expect(createObjectURL).toHaveBeenCalledWith(mockBlob);
+
+      // Check if a link element was created for the download
+      expect(linkSpy).toHaveBeenCalledWith('a');
+    });
+
+    // Check if the success notification was shown
+    expect(addNotification).toHaveBeenCalledWith('success', 'Download Started', 'Downloading test-file.pdf...');
+
+    // Restore the spy
+    linkSpy.mockRestore();
+  });
+
+  it('should show an error notification on download failure', async () => {
+    const mockError = new Error('Download failed');
+    mockedApiClient.downloadDocument.mockRejectedValue(mockError);
+
+    render(
+      <Router>
+        <LightweightCollectionDetail />
+      </Router>
+    );
+
+    await waitFor(() => expect(screen.getByRole('heading', { name: /test collection/i })).toBeInTheDocument());
+
+    // Clear the mock from the initial load notification
+    addNotification.mockClear();
+
+    const downloadButton = screen.getByTitle('Download document');
+    fireEvent.click(downloadButton);
+
+    await waitFor(() => {
+      expect(mockedApiClient.downloadDocument).toHaveBeenCalledWith('123', 'doc1');
+    });
+
+    await waitFor(() => {
+      expect(addNotification).toHaveBeenCalledWith('error', 'Download Error', 'Failed to download test-file.pdf.');
+    });
+  });
+});

frontend/src/components/collections/LightweightCollectionDetail.tsx

@@ -164,18 +164,23 @@ const LightweightCollectionDetail: React.FC = () => {
   };
 
   const handleDownloadDocument = async (file: CollectionFile) => {
+    if (!collection) return;
+
     try {
-      // Create a temporary download link
-      const downloadUrl = `${process.env.REACT_APP_BACKEND_URL || ''}/api/collections/${collection?.id}/documents/${file.id}/download`;
+      const blob = await apiClient.downloadDocument(collection.id, file.id);
+      const blobUrl = window.URL.createObjectURL(blob);
 
       // Create temporary link element and trigger download
       const link = document.createElement('a');
-      link.href = downloadUrl;
+      link.href = blobUrl;
       link.download = file.name;
       link.style.display = 'none';
       document.body.appendChild(link);
       link.click();
+
+      // Clean up the temporary link and blob URL
       document.body.removeChild(link);
+      window.URL.revokeObjectURL(blobUrl);
 
       addNotification('success', 'Download Started', `Downloading ${file.name}...`);
     } catch (error) {

frontend/src/services/apiClient.ts

@@ -431,6 +431,16 @@ class ApiClient {
     await this.client.delete(`/api/collections/${collectionId}/documents/${documentId}`);
   }
 
+  async downloadDocument(collectionId: string, documentId: string): Promise<Blob> {
+    const response: AxiosResponse<Blob> = await this.client.get(
+      `/api/collections/${collectionId}/documents/${documentId}/download`,
+      {
+        responseType: 'blob',
+      }
+    );
+    return response.data;
+  }
+
   // User API
   async getCurrentUser(): Promise<User> {
     // Use the correct auth endpoint that returns user info

frontend/src/setupTests.ts

@@ -0,0 +1,5 @@
+// jest-dom adds custom jest matchers for asserting on DOM nodes.
+// allows you to do things like:
+// expect(element).toHaveTextContent(/react/i)
+// learn more: https://github.com/testing-library/jest-dom
+import '@testing-library/jest-dom';

jules-scratch/verification/verify_download.py

@@ -0,0 +1,32 @@
+import re
+from playwright.sync_api import sync_playwright, expect
+
+def run(playwright):
+    browser = playwright.chromium.launch(headless=True)
+    context = browser.new_context()
+    page = context.new_page()
+
+    # Navigate to the collection detail page
+    page.goto("http://localhost:3000/lightweight-collections/8f60c52e-9d68-4a49-82a9-c8a46356a59b")
+
+    # Wait for the page to load
+    expect(page.get_by_role("heading", name=re.compile(r"Financial Reports Q2 2024", i))).to_be_visible()
+
+    # Find the first download button
+    download_button = page.get_by_title("Download document").first
+    expect(download_button).to_be_visible()
+
+    # Click the download button
+    download_button.click()
+
+    # Wait for the "Download Started" notification to appear
+    notification = page.get_by_text("Download Started")
+    expect(notification).to_be_visible()
+
+    # Take a screenshot
+    page.screenshot(path="jules-scratch/verification/verification.png")
+
+    browser.close()
+
+with sync_playwright() as playwright:
+    run(playwright)