From d9f5fbb1a4fb55b668efbf8e412a1b0266a028f6 Mon Sep 17 00:00:00 2001 From: Jim Lancaster <40281233+zoltan57@users.noreply.github.com> Date: Tue, 4 Aug 2026 18:34:35 -0500 Subject: [PATCH] Delete Document & Delete Source buttons now delete the underlying files --- src/transcription/services/documents.py | 16 ++++++++ src/transcription/services/transcription.py | 23 ++++++++++++ .../ui/components/table/sources.py | 18 +++++++++ src/transcription/ui/pages/sources_page.py | 15 ++++++++ tests/services/test_document_service.py | 37 ++++++++++++++++++- tests/services/test_transcription_service.py | 20 ++++++++-- tests/ui/test_sources_page.py | 17 +++++++++ 7 files changed, 141 insertions(+), 5 deletions(-) diff --git a/src/transcription/services/documents.py b/src/transcription/services/documents.py index 9a6f321..cbe56ce 100644 --- a/src/transcription/services/documents.py +++ b/src/transcription/services/documents.py @@ -4,6 +4,7 @@ from dataclasses import dataclass from datetime import UTC from datetime import datetime from pathlib import Path +import shutil from uuid import UUID from sqlalchemy.exc import IntegrityError @@ -119,6 +120,7 @@ class DocumentService(ServiceBase): async def delete_document(self, document: Document, *, session: AsyncSession | None = None) -> None: """Delete a document from the database.""" + document_id = document.id async with self._session_scope(session) as _session: existing = await _session.get( Document, @@ -152,6 +154,20 @@ class DocumentService(ServiceBase): await _session.delete(existing) await self._finalize(session=_session, caller_session=session) + self._delete_document_storage_folder(document_id=document_id) + + def _delete_document_storage_folder(self, *, document_id: UUID) -> None: + """Best-effort cleanup for document-scoped source storage.""" + document_dir = self.settings.upload_dir / "documents" / str(document_id) + if not document_dir.exists(): + return + + try: + shutil.rmtree(document_dir) + logger.info("Deleted document storage folder: %s", document_dir) + except OSError: + logger.warning("Failed to delete document storage folder: %s", document_dir) + async def create_person(self, person: Person, *, session: AsyncSession | None = None) -> Person: """Create a new person in the database.""" async with self._session_scope(session) as _session: diff --git a/src/transcription/services/transcription.py b/src/transcription/services/transcription.py index 5da5077..7612644 100644 --- a/src/transcription/services/transcription.py +++ b/src/transcription/services/transcription.py @@ -113,10 +113,13 @@ class TranscriptionService(ServiceBase): async def delete_source(self, source: Source, *, session: AsyncSession | None = None) -> None: """Delete a source page record.""" + source_file_path = source.file_path async with self._session_scope(session) as _session: await _session.delete(source) await self._finalize(session=_session, caller_session=session) + self._delete_source_file(source_file_path=source_file_path) + async def delete_unlinked_source(self, *, source_id: UUID, session: AsyncSession | None = None) -> None: """Delete a source only when no JobSource links exist.""" async with self._session_scope(session) as _session: @@ -141,9 +144,12 @@ class TranscriptionService(ServiceBase): suggestion="Remove JobSource links first, then retry deletion.", ) + source_file_path = source.file_path await _session.delete(source) await self._finalize(session=_session, caller_session=session) + self._delete_source_file(source_file_path=source_file_path) + async def list_sources( self, *, @@ -263,9 +269,26 @@ class TranscriptionService(ServiceBase): for job_source in matching_links: await _session.delete(job_source) + source_file_path = source.file_path await _session.delete(source) await self._finalize(session=_session, caller_session=session) + self._delete_source_file(source_file_path=source_file_path) + + def _delete_source_file(self, *, source_file_path: str) -> None: + """Best-effort cleanup for source media files.""" + candidate_path = Path(source_file_path) + resolved_path = candidate_path if candidate_path.is_absolute() else self.settings.upload_dir / candidate_path + + if not resolved_path.exists(): + return + + try: + resolved_path.unlink() + logger.info("Deleted source file: %s", resolved_path) + except OSError: + logger.warning("Failed to delete source file: %s", resolved_path) + async def list_job_sources( self, *, diff --git a/src/transcription/ui/components/table/sources.py b/src/transcription/ui/components/table/sources.py index 7d9751f..48d6fbc 100644 --- a/src/transcription/ui/components/table/sources.py +++ b/src/transcription/ui/components/table/sources.py @@ -23,6 +23,8 @@ class SourceTableRow: upload_name: str filename: str document_id: UUID + job_source_status: str | None = None + job_source_error_detail: str | None = None def _serialize_rows(rows: Sequence[SourceTableRow]) -> list[dict[str, Any]]: @@ -33,6 +35,8 @@ def _serialize_rows(rows: Sequence[SourceTableRow]) -> list[dict[str, Any]]: "upload_name": row.upload_name, "filename": row.filename, "document_id": str(row.document_id), + "job_source_status": row.job_source_status or "-", + "job_source_error_detail": row.job_source_error_detail or "-", } for row in rows ] @@ -51,6 +55,20 @@ def render_sources_table(rows: Sequence[SourceTableRow]) -> None: {"name": "page_number", "label": "Page", "field": "page_number", "sortable": True}, {"name": "upload_name", "label": "Upload Title", "field": "upload_name", "sortable": True, "classes": "font-serif"}, {"name": "filename", "label": "Stored Filename", "field": "filename", "sortable": True, "classes": "font-mono"}, + { + "name": "job_source_status", + "label": "Job Source Status", + "field": "job_source_status", + "sortable": True, + "classes": "font-mono", + }, + { + "name": "job_source_error_detail", + "label": "Job Source Error Detail", + "field": "job_source_error_detail", + "sortable": False, + "classes": "font-mono text-xs", + }, {"name": "document_id", "label": "Document ID", "field": "document_id", "sortable": True, "classes": "font-mono"}, ], default_sort_by="page_number", diff --git a/src/transcription/ui/pages/sources_page.py b/src/transcription/ui/pages/sources_page.py index 1fa58eb..9d98053 100644 --- a/src/transcription/ui/pages/sources_page.py +++ b/src/transcription/ui/pages/sources_page.py @@ -52,6 +52,7 @@ def register_page() -> None: job_label = None back_path = None sources: list[Source] = [] + job_source_by_source_id: dict[UUID, JobSource] = {} try: if document_id is not None: @@ -64,6 +65,10 @@ def register_page() -> None: job_label = str(job.id) back_path = f"/jobs/{job.id}" job_sources = await sources_service.list_job_sources(job_id=job.id) + job_source_by_source_id = { + job_source.source_id: job_source + for job_source in job_sources + } sources = [job_source.source for job_source in job_sources if job_source.source is not None] sources.sort(key=lambda item: (item.page_number, item.upload_name.casefold())) else: @@ -103,6 +108,16 @@ def register_page() -> None: upload_name=source.upload_name, filename=source.filename, document_id=source.document_id, + job_source_status=( + job_source_by_source_id[source.id].status.value + if source.id in job_source_by_source_id + else None + ), + job_source_error_detail=( + job_source_by_source_id[source.id].error_detail + if source.id in job_source_by_source_id + else None + ), ) for source in sources ] diff --git a/tests/services/test_document_service.py b/tests/services/test_document_service.py index ab5cb01..409eca9 100644 --- a/tests/services/test_document_service.py +++ b/tests/services/test_document_service.py @@ -2,6 +2,7 @@ from __future__ import annotations from datetime import UTC from datetime import datetime +from pathlib import Path from uuid import uuid4 import pytest @@ -87,8 +88,9 @@ async def test_delete_document_blocks_when_dependencies_exist(default_session_fa @pytest.mark.asyncio -async def test_delete_document_succeeds_when_unlinked(default_session_factory): +async def test_delete_document_succeeds_when_unlinked(default_session_factory, tmp_path): service = DocumentService(session_factory=default_session_factory) + service.settings.upload_dir = tmp_path document = await service.create_document( Document( @@ -98,12 +100,45 @@ async def test_delete_document_succeeds_when_unlinked(default_session_factory): ) ) + document_dir = service.settings.upload_dir / "documents" / str(document.id) + document_dir.mkdir(parents=True, exist_ok=True) + (document_dir / "leftover.txt").write_text("orphan", encoding="utf-8") + await service.delete_document(document) + assert not document_dir.exists() + with pytest.raises(DocumentError): await service.read_document_detail(document.id) +@pytest.mark.asyncio +async def test_delete_document_removes_populated_storage_tree(default_session_factory, tmp_path): + service = DocumentService(session_factory=default_session_factory) + service.settings.upload_dir = tmp_path + + document = await service.create_document( + Document( + id=uuid4(), + name="tree-delete", + document_type="memo", + ) + ) + + document_dir = service.settings.upload_dir / "documents" / str(document.id) + (document_dir / "page-1.jpg").parent.mkdir(parents=True, exist_ok=True) + (document_dir / "page-1.jpg").write_bytes(b"one") + (document_dir / "page-2.jpg").write_bytes(b"two") + (document_dir / "nested" / "manifest.json").parent.mkdir(parents=True, exist_ok=True) + (document_dir / "nested" / "manifest.json").write_text('{"ok": true}', encoding="utf-8") + + assert document_dir.exists() + + await service.delete_document(document) + + assert not document_dir.exists() + + @pytest.mark.asyncio async def test_read_person_detail_loads_document_links(default_session_factory): service = DocumentService(session_factory=default_session_factory) diff --git a/tests/services/test_transcription_service.py b/tests/services/test_transcription_service.py index b6cf929..51302b1 100644 --- a/tests/services/test_transcription_service.py +++ b/tests/services/test_transcription_service.py @@ -93,10 +93,11 @@ class TestTranscriptionServiceRevisionUpsert: assert revisions[0].revised_text == "Revision v2" @pytest.mark.asyncio - async def test_delete_source_from_job_context_removes_source_and_single_link(self, default_session_factory): + async def test_delete_source_from_job_context_removes_source_and_single_link(self, default_session_factory, tmp_path): documents = DocumentService(session_factory=default_session_factory) jobs = JobService(session_factory=default_session_factory) transcriptions = TranscriptionService(session_factory=default_session_factory) + transcriptions.settings.upload_dir = tmp_path document = Document(id=uuid4(), name="delete-source-success") await documents.create_document(document=document) @@ -104,12 +105,16 @@ class TestTranscriptionServiceRevisionUpsert: job = Job(document_id=document.id, status=JobStatus.QUEUED) await jobs.create_job(job=job) + stored_path = tmp_path / "documents" / str(document.id) / "delete.jpg" + stored_path.parent.mkdir(parents=True, exist_ok=True) + stored_path.write_bytes(b"data") + source = Source( document_id=document.id, page_number=1, upload_name="delete.jpg", filename="delete.jpg", - file_path="uploads/delete.jpg", + file_path=str(stored_path), ) async with transcriptions._session_scope() as session: session.add(source) @@ -122,6 +127,7 @@ class TestTranscriptionServiceRevisionUpsert: with pytest.raises(TranscriptionNotFoundError): await transcriptions.read_source(source.id) + assert not stored_path.exists() @pytest.mark.asyncio async def test_delete_source_from_job_context_blocks_when_other_job_links_exist(self, default_session_factory): @@ -156,19 +162,24 @@ class TestTranscriptionServiceRevisionUpsert: await transcriptions.delete_source_from_job_context(job_id=job_one.id, source_id=source.id) @pytest.mark.asyncio - async def test_delete_unlinked_source_succeeds(self, default_session_factory): + async def test_delete_unlinked_source_succeeds(self, default_session_factory, tmp_path): documents = DocumentService(session_factory=default_session_factory) transcriptions = TranscriptionService(session_factory=default_session_factory) + transcriptions.settings.upload_dir = tmp_path document = Document(id=uuid4(), name="delete-unlinked-source") await documents.create_document(document=document) + stored_path = tmp_path / "documents" / str(document.id) / "orphan.jpg" + stored_path.parent.mkdir(parents=True, exist_ok=True) + stored_path.write_bytes(b"data") + source = Source( document_id=document.id, page_number=1, upload_name="orphan.jpg", filename="orphan.jpg", - file_path="uploads/orphan.jpg", + file_path=str(stored_path), ) await transcriptions.create_source(source=source) @@ -176,6 +187,7 @@ class TestTranscriptionServiceRevisionUpsert: with pytest.raises(TranscriptionNotFoundError): await transcriptions.read_source(source.id) + assert not stored_path.exists() @pytest.mark.asyncio async def test_delete_unlinked_source_blocks_when_linked(self, default_session_factory): diff --git a/tests/ui/test_sources_page.py b/tests/ui/test_sources_page.py index c552390..c17d4ed 100644 --- a/tests/ui/test_sources_page.py +++ b/tests/ui/test_sources_page.py @@ -105,6 +105,23 @@ class TestSourcesPageRendering: assert "Sources for Job" in response.text assert "Back to Job" in response.text assert "job-page.png" in response.text + assert "Job Source Status" in response.text + + def test_sources_page_job_context_shows_job_source_status_and_error_detail(self, app_client, seed_job): + _, client = app_client + job_id = seed_job( + filename="job-failed-page.png", + status=JobStatus.FAILED, + transcription_text=None, + error_detail="Provider timed out", + ) + + response = client.get(f"/ui/sources?job_id={job_id}") + + assert response.status_code == 200 + assert "job-failed-page.png" in response.text + assert "failed" in response.text.lower() + assert "Provider timed out" in response.text def test_source_detail_page_renders_preview_and_revision_box(self, app_client, seed_job): _, client = app_client