From bbd7286c0bc0ff2dc1fd910335bf2964e4952e4d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 7 Feb 2026 15:08:08 +0000 Subject: [PATCH 1/4] Initial plan From 530c63ad14628c99606119b842081e232d7ecda3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 7 Feb 2026 15:12:49 +0000 Subject: [PATCH 2/4] Add server-side pagination, filtering, sorting, and file detail view - Updated /api/files endpoint with pagination, filtering, and sorting support - Added /api/files/{file_id} endpoint for detailed file information - Updated /files view to support server-side operations - Added /files/{file_id}/detail route for file detail page - Created new files.html with filters, status column, and pagination - Created file_detail.html for viewing processing history - Status computed from ProcessingLog entries (pending, processing, completed, failed) Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- app/api/files.py | 216 +++++++++++-- app/views/files.py | 154 ++++++++- frontend/templates/file_detail.html | 310 ++++++++++++++++++ frontend/templates/files.html | 473 ++++++++++++++++++---------- 4 files changed, 959 insertions(+), 194 deletions(-) create mode 100644 frontend/templates/file_detail.html diff --git a/app/api/files.py b/app/api/files.py index 17315ffa..e8cd14b8 100644 --- a/app/api/files.py +++ b/app/api/files.py @@ -1,15 +1,17 @@ """ File-related API endpoints """ -from fastapi import APIRouter, Request, HTTPException, Depends, UploadFile, File +from fastapi import APIRouter, Request, HTTPException, Depends, UploadFile, File, Query from sqlalchemy.orm import Session +from sqlalchemy import desc, asc, or_, func +from typing import Optional, List import logging import os import uuid import mimetypes from app.auth import require_login -from app.models import FileRecord +from app.models import FileRecord, ProcessingLog from app.config import settings from app.api.common import get_db from app.tasks.process_document import process_document @@ -22,29 +24,79 @@ router = APIRouter() @router.get("/files") @require_login -def list_files_api(request: Request, db: Session = Depends(get_db)): +def list_files_api( + request: Request, + db: Session = Depends(get_db), + page: int = Query(1, ge=1, description="Page number"), + per_page: int = Query(50, ge=1, le=200, description="Items per page"), + sort_by: str = Query("created_at", description="Sort field: id, original_filename, file_size, mime_type, created_at, status"), + sort_order: str = Query("desc", description="Sort order: asc or desc"), + search: Optional[str] = Query(None, description="Search in filename"), + mime_type: Optional[str] = Query(None, description="Filter by MIME type"), + status: Optional[str] = Query(None, description="Filter by processing status") +): """ - Returns a JSON list of all FileRecord entries. - Protected by `@require_login`, so only logged-in sessions can access. + Returns a paginated JSON list of FileRecord entries with processing status. + Supports server-side sorting, filtering, and searching. + + Query Parameters: + - page: Page number (default: 1) + - per_page: Items per page (default: 50, max: 200) + - sort_by: Field to sort by (default: created_at) + - sort_order: asc or desc (default: desc) + - search: Search in filename + - mime_type: Filter by MIME type + - status: Filter by processing status (pending, processing, completed, failed) Example response: - [ - { - "id": 123, - "filehash": "abc123...", - "original_filename": "example.pdf", - "local_filename": "/workdir/tmp/.pdf", - "file_size": 1048576, - "mime_type": "application/pdf", - "created_at": "2025-05-01T12:34:56.789000" - }, - ... - ] + { + "files": [...], + "pagination": { + "page": 1, + "per_page": 50, + "total_items": 150, + "total_pages": 3 + } + } """ - files = db.query(FileRecord).order_by(FileRecord.created_at.desc()).all() - # Return a simple list of dicts + # Start with base query + query = db.query(FileRecord) + + # Apply search filter + if search: + query = query.filter(FileRecord.original_filename.ilike(f"%{search}%")) + + # Apply MIME type filter + if mime_type: + query = query.filter(FileRecord.mime_type == mime_type) + + # Get total count before pagination + total_items = query.count() + + # Apply sorting + sort_column = { + "id": FileRecord.id, + "original_filename": FileRecord.original_filename, + "file_size": FileRecord.file_size, + "mime_type": FileRecord.mime_type, + "created_at": FileRecord.created_at + }.get(sort_by, FileRecord.created_at) + + if sort_order == "asc": + query = query.order_by(asc(sort_column)) + else: + query = query.order_by(desc(sort_column)) + + # Apply pagination + offset = (page - 1) * per_page + files = query.offset(offset).limit(per_page).all() + + # Build result with processing status result = [] for f in files: + # Get processing status for this file + processing_status = _get_file_processing_status(db, f.id) + result.append({ "id": f.id, "filehash": f.filehash, @@ -52,9 +104,131 @@ def list_files_api(request: Request, db: Session = Depends(get_db)): "local_filename": f.local_filename, "file_size": f.file_size, "mime_type": f.mime_type, - "created_at": f.created_at.isoformat() if f.created_at else None + "created_at": f.created_at.isoformat() if f.created_at else None, + "processing_status": processing_status }) - return result + + # Filter by status if requested (after computing statuses) + if status: + result = [f for f in result if f["processing_status"]["status"] == status] + total_items = len(result) # Update total for filtered results + + # Calculate pagination info + total_pages = (total_items + per_page - 1) // per_page + + return { + "files": result, + "pagination": { + "page": page, + "per_page": per_page, + "total_items": total_items, + "total_pages": total_pages + } + } + + +def _get_file_processing_status(db: Session, file_id: int) -> dict: + """ + Get the processing status for a file by checking its processing logs. + + Returns: + dict with status, last_step, and has_errors + """ + # Get all logs for this file + logs = db.query(ProcessingLog).filter( + ProcessingLog.file_id == file_id + ).order_by(ProcessingLog.timestamp.desc()).all() + + if not logs: + return { + "status": "pending", + "last_step": None, + "has_errors": False, + "total_steps": 0 + } + + # Check for failures + has_errors = any(log.status == "failure" for log in logs) + + # Check if any in progress + in_progress = any(log.status == "in_progress" for log in logs) + + # Get the latest log + latest_log = logs[0] + + # Determine overall status + if has_errors: + status = "failed" + elif in_progress: + status = "processing" + elif latest_log.status == "success": + status = "completed" + else: + status = "pending" + + return { + "status": status, + "last_step": latest_log.step_name, + "has_errors": has_errors, + "total_steps": len(logs) + } + + + +@router.get("/files/{file_id}") +@require_login +def get_file_details(request: Request, file_id: int, db: Session = Depends(get_db)): + """ + Get detailed information about a specific file including processing history. + """ + # Find the file record + file_record = db.query(FileRecord).filter(FileRecord.id == file_id).first() + + if not file_record: + raise HTTPException( + status_code=404, + detail=f"File record with ID {file_id} not found" + ) + + # Get processing logs + logs = db.query(ProcessingLog).filter( + ProcessingLog.file_id == file_id + ).order_by(ProcessingLog.timestamp.desc()).all() + + # Build log list + log_list = [] + for log in logs: + log_list.append({ + "id": log.id, + "task_id": log.task_id, + "step_name": log.step_name, + "status": log.status, + "message": log.message, + "timestamp": log.timestamp.isoformat() if log.timestamp else None + }) + + # Get processing status + processing_status = _get_file_processing_status(db, file_id) + + # Check if files exist on disk + files_on_disk = { + "original": os.path.exists(file_record.local_filename) if file_record.local_filename else False + } + + return { + "file": { + "id": file_record.id, + "filehash": file_record.filehash, + "original_filename": file_record.original_filename, + "local_filename": file_record.local_filename, + "file_size": file_record.file_size, + "mime_type": file_record.mime_type, + "created_at": file_record.created_at.isoformat() if file_record.created_at else None + }, + "processing_status": processing_status, + "logs": log_list, + "files_on_disk": files_on_disk + } @router.delete("/files/{file_id}") @require_login diff --git a/app/views/files.py b/app/views/files.py index 96fd7fae..dccf2110 100644 --- a/app/views/files.py +++ b/app/views/files.py @@ -1,8 +1,9 @@ """ File management views for displaying and managing files. """ -from fastapi import Request, Depends +from fastapi import Request, Depends, Query from sqlalchemy.orm import Session +from typing import Optional from app.views.base import APIRouter, templates, require_login, get_db, logger @@ -10,23 +11,113 @@ router = APIRouter() @router.get("/files") @require_login -def files_page(request: Request, db: Session = Depends(get_db)): +def files_page( + request: Request, + db: Session = Depends(get_db), + page: int = Query(1, ge=1), + per_page: int = Query(50, ge=1, le=200), + sort_by: str = Query("created_at"), + sort_order: str = Query("desc"), + search: Optional[str] = Query(None), + mime_type: Optional[str] = Query(None), + status: Optional[str] = Query(None) +): """ - Return the 'files.html' template with files from the database + Return the 'files.html' template with server-side pagination, sorting, and filtering """ try: # Import the model here to avoid circular imports - from app.models import FileRecord + from app.models import FileRecord, ProcessingLog + from sqlalchemy import desc, asc - # Fetch all files from the database - files = db.query(FileRecord).order_by(FileRecord.created_at.desc()).all() + # Start with base query + query = db.query(FileRecord) + + # Apply search filter + if search: + query = query.filter(FileRecord.original_filename.ilike(f"%{search}%")) + + # Apply MIME type filter + if mime_type: + query = query.filter(FileRecord.mime_type == mime_type) + + # Get total count before pagination + total_items = query.count() + + # Apply sorting + sort_column = { + "id": FileRecord.id, + "original_filename": FileRecord.original_filename, + "file_size": FileRecord.file_size, + "mime_type": FileRecord.mime_type, + "created_at": FileRecord.created_at + }.get(sort_by, FileRecord.created_at) + + if sort_order == "asc": + query = query.order_by(asc(sort_column)) + else: + query = query.order_by(desc(sort_column)) + + # Apply pagination + offset = (page - 1) * per_page + files = query.offset(offset).limit(per_page).all() + + # Add processing status to each file + files_with_status = [] + for file in files: + # Get processing logs for this file + logs = db.query(ProcessingLog).filter( + ProcessingLog.file_id == file.id + ).order_by(ProcessingLog.timestamp.desc()).all() + + # Determine status + if not logs: + processing_status = "pending" + else: + has_errors = any(log.status == "failure" for log in logs) + in_progress = any(log.status == "in_progress" for log in logs) + latest_log = logs[0] + + if has_errors: + processing_status = "failed" + elif in_progress: + processing_status = "processing" + elif latest_log.status == "success": + processing_status = "completed" + else: + processing_status = "pending" + + # Add status to file object as an attribute + file.processing_status = processing_status + files_with_status.append(file) + + # Calculate pagination info + total_pages = (total_items + per_page - 1) // per_page + + # Get unique MIME types for filter dropdown + mime_types = db.query(FileRecord.mime_type).distinct().filter( + FileRecord.mime_type.isnot(None) + ).all() + mime_types = [mt[0] for mt in mime_types if mt[0]] # Debug output - logger.info(f"Retrieved {len(files)} files from database") + logger.info(f"Retrieved {len(files_with_status)} files from database (page {page}/{total_pages})") return templates.TemplateResponse("files.html", { "request": request, - "files": files + "files": files_with_status, + "pagination": { + "page": page, + "per_page": per_page, + "total_items": total_items, + "total_pages": total_pages + }, + "sort_by": sort_by, + "sort_order": sort_order, + "search": search or "", + "mime_type": mime_type or "", + "status": status or "", + "mime_types": mime_types }) except Exception as e: # Log any errors @@ -35,5 +126,52 @@ def files_page(request: Request, db: Session = Depends(get_db)): return templates.TemplateResponse("files.html", { "request": request, "files": [], + "pagination": { + "page": 1, + "per_page": per_page, + "total_items": 0, + "total_pages": 0 + }, + "error": str(e) + }) + + +@router.get("/files/{file_id}/detail") +@require_login +def file_detail_page(request: Request, file_id: int, db: Session = Depends(get_db)): + """ + Return the file detail page showing processing history and file information + """ + try: + from app.models import FileRecord, ProcessingLog + import os + + # Find the file record + file_record = db.query(FileRecord).filter(FileRecord.id == file_id).first() + + if not file_record: + return templates.TemplateResponse("file_detail.html", { + "request": request, + "error": f"File with ID {file_id} not found" + }) + + # Get processing logs + logs = db.query(ProcessingLog).filter( + ProcessingLog.file_id == file_id + ).order_by(ProcessingLog.timestamp.asc()).all() + + # Check if file exists on disk + file_exists = os.path.exists(file_record.local_filename) if file_record.local_filename else False + + return templates.TemplateResponse("file_detail.html", { + "request": request, + "file": file_record, + "logs": logs, + "file_exists": file_exists + }) + except Exception as e: + logger.error(f"Error retrieving file details: {str(e)}") + return templates.TemplateResponse("file_detail.html", { + "request": request, "error": str(e) }) diff --git a/frontend/templates/file_detail.html b/frontend/templates/file_detail.html new file mode 100644 index 00000000..96153a5f --- /dev/null +++ b/frontend/templates/file_detail.html @@ -0,0 +1,310 @@ +{% extends "base.html" %} +{% block title %}File Details{% endblock %} + +{% block head_extra %} + +{% endblock %} + +{% block content %} +
+ + + Back to File List + + + {% if error %} +
+

Error: {{ error }}

+
+ {% else %} + + +
+

File Information

+
+
+ File ID + {{ file.id }} +
+
+ Original Filename + {{ file.original_filename }} +
+
+ File Hash + {{ file.filehash }} +
+
+ File Size + {{ (file.file_size / 1024) | round(2) }} KB ({{ file.file_size }} bytes) +
+
+ MIME Type + {{ file.mime_type or 'N/A' }} +
+
+ Created At + {{ file.created_at.strftime('%Y-%m-%d %H:%M:%S') if file.created_at else 'N/A' }} +
+
+ Local Path + {{ file.local_filename }} +
+
+ File on Disk + + {% if file_exists %} + + File exists + + {% else %} + + File not found + + {% endif %} + +
+
+
+ + +
+

Processing History

+ {% if logs %} +
+ {% for log in logs %} +
+
+
+
+ {{ log.step_name }} - {{ log.status }} +
+ {% if log.message %} +
{{ log.message }}
+ {% endif %} +
+ {{ log.timestamp.strftime('%Y-%m-%d %H:%M:%S') if log.timestamp else 'N/A' }} + {% if log.task_id %} + Task: {{ log.task_id[:16] }}... + {% endif %} +
+
+
+ {% endfor %} +
+ {% else %} +
+ +

No processing logs found for this file.

+

The file may not have been processed yet.

+
+ {% endif %} +
+ + {% endif %} +
+{% endblock %} diff --git a/frontend/templates/files.html b/frontend/templates/files.html index c421fcc3..1174cc24 100644 --- a/frontend/templates/files.html +++ b/frontend/templates/files.html @@ -3,10 +3,6 @@ {% block head_extra %} - - - - {% endblock %} @@ -182,34 +251,130 @@ {% endif %} + +
+
+
+ + +
+ +
+ + +
+ +
+ + +
+ +
+ + +
+ +
+ + +
+ + + + + +
+
- - - - - - + + + + + + + {% for file in files %} - + - + - + + {% else %} - + {% endfor %}
IDOriginal FilenameFile SizeMime TypeCreated AtActions + ID + + {% if sort_by == 'id' %} + {% if sort_order == 'asc' %}▲{% else %}▼{% endif %} + {% else %} + ⬍ + {% endif %} + + + Original Filename + + {% if sort_by == 'original_filename' %} + {% if sort_order == 'asc' %}▲{% else %}▼{% endif %} + {% else %} + ⬍ + {% endif %} + + + File Size + + {% if sort_by == 'file_size' %} + {% if sort_order == 'asc' %}▲{% else %}▼{% endif %} + {% else %} + ⬍ + {% endif %} + + + MIME Type + + {% if sort_by == 'mime_type' %} + {% if sort_order == 'asc' %}▲{% else %}▼{% endif %} + {% else %} + ⬍ + {% endif %} + + Status + Created At + + {% if sort_by == 'created_at' %} + {% if sort_order == 'asc' %}▲{% else %}▼{% endif %} + {% else %} + ⬍ + {% endif %} + + Actions
{{ file.id }} {{ file.original_filename }}{{ (file.file_size / 1024) | round(2) }} KB{{ (file.file_size / 1024) | round(2) }} KB {{ file.mime_type }}{{ file.created_at }} + + {{ file.processing_status | title }} + + {{ file.created_at.strftime('%Y-%m-%d %H:%M:%S') if file.created_at else 'N/A' }}
- -
@@ -217,13 +382,41 @@
No files foundNo files found
+ + {% if pagination.total_pages > 1 %} + + {% endif %} + - - - - {% endblock %} From b76fb147cb6a05a6b8f3d9f8357dbba578a5a6d4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 7 Feb 2026 15:15:30 +0000 Subject: [PATCH 3/4] Add tests for file listing and pagination features Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- tests/test_file_listing.py | 304 +++++++++++++++++++++++++++++++++++++ 1 file changed, 304 insertions(+) create mode 100644 tests/test_file_listing.py diff --git a/tests/test_file_listing.py b/tests/test_file_listing.py new file mode 100644 index 00000000..8f9d48b4 --- /dev/null +++ b/tests/test_file_listing.py @@ -0,0 +1,304 @@ +""" +Tests for file listing, pagination, filtering, and detail endpoints. +""" +import pytest +from fastapi.testclient import TestClient +from app.models import FileRecord, ProcessingLog +from datetime import datetime + + +@pytest.mark.integration +@pytest.mark.requires_db +class TestFileListingPagination: + """Tests for file listing with pagination, sorting, and filtering.""" + + def test_list_files_empty_with_pagination(self, client: TestClient): + """Test listing files when database is empty returns pagination structure.""" + response = client.get("/api/files") + assert response.status_code == 200 + data = response.json() + assert "files" in data + assert "pagination" in data + assert isinstance(data["files"], list) + assert len(data["files"]) == 0 + assert data["pagination"]["total_items"] == 0 + + def test_list_files_with_data(self, client: TestClient, db_session): + """Test listing files with sample data.""" + # Create sample files + for i in range(5): + file_record = FileRecord( + filehash=f"hash{i}", + original_filename=f"test{i}.pdf", + local_filename=f"/tmp/test{i}.pdf", + file_size=1024 * (i + 1), + mime_type="application/pdf" + ) + db_session.add(file_record) + db_session.commit() + + response = client.get("/api/files") + assert response.status_code == 200 + data = response.json() + assert len(data["files"]) == 5 + assert data["pagination"]["total_items"] == 5 + assert data["pagination"]["page"] == 1 + + def test_pagination_works(self, client: TestClient, db_session): + """Test that pagination correctly limits results.""" + # Create 10 sample files + for i in range(10): + file_record = FileRecord( + filehash=f"hash{i}", + original_filename=f"test{i}.pdf", + local_filename=f"/tmp/test{i}.pdf", + file_size=1024, + mime_type="application/pdf" + ) + db_session.add(file_record) + db_session.commit() + + # Get first page with 5 items per page + response = client.get("/api/files?page=1&per_page=5") + assert response.status_code == 200 + data = response.json() + assert len(data["files"]) == 5 + assert data["pagination"]["page"] == 1 + assert data["pagination"]["total_pages"] == 2 + + # Get second page + response = client.get("/api/files?page=2&per_page=5") + assert response.status_code == 200 + data = response.json() + assert len(data["files"]) == 5 + assert data["pagination"]["page"] == 2 + + def test_sorting_by_filename(self, client: TestClient, db_session): + """Test sorting files by filename.""" + # Create files with different names + for name in ["zebra.pdf", "apple.pdf", "middle.pdf"]: + file_record = FileRecord( + filehash=name, + original_filename=name, + local_filename=f"/tmp/{name}", + file_size=1024, + mime_type="application/pdf" + ) + db_session.add(file_record) + db_session.commit() + + # Sort ascending + response = client.get("/api/files?sort_by=original_filename&sort_order=asc") + assert response.status_code == 200 + data = response.json() + filenames = [f["original_filename"] for f in data["files"]] + assert filenames == ["apple.pdf", "middle.pdf", "zebra.pdf"] + + # Sort descending + response = client.get("/api/files?sort_by=original_filename&sort_order=desc") + assert response.status_code == 200 + data = response.json() + filenames = [f["original_filename"] for f in data["files"]] + assert filenames == ["zebra.pdf", "middle.pdf", "apple.pdf"] + + def test_sorting_by_file_size(self, client: TestClient, db_session): + """Test sorting files by size.""" + # Create files with different sizes + for i, size in enumerate([5000, 1000, 3000]): + file_record = FileRecord( + filehash=f"hash{i}", + original_filename=f"file{i}.pdf", + local_filename=f"/tmp/file{i}.pdf", + file_size=size, + mime_type="application/pdf" + ) + db_session.add(file_record) + db_session.commit() + + # Sort by size ascending + response = client.get("/api/files?sort_by=file_size&sort_order=asc") + assert response.status_code == 200 + data = response.json() + sizes = [f["file_size"] for f in data["files"]] + assert sizes == [1000, 3000, 5000] + + def test_search_filter(self, client: TestClient, db_session): + """Test searching files by filename.""" + # Create files with different names + for name in ["invoice_2024.pdf", "receipt_2024.pdf", "report.pdf"]: + file_record = FileRecord( + filehash=name, + original_filename=name, + local_filename=f"/tmp/{name}", + file_size=1024, + mime_type="application/pdf" + ) + db_session.add(file_record) + db_session.commit() + + # Search for "2024" + response = client.get("/api/files?search=2024") + assert response.status_code == 200 + data = response.json() + assert len(data["files"]) == 2 + filenames = [f["original_filename"] for f in data["files"]] + assert "invoice_2024.pdf" in filenames + assert "receipt_2024.pdf" in filenames + assert "report.pdf" not in filenames + + def test_mime_type_filter(self, client: TestClient, db_session): + """Test filtering files by MIME type.""" + # Create files with different MIME types + files_data = [ + ("file1.pdf", "application/pdf"), + ("file2.txt", "text/plain"), + ("file3.pdf", "application/pdf"), + ] + for filename, mime_type in files_data: + file_record = FileRecord( + filehash=filename, + original_filename=filename, + local_filename=f"/tmp/{filename}", + file_size=1024, + mime_type=mime_type + ) + db_session.add(file_record) + db_session.commit() + + # Filter by PDF mime type + response = client.get("/api/files?mime_type=application/pdf") + assert response.status_code == 200 + data = response.json() + assert len(data["files"]) == 2 + for file in data["files"]: + assert file["mime_type"] == "application/pdf" + + def test_processing_status_included(self, client: TestClient, db_session): + """Test that processing status is included in file listing.""" + # Create a file + file_record = FileRecord( + filehash="test_hash", + original_filename="test.pdf", + local_filename="/tmp/test.pdf", + file_size=1024, + mime_type="application/pdf" + ) + db_session.add(file_record) + db_session.commit() + + # Add a processing log + log = ProcessingLog( + file_id=file_record.id, + task_id="test_task", + step_name="test_step", + status="success", + message="Test message" + ) + db_session.add(log) + db_session.commit() + + response = client.get("/api/files") + assert response.status_code == 200 + data = response.json() + assert len(data["files"]) == 1 + assert "processing_status" in data["files"][0] + assert data["files"][0]["processing_status"]["status"] == "completed" + + +@pytest.mark.integration +@pytest.mark.requires_db +class TestFileDetailEndpoint: + """Tests for file detail endpoint.""" + + def test_get_file_detail_success(self, client: TestClient, db_session): + """Test getting details for an existing file.""" + # Create a file + file_record = FileRecord( + filehash="test_hash", + original_filename="test.pdf", + local_filename="/tmp/test.pdf", + file_size=1024, + mime_type="application/pdf" + ) + db_session.add(file_record) + db_session.commit() + + # Add processing logs + for i, status in enumerate(["success", "in_progress", "success"]): + log = ProcessingLog( + file_id=file_record.id, + task_id=f"task_{i}", + step_name=f"step_{i}", + status=status, + message=f"Message {i}" + ) + db_session.add(log) + db_session.commit() + + response = client.get(f"/api/files/{file_record.id}") + assert response.status_code == 200 + data = response.json() + + # Check file details + assert "file" in data + assert data["file"]["id"] == file_record.id + assert data["file"]["original_filename"] == "test.pdf" + + # Check processing status + assert "processing_status" in data + assert data["processing_status"]["status"] in ["completed", "processing"] + + # Check logs + assert "logs" in data + assert len(data["logs"]) == 3 + + def test_get_nonexistent_file_detail(self, client: TestClient): + """Test getting details for a non-existent file returns 404.""" + response = client.get("/api/files/99999") + assert response.status_code == 404 + + def test_file_detail_status_determination(self, client: TestClient, db_session): + """Test that processing status is correctly determined.""" + # Create a file + file_record = FileRecord( + filehash="test_hash", + original_filename="test.pdf", + local_filename="/tmp/test.pdf", + file_size=1024, + mime_type="application/pdf" + ) + db_session.add(file_record) + db_session.commit() + + # Test 1: No logs = pending + response = client.get(f"/api/files/{file_record.id}") + assert response.status_code == 200 + assert response.json()["processing_status"]["status"] == "pending" + + # Test 2: Success log = completed + log = ProcessingLog( + file_id=file_record.id, + task_id="task_1", + step_name="step_1", + status="success" + ) + db_session.add(log) + db_session.commit() + + response = client.get(f"/api/files/{file_record.id}") + assert response.status_code == 200 + assert response.json()["processing_status"]["status"] == "completed" + + # Test 3: Failure log = failed + log2 = ProcessingLog( + file_id=file_record.id, + task_id="task_2", + step_name="step_2", + status="failure" + ) + db_session.add(log2) + db_session.commit() + + response = client.get(f"/api/files/{file_record.id}") + assert response.status_code == 200 + assert response.json()["processing_status"]["status"] == "failed" From 775ffa5082e80f7224a41e024653fee65e59c18b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 7 Feb 2026 15:20:59 +0000 Subject: [PATCH 4/4] Address code review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fixed status filtering to occur before pagination for correct counts - Resolved N+1 query problem by batch-fetching processing statuses - Extracted status computation logic to shared utility function - Changed sort indicator from ⬍ to ↕ for better browser compatibility - Updated both API and view layers to use shared status utilities Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- app/api/files.py | 103 +++++++++++++++++----------------- app/utils/file_status.py | 102 +++++++++++++++++++++++++++++++++ app/views/files.py | 32 +++-------- frontend/templates/files.html | 10 ++-- 4 files changed, 165 insertions(+), 82 deletions(-) create mode 100644 app/utils/file_status.py diff --git a/app/api/files.py b/app/api/files.py index e8cd14b8..865250a0 100644 --- a/app/api/files.py +++ b/app/api/files.py @@ -16,6 +16,7 @@ from app.config import settings from app.api.common import get_db from app.tasks.process_document import process_document from app.tasks.convert_to_pdf import convert_to_pdf +from app.utils.file_status import get_file_processing_status, get_files_processing_status # Set up logging logger = logging.getLogger(__name__) @@ -70,7 +71,42 @@ def list_files_api( if mime_type: query = query.filter(FileRecord.mime_type == mime_type) - # Get total count before pagination + # Apply status filter (before pagination for correct counts) + if status: + # Subquery to get file IDs matching the status + if status == "pending": + # Files with no logs + subq = db.query(ProcessingLog.file_id).distinct() + query = query.filter(~FileRecord.id.in_(subq)) + elif status == "processing": + # Files with in_progress logs + subq = db.query(ProcessingLog.file_id).filter( + ProcessingLog.status == "in_progress" + ).distinct() + query = query.filter(FileRecord.id.in_(subq)) + elif status == "failed": + # Files with failure logs + subq = db.query(ProcessingLog.file_id).filter( + ProcessingLog.status == "failure" + ).distinct() + query = query.filter(FileRecord.id.in_(subq)) + elif status == "completed": + # Files with success logs but no failures or in_progress + success_files = db.query(ProcessingLog.file_id).filter( + ProcessingLog.status == "success" + ).distinct().subquery() + + failed_files = db.query(ProcessingLog.file_id).filter( + or_(ProcessingLog.status == "failure", ProcessingLog.status == "in_progress") + ).distinct().subquery() + + query = query.filter( + FileRecord.id.in_(db.query(success_files.c.file_id)) + ).filter( + ~FileRecord.id.in_(db.query(failed_files.c.file_id)) + ) + + # Get total count before pagination (after all filters) total_items = query.count() # Apply sorting @@ -91,12 +127,13 @@ def list_files_api( offset = (page - 1) * per_page files = query.offset(offset).limit(per_page).all() + # Get processing status for all files efficiently + file_ids = [f.id for f in files] + statuses = get_files_processing_status(db, file_ids) + # Build result with processing status result = [] for f in files: - # Get processing status for this file - processing_status = _get_file_processing_status(db, f.id) - result.append({ "id": f.id, "filehash": f.filehash, @@ -105,14 +142,14 @@ def list_files_api( "file_size": f.file_size, "mime_type": f.mime_type, "created_at": f.created_at.isoformat() if f.created_at else None, - "processing_status": processing_status + "processing_status": statuses.get(f.id, { + "status": "pending", + "last_step": None, + "has_errors": False, + "total_steps": 0 + }) }) - # Filter by status if requested (after computing statuses) - if status: - result = [f for f in result if f["processing_status"]["status"] == status] - total_items = len(result) # Update total for filtered results - # Calculate pagination info total_pages = (total_items + per_page - 1) // per_page @@ -129,49 +166,11 @@ def list_files_api( def _get_file_processing_status(db: Session, file_id: int) -> dict: """ - Get the processing status for a file by checking its processing logs. - - Returns: - dict with status, last_step, and has_errors + Deprecated: Use app.utils.file_status.get_file_processing_status instead. + Kept for backward compatibility with file detail endpoint. """ - # Get all logs for this file - logs = db.query(ProcessingLog).filter( - ProcessingLog.file_id == file_id - ).order_by(ProcessingLog.timestamp.desc()).all() - - if not logs: - return { - "status": "pending", - "last_step": None, - "has_errors": False, - "total_steps": 0 - } - - # Check for failures - has_errors = any(log.status == "failure" for log in logs) - - # Check if any in progress - in_progress = any(log.status == "in_progress" for log in logs) - - # Get the latest log - latest_log = logs[0] - - # Determine overall status - if has_errors: - status = "failed" - elif in_progress: - status = "processing" - elif latest_log.status == "success": - status = "completed" - else: - status = "pending" - - return { - "status": status, - "last_step": latest_log.step_name, - "has_errors": has_errors, - "total_steps": len(logs) - } + from app.utils.file_status import get_file_processing_status + return get_file_processing_status(db, file_id) diff --git a/app/utils/file_status.py b/app/utils/file_status.py new file mode 100644 index 00000000..627533fd --- /dev/null +++ b/app/utils/file_status.py @@ -0,0 +1,102 @@ +""" +Utility functions for file processing status determination. +""" +from typing import Dict, List +from sqlalchemy.orm import Session +from app.models import ProcessingLog + + +def get_file_processing_status(db: Session, file_id: int) -> Dict: + """ + Get the processing status for a file by checking its processing logs. + + Args: + db: Database session + file_id: ID of the file + + Returns: + dict with status, last_step, and has_errors + """ + # Get all logs for this file + logs = db.query(ProcessingLog).filter( + ProcessingLog.file_id == file_id + ).order_by(ProcessingLog.timestamp.desc()).all() + + return _compute_status_from_logs(logs) + + +def get_files_processing_status(db: Session, file_ids: List[int]) -> Dict[int, Dict]: + """ + Get processing status for multiple files efficiently. + + Args: + db: Database session + file_ids: List of file IDs + + Returns: + dict mapping file_id to status dict + """ + # Get all logs for these files in one query + logs = db.query(ProcessingLog).filter( + ProcessingLog.file_id.in_(file_ids) + ).order_by(ProcessingLog.file_id, ProcessingLog.timestamp.desc()).all() + + # Group logs by file_id + logs_by_file = {} + for log in logs: + if log.file_id not in logs_by_file: + logs_by_file[log.file_id] = [] + logs_by_file[log.file_id].append(log) + + # Compute status for each file + result = {} + for file_id in file_ids: + file_logs = logs_by_file.get(file_id, []) + result[file_id] = _compute_status_from_logs(file_logs) + + return result + + +def _compute_status_from_logs(logs: List[ProcessingLog]) -> Dict: + """ + Compute processing status from a list of processing logs. + + Args: + logs: List of ProcessingLog objects (should be ordered by timestamp desc) + + Returns: + dict with status, last_step, has_errors, and total_steps + """ + if not logs: + return { + "status": "pending", + "last_step": None, + "has_errors": False, + "total_steps": 0 + } + + # Check for failures + has_errors = any(log.status == "failure" for log in logs) + + # Check if any in progress + in_progress = any(log.status == "in_progress" for log in logs) + + # Get the latest log + latest_log = logs[0] + + # Determine overall status + if has_errors: + status = "failed" + elif in_progress: + status = "processing" + elif latest_log.status == "success": + status = "completed" + else: + status = "pending" + + return { + "status": status, + "last_step": latest_log.step_name, + "has_errors": has_errors, + "total_steps": len(logs) + } diff --git a/app/views/files.py b/app/views/files.py index dccf2110..8da5c545 100644 --- a/app/views/files.py +++ b/app/views/files.py @@ -6,6 +6,7 @@ from sqlalchemy.orm import Session from typing import Optional from app.views.base import APIRouter, templates, require_login, get_db, logger +from app.utils.file_status import get_files_processing_status router = APIRouter() @@ -62,33 +63,14 @@ def files_page( offset = (page - 1) * per_page files = query.offset(offset).limit(per_page).all() - # Add processing status to each file + # Get processing status for all files efficiently (avoids N+1) + file_ids = [f.id for f in files] + statuses = get_files_processing_status(db, file_ids) + + # Add status to each file files_with_status = [] for file in files: - # Get processing logs for this file - logs = db.query(ProcessingLog).filter( - ProcessingLog.file_id == file.id - ).order_by(ProcessingLog.timestamp.desc()).all() - - # Determine status - if not logs: - processing_status = "pending" - else: - has_errors = any(log.status == "failure" for log in logs) - in_progress = any(log.status == "in_progress" for log in logs) - latest_log = logs[0] - - if has_errors: - processing_status = "failed" - elif in_progress: - processing_status = "processing" - elif latest_log.status == "success": - processing_status = "completed" - else: - processing_status = "pending" - - # Add status to file object as an attribute - file.processing_status = processing_status + file.processing_status = statuses.get(file.id, {}).get("status", "pending") files_with_status.append(file) # Calculate pagination info diff --git a/frontend/templates/files.html b/frontend/templates/files.html index 1174cc24..5fcfcc22 100644 --- a/frontend/templates/files.html +++ b/frontend/templates/files.html @@ -308,7 +308,7 @@ {% if sort_by == 'id' %} {% if sort_order == 'asc' %}▲{% else %}▼{% endif %} {% else %} - ⬍ + ↕ {% endif %} @@ -318,7 +318,7 @@ {% if sort_by == 'original_filename' %} {% if sort_order == 'asc' %}▲{% else %}▼{% endif %} {% else %} - ⬍ + ↕ {% endif %} @@ -328,7 +328,7 @@ {% if sort_by == 'file_size' %} {% if sort_order == 'asc' %}▲{% else %}▼{% endif %} {% else %} - ⬍ + ↕ {% endif %} @@ -338,7 +338,7 @@ {% if sort_by == 'mime_type' %} {% if sort_order == 'asc' %}▲{% else %}▼{% endif %} {% else %} - ⬍ + ↕ {% endif %} @@ -349,7 +349,7 @@ {% if sort_by == 'created_at' %} {% if sort_order == 'asc' %}▲{% else %}▼{% endif %} {% else %} - ⬍ + ↕ {% endif %}