From 0a461343e7c9ee86aa122ac53d1dd17b24f4deee Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Feb 2026 08:14:09 +0000 Subject: [PATCH] fix: Add service containers and fix test failures - Add Redis and RabbitMQ services to CI workflow - Fix Jinja2 template error by passing file=None in error cases - Fix test expecting dict response format for list_files endpoint - Fix NOT NULL constraint by providing valid local_filename - Fix retry-subtask to validate subtask name before checking processed file - Add mock for process_document in reprocess test Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- .github/workflows/tests.yaml | 20 ++++++++++++ app/api/files.py | 48 ++++++++++++++--------------- app/views/files.py | 2 ++ frontend/templates/file_detail.html | 2 ++ tests/test_api.py | 7 +++-- tests/test_bulk_operations.py | 4 +-- tests/test_file_detail_endpoints.py | 9 +++++- 7 files changed, 62 insertions(+), 30 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 284c66f7..0817a6eb 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -5,6 +5,26 @@ on: [push, pull_request] jobs: test: runs-on: ubuntu-latest + services: + redis: + image: redis:7 + ports: + - 6379:6379 + options: >- + --health-cmd "redis-cli ping" + --health-interval 10s + --health-timeout 5s + --health-retries 5 + rabbitmq: + image: rabbitmq:3-management + ports: + - 5672:5672 + - 15672:15672 + options: >- + --health-cmd "rabbitmq-diagnostics -q ping" + --health-interval 10s + --health-timeout 5s + --health-retries 5 steps: - name: Checkout Code uses: actions/checkout@v3 diff --git a/app/api/files.py b/app/api/files.py index 0b7ce49c..44b6cbf0 100644 --- a/app/api/files.py +++ b/app/api/files.py @@ -437,30 +437,6 @@ def retry_subtask( if not file_record: raise HTTPException(status_code=404, detail=f"File with ID {file_id} not found") - # Check for processed file (upload tasks work with processed files) - workdir = settings.workdir - processed_dir = os.path.join(workdir, "processed") - - # Try to find the processed file - base_filename = os.path.splitext(file_record.original_filename)[0] - potential_paths = [ - os.path.join(processed_dir, f"{file_record.filehash}.pdf"), - os.path.join(processed_dir, f"{base_filename}_processed.pdf"), - os.path.join(processed_dir, file_record.original_filename), - ] - - file_path = None - for path in potential_paths: - if os.path.exists(path): - file_path = path - break - - if not file_path: - raise HTTPException( - status_code=400, - detail="Processed file not found. Cannot retry upload." - ) - # Map subtask names to their corresponding Celery tasks from app.tasks.upload_to_dropbox import upload_to_dropbox from app.tasks.upload_to_nextcloud import upload_to_nextcloud @@ -492,6 +468,30 @@ def retry_subtask( detail=f"Invalid subtask name: {subtask_name}. Must be one of: {', '.join(task_map.keys())}" ) + # Check for processed file (upload tasks work with processed files) + workdir = settings.workdir + processed_dir = os.path.join(workdir, "processed") + + # Try to find the processed file + base_filename = os.path.splitext(file_record.original_filename)[0] + potential_paths = [ + os.path.join(processed_dir, f"{file_record.filehash}.pdf"), + os.path.join(processed_dir, f"{base_filename}_processed.pdf"), + os.path.join(processed_dir, file_record.original_filename), + ] + + file_path = None + for path in potential_paths: + if os.path.exists(path): + file_path = path + break + + if not file_path: + raise HTTPException( + status_code=400, + detail="Processed file not found. Cannot retry upload." + ) + # Queue the specific upload task upload_task = task_map[subtask_name] task = upload_task.delay(file_path, file_id) diff --git a/app/views/files.py b/app/views/files.py index 5bc059d0..3f571730 100644 --- a/app/views/files.py +++ b/app/views/files.py @@ -170,6 +170,7 @@ def file_detail_page(request: Request, file_id: int, db: Session = Depends(get_d if not file_record: return templates.TemplateResponse("file_detail.html", { "request": request, + "file": None, "error": f"File with ID {file_id} not found" }) @@ -216,6 +217,7 @@ def file_detail_page(request: Request, file_id: int, db: Session = Depends(get_d logger.error(f"Error retrieving file details: {str(e)}") return templates.TemplateResponse("file_detail.html", { "request": request, + "file": None, "error": str(e) }) diff --git a/frontend/templates/file_detail.html b/frontend/templates/file_detail.html index 138f2595..3996422f 100644 --- a/frontend/templates/file_detail.html +++ b/frontend/templates/file_detail.html @@ -466,6 +466,7 @@ cursor: not-allowed; } + {% if file %} + {% endif %} {% endblock %} {% block content %} diff --git a/tests/test_api.py b/tests/test_api.py index 85ee78a5..13d110b5 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -68,8 +68,11 @@ class TestFileEndpoints: response = client.get("/api/files") assert response.status_code == 200 data = response.json() - assert isinstance(data, list) - assert len(data) == 0 + assert isinstance(data, dict) + assert "files" in data + assert "pagination" in data + assert isinstance(data["files"], list) + assert len(data["files"]) == 0 def test_get_nonexistent_file(self, client: TestClient): """Test getting a file that doesn't exist.""" diff --git a/tests/test_bulk_operations.py b/tests/test_bulk_operations.py index d59315d2..0602673a 100644 --- a/tests/test_bulk_operations.py +++ b/tests/test_bulk_operations.py @@ -147,9 +147,7 @@ class TestBulkOperations: file_record = FileRecord( filehash=f"hash{i}", original_filename=f"test{i}.pdf", - local_filename=( - f"/tmp/test{i}.pdf" if i == 0 else None - ), # Second file has no local file + local_filename=f"/tmp/test{i}.pdf", # Both files have local_filename file_size=1024, mime_type="application/pdf", ) diff --git a/tests/test_file_detail_endpoints.py b/tests/test_file_detail_endpoints.py index 8ab51719..756180a8 100644 --- a/tests/test_file_detail_endpoints.py +++ b/tests/test_file_detail_endpoints.py @@ -3,6 +3,7 @@ Tests for file detail view improvements including reprocessing and preview endpo """ import os import pytest +from unittest.mock import patch, MagicMock from fastapi.testclient import TestClient from app.models import FileRecord, ProcessingLog @@ -11,8 +12,14 @@ from app.models import FileRecord, ProcessingLog class TestFileReprocessing: """Tests for single file reprocessing endpoint.""" - def test_reprocess_existing_file(self, client: TestClient, db_session, sample_pdf_path): + @patch("app.api.files.process_document") + def test_reprocess_existing_file(self, mock_process_document, client: TestClient, db_session, sample_pdf_path): """Test reprocessing an existing file.""" + # Setup mock + mock_task = MagicMock() + mock_task.id = "test-task-123" + mock_process_document.delay.return_value = mock_task + # Create a file record file_record = FileRecord( filehash="abc123",