Delete Document & Delete Source buttons now delete the underlying files

This commit is contained in:
Jim Lancaster
2026-08-04 18:34:35 -05:00
parent 271633d1d5
commit d9f5fbb1a4
7 changed files with 141 additions and 5 deletions
+16
View File
@@ -4,6 +4,7 @@ from dataclasses import dataclass
from datetime import UTC from datetime import UTC
from datetime import datetime from datetime import datetime
from pathlib import Path from pathlib import Path
import shutil
from uuid import UUID from uuid import UUID
from sqlalchemy.exc import IntegrityError from sqlalchemy.exc import IntegrityError
@@ -119,6 +120,7 @@ class DocumentService(ServiceBase):
async def delete_document(self, document: Document, *, session: AsyncSession | None = None) -> None: async def delete_document(self, document: Document, *, session: AsyncSession | None = None) -> None:
"""Delete a document from the database.""" """Delete a document from the database."""
document_id = document.id
async with self._session_scope(session) as _session: async with self._session_scope(session) as _session:
existing = await _session.get( existing = await _session.get(
Document, Document,
@@ -152,6 +154,20 @@ class DocumentService(ServiceBase):
await _session.delete(existing) await _session.delete(existing)
await self._finalize(session=_session, caller_session=session) 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: async def create_person(self, person: Person, *, session: AsyncSession | None = None) -> Person:
"""Create a new person in the database.""" """Create a new person in the database."""
async with self._session_scope(session) as _session: async with self._session_scope(session) as _session:
@@ -113,10 +113,13 @@ class TranscriptionService(ServiceBase):
async def delete_source(self, source: Source, *, session: AsyncSession | None = None) -> None: async def delete_source(self, source: Source, *, session: AsyncSession | None = None) -> None:
"""Delete a source page record.""" """Delete a source page record."""
source_file_path = source.file_path
async with self._session_scope(session) as _session: async with self._session_scope(session) as _session:
await _session.delete(source) await _session.delete(source)
await self._finalize(session=_session, caller_session=session) 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: async def delete_unlinked_source(self, *, source_id: UUID, session: AsyncSession | None = None) -> None:
"""Delete a source only when no JobSource links exist.""" """Delete a source only when no JobSource links exist."""
async with self._session_scope(session) as _session: async with self._session_scope(session) as _session:
@@ -141,9 +144,12 @@ class TranscriptionService(ServiceBase):
suggestion="Remove JobSource links first, then retry deletion.", suggestion="Remove JobSource links first, then retry deletion.",
) )
source_file_path = source.file_path
await _session.delete(source) await _session.delete(source)
await self._finalize(session=_session, caller_session=session) await self._finalize(session=_session, caller_session=session)
self._delete_source_file(source_file_path=source_file_path)
async def list_sources( async def list_sources(
self, self,
*, *,
@@ -263,9 +269,26 @@ class TranscriptionService(ServiceBase):
for job_source in matching_links: for job_source in matching_links:
await _session.delete(job_source) await _session.delete(job_source)
source_file_path = source.file_path
await _session.delete(source) await _session.delete(source)
await self._finalize(session=_session, caller_session=session) 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( async def list_job_sources(
self, self,
*, *,
@@ -23,6 +23,8 @@ class SourceTableRow:
upload_name: str upload_name: str
filename: str filename: str
document_id: UUID 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]]: 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, "upload_name": row.upload_name,
"filename": row.filename, "filename": row.filename,
"document_id": str(row.document_id), "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 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": "page_number", "label": "Page", "field": "page_number", "sortable": True},
{"name": "upload_name", "label": "Upload Title", "field": "upload_name", "sortable": True, "classes": "font-serif"}, {"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": "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"}, {"name": "document_id", "label": "Document ID", "field": "document_id", "sortable": True, "classes": "font-mono"},
], ],
default_sort_by="page_number", default_sort_by="page_number",
@@ -52,6 +52,7 @@ def register_page() -> None:
job_label = None job_label = None
back_path = None back_path = None
sources: list[Source] = [] sources: list[Source] = []
job_source_by_source_id: dict[UUID, JobSource] = {}
try: try:
if document_id is not None: if document_id is not None:
@@ -64,6 +65,10 @@ def register_page() -> None:
job_label = str(job.id) job_label = str(job.id)
back_path = f"/jobs/{job.id}" back_path = f"/jobs/{job.id}"
job_sources = await sources_service.list_job_sources(job_id=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 = [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())) sources.sort(key=lambda item: (item.page_number, item.upload_name.casefold()))
else: else:
@@ -103,6 +108,16 @@ def register_page() -> None:
upload_name=source.upload_name, upload_name=source.upload_name,
filename=source.filename, filename=source.filename,
document_id=source.document_id, 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 for source in sources
] ]
+36 -1
View File
@@ -2,6 +2,7 @@ from __future__ import annotations
from datetime import UTC from datetime import UTC
from datetime import datetime from datetime import datetime
from pathlib import Path
from uuid import uuid4 from uuid import uuid4
import pytest import pytest
@@ -87,8 +88,9 @@ async def test_delete_document_blocks_when_dependencies_exist(default_session_fa
@pytest.mark.asyncio @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 = DocumentService(session_factory=default_session_factory)
service.settings.upload_dir = tmp_path
document = await service.create_document( document = await service.create_document(
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) await service.delete_document(document)
assert not document_dir.exists()
with pytest.raises(DocumentError): with pytest.raises(DocumentError):
await service.read_document_detail(document.id) 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 @pytest.mark.asyncio
async def test_read_person_detail_loads_document_links(default_session_factory): async def test_read_person_detail_loads_document_links(default_session_factory):
service = DocumentService(session_factory=default_session_factory) service = DocumentService(session_factory=default_session_factory)
+16 -4
View File
@@ -93,10 +93,11 @@ class TestTranscriptionServiceRevisionUpsert:
assert revisions[0].revised_text == "Revision v2" assert revisions[0].revised_text == "Revision v2"
@pytest.mark.asyncio @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) documents = DocumentService(session_factory=default_session_factory)
jobs = JobService(session_factory=default_session_factory) jobs = JobService(session_factory=default_session_factory)
transcriptions = TranscriptionService(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") document = Document(id=uuid4(), name="delete-source-success")
await documents.create_document(document=document) await documents.create_document(document=document)
@@ -104,12 +105,16 @@ class TestTranscriptionServiceRevisionUpsert:
job = Job(document_id=document.id, status=JobStatus.QUEUED) job = Job(document_id=document.id, status=JobStatus.QUEUED)
await jobs.create_job(job=job) 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( source = Source(
document_id=document.id, document_id=document.id,
page_number=1, page_number=1,
upload_name="delete.jpg", upload_name="delete.jpg",
filename="delete.jpg", filename="delete.jpg",
file_path="uploads/delete.jpg", file_path=str(stored_path),
) )
async with transcriptions._session_scope() as session: async with transcriptions._session_scope() as session:
session.add(source) session.add(source)
@@ -122,6 +127,7 @@ class TestTranscriptionServiceRevisionUpsert:
with pytest.raises(TranscriptionNotFoundError): with pytest.raises(TranscriptionNotFoundError):
await transcriptions.read_source(source.id) await transcriptions.read_source(source.id)
assert not stored_path.exists()
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_delete_source_from_job_context_blocks_when_other_job_links_exist(self, default_session_factory): 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) await transcriptions.delete_source_from_job_context(job_id=job_one.id, source_id=source.id)
@pytest.mark.asyncio @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) documents = DocumentService(session_factory=default_session_factory)
transcriptions = TranscriptionService(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") document = Document(id=uuid4(), name="delete-unlinked-source")
await documents.create_document(document=document) 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( source = Source(
document_id=document.id, document_id=document.id,
page_number=1, page_number=1,
upload_name="orphan.jpg", upload_name="orphan.jpg",
filename="orphan.jpg", filename="orphan.jpg",
file_path="uploads/orphan.jpg", file_path=str(stored_path),
) )
await transcriptions.create_source(source=source) await transcriptions.create_source(source=source)
@@ -176,6 +187,7 @@ class TestTranscriptionServiceRevisionUpsert:
with pytest.raises(TranscriptionNotFoundError): with pytest.raises(TranscriptionNotFoundError):
await transcriptions.read_source(source.id) await transcriptions.read_source(source.id)
assert not stored_path.exists()
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_delete_unlinked_source_blocks_when_linked(self, default_session_factory): async def test_delete_unlinked_source_blocks_when_linked(self, default_session_factory):
+17
View File
@@ -105,6 +105,23 @@ class TestSourcesPageRendering:
assert "Sources for Job" in response.text assert "Sources for Job" in response.text
assert "Back to Job" in response.text assert "Back to Job" in response.text
assert "job-page.png" 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): def test_source_detail_page_renders_preview_and_revision_box(self, app_client, seed_job):
_, client = app_client _, client = app_client