Merge pull request #251 from christianlouis/claude/fix-failing-tests

fix(tests): fix remaining 8 failing tests - storage reorganization and file detail enhancements

tests/conftest.py

@@ -167,6 +167,55 @@ startxref
     return pdf_path
 
 
+@pytest.fixture
+def sample_pdf_file(test_workdir):
+    """Create a sample PDF file for testing, returning Path object."""
+    from pathlib import Path
+
+    pdf_path = Path(test_workdir) / "test.pdf"
+
+    # Create a minimal valid PDF
+    pdf_content = b"""%PDF-1.4
+1 0 obj
+<<
+/Type /Catalog
+/Pages 2 0 R
+>>
+endobj
+2 0 obj
+<<
+/Type /Pages
+/Kids [3 0 R]
+/Count 1
+>>
+endobj
+3 0 obj
+<<
+/Type /Page
+/Parent 2 0 R
+/MediaBox [0 0 612 792]
+>>
+endobj
+xref
+0 4
+0000000000 65535 f
+0000000009 00000 n
+0000000058 00000 n
+0000000115 00000 n
+trailer
+<<
+/Size 4
+/Root 1 0 R
+>>
+startxref
+197
+%%EOF
+"""
+
+    pdf_path.write_bytes(pdf_content)
+    return pdf_path
+
+
 @pytest.fixture
 def sample_text_file(test_workdir) -> str:
     """Create a sample text file for testing."""

tests/test_endpoint_registration.py

@@ -6,6 +6,7 @@ that endpoints remain accessible after code refactoring or reorganization.
 """
 
 import pytest
+from unittest.mock import Mock, patch
 
 # Test constants
 TEST_URL = "https://example.com/test.pdf"
@@ -15,8 +16,23 @@ TEST_URL = "https://example.com/test.pdf"
 class TestEndpointRegistration:
     """Verify that critical API endpoints are registered in the FastAPI app"""
 
-    def test_process_url_endpoint_exists(self, client):
+    @patch("app.api.url_upload.requests.get")
+    @patch("app.api.url_upload.process_document")
+    def test_process_url_endpoint_exists(self, mock_process_document, mock_requests_get, client):
         """Verify that /api/process-url endpoint is registered and accessible"""
+        # Mock successful download to prevent actual HTTP requests
+        mock_response = Mock()
+        mock_response.status_code = 200
+        mock_response.headers = {"Content-Type": "application/pdf", "Content-Length": "1024"}
+        mock_response.iter_content = Mock(return_value=[b"PDF content"])
+        mock_response.raise_for_status = Mock()
+        mock_requests_get.return_value = mock_response
+
+        # Mock Celery task
+        mock_task = Mock()
+        mock_task.id = "test-task-id"
+        mock_process_document.delay.return_value = mock_task
+
         # Make a request to the endpoint - it should not return 404
         response = client.post(
             "/api/process-url",
@@ -32,8 +48,23 @@ class TestEndpointRegistration:
             f"Verify that url_upload_router is included in app/api/__init__.py"
         )
 
-    def test_process_url_endpoint_accepts_post(self, client):
+    @patch("app.api.url_upload.requests.get")
+    @patch("app.api.url_upload.process_document")
+    def test_process_url_endpoint_accepts_post(self, mock_process_document, mock_requests_get, client):
         """Verify that /api/process-url accepts POST requests"""
+        # Mock successful download to prevent actual HTTP requests
+        mock_response = Mock()
+        mock_response.status_code = 200
+        mock_response.headers = {"Content-Type": "application/pdf", "Content-Length": "1024"}
+        mock_response.iter_content = Mock(return_value=[b"PDF content"])
+        mock_response.raise_for_status = Mock()
+        mock_requests_get.return_value = mock_response
+
+        # Mock Celery task
+        mock_task = Mock()
+        mock_task.id = "test-task-id"
+        mock_process_document.delay.return_value = mock_task
+
         # Try POST request
         response = client.post(
             "/api/process-url",
@@ -46,8 +77,23 @@ class TestEndpointRegistration:
             f"Verify the endpoint is decorated with @router.post()"
         )
 
-    def test_api_router_included_in_app(self, client):
+    @patch("app.api.url_upload.requests.get")
+    @patch("app.api.url_upload.process_document")
+    def test_api_router_included_in_app(self, mock_process_document, mock_requests_get, client):
         """Verify that the main API router is included in the FastAPI app"""
+        # Mock successful download for /api/process-url test
+        mock_response = Mock()
+        mock_response.status_code = 200
+        mock_response.headers = {"Content-Type": "application/pdf", "Content-Length": "1024"}
+        mock_response.iter_content = Mock(return_value=[b"PDF content"])
+        mock_response.raise_for_status = Mock()
+        mock_requests_get.return_value = mock_response
+
+        # Mock Celery task
+        mock_task = Mock()
+        mock_task.id = "test-task-id"
+        mock_process_document.delay.return_value = mock_task
+
         # Test a few known API endpoints to ensure the /api prefix works
         endpoints_to_check = [
             ("/api/process-url", "post"),

tests/test_file_queries.py

@@ -9,7 +9,7 @@ from sqlalchemy import create_engine
 from sqlalchemy.orm import sessionmaker
 
 from app.database import Base
-from app.models import FileRecord, ProcessingLog
+from app.models import FileRecord, FileProcessingStep
 from app.utils.file_queries import apply_status_filter
 
 
@@ -28,7 +28,7 @@ def db_session():
 @pytest.fixture
 def sample_files(db_session):
     """Create sample files with different processing statuses."""
-    # File 1: pending (no logs)
+    # File 1: pending (no steps)
     file1 = FileRecord(
         filehash="hash1",
         original_filename="pending.pdf",
@@ -39,7 +39,7 @@ def sample_files(db_session):
     db_session.add(file1)
     db_session.flush()
 
-    # File 2: processing (has in_progress log)
+    # File 2: processing (has in_progress step)
     file2 = FileRecord(
         filehash="hash2",
         original_filename="processing.pdf",
@@ -50,16 +50,14 @@ def sample_files(db_session):
     db_session.add(file2)
     db_session.flush()
 
-    log2 = ProcessingLog(
+    step2 = FileProcessingStep(
         file_id=file2.id,
-        task_id="task2",
         step_name="extract_text",
         status="in_progress",
-        message="Processing...",
     )
-    db_session.add(log2)
+    db_session.add(step2)
 
-    # File 3: failed (has failure log)
+    # File 3: failed (has failure step)
     file3 = FileRecord(
         filehash="hash3",
         original_filename="failed.pdf",
@@ -70,16 +68,15 @@ def sample_files(db_session):
     db_session.add(file3)
     db_session.flush()
 
-    log3 = ProcessingLog(
+    step3 = FileProcessingStep(
         file_id=file3.id,
-        task_id="task3",
         step_name="extract_text",
         status="failure",
-        message="Error occurred",
+        error_message="Error occurred",
     )
-    db_session.add(log3)
+    db_session.add(step3)
 
-    # File 4: completed (has success log, no failures)
+    # File 4: completed (has success step, no failures)
     file4 = FileRecord(
         filehash="hash4",
         original_filename="completed.pdf",
@@ -90,10 +87,10 @@ def sample_files(db_session):
     db_session.add(file4)
     db_session.flush()
 
-    log4 = ProcessingLog(file_id=file4.id, task_id="task4", step_name="extract_text", status="success", message="Done")
+    step4 = FileProcessingStep(file_id=file4.id, step_name="extract_text", status="success")
-    db_session.add(log4)
+    db_session.add(step4)
 
-    # File 5: completed with multiple success logs
+    # File 5: completed with multiple success steps
     file5 = FileRecord(
         filehash="hash5",
         original_filename="completed2.pdf",
@@ -104,21 +101,17 @@ def sample_files(db_session):
     db_session.add(file5)
     db_session.flush()
 
-    log5a = ProcessingLog(
+    step5a = FileProcessingStep(
         file_id=file5.id,
-        task_id="task5a",
         step_name="extract_text",
         status="success",
-        message="Step 1 done",
     )
-    log5b = ProcessingLog(
+    step5b = FileProcessingStep(
         file_id=file5.id,
-        task_id="task5b",
         step_name="extract_metadata_with_gpt",
         status="success",
-        message="Step 2 done",
     )
-    db_session.add_all([log5a, log5b])
+    db_session.add_all([step5a, step5b])
 
     # File 6: has success but also failure (should be filtered out from completed)
     file6 = FileRecord(
@@ -131,21 +124,18 @@ def sample_files(db_session):
     db_session.add(file6)
     db_session.flush()
 
-    log6a = ProcessingLog(
+    step6a = FileProcessingStep(
         file_id=file6.id,
-        task_id="task6a",
         step_name="extract_text",
         status="success",
-        message="Step 1 done",
     )
-    log6b = ProcessingLog(
+    step6b = FileProcessingStep(
         file_id=file6.id,
-        task_id="task6b",
         step_name="upload_to_s3",
         status="failure",
-        message="Upload failed",
+        error_message="Upload failed",
     )
-    db_session.add_all([log6a, log6b])
+    db_session.add_all([step6a, step6b])
 
     db_session.commit()
 
@@ -172,12 +162,12 @@ def test_apply_status_filter_none(db_session, sample_files):
 
 @pytest.mark.unit
 def test_apply_status_filter_pending(db_session, sample_files):
-    """Test filtering for pending files (no logs)."""
+    """Test filtering for pending files (no steps)."""
     query = db_session.query(FileRecord)
     filtered_query = apply_status_filter(query, db_session, "pending")
     results = filtered_query.all()
 
-    # Should return only file1 (no logs)
+    # Should return only file1 (no steps)
     assert len(results) == 1
     assert results[0].filehash == "hash1"
     assert results[0].original_filename == "pending.pdf"
@@ -190,7 +180,7 @@ def test_apply_status_filter_processing(db_session, sample_files):
     filtered_query = apply_status_filter(query, db_session, "processing")
     results = filtered_query.all()
 
-    # Should return only file2 (has in_progress log)
+    # Should return only file2 (has in_progress step)
     assert len(results) == 1
     assert results[0].filehash == "hash2"
     assert results[0].original_filename == "processing.pdf"
@@ -203,7 +193,7 @@ def test_apply_status_filter_failed(db_session, sample_files):
     filtered_query = apply_status_filter(query, db_session, "failed")
     results = filtered_query.all()
 
-    # Should return file3 and file6 (both have failure logs)
+    # Should return file3 and file6 (both have failure steps)
     assert len(results) == 2
     filehashes = {r.filehash for r in results}
     assert "hash3" in filehashes
@@ -217,7 +207,7 @@ def test_apply_status_filter_completed(db_session, sample_files):
     filtered_query = apply_status_filter(query, db_session, "completed")
     results = filtered_query.all()
 
-    # Should return file4 and file5 (success logs, no failures)
+    # Should return file4 and file5 (success steps, no failures)
     # file6 should NOT be included (has both success and failure)
     assert len(results) == 2
     filehashes = {r.filehash for r in results}

tests/test_file_status_fix.py

@@ -1,331 +1,203 @@
 """
-Tests for file status and metrics calculation bug fixes.
+Tests for file status and metrics calculation using FileProcessingStep model.
 
 This test module verifies that:
-1. Status calculation only considers the latest status per unique step
+1. Status calculation uses FileProcessingStep entries correctly
-2. Metrics counting only uses the latest status per unique step
+2. Metrics counting uses FileProcessingStep entries correctly
 3. Files with completed steps show "completed" not "processing"
 """
 
 from datetime import datetime, timedelta
 
 import pytest
+from sqlalchemy import create_engine
+from sqlalchemy.orm import sessionmaker
 
-from app.utils.file_status import _compute_status_from_logs
+from app.database import Base
-from app.views.files import _compute_step_summary
+from app.models import FileRecord, FileProcessingStep
+from app.utils.step_manager import get_file_overall_status, get_step_summary, initialize_file_steps, update_step_status
+
+
+@pytest.fixture
+def db_session():
+    """Create an in-memory SQLite database for testing."""
+    engine = create_engine("sqlite:///:memory:")
+    Base.metadata.create_all(engine)
+    SessionLocal = sessionmaker(bind=engine)
+    session = SessionLocal()
+    yield session
+    session.close()
+    Base.metadata.drop_all(engine)
 
 
 @pytest.mark.unit
-class TestFileStatusBugFixes:
+class TestFileStatusCalculation:
-    """Test fixes for status calculation bugs."""
+    """Test file status calculation using FileProcessingStep."""
 
-    def test_status_not_stuck_on_old_in_progress(self):
+    def test_status_completed_when_all_steps_success(self, db_session):
         """
-        Test that status doesn't show "processing" when old in_progress logs exist
+        Test that status shows "completed" when all steps are success.
-        but latest status for all steps is success.
-
-        This simulates the bug where a file shows "Processing" even though
-        all steps have completed successfully.
         """
+        # Create file and initialize steps
+        file_record = FileRecord(
+            filehash="test1",
+            original_filename="test.pdf",
+            local_filename="/tmp/test.pdf",
+            file_size=1024
+        )
+        db_session.add(file_record)
+        db_session.commit()
 
-        class MockLog:
+        initialize_file_steps(db_session, file_record.id)
-            def __init__(self, step_name, status, timestamp):
-                self.step_name = step_name
-                self.status = status
-                self.timestamp = timestamp
 
-        now = datetime.now()
+        # Mark all steps as success
-        # Simulate logs ordered by timestamp desc (latest first)
+        from app.utils.step_manager import MAIN_PROCESSING_STEPS
-        logs = [
+        for step_name in MAIN_PROCESSING_STEPS:
-            # Latest logs (all success)
+            update_step_status(db_session, file_record.id, step_name, "success")
-            MockLog("upload_to_dropbox", "success", now - timedelta(minutes=1)),
-            MockLog("extract_metadata_with_gpt", "success", now - timedelta(minutes=2)),
-            MockLog("check_text", "success", now - timedelta(minutes=3)),
-            # Older in_progress logs that should be ignored
-            MockLog("upload_to_dropbox", "in_progress", now - timedelta(minutes=5)),
-            MockLog("extract_metadata_with_gpt", "in_progress", now - timedelta(minutes=6)),
-            MockLog("check_text", "in_progress", now - timedelta(minutes=7)),
-        ]
 
-        result = _compute_status_from_logs(logs)
+        result = get_file_overall_status(db_session, file_record.id)
 
-        # Should be completed, not processing
         assert result["status"] == "completed"
         assert result["has_errors"] is False
 
-    def test_status_shows_processing_for_active_tasks(self):
+    def test_status_processing_when_steps_in_progress(self, db_session):
         """
-        Test that status correctly shows "processing" when there are
+        Test that status shows "processing" when there are in_progress steps.
-        actually in-progress tasks (based on latest status).
         """
+        file_record = FileRecord(
+            filehash="test2",
+            original_filename="test2.pdf",
+            local_filename="/tmp/test2.pdf",
+            file_size=2048
+        )
+        db_session.add(file_record)
+        db_session.commit()
 
-        class MockLog:
+        initialize_file_steps(db_session, file_record.id)
-            def __init__(self, step_name, status, timestamp):
-                self.step_name = step_name
-                self.status = status
-                self.timestamp = timestamp
 
-        now = datetime.now()
+        # Mark some steps as success, one as in_progress
-        logs = [
+        update_step_status(db_session, file_record.id, "create_file_record", "success")
-            # One task actually in progress
+        update_step_status(db_session, file_record.id, "check_text", "in_progress")
-            MockLog("upload_to_dropbox", "in_progress", now - timedelta(minutes=1)),
-            # Other tasks completed
-            MockLog("extract_metadata_with_gpt", "success", now - timedelta(minutes=2)),
-            MockLog("check_text", "success", now - timedelta(minutes=3)),
-        ]
 
-        result = _compute_status_from_logs(logs)
+        result = get_file_overall_status(db_session, file_record.id)
 
-        # Should be processing because one task is actually in progress
         assert result["status"] == "processing"
         assert result["has_errors"] is False
 
-    def test_status_shows_failed_when_latest_has_failure(self):
+    def test_status_failed_when_steps_have_failure(self, db_session):
         """
-        Test that status shows "failed" when the latest status for any step is failure.
+        Test that status shows "failed" when any step has failure status.
         """
+        file_record = FileRecord(
+            filehash="test3",
+            original_filename="test3.pdf",
+            local_filename="/tmp/test3.pdf",
+            file_size=3072
+        )
+        db_session.add(file_record)
+        db_session.commit()
 
-        class MockLog:
+        initialize_file_steps(db_session, file_record.id)
-            def __init__(self, step_name, status, timestamp):
-                self.step_name = step_name
-                self.status = status
-                self.timestamp = timestamp
 
-        now = datetime.now()
+        # Mark some steps as success, one as failure
-        logs = [
+        update_step_status(db_session, file_record.id, "create_file_record", "success")
-            # One task failed (latest status)
+        update_step_status(db_session, file_record.id, "check_text", "success")
-            MockLog("upload_to_s3", "failure", now - timedelta(minutes=1)),
+        update_step_status(db_session, file_record.id, "extract_text", "failure", error_message="OCR failed")
-            # Other tasks completed
-            MockLog("upload_to_dropbox", "success", now - timedelta(minutes=2)),
-            MockLog("extract_metadata_with_gpt", "success", now - timedelta(minutes=3)),
-        ]
 
-        result = _compute_status_from_logs(logs)
+        result = get_file_overall_status(db_session, file_record.id)
 
         assert result["status"] == "failed"
         assert result["has_errors"] is True
 
 
 @pytest.mark.unit
-class TestMetricsCountingBugFixes:
+class TestMetricsCounting:
-    """Test fixes for metrics counting bugs."""
+    """Test metrics counting using FileProcessingStep."""
 
-    def test_main_steps_not_double_counted(self):
+    def test_main_steps_counted_correctly(self, db_session):
         """
-        Test that main processing steps are only counted once per step,
+        Test that main processing steps are counted correctly.
-        using the latest status, not counting all historical logs.
-
-        This simulates the bug where metrics show incorrect counts because
-        they count all logs instead of just the latest per step.
         """
-        from datetime import datetime, timedelta
+        file_record = FileRecord(
+            filehash="test4",
+            original_filename="test4.pdf",
+            local_filename="/tmp/test4.pdf",
+            file_size=4096
+        )
+        db_session.add(file_record)
+        db_session.commit()
 
-        class MockLog:
+        initialize_file_steps(db_session, file_record.id)
-            def __init__(self, step_name, status, timestamp):
-                self.step_name = step_name
-                self.status = status
-                self.timestamp = timestamp
 
-        now = datetime.now()
+        # Mark some steps with different statuses
-        # Simulate logs ordered by timestamp desc (latest first)
+        update_step_status(db_session, file_record.id, "create_file_record", "success")
-        logs = [
+        update_step_status(db_session, file_record.id, "check_text", "success")
-            # Latest status for each step (all success)
+        update_step_status(db_session, file_record.id, "extract_text", "in_progress")
-            MockLog("hash_file", "success", now - timedelta(minutes=1)),
-            MockLog("create_file_record", "success", now - timedelta(minutes=2)),
-            MockLog("extract_metadata_with_gpt", "success", now - timedelta(minutes=3)),
-            # Older in_progress logs that should be ignored
-            MockLog("hash_file", "in_progress", now - timedelta(minutes=5)),
-            MockLog("create_file_record", "in_progress", now - timedelta(minutes=6)),
-            MockLog("extract_metadata_with_gpt", "in_progress", now - timedelta(minutes=7)),
-        ]
 
-        summary = _compute_step_summary(logs)
+        summary = get_step_summary(db_session, file_record.id)
 
-        # Should count each main step only once
+        # Should count each step once
-        assert summary["total_main_steps"] == 3
+        from app.utils.step_manager import MAIN_PROCESSING_STEPS
-        assert summary["main"]["success"] == 3
+        assert summary["total_main_steps"] == len(MAIN_PROCESSING_STEPS)
-        assert summary["main"]["in_progress"] == 0  # No steps actually in progress
+        assert summary["main"]["success"] == 2
-        assert summary["main"]["failure"] == 0
+        assert summary["main"]["in_progress"] == 1
 
-    def test_upload_tasks_not_double_counted(self):
+    def test_upload_tasks_counted_correctly(self, db_session):
         """
-        Test that upload tasks are only counted once per destination,
+        Test that upload tasks are counted correctly.
-        using the latest status.
         """
-        from datetime import datetime, timedelta
+        file_record = FileRecord(
+            filehash="test5",
+            original_filename="test5.pdf",
+            local_filename="/tmp/test5.pdf",
+            file_size=5120
+        )
+        db_session.add(file_record)
+        db_session.commit()
 
-        class MockLog:
+        initialize_file_steps(db_session, file_record.id)
-            def __init__(self, step_name, status, timestamp):
+        from app.utils.step_manager import add_upload_steps
-                self.step_name = step_name
+        add_upload_steps(db_session, file_record.id, ["dropbox", "s3", "nextcloud"])
-                self.status = status
-                self.timestamp = timestamp
 
-        now = datetime.now()
+        # Mark upload steps with different statuses
-        # Logs ordered by timestamp desc (latest first)
+        update_step_status(db_session, file_record.id, "upload_to_dropbox", "success")
-        logs = [
+        update_step_status(db_session, file_record.id, "upload_to_s3", "failure")
-            # Latest status for uploads
+        update_step_status(db_session, file_record.id, "upload_to_nextcloud", "in_progress")
-            MockLog("upload_to_dropbox", "success", now - timedelta(minutes=1)),
-            MockLog("upload_to_s3", "success", now - timedelta(minutes=2)),
-            MockLog("upload_to_nextcloud", "success", now - timedelta(minutes=3)),
-            # Queue logs (older, should use upload_to_ as latest)
-            MockLog("queue_dropbox", "success", now - timedelta(minutes=4)),
-            MockLog("queue_s3", "in_progress", now - timedelta(minutes=5)),
-            MockLog("queue_nextcloud", "success", now - timedelta(minutes=6)),
-            # Even older in_progress logs
-            MockLog("upload_to_dropbox", "in_progress", now - timedelta(minutes=7)),
-            MockLog("upload_to_s3", "in_progress", now - timedelta(minutes=8)),
-        ]
 
-        summary = _compute_step_summary(logs)
+        summary = get_step_summary(db_session, file_record.id)
 
-        # Should count unique upload destinations
+        # Should count only upload_to_* steps (not queue_* steps)
-        # Note: queue_X and upload_to_X are separate steps
+        assert summary["total_upload_tasks"] == 3
-        assert summary["total_upload_tasks"] == 6  # 3 upload_to + 3 queue
-        assert summary["uploads"]["success"] == 5  # All uploads success, 2 queue success
-        assert summary["uploads"]["in_progress"] == 1  # 1 queue in_progress
-
-    def test_accurate_metrics_for_completed_file(self):
-        """
-        Test the scenario from the issue: File with 6 actual uploads
-        should show 6, not 12.
-        """
-        from datetime import datetime, timedelta
-
-        class MockLog:
-            def __init__(self, step_name, status, timestamp):
-                self.step_name = step_name
-                self.status = status
-                self.timestamp = timestamp
-
-        now = datetime.now()
-        # Simulate 6 successful uploads with their queue steps
-        logs = []
-        services = ["dropbox", "s3", "nextcloud", "google_drive", "onedrive", "webdav"]
-
-        # Add latest status (all success) - most recent
-        for i, service in enumerate(services):
-            logs.append(MockLog(f"upload_to_{service}", "success", now - timedelta(minutes=i * 2)))
-            logs.append(MockLog(f"queue_{service}", "success", now - timedelta(minutes=i * 2 + 1)))
-
-        # Add some older in_progress logs
-        base_offset = len(services) * 2
-        for i, service in enumerate(services):
-            logs.append(MockLog(f"upload_to_{service}", "in_progress", now - timedelta(minutes=base_offset + i * 2)))
-            logs.append(MockLog(f"queue_{service}", "in_progress", now - timedelta(minutes=base_offset + i * 2 + 1)))
-
-        summary = _compute_step_summary(logs)
-
-        # Should have 12 total upload tasks (6 upload_to + 6 queue)
-        assert summary["total_upload_tasks"] == 12
-        # All should be success (latest status)
-        assert summary["uploads"]["success"] == 12
-        assert summary["uploads"]["in_progress"] == 0
-
-    def test_mixed_upload_statuses(self):
-        """
-        Test that upload metrics correctly reflect mixed statuses.
-        """
-        from datetime import datetime, timedelta
-
-        class MockLog:
-            def __init__(self, step_name, status, timestamp):
-                self.step_name = step_name
-                self.status = status
-                self.timestamp = timestamp
-
-        now = datetime.now()
-        logs = [
-            # Latest statuses (ordered by timestamp desc)
-            MockLog("upload_to_dropbox", "success", now - timedelta(minutes=1)),
-            MockLog("upload_to_s3", "failure", now - timedelta(minutes=2)),
-            MockLog("upload_to_nextcloud", "in_progress", now - timedelta(minutes=3)),
-            MockLog("queue_dropbox", "success", now - timedelta(minutes=4)),
-            MockLog("queue_s3", "success", now - timedelta(minutes=5)),
-            MockLog("queue_nextcloud", "success", now - timedelta(minutes=6)),
-        ]
-
-        summary = _compute_step_summary(logs)
-
-        assert summary["total_upload_tasks"] == 6
-        assert summary["uploads"]["success"] == 4  # 1 upload + 3 queue
-        assert summary["uploads"]["failure"] == 1  # 1 upload
-        assert summary["uploads"]["in_progress"] == 1  # 1 upload
-
-    def test_order_independent_ascending(self):
-        """
-        Test that _compute_step_summary works correctly with ascending order logs
-        (as used in production by file_detail_page).
-
-        This test ensures the function correctly selects the latest status per step
-        based on timestamp, not position in the list.
-        """
-
-        class MockLog:
-            def __init__(self, step_name, status, timestamp):
-                self.step_name = step_name
-                self.status = status
-                self.timestamp = timestamp
-
-        now = datetime.now()
-        # Logs ordered by timestamp ASCENDING (oldest first) - like production
-        logs = [
-            # Older in_progress logs (should be ignored)
-            MockLog("hash_file", "in_progress", now - timedelta(minutes=7)),
-            MockLog("create_file_record", "in_progress", now - timedelta(minutes=6)),
-            MockLog("extract_metadata_with_gpt", "in_progress", now - timedelta(minutes=5)),
-            MockLog("upload_to_dropbox", "in_progress", now - timedelta(minutes=4)),
-            # Latest status for each step (all success) - at the end
-            MockLog("hash_file", "success", now - timedelta(minutes=3)),
-            MockLog("create_file_record", "success", now - timedelta(minutes=2)),
-            MockLog("extract_metadata_with_gpt", "success", now - timedelta(minutes=1)),
-            MockLog("upload_to_dropbox", "success", now),
-        ]
-
-        summary = _compute_step_summary(logs)
-
-        # Should use latest status (success) not first seen (in_progress)
-        assert summary["total_main_steps"] == 3
-        assert summary["main"]["success"] == 3
-        assert summary["main"]["in_progress"] == 0
-
-        assert summary["total_upload_tasks"] == 1
         assert summary["uploads"]["success"] == 1
-        assert summary["uploads"]["in_progress"] == 0
+        assert summary["uploads"]["failure"] == 1
+        assert summary["uploads"]["in_progress"] == 1
 
-    def test_order_independent_mixed(self):
+    def test_accurate_metrics_for_completed_file(self, db_session):
         """
-        Test that _compute_step_summary works correctly with randomly ordered logs.
+        Test metrics for a file with multiple successful uploads.
-
-        This ensures the function truly is order-independent.
         """
+        file_record = FileRecord(
+            filehash="test6",
+            original_filename="test6.pdf",
+            local_filename="/tmp/test6.pdf",
+            file_size=6144
+        )
+        db_session.add(file_record)
+        db_session.commit()
 
-        class MockLog:
+        initialize_file_steps(db_session, file_record.id)
-            def __init__(self, step_name, status, timestamp):
+        from app.utils.step_manager import add_upload_steps
-                self.step_name = step_name
+        services = ["dropbox", "s3", "nextcloud", "google_drive", "onedrive", "webdav"]
-                self.status = status
+        add_upload_steps(db_session, file_record.id, services)
-                self.timestamp = timestamp
 
-        now = datetime.now()
+        # Mark all uploads as success
-        # Logs in mixed order
+        for service in services:
-        logs = [
+            update_step_status(db_session, file_record.id, f"upload_to_{service}", "success")
-            MockLog("upload_to_s3", "in_progress", now - timedelta(minutes=8)),
-            MockLog("hash_file", "success", now - timedelta(minutes=1)),  # Latest for hash_file
-            MockLog("upload_to_dropbox", "in_progress", now - timedelta(minutes=7)),
-            MockLog("hash_file", "in_progress", now - timedelta(minutes=5)),  # Older, should be ignored
-            MockLog("upload_to_s3", "failure", now - timedelta(minutes=2)),  # Latest for S3
-            MockLog("create_file_record", "in_progress", now - timedelta(minutes=6)),
-            MockLog("upload_to_dropbox", "success", now),  # Latest for Dropbox
-            MockLog("create_file_record", "success", now - timedelta(minutes=3)),  # Latest for create
-        ]
 
-        summary = _compute_step_summary(logs)
+        summary = get_step_summary(db_session, file_record.id)
 
-        # Should correctly identify latest status for each step
+        # Should have 6 upload_to_* tasks
-        assert summary["total_main_steps"] == 2
+        assert summary["total_upload_tasks"] == 6
-        assert summary["main"]["success"] == 2  # hash_file and create_file_record
+        assert summary["uploads"]["success"] == 6
-        assert summary["main"]["in_progress"] == 0
+        assert summary["uploads"]["failure"] == 0
-
-        assert summary["total_upload_tasks"] == 2
-        assert summary["uploads"]["success"] == 1  # dropbox
-        assert summary["uploads"]["failure"] == 1  # s3
         assert summary["uploads"]["in_progress"] == 0

tests/test_step_manager.py

@@ -51,13 +51,13 @@ class TestFileProcessingStepModel:
         db_session.commit()
 
         # Create a processing step
-        step = FileProcessingStep(file_id=file_record.id, step_name="hash_file", status="success")
+        step = FileProcessingStep(file_id=file_record.id, step_name="create_file_record", status="success")
         db_session.add(step)
         db_session.commit()
 
         assert step.id is not None
         assert step.file_id == file_record.id
-        assert step.step_name == "hash_file"
+        assert step.step_name == "create_file_record"
         assert step.status == "success"
 
     def test_unique_constraint(self, db_session: Session):
@@ -70,12 +70,12 @@ class TestFileProcessingStepModel:
         db_session.commit()
 
         # Create first step
-        step1 = FileProcessingStep(file_id=file_record.id, step_name="hash_file", status="in_progress")
+        step1 = FileProcessingStep(file_id=file_record.id, step_name="create_file_record", status="in_progress")
         db_session.add(step1)
         db_session.commit()
 
         # Try to create duplicate step - should fail
-        step2 = FileProcessingStep(file_id=file_record.id, step_name="hash_file", status="success")
+        step2 = FileProcessingStep(file_id=file_record.id, step_name="create_file_record", status="success")
         db_session.add(step2)
 
         with pytest.raises(Exception):  # SQLAlchemy will raise an integrity error
@@ -145,12 +145,12 @@ class TestStepManager:
 
         # Update a step that doesn't exist yet
         now = datetime.now()
-        update_step_status(db_session, file_record.id, "hash_file", "in_progress", started_at=now)
+        update_step_status(db_session, file_record.id, "create_file_record", "in_progress", started_at=now)
 
         # Verify step was created
         step = (
             db_session.query(FileProcessingStep)
-            .filter(FileProcessingStep.file_id == file_record.id, FileProcessingStep.step_name == "hash_file")
+            .filter(FileProcessingStep.file_id == file_record.id, FileProcessingStep.step_name == "create_file_record")
             .first()
         )
 
@@ -172,13 +172,13 @@ class TestStepManager:
         # Update an existing step
         now = datetime.now()
         update_step_status(
-            db_session, file_record.id, "hash_file", "success", started_at=now - timedelta(seconds=5), completed_at=now
+            db_session, file_record.id, "create_file_record", "success", started_at=now - timedelta(seconds=5), completed_at=now
         )
 
         # Verify step was updated
         step = (
             db_session.query(FileProcessingStep)
-            .filter(FileProcessingStep.file_id == file_record.id, FileProcessingStep.step_name == "hash_file")
+            .filter(FileProcessingStep.file_id == file_record.id, FileProcessingStep.step_name == "create_file_record")
             .first()
         )
 
@@ -197,21 +197,21 @@ class TestStepManager:
 
         initialize_file_steps(db_session, file_record.id)
 
-        # Update some steps
+        # Update some steps - use actual step names from MAIN_PROCESSING_STEPS
         now = datetime.now()
-        update_step_status(db_session, file_record.id, "hash_file", "success", completed_at=now)
+        update_step_status(db_session, file_record.id, "create_file_record", "success", completed_at=now)
-        update_step_status(db_session, file_record.id, "create_file_record", "in_progress", started_at=now)
+        update_step_status(db_session, file_record.id, "check_text", "in_progress", started_at=now)
-        update_step_status(db_session, file_record.id, "check_text", "failure", error_message="Failed to check text")
+        update_step_status(db_session, file_record.id, "extract_text", "failure", error_message="Failed to extract text")
 
         # Get all step statuses
         status_map = get_file_step_status(db_session, file_record.id)
 
         assert len(status_map) == len(MAIN_PROCESSING_STEPS)
-        assert status_map["hash_file"]["status"] == "success"
+        assert status_map["create_file_record"]["status"] == "success"
-        assert status_map["hash_file"]["completed_at"] == now
+        assert status_map["create_file_record"]["completed_at"] == now
-        assert status_map["create_file_record"]["status"] == "in_progress"
+        assert status_map["check_text"]["status"] == "in_progress"
-        assert status_map["check_text"]["status"] == "failure"
+        assert status_map["extract_text"]["status"] == "failure"
-        assert status_map["check_text"]["error_message"] == "Failed to check text"
+        assert status_map["extract_text"]["error_message"] == "Failed to extract text"
 
     def test_get_file_overall_status_pending(self, db_session: Session):
         """Test overall status for a file with pending steps."""
@@ -242,9 +242,9 @@ class TestStepManager:
         initialize_file_steps(db_session, file_record.id)
 
         # Mark some steps as complete and one as in progress
-        update_step_status(db_session, file_record.id, "hash_file", "success")
         update_step_status(db_session, file_record.id, "create_file_record", "success")
-        update_step_status(db_session, file_record.id, "check_text", "in_progress")
+        update_step_status(db_session, file_record.id, "check_text", "success")
+        update_step_status(db_session, file_record.id, "extract_text", "in_progress")
 
         status = get_file_overall_status(db_session, file_record.id)
 
@@ -285,9 +285,9 @@ class TestStepManager:
         initialize_file_steps(db_session, file_record.id)
 
         # Mark some steps as success and one as failure
-        update_step_status(db_session, file_record.id, "hash_file", "success")
         update_step_status(db_session, file_record.id, "create_file_record", "success")
-        update_step_status(db_session, file_record.id, "check_text", "failure", error_message="OCR failed")
+        update_step_status(db_session, file_record.id, "check_text", "success")
+        update_step_status(db_session, file_record.id, "extract_text", "failure", error_message="OCR failed")
 
         status = get_file_overall_status(db_session, file_record.id)
 
@@ -308,12 +308,12 @@ class TestStepManager:
         add_upload_steps(db_session, file_record.id, ["dropbox", "s3", "nextcloud"])
 
         # Update statuses
-        update_step_status(db_session, file_record.id, "hash_file", "success")
         update_step_status(db_session, file_record.id, "create_file_record", "success")
-        update_step_status(db_session, file_record.id, "check_text", "in_progress")
+        update_step_status(db_session, file_record.id, "check_text", "success")
+        update_step_status(db_session, file_record.id, "extract_text", "in_progress")
         update_step_status(db_session, file_record.id, "upload_to_dropbox", "success")
         update_step_status(db_session, file_record.id, "upload_to_s3", "failure")
-        update_step_status(db_session, file_record.id, "queue_nextcloud", "in_progress")
+        update_step_status(db_session, file_record.id, "upload_to_nextcloud", "in_progress")
 
         summary = get_step_summary(db_session, file_record.id)
 
@@ -322,10 +322,10 @@ class TestStepManager:
         assert summary["main"]["in_progress"] == 1
         assert summary["main"]["queued"] >= 5  # Remaining pending steps
 
-        # Check upload counts
+        # Check upload counts (only upload_to_* steps, not queue_* steps)
         assert summary["uploads"]["success"] == 1
         assert summary["uploads"]["failure"] == 1
         assert summary["uploads"]["in_progress"] == 1
 
         assert summary["total_main_steps"] == len(MAIN_PROCESSING_STEPS)
-        assert summary["total_upload_tasks"] == 6  # 3 destinations x 2 steps each
+        assert summary["total_upload_tasks"] == 3  # 3 upload_to_* steps counted

tests/test_storage_reorganization.py

@@ -120,14 +120,73 @@ startxref
         """Test that reprocessing doesn't create a duplicate original"""
         from app.tasks.process_document import process_document
 
-        # Create test file and original
+        # Create test file and original with valid PDF content
+        pdf_content = b"""%PDF-1.4
+1 0 obj
+<<
+/Type /Catalog
+/Pages 2 0 R
+>>
+endobj
+2 0 obj
+<<
+/Type /Pages
+/Kids [3 0 R]
+/Count 1
+>>
+endobj
+3 0 obj
+<<
+/Type /Page
+/Parent 2 0 R
+/MediaBox [0 0 612 792]
+/Resources <<
+/Font <<
+/F1 <<
+/Type /Font
+/Subtype /Type1
+/BaseFont /Helvetica
+>>
+>>
+>>
+/Contents 4 0 R
+>>
+endobj
+4 0 obj
+<<
+/Length 44
+>>
+stream
+BT
+/F1 12 Tf
+100 700 Td
+(Test content) Tj
+ET
+endstream
+endobj
+xref
+0 5
+0000000000 65535 f
+0000000009 00000 n
+0000000058 00000 n
+0000000115 00000 n
+0000000306 00000 n
+trailer
+<<
+/Size 5
+/Root 1 0 R
+>>
+startxref
+399
+%%EOF
+"""
         test_pdf = tmp_path / "test.pdf"
-        test_pdf.write_bytes(b"%PDF-1.4\ntest")
+        test_pdf.write_bytes(pdf_content)
 
         original_dir = tmp_path / "original"
         original_dir.mkdir()
         original_file = original_dir / "existing-original.pdf"
-        original_file.write_bytes(b"%PDF-1.4\noriginal")
+        original_file.write_bytes(pdf_content)
         
         # Create existing file record
         file_record = FileRecord(

tests/test_url_upload.py

@@ -167,8 +167,23 @@ class TestURLUploadValidation:
 class TestURLUploadEndpoint:
     """Integration tests for URL upload endpoint"""
 
-    def test_process_url_requires_authentication(self, client, monkeypatch):
+    @patch("app.api.url_upload.requests.get")
+    @patch("app.api.url_upload.process_document")
+    def test_process_url_requires_authentication(self, mock_process_document, mock_requests_get, client, monkeypatch):
         """Test that endpoint requires authentication when auth is enabled"""
+        # Mock successful download to prevent actual HTTP requests
+        mock_response = Mock()
+        mock_response.status_code = 200
+        mock_response.headers = {"Content-Type": "application/pdf", "Content-Length": "1024"}
+        mock_response.iter_content = Mock(return_value=[b"PDF content"])
+        mock_response.raise_for_status = Mock()
+        mock_requests_get.return_value = mock_response
+
+        # Mock Celery task
+        mock_task = Mock()
+        mock_task.id = "test-task-id"
+        mock_process_document.delay.return_value = mock_task
+
         # Temporarily enable auth for this test
         monkeypatch.setenv("AUTH_ENABLED", "True")