diff --git a/backend/alembic/versions/c7a8d9e1f2a3_create_author_and_book_author_tables.py b/backend/alembic/versions/c7a8d9e1f2a3_create_author_and_book_author_tables.py new file mode 100644 index 00000000..b72e6c72 --- /dev/null +++ b/backend/alembic/versions/c7a8d9e1f2a3_create_author_and_book_author_tables.py @@ -0,0 +1,104 @@ +"""create author and book_author tables and backfill + +Revision ID: c7a8d9e1f2a3 +Revises: e2f3a4b5c6d7 +Create Date: 2026-08-24 21:00:00 +""" + +import sqlalchemy as sa +from alembic import op + + +# revision identifiers, used by Alembic. +revision = "c7a8d9e1f2a3" +down_revision = "e2f3a4b5c6d7" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + bind = op.get_bind() + inspector = sa.inspect(bind) + + if not inspector.has_table("author"): + op.create_table( + "author", + sa.Column("id", sa.Integer(), nullable=False), + sa.Column("user_id", sa.Integer(), nullable=False), + sa.Column("name", sa.String(), nullable=False), + sa.Column("created_at", sa.DateTime(), nullable=False, server_default=sa.text("CURRENT_TIMESTAMP")), + sa.ForeignKeyConstraint(["user_id"], ["user.id"]), + sa.UniqueConstraint("user_id", "name", name="uq_author_user_id_name"), + sa.PrimaryKeyConstraint("id"), + ) + + existing_author_indexes = {idx["name"] for idx in inspector.get_indexes("author")} + if "ix_author_user_id" not in existing_author_indexes: + op.create_index("ix_author_user_id", "author", ["user_id"], unique=False) + if "ix_author_name" not in existing_author_indexes: + op.create_index("ix_author_name", "author", ["name"], unique=False) + + if not inspector.has_table("book_author"): + op.create_table( + "book_author", + sa.Column("book_id", sa.Integer(), nullable=False), + sa.Column("author_id", sa.Integer(), nullable=False), + sa.ForeignKeyConstraint(["book_id"], ["book.id"]), + sa.ForeignKeyConstraint(["author_id"], ["author.id"]), + sa.PrimaryKeyConstraint("book_id", "author_id"), + ) + + existing_book_author_indexes = {idx["name"] for idx in inspector.get_indexes("book_author")} + if "ix_book_author_book_id" not in existing_book_author_indexes: + op.create_index("ix_book_author_book_id", "book_author", ["book_id"], unique=False) + if "ix_book_author_author_id" not in existing_book_author_indexes: + op.create_index("ix_book_author_author_id", "book_author", ["author_id"], unique=False) + + # Backfill authors from the legacy book.author column. Each legacy value is + # treated as a single author name so names like "Asimov, Isaac" are preserved. + rows = bind.execute( + sa.text("SELECT id, user_id, author FROM book WHERE author IS NOT NULL AND author <> ''") + ).fetchall() + + for book_id, user_id, raw in rows: + name = " ".join(raw.strip().split()) + if not name: + continue + + author_id = bind.execute( + sa.text("SELECT id FROM author WHERE user_id = :user_id AND name = :name"), + {"user_id": user_id, "name": name}, + ).scalar() + if author_id is None: + author_id = bind.execute( + sa.text("INSERT INTO author (user_id, name) VALUES (:user_id, :name) RETURNING id"), + {"user_id": user_id, "name": name}, + ).scalar_one() + + bind.execute( + sa.text( + "INSERT OR IGNORE INTO book_author (book_id, author_id) VALUES (:book_id, :author_id)" + ), + {"book_id": book_id, "author_id": author_id}, + ) + + +def downgrade() -> None: + bind = op.get_bind() + inspector = sa.inspect(bind) + + if inspector.has_table("book_author"): + existing_book_author_indexes = {idx["name"] for idx in inspector.get_indexes("book_author")} + if "ix_book_author_author_id" in existing_book_author_indexes: + op.drop_index("ix_book_author_author_id", table_name="book_author") + if "ix_book_author_book_id" in existing_book_author_indexes: + op.drop_index("ix_book_author_book_id", table_name="book_author") + op.drop_table("book_author") + + if inspector.has_table("author"): + existing_author_indexes = {idx["name"] for idx in inspector.get_indexes("author")} + if "ix_author_name" in existing_author_indexes: + op.drop_index("ix_author_name", table_name="author") + if "ix_author_user_id" in existing_author_indexes: + op.drop_index("ix_author_user_id", table_name="author") + op.drop_table("author") \ No newline at end of file diff --git a/backend/alembic/versions/f1b2c3d4e5a6_drop_author_column_from_book.py b/backend/alembic/versions/f1b2c3d4e5a6_drop_author_column_from_book.py new file mode 100644 index 00000000..8b51979b --- /dev/null +++ b/backend/alembic/versions/f1b2c3d4e5a6_drop_author_column_from_book.py @@ -0,0 +1,42 @@ +"""drop author column from book + +Revision ID: f1b2c3d4e5a6 +Revises: c7a8d9e1f2a3 +Create Date: 2026-08-24 21:10:00 +""" + +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = "f1b2c3d4e5a6" +down_revision = "c7a8d9e1f2a3" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + with op.batch_alter_table("book", schema=None) as batch_op: + batch_op.drop_column("author") + + +def downgrade() -> None: + with op.batch_alter_table("book", schema=None) as batch_op: + batch_op.add_column(sa.Column("author", sa.String(), nullable=True, server_default="")) + + # Re-populate book.author from the relation tables before the author tables + # are dropped by the previous revision's downgrade. + bind = op.get_bind() + bind.execute( + sa.text( + """ + UPDATE book SET author = ( + SELECT group_concat(a.name, ', ') + FROM book_author ba + JOIN author a ON ba.author_id = a.id + WHERE ba.book_id = book.id + ) + """ + ) + ) \ No newline at end of file diff --git a/backend/app/models.py b/backend/app/models.py index b3a48178..425b2cc2 100644 --- a/backend/app/models.py +++ b/backend/app/models.py @@ -70,7 +70,6 @@ class Book(SQLModel, table=True): id: Optional[int] = Field(default=None, primary_key=True) title: str = Field(index=True) subtitle: Optional[str] = None - author: str = Field(default="", index=True) isbn: Optional[str] = Field(default=None) cover_url: Optional[str] = None publisher: Optional[str] = None @@ -128,6 +127,30 @@ class BookTag(SQLModel, table=True): tag_id: int = Field(foreign_key="tag.id", primary_key=True, index=True) +class Author(SQLModel, table=True): + """A user-specific author name that can be associated with books.""" + + __tablename__: str = "author" + __table_args__ = (sa.UniqueConstraint("user_id", "name", name="uq_author_user_id_name"),) + + id: Optional[int] = Field(default=None, primary_key=True) + user_id: int = Field(foreign_key="user.id", index=True) + name: str = Field(index=True) + created_at: datetime = Field( + default_factory=utcnow, + sa_column=Column(UtcDateTime, default=utcnow) + ) + + +class BookAuthor(SQLModel, table=True): + """Many-to-many association between books and authors.""" + + __tablename__: str = "book_author" + + book_id: int = Field(foreign_key="book.id", primary_key=True) + author_id: int = Field(foreign_key="author.id", primary_key=True, index=True) + + class User(SQLModel, table=True): """A user account.""" diff --git a/backend/app/routers/books.py b/backend/app/routers/books.py index e33a3e72..c6c5e8de 100644 --- a/backend/app/routers/books.py +++ b/backend/app/routers/books.py @@ -12,7 +12,7 @@ from app.auth import require_user from app.config import settings from app.database import get_session -from app.models import AcquisitionStatus, Book, BookTag, ReadingProgress, ReadingStatus, Tag, User +from app.models import AcquisitionStatus, Author, Book, BookAuthor, BookTag, ReadingProgress, ReadingStatus, Tag, User from app.schemas import ( BookCreate, BookListResponse, @@ -26,6 +26,13 @@ SuggestionList, TagCloudEntry, ) +from app.services.authors import ( + cleanup_orphan_authors, + join_authors, + load_authors_batch, + resolve_authors_payload, + sync_book_authors, +) from app.services.cover_storage import ( delete_cover_file, local_cover_filename, @@ -130,11 +137,14 @@ def _raise_integrity_conflict(exc: IntegrityError) -> None: raise -def _build_book_read_with_tags(book: Book, tags_text: str | None) -> BookRead: - """Build a BookRead from a Book model with a pre-resolved tags string.""" +def _build_book_read_with_tags(book: Book, tags_text: str | None, authors: list[str] | None = None) -> BookRead: + """Build a BookRead from a Book model with pre-resolved tags and authors.""" payload = book.model_dump() payload.pop("user_id", None) payload["tags"] = tags_text + authors = authors or [] + payload["authors"] = authors + payload["author"] = join_authors(authors) return BookRead.model_validate(payload) @@ -227,9 +237,10 @@ def list_books( logger.debug("list_books — returning %d/%d book(s)", len(books), total) book_ids = [b.id for b in books if b.id is not None] book_tags_map = load_tags_batch(session, book_ids) if book_ids else {} + book_authors_map = load_authors_batch(session, book_ids) if book_ids else {} return BookListResponse( books=[ - _build_book_read_with_tags(book, book_tags_map.get(book.id)) + _build_book_read_with_tags(book, book_tags_map.get(book.id), book_authors_map.get(book.id)) for book in books ], total=total, @@ -348,10 +359,22 @@ def suggest_authors( current_user: User = Depends(require_user), session: Session = Depends(get_session), ) -> SuggestionList: - """Autocomplete author names from the user's existing books.""" + """Autocomplete author names from the user's existing authors.""" assert current_user.id is not None - suggestions = _suggest_field(session, current_user.id, "author", q, limit) - return SuggestionList(suggestions=suggestions) + if not q.strip(): + return SuggestionList(suggestions=[]) + pattern = f"%{_escape_like(q)}%" + rows = session.exec( + select(Author.name) + .where( + Author.user_id == current_user.id, + col(Author.name).ilike(pattern, escape="\\"), + ) + .distinct() + .order_by(Author.name) + .limit(limit) + ).all() + return SuggestionList(suggestions=list(rows)) @router.get("/suggestions/publishers", response_model=SuggestionList) @@ -420,6 +443,8 @@ async def create_book( book_data["language"] = _normalize_language(book_data.get("language")) book_data["cover_url"] = cover_url book_data.pop("tags", None) + book_data.pop("author", None) + book_data.pop("authors", None) book_data["user_id"] = current_user.id _validate_dates(book_data) book = Book.model_validate(book_data) @@ -430,6 +455,8 @@ async def create_book( session.rollback() _raise_integrity_conflict(exc) sync_book_tags(session, current_user.id, book.id or 0, book_in.tags) + names = resolve_authors_payload(book_in.author, book_in.authors) or [] + sync_book_authors(session, current_user.id, book.id or 0, names) try: session.commit() except IntegrityError as exc: @@ -475,6 +502,10 @@ async def update_book( update_data["language"] = _normalize_language(update_data.get("language")) tags_provided = "tags" in update_data tags_raw = update_data.pop("tags", None) if tags_provided else None + authors_payload = resolve_authors_payload( + update_data.pop("author", None), update_data.pop("authors", None) + ) + authors_provided = authors_payload is not None target_status = update_data.get("reading_status", book.reading_status) # Download external cover URL -> local file. @@ -520,6 +551,10 @@ async def update_book( assert book.id is not None sync_book_tags(session, current_user.id, book.id, tags_raw) cleanup_orphan_tags(session, current_user.id) + if authors_provided: + assert book.id is not None + sync_book_authors(session, current_user.id, book.id, authors_payload) + cleanup_orphan_authors(session, current_user.id) try: session.commit() except IntegrityError as exc: @@ -676,11 +711,14 @@ def delete_book( for link in session.exec(select(BookTag).where(BookTag.book_id == book.id)).all(): session.delete(link) + for link in session.exec(select(BookAuthor).where(BookAuthor.book_id == book.id)).all(): + session.delete(link) for entry in session.exec( select(ReadingProgress).where(ReadingProgress.book_id == book.id) ).all(): session.delete(entry) session.delete(book) cleanup_orphan_tags(session, current_user.id) + cleanup_orphan_authors(session, current_user.id) session.commit() logger.info("Deleted book id=%s", book_id) diff --git a/backend/app/routers/data.py b/backend/app/routers/data.py index 94cb0b23..876757df 100644 --- a/backend/app/routers/data.py +++ b/backend/app/routers/data.py @@ -5,7 +5,7 @@ from datetime import datetime from typing import Any -from fastapi import APIRouter, Depends, File, HTTPException, Response, UploadFile +from fastapi import APIRouter, Depends, File, Form, HTTPException, Response, UploadFile from fastapi.responses import StreamingResponse from sqlalchemy.exc import IntegrityError from sqlmodel import Session, col, select @@ -34,6 +34,7 @@ from app.services.data_import import ( BOOK_IMPORT_FIELDS, PREDEFINED_MAPPINGS, + canonicalize_mapping, compute_schema_fingerprint, execute_import, get_predefined_mapping, @@ -54,7 +55,7 @@ def _mapping_read(model: ImportMapping) -> DataImportMappingRead: id=model.id or 0, name=model.name, source_fields=json.loads(model.source_fields_json), - mapping={k: ImportFieldConfig(**v) for k, v in raw_mapping.items()}, + mapping=canonicalize_mapping({k: ImportFieldConfig(**v) for k, v in raw_mapping.items()}), created_at=model.created_at, updated_at=model.updated_at, is_predefined=False, @@ -87,9 +88,13 @@ def export_data( @router.post("/import/parse", response_model=DataImportParseResponse) async def parse_import_file( file: UploadFile = File(...), + delimiter: str = Form(","), current_user: User = Depends(require_user), ) -> DataImportParseResponse: - """Parse an uploaded CSV or JSON import file and return field info and samples.""" + """Parse an uploaded CSV or JSON import file and return field info and samples. + + ``delimiter`` is the single-character CSV field separator (ignored for JSON). + """ assert current_user.id is not None allowed_content_types = { "text/csv", @@ -101,7 +106,7 @@ async def parse_import_file( if file.content_type and file.content_type not in allowed_content_types: raise HTTPException(status_code=415, detail="Unsupported upload content type. Use CSV or JSON files.") try: - payload = parse_upload(await file.read(), file.filename or "upload", current_user.id) + payload = parse_upload(await file.read(), file.filename or "upload", current_user.id, delimiter) except (ValueError, json.JSONDecodeError) as exc: raise HTTPException(status_code=400, detail=str(exc)) from exc return DataImportParseResponse.model_validate(payload) @@ -143,7 +148,7 @@ def save_import_mapping( ) ).first() - mapping_dict = {k: v.model_dump() for k, v in body.mapping.items()} + mapping_dict = {k: v.model_dump() for k, v in canonicalize_mapping(body.mapping).items()} if existing: existing.source_fields_json = json.dumps(body.source_fields) existing.mapping_json = json.dumps(mapping_dict) @@ -226,7 +231,7 @@ def get_import_mapping( id=mapping_id, name=str(pm.get("name", "")), source_fields=list(raw_sources), - mapping={k: ImportFieldConfig(**v) for k, v in raw_mapping.items()}, + mapping=canonicalize_mapping({k: ImportFieldConfig(**v) for k, v in raw_mapping.items()}), created_at=datetime(2000, 1, 1), updated_at=datetime(2000, 1, 1), is_predefined=True, diff --git a/backend/app/routers/hygiene.py b/backend/app/routers/hygiene.py index 8ea294ba..46e509cd 100644 --- a/backend/app/routers/hygiene.py +++ b/backend/app/routers/hygiene.py @@ -10,7 +10,7 @@ from app.auth import require_user from app.config import settings from app.database import get_session -from app.models import Book, User +from app.models import Book, BookAuthor, User from app.schemas import ( HygieneAttribute, HygieneBatchUpdateRequest, @@ -18,8 +18,9 @@ HygieneMissingBook, HygieneMissingResponse, ) +from app.services.authors import cleanup_orphan_authors, join_authors, load_authors_batch, sync_book_authors from app.services.cover_import import import_cover_from_url, is_external_cover_url -from app.services.tags import build_book_read +from app.services.tags import load_tags_batch logger = logging.getLogger(__name__) @@ -30,18 +31,24 @@ def _missing_condition(attr: HygieneAttribute): """Return a SQLAlchemy filter condition for a given attribute being missing.""" - col = getattr(Book, attr.value) if attr == HygieneAttribute.author: - return or_(col == "", col.is_(None)) + return ~col(Book.id).in_( + select(BookAuthor.book_id).where(BookAuthor.book_id == col(Book.id)) + ) + col_expr = getattr(Book, attr.value) if attr == HygieneAttribute.page_count: - return or_(col == 0, col.is_(None)) - return col.is_(None) + return or_(col_expr == 0, col_expr.is_(None)) + return col_expr.is_(None) -def _compute_missing_attributes(book: Book) -> list[HygieneAttribute]: +def _compute_missing_attributes(book: Book, author_names: list[str]) -> list[HygieneAttribute]: """Return the list of hygiene attributes that are missing for a given book.""" missing: list[HygieneAttribute] = [] for attr in HygieneAttribute: + if attr == HygieneAttribute.author: + if not author_names: + missing.append(attr) + continue val = getattr(book, attr.value) is_missing = val is None or val == "" or val == 0 if is_missing: @@ -102,21 +109,26 @@ def list_missing( ).all() hygiene_books = [] + book_ids = [book.id for book in books if book.id is not None] + tags_map = load_tags_batch(session, book_ids) + authors_map = load_authors_batch(session, book_ids) for book in books: - br = build_book_read(session, book) - missing_attrs = _compute_missing_attributes(book) + assert book.id is not None + author_names = authors_map.get(book.id, []) + missing_attrs = _compute_missing_attributes(book, author_names) hygiene_books.append(HygieneMissingBook( - id=br.id, - title=br.title, - author=br.author, - isbn=br.isbn, - publisher=br.publisher, - published_year=br.published_year, - blurb=br.blurb, - language=br.language, - subtitle=br.subtitle, - page_count=br.page_count or 0, - cover_url=br.cover_url, + id=book.id, + title=book.title, + author=join_authors(author_names), + authors=author_names, + isbn=book.isbn, + publisher=book.publisher, + published_year=book.published_year, + blurb=book.blurb, + language=book.language, + subtitle=book.subtitle, + page_count=book.page_count or 0, + cover_url=book.cover_url, missing_attributes=[a for a in requested if a in missing_attrs], )) @@ -233,8 +245,18 @@ async def batch_update( skipped_ids: list[int] = [] to_update_ids: list[int] = [] + current_authors_map = ( + load_authors_batch(session, [b.id for b in books if b.id is not None]) + if req.field == HygieneAttribute.author + else {} + ) for book in books: - current_val = getattr(book, req.field.value) + if req.field == HygieneAttribute.author: + # Compare against the current author set so multi-author books are + # only skipped when they already contain exactly the target author. + current_val = ", ".join(current_authors_map.get(book.id, [])) or None + else: + current_val = getattr(book, req.field.value) if current_val == req.value: skipped_ids.append(book.id) # ty: ignore[invalid-argument-type] else: @@ -242,19 +264,36 @@ async def batch_update( updated = 0 if to_update_ids: - try: - stmt = ( - sqlmodel_update(Book) - .where(col(Book.id).in_(to_update_ids)) - .values({req.field.value: req.value}) - ) - updated = len(to_update_ids) - session.exec(stmt) - session.commit() - except Exception: - session.rollback() - logger.exception("Batch update failed for %d books", len(to_update_ids)) - raise HTTPException(status_code=500, detail="Batch update failed due to a database error") + if req.field == HygieneAttribute.author: + # Authors live in a relation table; a bulk SQL update does not work. + assert current_user.id is not None + for book in books: + if book.id not in set(to_update_ids): + continue + assert book.id is not None + sync_book_authors(session, current_user.id, book.id, [str(req.value)]) + updated += 1 + cleanup_orphan_authors(session, current_user.id) + try: + session.commit() + except Exception: + session.rollback() + logger.exception("Batch author update failed for %d books", len(to_update_ids)) + raise HTTPException(status_code=500, detail="Batch update failed due to a database error") + else: + try: + stmt = ( + sqlmodel_update(Book) + .where(col(Book.id).in_(to_update_ids)) + .values({req.field.value: req.value}) + ) + updated = len(to_update_ids) + session.exec(stmt) + session.commit() + except Exception: + session.rollback() + logger.exception("Batch update failed for %d books", len(to_update_ids)) + raise HTTPException(status_code=500, detail="Batch update failed due to a database error") return HygieneBatchUpdateResponse( updated=updated, diff --git a/backend/app/routers/import_.py b/backend/app/routers/import_.py index d094bdc9..acde920c 100644 --- a/backend/app/routers/import_.py +++ b/backend/app/routers/import_.py @@ -16,6 +16,7 @@ from app.models import Book, User from app.schemas import BookImportCandidate, BookImportRequest, BookRead from app.services import book_import +from app.services.authors import resolve_authors_payload, sync_book_authors from app.services.cover_storage import download_cover from app.services.tags import build_book_read, sync_book_tags @@ -145,7 +146,6 @@ async def import_book( book = Book( title=c.title, subtitle=c.subtitle, - author=c.author or "", isbn=c.isbn, cover_url=cover_url, publisher=c.publisher, @@ -164,6 +164,8 @@ async def import_book( session.rollback() _raise_integrity_conflict(exc) sync_book_tags(session, current_user.id, book.id or 0, c.tags) + names = resolve_authors_payload(c.author, c.authors) or [] + sync_book_authors(session, current_user.id, book.id or 0, names) try: session.commit() except IntegrityError as exc: diff --git a/backend/app/routers/profile.py b/backend/app/routers/profile.py index eddff7e4..b838b98e 100644 --- a/backend/app/routers/profile.py +++ b/backend/app/routers/profile.py @@ -158,8 +158,8 @@ def reset_data( raise logger.warning( - "User %s reset personal data: books=%s tags=%s progress_entries=%s", - current_user.id, deleted.books, deleted.tags, deleted.progress_entries, + "User %s reset personal data: books=%s tags=%s authors=%s progress_entries=%s", + current_user.id, deleted.books, deleted.tags, deleted.authors, deleted.progress_entries, ) return DataResetResponse( @@ -167,6 +167,7 @@ def reset_data( deleted=DataResetDeleted( books=deleted.books, tags=deleted.tags, + authors=deleted.authors, progress_entries=deleted.progress_entries, ), ) diff --git a/backend/app/routers/statistics.py b/backend/app/routers/statistics.py index c14d2db4..5be6f446 100644 --- a/backend/app/routers/statistics.py +++ b/backend/app/routers/statistics.py @@ -14,7 +14,8 @@ from app.auth import require_user from app.database import get_session -from app.models import AcquisitionStatus, Book, ReadingProgress, ReadingStatus, User, UserSettings +from app.models import AcquisitionStatus, Author, Book, BookAuthor, ReadingProgress, ReadingStatus, User, UserSettings +from app.services.authors import join_authors, load_authors_batch from app.schemas import ( AcquisitionStatusDistribution, DailyPages, @@ -349,6 +350,10 @@ def get_statistics( current_year = now.year books = list(session.exec(select(Book).where(Book.user_id == current_user.id)).all()) + total_authors = session.exec( + select(func.count()).select_from(Author).where(Author.user_id == current_user.id) + ).one() + status_counts = Counter(book.reading_status for book in books) status_distribution = StatusDistribution( want_to_read=status_counts.get(ReadingStatus.want_to_read, 0), @@ -509,25 +514,37 @@ def get_statistics( else: books_finished_per_year = [] - author_counts: Counter[str] = Counter() - for book in books: - if book.author and book.author.strip(): - author_counts[book.author.strip()] += 1 + author_count_label = func.count(func.distinct(BookAuthor.book_id)).label("cnt") + author_count_rows = session.exec( + select(Author.name, author_count_label) + .join(BookAuthor, col(BookAuthor.author_id) == col(Author.id)) + .join(Book, col(Book.id) == col(BookAuthor.book_id)) + .where(Book.user_id == current_user.id) + .group_by(col(Author.id)) + .order_by(author_count_label.desc(), col(Author.name).asc()) + .limit(3) + ).all() + author_counts = Counter({name: count for name, count in author_count_rows}) top_authors: list[TopAuthor] = [] if author_counts: - author_items = sorted(author_counts.items(), key=lambda item: item[0].lower()) - top_author_counts = sorted(author_items, key=lambda item: item[1], reverse=True)[:3] + top_author_counts = author_counts.most_common(3) top_author_names = [name for name, _ in top_author_counts] covers_by_author: dict[str, list[TopAuthorCover]] = {} for author_name in top_author_names: max_slots = min(5, author_counts[author_name]) + book_ids_with_author = select(BookAuthor.book_id).join( + Author, col(Author.id) == col(BookAuthor.author_id) + ).where( + Author.user_id == current_user.id, + Author.name == author_name, + ) cover_rows = session.exec( select(Book.id, Book.title, Book.reading_status, Book.cover_url) .where( Book.user_id == current_user.id, - Book.author == author_name, + col(Book.id).in_(book_ids_with_author), col(Book.cover_url).is_not(None), ) .order_by(col(Book.id)) @@ -544,7 +561,7 @@ def get_statistics( select(Book.id, Book.title, Book.reading_status, Book.cover_url) .where( Book.user_id == current_user.id, - Book.author == author_name, + col(Book.id).in_(book_ids_with_author), col(Book.cover_url).is_(None), ) .order_by(col(Book.id)) @@ -573,28 +590,36 @@ def get_statistics( average_rating = round(mean(rating_values), 2) if rating_values else None rated_books = [b for b in books if b.rating is not None] + rated_book_ids = [b.id for b in rated_books if b.id is not None] + rated_authors_map = load_authors_batch(session, rated_book_ids) def _rating_sort_key(book: Book) -> tuple[int, float]: assert book.rating is not None return (book.rating, -(book.date_added or datetime.min).timestamp()) + # Top rated: highest rating first; ties broken by newest-added first. top_rated_books = [] - for b in sorted(rated_books, key=_rating_sort_key): + for b in sorted(rated_books, key=lambda x: (-_rating_sort_key(x)[0], _rating_sort_key(x)[1])): assert b.id is not None assert b.rating is not None + author_names = rated_authors_map.get(b.id, []) top_rated_books.append( - TopRatedBook(book_id=b.id, title=b.title or "", author=b.author, rating=b.rating, reading_status=b.reading_status, cover_url=b.cover_url) + TopRatedBook(book_id=b.id, title=b.title or "", author=join_authors(author_names), authors=author_names, rating=b.rating, reading_status=b.reading_status, cover_url=b.cover_url) ) + # Worst rated: lowest rating first; ties broken by newest-added first. worst_rated_books = [] - for b in sorted(rated_books, key=lambda x: (-_rating_sort_key(x)[0], -_rating_sort_key(x)[1])): + for b in sorted(rated_books, key=_rating_sort_key): assert b.id is not None assert b.rating is not None + author_names = rated_authors_map.get(b.id, []) worst_rated_books.append( - TopRatedBook(book_id=b.id, title=b.title or "", author=b.author, rating=b.rating, reading_status=b.reading_status, cover_url=b.cover_url) + TopRatedBook(book_id=b.id, title=b.title or "", author=join_authors(author_names), authors=author_names, rating=b.rating, reading_status=b.reading_status, cover_url=b.cover_url) ) return StatisticsResponse( + total_books=len(books), + total_authors=total_authors, avg_books_per_month=avg_books_per_month, busiest_month=busiest_month, busiest_month_count=busiest_month_count, diff --git a/backend/app/schemas.py b/backend/app/schemas.py index 4d127e69..c5ec3c06 100644 --- a/backend/app/schemas.py +++ b/backend/app/schemas.py @@ -1,11 +1,12 @@ """Pydantic / SQLModel request and response schemas for the API.""" -from typing import Optional +from typing import Optional, Any from datetime import datetime from enum import Enum from typing import Literal -from pydantic import BaseModel, field_validator +from pydantic import BaseModel, field_validator, model_validator +import pydantic from sqlmodel import Field, SQLModel from sqlmodel._compat import SQLModelConfig @@ -39,9 +40,28 @@ class ReadingProgressLatest(SQLModel): class BookCreate(SQLModel): """Request body to create a new book.""" + + @model_validator(mode="before") + @classmethod + def require_author(cls, data: Any) -> Any: + if not isinstance(data, dict): + return data + author = data.get("author") + authors = data.get("authors") + has_author = bool( + (isinstance(author, str) and author.strip()) + or (isinstance(authors, list) and any(isinstance(a, str) and a.strip() for a in authors)) + ) + if not has_author: + raise ValueError( + "A book must have at least one author: provide either 'author' or 'authors'." + ) + return data + title: str subtitle: Optional[str] = None - author: str + author: Optional[str] = pydantic.Field(default=None, deprecated=True) + authors: Optional[list[str]] = None isbn: Optional[str] = None cover_url: Optional[str] = None publisher: Optional[str] = None @@ -62,7 +82,8 @@ class BookUpdate(SQLModel): """Request body to partially update a book.""" title: Optional[str] = None subtitle: Optional[str] = None - author: Optional[str] = None + author: Optional[str] = pydantic.Field(default=None, deprecated=True) + authors: Optional[list[str]] = None isbn: Optional[str] = None cover_url: Optional[str] = None publisher: Optional[str] = None @@ -83,7 +104,8 @@ class BookImportCandidate(SQLModel): """A book result from an external API, not yet persisted locally.""" title: str subtitle: Optional[str] = None - author: Optional[str] = None + author: Optional[str] = pydantic.Field(default=None, deprecated=True) + authors: Optional[list[str]] = None isbn: Optional[str] = None cover_url: Optional[str] = None publisher: Optional[str] = None @@ -129,7 +151,8 @@ class BookRead(SQLModel): id: int title: str subtitle: Optional[str] - author: Optional[str] + author: Optional[str] = pydantic.Field(deprecated=True) + authors: list[str] = [] isbn: Optional[str] cover_url: Optional[str] publisher: Optional[str] @@ -245,7 +268,8 @@ class TopRatedBook(SQLModel): """A book appearing in top/worst rated lists.""" book_id: int title: str - author: Optional[str] + author: Optional[str] = pydantic.Field(deprecated=True) + authors: list[str] = [] rating: int reading_status: ReadingStatus cover_url: Optional[str] @@ -253,6 +277,8 @@ class TopRatedBook(SQLModel): class StatisticsResponse(SQLModel): """Full statistics dashboard response.""" + total_books: int + total_authors: int avg_books_per_month: Optional[float] busiest_month: Optional[str] busiest_month_count: Optional[int] @@ -385,6 +411,7 @@ class DataResetDeleted(SQLModel): """Counts of deleted items after a data reset.""" books: int tags: int + authors: int progress_entries: int @@ -481,7 +508,8 @@ class HygieneMissingBook(SQLModel): """A single book in the data-hygiene listing with its missing fields annotated.""" id: int title: str - author: str | None + author: str | None = pydantic.Field(deprecated=True) + authors: list[str] = [] isbn: str | None publisher: str | None published_year: int | None @@ -613,8 +641,8 @@ class DataImportValidateResponse(SQLModel): class DataImportPreviewRow(SQLModel): """A single row in the import preview.""" row_number: int - source: dict[str, str] - transformed: dict[str, Optional[str]] + source: dict[str, Any] + transformed: dict[str, Any] errors: list[str] diff --git a/backend/app/services/authors.py b/backend/app/services/authors.py new file mode 100644 index 00000000..396e4795 --- /dev/null +++ b/backend/app/services/authors.py @@ -0,0 +1,232 @@ +"""Author parsing, synchronization, and query helpers. + +Authors are stored per-user in the ``author`` table and linked to books through +the ``book_author`` relation table, mirroring how tags work. The public API keeps +an ``author`` joined-string field for backward compatibility alongside the new +``authors`` list field. +""" + +import re + +from sqlmodel import Session, col, select + +from app.models import Author, Book, BookAuthor +from app.time_utils import utcnow + + +def normalize_author_name(name: str) -> str | None: + """Trim whitespace and collapse internal whitespace in an author name.""" + cleaned = " ".join(name.strip().split()) + return cleaned or None + + +def normalize_author_list(names: list[str] | None) -> list[str]: + """Normalize a list of author names from the API or UI. + + Trims whitespace, collapses internal whitespace, and deduplicates + case-insensitively while preserving the first-cased spelling seen. + """ + if not names: + return [] + seen: set[str] = set() + result: list[str] = [] + for name in names: + cleaned = normalize_author_name(name) + if cleaned is None: + continue + key = cleaned.lower() + if key in seen: + continue + seen.add(key) + result.append(cleaned) + return result + + +def parse_legacy_author(raw: str | None) -> list[str]: + """Parse a legacy comma-separated author string (for BookCreate/Update.author). + + Mirrors tag parsing: split on commas, trim, collapse whitespace, deduplicate. + """ + if not raw: + return [] + seen: set[str] = set() + parsed: list[str] = [] + for piece in raw.split(","): + cleaned = normalize_author_name(piece) + if cleaned is None: + continue + key = cleaned.lower() + if key in seen: + continue + seen.add(key) + parsed.append(cleaned) + return parsed + + +def parse_authors(raw: str | list[str] | None) -> list[str]: + """Normalize an author value from a file import. + + A list contributes one author per entry. A string is passed through + ``split_author_string``: it stays a single author unless it contains a + ``;``, `` & ``, or `` and `` separator (how the CSV export writes the + ``authors`` column), so it can round-trip. Commas inside a name like + ``"Asimov, Isaac"`` are always preserved. The result is deduplicated + case-insensitively while preserving the first spelling seen. + """ + if raw is None: + return [] + pieces = split_author_string(raw) if isinstance(raw, str) else raw + seen: set[str] = set() + result: list[str] = [] + for piece in pieces: + name = normalize_author_name(piece) + if name is None: + continue + key = name.lower() + if key in seen: + continue + seen.add(key) + result.append(name) + return result + + +def resolve_authors_payload( + author: str | None = None, + authors: list[str] | None = None, +) -> list[str] | None: + """Resolve the hybrid create/update payload. + + * If ``authors`` is provided (including ``[]``), use it. + * Otherwise fall back to parsing the legacy ``author`` string. + * If neither is provided, return None so the caller can treat it as + "not provided" (for updates) or "empty" (for creates). + """ + if authors is not None: + return normalize_author_list(authors) + if author is not None: + return parse_legacy_author(author) + return None + + +# Conservative separator regex used only when an external source returns a +# single delimited string. Never splits on commas. +_AUTHOR_SEPARATOR_RE = re.compile( + r"\s*;\s*|\s+&\s+|\s+and\s+", + flags=re.IGNORECASE, +) + + +def split_author_string(value: str | None) -> list[str]: + """Split a single author string from an external source. + + Only semicolons, ``" & "``, and ``" and "`` are treated as separators, so + names containing commas (e.g. ``"Asimov, Isaac"``) are preserved. + If no known separator is found, the whole string is returned as one author. + """ + if not value: + return [] + if not _AUTHOR_SEPARATOR_RE.search(value): + name = normalize_author_name(value) + return [name] if name else [] + parts = _AUTHOR_SEPARATOR_RE.split(value) + return [name for name in (normalize_author_name(p) for p in parts) if name] + + +def sync_book_authors( + session: Session, user_id: int, book_id: int, names: list[str] | None +) -> None: + """Set the authors for a book to *names*, creating Author rows as needed.""" + parsed = normalize_author_list(names) + + existing_links = list( + session.exec(select(BookAuthor).where(BookAuthor.book_id == book_id)).all() + ) + existing_author_ids = {link.author_id for link in existing_links} + + if not parsed: + for link in existing_links: + session.delete(link) + return + + existing_authors = list( + session.exec( + select(Author).where(Author.user_id == user_id, col(Author.name).in_(parsed)) + ).all() + ) + name_to_author = {author.name: author for author in existing_authors} + + for name in parsed: + if name in name_to_author: + continue + author = Author(user_id=user_id, name=name, created_at=utcnow()) + session.add(author) + session.flush() + name_to_author[name] = author + + target_ids: set[int] = set() + for name in parsed: + author_id = name_to_author[name].id + if author_id is not None: + target_ids.add(author_id) + + for author_id in target_ids - existing_author_ids: + session.add(BookAuthor(book_id=book_id, author_id=author_id)) + + for link in existing_links: + if link.author_id not in target_ids: + session.delete(link) + + +def cleanup_orphan_authors(session: Session, user_id: int) -> None: + """Delete authors of this user that are no longer linked to any book.""" + linked_ids = set( + session.exec( + select(BookAuthor.author_id).where( + col(BookAuthor.author_id).in_( + select(Author.id).where(Author.user_id == user_id) + ) + ) + ).all() + ) + authors = list(session.exec(select(Author).where(Author.user_id == user_id)).all()) + for author in authors: + if author.id not in linked_ids: + session.delete(author) + + +def authors_list_for_book(session: Session, book_id: int | None) -> list[str]: + """Return ordered author names for a book.""" + if book_id is None: + return [] + names = list( + session.exec( + select(Author.name) + .join(BookAuthor, col(BookAuthor.author_id) == col(Author.id)) + .where(BookAuthor.book_id == book_id) + .order_by(col(Author.name).asc()) + ).all() + ) + return names + + +def load_authors_batch(session: Session, book_ids: list[int]) -> dict[int, list[str]]: + """Batch-load author lists for many book IDs.""" + if not book_ids: + return {} + rows = session.exec( + select(BookAuthor.book_id, Author.name) + .join(Author, col(Author.id) == col(BookAuthor.author_id)) + .where(col(BookAuthor.book_id).in_(book_ids)) + .order_by(col(BookAuthor.book_id), col(Author.name).asc()) + ).all() + result: dict[int, list[str]] = {} + for book_id, name in rows: + result.setdefault(book_id, []).append(name) + return {bid: names for bid, names in result.items()} + + +def join_authors(names: list[str] | None) -> str | None: + """Join author names for the legacy ``author`` response field.""" + if not names: + return None + return ", ".join(names) \ No newline at end of file diff --git a/backend/app/services/book_import.py b/backend/app/services/book_import.py index d1c34d16..13b3be53 100644 --- a/backend/app/services/book_import.py +++ b/backend/app/services/book_import.py @@ -364,7 +364,7 @@ async def _search_open_library( def map_open_library(doc: dict) -> BookImportCandidate: """Map a single Open Library search doc to BookImportCandidate.""" # Authors: list of strings - authors = doc.get("author_name") or [] + authors = [a for a in (doc.get("author_name") or []) if a] author = ", ".join(authors) if authors else None # ISBN: first entry of the list, prefer ISBN-13 (length 13) @@ -391,6 +391,7 @@ def map_open_library(doc: dict) -> BookImportCandidate: title=doc["title"], subtitle=doc.get("subtitle") or None, author=author, + authors=authors, isbn=isbn, cover_url=cover_url, publisher=publisher, @@ -561,7 +562,7 @@ def map_google_books(item: dict) -> BookImportCandidate: vi = item.get("volumeInfo", {}) # Authors - authors: list[str] = vi.get("authors") or [] + authors: list[str] = [a for a in (vi.get("authors") or []) if a] author = ", ".join(authors) if authors else None # ISBN: prefer ISBN_13 @@ -596,6 +597,7 @@ def map_google_books(item: dict) -> BookImportCandidate: title=vi["title"], subtitle=vi.get("subtitle") or None, author=author, + authors=authors, isbn=isbn, cover_url=cover_url, publisher=vi.get("publisher"), @@ -770,12 +772,12 @@ def map_hardcover(edition: dict) -> BookImportCandidate | None: return None contributions = edition.get("contributions") or [] - author = None + authors: list[str] = [] for c in contributions: - author_name = c.get("author", {}).get("name") + author_name = (c.get("author") or {}).get("name") if author_name: - author = author_name - break + authors.append(author_name) + author = ", ".join(authors) if authors else None isbn = edition.get("isbn_13") or None @@ -811,6 +813,7 @@ def map_hardcover(edition: dict) -> BookImportCandidate | None: title=title, subtitle=edition.get("subtitle") or None, author=author, + authors=authors, isbn=isbn, cover_url=cover_url, publisher=publisher, diff --git a/backend/app/services/data_export.py b/backend/app/services/data_export.py index c6f047f2..18e97206 100644 --- a/backend/app/services/data_export.py +++ b/backend/app/services/data_export.py @@ -12,14 +12,16 @@ from app._build_info import __git_sha__, __version__ from app.models import Book, BookTag, ReadingProgress, Tag, User +from app.services.authors import authors_list_for_book from app.time_utils import utcnow from app.services.cover_storage import local_cover_filename, resolve_cover_path -from app.services.tags import tags_text_for_book +from app.services.tags import tags_list_for_book BOOK_CSV_FIELDS: list[str] = [ "title", "subtitle", "author", + "authors", "isbn", "publisher", "published_year", @@ -48,18 +50,31 @@ def _serialize_datetime(value: datetime | None) -> str | None: return value.astimezone(timezone.utc).isoformat().replace("+00:00", "Z") -def _book_to_dict(session: Session, book: Book) -> dict: - """Convert a Book model to a flat export dict.""" +def _book_to_dict(session: Session, book: Book, export_format: str) -> dict: + """Convert a Book model to a flat export dict. + + JSON exports mirror the API shape: ``author`` is the joined string (using + ``; `` so it round-trips through the adaptive import), ``authors`` is the + name list, and ``tags`` is a list of tag names. CSV has no list type: both + the ``author`` and ``authors`` columns use ``; `` (so they round-trip + through ``parse_authors``), and ``tags`` is a comma-separated string. + """ + authors = authors_list_for_book(session, book.id) + tags = tags_list_for_book(session, book.id) + author_value = "; ".join(authors) if authors else None + tags_value = tags if export_format == "json" else (", ".join(tags) if tags else None) + authors_value = authors if export_format == "json" else ("; ".join(authors) if authors else None) return { "title": book.title, "subtitle": book.subtitle, - "author": book.author, + "author": author_value, + "authors": authors_value, "isbn": book.isbn, "publisher": book.publisher, "published_year": book.published_year, "page_count": book.page_count, "language": book.language, - "tags": tags_text_for_book(session, book.id) if book.id else None, + "tags": tags_value, "notes": book.notes, "blurb": book.blurb, "rating": book.rating, @@ -150,7 +165,7 @@ def build_export_zip( book_titles = {book.id: book.title for book in books if book.id is not None} - books_rows = [_book_to_dict(session, book) for book in books] + books_rows = [_book_to_dict(session, book, export_format) for book in books] progress_rows = [_progress_to_dict(entry, book_titles) for entry in progress_entries] tag_rows = [_tag_to_dict(tag, tag_counts) for tag in tags] diff --git a/backend/app/services/data_import.py b/backend/app/services/data_import.py index 1b353cd1..89144b49 100644 --- a/backend/app/services/data_import.py +++ b/backend/app/services/data_import.py @@ -20,6 +20,7 @@ logger = logging.getLogger(__name__) from app.time_utils import utcnow +from app.services.authors import parse_authors, sync_book_authors from app.services.cover_storage import download_cover from app.services.tags import sync_book_tags from app.services.isbn_utils import normalize_isbn @@ -27,7 +28,7 @@ BOOK_IMPORT_FIELDS: list[str] = [ "title", "subtitle", - "author", + "authors", "isbn", "publisher", "published_year", @@ -39,6 +40,7 @@ "rating", "reading_status", "acquisition_status", + "date_added", "date_started", "date_finished", "cover_url", @@ -50,9 +52,9 @@ "name": "title", "subtitle": "subtitle", "book subtitle": "subtitle", - "author": "author", - "authors": "author", - "author name": "author", + "author": "authors", + "authors": "authors", + "author name": "authors", "isbn": "isbn", "isbn13": "isbn", "isbn10": "isbn", @@ -79,6 +81,8 @@ "acquisition": "acquisition_status", "availability": "acquisition_status", "ownership": "acquisition_status", + "date added": "date_added", + "added": "date_added", "date started": "date_started", "started": "date_started", "date finished": "date_finished", @@ -128,23 +132,29 @@ def delete_parsed_upload(file_id: str, user_id: int) -> None: _temp_file_path(user_id, file_id).unlink(missing_ok=True) -def _to_flat_row(row: dict) -> dict[str, str | int | float | bool | None]: - """Flatten a row dict, rejecting nested values.""" - flat: dict[str, str | int | float | bool | None] = {} +def _to_flat_row(row: dict) -> dict[str, object]: + """Flatten a row dict, rejecting nested dict values. + + List values (e.g. a JSON ``authors`` array) are kept as-is so the adaptive + author handling can turn each entry into a separate author. + """ + flat: dict[str, object] = {} for key, value in row.items(): - if isinstance(value, (dict, list)): + if isinstance(value, dict): raise ValueError("error.importNestedValuesNotSupported") flat[str(key)] = value return flat -def parse_upload(content: bytes, filename: str, user_id: int) -> dict: +def parse_upload(content: bytes, filename: str, user_id: int, delimiter: str = ",") -> dict: """Parse an uploaded CSV or JSON file and persist the parsed result to disk. Args: content: Raw file bytes. filename: Original filename (used to detect format). user_id: Owner of the upload. + delimiter: Single-character field separator used for CSV files + (ignored for JSON). Returns: A dict with file_id, format, source_fields, sample_rows, and row_count. @@ -159,9 +169,11 @@ def parse_upload(content: bytes, filename: str, user_id: int) -> dict: lower = filename.lower() if lower.endswith(".csv"): + if len(delimiter) != 1: + raise ValueError("error.importInvalidDelimiter") parsed_format = "csv" text = content.decode("utf-8-sig") - reader = csv.DictReader(text.splitlines()) + reader = csv.DictReader(text.splitlines(), delimiter=delimiter) if not reader.fieldnames: raise ValueError("error.importMissingHeader") rows = [_to_flat_row(row) for row in reader] @@ -386,21 +398,39 @@ def _parse_reading_status(value: object) -> ReadingStatus: def _build_transform_cache( mapping: dict[str, "ImportFieldConfig"], -) -> dict[str, Callable[..., str]]: +) -> dict[str, Callable[..., Any]]: """Compile all transform expressions into a cache of callables.""" from app.services.transform_engine import compile_transform - cache: dict[str, Callable[..., str]] = {} + cache: dict[str, Callable[..., Any]] = {} for target, config in mapping.items(): if config.transform: cache[target] = compile_transform(config.transform) return cache +def canonicalize_mapping( + mapping: dict[str, ImportFieldConfig], +) -> dict[str, ImportFieldConfig]: + """Normalize a mapping dict, renaming the legacy ``author`` target to ``authors``. + + Both spellings refer to the same per-book author relation; saved mappings + created before the rename may still use ``author`` as the target key. If + both keys are present, the first occurrence in iteration order wins. + """ + result: dict[str, ImportFieldConfig] = {} + for target, config in mapping.items(): + canonical = "authors" if target == "author" else target + if canonical in result: + continue + result[canonical] = config + return result + + def _mapped_row( row: dict, mapping: dict[str, "ImportFieldConfig"], - transform_cache: dict[str, Callable[..., str]], + transform_cache: dict[str, Callable[..., Any]], context: dict[str, Any], errors: list[str] | None = None, ) -> dict: @@ -411,18 +441,35 @@ def _mapped_row( if not source: continue value = row.get(source, "") + if target == "authors" and isinstance(value, list): + # Adaptive authors handling: an array contributes one author per + # entry. Transforms are skipped for array values (they operate on + # scalars); apply them in the source file or use the string form. + mapped[target] = value + continue + if target == "tags" and isinstance(value, list): + # Adaptive tags handling: an array contributes one tag per entry. + # Like authors, transforms are skipped for array values. + mapped[target] = value + continue value_str = "" if value is None else str(value) if target in transform_cache: from app.services.transform_engine import TransformExecutionError, execute_transform try: - value_str = execute_transform( + transform_result = execute_transform( transform_cache[target], value_str, row, context ) except TransformExecutionError as exc: if errors is not None: errors.append(f"\x1f{target}\x1f{exc}") continue + if target in ("authors", "tags") and isinstance(transform_result, list): + # A transform may return a list (e.g. value.split(';')) for the + # adaptive authors/tags targets; keep it so each entry is used. + mapped[target] = transform_result + continue + value_str = str(transform_result) mapped[target] = value_str return mapped @@ -484,6 +531,7 @@ def validate_import( parsed = load_parsed_upload(file_id, user.id) rows = parsed.get("rows", []) source_fields = set(parsed.get("source_fields", [])) + mapping = canonicalize_mapping(mapping) warnings, errors = _validate_mapping(mapping, source_fields, require_acquisition_status) @@ -527,6 +575,12 @@ def validate_import( errors.append(f"Row {idx}: {exc}") continue + date_added: datetime | None = None + try: + date_added = _parse_datetime(row_data.get("date_added"), "date_added") + except ValueError as exc: + errors.append(f"Row {idx}: {exc}") + date_started: datetime | None = None try: date_started = _parse_datetime(row_data.get("date_started"), "date_started") @@ -549,6 +603,12 @@ def validate_import( "without a finish date the book will not count toward monthly statistics" ) + if date_added and date_finished and date_added > date_finished: + warnings.append( + f"Row {idx}: date_added is after date_finished; the book will appear " + "as added after it was finished" + ) + if create_progress_for_read and reading_status == ReadingStatus.read and not row_data.get("page_count"): warnings.append(f"Row {idx}: marked as 'read' but has no page count; will not create a progress entry") @@ -596,6 +656,7 @@ def preview_import( parsed = load_parsed_upload(file_id, user.id) rows = parsed.get("rows", []) source_fields = set(parsed.get("source_fields", [])) + mapping = canonicalize_mapping(mapping) _warnings, mapping_errors = _validate_mapping(mapping, source_fields, require_acquisition_status) if mapping_errors: @@ -633,6 +694,12 @@ def preview_import( except ValueError as exc: row_errors.append(str(exc)) + date_added: datetime | None = None + try: + date_added = _parse_datetime(row_data.get("date_added"), "date_added") + except ValueError as exc: + row_errors.append(str(exc)) + date_started: datetime | None = None try: date_started = _parse_datetime(row_data.get("date_started"), "date_started") @@ -648,16 +715,22 @@ def preview_import( if date_started and date_finished and date_started > date_finished: row_errors.append("date_started is after date_finished") + if date_added and date_finished and date_added > date_finished: + row_errors.append( + "date_added is after date_finished; the book will appear " + "as added after it was finished" + ) + if reading_status == ReadingStatus.read and not date_finished: row_errors.append( "Marked as 'read' but has no finished date; " "without a finish date the book will not count toward monthly statistics" ) - # Convert raw values to strings for display - source_display = {k: str(v) if v is not None else "" for k, v in row.items()} - # Convert transformed values to strings for display - transformed_display = {k: str(v) if v is not None else "" for k, v in row_data.items()} + # Keep list values (e.g. JSON `authors`/`tags` arrays) as lists so the + # preview renders them as JSON arrays; None is shown as an empty string. + source_display = {k: v if v is not None else "" for k, v in row.items()} + transformed_display = {k: v if v is not None else "" for k, v in row_data.items()} preview_rows.append({ "row_number": idx, @@ -704,6 +777,7 @@ async def execute_import( rollback_all = import_mode == "rollback_all" source_fields = set(parsed.get("source_fields", [])) + mapping = canonicalize_mapping(mapping) _warnings, mapping_errors = _validate_mapping(mapping, source_fields, require_acquisition_status) if mapping_errors: yield {"event": "error", "message": "; ".join(mapping_errors)} @@ -738,6 +812,12 @@ async def execute_import( None if row_data.get("language") is None else str(row_data.get("language")) ) date_errors: list[str] = [] + date_added: datetime | None = None + try: + date_added = _parse_datetime(row_data.get("date_added"), "date_added") + except ValueError as exc: + date_errors.append(str(exc)) + date_started: datetime | None = None try: date_started = _parse_datetime(row_data.get("date_started"), "date_started") @@ -756,6 +836,12 @@ async def execute_import( if date_started and date_finished and date_started > date_finished: raise ValueError("date_started is after date_finished") + if date_added and date_finished and date_added > date_finished: + raise ValueError( + "date_added is after date_finished; the book would appear " + "as added after it was finished" + ) + if reading_status == ReadingStatus.read and not date_finished: raise ValueError("Marked as 'read' but has no finished date") @@ -772,7 +858,6 @@ async def execute_import( book = Book( title=title, subtitle=None if row_data.get("subtitle") in (None, "") else str(row_data.get("subtitle")), - author=None if row_data.get("author") in (None, "") else str(row_data.get("author")), # ty: ignore[invalid-argument-type] isbn=None if row_data.get("isbn") in (None, "") else str(row_data.get("isbn")), cover_url=cover_url, publisher=None if row_data.get("publisher") in (None, "") else str(row_data.get("publisher")), @@ -784,6 +869,7 @@ async def execute_import( rating=rating, reading_status=reading_status, acquisition_status=acquisition_status, + date_added=date_added or utcnow(), date_started=date_started, date_finished=date_finished, user_id=user.id, @@ -792,6 +878,13 @@ async def execute_import( session.flush() assert book.id is not None + sync_book_authors( + session, + user.id, + book.id, + parse_authors(row_data.get("authors")), + ) + if create_progress_for_read and reading_status == ReadingStatus.read and page_count is not None and date_finished is not None: log_date = date_finished if log_date.tzinfo is None: @@ -808,7 +901,7 @@ async def execute_import( session, user.id, book.id, - None if row_data.get("tags") in (None, "") else str(row_data.get("tags")), + None if row_data.get("tags") in (None, "") else row_data.get("tags"), ) if not rollback_all: @@ -859,7 +952,7 @@ async def execute_import( "mapping": { "title": {"source": "Title", "transform": None}, "subtitle": {"source": "", "transform": None}, - "author": {"source": "Author", "transform": None}, + "authors": {"source": "Author", "transform": None}, "isbn": {"source": "ISBN13", "transform": "value.replace('=', '').replace('\"', '').strip() if value else None"}, "publisher": {"source": "Publisher", "transform": None}, "published_year": { diff --git a/backend/app/services/search.py b/backend/app/services/search.py index 471007cd..65117959 100644 --- a/backend/app/services/search.py +++ b/backend/app/services/search.py @@ -16,12 +16,11 @@ import sqlalchemy as sa from sqlmodel import col, or_, select -from app.models import AcquisitionStatus, Book, BookTag, Tag +from app.models import AcquisitionStatus, Author, Book, BookAuthor, BookTag, Tag # Fields that can be targeted with a prefix. The keys are the canonical, # always-English prefix names; the values are the book model columns. FIELD_COLUMNS: dict[str, Any] = { - "author": Book.author, "title": Book.title, "publisher": Book.publisher, "language": Book.language, @@ -32,14 +31,15 @@ # Availability is a special case: it maps to an exact enum comparison. AVAILABILITY_PREFIX = "availability" TAG_PREFIX = "tag" +AUTHOR_PREFIX = "author" SUPPORTED_PREFIXES: frozenset[str] = frozenset( - [*FIELD_COLUMNS.keys(), AVAILABILITY_PREFIX, TAG_PREFIX] + [*FIELD_COLUMNS.keys(), AVAILABILITY_PREFIX, TAG_PREFIX, AUTHOR_PREFIX] ) # Default fields searched by an unprefixed term (unchanged from the previous -# single-pattern search). -DEFAULT_SEARCH_COLUMNS: tuple[str, ...] = ("title", "subtitle", "author", "blurb") +# single-pattern search, minus the removed Book.author column). +DEFAULT_SEARCH_COLUMNS: tuple[str, ...] = ("title", "subtitle", "blurb") # ``:value`` — value is either a quoted string or a non-space token. _FIELD_TERM_RE = re.compile(r"^([a-zA-Z_]+):(\"(?:\\.|[^\"\\])*\"|\S+)") @@ -148,11 +148,23 @@ def _tag_condition(value: str, user_id: int) -> Any: return col(Book.id).in_(matching_tag_book_ids) +def _author_condition(value: str, user_id: int) -> Any: + """Return a condition matching books that have an author containing *value*.""" + escaped = _escape_like(value) + matching_author_book_ids = ( + select(BookAuthor.book_id) + .join(Author, col(Author.id) == BookAuthor.author_id) + .where(Author.user_id == user_id, col(Author.name).ilike(f"%{escaped}%", escape="\\")) + ) + return col(Book.id).in_(matching_author_book_ids) + + def _unprefixed_condition(value: str, user_id: int) -> Any: """Build the cross-field substring condition for an unprefixed term.""" return or_( *[_ilike(getattr(Book, column), value) for column in DEFAULT_SEARCH_COLUMNS], _tag_condition(value, user_id), + _author_condition(value, user_id), ) @@ -172,6 +184,8 @@ def _field_condition(field: str, value: str, user_id: int) -> Any | None: return _availability_condition(value) if field == TAG_PREFIX: return _tag_condition(value, user_id) + if field == AUTHOR_PREFIX: + return _author_condition(value, user_id) column = FIELD_COLUMNS[field] return _ilike(column, value) diff --git a/backend/app/services/tags.py b/backend/app/services/tags.py index 68c60a9c..85eb353c 100644 --- a/backend/app/services/tags.py +++ b/backend/app/services/tags.py @@ -6,14 +6,15 @@ from app.models import Book, BookTag, Tag from app.schemas import BookRead +from app.services.authors import authors_list_for_book, join_authors from app.time_utils import utcnow -def parse_tags(raw_tags: str | None) -> list[str]: - """Parse and deduplicate a comma-separated tag string. +def parse_tags(raw_tags: str | list[str] | None) -> list[str]: + """Parse and deduplicate tag names. - Args: - raw_tags: Comma-separated tag names or None. + Accepts a comma-separated string (legacy/CSV) or a list of names (JSON + export/import). A list contributes one tag per entry. Returns: A list of normalized, unique tag name strings. @@ -21,10 +22,12 @@ def parse_tags(raw_tags: str | None) -> list[str]: if not raw_tags: return [] + pieces: list[str] = raw_tags if isinstance(raw_tags, list) else raw_tags.split(",") + seen: set[str] = set() parsed: list[str] = [] - for piece in raw_tags.split(","): - normalized = " ".join(piece.strip().split()) + for piece in pieces: + normalized = " ".join(str(piece).strip().split()) if not normalized: continue key = normalized.lower() @@ -35,7 +38,9 @@ def parse_tags(raw_tags: str | None) -> list[str]: return parsed -def sync_book_tags(session: Session, user_id: int, book_id: int, raw_tags: str | None) -> None: +def sync_book_tags( + session: Session, user_id: int, book_id: int, raw_tags: str | list[str] | None +) -> None: """Synchronize the tag associations for a book to match *raw_tags*. Creates new tags as needed and removes stale BookTag links. @@ -44,7 +49,7 @@ def sync_book_tags(session: Session, user_id: int, book_id: int, raw_tags: str | session: Active database session. user_id: Owner of the tags. book_id: Target book. - raw_tags: Comma-separated tag names or None (clears all tags). + raw_tags: Comma-separated tag names, a list of names, or None (clears all tags). """ parsed = parse_tags(raw_tags) @@ -92,9 +97,11 @@ def cleanup_orphan_tags(session: Session, user_id: int) -> None: session.delete(tag) -def tags_text_for_book(session: Session, book_id: int) -> str | None: - """Return a comma-separated tag string for a given book, or None.""" - names = list( +def tags_list_for_book(session: Session, book_id: int | None) -> list[str]: + """Return the sorted tag names for a given book, or an empty list.""" + if book_id is None: + return [] + return list( session.exec( select(Tag.name) .join(BookTag, col(BookTag.tag_id) == col(Tag.id)) @@ -102,6 +109,11 @@ def tags_text_for_book(session: Session, book_id: int) -> str | None: .order_by(col(Tag.name).asc()) ).all() ) + + +def tags_text_for_book(session: Session, book_id: int) -> str | None: + """Return a comma-separated tag string for a given book, or None.""" + names = tags_list_for_book(session, book_id) if not names: return None return ", ".join(names) @@ -131,4 +143,7 @@ def build_book_read(session: Session, book: Book) -> BookRead: payload = book.model_dump() payload.pop("user_id", None) payload["tags"] = tags_text_for_book(session, book.id) if book.id is not None else None + authors = authors_list_for_book(session, book.id) + payload["authors"] = authors + payload["author"] = join_authors(authors) return BookRead.model_validate(payload) diff --git a/backend/app/services/transform_engine.py b/backend/app/services/transform_engine.py index f24a89d1..a6c9a3e1 100644 --- a/backend/app/services/transform_engine.py +++ b/backend/app/services/transform_engine.py @@ -133,7 +133,7 @@ def _validate_ast(source: str) -> list[str]: return errors -def compile_transform(code: str) -> Callable[..., str]: +def compile_transform(code: str) -> Callable[..., Any]: """Compile a Python code block into a restricted callable. The user's code is wrapped in a function definition: @@ -196,17 +196,23 @@ def execute_transform( value: str, row: dict[str, str], context: dict[str, Any], -) -> str: - """Execute a compiled transform and enforce string return.""" +) -> Any: + """Execute a compiled transform and return its result. + + ``None`` results are coerced to the empty string. A ``list`` result is + passed through untouched so transforms on the adaptive ``authors``/``tags`` + targets can return an array (e.g. ``value.split(';')``); every other + non-string value is stringified, as before. + """ try: result = fn(value=value, row=row, context=context) except Exception as exc: raise TransformExecutionError(str(exc)) from exc if result is None: return "" - if not isinstance(result, str): - return str(result) - return result + if isinstance(result, list): + return result + return str(result) def validate_transform(code: str) -> list[str]: diff --git a/backend/app/services/user_deletion.py b/backend/app/services/user_deletion.py index 0634ac48..9cd11067 100644 --- a/backend/app/services/user_deletion.py +++ b/backend/app/services/user_deletion.py @@ -6,7 +6,7 @@ from fastapi import HTTPException, status from sqlmodel import Session, col, func, select -from app.models import ApiKey, Book, BookTag, OidcLink, ReadingProgress, Tag, User, UserRole, UserSettings +from app.models import ApiKey, Author, Book, BookAuthor, BookTag, OidcLink, ReadingProgress, Tag, User, UserRole, UserSettings from app.time_utils import utcnow from app.services.cover_storage import delete_cover_file, local_cover_filename @@ -16,6 +16,7 @@ class ReadingDataDeletionCounts: """Counts of items deleted during a reading-data or account deletion.""" books: int tags: int + authors: int progress_entries: int @@ -47,6 +48,9 @@ def delete_user_reading_data(session: Session, user_id: int, covers_dir: str) -> tags_count = session.exec( select(func.count()).select_from(Tag).where(Tag.user_id == user_id) ).one() + authors_count = session.exec( + select(func.count()).select_from(Author).where(Author.user_id == user_id) + ).one() if book_ids: for cover_url in {book.cover_url for book in user_books if book.cover_url}: @@ -62,18 +66,25 @@ def delete_user_reading_data(session: Session, user_id: int, covers_dir: str) -> for link in session.exec(select(BookTag).where(col(BookTag.book_id).in_(book_ids))).all(): session.delete(link) + for link in session.exec(select(BookAuthor).where(col(BookAuthor.book_id).in_(book_ids))).all(): + session.delete(link) + for entry in session.exec(select(ReadingProgress).where(ReadingProgress.user_id == user_id)).all(): session.delete(entry) for tag in session.exec(select(Tag).where(Tag.user_id == user_id)).all(): session.delete(tag) + for author in session.exec(select(Author).where(Author.user_id == user_id)).all(): + session.delete(author) + for book in user_books: session.delete(book) return ReadingDataDeletionCounts( books=len(user_books), tags=tags_count, + authors=authors_count, progress_entries=progress_count, ) diff --git a/backend/tests/test_admin.py b/backend/tests/test_admin.py index 6ef2d0d4..658550e1 100644 --- a/backend/tests/test_admin.py +++ b/backend/tests/test_admin.py @@ -208,7 +208,7 @@ def test_admin_restore_success(admin_client_with_file_db: tuple[TestClient, str] # 2. Modify the database (add a new book) conn = sqlite3.connect(db_path) - conn.execute("INSERT INTO book (title, author, page_count, user_id, reading_status, acquisition_status) VALUES ('New Book', '', 0, 1, 'read', 'owned')") + conn.execute("INSERT INTO book (title, page_count, user_id, reading_status, acquisition_status) VALUES ('New Book', 0, 1, 'read', 'owned')") conn.commit() row = conn.execute("SELECT COUNT(*) FROM book").fetchone() assert row[0] == 2 diff --git a/backend/tests/test_authors.py b/backend/tests/test_authors.py new file mode 100644 index 00000000..ae4b10c7 --- /dev/null +++ b/backend/tests/test_authors.py @@ -0,0 +1,193 @@ +"""Tests for the author parsing and relation helpers.""" + +from sqlmodel import Session, col, select + +from app.models import Author, BookAuthor +from app.services.authors import ( + authors_list_for_book, + join_authors, + normalize_author_list, + normalize_author_name, + parse_authors, + parse_legacy_author, + resolve_authors_payload, + split_author_string, + sync_book_authors, +) + + +# ── normalization ────────────────────────────────────────────────────────────── + +def test_normalize_author_name_collapses_whitespace() -> None: + assert normalize_author_name(" Isaac Asimov ") == "Isaac Asimov" + + +def test_normalize_author_name_empty_returns_none() -> None: + assert normalize_author_name(" ") is None + + +def test_normalize_author_list_dedupes_case_insensitively() -> None: + assert normalize_author_list(["Isaac Asimov", "isaac asimov", "Robert A. Heinlein"]) == [ + "Isaac Asimov", + "Robert A. Heinlein", + ] + + +def test_normalize_author_list_drops_blank_entries() -> None: + assert normalize_author_list([" ", "Asimov, Isaac"]) == ["Asimov, Isaac"] + + +# ── parsing ─────────────────────────────────────────────────────────────────── + +def test_parse_legacy_author_splits_on_commas() -> None: + assert parse_legacy_author("Isaac Asimov, Robert A. Heinlein") == [ + "Isaac Asimov", + "Robert A. Heinlein", + ] + + +def test_parse_legacy_author_blank_pieces_skipped() -> None: + assert parse_legacy_author("Isaac Asimov, , Robert A. Heinlein") == [ + "Isaac Asimov", + "Robert A. Heinlein", + ] + + +def test_parse_authors_string_is_single_author() -> None: + """A file-import string without a separator becomes one author.""" + assert parse_authors("Asimov, Isaac") == ["Asimov, Isaac"] + assert parse_authors("Frank Herbert") == ["Frank Herbert"] + + +def test_parse_authors_string_splits_on_semicolon() -> None: + """Strings may encode several authors with `;` (CSV export round-trip).""" + assert parse_authors("Frank Herbert; Brian Herbert") == ["Frank Herbert", "Brian Herbert"] + + +def test_parse_authors_list_is_one_per_entry() -> None: + assert parse_authors(["Frank Herbert", "Brian Herbert"]) == [ + "Frank Herbert", + "Brian Herbert", + ] + + +def test_parse_authors_none_returns_empty() -> None: + assert parse_authors(None) == [] + + +# ── hybrid payload resolution ───────────────────────────────────────────────── + +def test_resolve_authors_payload_authors_takes_precedence() -> None: + assert resolve_authors_payload(author="A, B", authors=["C", "D"]) == ["C", "D"] + + +def test_resolve_authors_payload_empty_list_clears() -> None: + assert resolve_authors_payload(author="A", authors=[]) == [] + + +def test_resolve_authors_payload_falls_back_to_legacy() -> None: + assert resolve_authors_payload(author="A, B") == ["A", "B"] + + +def test_resolve_authors_payload_none_means_not_provided() -> None: + assert resolve_authors_payload() is None + + +# ── external splitter ───────────────────────────────────────────────────────── + +def test_split_author_string_preserves_comma_names() -> None: + assert split_author_string("Asimov, Isaac") == ["Asimov, Isaac"] + + +def test_split_author_string_splits_on_semicolon() -> None: + assert split_author_string("Asimov, Isaac; Clarke, Arthur") == [ + "Asimov, Isaac", + "Clarke, Arthur", + ] + + +def test_split_author_string_splits_on_ampersand() -> None: + assert split_author_string("Frank Herbert & Brian Herbert") == [ + "Frank Herbert", + "Brian Herbert", + ] + + +def test_split_author_string_splits_on_and() -> None: + assert split_author_string("Terry Pratchett and Neil Gaiman") == [ + "Terry Pratchett", + "Neil Gaiman", + ] + + +def test_split_author_string_empty_returns_empty() -> None: + assert split_author_string(None) == [] + assert split_author_string(" ") == [] + + +# ── sync / query ────────────────────────────────────────────────────────────── + +def test_sync_book_authors_creates_rows_and_links(session: Session) -> None: + user_id = 1 + book_id = 1 + sync_book_authors(session, user_id, book_id, ["Isaac Asimov", "Frank Herbert"]) + session.commit() + + names = authors_list_for_book(session, book_id) + assert names == ["Frank Herbert", "Isaac Asimov"] # alphabetical + + authors = session.exec(select(Author).where(Author.user_id == user_id)).all() + assert {a.name for a in authors} == {"Isaac Asimov", "Frank Herbert"} + + +def test_sync_book_authors_reuses_existing_author(session: Session) -> None: + user_id = 1 + sync_book_authors(session, user_id, 1, ["Isaac Asimov"]) + session.commit() + first_id = session.exec( + select(Author.id).where(Author.user_id == user_id, Author.name == "Isaac Asimov") + ).one() + + sync_book_authors(session, user_id, 2, ["Isaac Asimov", "Frank Herbert"]) + session.commit() + + second_id = session.exec( + select(Author.id).where(Author.user_id == user_id, Author.name == "Isaac Asimov") + ).one() + assert first_id == second_id + + link_count = session.exec( + select(col(BookAuthor.author_id)).where(BookAuthor.author_id == second_id) + ).all() + assert len(link_count) == 2 + + +def test_sync_book_authors_removes_stale_links(session: Session) -> None: + user_id = 1 + sync_book_authors(session, user_id, 1, ["Isaac Asimov", "Frank Herbert"]) + session.commit() + + sync_book_authors(session, user_id, 1, ["Frank Herbert"]) + session.commit() + + names = authors_list_for_book(session, 1) + assert names == ["Frank Herbert"] + + +def test_sync_book_authors_empty_clears_links(session: Session) -> None: + user_id = 1 + sync_book_authors(session, user_id, 1, ["Isaac Asimov"]) + session.commit() + + sync_book_authors(session, user_id, 1, []) + session.commit() + + assert authors_list_for_book(session, 1) == [] + + +# ── join ────────────────────────────────────────────────────────────────────── + +def test_join_authors() -> None: + assert join_authors(["A", "B"]) == "A, B" + assert join_authors([]) is None + assert join_authors(None) is None \ No newline at end of file diff --git a/backend/tests/test_books.py b/backend/tests/test_books.py index 852ff2e6..32567309 100644 --- a/backend/tests/test_books.py +++ b/backend/tests/test_books.py @@ -81,6 +81,105 @@ def test_create_book_invalid_rating_returns_422(client: TestClient) -> None: assert resp.status_code == 422 +# ── authors (multiple) ───────────────────────────────────────────────────────── + +def test_create_book_with_authors_list(client: TestClient) -> None: + resp = client.post("/api/books", json={ + "title": "Good Omens", + "authors": ["Terry Pratchett", "Neil Gaiman"], + "page_count": 288, + }) + assert resp.status_code == 201 + data = resp.json() + assert data["authors"] == ["Neil Gaiman", "Terry Pratchett"] + assert data["author"] == "Neil Gaiman, Terry Pratchett" + + +def test_create_book_legacy_author_comma_separated(client: TestClient) -> None: + resp = client.post("/api/books", json={"title": "Dune", "author": "Frank Herbert", "page_count": 412}) + assert resp.status_code == 201 + data = resp.json() + assert data["authors"] == ["Frank Herbert"] + assert data["author"] == "Frank Herbert" + + +def test_create_book_authors_takes_precedence_over_author(client: TestClient) -> None: + resp = client.post("/api/books", json={ + "title": "Dune", + "author": "Wrong Author", + "authors": ["Frank Herbert"], + "page_count": 412, + }) + assert resp.status_code == 201 + data = resp.json() + assert data["authors"] == ["Frank Herbert"] + + +def test_create_book_no_author_returns_422(client: TestClient) -> None: + resp = client.post("/api/books", json={"title": "No Author", "page_count": 100}) + assert resp.status_code == 422 + detail = resp.json()["detail"] + assert any( + "at least one author" in item.get("msg", "").lower() + for item in detail + if isinstance(item, dict) + ) + + +def test_update_book_authors_replaces(client: TestClient) -> None: + book = _create_book(client, title="Dune", author="Frank Herbert") + resp = client.patch(f"/api/books/{book['id']}", json={"authors": ["Frank Herbert", "Brian Herbert"]}) + assert resp.status_code == 200 + data = resp.json() + assert data["authors"] == ["Brian Herbert", "Frank Herbert"] + assert data["author"] == "Brian Herbert, Frank Herbert" + + +def test_update_book_authors_empty_clears(client: TestClient) -> None: + book = _create_book(client, title="Dune", author="Frank Herbert") + resp = client.patch(f"/api/books/{book['id']}", json={"authors": []}) + assert resp.status_code == 200 + data = resp.json() + assert data["authors"] == [] + assert data["author"] is None + + +def test_update_book_without_authors_keeps_them(client: TestClient) -> None: + book = _create_book(client, title="Dune", author="Frank Herbert") + resp = client.patch(f"/api/books/{book['id']}", json={"title": "Dune 2"}) + assert resp.status_code == 200 + data = resp.json() + assert data["authors"] == ["Frank Herbert"] + + +def test_update_book_legacy_author_parses(client: TestClient) -> None: + book = _create_book(client, title="Dune", author="Frank Herbert") + resp = client.patch(f"/api/books/{book['id']}", json={"author": "Isaac Asimov"}) + assert resp.status_code == 200 + data = resp.json() + assert data["authors"] == ["Isaac Asimov"] + + +def test_list_books_joins_authors(client: TestClient) -> None: + _create_book(client, title="Good Omens", authors=["Terry Pratchett", "Neil Gaiman"]) + resp = client.get("/api/books") + assert resp.status_code == 200 + body = resp.json() + assert body["total"] == 1 + book = body["books"][0] + assert book["authors"] == ["Neil Gaiman", "Terry Pratchett"] + assert book["author"] == "Neil Gaiman, Terry Pratchett" + + +def test_get_book_returns_authors_list(client: TestClient) -> None: + book = _create_book(client, title="Good Omens", authors=["Terry Pratchett", "Neil Gaiman"]) + resp = client.get(f"/api/books/{book['id']}") + assert resp.status_code == 200 + data = resp.json() + assert data["authors"] == ["Neil Gaiman", "Terry Pratchett"] + assert data["author"] == "Neil Gaiman, Terry Pratchett" + + # ── list ────────────────────────────────────────────────────────────────────── def test_list_books_empty(client: TestClient) -> None: @@ -1137,6 +1236,32 @@ def test_suggest_authors_deduplication(client: TestClient) -> None: assert resp.json()["suggestions"] == ["Frank Herbert"] +def test_search_author_prefix_matches_any_author(client: TestClient) -> None: + _create_book(client, title="Good Omens", authors=["Terry Pratchett", "Neil Gaiman"]) + _create_book(client, title="Hogfather", author="Terry Pratchett") + resp = client.get("/api/books?q=author:Gaiman") + assert resp.status_code == 200 + body = resp.json() + assert [b["title"] for b in body["books"]] == ["Good Omens"] + + +def test_search_unprefixed_matches_author(client: TestClient) -> None: + _create_book(client, title="Good Omens", authors=["Terry Pratchett", "Neil Gaiman"]) + resp = client.get("/api/books?q=Pratchett") + assert resp.status_code == 200 + body = resp.json() + assert [b["title"] for b in body["books"]] == ["Good Omens"] + + +def test_search_negated_author_excludes(client: TestClient) -> None: + _create_book(client, title="Good Omens", authors=["Terry Pratchett", "Neil Gaiman"]) + _create_book(client, title="Hogfather", author="Terry Pratchett") + resp = client.get("/api/books?q=-author:Gaiman") + assert resp.status_code == 200 + body = resp.json() + assert [b["title"] for b in body["books"]] == ["Hogfather"] + + def test_suggest_publishers_returns_matching(client: TestClient) -> None: _create_book(client, title="A", publisher="Ace Books") _create_book(client, title="B", publisher="Bantam Books") diff --git a/backend/tests/test_data.py b/backend/tests/test_data.py index 209da80c..d04d291d 100644 --- a/backend/tests/test_data.py +++ b/backend/tests/test_data.py @@ -141,6 +141,12 @@ def test_data_export_with_progress_and_tags(client: TestClient) -> None: assert export_resp.status_code == 200 with zipfile.ZipFile(io.BytesIO(export_resp.content), "r") as zf: + books = json.loads(zf.read("books.json")) + assert len(books) == 1 + assert books[0]["author"] == "Frank Herbert" + assert books[0]["authors"] == ["Frank Herbert"] + assert books[0]["tags"] == ["Sci-Fi"] + progress = json.loads(zf.read("progress.json")) assert len(progress) == 1 assert progress[0]["page"] == 100 @@ -152,6 +158,33 @@ def test_data_export_with_progress_and_tags(client: TestClient) -> None: assert tags[0]["book_count"] == 1 +def test_data_export_json_author_string_and_tags_list(client: TestClient) -> None: + create_resp = client.post( + "/api/books", + json={ + "title": "Good Omens", + "authors": ["Terry Pratchett", "Neil Gaiman"], + "tags": "fantasy,humor", + "page_count": 288, + "reading_status": "read", + }, + ) + assert create_resp.status_code == 201 + + export_resp = client.post( + "/api/data/export", + json={"datasets": ["books"], "format": "json"}, + ) + assert export_resp.status_code == 200 + + with zipfile.ZipFile(io.BytesIO(export_resp.content), "r") as zf: + books = json.loads(zf.read("books.json")) + assert len(books) == 1 + assert books[0]["author"] == "Neil Gaiman; Terry Pratchett" + assert books[0]["authors"] == ["Neil Gaiman", "Terry Pratchett"] + assert books[0]["tags"] == ["fantasy", "humor"] + + def test_data_import_parse_and_suggest_mapping(client: TestClient, monkeypatch: MonkeyPatch, tmp_path: Path) -> None: monkeypatch.setattr(settings, "import_temp_dir", str(tmp_path / "import_temp")) csv_payload = "Title,Author,My Rating\nDune,Frank Herbert,5\n" @@ -172,7 +205,7 @@ def test_data_import_parse_and_suggest_mapping(client: TestClient, monkeypatch: assert suggest_resp.status_code == 200 suggested = suggest_resp.json()["suggested_mapping"] assert suggested["title"]["source"] == "Title" - assert suggested["author"]["source"] == "Author" + assert suggested["authors"]["source"] == "Author" assert suggested["rating"]["source"] == "My Rating" diff --git a/backend/tests/test_data_import.py b/backend/tests/test_data_import.py index c8529e1a..d9ea2869 100644 --- a/backend/tests/test_data_import.py +++ b/backend/tests/test_data_import.py @@ -1,7 +1,9 @@ """Unit tests for app.services.data_import module.""" +import io import json import os +import zipfile from datetime import datetime, timezone from io import BytesIO from pathlib import Path @@ -11,12 +13,15 @@ import pytest from pytest import MonkeyPatch from sqlalchemy.exc import IntegrityError -from sqlmodel import Session +from sqlmodel import Session, select from app.config import settings -from app.models import Book, ReadingStatus, User, UserRole +from app.models import AcquisitionStatus, Book, ReadingStatus, User, UserRole from app.schemas import ImportFieldConfig from app.services import data_import as di +from app.services.authors import authors_list_for_book, sync_book_authors +from app.services.data_export import _serialize_datetime, build_export_zip +from app.services.tags import sync_book_tags, tags_list_for_book # ── _display_value / _format_value_error ────────────────────────────────────── @@ -50,9 +55,21 @@ def test_to_flat_row_nested_dict_raises() -> None: di._to_flat_row({"key": {"nested": 1}}) -def test_to_flat_row_nested_list_raises() -> None: - with pytest.raises(ValueError, match="error.importNestedValuesNotSupported"): - di._to_flat_row({"key": [1, 2]}) +def test_to_flat_row_list_is_preserved() -> None: + """List values are kept as-is so JSON author arrays survive flattening.""" + flat = di._to_flat_row({"author": ["Asimov, Isaac", "Robert Heinlein"]}) + assert flat["author"] == ["Asimov, Isaac", "Robert Heinlein"] + + +def test_canonicalize_mapping_renames_author_target() -> None: + """Legacy 'author' targets map to the canonical 'authors' field.""" + from app.schemas import ImportFieldConfig + + canonical = di.canonicalize_mapping( + {"title": ImportFieldConfig(source="Title"), "author": ImportFieldConfig(source="Author")} + ) + assert set(canonical) == {"title", "authors"} + assert canonical["authors"].source == "Author" # ── parse_upload ────────────────────────────────────────────────────────────── @@ -73,6 +90,21 @@ def test_parse_upload_csv_missing_header() -> None: di.parse_upload(b"\n", "test.csv", 1) +def test_parse_upload_csv_custom_delimiter(monkeypatch: MonkeyPatch, tmp_path: Path) -> None: + monkeypatch.setattr(settings, "import_temp_dir", str(tmp_path)) + csv = "Title;Author\nDune;Frank Herbert\n" + result = di.parse_upload(csv.encode(), "test.csv", 1, delimiter=";") + assert result["format"] == "csv" + assert result["source_fields"] == ["Title", "Author"] + assert result["sample_rows"][0] == {"Title": "Dune", "Author": "Frank Herbert"} + + +def test_parse_upload_csv_invalid_delimiter(monkeypatch: MonkeyPatch, tmp_path: Path) -> None: + monkeypatch.setattr(settings, "import_temp_dir", str(tmp_path)) + with pytest.raises(ValueError, match="error.importInvalidDelimiter"): + di.parse_upload(b"Title\nDune\n", "test.csv", 1, delimiter=";;") + + def test_parse_upload_json_not_array() -> None: payload = json.dumps({"key": "value"}).encode() with pytest.raises(ValueError, match="error.importJsonMustBeArray"): @@ -241,6 +273,60 @@ def test_mapped_row_skips_empty_source() -> None: assert result == {"author": ""} # row.get("B") returns None -> "" +def test_mapped_row_array_authors_skips_transform() -> None: + """A list source mapped to `authors` bypasses the transform (scalar-only).""" + result = di._mapped_row( + {"A": ["Neil Gaiman", "Terry Pratchett"]}, + {"authors": ImportFieldConfig(source="A", transform="value.upper()")}, + {}, + {}, + ) + assert result == {"authors": ["Neil Gaiman", "Terry Pratchett"]} + + +def test_mapped_row_transform_returning_list_kept_for_authors() -> None: + """A transform that returns a list (e.g. value.split(';')) stays a list for + the adaptive `authors` target, so the preview renders a JSON array.""" + transform_cache = di._build_transform_cache( + {"authors": ImportFieldConfig(source="A", transform="value.split(';')")} + ) + result = di._mapped_row( + {"A": "Doe, Jane; Mike; mansarde"}, + {"authors": ImportFieldConfig(source="A", transform="value.split(';')")}, + transform_cache, + {}, + ) + assert result == {"authors": ["Doe, Jane", " Mike", " mansarde"]} + + +def test_mapped_row_transform_returning_list_kept_for_tags() -> None: + """Same list pass-through applies to the adaptive `tags` target.""" + transform_cache = di._build_transform_cache( + {"tags": ImportFieldConfig(source="A", transform="value.split(',')")} + ) + result = di._mapped_row( + {"A": "fantasy,humor"}, + {"tags": ImportFieldConfig(source="A", transform="value.split(',')")}, + transform_cache, + {}, + ) + assert result == {"tags": ["fantasy", "humor"]} + + +def test_mapped_row_transform_returning_list_stringified_for_scalar_target() -> None: + """A list result on a non-adaptive target (e.g. title) is stringified.""" + transform_cache = di._build_transform_cache( + {"title": ImportFieldConfig(source="A", transform="value.split(' ')")} + ) + result = di._mapped_row( + {"A": "Dune Messiah"}, + {"title": ImportFieldConfig(source="A", transform="value.split(' ')")}, + transform_cache, + {}, + ) + assert result == {"title": "['Dune', 'Messiah']"} + + # ── _validate_mapping ───────────────────────────────────────────── def test_validate_mapping_empty_mapping() -> None: @@ -289,6 +375,47 @@ def test_parse_acquisition_status_rejects_invalid_value() -> None: # ── preview_import ──────────────────────────────────────────────────────────── +def test_preview_import_json_lists_render_as_arrays( + session: Session, tmp_path: Path, monkeypatch: MonkeyPatch +) -> None: + """JSON `authors`/`tags` arrays stay arrays in the preview, and the + canonical `authors` target (not legacy `author`) is shown.""" + monkeypatch.setattr(settings, "import_temp_dir", str(tmp_path)) + user = _create_test_user(session) + payload = { + "rows": [ + { + "title": "Good Omens", + "author": "Neil Gaiman; Terry Pratchett", + "authors": ["Neil Gaiman", "Terry Pratchett"], + "tags": ["fantasy", "humor"], + "page_count": 288, + "reading_status": "want_to_read", + } + ], + "source_fields": ["title", "author", "authors", "tags", "page_count", "reading_status"], + } + file_id = "test_preview_lists" + path = di._temp_file_path(user.id, file_id) # ty: ignore[invalid-argument-type] + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps(payload)) + + mapping = di.suggest_mapping(list(payload["source_fields"])) + result = di.preview_import(file_id, user, mapping) + assert len(result["preview_rows"]) == 1 + row = result["preview_rows"][0] + + # Source keeps raw file values: author string stays a string, lists stay lists. + assert row["source"]["author"] == "Neil Gaiman; Terry Pratchett" + assert row["source"]["authors"] == ["Neil Gaiman", "Terry Pratchett"] + assert row["source"]["tags"] == ["fantasy", "humor"] + + # Transformed shows only the canonical `authors` target, as an array. + assert "author" not in row["transformed"] + assert row["transformed"]["authors"] == ["Neil Gaiman", "Terry Pratchett"] + assert row["transformed"]["tags"] == ["fantasy", "humor"] + + def test_preview_import_basic(session: Session, tmp_path: Path, monkeypatch: MonkeyPatch) -> None: monkeypatch.setattr(settings, "import_temp_dir", str(tmp_path)) user = _create_test_user(session) @@ -351,6 +478,11 @@ def test_preview_import_mapping_errors(session: Session, tmp_path: Path, monkeyp # ── validate_import ─────────────────────────────────────────────────────────── +def _require(value: int | None) -> int: + assert value is not None + return value + + def _create_test_user(session: Session) -> User: """Create and return a test user for import tests.""" from app.auth import get_password_hash @@ -547,6 +679,240 @@ async def test_execute_import_mapping_errors(session: Session, tmp_path: Path, m assert any("Invalid mapping target" in e.get("message", "") for e in events) +@pytest.mark.anyio +async def test_execute_import_tags_list_and_multi_author(session: Session, tmp_path: Path, monkeypatch: MonkeyPatch) -> None: + """A JSON backup with list tags and list authors (as exported) round-trips.""" + monkeypatch.setattr(settings, "import_temp_dir", str(tmp_path)) + user = _create_test_user(session) + payload = { + "rows": [ + { + "title": "Good Omens", + "author": "Neil Gaiman; Terry Pratchett", + "tags": ["fantasy", "humor"], + "page_count": "288", + "reading_status": "want_to_read", + } + ], + "source_fields": ["title", "author", "tags", "page_count", "reading_status"], + } + file_id = "test_exec_tags_list" + path = di._temp_file_path(user.id, file_id) # ty: ignore[invalid-argument-type] + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps(payload)) + + mapping = { + "title": ImportFieldConfig(source="title"), + "author": ImportFieldConfig(source="author"), + "tags": ImportFieldConfig(source="tags"), + "page_count": ImportFieldConfig(source="page_count"), + "reading_status": ImportFieldConfig(source="reading_status"), + } + events = [] + async for event in di.execute_import(file_id, user, mapping, session, "continue_on_error"): + events.append(event) + complete = [e for e in events if e["event"] == "complete"][0] + assert complete["imported"] == 1 + + book = session.exec(select(Book).where(Book.user_id == user.id)).one() + assert authors_list_for_book(session, book.id) == ["Neil Gaiman", "Terry Pratchett"] + assert tags_list_for_book(session, book.id) == ["fantasy", "humor"] + + +@pytest.mark.anyio +async def test_execute_import_full_library_json_round_trip( + session: Session, tmp_path: Path, monkeypatch: MonkeyPatch +) -> None: + """Export a full library as JSON and re-import it, verifying every field. + + Covers a fully-populated book, a multi-author/tagged book, and a minimal + book, including ``date_added``. + """ + monkeypatch.setattr(settings, "import_temp_dir", str(tmp_path)) + user = _create_test_user(session) + user_id = _require(user.id) + + # ── Source library ────────────────────────────────────────────────────── + full = Book( + title="Dune", + subtitle="A sci-fi classic", + isbn="9780441013593", + publisher="Ace Books", + published_year=1965, + page_count=412, + language="EN", + notes="Spice must flow.", + blurb="A desert planet, a young duke, and a spice that grants prescience.", + rating=5, + reading_status=ReadingStatus.read, + acquisition_status=AcquisitionStatus.owned, + date_added=datetime(2024, 12, 1, 8, 0, tzinfo=timezone.utc), + date_started=datetime(2025, 1, 10, 9, 30, tzinfo=timezone.utc), + date_finished=datetime(2025, 1, 20, 21, 45, tzinfo=timezone.utc), + user_id=user.id, + ) + multi = Book( + title="Good Omens", + subtitle=None, + isbn="9780060853983", + publisher="William Morrow", + published_year=1990, + page_count=288, + language="EN", + notes=None, + blurb="An angel, a demon, and an approaching apocalypse.", + rating=4, + reading_status=ReadingStatus.want_to_read, + acquisition_status=AcquisitionStatus.digital_access, + date_started=None, + date_finished=None, + user_id=user.id, + ) + minimal = Book( + title="Minimal", + page_count=100, + reading_status=ReadingStatus.want_to_read, + acquisition_status=AcquisitionStatus.owned, + user_id=user.id, + ) + session.add_all([full, multi, minimal]) + session.flush() + full_id = _require(full.id) + multi_id = _require(multi.id) + minimal_id = _require(minimal.id) + + sync_book_authors(session, user_id, full_id, ["Frank Herbert"]) + sync_book_tags(session, user_id, full_id, "sci-fi,classic") + sync_book_authors(session, user_id, multi_id, ["Neil Gaiman", "Terry Pratchett"]) + sync_book_tags(session, user_id, multi_id, ["fantasy", "humor"]) + session.commit() + + # ── Export ─────────────────────────────────────────────────────────────── + zip_bytes, _ = build_export_zip(session, user, ["books"], "json", settings.covers_dir) + with zipfile.ZipFile(io.BytesIO(zip_bytes)) as zf: + exported_rows = json.loads(zf.read("books.json")) + assert len(exported_rows) == 3 + exported_by_title = {row["title"]: row for row in exported_rows} + + # Exported multi-author author string must be the "; "-joined form. + assert exported_by_title["Good Omens"]["author"] == "Neil Gaiman; Terry Pratchett" + assert exported_by_title["Good Omens"]["authors"] == ["Neil Gaiman", "Terry Pratchett"] + assert exported_by_title["Good Omens"]["tags"] == ["fantasy", "humor"] + + # Remove the source library so only the imported copies remain. + for book in (full, multi, minimal): + session.delete(book) + session.commit() + + # ── Re-import the exported rows ────────────────────────────────────────── + file_id = "roundtrip_full" + path = di._temp_file_path(user.id, file_id) # ty: ignore[invalid-argument-type] + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text( + json.dumps({"rows": exported_rows, "source_fields": list(exported_rows[0].keys())}) + ) + + mapping = { + field: ImportFieldConfig(source=field) + for field in ( + "title", + "subtitle", + "author", + "isbn", + "publisher", + "published_year", + "page_count", + "language", + "tags", + "notes", + "blurb", + "rating", + "reading_status", + "acquisition_status", + "date_added", + "date_started", + "date_finished", + "cover_url", + ) + } + events = [] + async for event in di.execute_import( + file_id, user, mapping, session, "continue_on_error", require_acquisition_status=True + ): + events.append(event) + complete = [e for e in events if e["event"] == "complete"][0] + assert complete["imported"] == 3 + assert complete["failed"] == 0 + + imported = { + b.title: b for b in session.exec(select(Book).where(Book.user_id == user.id)).all() + } + assert set(imported) == {"Dune", "Good Omens", "Minimal"} + + for title, row in exported_by_title.items(): + book = imported[title] + assert book.subtitle == row["subtitle"] + assert book.isbn == row["isbn"] + assert book.publisher == row["publisher"] + assert book.published_year == row["published_year"] + assert book.page_count == row["page_count"] + assert book.language == row["language"] + assert book.notes == row["notes"] + assert book.blurb == row["blurb"] + assert book.rating == row["rating"] + assert book.reading_status.value == row["reading_status"] + assert book.acquisition_status.value == row["acquisition_status"] + assert book.cover_url == row["cover_url"] + assert _serialize_datetime(book.date_added) == row["date_added"] + assert _serialize_datetime(book.date_started) == row["date_started"] + assert _serialize_datetime(book.date_finished) == row["date_finished"] + assert authors_list_for_book(session, book.id) == row["authors"] + assert tags_list_for_book(session, book.id) == row["tags"] + + +@pytest.mark.anyio +async def test_execute_import_preserves_date_added( + session: Session, tmp_path: Path, monkeypatch: MonkeyPatch +) -> None: + """A mapped ``date_added`` is used instead of the import timestamp.""" + monkeypatch.setattr(settings, "import_temp_dir", str(tmp_path)) + user = _create_test_user(session) + payload = { + "rows": [ + { + "title": "Old Book", + "date_added": "2020-06-01T10:00:00Z", + "page_count": "100", + "reading_status": "want_to_read", + "acquisition_status": "owned", + } + ], + "source_fields": ["title", "date_added", "page_count", "reading_status", "acquisition_status"], + } + file_id = "test_exec_date_added" + path = di._temp_file_path(user.id, file_id) # ty: ignore[invalid-argument-type] + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps(payload)) + + mapping = { + "title": ImportFieldConfig(source="title"), + "date_added": ImportFieldConfig(source="date_added"), + "page_count": ImportFieldConfig(source="page_count"), + "reading_status": ImportFieldConfig(source="reading_status"), + "acquisition_status": ImportFieldConfig(source="acquisition_status"), + } + events = [] + async for event in di.execute_import( + file_id, user, mapping, session, "continue_on_error", require_acquisition_status=True + ): + events.append(event) + complete = [e for e in events if e["event"] == "complete"][0] + assert complete["imported"] == 1 + + book = session.exec(select(Book).where(Book.user_id == user.id)).one() + assert _serialize_datetime(book.date_added) == "2020-06-01T10:00:00Z" + + @pytest.mark.anyio async def test_execute_import_rating_out_of_range_set_to_none(session: Session, tmp_path: Path, monkeypatch: MonkeyPatch) -> None: monkeypatch.setattr(settings, "import_temp_dir", str(tmp_path)) diff --git a/backend/tests/test_hygiene.py b/backend/tests/test_hygiene.py index ef065d63..8928b334 100644 --- a/backend/tests/test_hygiene.py +++ b/backend/tests/test_hygiene.py @@ -3,17 +3,22 @@ import pytest from fastapi.testclient import TestClient from pytest import MonkeyPatch -from sqlmodel import Session +from sqlmodel import Session, col, select -from app.models import Book, ReadingStatus, User +from app.models import Author, Book, BookAuthor, ReadingStatus, User from app.routers import hygiene as hygiene_router +from app.services.authors import normalize_author_list def _create_book(session: Session, user_id: int, **overrides: object) -> Book: - """Create a test book with sensible defaults.""" + """Create a test book with sensible defaults. + + The ``author`` override is applied through the author relation table, so an + empty string or None leaves the book without authors. + """ + raw_author: object = overrides.pop("author", "Test Author") defaults: dict = { "title": "Test Book", - "author": "Test Author", "isbn": None, "publisher": "Test Publisher", "published_year": 2020, @@ -30,6 +35,25 @@ def _create_book(session: Session, user_id: int, **overrides: object) -> Book: session.add(book) session.commit() session.refresh(book) + + names: list[str] = ( + [str(raw_author).strip()] + if isinstance(raw_author, str) and str(raw_author).strip() + else [] + ) + if names and book.id is not None: + for name in names: + author = session.exec( + select(Author).where(Author.user_id == user_id, Author.name == name) + ).first() + if author is None: + author = Author(user_id=user_id, name=name) + session.add(author) + session.flush() + assert author.id is not None + session.add(BookAuthor(book_id=book.id, author_id=author.id)) + session.commit() + session.refresh(book) return book @@ -154,8 +178,22 @@ def test_batch_update_single_field(self, client: TestClient, session: Session) - session.refresh(b1) session.refresh(b2) - assert b1.author == "New Author" - assert b2.author == "New Author" + author_names = list( + session.exec( + select(Author.name) + .join(BookAuthor, col(BookAuthor.author_id) == col(Author.id)) + .where(BookAuthor.book_id == b1.id) + ).all() + ) + assert author_names == ["New Author"] + author_names = list( + session.exec( + select(Author.name) + .join(BookAuthor, col(BookAuthor.author_id) == col(Author.id)) + .where(BookAuthor.book_id == b2.id) + ).all() + ) + assert author_names == ["New Author"] def test_batch_update_too_many_ids(self, client: TestClient, session: Session) -> None: """Rejects more than 500 book IDs.""" @@ -393,7 +431,7 @@ def test_batch_update_database_error(self, client: TestClient, session: Session, """A database error during the update should return 500.""" from sqlalchemy.sql.dml import Update - b1 = _create_book(session, 1, title="B1", author="Old") + b1 = _create_book(session, 1, title="B1", published_year=2020) original_exec = session.exec def fake_exec(statement, *args, **kwargs): @@ -405,8 +443,8 @@ def fake_exec(statement, *args, **kwargs): resp = client.post("/api/hygiene/batch-update", json={ "book_ids": [b1.id], - "field": "author", - "value": "New", + "field": "published_year", + "value": 2021, }) assert resp.status_code == 500 assert resp.json()["detail"] == "Batch update failed due to a database error" diff --git a/backend/tests/test_statistics.py b/backend/tests/test_statistics.py index 9da70e36..f44a9769 100644 --- a/backend/tests/test_statistics.py +++ b/backend/tests/test_statistics.py @@ -31,6 +31,8 @@ def test_statistics_empty_library(client: Any) -> None: resp = client.get("/api/statistics") assert resp.status_code == 200 data = resp.json() + assert data["total_books"] == 0 + assert data["total_authors"] == 0 assert data["avg_books_per_month"] is None assert data["busiest_month"] is None assert data["avg_page_count"] is None @@ -83,6 +85,8 @@ def test_statistics_core_metrics_and_distributions(client: Any) -> None: assert resp.status_code == 200 data = resp.json() assert data["avg_books_per_month"] == 1.5 + assert data["total_books"] == 5 + assert data["total_authors"] == 2 assert data["busiest_month"] == "2026-01" assert data["busiest_month_count"] == 2 assert data["avg_page_count"] == 164.0 @@ -660,5 +664,5 @@ def test_statistics_top_and_worst_rated_books(client: Any) -> None: data = resp.json() assert data["books_with_rating"] == 4 assert data["average_rating"] == 3.5 - assert [b["title"] for b in data["top_rated_books"]] == ["Bad", "Okay", "Good", "Best"] - assert [b["title"] for b in data["worst_rated_books"]] == ["Best", "Good", "Okay", "Bad"] + assert [b["title"] for b in data["top_rated_books"]] == ["Best", "Good", "Okay", "Bad"] + assert [b["title"] for b in data["worst_rated_books"]] == ["Bad", "Okay", "Good", "Best"] diff --git a/backend/tests/test_transform_engine.py b/backend/tests/test_transform_engine.py index a83280fc..6b9b316f 100644 --- a/backend/tests/test_transform_engine.py +++ b/backend/tests/test_transform_engine.py @@ -135,10 +135,12 @@ def test_bool_return_becomes_string(self) -> None: result = te.execute_transform(fn, "x", {}, {}) assert result == "True" - def test_list_return_becomes_string(self) -> None: + def test_list_return_is_preserved(self) -> None: + """Non-string results (e.g. lists for the adaptive authors/tags targets) + are passed through untouched; scalar fields stringify in the caller.""" fn = te.compile_transform("return [1, 2, 3]") result = te.execute_transform(fn, "x", {}, {}) - assert result == "[1, 2, 3]" + assert result == [1, 2, 3] def test_no_return_statement(self) -> None: fn = te.compile_transform("x = value.strip()") diff --git a/docs/.vitepress/config.base.ts b/docs/.vitepress/config.base.ts index 26cad6ca..aa79461d 100644 --- a/docs/.vitepress/config.base.ts +++ b/docs/.vitepress/config.base.ts @@ -10,6 +10,12 @@ export default defineConfig({ __GIT_SHA__: JSON.stringify(gitSha), __GIT_SHA_SHORT__: JSON.stringify(gitSha.slice(0, 7)), }, + optimizeDeps: { + // fastdom is a CJS/UMD dependency of mermaid; without pre-bundling, + // Vite's dev server fails to expose its `default` export ("does not + // provide an export named 'default'"), which renders the dev site blank. + include: ['fastdom', 'fastdom/extensions/fastdom-promised.js'], + }, server: { host: true, port: 5174, diff --git a/docs/api/index.md b/docs/api/index.md index 53b8bee5..a972eb6a 100644 --- a/docs/api/index.md +++ b/docs/api/index.md @@ -46,8 +46,24 @@ curl -H "X-API-Key: YOUR_KEY_HERE" \ curl -X POST \ -H "X-API-Key: YOUR_KEY_HERE" \ -H "Content-Type: application/json" \ - -d '{"title": "The Great Gatsby", "author": "F. Scott Fitzgerald"}' \ + -d '{"title": "The Great Gatsby", "authors": ["F. Scott Fitzgerald"]}' \ http://localhost:8000/api/books +``` + +### Book author fields + +Book responses contain two author fields: + +- `author` — **deprecated**. The **joined** string of all authors (e.g. `"Neil Gaiman, Terry Pratchett"`). Kept for backward compatibility with existing consumers; use `authors` instead. +- `authors` — the **list** of individual author names (e.g. `["Neil Gaiman", "Terry Pratchett"]`). + +The `author` field is marked as **deprecated** in the OpenAPI spec (visible in Swagger UI) on all book schemas. It still works but may be removed in a future release. + +When creating a book you must provide at least one author — either `authors` as a list, or the legacy `author` string. If both are sent, `authors` takes precedence. A request with neither (or with an empty `authors` list) is rejected with a `422` validation error. + +For updates, `author`/`authors` are optional; if you send an empty `authors` list the book's authors are cleared. + +The legacy `author` string is **parsed on commas, tag-style** (e.g. `"Isaac Asimov, Frank Herbert"` becomes two authors). This only applies to the API create/update path. It differs from **file import** (CSV/JSON), where a single author string is split on `;`, ` & `, or ` and ` — never on commas — so a name like `"Asimov, Isaac"` stays one author. See [Import & Export](../guide/using-librislog/import-export.md) for the import behaviour. # Update reading status curl -X POST \ diff --git a/docs/guide/database-layout.md b/docs/guide/database-layout.md index db8e8e5b..8d96c83f 100644 --- a/docs/guide/database-layout.md +++ b/docs/guide/database-layout.md @@ -1,3 +1,372 @@ # Database Layout -> _Auto-generated from SQLModel metadata — run `npm run docs:gen-db` (or `uv run --directory backend python scripts/gen_db_docs.py`) to regenerate._ +> _Auto-generated from SQLModel metadata on 2026-08-24._ + +This page documents the LibrisLog database schema. It is intended for +developers who need to understand the data model, write queries, or extend +the application. + +```mermaid +erDiagram + + user ||--|{ apikey : "1:N" + user ||--|{ author : "1:N" + user ||--o{ book : "0..N" + user ||--|{ embed_token : "1:N" + user ||--|{ import_mapping : "1:N" + user ||--|| oidclink : "1:1" + user ||--|{ tag : "1:N" + user ||--|| usersettings : "1:1" + book ||--|{ book_author : "1:N" + author ||--|{ book_author : "1:N" + book ||--|{ book_tag : "1:N" + tag ||--|{ book_tag : "1:N" + book ||--|{ reading_progress : "1:N" + user ||--|{ reading_progress : "1:N" + + user { + integer id PK + varchar firstname + varchar lastname + varchar email UK + varchar role + varchar hashed_password + integer credentials_version + datetime created_at + datetime updated_at + } + + apikey { + integer id PK + integer user_id + varchar key_prefix + varchar key_hash UK + varchar key_encrypted + varchar description + datetime created_at + datetime last_used_at + datetime revoked_at + } + + author { + integer id PK + integer user_id + varchar name + datetime created_at + } + + book { + integer id PK + varchar title + varchar subtitle + varchar isbn + varchar cover_url + varchar publisher + integer published_year + integer page_count + varchar(2) language + varchar notes + varchar blurb + integer rating + varchar reading_status + varchar acquisition_status + integer user_id + datetime date_added + datetime date_started + datetime date_finished + } + + embed_token { + integer id PK + integer user_id + varchar(255) name + varchar token_prefix + varchar token_hash UK + varchar scopes + varchar allowed_origins + datetime expires_at + datetime last_used_at + datetime created_at + datetime revoked_at + } + + import_mapping { + integer id PK + integer user_id + varchar(255) name + varchar(64) schema_fingerprint + varchar source_fields_json + varchar mapping_json + datetime created_at + datetime updated_at + } + + oidclink { + integer id PK + integer user_id UK + varchar provider_id + varchar oidc_sub UK + varchar oidc_email + varchar oidc_name + datetime linked_at + } + + tag { + integer id PK + integer user_id + varchar name + datetime created_at + } + + usersettings { + integer id PK + integer user_id UK + varchar(10) language + varchar(64) timezone + varchar(20) theme + varchar(30) custom_theme + } + + book_author { + integer book_id PK + integer author_id PK + } + + book_tag { + integer book_id PK + integer tag_id PK + } + + reading_progress { + integer id PK + integer book_id + integer user_id + integer page + datetime created_at + datetime updated_at + } + +``` + +## Tables + +### `user` + +A user account. + +| Column | Type | Constraints | Notes | +|--------|------|-------------|-------| +| `id` | `INTEGER` | PK | Auto-increment | +| `firstname` | `VARCHAR` | NOT NULL | | +| `lastname` | `VARCHAR` | NOT NULL | | +| `email` | `VARCHAR` | UNIQUE, NOT NULL | | +| `role` | `VARCHAR` | NOT NULL, INDEX | default `user` | +| `hashed_password` | `VARCHAR` | NOT NULL | | +| `credentials_version` | `INTEGER` | NOT NULL | default 0 | +| `created_at` | `DATETIME` | | UTC | +| `updated_at` | `DATETIME` | | UTC | + +### `apikey` + +API key for programmatic access. + +| Column | Type | Constraints | Notes | +|--------|------|-------------|-------| +| `id` | `INTEGER` | PK | Auto-increment | +| `user_id` | `INTEGER` | FK → user.id, NOT NULL, INDEX | | +| `key_prefix` | `VARCHAR` | NOT NULL, INDEX | | +| `key_hash` | `VARCHAR` | UNIQUE, NOT NULL | | +| `key_encrypted` | `VARCHAR` | | | +| `description` | `VARCHAR` | | | +| `created_at` | `DATETIME` | | UTC | +| `last_used_at` | `DATETIME` | | UTC | +| `revoked_at` | `DATETIME` | | UTC | + +### `author` + +A user-specific author name that can be associated with books. + +| Column | Type | Constraints | Notes | +|--------|------|-------------|-------| +| `id` | `INTEGER` | PK | Auto-increment | +| `user_id` | `INTEGER` | FK → user.id, NOT NULL, INDEX | | +| `name` | `VARCHAR` | NOT NULL, INDEX | | +| `created_at` | `DATETIME` | | UTC | + +**Unique constraint:** `(user_id, name)` — uq_author_user_id_name + +### `book` + +A book in the user's library. + +| Column | Type | Constraints | Notes | +|--------|------|-------------|-------| +| `id` | `INTEGER` | PK | Auto-increment | +| `title` | `VARCHAR` | NOT NULL, INDEX | | +| `subtitle` | `VARCHAR` | | | +| `isbn` | `VARCHAR` | | | +| `cover_url` | `VARCHAR` | | | +| `publisher` | `VARCHAR` | | | +| `published_year` | `INTEGER` | | | +| `page_count` | `INTEGER` | NOT NULL | default 0 | +| `language` | `VARCHAR(2)` | | | +| `notes` | `VARCHAR` | | | +| `blurb` | `VARCHAR` | | | +| `rating` | `INTEGER` | | ≥ 1; ≤ 5 | +| `reading_status` | `VARCHAR` | NOT NULL, INDEX | default `want_to_read` | +| `acquisition_status` | `VARCHAR` | NOT NULL, INDEX | default `owned` | +| `user_id` | `INTEGER` | FK → user.id, INDEX | | +| `date_added` | `DATETIME` | INDEX | UTC | +| `date_started` | `DATETIME` | INDEX | UTC | +| `date_finished` | `DATETIME` | INDEX | UTC | + +**Unique constraint:** `(user_id, isbn)` — uq_book_user_id_isbn + +### `embed_token` + +A scoped embed token for iframe/dashboard integrations. + +| Column | Type | Constraints | Notes | +|--------|------|-------------|-------| +| `id` | `INTEGER` | PK | Auto-increment | +| `user_id` | `INTEGER` | FK → user.id, NOT NULL, INDEX | | +| `name` | `VARCHAR(255)` | NOT NULL | | +| `token_prefix` | `VARCHAR` | NOT NULL, INDEX | | +| `token_hash` | `VARCHAR` | UNIQUE, NOT NULL | | +| `scopes` | `VARCHAR` | NOT NULL | default `embed:stats:read` | +| `allowed_origins` | `VARCHAR` | | | +| `expires_at` | `DATETIME` | | UTC | +| `last_used_at` | `DATETIME` | | UTC | +| `created_at` | `DATETIME` | | UTC | +| `revoked_at` | `DATETIME` | | UTC | + +### `import_mapping` + +A saved column-mapping configuration for data import. + +| Column | Type | Constraints | Notes | +|--------|------|-------------|-------| +| `id` | `INTEGER` | PK | Auto-increment | +| `user_id` | `INTEGER` | FK → user.id, NOT NULL, INDEX | | +| `name` | `VARCHAR(255)` | NOT NULL | | +| `schema_fingerprint` | `VARCHAR(64)` | NOT NULL, INDEX | | +| `source_fields_json` | `VARCHAR` | NOT NULL | | +| `mapping_json` | `VARCHAR` | NOT NULL | | +| `created_at` | `DATETIME` | | UTC | +| `updated_at` | `DATETIME` | | UTC | + +**Unique constraint:** `(user_id, name)` — uq_import_mapping_user_id_name + +### `oidclink` + +Links an OIDC identity to a local user account. + +| Column | Type | Constraints | Notes | +|--------|------|-------------|-------| +| `id` | `INTEGER` | PK | Auto-increment | +| `user_id` | `INTEGER` | FK → user.id, UNIQUE, NOT NULL | | +| `provider_id` | `VARCHAR` | NOT NULL, INDEX | | +| `oidc_sub` | `VARCHAR` | UNIQUE, NOT NULL | | +| `oidc_email` | `VARCHAR` | | | +| `oidc_name` | `VARCHAR` | | | +| `linked_at` | `DATETIME` | | UTC | + +### `tag` + +A user-specific tag that can be applied to books. + +| Column | Type | Constraints | Notes | +|--------|------|-------------|-------| +| `id` | `INTEGER` | PK | Auto-increment | +| `user_id` | `INTEGER` | FK → user.id, NOT NULL, INDEX | | +| `name` | `VARCHAR` | NOT NULL, INDEX | | +| `created_at` | `DATETIME` | | UTC | + +**Unique constraint:** `(user_id, name)` — uq_tag_user_id_name + +### `usersettings` + +Per-user settings such as language, timezone, and theme. + +| Column | Type | Constraints | Notes | +|--------|------|-------------|-------| +| `id` | `INTEGER` | PK | Auto-increment | +| `user_id` | `INTEGER` | FK → user.id, UNIQUE, NOT NULL | | +| `language` | `VARCHAR(10)` | NOT NULL | default `en` | +| `timezone` | `VARCHAR(64)` | NOT NULL | default `UTC` | +| `theme` | `VARCHAR(20)` | NOT NULL | default `light` | +| `custom_theme` | `VARCHAR(30)` | | | + +### `book_author` + +Many-to-many association between books and authors. + +| Column | Type | Constraints | Notes | +|--------|------|-------------|-------| +| `book_id` | `INTEGER` | PK, FK → book.id | | +| `author_id` | `INTEGER` | PK, FK → author.id | | + +### `book_tag` + +Many-to-many association between books and tags. + +| Column | Type | Constraints | Notes | +|--------|------|-------------|-------| +| `book_id` | `INTEGER` | PK, FK → book.id | | +| `tag_id` | `INTEGER` | PK, FK → tag.id | | + +### `reading_progress` + +A page-number reading progress entry for a book. + +| Column | Type | Constraints | Notes | +|--------|------|-------------|-------| +| `id` | `INTEGER` | PK | Auto-increment | +| `book_id` | `INTEGER` | FK → book.id, NOT NULL, INDEX | | +| `user_id` | `INTEGER` | FK → user.id, NOT NULL, INDEX | | +| `page` | `INTEGER` | NOT NULL | ≥ 0 | +| `created_at` | `DATETIME` | | UTC | +| `updated_at` | `DATETIME` | | UTC | + + +## Enums + +### `AcquisitionStatus` + +| Value | Meaning | +|-------|---------| +| `owned` | Owned | +| `borrowed` | Borrowed | +| `digital_access` | Digital Access | +| `to_acquire` | To Acquire | + +### `ReadingStatus` + +| Value | Meaning | +|-------|---------| +| `want_to_read` | Want To Read | +| `currently_reading` | Currently Reading | +| `read` | Read | +| `did_not_finish` | Did Not Finish | + +### `UserRole` + +| Value | Meaning | +|-------|---------| +| `admin` | Admin | +| `user` | User | + + +## Conventions + +- **Timestamps** are stored as UTC via the `UtcDateTime` type decorator. + Values are stored as naive UTC in SQLite and returned as timezone-aware + `datetime` objects by the application. +- **Soft deletes** — `ApiKey` and `EmbedToken` use a `revoked_at` timestamp + instead of `DELETE`. Revoked entries are excluded from all queries. +- **Foreign keys** — all user-owned tables reference `user.id` via foreign + key constraints. Cascading behavior is handled in application code (not + at the database level). +- **Unique constraints** — compound constraints like `(user_id, isbn)` on + `book` and `(user_id, name)` on `tag` enforce per-user uniqueness without + restricting other users. diff --git a/docs/guide/using-librislog/import-export.md b/docs/guide/using-librislog/import-export.md index dd5fbe75..8b21b094 100644 --- a/docs/guide/using-librislog/import-export.md +++ b/docs/guide/using-librislog/import-export.md @@ -29,6 +29,12 @@ On mobile devices: If no search results are found, enter book details manually. Title, author, page count, and availability are required; all other fields are optional. +Authors can be added as multiple values: type a name and press **Enter** (or pick a suggestion) to add a chip. A book can have any number of authors. Commas inside an author name (e.g. `Asimov, Isaac`) are preserved — they are not treated as separators. + +### Search Import + +When a search source returns multiple authors for a book (e.g. Open Library, Google Books, or Hardcover), the app keeps them as a list and creates one author per entry. Sources that return a single combined string are split only on `;`, ` & `, or ` and ` — never on commas. + ## Data Export Export your entire library or subsets of data: @@ -67,6 +73,10 @@ Import data from external sources: - **JSON** — LibrisLog export format - **CSV** — Custom field mapping supported +The JSON export mirrors the API shape: `author` is the joined string (separated with `; `), `authors` is the list of names, and `tags` is a list of tag names. All three round-trip through the adaptive import. + +For CSV files, a **delimiter** field appears once a `.csv` file is selected (default `,`). Enter the character your file uses to separate columns (e.g. `;` for German/Excel exports) before clicking **Parse file**. + ### Field Mapping When importing CSV, map source columns to LibrisLog fields: @@ -76,6 +86,19 @@ When importing CSV, map source columns to LibrisLog fields: `acquisition_status` is required for imports. Map it to one of `owned`, `borrowed`, `digital_access`, or `to_acquire`; use a transform when the source file uses different names. +`date_added` is importable too — useful when migrating from another tool so the original "added to library" dates are preserved (the LibrisLog JSON export includes it, so exports round-trip losslessly). If a row has no `date_added`, the import timestamp is used. + +#### Authors are adaptive + +The import target field is **`authors`**. Its source value adapts: + +- **Array value** (e.g. a JSON `authors` list) → each array entry becomes a separate author. +- **String value** (e.g. a CSV cell) → normally becomes **one** author, and commas inside the name are preserved, so `"Asimov, Isaac"` stays a single author. To encode several authors in a single cell, separate them with `;`, ` & `, or ` and ` (e.g. `"Frank Herbert; Brian Herbert"`). This is how the CSV export writes the dedicated `authors` column, so exports round-trip losslessly. + +The import preview shows how each row's author value will be interpreted before you import. + +The `tags` field is adaptive too: a JSON `tags` array contributes one tag per entry, while a comma-separated string (CSV) is split on commas. + ### Transform DSL Per-field Python expressions allow data transformation: diff --git a/docs/guide/using-librislog/library.md b/docs/guide/using-librislog/library.md index 76436e34..d8bdf321 100644 --- a/docs/guide/using-librislog/library.md +++ b/docs/guide/using-librislog/library.md @@ -54,6 +54,8 @@ Clicking a book opens the detail dialog/drawer showing: Use the "Add Book" button to manually enter book details. Fill in title, author, and optional fields like ISBN, publisher, page count, etc. +A book can have **multiple authors**: type a name and press **Enter** to add it as a chip. Authors are shown joined with "; " throughout the app, so names written last-name-first (e.g. `"Doe, Jane"`) stay unambiguous. + ### Import Search Search external sources for book metadata: diff --git a/docs/guide/using-librislog/search.md b/docs/guide/using-librislog/search.md index 6b457843..8e08d2e1 100644 --- a/docs/guide/using-librislog/search.md +++ b/docs/guide/using-librislog/search.md @@ -19,6 +19,8 @@ Use `:` to search in a single field. The field prefixes are always Use quotes for values that contain spaces: `title:"The Silmarillion"`. +The `author:` prefix matches **any** author assigned to a book — a book with multiple authors matches if any of them contains the search value. + ### Availability values The `availability` prefix matches the exact acquisition status. Accepted values include: diff --git a/docs/guide/using-librislog/statistics.md b/docs/guide/using-librislog/statistics.md index bc934127..22964082 100644 --- a/docs/guide/using-librislog/statistics.md +++ b/docs/guide/using-librislog/statistics.md @@ -4,7 +4,7 @@ The statistics page provides insights into your reading habits with charts, tota ## Overview Cards -At the top of the statistics page, four key metrics are displayed: +At the top of the statistics page, five key metrics are displayed: ![Statistics Overview](/screenshots/statistics.png) @@ -14,6 +14,7 @@ At the top of the statistics page, four key metrics are displayed: | **Busiest Month** | The month with the most books finished | | **Avg Page Count** | Average number of pages across all books | | **Most Popular Language** | The language you read most (based on book count) | +| **Total Books** | Total number of books, plus how many different authors they're from | ## Distribution Charts diff --git a/frontend/e2e/fixtures/pages/add-book-modal.page.ts b/frontend/e2e/fixtures/pages/add-book-modal.page.ts index 006ec7cf..7ccbaf2a 100644 --- a/frontend/e2e/fixtures/pages/add-book-modal.page.ts +++ b/frontend/e2e/fixtures/pages/add-book-modal.page.ts @@ -16,6 +16,7 @@ export class AddBookModalPage { async fillAuthor(author: string) { const input = this.page.locator('[role="dialog"] input[name="author"], [role="dialog"] input[placeholder*="Author"]').first(); await input.fill(author); + await input.press('Enter'); } async clickSave() { diff --git a/frontend/e2e/fixtures/pages/book-drawer.page.ts b/frontend/e2e/fixtures/pages/book-drawer.page.ts index 4ef1ec92..f41f3d38 100644 --- a/frontend/e2e/fixtures/pages/book-drawer.page.ts +++ b/frontend/e2e/fixtures/pages/book-drawer.page.ts @@ -15,6 +15,7 @@ export class BookDrawerPage { async fillAuthor(author: string) { const input = this.page.locator('input[name="author"], input[placeholder*="Author"]').first(); await input.fill(author); + await input.press('Enter'); } async clickSave() { diff --git a/frontend/e2e/fixtures/seed-data.ts b/frontend/e2e/fixtures/seed-data.ts index a950a6dd..478f4e2d 100644 --- a/frontend/e2e/fixtures/seed-data.ts +++ b/frontend/e2e/fixtures/seed-data.ts @@ -8,6 +8,7 @@ export const SEED_USER = { export interface SeedBook { title: string; author: string; + authors?: string[]; isbn?: string; page_count?: number; reading_status: 'want_to_read' | 'currently_reading' | 'read' | 'did_not_finish'; @@ -27,7 +28,8 @@ export const SEED_BOOKS: SeedBook[] = [ { title: '1984', author: 'George Orwell', isbn: '9780451524935', reading_status: 'read', rating: 5, page_count: 328, date_started: '2024-10-01', date_finished: '2024-10-20' }, { title: 'Brave New World', author: 'Aldous Huxley', isbn: '9780060850524', reading_status: 'read', rating: 4, page_count: 311, date_started: '2024-09-01', date_finished: '2024-09-18' }, { title: 'Atlas Shrugged', author: 'Ayn Rand', reading_status: 'did_not_finish', page_count: 1168 }, - { title: 'Die Fragezeichen', author: 'Christoph Dittert', reading_status: 'want_to_read' }, - { title: 'Cars & Mercedes', author: 'Jane Driver', reading_status: 'want_to_read', tags: 'cars,audi' }, - { title: 'Cars Only', author: 'Jane Driver', reading_status: 'want_to_read', tags: 'cars' }, + { title: 'Die Fragezeichen', author: 'Christoph Dittert', reading_status: 'want_to_read', page_count: 96 }, + { title: 'Cars & Mercedes', author: 'Jane Driver', reading_status: 'want_to_read', tags: 'cars,audi', page_count: 120 }, + { title: 'Cars Only', author: 'Jane Driver', reading_status: 'want_to_read', tags: 'cars', page_count: 100 }, + { title: 'Good Omens', author: 'Terry Pratchett, Neil Gaiman', authors: ['Terry Pratchett', 'Neil Gaiman'], isbn: '9780060853983', reading_status: 'want_to_read', page_count: 288 }, ]; diff --git a/frontend/e2e/fixtures/seed.api.ts b/frontend/e2e/fixtures/seed.api.ts index cfb81987..b1c30086 100644 --- a/frontend/e2e/fixtures/seed.api.ts +++ b/frontend/e2e/fixtures/seed.api.ts @@ -18,13 +18,17 @@ async function getCsrfToken(page: Page): Promise { export async function seedBooks(page: Page, books: SeedBook[]): Promise { for (const book of books) { const csrf = await getCsrfToken(page); - await page.request.post(bookApiPath(), { + const resp = await page.request.post(bookApiPath(), { data: book, headers: { 'Content-Type': 'application/json', 'X-CSRF-Token': csrf, }, }); + // 409 = book already seeded by an earlier test in this suite; that's fine. + if (!resp.ok() && resp.status() !== 409) { + throw new Error(`Seeding book "${book.title}" failed: ${resp.status()} ${await resp.text()}`); + } } } diff --git a/frontend/e2e/specs/03-library-browsing.spec.ts b/frontend/e2e/specs/03-library-browsing.spec.ts index d4f4be17..0971e9c6 100644 --- a/frontend/e2e/specs/03-library-browsing.spec.ts +++ b/frontend/e2e/specs/03-library-browsing.spec.ts @@ -78,7 +78,10 @@ test.describe('Library Browsing', () => { await expect(availability).toHaveValue(''); await modal.getByLabel('Title *').fill('Digital E2E Book'); - await modal.getByRole('searchbox', { name: /Author/ }).fill('E2E Author'); + // The author field is a chip input: type and press Enter to add the author. + const authorInput = modal.getByRole('textbox', { name: /Author/ }); + await authorInput.fill('E2E Author'); + await authorInput.press('Enter'); await modal.getByLabel(/Pages/).fill('200'); await availability.selectOption('digital_access'); await modal.getByRole('button', { name: 'Add Book' }).click(); diff --git a/frontend/e2e/specs/05-edit-book.spec.ts b/frontend/e2e/specs/05-edit-book.spec.ts index e9a46280..89940acf 100644 --- a/frontend/e2e/specs/05-edit-book.spec.ts +++ b/frontend/e2e/specs/05-edit-book.spec.ts @@ -88,4 +88,41 @@ test.describe('Edit Book', () => { const checkedStar = page.locator('[role="dialog"] input[type="radio"]:checked'); await expect(checkedStar).toHaveAttribute('aria-label', /2 (star|Stern)/); }); + + test('5.4 add a second author in the edit drawer', async ({ page }) => { + const library = new LibraryPage(page); + await library.goto(); + await page.waitForTimeout(1000); + + await library.switchTab('want to read'); + await page.waitForTimeout(500); + + const cards = library.getBookCards(); + await expect(cards.first()).toBeVisible({ timeout: 5000 }); + + // Pick the Dune card (author: Frank Herbert). + const duneCard = cards.filter({ hasText: 'Dune' }).first(); + await expect(duneCard).toBeVisible({ timeout: 5000 }); + await duneCard.click(); + await page.waitForSelector('[role="dialog"]', { timeout: 5000 }); + + const editBtn = page.locator('[role="dialog"] button').filter({ hasText: 'Edit' }); + await editBtn.first().click(); + await page.waitForTimeout(500); + + const authorInput = page.locator('input[name="author"]'); + await expect(authorInput).toBeVisible({ timeout: 5000 }); + + // Add a second author chip by typing and pressing Enter. + await authorInput.fill('Brian Herbert'); + await authorInput.press('Enter'); + await expect(page.locator('text=Brian Herbert')).toBeVisible({ timeout: 5000 }); + + await page.locator('button[type="submit"]').click(); + await page.waitForTimeout(800); + + // Verify the joined author string appears in the library detail. + // Authors are returned sorted alphabetically: Brian Herbert, Frank Herbert. + await expect(page.getByText(/Brian Herbert; Frank Herbert/i)).toBeVisible({ timeout: 5000 }); + }); }); diff --git a/frontend/e2e/specs/06-data-hygiene.spec.ts b/frontend/e2e/specs/06-data-hygiene.spec.ts index 2510ae9e..054a90fa 100644 --- a/frontend/e2e/specs/06-data-hygiene.spec.ts +++ b/frontend/e2e/specs/06-data-hygiene.spec.ts @@ -1,6 +1,6 @@ import { test, expect } from '@playwright/test'; import { loginViaUi } from '../fixtures/auth.fixture'; -import { deleteAllBooks } from '../fixtures/seed.api'; +import { deleteAllBooks, seedBooks } from '../fixtures/seed.api'; import { SEED_USER } from '../fixtures/seed-data'; test.describe('Data Hygiene', () => { @@ -9,23 +9,34 @@ test.describe('Data Hygiene', () => { await deleteAllBooks(page); // Seed books with missing attributes. - // author, title, page_count are mandatory in BookCreate, so - // "missing" means empty string for author / 0 for page_count. + // title and page_count are mandatory in BookCreate, so "missing" + // means 0 for page_count. Author is mandatory at creation too, + // so the "Missing Author" book starts with a placeholder author + // whose authors list is cleared afterwards via PATCH (updates + // allow emptying the authors list). const books = [ { title: 'Complete Book', author: 'Test Author', isbn: '9780000000001', publisher: 'Test Pub', page_count: 200, reading_status: 'want_to_read' as const }, - { title: 'Missing Author', author: '', isbn: '9780000000002', publisher: 'Test Pub', page_count: 150, reading_status: 'want_to_read' as const }, + { title: 'Missing Author', author: 'Placeholder Author', isbn: '9780000000002', publisher: 'Test Pub', page_count: 150, reading_status: 'want_to_read' as const }, { title: 'Missing ISBN', author: 'No ISBN', page_count: 300, reading_status: 'want_to_read' as const }, { title: 'Missing Page Count', author: 'Page Author', page_count: 0, reading_status: 'want_to_read' as const }, { title: 'Missing Publisher', author: 'Pub Missing', isbn: '9780000000003', page_count: 250, reading_status: 'want_to_read' as const }, ]; - - for (const book of books) { - const csrfResp = await page.request.get('/api/auth/csrf'); - const { csrf_token } = await csrfResp.json(); - await page.request.post('/api/books', { - data: book, - headers: { 'Content-Type': 'application/json', 'X-CSRF-Token': csrf_token }, - }); + await seedBooks(page, books); + + const csrfResp = await page.request.get('/api/auth/csrf'); + const { csrf_token } = await csrfResp.json(); + const listResp = await page.request.get('/api/books?limit=200'); + const { books: created } = await listResp.json(); + const missingAuthor = (created as Array<{ id: number; title: string }>).find( + (book) => book.title === 'Missing Author' + ); + if (!missingAuthor) throw new Error('Seeded book "Missing Author" not found.'); + const patchResp = await page.request.patch(`/api/books/${missingAuthor.id}`, { + data: { authors: [] }, + headers: { 'Content-Type': 'application/json', 'X-CSRF-Token': csrf_token }, + }); + if (!patchResp.ok()) { + throw new Error(`Clearing authors failed: ${patchResp.status()} ${await patchResp.text()}`); } }); diff --git a/frontend/e2e/specs/08-statistics.spec.ts b/frontend/e2e/specs/08-statistics.spec.ts index 225aeb77..1fa43a5d 100644 --- a/frontend/e2e/specs/08-statistics.spec.ts +++ b/frontend/e2e/specs/08-statistics.spec.ts @@ -33,4 +33,24 @@ test.describe('Statistics', () => { await expect(page.getByText('The Great Gatsby').first()).toBeVisible(); await expect(page.getByText('Brave New World').first()).toBeVisible(); }); + + test('8.3 top authors reflect multi-author books', async ({ page }) => { + // Give Frank Herbert more books (Good Omens already counts Pratchett/Gaiman). + await seedBooks(page, [ + { title: 'Children of Dune', author: 'Frank Herbert', reading_status: 'read', rating: 4, page_count: 408, date_started: '2024-01-01', date_finished: '2024-01-20' }, + { title: 'God Emperor of Dune', author: 'Frank Herbert', reading_status: 'read', rating: 4, page_count: 496, date_started: '2024-02-01', date_finished: '2024-02-20' }, + ]); + + await page.goto('/statistics'); + await page.waitForTimeout(2000); + + // Frank Herbert now has the most books and must be the top author. + await expect(page.getByText(/Top Authors|Beliebteste Autoren/i)).toBeVisible(); + const topAuthorsCard = page + .locator('.card') + .filter({ has: page.getByRole('heading', { name: /Top Authors|Beliebteste Autoren/i }) }); + const frankCard = topAuthorsCard.locator('.rounded-xl').filter({ hasText: 'Frank Herbert' }).first(); + await expect(frankCard).toBeVisible({ timeout: 5000 }); + await expect(frankCard.getByText('#1', { exact: true })).toBeVisible(); + }); }); diff --git a/frontend/e2e/specs/09-data-import.spec.ts b/frontend/e2e/specs/09-data-import.spec.ts index 6364d8f5..0c6c8474 100644 --- a/frontend/e2e/specs/09-data-import.spec.ts +++ b/frontend/e2e/specs/09-data-import.spec.ts @@ -39,7 +39,7 @@ test.describe('Data Import', () => { await expect(page.locator('select[name="mapping-target-title"]')).toBeVisible({ timeout: 10000 }); await page.locator('select[name="mapping-target-title"]').selectOption('title'); - await page.locator('select[name="mapping-target-author"]').selectOption('author'); + await page.locator('select[name="mapping-target-authors"]').selectOption('author'); await page.locator('select[name="mapping-target-isbn"]').selectOption('isbn'); await page.locator('select[name="mapping-target-page_count"]').selectOption('pages'); await page.locator('select[name="mapping-target-reading_status"]').selectOption('status'); @@ -95,7 +95,7 @@ test.describe('Data Import', () => { await expect(page.locator('select[name="mapping-target-title"]')).toBeVisible({ timeout: 10000 }); await page.locator('select[name="mapping-target-title"]').selectOption('title'); - await page.locator('select[name="mapping-target-author"]').selectOption('author'); + await page.locator('select[name="mapping-target-authors"]').selectOption('author'); await page.locator('select[name="mapping-target-isbn"]').selectOption('isbn'); await page.locator('select[name="mapping-target-page_count"]').selectOption('pages'); await page.locator('select[name="mapping-target-reading_status"]').selectOption('status'); @@ -124,7 +124,7 @@ test.describe('Data Import', () => { await page.waitForTimeout(2000); await page.locator('select[name="mapping-target-title"]').selectOption('title'); - await page.locator('select[name="mapping-target-author"]').selectOption('author'); + await page.locator('select[name="mapping-target-authors"]').selectOption('author'); await page.locator('select[name="mapping-target-isbn"]').selectOption('isbn'); await page.locator('select[name="mapping-target-page_count"]').selectOption('pages'); await page.locator('select[name="mapping-target-reading_status"]').selectOption('status'); @@ -153,7 +153,7 @@ test.describe('Data Import', () => { await page.waitForTimeout(2000); await page.locator('select[name="mapping-target-title"]').selectOption('title'); - await page.locator('select[name="mapping-target-author"]').selectOption('author'); + await page.locator('select[name="mapping-target-authors"]').selectOption('author'); await page.locator('select[name="mapping-target-isbn"]').selectOption('isbn'); await page.locator('select[name="mapping-target-page_count"]').selectOption('pages'); await page.locator('select[name="mapping-target-reading_status"]').selectOption('status'); @@ -180,4 +180,99 @@ test.describe('Data Import', () => { expect(books).toHaveLength(1); expect(books[0].acquisition_status).toBe('to_acquire'); }); + + test('9.6 JSON author array maps to multiple authors', async ({ page }) => { + await deleteAllBooks(page); + await page.goto('/data?tab=import'); + await page.waitForTimeout(1000); + + const JSON_DATA = JSON.stringify([ + { + title: 'Good Omens', + authors: ['Terry Pratchett', 'Neil Gaiman'], + isbn: '9780060853983', + pages: 288, + status: 'want_to_read', + availability: 'owned', + }, + ]); + + await page.locator('input[type="file"]').setInputFiles({ + name: 'test-books.json', + mimeType: 'application/json', + buffer: Buffer.from(JSON_DATA), + }); + + await page.locator('button').filter({ hasText: 'Parse file' }).click(); + await page.waitForTimeout(2000); + + await page.locator('select[name="mapping-target-title"]').selectOption('title'); + await page.locator('select[name="mapping-target-authors"]').selectOption('authors'); + await page.locator('select[name="mapping-target-isbn"]').selectOption('isbn'); + await page.locator('select[name="mapping-target-page_count"]').selectOption('pages'); + await page.locator('select[name="mapping-target-reading_status"]').selectOption('status'); + await page.locator('select[name="mapping-target-acquisition_status"]').selectOption('availability'); + + await page.locator('button').filter({ hasText: 'Generate' }).click(); + await page.waitForTimeout(2000); + + await page.locator('button').filter({ hasText: 'Simulate' }).click(); + await page.waitForTimeout(2000); + + await expect(page.locator('body')).toContainText('Validation passed.', { timeout: 10000 }); + + await page.locator('button.btn-secondary.btn-sm').filter({ hasText: 'Import now' }).click(); + await page.locator('dialog.modal-open .btn-secondary').filter({ hasText: 'Import now' }).waitFor({ state: 'visible', timeout: 5000 }); + await page.locator('dialog.modal-open .btn-secondary').filter({ hasText: 'Import now' }).click(); + await page.waitForTimeout(2000); + + const response = await page.request.get('/api/books?q=Good%20Omens'); + const books = (await response.json()).books; + expect(books).toHaveLength(1); + expect(books[0].authors).toEqual(['Neil Gaiman', 'Terry Pratchett']); + }); + + test('9.7 CSV author cell with a comma stays a single author', async ({ page }) => { + await deleteAllBooks(page); + await page.goto('/data?tab=import'); + await page.waitForTimeout(1000); + + const CSV_COMMA = `title,author,isbn,pages,status,availability +"Fahrenheit 451","Bradbury, Ray","9781451673319",249,want_to_read,owned`; + + await page.locator('input[type="file"]').setInputFiles({ + name: 'test-books.csv', + mimeType: 'text/csv', + buffer: Buffer.from(CSV_COMMA), + }); + + await page.locator('button').filter({ hasText: 'Parse file' }).click(); + await page.waitForTimeout(2000); + + await page.locator('select[name="mapping-target-title"]').selectOption('title'); + await page.locator('select[name="mapping-target-authors"]').selectOption('author'); + await page.locator('select[name="mapping-target-isbn"]').selectOption('isbn'); + await page.locator('select[name="mapping-target-page_count"]').selectOption('pages'); + await page.locator('select[name="mapping-target-reading_status"]').selectOption('status'); + await page.locator('select[name="mapping-target-acquisition_status"]').selectOption('availability'); + + await page.locator('button').filter({ hasText: 'Generate' }).click(); + await page.waitForTimeout(2000); + + await page.locator('button').filter({ hasText: 'Simulate' }).click(); + await page.waitForTimeout(2000); + + await expect(page.locator('body')).toContainText('Validation passed.', { timeout: 10000 }); + + await page.locator('button.btn-secondary.btn-sm').filter({ hasText: 'Import now' }).click(); + await page.locator('dialog.modal-open .btn-secondary').filter({ hasText: 'Import now' }).waitFor({ state: 'visible', timeout: 5000 }); + await page.locator('dialog.modal-open .btn-secondary').filter({ hasText: 'Import now' }).click(); + await page.waitForTimeout(2000); + + const response = await page.request.get('/api/books?q=Fahrenheit'); + const books = (await response.json()).books; + expect(books).toHaveLength(1); + expect(books[0].authors).toEqual(['Bradbury, Ray']); + expect(books[0].author).toBe('Bradbury, Ray'); + }); }); diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index b6d25b04..30ae7ffb 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -529,13 +529,14 @@ export const api = { return res.blob(); }, - async parseImportFile(file: File): Promise { + async parseImportFile(file: File, delimiter = ','): Promise { const headers: Record = { ...authHeaders() }; const csrf = get(csrfToken); if (csrf) headers['X-CSRF-Token'] = csrf; const form = new FormData(); form.append('file', file); + form.append('delimiter', delimiter); const res = await fetch(`${BASE}/data/import/parse`, { method: 'POST', headers, diff --git a/frontend/src/lib/components/AddBookModal.svelte b/frontend/src/lib/components/AddBookModal.svelte index e2a67784..9bb46695 100644 --- a/frontend/src/lib/components/AddBookModal.svelte +++ b/frontend/src/lib/components/AddBookModal.svelte @@ -28,7 +28,7 @@ // Manual form state let title = $state(''); let subtitle = $state(''); - let author = $state(''); + let authors = $state([]); let isbn = $state(''); let publisher = $state(''); let published_year = $state(''); @@ -46,7 +46,7 @@ function reset() { title = ''; subtitle = ''; - author = ''; + authors = []; isbn = ''; publisher = ''; published_year = ''; @@ -64,7 +64,7 @@ async function submitManual() { if (!title.trim()) return; - if (!author.trim()) return; + if (authors.length === 0) return; if (!page_count) return; if (!acquisitionStatus) return; submitting = true; @@ -72,7 +72,7 @@ const book = await api.books.create({ title: title.trim(), subtitle: subtitle || null, - author: author.trim(), + authors, isbn: isbn || null, publisher: publisher || null, published_year: published_year ? parseInt(published_year) : null, @@ -149,11 +149,12 @@
- api.books.suggestions.authors(q)} />