diff --git a/.env.example b/.env.example index a8e78f6..1bbc274 100644 --- a/.env.example +++ b/.env.example @@ -52,7 +52,6 @@ SQLITE_CHECK_SAME_THREAD=false # --- filesystem paths --- UPLOAD_DIR=./data PROMPT_DIR=./prompts -HOMEPAGE_DIR=./data/homepage DATABASE_BACKUP_DIR=./data/backups # --- worker reliability --- diff --git a/docs/data_migration.md b/docs/data_migration.md index 25bfd7b..794c8dc 100644 --- a/docs/data_migration.md +++ b/docs/data_migration.md @@ -33,7 +33,7 @@ uv run python tools/export_import_migration.py migrate --bundle-dir .migration-b ## What gets migrated -- Tables (in dependency order): `document_type`, `person_role`, `document`, `person`, `document_person`, `job`, `source`, `job_source`, `execution_attempt`. +- Tables (in dependency order): `document_type`, `person_role`, `tag`, `document`, `person`, `photo`, `document_person`, `document_tag`, `job`, `source`, `job_source`, `execution_attempt`. - Media tree under `UPLOAD_DIR`. The bundle contains: @@ -42,7 +42,12 @@ The bundle contains: Path normalization during export/import: - `source.file_path` is normalized to `documents/...` (upload-root-relative POSIX). -- `person.portrait_path` is normalized to `persons/...` (upload-root-relative POSIX). +- `photo.path` is normalized to `photos/...` (upload-root-relative POSIX). + +Legacy V4.x portrait/homepage backfill in the export step: +- If the source DB has no `photo` table, the exporter synthesizes `photo` rows from legacy `person.portrait_path` values and from legacy homepage image files under `UPLOAD_DIR/homepage`. +- Legacy portrait and homepage image files are copied into the unified `UPLOAD_DIR/photos/{photo_id}{suffix}` layout in the migration bundle. +- Legacy homepage markdown is relocated from `UPLOAD_DIR/homepage/homepage.md` to `UPLOAD_DIR/homepage.md`. ## Cutover diff --git a/docs/roadmap_plan.md b/docs/roadmap_plan.md index 078cd6b..8671b85 100644 --- a/docs/roadmap_plan.md +++ b/docs/roadmap_plan.md @@ -114,28 +114,26 @@ Deferred out of this release: ## V5.0 — Unified Photos Table (major data-model change) -Per your feedback, this replaces the earlier "Track B" idea with a single -shared model: a **`photos` table** holding an image reference plus a text -`description` field, used by **both** the homepage gallery and Person -portraits — rather than building two separate, one-off metadata layers. +V5.0 standardizes homepage images and Person portraits into one shared `photo` +table and one storage layout. -Rough shape (subject to the further discussion you flagged before any -implementation): -- `photos`: id, image path/reference, description, and whatever - owner/context linkage is needed (e.g. a polymorphic or nullable - `person_id` plus a `context`/`kind` discriminator such as `"homepage"` vs - `"portrait"` — exact shape is an open design question, not decided here). -- Person gets **multiple portraits** instead of the current single - `portrait_path` string column — needs a migration to move existing - single-portrait data into the new table. -- Homepage images gain **descriptions**, using the same table instead of a - separate sidecar/caption mechanism. -- Because this is a genuine shared-schema decision (one table, two - consumers, plus a migration of existing single-portrait data), this is - correctly a 5.x "major change" rather than a small add-on — **flagged for - a follow-up design discussion before implementation starts**, specifically - on the exact `photos` table shape and how "which photos belong to which - person/context" should be modeled. +Finalized shape: +- `photo`: `id`, nullable `person_id`, `path`, `description`, `is_primary`, + timestamps. +- `person_id IS NULL` = homepage photos; non-null = Person photos. +- `is_primary` is the featured/first photo for that owner (homepage or Person). +- No separate context enum; ownership is derived from `person_id`. +- All image files are stored under `UPLOAD_DIR/photos/{photo_id}{suffix}`. +- `Person.portrait_path` is removed. +- `HOMEPAGE_DIR` is retired; homepage markdown remains file-backed at + `UPLOAD_DIR/homepage.md`. + +Migration policy for legacy installs: +- Export/import rebuild remains the migration mechanism. +- Legacy `person.portrait_path` values are backfilled into `photo` rows. +- Legacy homepage images under `UPLOAD_DIR/homepage` are backfilled into + homepage `photo` rows. +- Legacy homepage markdown is relocated to `UPLOAD_DIR/homepage.md`. --- diff --git a/docs/schema.md b/docs/schema.md index 3ed7db2..963b0aa 100644 --- a/docs/schema.md +++ b/docs/schema.md @@ -7,10 +7,10 @@ This document is the field-accurate Version 4 schema contract aligned to `src/tr - `src/transcription/db/models.py:60-78` (status and purpose enums) - `src/transcription/db/models.py:80-120` (`DocumentType`, `PersonRole`) - `src/transcription/db/models.py:122-172` (`Tag`, `Document`) -- `src/transcription/db/models.py:175-258` (`Person`, `DocumentPerson`, `DocumentTag`) -- `src/transcription/db/models.py:260-322` (`Job`) -- `src/transcription/db/models.py:325-437` (`Source`, `JobSource`) -- `src/transcription/db/models.py:440-497` (`ExecutionAttempt`) +- `src/transcription/db/models.py:175-281` (`Person`, `Photo`, `DocumentPerson`, `DocumentTag`) +- `src/transcription/db/models.py:285-347` (`Job`) +- `src/transcription/db/models.py:350-462` (`Source`, `JobSource`) +- `src/transcription/db/models.py:465-522` (`ExecutionAttempt`) ## Entity Relationship Overview @@ -22,6 +22,7 @@ erDiagram Document ||--o{ DocumentPerson : links Document ||--o{ DocumentTag : tagged Person ||--o{ DocumentPerson : links + Person ||--o{ Photo : owns PersonRole ||--o{ DocumentPerson : labels Tag ||--o{ DocumentTag : labels Job ||--o{ JobSource : includes @@ -119,12 +120,23 @@ erDiagram | `death_date_raw` | `str \| None` | optional | | `death_place` | `str \| None` | optional | | `biography` | `str \| None` | optional | -| `portrait_path` | `str \| None` | optional upload-root-relative POSIX path (`persons/...`) | | `family_search_id` | `str \| None` | nullable unique | | `metadata_` | `dict[str, JsonValue] \| None` | stored as DB column `metadata` (`JSONBCompat`) | | `created_at` | `datetime` | default now | | `updated_at` | `datetime` | default now, onupdate | +### `Photo` + +| Field | Type | Notes | +| :--- | :--- | :--- | +| `id` | `UUID` | PK | +| `person_id` | `UUID \| None` | nullable FK -> `person.id`, indexed (`NULL` = homepage photo) | +| `path` | `str` | required upload-root-relative POSIX path (`photos/...`) | +| `description` | `str \| None` | optional | +| `is_primary` | `bool` | default `False`; owner-level "featured/primary" marker | +| `created_at` | `datetime` | default now | +| `updated_at` | `datetime` | default now, onupdate | + ### `DocumentPerson` | Field | Type | Notes | diff --git a/docs/ui/pages/home.md b/docs/ui/pages/home.md index 7ac5351..178a868 100644 --- a/docs/ui/pages/home.md +++ b/docs/ui/pages/home.md @@ -2,21 +2,22 @@ ## Purpose -Home provides a user-maintained landing page for the local archive. It combines one current image with Markdown text and lets the operator edit both without changing application source or prompt assets. +Home provides a user-maintained landing page for the local archive. It combines a database-backed image gallery with Markdown text and lets the operator edit both without changing application source or prompt assets. ## Routes | Route | Browser path | Purpose | | --- | --- | --- | -| `/homepage` | `/ui/homepage` | View current homepage image and Markdown. | -| `/homepage/edit` | `/ui/homepage/edit` | Upload an image and edit Markdown. | +| `/homepage` | `/ui/homepage` | View homepage gallery and Markdown. | +| `/homepage/edit` | `/ui/homepage/edit` | Upload images, manage image metadata, and edit Markdown. | The application root and `/ui` redirect to `/ui/homepage`. ## View Behavior - The visible page heading is **Home**; the browser tab title is **VibeScribe Home**. -- The latest homepage image appears in the shared dark-room viewer. +- The featured homepage image (`photo.is_primary`) is shown first; remaining images are shown in random order. +- The current image appears in the shared dark-room viewer with its description. - Saved Markdown is rendered in the **Home Text** card. - Missing text displays `No homepage text saved yet.` - Missing image displays the viewer's empty state. @@ -25,26 +26,26 @@ The application root and `/ui` redirect to `/ui/homepage`. ## Edit Behavior -- The image upload accepts JPEG, PNG, GIF, WebP, BMP, and TIFF files. -- A successful upload immediately stores the file, updates the preview to that image, and displays a positive notification. +- The image upload accepts JPEG, PNG, GIF, WebP, BMP, and TIFF files and supports multi-file uploads. +- A successful upload immediately stores files in the shared `photo` table/media layout and displays a positive notification. +- The editor supports per-image description edits, setting a featured image, and deleting the current image. - The Markdown textarea is initialized from the currently stored homepage text. - **Save** writes the textarea content, displays `Homepage saved`, and returns to Home. - **Cancel** returns to Home without saving textarea changes. An image already uploaded during the edit session remains stored. ## Storage Contract -- Homepage content is mutable application data under `data/homepage`. -- Markdown is stored in `homepage.md`. -- Uploaded images keep a sanitized basename. -- The view selects the supported image with the most recent modification time. -- Homepage files are not transcription prompts and are not database records. +- Homepage markdown text is mutable application data at `UPLOAD_DIR/homepage.md`. +- Homepage images are stored as `photo` rows (`person_id = NULL`) with files under `UPLOAD_DIR/photos/`. +- Uploaded images are renamed to `{photo_id}{suffix}`. +- Homepage images are database records; markdown remains file-backed. ## Acceptance Checklist - `/`, `/ui`, and the application brand reach Home. - Home renders with or without stored Markdown and image content. - Edit loads existing Markdown. -- A supported image upload updates the preview and becomes the latest homepage image. +- Supported image upload stores one or more images and makes the first image featured when no featured image exists yet. - Save persists Markdown and returns to Home. - Cancel does not save changed Markdown. @@ -59,6 +60,5 @@ The application root and `/ui` redirect to `/ui/homepage`. ## Known Limitations -- Homepage storage location is configured by application settings and must remain writable in the active runtime environment. +- Homepage markdown storage location is `UPLOAD_DIR/homepage.md` and must remain writable in the active runtime environment. - Uploading an image is immediate and is not rolled back by Cancel. -- The editor does not currently delete or select among previously uploaded images. diff --git a/docs/ui/pages/people.md b/docs/ui/pages/people.md index 22b4c2b..bdd59e0 100644 --- a/docs/ui/pages/people.md +++ b/docs/ui/pages/people.md @@ -2,7 +2,7 @@ ## Purpose -People manages reusable historical-person records. A Person may appear in many Documents under different relationship roles and may optionally carry a portrait and FamilySearch identifier. +People manages reusable historical-person records. A Person may appear in many Documents under different relationship roles and may optionally carry one or more photos plus a FamilySearch identifier. ## Routes @@ -38,7 +38,6 @@ Optional: - Exact and approximate birth/death dates. - Birth/death places. - Biography. -- Portrait path or uploaded portrait. - FamilySearch ID. Rules: @@ -46,7 +45,7 @@ Rules: - Missing Full name blocks save with a warning. - Exact date inputs are native browser date inputs. - FamilySearch IDs are normalized and validated by `PeopleService`. -- Portrait uploads are stored under the configured upload root in a Person-specific directory and update Portrait path. +- Photos are managed on Person Detail (not in create/edit form fields). - Metadata JSON remains hidden. - Save success returns to Person Detail. @@ -54,7 +53,8 @@ Rules: - The header provides **New Document**, **Edit Person**, and **Delete**. - **New Document** opens Document creation with this Person requested for author preselection. -- The portrait viewer resolves supported relative upload paths and absolute HTTP/data URLs. +- Person Detail includes a photo gallery card with multi-file upload, per-photo description edits, set-primary, and delete. +- Primary photo is shown first and labeled as the primary portrait. - Biographical Record shows names, compact birth/death dates, and places. - Birth and death place values are clickable links to Google Maps when present. - FamilySearch ID is shown as a metadata value and is clickable to the FamilySearch person details route when present. @@ -67,6 +67,7 @@ Rules: ## Delete Behavior - The page warns when linked Document relationships exist. +- Delete is blocked when related Photos exist. - Confirmed deletion removes the Person and its relationship links; it does not delete Documents. - Success returns to the People list. - Missing or already-deleted records return to a safe list state. @@ -76,7 +77,7 @@ Rules: - List fields, alignment, date fallback, search, sorting, and navigation match this contract. - Full name is enforced on create and edit. - FamilySearch ID validation and link generation use the fixed supported identifier format. -- Portrait upload and rendering remain constrained to supported media paths. +- Photo upload and rendering remain constrained to supported media paths. - New Document carries the Person context. - Linked Documents show the correct role and target. - Delete wording distinguishes removal of relationship links from deletion of Documents. diff --git a/src/transcription/config.py b/src/transcription/config.py index 658f9e1..edfd334 100644 --- a/src/transcription/config.py +++ b/src/transcription/config.py @@ -107,7 +107,6 @@ class Settings(BaseSettings): # --- filesystem paths --- upload_dir: Path = Path("./data") prompt_dir: Path = Path("./prompts") - homepage_dir: Path = Path("./data/homepage") database_backup_dir: Path = Path("./data/backups") # --- worker reliability --- diff --git a/src/transcription/db/migration.py b/src/transcription/db/migration.py index e1364ea..d32b0b8 100644 --- a/src/transcription/db/migration.py +++ b/src/transcription/db/migration.py @@ -9,6 +9,7 @@ from datetime import datetime from pathlib import Path from typing import Any from uuid import UUID +from uuid import uuid4 from sqlalchemy import URL from sqlalchemy import MetaData @@ -32,6 +33,7 @@ EXPORT_TABLE_ORDER = ( "tag", "document", "person", + "photo", "document_person", "document_tag", "job", @@ -65,9 +67,12 @@ def export_bundle(*, source_db_url: str, source_upload_dir: Path, bundle_dir: Pa } engine = create_engine(source_db_url) + legacy_portrait_rows: list[dict[str, Any]] = [] + source_has_photo_table = False try: inspector = sqlalchemy_inspect(engine) source_tables = set(inspector.get_table_names()) + source_has_photo_table = "photo" in source_tables metadata = MetaData() metadata.reflect(bind=engine) current_metadata = SQLModel.metadata @@ -83,6 +88,12 @@ def export_bundle(*, source_db_url: str, source_upload_dir: Path, bundle_dir: Pa export_columns = [ column.name for column in target_table.columns if column.name in source_table.columns ] + if table_name == "person" and "portrait_path" in source_table.columns: + legacy_portrait_rows = connection.execute( + select(source_table.c["id"], source_table.c["portrait_path"]).where( + source_table.c["portrait_path"].is_not(None) + ) + ).mappings().all() rows = connection.execute(select(*(source_table.c[name] for name in export_columns))).mappings().all() payload["tables"][table_name] = [ _serialize_row(row, table_name=table_name, source_upload_dir=source_upload_dir) for row in rows @@ -90,8 +101,6 @@ def export_bundle(*, source_db_url: str, source_upload_dir: Path, bundle_dir: Pa finally: engine.dispose() - export_json.write_text(json.dumps(payload, indent=2), encoding="utf-8") - if uploads_bundle_dir.exists(): shutil.rmtree(uploads_bundle_dir) if source_upload_dir.exists(): @@ -99,6 +108,16 @@ def export_bundle(*, source_db_url: str, source_upload_dir: Path, bundle_dir: Pa else: uploads_bundle_dir.mkdir(parents=True, exist_ok=True) + _prepare_photo_payload_and_uploads( + payload=payload, + uploads_bundle_dir=uploads_bundle_dir, + source_has_photo_table=source_has_photo_table, + legacy_portrait_rows=legacy_portrait_rows, + ) + _relocate_homepage_markdown(uploads_bundle_dir=uploads_bundle_dir) + + export_json.write_text(json.dumps(payload, indent=2), encoding="utf-8") + def import_bundle(*, target_db_url: str, target_upload_dir: Path, bundle_dir: Path) -> None: export_json = bundle_dir / "database.json" @@ -162,11 +181,11 @@ def _serialize_row(row: dict[str, Any], *, table_name: str, source_upload_dir: P preferred_prefix="documents/", ) continue - if table_name == "person" and key == "portrait_path" and isinstance(serialized_value, str): + if table_name == "photo" and key == "path" and isinstance(serialized_value, str): serialized[key] = _canonical_media_relative_path( serialized_value, source_upload_dir=source_upload_dir, - preferred_prefix="persons/", + preferred_prefix="photos/", ) continue serialized[key] = serialized_value @@ -237,7 +256,7 @@ def _canonical_media_relative_path(value: str, *, source_upload_dir: Path, prefe normalized = "persons/" + normalized[len("portraits/") :] lowered = normalized.casefold() - for prefix in ("documents/", "persons/"): + for prefix in ("documents/", "photos/", "persons/"): marker = f"/{prefix}" index = lowered.find(marker) if index >= 0: @@ -248,3 +267,92 @@ def _canonical_media_relative_path(value: str, *, source_upload_dir: Path, prefe if not lowered.startswith(preferred_prefix): return normalized return Path(normalized).as_posix() + + +def _prepare_photo_payload_and_uploads( + *, + payload: dict[str, Any], + uploads_bundle_dir: Path, + source_has_photo_table: bool, + legacy_portrait_rows: list[dict[str, Any]], +) -> None: + photo_rows = payload.setdefault("tables", {}).setdefault("photo", []) + photos_dir = uploads_bundle_dir / "photos" + photos_dir.mkdir(parents=True, exist_ok=True) + + if source_has_photo_table: + return + + now_iso = datetime.now().isoformat() + for row in legacy_portrait_rows: + portrait_path = row.get("portrait_path") + person_id = row.get("id") + if not isinstance(portrait_path, str) or not portrait_path.strip(): + continue + if person_id is None: + continue + canonical = _canonical_media_relative_path( + portrait_path, + source_upload_dir=uploads_bundle_dir, + preferred_prefix="persons/", + ) + source_file = uploads_bundle_dir / canonical + suffix = Path(canonical).suffix.lower() or ".jpg" + photo_id = str(uuid4()) + relative_path = f"photos/{photo_id}{suffix}" + if source_file.exists(): + target_file = uploads_bundle_dir / relative_path + target_file.parent.mkdir(parents=True, exist_ok=True) + shutil.copy2(source_file, target_file) + else: + relative_path = canonical + photo_rows.append( + { + "id": photo_id, + "person_id": str(person_id), + "path": relative_path, + "description": None, + "is_primary": True, + "created_at": now_iso, + "updated_at": now_iso, + } + ) + + legacy_homepage_dir = uploads_bundle_dir / "homepage" + if not legacy_homepage_dir.exists(): + return + + homepage_images = sorted( + [ + path + for path in legacy_homepage_dir.iterdir() + if path.is_file() and path.suffix.lower() in {".jpg", ".jpeg", ".png", ".gif", ".webp", ".bmp", ".tif", ".tiff"} + ], + key=lambda path: (path.stat().st_mtime, path.name), + ) + for index, image_path in enumerate(homepage_images): + photo_id = str(uuid4()) + relative_path = f"photos/{photo_id}{image_path.suffix.lower()}" + target_file = uploads_bundle_dir / relative_path + target_file.parent.mkdir(parents=True, exist_ok=True) + shutil.copy2(image_path, target_file) + photo_rows.append( + { + "id": photo_id, + "person_id": None, + "path": relative_path, + "description": None, + "is_primary": index == 0, + "created_at": now_iso, + "updated_at": now_iso, + } + ) + + +def _relocate_homepage_markdown(*, uploads_bundle_dir: Path) -> None: + legacy_markdown = uploads_bundle_dir / "homepage" / "homepage.md" + target_markdown = uploads_bundle_dir / "homepage.md" + if not legacy_markdown.exists() or target_markdown.exists(): + return + target_markdown.parent.mkdir(parents=True, exist_ok=True) + shutil.copy2(legacy_markdown, target_markdown) diff --git a/src/transcription/db/models.py b/src/transcription/db/models.py index 2789955..7a88acb 100644 --- a/src/transcription/db/models.py +++ b/src/transcription/db/models.py @@ -186,7 +186,6 @@ class Person(SQLModel, table=True): death_date_raw: str | None = None death_place: str | None = None biography: str | None = None - portrait_path: str | None = None family_search_id: str | None = Field(default=None, unique=True) metadata_: dict[str, JsonValue] | None = Field( default=None, @@ -201,6 +200,32 @@ class Person(SQLModel, table=True): document_people: list["DocumentPerson"] = Relationship( back_populates="person", sa_relationship_kwargs={"lazy": "raise"} ) + photos: list["Photo"] = Relationship( + back_populates="person", + sa_relationship_kwargs={"lazy": "raise"}, + ) + + +class Photo(SQLModel, table=True): + """A reusable image record for Person and homepage galleries.""" + + __tablename__ = "photo" + + id: UUID = Field(default_factory=uuid4, primary_key=True) + person_id: UUID | None = Field(default=None, foreign_key="person.id", index=True) + path: str + description: str | None = None + is_primary: bool = False + created_at: datetime = Field(default_factory=lambda: datetime.now(UTC)) + updated_at: datetime = Field( + default_factory=lambda: datetime.now(UTC), + sa_column_kwargs={"onupdate": lambda: datetime.now(UTC)}, + ) + + person: Optional["Person"] = Relationship( + back_populates="photos", + sa_relationship_kwargs={"lazy": "raise"}, + ) class DocumentPerson(SQLModel, table=True): diff --git a/src/transcription/db/operations.py b/src/transcription/db/operations.py index 4b11a30..c6c27c4 100644 --- a/src/transcription/db/operations.py +++ b/src/transcription/db/operations.py @@ -93,18 +93,16 @@ async def reconcile_canonical_media_paths(*, engine: AsyncEngine | None = None) ) rows_changed += 1 - if "person" in table_names: - rows = sync_connection.execute( - text('select id, portrait_path from "person" where portrait_path is not null') - ).mappings().all() + if "photo" in table_names: + rows = sync_connection.execute(text('select id, path from "photo" where path is not null')).mappings().all() for row in rows: - original = str(row["portrait_path"]) - normalized = _canonical_relative_path(original, preferred_prefix="persons/") + original = str(row["path"]) + normalized = _canonical_relative_path(original, preferred_prefix="photos/") if normalized is None or normalized == original: continue sync_connection.execute( - text('update "person" set portrait_path = :portrait_path where id = :id'), - {"id": row["id"], "portrait_path": normalized}, + text('update "photo" set path = :path where id = :id'), + {"id": row["id"], "path": normalized}, ) rows_changed += 1 return rows_changed @@ -135,7 +133,7 @@ def _canonical_relative_path(value: str, *, preferred_prefix: str) -> str | None normalized = normalized[len("data/") :] lowered = normalized.casefold() - for prefix in ("documents/", "persons/", "portraits/"): + for prefix in ("documents/", "photos/", "persons/", "portraits/"): marker = f"/{prefix}" index = lowered.find(marker) if index >= 0: diff --git a/src/transcription/services/__init__.py b/src/transcription/services/__init__.py index b9b4b1b..3694bd2 100644 --- a/src/transcription/services/__init__.py +++ b/src/transcription/services/__init__.py @@ -12,6 +12,7 @@ from .documents import DocumentService from .evidence import EvidenceService from .jobs import JobService from .people import PeopleService +from .photos import PhotosService from .prompts import PromptStore from .sources import SourceService @@ -20,6 +21,7 @@ __all__ = [ "EvidenceService", "JobService", "PeopleService", + "PhotosService", "PromptStore", "ServiceBundle", "SourceService", @@ -34,6 +36,7 @@ class ServiceBundle: sources: SourceService = field(default_factory=SourceService) jobs: JobService = field(default_factory=JobService) people: PeopleService = field(default_factory=PeopleService) + photos: PhotosService = field(default_factory=PhotosService) evidence: EvidenceService = field(default_factory=EvidenceService) @classmethod @@ -51,6 +54,7 @@ class ServiceBundle: sources=SourceService(session_factory=session_factory, settings=settings), jobs=JobService(session_factory=session_factory, settings=settings), people=PeopleService(session_factory=session_factory, settings=settings), + photos=PhotosService(session_factory=session_factory, settings=settings), evidence=EvidenceService(session_factory=session_factory, settings=settings), ) diff --git a/src/transcription/services/people.py b/src/transcription/services/people.py index ee73865..1f11421 100644 --- a/src/transcription/services/people.py +++ b/src/transcription/services/people.py @@ -6,7 +6,6 @@ import logging import re from collections.abc import Sequence from dataclasses import dataclass -from pathlib import Path from typing import Any from uuid import UUID @@ -18,23 +17,21 @@ from sqlmodel import select from sqlmodel.ext.asyncio.session import AsyncSession from ..config import Settings -from ..config import get_settings from ..db.loading import orm_attribute from ..db.loading import selectinload from ..db.models import Document from ..db.models import DocumentPerson +from ..db.models import Photo from ..db.models import Person from ..db.models import PersonRole from ..errors import AppError from ..errors import ErrorCategory from .base import ServiceBase -from .media_storage import persist_named_media from .registry import RegistryService from .registry import RegistrySummary logger = logging.getLogger(__name__) -PORTRAIT_EXTENSIONS = frozenset({".jpg", ".jpeg", ".png", ".gif", ".webp", ".bmp", ".tif", ".tiff"}) FAMILY_SEARCH_ID_PATTERN = re.compile(r"^[A-Z0-9]{4}-[A-Z0-9]{3}$") @@ -42,10 +39,6 @@ class PeopleError(AppError): """Raised when a Person or document-person relationship operation fails.""" -class PersonMediaError(PeopleError): - """Raised when Person portrait media cannot be validated or persisted.""" - - class PersonRoleError(PeopleError): """Raised when Person Role maintenance fails.""" @@ -137,10 +130,16 @@ class PeopleService(ServiceBase): existing = await _session.get( Person, person.id, - options=(selectinload(Person.document_people),), + options=(selectinload(Person.document_people), selectinload(Person.photos)), ) if existing is None: raise self._not_found(f"Person with id {person.id} not found") + if existing.photos: + raise PeopleError( + "Person delete blocked by related records: Photos", + category=ErrorCategory.VALIDATION, + suggestion="Delete or reassign Person photos before deleting this record.", + ) for link in list(existing.document_people): await _session.delete(link) await _session.delete(existing) @@ -206,6 +205,7 @@ class PeopleService(ServiceBase): .selectinload(orm_attribute(DocumentPerson.document)) .selectinload(orm_attribute(Document.sources)), selectinload(Person.document_people).selectinload(orm_attribute(DocumentPerson.role_ref)), + selectinload(Person.photos), ) .where(Person.id == person_id) .execution_options(populate_existing=True) @@ -513,38 +513,3 @@ class PeopleService(ServiceBase): category=ErrorCategory.NOT_FOUND, suggestion="Verify the requested Person or relationship id and retry.", ) - - -async def store_person_portrait( - *, - person_id: UUID, - filename: str, - file_bytes: bytes, - settings: Settings | None = None, -) -> Path: - """Persist Person portrait media under persons/.""" - if not file_bytes: - raise PersonMediaError( - "Person portrait content is empty", - category=ErrorCategory.VALIDATION, - suggestion="Select a non-empty portrait file and retry.", - ) - suffix = Path(filename).suffix.lower() - if suffix not in PORTRAIT_EXTENSIONS: - raise PersonMediaError( - f"Unsupported portrait format: {suffix or ''}", - category=ErrorCategory.USER_INPUT, - suggestion="Use JPG, JPEG, PNG, GIF, WEBP, BMP, or TIFF portrait media.", - ) - - runtime_settings = settings or get_settings() - return await persist_named_media( - root=runtime_settings.upload_dir, - namespace=Path("persons") / str(person_id), - filename=filename, - file_bytes=file_bytes, - error=PersonMediaError, - failure_message="Failed to persist Person portrait media", - failure_suggestion="Check media directory permissions and available disk space, then retry.", - log_label="Person portrait media", - ) diff --git a/src/transcription/services/photos.py b/src/transcription/services/photos.py new file mode 100644 index 0000000..592f13d --- /dev/null +++ b/src/transcription/services/photos.py @@ -0,0 +1,209 @@ +"""Photo service for Person and homepage image records.""" + +from __future__ import annotations + +import asyncio +import random +from pathlib import Path +from uuid import UUID +from uuid import uuid4 + +from sqlalchemy.ext.asyncio import async_sessionmaker +from sqlmodel import select +from sqlmodel.ext.asyncio.session import AsyncSession + +from ..config import Settings +from ..db.models import Photo +from ..errors import AppError +from ..errors import ErrorCategory +from .base import ServiceBase +from .media_storage import persist_named_media + +PHOTO_EXTENSIONS = frozenset({".jpg", ".jpeg", ".png", ".gif", ".webp", ".bmp", ".tif", ".tiff"}) + + +class PhotoError(AppError): + """Raised when photo operations fail.""" + + +class PhotosService(ServiceBase): + """Manage homepage and Person photos.""" + + def __init__( + self, + session_factory: async_sessionmaker[AsyncSession] | None = None, + settings: Settings | None = None, + ) -> None: + super().__init__(session_factory, settings) + + async def create_photo( + self, + *, + filename: str, + file_bytes: bytes, + person_id: UUID | None, + description: str | None = None, + is_primary: bool | None = None, + session: AsyncSession | None = None, + ) -> Photo: + if not file_bytes: + raise PhotoError( + "Photo content is empty", + category=ErrorCategory.VALIDATION, + suggestion="Select a non-empty image file and retry.", + ) + + suffix = Path(filename).suffix.lower() + if suffix not in PHOTO_EXTENSIONS: + raise PhotoError( + f"Unsupported photo format: {suffix or ''}", + category=ErrorCategory.USER_INPUT, + suggestion="Use JPG, JPEG, PNG, GIF, WEBP, BMP, or TIFF image files.", + ) + + photo_id = uuid4() + async with self._session_scope(session) as _session: + existing = await self._list_owner_photos(session=_session, person_id=person_id) + should_be_primary = bool(is_primary) if is_primary is not None else len(existing) == 0 + if should_be_primary: + await self._clear_owner_primary(session=_session, person_id=person_id) + + stored_path = await persist_named_media( + root=self.settings.upload_dir, + namespace=Path("photos"), + filename=filename, + filename_stem=str(photo_id), + file_bytes=file_bytes, + error=PhotoError, + failure_message="Failed to persist photo media", + failure_suggestion="Check media directory permissions and available disk space, then retry.", + log_label="photo media", + ) + relative_path = self._relative_upload_path(stored_path) + + photo = Photo( + id=photo_id, + person_id=person_id, + path=relative_path, + description=(description or "").strip() or None, + is_primary=should_be_primary, + ) + _session.add(photo) + await self._finalize(session=_session, caller_session=session, refresh=(photo,)) + return photo + + async def list_photos( + self, + *, + person_id: UUID | None, + session: AsyncSession | None = None, + ) -> list[Photo]: + async with self._session_scope(session) as _session: + photos = await self._list_owner_photos(session=_session, person_id=person_id) + + primary = [photo for photo in photos if photo.is_primary] + non_primary = [photo for photo in photos if not photo.is_primary] + random.shuffle(non_primary) + return [*primary[:1], *non_primary] + + async def set_primary( + self, + *, + photo_id: UUID, + session: AsyncSession | None = None, + ) -> Photo: + async with self._session_scope(session) as _session: + photo = await self._get_or_raise( + Photo, + photo_id, + session=_session, + error=PhotoError, + noun="Photo", + suggestion="Refresh and retry with a valid photo record.", + ) + await self._clear_owner_primary(session=_session, person_id=photo.person_id) + photo.is_primary = True + await self._finalize(session=_session, caller_session=session, refresh=(photo,)) + return photo + + async def update_description( + self, + *, + photo_id: UUID, + description: str | None, + session: AsyncSession | None = None, + ) -> Photo: + async with self._session_scope(session) as _session: + photo = await self._get_or_raise( + Photo, + photo_id, + session=_session, + error=PhotoError, + noun="Photo", + suggestion="Refresh and retry with a valid photo record.", + ) + photo.description = (description or "").strip() or None + await self._finalize(session=_session, caller_session=session, refresh=(photo,)) + return photo + + async def delete_photo( + self, + *, + photo_id: UUID, + session: AsyncSession | None = None, + ) -> None: + async with self._session_scope(session) as _session: + photo = await self._get_or_raise( + Photo, + photo_id, + session=_session, + error=PhotoError, + noun="Photo", + suggestion="Refresh and retry with a valid photo record.", + ) + owner_person_id = photo.person_id + deleted_primary = photo.is_primary + media_path = self.settings.upload_dir / Path(photo.path) + await _session.delete(photo) + + if deleted_primary: + replacement = await self._owner_oldest_photo(session=_session, person_id=owner_person_id) + if replacement is not None: + replacement.is_primary = True + + await self._finalize(session=_session, caller_session=session) + + await asyncio.to_thread(media_path.unlink, missing_ok=True) + + async def _list_owner_photos(self, *, session: AsyncSession, person_id: UUID | None) -> list[Photo]: + query = select(Photo) + if person_id is None: + query = query.where(Photo.person_id.is_(None)) + else: + query = query.where(Photo.person_id == person_id) + query = query.order_by(Photo.created_at.asc(), Photo.id.asc()) + return list((await session.exec(query)).all()) + + async def _owner_oldest_photo(self, *, session: AsyncSession, person_id: UUID | None) -> Photo | None: + query = select(Photo) + if person_id is None: + query = query.where(Photo.person_id.is_(None)) + else: + query = query.where(Photo.person_id == person_id) + query = query.order_by(Photo.created_at.asc(), Photo.id.asc()).limit(1) + return (await session.exec(query)).first() + + async def _clear_owner_primary(self, *, session: AsyncSession, person_id: UUID | None) -> None: + query = select(Photo).where(Photo.is_primary.is_(True)) + if person_id is None: + query = query.where(Photo.person_id.is_(None)) + else: + query = query.where(Photo.person_id == person_id) + for current in (await session.exec(query)).all(): + current.is_primary = False + + def _relative_upload_path(self, absolute_path: Path) -> str: + try: + return absolute_path.resolve().relative_to(self.settings.upload_dir.resolve()).as_posix() + except ValueError: + return absolute_path.name diff --git a/src/transcription/ui/components/media_urls.py b/src/transcription/ui/components/media_urls.py index 3b4c2ae..a83f899 100644 --- a/src/transcription/ui/components/media_urls.py +++ b/src/transcription/ui/components/media_urls.py @@ -7,7 +7,7 @@ from urllib.parse import quote _ABSOLUTE_SCHEMES = ("http://", "https://", "data:") _UPLOAD_ROUTE_PREFIX = "/uploads/" -_CANONICAL_PREFIXES = ("documents/", "persons/") +_CANONICAL_PREFIXES = ("documents/", "persons/", "photos/") def absolute_upload_url(path: str, *, base_url: str) -> str: diff --git a/src/transcription/ui/homepage_store.py b/src/transcription/ui/homepage_store.py index e2b3315..2333e01 100644 --- a/src/transcription/ui/homepage_store.py +++ b/src/transcription/ui/homepage_store.py @@ -1,11 +1,4 @@ -"""File-backed storage helpers for the homepage content. - -The homepage storage root is a configured setting (``homepage_dir``) like every -other storage root, rather than a path derived from this module's location. The -previous ``Path(__file__).parents[3]`` form was both unconfigurable and wrong -outside a source checkout, since an installed distribution would resolve it into -the package directory. -""" +"""File-backed storage helpers for homepage markdown content.""" from __future__ import annotations @@ -13,38 +6,20 @@ from pathlib import Path from transcription.config import Settings from transcription.config import get_settings -from transcription.errors import AppError -from transcription.services.media_storage import persist_named_media HOME_PAGE_MARKDOWN_NAME = "homepage.md" -SUPPORTED_IMAGE_SUFFIXES = {".jpg", ".jpeg", ".png", ".gif", ".webp", ".bmp", ".tif", ".tiff"} - - -class HomepageStorageError(AppError): - """Raised when homepage media cannot be persisted.""" - - -def homepage_dir(settings: Settings | None = None) -> Path: - """Return the configured homepage storage directory.""" - return (settings or get_settings()).homepage_dir def homepage_markdown_path(settings: Settings | None = None) -> Path: - """Return the configured homepage markdown file path.""" - return homepage_dir(settings) / HOME_PAGE_MARKDOWN_NAME - - -def ensure_homepage_storage(settings: Settings | None = None) -> Path: - """Create the homepage storage directory when needed and return it.""" - directory = homepage_dir(settings) - directory.mkdir(parents=True, exist_ok=True) - return directory + """Return the homepage markdown file path under upload storage.""" + runtime_settings = settings or get_settings() + return runtime_settings.upload_dir / HOME_PAGE_MARKDOWN_NAME def read_homepage_markdown(settings: Settings | None = None) -> str: """Read the saved homepage markdown text.""" - ensure_homepage_storage(settings) path = homepage_markdown_path(settings) + path.parent.mkdir(parents=True, exist_ok=True) if not path.exists(): return "" return path.read_text(encoding="utf-8") @@ -52,47 +27,6 @@ def read_homepage_markdown(settings: Settings | None = None) -> str: def save_homepage_markdown(markdown_text: str, settings: Settings | None = None) -> None: """Persist the homepage markdown text.""" - ensure_homepage_storage(settings) - homepage_markdown_path(settings).write_text(markdown_text, encoding="utf-8") - - -async def store_homepage_image( - *, - filename: str, - file_bytes: bytes, - settings: Settings | None = None, -) -> Path: - """Persist an uploaded homepage image in the shared homepage folder.""" - safe_name = Path(filename).name - if not safe_name: - msg = "Homepage image filename is required" - raise ValueError(msg) - - return await persist_named_media( - root=homepage_dir(settings), - filename=safe_name, - preserve_original_name=True, - file_bytes=file_bytes, - error=HomepageStorageError, - failure_message="Failed to persist homepage image", - failure_suggestion="Check homepage directory permissions and available disk space, then retry.", - log_label="homepage image", - ) - - -def list_homepage_images(settings: Settings | None = None) -> list[Path]: - """List stored homepage images in the order they were last updated.""" - directory = ensure_homepage_storage(settings) - - image_paths = [ - path for path in directory.iterdir() if path.is_file() and path.suffix.lower() in SUPPORTED_IMAGE_SUFFIXES - ] - return sorted(image_paths, key=lambda path: (path.stat().st_mtime, path.name)) - - -def latest_homepage_image(settings: Settings | None = None) -> Path | None: - """Return the most recently updated homepage image, if one exists.""" - image_paths = list_homepage_images(settings) - if not image_paths: - return None - return image_paths[-1] + path = homepage_markdown_path(settings) + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(markdown_text, encoding="utf-8") diff --git a/src/transcription/ui/pages/home_page.py b/src/transcription/ui/pages/home_page.py index 98e930f..9caffcd 100644 --- a/src/transcription/ui/pages/home_page.py +++ b/src/transcription/ui/pages/home_page.py @@ -3,54 +3,71 @@ from __future__ import annotations from collections.abc import Callable -from pathlib import Path +from fastapi import Request from nicegui import events from nicegui import ui +from transcription.config import Settings +from transcription.db.models import Photo +from transcription.services.photos import PhotoError +from transcription.services.photos import PhotosService from transcription.ui.components.app_shell import render_navigation_header from transcription.ui.components.cards import archival_card +from transcription.ui.components.media_urls import resolve_media_url from transcription.ui.components.primitives import render_empty_state from transcription.ui.components.primitives import section_header_row from transcription.ui.components.upload_panel import IMAGE_UPLOAD_EXTENSIONS from transcription.ui.components.upload_panel import render_upload_picker from transcription.ui.components.viewers import dark_room_viewer -from transcription.ui.homepage_store import list_homepage_images from transcription.ui.homepage_store import read_homepage_markdown from transcription.ui.homepage_store import save_homepage_markdown -from transcription.ui.homepage_store import store_homepage_image +from transcription.ui.runtime import resolve_runtime_settings from transcription.ui.theme import page_header +from ...db.session import SessionFactoryDep -def _shift_gallery_index(*, image_paths: list[Path], active_index: list[int], step: int) -> None: - if len(image_paths) < 2: + +def _shift_gallery_index(*, photos: list[Photo], active_index: list[int], step: int) -> None: + if len(photos) < 2: active_index[0] = 0 return - active_index[0] = (active_index[0] + step) % len(image_paths) + active_index[0] = (active_index[0] + step) % len(photos) def _render_homepage_gallery( *, - image_paths: list[Path], + photos: list[Photo], active_index: list[int], + settings: Settings, + base_url: str, enable_rotation: bool = False, rotate_enabled: list[bool] | None = None, on_change: Callable[[], None] | None = None, ) -> None: - if not image_paths: + if not photos: render_empty_state("No homepage image uploaded yet.") return - if active_index[0] >= len(image_paths): - active_index[0] = len(image_paths) - 1 + if active_index[0] >= len(photos): + active_index[0] = len(photos) - 1 if active_index[0] < 0: active_index[0] = 0 - current_path = image_paths[active_index[0]] - dark_room_viewer(str(current_path), count_label="Homepage Image") + current_photo = photos[active_index[0]] + dark_room_viewer( + resolve_media_url(current_photo.path, upload_dir=settings.upload_dir, base_url=base_url), + count_label="Homepage Image", + ) + if current_photo.description: + ui.label(current_photo.description).classes("text-xs") + else: + ui.label("No description").classes("text-xs ui-text-muted") + if current_photo.is_primary: + ui.label("Featured image").classes("text-[11px] font-semibold ui-text-primary") def move(step: int) -> None: - _shift_gallery_index(image_paths=image_paths, active_index=active_index, step=step) + _shift_gallery_index(photos=photos, active_index=active_index, step=step) if on_change is not None: on_change() @@ -65,10 +82,10 @@ def _render_homepage_gallery( on_click=lambda: move(1), icon="chevron_right", ).props("flat dense icon-right") - if len(image_paths) < 2: + if len(photos) < 2: previous.props("disable") following.props("disable") - ui.label(f"{active_index[0] + 1} of {len(image_paths)}").classes("text-xs ui-text-muted") + ui.label(f"{active_index[0] + 1} of {len(photos)}").classes("text-xs ui-text-muted") if enable_rotation and rotate_enabled is not None: def set_rotation(enabled: bool) -> None: @@ -124,17 +141,21 @@ def register_page() -> None: """Register the homepage routes.""" @ui.page("/homepage", title="VibeScribe Home") - def homepage_page() -> None: + async def homepage_page(request: Request, session_factory: SessionFactoryDep) -> None: + photos_service = PhotosService(session_factory=session_factory) + settings = resolve_runtime_settings(request) render_navigation_header(current_path="/homepage") - image_paths = list_homepage_images() - active_index = [len(image_paths) - 1 if image_paths else 0] + photos = await photos_service.list_photos(person_id=None) + active_index = [0] @ui.refreshable def render_image_panel() -> None: with archival_card(title="Homepage Images"): _render_homepage_gallery( - image_paths=image_paths, + photos=photos, active_index=active_index, + settings=settings, + base_url=str(request.base_url), on_change=render_image_panel.refresh, ) @@ -152,11 +173,13 @@ def register_page() -> None: ) @ui.page("/homepage/edit", title="Edit Homepage") - def homepage_edit_page() -> None: + async def homepage_edit_page(request: Request, session_factory: SessionFactoryDep) -> None: + photos_service = PhotosService(session_factory=session_factory) + settings = resolve_runtime_settings(request) render_navigation_header(current_path="/homepage") - preview_images = [*list_homepage_images()] - active_index = [len(preview_images) - 1 if preview_images else 0] + photos = await photos_service.list_photos(person_id=None) + active_index = [0] rotate_enabled = [False] markdown_input = [None] @@ -164,28 +187,76 @@ def register_page() -> None: def render_image_panel() -> None: with archival_card(title="Homepage Images"): _render_homepage_gallery( - image_paths=preview_images, + photos=photos, active_index=active_index, + settings=settings, + base_url=str(request.base_url), enable_rotation=True, rotate_enabled=rotate_enabled, on_change=render_image_panel.refresh, ) + if photos: + current_photo = photos[active_index[0]] + description_input = ui.input( + label="Image description", + value=current_photo.description or "", + ).props("outlined dense").classes("w-full") + + async def save_description() -> None: + try: + await photos_service.update_description( + photo_id=current_photo.id, + description=(description_input.value or "").strip() or None, + ) + except PhotoError as exc: + ui.notify(str(exc), type="negative") + return + ui.navigate.to("/homepage/edit") + + async def set_featured() -> None: + try: + await photos_service.set_primary(photo_id=current_photo.id) + except PhotoError as exc: + ui.notify(str(exc), type="negative") + return + ui.navigate.to("/homepage/edit") + + async def delete_photo() -> None: + try: + await photos_service.delete_photo(photo_id=current_photo.id) + except PhotoError as exc: + ui.notify(str(exc), type="negative") + return + ui.navigate.to("/homepage/edit") + + with ui.row().classes("w-full items-center gap-2"): + ui.button("Save description", on_click=save_description, icon="save").props("flat") + if not current_photo.is_primary: + ui.button("Set featured", on_click=set_featured, icon="star").props("flat") + ui.button("Delete image", on_click=delete_photo, icon="delete").props("flat color=negative") + def rotate_gallery() -> None: if not rotate_enabled[0]: return - _shift_gallery_index(image_paths=preview_images, active_index=active_index, step=1) + _shift_gallery_index(photos=photos, active_index=active_index, step=1) render_image_panel.refresh() ui.timer(interval=600, callback=rotate_gallery) async def on_upload(event: events.UploadEventArguments) -> None: payload = await event.file.read() - stored = await store_homepage_image(filename=event.file.name, file_bytes=payload) - preview_images.append(stored) - active_index[0] = len(preview_images) - 1 + try: + await photos_service.create_photo( + person_id=None, + filename=event.file.name, + file_bytes=payload, + ) + except PhotoError as exc: + ui.notify(str(exc), type="negative") + return ui.notify(f"Uploaded {event.file.name}", type="positive") - render_image_panel.refresh() + ui.navigate.to("/homepage/edit") async def save_homepage() -> None: save_homepage_markdown((markdown_input[0].value if markdown_input[0] is not None else "") or "") diff --git a/src/transcription/ui/pages/people_page.py b/src/transcription/ui/pages/people_page.py index 835b855..8d5fc10 100644 --- a/src/transcription/ui/pages/people_page.py +++ b/src/transcription/ui/pages/people_page.py @@ -7,7 +7,6 @@ from uuid import UUID from uuid import uuid4 from fastapi import Request -from nicegui import events from nicegui import ui from transcription.config import Settings @@ -15,8 +14,8 @@ from transcription.db.models import Person from transcription.errors import ErrorCategory from transcription.services.people import PeopleError from transcription.services.people import PeopleService -from transcription.services.people import PersonMediaError -from transcription.services.people import store_person_portrait +from transcription.services.photos import PhotoError +from transcription.services.photos import PhotosService from transcription.ui.components.app_shell import render_navigation_header from transcription.ui.components.cards import archival_card from transcription.ui.components.confirm_delete import render_delete_actions @@ -60,7 +59,6 @@ class PersonFormFields: death_date_raw: ui.input death_place: ui.input biography: ui.textarea - portrait_path: ui.input family_search_id: ui.input @@ -117,8 +115,6 @@ def register_page() -> None: # noqa: PLR0915 page_header("Create Person Record", subtitle="Full name is required.") form = _render_person_form_fields( - request=request, - person_id=draft_person_id, ) async def submit_create() -> None: @@ -142,7 +138,6 @@ def register_page() -> None: # noqa: PLR0915 death_date_raw=(form.death_date_raw.value or "").strip() or None, death_place=(form.death_place.value or "").strip() or None, biography=(form.biography.value or "").strip() or None, - portrait_path=(form.portrait_path.value or "").strip() or None, family_search_id=(form.family_search_id.value or "").strip() or None, ) @@ -165,6 +160,7 @@ def register_page() -> None: # noqa: PLR0915 @ui.page("/people/{person_id}") async def person_detail_page(person_id: str, request: Request, session_factory: SessionFactoryDep) -> None: people_service = PeopleService(session_factory=session_factory) + photos_service = PhotosService(session_factory=session_factory) render_navigation_header(current_path="/people") parsed_person_id = parsed_record_id(person_id, noun="Person") @@ -203,8 +199,9 @@ def register_page() -> None: # noqa: PLR0915 ) with ui.grid().classes("w-full grid-cols-12 gap-4"): - _render_person_portrait_zone( + await _render_person_photo_zone( person, + photos_service=photos_service, settings=resolve_runtime_settings(request), request=request, ) @@ -233,9 +230,7 @@ def register_page() -> None: # noqa: PLR0915 page_header("Edit Person Record", subtitle="Full name is required.") form = _render_person_form_fields( - request=request, person=person, - person_id=person.id, ) async def submit_edit() -> None: @@ -259,7 +254,6 @@ def register_page() -> None: # noqa: PLR0915 death_date_raw=(form.death_date_raw.value or "").strip() or None, death_place=(form.death_place.value or "").strip() or None, biography=(form.biography.value or "").strip() or None, - portrait_path=(form.portrait_path.value or "").strip() or None, family_search_id=(form.family_search_id.value or "").strip() or None, metadata_=person.metadata_, created_at=person.created_at, @@ -345,9 +339,7 @@ def register_page() -> None: # noqa: PLR0915 def _render_person_form_fields( *, - request: Request, person: Person | None = None, - person_id: UUID, ) -> PersonFormFields: with archival_card(extra_classes="gap-3"): with ui.row().classes("w-full gap-3 grid grid-cols-1 md:grid-cols-3"): @@ -418,11 +410,6 @@ def _render_person_form_fields( .props("outlined autogrow") .classes("w-full ui-form-surface") ) - portrait_path_input = ( - ui.input(label="Portrait path", value=person.portrait_path if person and person.portrait_path else "") - .props("outlined") - .classes("w-full ui-form-surface") - ) family_search_id_input = ( ui.input( label="FamilySearch ID", @@ -433,12 +420,6 @@ def _render_person_form_fields( .classes("w-full ui-form-surface") ) - _bind_portrait_file_picker( - portrait_path_input, - settings=resolve_runtime_settings(request), - person_id=person_id, - ) - return PersonFormFields( full_name=full_name_input, display_name=display_name_input, @@ -450,19 +431,96 @@ def _render_person_form_fields( death_date_raw=death_date_raw_input, death_place=death_place_input, biography=biography_input, - portrait_path=portrait_path_input, family_search_id=family_search_id_input, ) -def _render_person_portrait_zone(person: Person, *, settings: Settings, request: Request) -> None: - portrait_src = resolve_media_url( - person.portrait_path, - upload_dir=settings.upload_dir, - base_url=str(request.base_url), - ) +async def _render_person_photo_zone( + person: Person, + *, + photos_service: PhotosService, + settings: Settings, + request: Request, +) -> None: + photos = await photos_service.list_photos(person_id=person.id) + with ui.column().classes("col-span-12 lg:col-span-4"): - dark_room_viewer(portrait_src, count_label="Portrait Media") + with archival_card(title="Photos", extra_classes="gap-3"): + async def on_photo_selected(event) -> None: + payload = await event.file.read() + try: + await photos_service.create_photo( + person_id=person.id, + filename=event.file.name, + file_bytes=payload, + ) + except PhotoError as exc: + ui.notify(str(exc), type="negative") + return + ui.notify("Photo uploaded.", type="positive") + ui.navigate.to(f"/people/{person.id}") + + render_upload_picker( + on_upload=on_photo_selected, + label="Upload photo(s)", + extensions=IMAGE_UPLOAD_EXTENSIONS, + multiple=True, + ) + + if not photos: + render_empty_state("No portrait photo uploaded yet.") + return + + for photo in photos: + dark_room_viewer( + resolve_media_url( + photo.path, + upload_dir=settings.upload_dir, + base_url=str(request.base_url), + ), + count_label="Primary Portrait" if photo.is_primary else "Portrait Media", + ) + description_input = ( + ui.input( + label="Description", + value=photo.description or "", + ) + .props("outlined dense") + .classes("w-full") + ) + + async def save_description(*, photo_id: UUID = photo.id, input_control: ui.input = description_input) -> None: + try: + await photos_service.update_description( + photo_id=photo_id, + description=(input_control.value or "").strip() or None, + ) + except PhotoError as exc: + ui.notify(str(exc), type="negative") + return + ui.navigate.to(f"/people/{person.id}") + + async def set_primary(*, photo_id: UUID = photo.id) -> None: + try: + await photos_service.set_primary(photo_id=photo_id) + except PhotoError as exc: + ui.notify(str(exc), type="negative") + return + ui.navigate.to(f"/people/{person.id}") + + async def delete_photo(*, photo_id: UUID = photo.id) -> None: + try: + await photos_service.delete_photo(photo_id=photo_id) + except PhotoError as exc: + ui.notify(str(exc), type="negative") + return + ui.navigate.to(f"/people/{person.id}") + + with ui.row().classes("w-full items-center gap-2 mb-4"): + ui.button("Save description", on_click=save_description, icon="save").props("flat") + if not photo.is_primary: + ui.button("Set primary", on_click=set_primary, icon="star").props("flat") + ui.button("Delete photo", on_click=delete_photo, icon="delete").props("flat color=negative") def _render_person_biographical_zone(person: Person) -> None: @@ -577,38 +635,4 @@ def _render_linked_documents(person: Person) -> None: ) -# --- Utilities & Input Binding Helpers --- - - -def _bind_portrait_file_picker(portrait_path_input: ui.input, *, settings: Settings, person_id: UUID) -> None: - async def on_portrait_selected(event: events.UploadEventArguments) -> None: - payload = await event.file.read() - try: - stored_path = await store_person_portrait( - person_id=person_id, - filename=event.file.name, - file_bytes=payload, - settings=settings, - ) - except PersonMediaError as exc: - ui.notify(str(exc), type="negative") - return - except Exception as exc: # noqa: BLE001 - show_error(exc, title="Upload failed", operation="people.portrait.store") - return - - try: - relative_path = stored_path.resolve().relative_to(settings.upload_dir.resolve()).as_posix() - except ValueError: - relative_path = stored_path.name - - portrait_path_input.value = relative_path - ui.notify("Portrait uploaded.", type="positive") - - render_upload_picker( - on_upload=on_portrait_selected, - label="Choose portrait file", - extensions=IMAGE_UPLOAD_EXTENSIONS, - ) - portrait_dir = settings.upload_dir / "persons" / str(person_id) - ui.label(f"Portraits are stored under {portrait_dir}.").classes("text-xs ui-text-muted") +# --- Utilities --- diff --git a/tests/services/test_document_service.py b/tests/services/test_document_service.py index 0bb4d7e..dda6ac2 100644 --- a/tests/services/test_document_service.py +++ b/tests/services/test_document_service.py @@ -15,6 +15,7 @@ from transcription.db.models import Job from transcription.db.models import Person from transcription.db.models import Source from transcription.db.models import Tag +from transcription.db.models import Photo from transcription.services.documents import DocumentDeleteBlockedError from transcription.services.documents import DocumentError from transcription.services.documents import DocumentService @@ -275,6 +276,25 @@ async def test_delete_person_succeeds_when_unlinked(default_session_factory): await service.read_person_detail(person.id) +@pytest.mark.asyncio +async def test_delete_person_blocks_when_photos_exist(default_session_factory): + service = PeopleService(session_factory=default_session_factory) + + person = await service.create_person(Person(full_name="Photo Protected Person")) + async with service._session_scope() as session: + session.add( + Photo( + person_id=person.id, + path="photos/sample.png", + is_primary=True, + ) + ) + await session.commit() + + with pytest.raises(PeopleError, match="Photos"): + await service.delete_person(person) + + @pytest.mark.asyncio async def test_create_document_uses_existing_document_type_registry(default_session_factory): service = DocumentService(session_factory=default_session_factory) diff --git a/tests/services/test_photo_service.py b/tests/services/test_photo_service.py new file mode 100644 index 0000000..534ca07 --- /dev/null +++ b/tests/services/test_photo_service.py @@ -0,0 +1,53 @@ +from __future__ import annotations + +import pytest + +from transcription.config import Settings +from transcription.db.models import Person +from transcription.services.people import PeopleService +from transcription.services.photos import PhotosService + +PNG_BYTES = bytes.fromhex( + "89504e470d0a1a0a0000000d49484452000000010000000108060000001f15c4890000000a49444154789c6360000002000100" + "05fe02fea7b1b8000000004945" +) + b"NDAE\xae\x42\x60\x82" + + +@pytest.mark.asyncio +async def test_create_photo_persists_media_and_primary_state(default_session_factory, tmp_path): + settings = Settings(openrouter_api_key="test-key", upload_dir=tmp_path / "uploads") + people = PeopleService(session_factory=default_session_factory) + photos = PhotosService(session_factory=default_session_factory, settings=settings) + person = await people.create_person(Person(full_name="Photo Person")) + + first = await photos.create_photo(person_id=person.id, filename="one.png", file_bytes=PNG_BYTES) + second = await photos.create_photo(person_id=person.id, filename="two.png", file_bytes=PNG_BYTES) + listed = await photos.list_photos(person_id=person.id) + + assert first.path.startswith("photos/") + assert (settings.upload_dir / first.path).exists() + assert first.is_primary is True + assert second.is_primary is False + assert listed[0].is_primary is True + + +@pytest.mark.asyncio +async def test_set_primary_and_delete_promotes_next_photo(default_session_factory, tmp_path): + settings = Settings(openrouter_api_key="test-key", upload_dir=tmp_path / "uploads") + people = PeopleService(session_factory=default_session_factory) + photos = PhotosService(session_factory=default_session_factory, settings=settings) + person = await people.create_person(Person(full_name="Primary Person")) + + first = await photos.create_photo(person_id=person.id, filename="one.png", file_bytes=PNG_BYTES) + second = await photos.create_photo(person_id=person.id, filename="two.png", file_bytes=PNG_BYTES) + + await photos.set_primary(photo_id=second.id) + switched = await photos.list_photos(person_id=person.id) + assert switched[0].id == second.id + assert switched[0].is_primary is True + + await photos.delete_photo(photo_id=second.id) + remaining = await photos.list_photos(person_id=person.id) + assert len(remaining) == 1 + assert remaining[0].id == first.id + assert remaining[0].is_primary is True diff --git a/tests/services/test_store.py b/tests/services/test_store.py index df6f918..efde589 100644 --- a/tests/services/test_store.py +++ b/tests/services/test_store.py @@ -11,7 +11,7 @@ from transcription.db.models import Job from transcription.db.models import JobSource from transcription.db.models import Source from transcription.errors import ErrorCategory -from transcription.services.people import store_person_portrait +from transcription.services.photos import PhotosService from transcription.services.sources import source_mime_type from transcription.services.store import SourceStorageError from transcription.services.store import StoredSourceFile @@ -121,18 +121,18 @@ async def test_create_document_job_stores_source_under_document_id_directory(asy @pytest.mark.asyncio -async def test_store_person_portrait_stores_file_under_person_id_directory(tmp_path): +async def test_store_person_photo_stores_file_under_shared_photos_directory(default_session_factory, tmp_path): settings = Settings(openrouter_api_key="test-key", upload_dir=tmp_path) - person_id = uuid4() + service = PhotosService(session_factory=default_session_factory, settings=settings) - stored_path = await store_person_portrait( - person_id=person_id, + created = await service.create_photo( + person_id=None, filename="portrait.png", file_bytes=b"portrait-bytes", - settings=settings, ) - assert stored_path.parent == (tmp_path / "persons" / str(person_id)) + stored_path = tmp_path / created.path + assert stored_path.parent == (tmp_path / "photos") assert stored_path.exists() diff --git a/tests/test_db.py b/tests/test_db.py index ac12848..8072ff8 100644 --- a/tests/test_db.py +++ b/tests/test_db.py @@ -3,6 +3,7 @@ import warnings import pytest +import pytest_asyncio import sqlalchemy as sa from sqlalchemy import inspect from sqlalchemy import text @@ -25,6 +26,13 @@ from transcription.db.models import PersonRole from transcription.db.models import Source +@pytest_asyncio.fixture(autouse=True) +async def _reset_database_runtime(): + await dispose_database_runtime() + yield + await dispose_database_runtime() + + @pytest.mark.asyncio async def test_create_all_creates_expected_tables(tmp_path): settings = Settings( @@ -43,6 +51,7 @@ async def test_create_all_creates_expected_tables(tmp_path): assert "document_type" in table_names assert "tag" in table_names assert "person" in table_names + assert "photo" in table_names assert "person_role" in table_names assert "document_person" in table_names assert "document_tag" in table_names @@ -205,7 +214,7 @@ async def test_reconcile_legacy_job_source_columns_drops_executed_at(tmp_path): @pytest.mark.asyncio -async def test_reconcile_canonical_media_paths_normalizes_source_and_person_paths(tmp_path): +async def test_reconcile_canonical_media_paths_normalizes_source_and_photo_paths(tmp_path): settings = Settings( openrouter_api_key="test-key", database=SqliteSettings(path=str(tmp_path / "canonical-paths.db")), @@ -218,10 +227,22 @@ async def test_reconcile_canonical_media_paths_normalizes_source_and_person_path async with runtime.engine.begin() as connection: await connection.execute( text( - 'insert into "person" (id, full_name, portrait_path, created_at, updated_at) ' - 'values (:id, :full_name, :portrait_path, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)' + 'insert into "person" (id, full_name, created_at, updated_at) ' + 'values (:id, :full_name, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)' ), - {"id": "11" * 16, "full_name": "Portrait", "portrait_path": "portraits/person/seeded.png"}, + {"id": "11" * 16, "full_name": "Portrait"}, + ) + await connection.execute( + text( + 'insert into "photo" (id, person_id, path, is_primary, created_at, updated_at) ' + 'values (:id, :person_id, :path, :is_primary, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)' + ), + { + "id": "44" * 16, + "person_id": "11" * 16, + "path": "data\\photos\\seeded.png", + "is_primary": 1, + }, ) await connection.execute( text( @@ -253,11 +274,11 @@ async def test_reconcile_canonical_media_paths_normalizes_source_and_person_path source_path = ( await connection.execute(text('select file_path from "source" where id = :id'), {"id": "33" * 16}) ).scalar_one() - portrait_path = ( - await connection.execute(text('select portrait_path from "person" where id = :id'), {"id": "11" * 16}) + photo_path = ( + await connection.execute(text('select path from "photo" where id = :id'), {"id": "44" * 16}) ).scalar_one() assert source_path == "documents/doc-1/page.png" - assert portrait_path == "persons/person/seeded.png" + assert photo_path == "photos/seeded.png" finally: await dispose_database_runtime() diff --git a/tests/test_meta_contract_guards.py b/tests/test_meta_contract_guards.py index 213366c..5ca6013 100644 --- a/tests/test_meta_contract_guards.py +++ b/tests/test_meta_contract_guards.py @@ -167,7 +167,6 @@ def test_env_example_default_values_match_settings_defaults(): "SQLITE_CHECK_SAME_THREAD": str(defaults.sqlite_check_same_thread).lower(), "UPLOAD_DIR": str(defaults.upload_dir), "PROMPT_DIR": str(defaults.prompt_dir), - "HOMEPAGE_DIR": str(defaults.homepage_dir), "DATABASE_BACKUP_DIR": str(defaults.database_backup_dir), "WORKER_MAX_RETRIES": str(defaults.worker_max_retries), "WORKER_PROVIDER_TIMEOUT_SECONDS": str(defaults.worker_provider_timeout_seconds), @@ -176,7 +175,7 @@ def test_env_example_default_values_match_settings_defaults(): "WORKER_FAIL_ON_FINISH_REASON_LENGTH": str(defaults.worker_fail_on_finish_reason_length).lower(), } active = _active_env_example_values() - path_like_keys = {"LOG_DIR", "UPLOAD_DIR", "PROMPT_DIR", "HOMEPAGE_DIR", "DATABASE_BACKUP_DIR"} + path_like_keys = {"LOG_DIR", "UPLOAD_DIR", "PROMPT_DIR", "DATABASE_BACKUP_DIR"} mismatches = { key: { "expected": _normalize_env_path_value(expected_value) if key in path_like_keys else expected_value, diff --git a/tests/tools/test_export_import_migration.py b/tests/tools/test_export_import_migration.py index 0f4b724..b9f4a97 100644 --- a/tests/tools/test_export_import_migration.py +++ b/tests/tools/test_export_import_migration.py @@ -6,6 +6,7 @@ from pathlib import Path from uuid import uuid4 from sqlalchemy import create_engine +from sqlalchemy import text from sqlalchemy import select from sqlmodel import SQLModel @@ -114,3 +115,70 @@ def test_export_import_migration_round_trips_db_and_uploads(tmp_path): copied_media_path = target_upload_dir / "documents" / str(document_id) / filename assert copied_media_path.read_bytes() == b"sample-image" + + +def test_export_import_migration_backfills_legacy_portraits_and_homepage_images(tmp_path): + source_db_path = tmp_path / "source-legacy.db" + target_db_path = tmp_path / "target-legacy.db" + source_upload_dir = tmp_path / "source_uploads" + target_upload_dir = tmp_path / "target_uploads" + bundle_dir = tmp_path / "bundle-legacy" + + source_db_url = sqlite_url_from_path(source_db_path) + target_db_url = sqlite_url_from_path(target_db_path) + + person_id = "11" * 16 + portrait_file = source_upload_dir / "persons" / "legacy" / "portrait.png" + portrait_file.parent.mkdir(parents=True, exist_ok=True) + portrait_file.write_bytes(b"portrait") + homepage_file = source_upload_dir / "homepage" / "banner.jpg" + homepage_file.parent.mkdir(parents=True, exist_ok=True) + homepage_file.write_bytes(b"homepage") + (source_upload_dir / "homepage" / "homepage.md").write_text("# Legacy Home", encoding="utf-8") + + engine = create_engine(source_db_url) + try: + with engine.begin() as connection: + connection.execute( + text( + 'create table "person" (' + "id char(32) primary key, " + "full_name varchar not null, " + "portrait_path varchar, " + "created_at datetime not null, " + "updated_at datetime not null" + ")" + ) + ) + connection.execute( + text( + 'insert into "person" (id, full_name, portrait_path, created_at, updated_at) ' + 'values (:id, :full_name, :portrait_path, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)' + ), + {"id": person_id, "full_name": "Legacy Portrait", "portrait_path": "persons/legacy/portrait.png"}, + ) + finally: + engine.dispose() + + export_bundle(source_db_url=source_db_url, source_upload_dir=source_upload_dir, bundle_dir=bundle_dir) + import_bundle(target_db_url=target_db_url, target_upload_dir=target_upload_dir, bundle_dir=bundle_dir) + + target_engine = create_engine(target_db_url) + try: + with target_engine.connect() as connection: + photos = connection.execute( + text('select person_id, path, is_primary from "photo" order by person_id is not null desc, created_at asc') + ).all() + assert len(photos) == 2 + person_photo = next(row for row in photos if row[0] is not None) + homepage_photo = next(row for row in photos if row[0] is None) + assert person_photo[2] == 1 + assert homepage_photo[2] == 1 + assert str(person_photo[1]).startswith("photos/") + assert str(homepage_photo[1]).startswith("photos/") + finally: + target_engine.dispose() + + assert (target_upload_dir / str(person_photo[1])).read_bytes() == b"portrait" + assert (target_upload_dir / str(homepage_photo[1])).read_bytes() == b"homepage" + assert (target_upload_dir / "homepage.md").read_text(encoding="utf-8") == "# Legacy Home" diff --git a/tests/ui/conftest.py b/tests/ui/conftest.py index 110445c..cd89e4a 100644 --- a/tests/ui/conftest.py +++ b/tests/ui/conftest.py @@ -30,6 +30,7 @@ from transcription.db.models import JobSource from transcription.db.models import JobSourceStatus from transcription.db.models import JobStatus from transcription.db.models import Person +from transcription.db.models import Photo from transcription.db.models import Source from transcription.db.models import Tag @@ -80,6 +81,7 @@ async def clear_ui_database( await session.exec(delete(Source)) await session.exec(delete(Job)) await session.exec(delete(Document)) + await session.exec(delete(Photo)) await session.exec(delete(Person)) await session.exec(delete(Tag)) await session.commit() diff --git a/tests/ui/test_homepage_store.py b/tests/ui/test_homepage_store.py index 9560467..1c59add 100644 --- a/tests/ui/test_homepage_store.py +++ b/tests/ui/test_homepage_store.py @@ -1,64 +1,36 @@ -"""Homepage storage resolves its root from settings rather than from `__file__`. +"""Homepage markdown storage tests.""" -The previous module derived its directory from ``Path(__file__).parents[3]``, -which could not be configured and resolved into the installed package directory -outside a source checkout. -""" - -import pytest +from pathlib import Path from transcription.config import Settings -from transcription.ui.homepage_store import homepage_dir -from transcription.ui.homepage_store import latest_homepage_image -from transcription.ui.homepage_store import list_homepage_images +from transcription.ui.homepage_store import homepage_markdown_path from transcription.ui.homepage_store import read_homepage_markdown from transcription.ui.homepage_store import save_homepage_markdown -from transcription.ui.homepage_store import store_homepage_image - -PNG_BYTES = bytes.fromhex( - "89504e470d0a1a0a0000000d49484452000000010000000108060000001f15c4890000000a49444154789c6360000002000100" - "05fe02fea7b1b8000000004945" -) + b"NDAE\xae\x42\x60\x82" -def _settings(tmp_path) -> Settings: - return Settings(openrouter_api_key="test-key-abc123", homepage_dir=tmp_path / "homepage") +def _settings(tmp_path: Path) -> Settings: + return Settings(openrouter_api_key="test-key-abc123", upload_dir=tmp_path / "uploads") -def test_homepage_dir_follows_the_configured_setting(tmp_path): +def test_markdown_path_uses_upload_dir_root(tmp_path): settings = _settings(tmp_path) - assert homepage_dir(settings) == tmp_path / "homepage" + assert homepage_markdown_path(settings) == tmp_path / "uploads" / "homepage.md" -def test_markdown_round_trips_through_the_configured_directory(tmp_path): +def test_markdown_round_trip_uses_upload_dir_root(tmp_path): settings = _settings(tmp_path) assert read_homepage_markdown(settings) == "" - save_homepage_markdown("# Archive", settings) - assert (tmp_path / "homepage" / "homepage.md").read_text(encoding="utf-8") == "# Archive" + assert (tmp_path / "uploads" / "homepage.md").read_text(encoding="utf-8") == "# Archive" assert read_homepage_markdown(settings) == "# Archive" -@pytest.mark.asyncio -async def test_images_are_stored_and_listed_from_the_configured_directory(tmp_path): - settings = _settings(tmp_path) - - assert list_homepage_images(settings) == [] - assert latest_homepage_image(settings) is None - - stored = await store_homepage_image(filename="banner.png", file_bytes=PNG_BYTES, settings=settings) - - assert stored.parent == tmp_path / "homepage" - assert list_homepage_images(settings) == [stored] - assert latest_homepage_image(settings) == stored - - -def test_two_configurations_do_not_share_storage(tmp_path): - first = Settings(openrouter_api_key="test-key-abc123", homepage_dir=tmp_path / "a") - second = Settings(openrouter_api_key="test-key-abc123", homepage_dir=tmp_path / "b") +def test_two_configurations_do_not_share_markdown_storage(tmp_path): + first = Settings(openrouter_api_key="test-key-abc123", upload_dir=tmp_path / "a") + second = Settings(openrouter_api_key="test-key-abc123", upload_dir=tmp_path / "b") save_homepage_markdown("first", first) diff --git a/tests/ui/test_people_page.py b/tests/ui/test_people_page.py index 7ad6507..b57d9d1 100644 --- a/tests/ui/test_people_page.py +++ b/tests/ui/test_people_page.py @@ -12,6 +12,7 @@ from transcription.db.models import Document from transcription.db.models import DocumentPerson from transcription.db.models import Person from transcription.db.models import PersonRole +from transcription.db.models import Photo from transcription.db.models import Source @@ -99,7 +100,6 @@ class TestPeoplePageRendering: death_date_raw="1992", death_place="Arlington", biography="Computer pioneer", - portrait_path="/images/grace.jpg", family_search_id="G8T4-MDQ", ) session.add(person) @@ -132,20 +132,25 @@ class TestPeoplePageRendering: assert "No linked documents yet." in response.text @pytest.mark.asyncio - async def test_person_detail_page_resolves_relative_portrait_path(self, app_client): + async def test_person_detail_page_resolves_relative_photo_path(self, app_client): app, client = app_client upload_dirs = {app.state.settings.upload_dir, get_settings().upload_dir} for upload_dir in upload_dirs: - portrait_file = upload_dir / "persons" / "person" / "seeded.png" - portrait_file.parent.mkdir(parents=True, exist_ok=True) - portrait_file.write_bytes(b"portrait") + photo_file = upload_dir / "photos" / "seeded.png" + photo_file.parent.mkdir(parents=True, exist_ok=True) + photo_file.write_bytes(b"portrait") async with session_scope() as session: - person = Person( - full_name="Portrait Person", - portrait_path="persons/person/seeded.png", - ) + person = Person(full_name="Portrait Person") session.add(person) + await session.flush() + session.add( + Photo( + person_id=person.id, + path="photos/seeded.png", + is_primary=True, + ) + ) await session.commit() person_id = str(person.id)