From 3ae6cf88cf4f265249daf46b55968c1bf04dd35c Mon Sep 17 00:00:00 2001 From: codebude Date: Thu, 22 Oct 2026 00:23:59 +0200 Subject: [PATCH 01/25] Backend: reading sessions/re-reads (issue #114) - Add ReadingSession model + ReadingProgress.session_id - Alembic migration with per-book backfill - Session service (mirror, reconcile, read-again, ordinals) - Sessions router (CRUD, read-again, timeline) - Session-aware progress and statistics (total_reads, reread_books) - Export sessions dataset, user deletion, import session creation --- .../a3b4c5d6e7f8_add_reading_session_table.py | 123 +++++++ backend/app/main.py | 3 +- backend/app/models.py | 46 +++ backend/app/routers/books.py | 21 +- backend/app/routers/progress.py | 107 ++++-- backend/app/routers/sessions.py | 304 +++++++++++++++++ backend/app/routers/statistics.py | 105 ++++-- backend/app/schemas.py | 68 +++- backend/app/services/data_export.py | 35 +- backend/app/services/data_import.py | 6 +- backend/app/services/reading_sessions.py | 289 ++++++++++++++++ backend/app/services/statistics.py | 321 +++++++++++++----- backend/app/services/tags.py | 7 + backend/app/services/user_deletion.py | 10 +- backend/tests/test_reading_sessions.py | 122 +++++++ 15 files changed, 1409 insertions(+), 158 deletions(-) create mode 100644 backend/alembic/versions/a3b4c5d6e7f8_add_reading_session_table.py create mode 100644 backend/app/routers/sessions.py create mode 100644 backend/app/services/reading_sessions.py create mode 100644 backend/tests/test_reading_sessions.py diff --git a/backend/alembic/versions/a3b4c5d6e7f8_add_reading_session_table.py b/backend/alembic/versions/a3b4c5d6e7f8_add_reading_session_table.py new file mode 100644 index 00000000..739d6b2f --- /dev/null +++ b/backend/alembic/versions/a3b4c5d6e7f8_add_reading_session_table.py @@ -0,0 +1,123 @@ +"""add reading_session table and reading_progress.session_id + +Revision ID: a3b4c5d6e7f8 +Revises: d7e8f9a0b1c2 +Create Date: 2026-10-22 00:00:00.000000 + +""" +from datetime import datetime, timezone +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision: str = 'a3b4c5d6e7f8' +down_revision: Union[str, Sequence[str], None] = 'd7e8f9a0b1c2' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def _session_status(status: str, date_started, date_finished) -> str: + """Map a book's reading status to the equivalent session status.""" + if status == 'currently_reading': + return 'currently_reading' + if status == 'did_not_finish': + return 'did_not_finish' + if status == 'read': + return 'read' + # want_to_read book that still carries reading evidence + return 'read' if date_finished is not None else 'currently_reading' + + +def upgrade() -> None: + op.create_table('reading_session', + sa.Column('id', sa.Integer(), nullable=False), + sa.Column('book_id', sa.Integer(), nullable=False), + sa.Column('user_id', sa.Integer(), nullable=False), + sa.Column('status', sa.VARCHAR(), nullable=False), + sa.Column('date_started', sa.DateTime(), nullable=True), + sa.Column('date_finished', sa.DateTime(), nullable=True), + sa.Column('is_current', sa.Boolean(), nullable=False, server_default=sa.text('0')), + sa.Column('created_at', sa.DateTime(), nullable=False), + sa.Column('updated_at', sa.DateTime(), nullable=False), + sa.ForeignKeyConstraint(['book_id'], ['book.id'], ondelete='CASCADE'), + sa.ForeignKeyConstraint(['user_id'], ['user.id'], ), + sa.PrimaryKeyConstraint('id'), + ) + op.create_index(op.f('ix_reading_session_book_id'), 'reading_session', ['book_id'], unique=False) + op.create_index(op.f('ix_reading_session_user_id'), 'reading_session', ['user_id'], unique=False) + op.create_index(op.f('ix_reading_session_status'), 'reading_session', ['status'], unique=False) + op.create_index(op.f('ix_reading_session_date_started'), 'reading_session', ['date_started'], unique=False) + op.create_index(op.f('ix_reading_session_date_finished'), 'reading_session', ['date_finished'], unique=False) + op.create_index(op.f('ix_reading_session_is_current'), 'reading_session', ['is_current'], unique=False) + op.create_index( + 'uq_reading_session_book_current', 'reading_session', ['book_id'], + unique=True, sqlite_where=sa.text('is_current = 1'), + ) + + op.add_column('reading_progress', sa.Column('session_id', sa.Integer(), nullable=True)) + op.create_index(op.f('ix_reading_progress_session_id'), 'reading_progress', ['session_id'], unique=False) + op.create_foreign_key( + 'fk_reading_progress_session_id', 'reading_progress', 'reading_session', + ['session_id'], ['id'], + ) + + bind = op.get_bind() + now = datetime.now(timezone.utc).replace(tzinfo=None) + + # Backfill: one session per book with reading evidence, mirroring the book + # dates verbatim so a single-session book behaves identically to before. + books = bind.execute( + sa.text("SELECT id, user_id, reading_status, date_started, date_finished FROM book") + ).fetchall() + for book_id, user_id, status, date_started, date_finished in books: + has_progress = bind.execute( + sa.text("SELECT 1 FROM reading_progress WHERE book_id = :bid LIMIT 1"), + {"bid": book_id}, + ).first() + if status == 'want_to_read' and date_started is None and date_finished is None and not has_progress: + continue + + session_status = _session_status(status, date_started, date_finished) + is_current = 1 if session_status == 'currently_reading' else 0 + bind.execute( + sa.text( + "INSERT INTO reading_session " + "(book_id, user_id, status, date_started, date_finished, is_current, created_at, updated_at) " + "VALUES (:book_id, :user_id, :status, :date_started, :date_finished, :is_current, :now, :now)" + ), + { + "book_id": book_id, + "user_id": user_id, + "status": session_status, + "date_started": date_started, + "date_finished": date_finished, + "is_current": is_current, + "now": now, + }, + ) + session_id = bind.execute( + sa.text("SELECT id FROM reading_session WHERE book_id = :bid ORDER BY id DESC LIMIT 1"), + {"bid": book_id}, + ).scalar() + if session_id is not None: + bind.execute( + sa.text("UPDATE reading_progress SET session_id = :sid WHERE book_id = :bid"), + {"sid": session_id, "bid": book_id}, + ) + + +def downgrade() -> None: + op.drop_constraint('fk_reading_progress_session_id', 'reading_progress', type_='foreignkey') + op.drop_index(op.f('ix_reading_progress_session_id'), table_name='reading_progress') + op.drop_column('reading_progress', 'session_id') + op.drop_index('uq_reading_session_book_current', table_name='reading_session') + op.drop_index(op.f('ix_reading_session_is_current'), table_name='reading_session') + op.drop_index(op.f('ix_reading_session_date_finished'), table_name='reading_session') + op.drop_index(op.f('ix_reading_session_date_started'), table_name='reading_session') + op.drop_index(op.f('ix_reading_session_status'), table_name='reading_session') + op.drop_index(op.f('ix_reading_session_user_id'), table_name='reading_session') + op.drop_index(op.f('ix_reading_session_book_id'), table_name='reading_session') + op.drop_table('reading_session') diff --git a/backend/app/main.py b/backend/app/main.py index b619cc84..4529e3cc 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -13,7 +13,7 @@ from app._build_info import __git_sha__, __version__ from app.config import settings from app.logging_config import configure_logging -from app.routers import admin, auth, books, config, cover_candidates, covers, data, docs, embed, health, hygiene, import_, oidc, profile, progress, public_profile, share_links, statistics, users +from app.routers import admin, auth, books, config, cover_candidates, covers, data, docs, embed, health, hygiene, import_, oidc, profile, progress, public_profile, sessions, share_links, statistics, users from app.services.cover_storage import cleanup_orphan_covers from app.services.data_import import cleanup_temp_files from app.services.telemetry import send_telemetry_once @@ -230,6 +230,7 @@ async def proxy_headers_middleware(request: Request, call_next) -> Response: app.include_router(public_profile.router) app.include_router(oidc.router) app.include_router(progress.router) +app.include_router(sessions.router) app.include_router(docs.router) app.include_router(health.router) app.include_router(hygiene.router) diff --git a/backend/app/models.py b/backend/app/models.py index 88e5db2d..a323ad09 100644 --- a/backend/app/models.py +++ b/backend/app/models.py @@ -251,6 +251,51 @@ class ApiKey(SQLModel, table=True): ) +class ReadingSession(SQLModel, table=True): + """A single reading period (first read, re-read, etc.) of a book. + + ``Book.date_started`` / ``Book.date_finished`` remain a denormalized mirror + of the *current* session (``is_current=True``), or of the latest session when + none is current, so every existing consumer keeps working unchanged. A book + with a single session therefore behaves identically to the pre-session model. + + At most one current session exists per book (enforced by a partial unique + index on ``is_current`` and by the service layer). + """ + + __tablename__: str = "reading_session" + __table_args__ = ( + sa.Index( + "uq_reading_session_book_current", + "book_id", + unique=True, + sqlite_where=sa.text("is_current = 1"), + ), + ) + + id: Optional[int] = Field(default=None, primary_key=True) + book_id: int = Field(foreign_key="book.id", index=True) + user_id: int = Field(foreign_key="user.id", index=True) + status: ReadingStatus = Field(index=True) + date_started: Optional[datetime] = Field( + default=None, + sa_column=Column(UtcDateTime, index=True) + ) + date_finished: Optional[datetime] = Field( + default=None, + sa_column=Column(UtcDateTime, index=True) + ) + is_current: bool = Field(default=False, index=True) + created_at: datetime = Field( + default_factory=utcnow, + sa_column=Column(UtcDateTime, default=utcnow) + ) + updated_at: datetime = Field( + default_factory=utcnow, + sa_column=Column(UtcDateTime, default=utcnow) + ) + + class ReadingProgress(SQLModel, table=True): """A page-number reading progress entry for a book.""" @@ -259,6 +304,7 @@ class ReadingProgress(SQLModel, table=True): id: Optional[int] = Field(default=None, primary_key=True) book_id: int = Field(foreign_key="book.id", index=True) user_id: int = Field(foreign_key="user.id", index=True) + session_id: Optional[int] = Field(default=None, foreign_key="reading_session.id", index=True) page: int = Field(ge=0) created_at: datetime = Field( default_factory=utcnow, diff --git a/backend/app/routers/books.py b/backend/app/routers/books.py index f311813c..a92a03b1 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, Author, Book, BookAuthor, BookTag, Medium, ReadingProgress, ReadingStatus, Tag, User, UserSettings +from app.models import AcquisitionStatus, Author, Book, BookAuthor, BookTag, Medium, ReadingProgress, ReadingSession, ReadingStatus, Tag, User, UserSettings from app.schemas import ( BookCreate, BookListResponse, @@ -39,6 +39,7 @@ ) from app.services.cover_import import import_cover_from_url, is_external_cover_url from app.services.quote_cache import get_or_fetch_dashboard_quote +from app.services.reading_sessions import create_initial_session, reconcile_book_sessions, session_counts from app.services.search import _escape_like, apply_search_filter from app.services.tags import build_book_read, cleanup_orphan_tags, load_tags_batch, sync_book_tags from app.time_utils import utcnow @@ -260,12 +261,17 @@ def list_books( 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 {} + session_counts_map = session_counts(session, book_ids) if book_ids else {} book_reads: list[BookRead] = [] for book in books: assert book.id is not None - book_reads.append( - _build_book_read_with_tags(book, book_tags_map.get(book.id), book_authors_map.get(book.id)) + counts = session_counts_map.get(book.id, {}) + book_read = _build_book_read_with_tags( + book, book_tags_map.get(book.id), book_authors_map.get(book.id) ) + book_read.session_count = counts.get("session_count", 0) + book_read.read_count = counts.get("read_count", 0) + book_reads.append(book_read) return BookListResponse( books=book_reads, total=total, @@ -488,6 +494,8 @@ async def create_book( session.rollback() _raise_integrity_conflict(exc) session.refresh(book) + create_initial_session(session, book) + session.commit() logger.info("Created book: %r (id=%s)", book.title, book.id) return build_book_read(session, book) @@ -586,6 +594,7 @@ async def update_book( except IntegrityError as exc: session.rollback() _raise_integrity_conflict(exc) + reconcile_book_sessions(session, book) if tags_provided: assert book.id is not None sync_book_tags(session, current_user.id, book.id, tags_raw) @@ -738,6 +747,8 @@ def transition_status( ) book.sqlmodel_update(update_data) session.add(book) + session.flush() + reconcile_book_sessions(session, book) session.commit() session.refresh(book) return StatusTransitionResponse(book=build_book_read(session, book), date_conflict=None) @@ -773,6 +784,10 @@ def delete_book( select(ReadingProgress).where(ReadingProgress.book_id == book.id) ).all(): session.delete(entry) + for reading_session in session.exec( + select(ReadingSession).where(ReadingSession.book_id == book.id) + ).all(): + session.delete(reading_session) session.delete(book) cleanup_orphan_tags(session, current_user.id) cleanup_orphan_authors(session, current_user.id) diff --git a/backend/app/routers/progress.py b/backend/app/routers/progress.py index f0839574..459d0483 100644 --- a/backend/app/routers/progress.py +++ b/backend/app/routers/progress.py @@ -1,16 +1,16 @@ """Reading progress CRUD endpoints and batch latest-progress query.""" import logging -from typing import List +from typing import List, Optional -from fastapi import APIRouter, Depends, HTTPException -from sqlalchemy import func +from fastapi import APIRouter, Depends, HTTPException, Query from sqlmodel import Session, col, select from app.auth import require_user from app.database import get_session -from app.models import Book, ReadingProgress, User +from app.models import Book, ReadingProgress, ReadingSession, User from app.schemas import ReadingProgressCreate, ReadingProgressLatest, ReadingProgressRead, ReadingProgressUpdate +from app.services.reading_sessions import active_session, sessions_for_book logger = logging.getLogger(__name__) @@ -39,20 +39,32 @@ def create_progress_entry( detail=f"Page cannot exceed book page count ({book.page_count})", ) + session_id = data.session_id + if session_id is None: + current = active_session(session, book_id) + if current is not None: + session_id = current.id + else: + sessions = sessions_for_book(session, book_id) + if sessions: + session_id = sessions[0].id + entry = ReadingProgress( book_id=book_id, user_id=current_user.id, page=data.page, + session_id=session_id, ) session.add(entry) session.commit() session.refresh(entry) assert entry.id is not None - logger.debug("Created progress entry: book_id=%s page=%s", book_id, data.page) + logger.debug("Created progress entry: book_id=%s page=%s session_id=%s", book_id, data.page, session_id) return ReadingProgressRead( id=entry.id, book_id=entry.book_id, page=entry.page, + session_id=entry.session_id, created_at=entry.created_at, updated_at=entry.updated_at, ) @@ -61,27 +73,34 @@ def create_progress_entry( @router.get("/{book_id}/progress", response_model=List[ReadingProgressRead]) def list_progress_entries( book_id: int, + session_id: Optional[int] = Query(default=None), current_user: User = Depends(require_user), session: Session = Depends(get_session), ) -> List[ReadingProgressRead]: - """List all progress entries for a book, newest first.""" + """List progress entries for a book, newest first. + + When ``session_id`` is omitted, all entries are returned (matching the + pre-session behaviour). Pass ``session_id`` to filter to a single session. + """ book = session.get(Book, book_id) if not book or book.user_id != current_user.id: raise HTTPException(status_code=404, detail="Book not found") + statement = select(ReadingProgress).where( + ReadingProgress.book_id == book_id, + ReadingProgress.user_id == current_user.id, + ) + if session_id is not None: + statement = statement.where(ReadingProgress.session_id == session_id) rows = session.exec( - select(ReadingProgress) - .where( - ReadingProgress.book_id == book_id, - ReadingProgress.user_id == current_user.id, - ) - .order_by(col(ReadingProgress.created_at).desc()) + statement.order_by(col(ReadingProgress.created_at).desc()) ).all() return [ ReadingProgressRead( id=r.id, # ty: ignore[invalid-argument-type] book_id=r.book_id, page=r.page, + session_id=r.session_id, created_at=r.created_at, updated_at=r.updated_at, ) @@ -111,6 +130,7 @@ def update_progress_entry( id=entry.id, # ty: ignore[invalid-argument-type] book_id=entry.book_id, page=entry.page, + session_id=entry.session_id, created_at=entry.created_at, updated_at=entry.updated_at, ) @@ -139,26 +159,53 @@ def get_latest_progress_batch( current_user: User = Depends(require_user), session: Session = Depends(get_session), ) -> List[ReadingProgressLatest]: - """Return the latest progress page for each book in a comma-separated list of IDs.""" + """Return the latest progress page for each book in a comma-separated list of IDs. + + Session-aware: for a book with a current (open) session, the latest page of + *that* session is returned, so a re-read correctly shows 0 % while the + previous session's entries stay separate. Books without a current session + fall back to the latest page across all entries (legacy behaviour). + """ ids = [int(i.strip()) for i in book_ids.split(",") if i.strip()] if not ids: return [] - subq = ( - select( - ReadingProgress.book_id, - ReadingProgress.page, - func.row_number() - .over(partition_by=col(ReadingProgress.book_id), order_by=col(ReadingProgress.created_at).desc()) - .label("rn"), - ) - .where( - col(ReadingProgress.book_id).in_(ids), - ReadingProgress.user_id == current_user.id, - ) - .subquery() + current_sessions = { + s.book_id: s.id + for s in session.exec( + select(ReadingSession).where( + col(ReadingSession.book_id).in_(ids), + ReadingSession.is_current.is_(True), + ) + ).all() + } + + entries = list( + session.exec( + select(ReadingProgress) + .where( + col(ReadingProgress.book_id).in_(ids), + ReadingProgress.user_id == current_user.id, + ) + .order_by(col(ReadingProgress.created_at)) + ).all() ) - rows = session.exec( - select(subq.c.book_id, subq.c.page).where(subq.c.rn == 1) - ).all() - return [ReadingProgressLatest(book_id=book_id, current_page=page) for book_id, page in rows] + + latest_by_book_session: dict[tuple[int, int | None], ReadingProgress] = {} + for entry in entries: + key = (entry.book_id, entry.session_id) + prev = latest_by_book_session.get(key) + if prev is None or entry.created_at >= prev.created_at: + latest_by_book_session[key] = entry + + results: list[ReadingProgressLatest] = [] + for book_id in ids: + current_sid = current_sessions.get(book_id) + if current_sid is not None: + latest = latest_by_book_session.get((book_id, current_sid)) + else: + candidates = [e for e in entries if e.book_id == book_id] + latest = max(candidates, key=lambda e: e.created_at) if candidates else None + if latest is not None: + results.append(ReadingProgressLatest(book_id=book_id, current_page=latest.page)) + return results diff --git a/backend/app/routers/sessions.py b/backend/app/routers/sessions.py new file mode 100644 index 00000000..d608e5ba --- /dev/null +++ b/backend/app/routers/sessions.py @@ -0,0 +1,304 @@ +"""Reading session CRUD, re-read, and timeline endpoints.""" + +import logging +from datetime import datetime +from typing import List, Optional + +from fastapi import APIRouter, Depends, HTTPException +from sqlmodel import Session, col, select + +from app.auth import require_user +from app.database import get_session +from app.models import Book, ReadingProgress, ReadingSession, ReadingStatus, User +from app.routers.books import _validate_dates +from app.schemas import ( + BookRead, + ReadAgainRequest, + ReadingSessionCreate, + ReadingSessionRead, + ReadingSessionUpdate, + SessionMutationResponse, + TimelineSessionEntry, +) +from app.services.authors import join_authors, load_authors_batch +from app.services.reading_sessions import ( + assign_read_numbers, + build_session_read, + progress_aggregates, + recompute_book_mirror, + sessions_for_book, + start_read_again, +) +from app.services.tags import build_book_read +from app.time_utils import utcnow + +logger = logging.getLogger(__name__) + +router = APIRouter(prefix="/api/books", tags=["sessions"]) + + +def _get_owned_book(session: Session, book_id: int, user_id: int) -> Book: + book = session.get(Book, book_id) + if not book or book.user_id != user_id: + raise HTTPException(status_code=404, detail="Book not found") + return book + + +def _get_owned_session( + session: Session, book_id: int, session_id: int, user_id: int +) -> ReadingSession: + reading_session = session.get(ReadingSession, session_id) + if ( + not reading_session + or reading_session.book_id != book_id + or reading_session.user_id != user_id + ): + raise HTTPException(status_code=404, detail="Reading session not found") + return reading_session + + +def _to_session_reads( + session: Session, sessions: list[ReadingSession] +) -> list[ReadingSessionRead]: + if not sessions: + return [] + read_numbers = assign_read_numbers(sessions) + aggs = progress_aggregates(session, sessions[0].book_id) + return [ + build_session_read(session, s, read_numbers.get(s.id), aggs.get(s.id or 0)) + for s in sessions + ] + + +@router.get("", response_model=List[TimelineSessionEntry]) +def list_completed_sessions( + current_user: User = Depends(require_user), + session: Session = Depends(get_session), +) -> List[TimelineSessionEntry]: + """Return every completed read (session) across the user's books, newest first.""" + rows = session.exec( + select(ReadingSession, Book) + .join(Book, col(Book.id) == col(ReadingSession.book_id)) + .where( + ReadingSession.user_id == current_user.id, + ReadingSession.status == ReadingStatus.read, + col(ReadingSession.date_finished).is_not(None), + ) + .order_by(col(ReadingSession.date_finished).desc()) + ).all() + + per_book: dict[int, list[ReadingSession]] = {} + for rs, _book in rows: + per_book.setdefault(rs.book_id, []).append(rs) + authors_map = load_authors_batch(session, list(per_book)) + + result: list[TimelineSessionEntry] = [] + for rs, book in rows: + read_number = assign_read_numbers(per_book[rs.book_id]).get(rs.id) + authors = authors_map.get(book.id, []) + result.append( + TimelineSessionEntry( + session_id=rs.id or 0, + book_id=book.id or 0, + title=book.title, + author=join_authors(authors), + authors=authors, + cover_url=book.cover_url, + rating=book.rating, + page_count=book.page_count, + date_finished=rs.date_finished, + read_number=read_number, + is_reread=read_number is not None and read_number >= 2, + ) + ) + return result + + +@router.get("/{book_id}/sessions", response_model=List[ReadingSessionRead]) +def list_sessions( + book_id: int, + current_user: User = Depends(require_user), + session: Session = Depends(get_session), +) -> List[ReadingSessionRead]: + """List a book's reading sessions, newest first.""" + _get_owned_book(session, book_id, current_user.id) + return _to_session_reads(session, sessions_for_book(session, book_id)) + + +@router.post("/{book_id}/sessions", response_model=SessionMutationResponse, status_code=201) +def create_session( + book_id: int, + data: ReadingSessionCreate, + current_user: User = Depends(require_user), + session: Session = Depends(get_session), +) -> SessionMutationResponse: + """Add a historical (past) reading session. + + Past reads are pure history: they never become current and never change the + book's status or dates. + """ + book = _get_owned_book(session, book_id, current_user.id) + if data.status == ReadingStatus.want_to_read: + raise HTTPException(status_code=422, detail="A session cannot be 'want_to_read'.") + if data.status == ReadingStatus.currently_reading: + raise HTTPException( + status_code=422, + detail="Use the 'Read again' action to start a current session.", + ) + if data.status == ReadingStatus.read and data.date_finished is None: + raise HTTPException( + status_code=422, detail="A completed session must have a finish date." + ) + _validate_dates({"date_started": data.date_started, "date_finished": data.date_finished}) + + reading_session = ReadingSession( + book_id=book.id, + user_id=current_user.id, + status=data.status, + date_started=data.date_started, + date_finished=data.date_finished, + is_current=False, + ) + session.add(reading_session) + session.commit() + session.refresh(reading_session) + + sessions = sessions_for_book(session, book_id) + return SessionMutationResponse( + session=build_session_read( + session, + reading_session, + assign_read_numbers(sessions).get(reading_session.id), + ), + book=build_book_read(session, book), + ) + + +@router.patch("/{book_id}/sessions/{session_id}", response_model=SessionMutationResponse) +def update_session( + book_id: int, + session_id: int, + data: ReadingSessionUpdate, + current_user: User = Depends(require_user), + session: Session = Depends(get_session), +) -> SessionMutationResponse: + """Edit a session's dates or status; re-mirrors the book if it is the current one.""" + book = _get_owned_book(session, book_id, current_user.id) + reading_session = _get_owned_session(session, book_id, session_id, current_user.id) + + update = data.model_dump(exclude_unset=True) + if "status" in update and update["status"] == ReadingStatus.want_to_read: + raise HTTPException(status_code=422, detail="A session cannot be 'want_to_read'.") + _validate_dates( + { + "date_started": update.get("date_started", reading_session.date_started), + "date_finished": update.get("date_finished", reading_session.date_finished), + } + ) + + for field, value in update.items(): + setattr(reading_session, field, value) + + if reading_session.status == ReadingStatus.read and reading_session.date_finished is None: + raise HTTPException( + status_code=422, detail="A completed session must have a finish date." + ) + + was_current = reading_session.is_current + if was_current and reading_session.status != ReadingStatus.currently_reading: + reading_session.is_current = False + if reading_session.date_finished is None: + reading_session.date_finished = utcnow() + + session.add(reading_session) + session.commit() + session.refresh(reading_session) + + if was_current: + book.reading_status = reading_session.status + recompute_book_mirror(book, sessions_for_book(session, book_id)) + session.add(book) + session.commit() + session.refresh(book) + + sessions = sessions_for_book(session, book_id) + return SessionMutationResponse( + session=build_session_read( + session, + reading_session, + assign_read_numbers(sessions).get(session_id), + ), + book=build_book_read(session, book), + ) + + +@router.delete("/{book_id}/sessions/{session_id}", response_model=BookRead) +def delete_session( + book_id: int, + session_id: int, + current_user: User = Depends(require_user), + session: Session = Depends(get_session), +) -> BookRead: + """Delete a session and its progress entries, then re-mirror the book.""" + book = _get_owned_book(session, book_id, current_user.id) + reading_session = _get_owned_session(session, book_id, session_id, current_user.id) + was_current = reading_session.is_current + + for entry in session.exec( + select(ReadingProgress).where(ReadingProgress.session_id == session_id) + ).all(): + session.delete(entry) + + session.delete(reading_session) + session.commit() + + remaining = sessions_for_book(session, book_id) + if not remaining: + book.reading_status = ReadingStatus.want_to_read + book.date_started = None + book.date_finished = None + session.add(book) + session.commit() + elif was_current: + promoted = remaining[0] + promoted.is_current = True + session.add(promoted) + book.reading_status = promoted.status + recompute_book_mirror(book, remaining) + session.add(book) + session.commit() + + return build_book_read(session, book) + + +@router.post("/{book_id}/read-again", response_model=SessionMutationResponse) +def read_again( + book_id: int, + data: ReadAgainRequest, + current_user: User = Depends(require_user), + session: Session = Depends(get_session), +) -> SessionMutationResponse: + """Start a re-read: keep earlier sessions and open a new current session.""" + book = _get_owned_book(session, book_id, current_user.id) + if book.reading_status not in (ReadingStatus.read, ReadingStatus.did_not_finish): + raise HTTPException( + status_code=422, detail="Only finished books can be read again." + ) + + if data.date_started is not None: + _validate_dates({"date_started": data.date_started, "date_finished": None}) + + new_session = start_read_again(session, book, data.date_started) + session.commit() + session.refresh(book) + session.refresh(new_session) + + sessions = sessions_for_book(session, book_id) + return SessionMutationResponse( + session=build_session_read( + session, + new_session, + assign_read_numbers(sessions).get(new_session.id), + ), + book=build_book_read(session, book), + ) diff --git a/backend/app/routers/statistics.py b/backend/app/routers/statistics.py index 6d011cb0..dd978682 100644 --- a/backend/app/routers/statistics.py +++ b/backend/app/routers/statistics.py @@ -13,7 +13,7 @@ from app.auth import require_user from app.database import get_session -from app.models import Book, ReadingProgress, ReadingStatus, User, UserSettings +from app.models import Book, ReadingProgress, ReadingSession, ReadingStatus, User, UserSettings from app.schemas import ( DailyPages, DailyPagesResponse, @@ -80,7 +80,13 @@ def get_gamification( .order_by(col(Book.id)) ).all() ) + sessions = list( + session.exec( + select(ReadingSession).where(ReadingSession.user_id == current_user.id) + ).all() + ) book_ids_with_progress = {entry.book_id for entry in entries} + book_by_id = {book.id: book for book in books if book.id is not None} active_dates = {_day_key(entry.created_at, tz) for entry in entries} for book in books: @@ -95,7 +101,7 @@ def get_gamification( longest, longest_start, longest_end = longest_streak(active_dates) goals = _compute_goal_progress( - tz, settings, today, entries, books, book_ids_with_progress + tz, settings, today, entries, books, book_ids_with_progress, sessions, book_by_id ) return GamificationResponse( @@ -165,37 +171,86 @@ def get_pages_per_day( books = list( session.exec(select(Book).where(Book.user_id == current_user.id)).all() ) + book_by_id = {book.id: book for book in books if book.id is not None} + sessions = list( + session.exec( + select(ReadingSession).where(ReadingSession.user_id == current_user.id) + ).all() + ) + books_with_sessions = {s.book_id for s in sessions} - # Rebuild virtual entries with a simple namespace replacement. + # Rebuild virtual entries with a simple namespace replacement, per session. from types import SimpleNamespace - virtual_entries = [ - SimpleNamespace( - book_id=book.id, - page=0, - created_at=book.date_started, + virtual_entries = [] + for reading_session in sessions: + if reading_session.date_started is None: + continue + if reading_session.status == ReadingStatus.read and reading_session.date_finished is None: + continue + virtual_entries.append( + SimpleNamespace( + book_id=reading_session.book_id, + session_id=reading_session.id, + page=0, + created_at=reading_session.date_started, + ) + ) + for book in books: + if book.id in books_with_sessions: + continue + if book.id not in all_book_ids_with_progress or not book.date_started: + continue + if book.reading_status == ReadingStatus.read and not book.date_finished: + continue + virtual_entries.append( + SimpleNamespace(book_id=book.id, session_id=None, page=0, created_at=book.date_started) ) - for book in books - if book.id in all_book_ids_with_progress - and book.date_started - and not (book.reading_status == ReadingStatus.read and not book.date_finished) - ] all_progress_entries = list(progress_entries) + virtual_entries progress_daily = _extract_progress_daily_pages(all_progress_entries, tz, start_date_utc, end_date_utc) - fallback_books = [ - b - for b in books - if b.id not in all_book_ids_with_progress - and b.reading_status == ReadingStatus.read - and b.date_started - and b.date_finished - and b.page_count - # Only include books whose reading period could overlap the window. - and _naive_utc(b.date_finished) >= start_date_utc - ] - fallback_daily = _extract_book_level_daily_pages(fallback_books, tz, start_date_utc, end_date_utc) + progress_session_ids = {entry.session_id for entry in progress_entries if entry.session_id is not None} + fallback_intervals = [] + for reading_session in sessions: + if reading_session.status != ReadingStatus.read: + continue + if not (reading_session.date_started and reading_session.date_finished): + continue + if reading_session.id in progress_session_ids: + continue + book = book_by_id.get(reading_session.book_id) + if book is None or not book.page_count: + continue + fallback_intervals.append( + SimpleNamespace( + book_id=reading_session.book_id, + session_id=reading_session.id, + date_started=reading_session.date_started, + date_finished=reading_session.date_finished, + page_count=book.page_count, + ) + ) + for book in books: + if book.id in books_with_sessions or book.id in all_book_ids_with_progress: + continue + if ( + book.reading_status == ReadingStatus.read + and book.date_started + and book.date_finished + and book.page_count + and _naive_utc(book.date_finished) >= start_date_utc + ): + fallback_intervals.append( + SimpleNamespace( + book_id=book.id, + session_id=None, + date_started=book.date_started, + date_finished=book.date_finished, + page_count=book.page_count, + ) + ) + fallback_daily = _extract_book_level_daily_pages(fallback_intervals, tz, start_date_utc, end_date_utc) combined: dict[str, float] = {} for k, v in progress_daily.items(): diff --git a/backend/app/schemas.py b/backend/app/schemas.py index 1f4a114b..2f75a45e 100644 --- a/backend/app/schemas.py +++ b/backend/app/schemas.py @@ -16,6 +16,7 @@ class ReadingProgressCreate(SQLModel): """Request body to create a reading progress entry.""" page: int = Field(ge=0) + session_id: Optional[int] = None class ReadingProgressRead(SQLModel): @@ -23,10 +24,47 @@ class ReadingProgressRead(SQLModel): id: int book_id: int page: int + session_id: Optional[int] = None created_at: datetime updated_at: datetime +class ReadingSessionCreate(SQLModel): + """Request body to create a reading session (manual past read or re-read).""" + status: ReadingStatus + date_started: Optional[datetime] = None + date_finished: Optional[datetime] = None + + +class ReadingSessionUpdate(SQLModel): + """Request body to update a reading session's dates or status.""" + status: Optional[ReadingStatus] = None + date_started: Optional[datetime] = None + date_finished: Optional[datetime] = None + + +class ReadingSessionRead(SQLModel): + """Response schema for a reading session, including the chronological ordinal.""" + id: int + book_id: int + status: ReadingStatus + date_started: Optional[datetime] = None + date_finished: Optional[datetime] = None + is_current: bool = False + read_number: Optional[int] = None + is_reread: bool = False + progress_count: int = 0 + last_page: Optional[int] = None + max_page: Optional[int] = None + created_at: datetime + updated_at: datetime + + +class ReadAgainRequest(SQLModel): + """Request body to start a re-read of a book.""" + date_started: Optional[datetime] = None + + class ReadingProgressUpdate(SQLModel): """Request body to update a reading progress entry's date.""" created_at: datetime @@ -172,6 +210,29 @@ class BookRead(SQLModel): date_added: datetime date_started: Optional[datetime] date_finished: Optional[datetime] + read_count: int = 0 + session_count: int = 0 + + +class SessionMutationResponse(SQLModel): + """Response after a session mutation, returning the session and its book.""" + session: ReadingSessionRead + book: BookRead + + +class TimelineSessionEntry(SQLModel): + """A single completed reading session enriched with book metadata for the timeline.""" + session_id: int + book_id: int + title: str + author: Optional[str] = None + authors: list[str] = [] + cover_url: Optional[str] = None + rating: Optional[int] = None + page_count: Optional[int] = None + date_finished: Optional[datetime] = None + read_number: Optional[int] = None + is_reread: bool = False class BookListResponse(BaseModel): @@ -318,6 +379,8 @@ class StatisticsResponse(SQLModel): average_rating: Optional[float] top_rated_books: list[TopRatedBook] worst_rated_books: list[TopRatedBook] + total_reads: int = 0 + reread_books: int = 0 class GoalType(str, Enum): @@ -494,6 +557,7 @@ class DataResetDeleted(SQLModel): tags: int authors: int progress_entries: int + sessions: int = 0 class DataResetResponse(SQLModel): @@ -530,6 +594,7 @@ class StatusTransitionRequest(SQLModel): skip_auto_date_started: bool = False clear_date_started: bool = False clear_date_finished: bool = False + start_new_session: bool = False class DateConflict(SQLModel): @@ -640,7 +705,7 @@ class DailyPagesResponse(SQLModel): class DataExportRequest(SQLModel): """Data export request body.""" - datasets: list[Literal["books", "progress", "tags", "covers"]] + datasets: list[Literal["books", "progress", "tags", "covers", "sessions"]] format: Literal["csv", "json"] @@ -815,6 +880,7 @@ class PublicProfileStatisticsKey(str, Enum): average_rating = "average_rating" top_rated_books = "top_rated_books" worst_rated_books = "worst_rated_books" + total_reads = "total_reads" class PublicProfileVisibilityConfig(SQLModel): diff --git a/backend/app/services/data_export.py b/backend/app/services/data_export.py index f105b517..2ca3d8fd 100644 --- a/backend/app/services/data_export.py +++ b/backend/app/services/data_export.py @@ -11,7 +11,7 @@ from sqlmodel import Session, col, select from app._build_info import __git_sha__, __version__ -from app.models import Book, BookTag, ReadingProgress, Tag, User +from app.models import Book, BookTag, ReadingProgress, ReadingSession, 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 @@ -40,7 +40,8 @@ "cover_url", ] -PROGRESS_CSV_FIELDS: list[str] = ["book_id", "book_title", "page", "created_at", "updated_at"] +PROGRESS_CSV_FIELDS: list[str] = ["book_id", "book_title", "page", "created_at", "updated_at", "session_id"] +SESSION_CSV_FIELDS: list[str] = ["session_id", "book_id", "book_title", "status", "date_started", "date_finished", "is_current", "created_at"] TAG_CSV_FIELDS: list[str] = ["id", "name", "book_count", "created_at"] @@ -97,6 +98,21 @@ def _progress_to_dict(entry: ReadingProgress, book_titles: dict[int, str]) -> di "page": entry.page, "created_at": _serialize_datetime(entry.created_at), "updated_at": _serialize_datetime(entry.updated_at), + "session_id": entry.session_id, + } + + +def _session_to_dict(entry: ReadingSession, book_titles: dict[int, str]) -> dict: + """Convert a ReadingSession to a flat export dict.""" + return { + "session_id": entry.id, + "book_id": entry.book_id, + "book_title": book_titles.get(entry.book_id), + "status": entry.status.value, + "date_started": _serialize_datetime(entry.date_started), + "date_finished": _serialize_datetime(entry.date_finished), + "is_current": entry.is_current, + "created_at": _serialize_datetime(entry.created_at), } @@ -152,6 +168,13 @@ def build_export_zip( ).all() ) tags = list(session.exec(select(Tag).where(Tag.user_id == user.id)).all()) + sessions = list( + session.exec( + select(ReadingSession) + .join(Book, col(Book.id) == col(ReadingSession.book_id)) + .where(ReadingSession.user_id == user.id, Book.user_id == user.id) + ).all() + ) tag_counts_rows = list( session.exec( select(BookTag.tag_id, BookTag.book_id) @@ -169,12 +192,14 @@ def build_export_zip( 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] + session_rows = [_session_to_dict(entry, book_titles) for entry in sessions] tag_rows = [_tag_to_dict(tag, tag_counts) for tag in tags] counts = { "books": len(books_rows), "progress_entries": len(progress_rows), "tags": len(tag_rows), + "sessions": len(session_rows), "covers": 0, } @@ -198,6 +223,12 @@ def build_export_zip( else: zip_file.writestr("tags.csv", _dump_csv(tag_rows, TAG_CSV_FIELDS)) + if "sessions" in datasets: + if export_format == "json": + zip_file.writestr("sessions.json", json.dumps(session_rows, indent=2)) + else: + zip_file.writestr("sessions.csv", _dump_csv(session_rows, SESSION_CSV_FIELDS)) + if "covers" in datasets: covers_written = 0 for book in books: diff --git a/backend/app/services/data_import.py b/backend/app/services/data_import.py index 8770b96c..45524d45 100644 --- a/backend/app/services/data_import.py +++ b/backend/app/services/data_import.py @@ -21,8 +21,9 @@ from sqlmodel import Session, col, select from app.config import settings -from app.models import AcquisitionStatus, Book, Medium, ReadingProgress, ReadingStatus, User, normalize_medium_key +from app.models import AcquisitionStatus, Book, Medium, ReadingProgress, ReadingSession, ReadingStatus, User, normalize_medium_key from app.schemas import ImportFieldConfig +from app.services.reading_sessions import create_initial_session logger = logging.getLogger(__name__) from app.time_utils import utcnow @@ -999,6 +1000,8 @@ async def execute_import( session.flush() assert book.id is not None + initial_session = create_initial_session(session, book) + sync_book_authors( session, user.id, @@ -1015,6 +1018,7 @@ async def execute_import( user_id=user.id, page=page_count, created_at=log_date, + session_id=initial_session.id if initial_session else None, ) session.add(progress_entry) diff --git a/backend/app/services/reading_sessions.py b/backend/app/services/reading_sessions.py new file mode 100644 index 00000000..ab014921 --- /dev/null +++ b/backend/app/services/reading_sessions.py @@ -0,0 +1,289 @@ +"""Reading session lifecycle, mirroring, and query helpers. + +This module is the single place that owns the invariants between ``Book`` and its +``ReadingSession`` rows: + +- A book has at most one *current* session (``is_current=True``), which is the + session being read right now. +- ``Book.date_started`` / ``Book.date_finished`` mirror the current session, or + the latest session when none is current (``NULL`` when no session exists). +- Historical sessions are never modified implicitly by status transitions, only + by explicit session edits. + +Keeping the mirror here (and calling it after every write that touches reading +dates) guarantees that a book with a single session behaves identically to the +pre-session model. +""" + +from __future__ import annotations + +from datetime import datetime +from typing import Optional + +from sqlmodel import Session, col, select + +from app.models import Book, ReadingProgress, ReadingSession, ReadingStatus +from app.schemas import ReadingSessionRead +from app.time_utils import utcnow + + +def session_counts(session: Session, book_ids: list[int]) -> dict[int, dict[str, int]]: + """Return ``{book_id: {"session_count": n, "read_count": n}}`` for the ids.""" + if not book_ids: + return {} + counts: dict[int, dict[str, int]] = { + bid: {"session_count": 0, "read_count": 0} for bid in book_ids + } + rows = session.exec( + select(ReadingSession.book_id, ReadingSession.status).where( + col(ReadingSession.book_id).in_(book_ids) + ) + ).all() + for book_id, status in rows: + entry = counts.setdefault(book_id, {"session_count": 0, "read_count": 0}) + entry["session_count"] += 1 + if status == ReadingStatus.read: + entry["read_count"] += 1 + return counts + + +def sessions_for_book(session: Session, book_id: int) -> list[ReadingSession]: + """Return all sessions of a book, newest first (by created_at, then id).""" + return list( + session.exec( + select(ReadingSession) + .where(ReadingSession.book_id == book_id) + .order_by(col(ReadingSession.created_at).desc(), col(ReadingSession.id).desc()) + ).all() + ) + + +def active_session(session: Session, book_id: int) -> ReadingSession | None: + """Return the book's current (open) session, or ``None``.""" + return session.exec( + select(ReadingSession).where( + ReadingSession.book_id == book_id, + ReadingSession.is_current.is_(True), + ) + ).first() + + +def assign_read_numbers(sessions: list[ReadingSession]) -> dict[int, int]: + """Compute and return ``{session_id: read_number}`` for completed sessions. + + Completed (``read``) sessions are ordered chronologically by finish date + (tie-break: start date, then id) and numbered from 1. Non-completed sessions + get no read number. + """ + completed = [s for s in sessions if s.status == ReadingStatus.read] + completed.sort( + key=lambda s: ( + s.date_finished or s.date_started or utcnow(), + s.date_started or utcnow(), + s.id or 0, + ) + ) + return {s.id: idx + 1 for idx, s in enumerate(completed)} + + +def progress_aggregates( + session: Session, book_id: int +) -> dict[int, tuple[int, int | None, int | None]]: + """Return ``{session_id: (count, last_page, max_page)}`` for a book's sessions. + + Entries without a session are grouped under key ``0``. + """ + rows = session.exec( + select(ReadingProgress.session_id, ReadingProgress.page).where( + ReadingProgress.book_id == book_id + ) + ).all() + agg: dict[int, list[int]] = {} + for sid, page in rows: + agg.setdefault(sid or 0, []).append(page) + result: dict[int, tuple[int, int | None, int | None]] = {} + for sid, pages in agg.items(): + result[sid] = (len(pages), max(pages), max(pages)) + return result + + +def build_session_read( + session: Session, + reading_session: ReadingSession, + read_number: Optional[int], + agg: Optional[tuple[int, int | None, int | None]] = None, +) -> ReadingSessionRead: + """Build a :class:`ReadingSessionRead` for a session, with aggregates.""" + if agg is None: + aggs = progress_aggregates(session, reading_session.book_id) + agg = aggs.get(reading_session.id or 0, (0, None, None)) + count, last_page, max_page = agg + return ReadingSessionRead( + id=reading_session.id or 0, + book_id=reading_session.book_id, + status=reading_session.status, + date_started=reading_session.date_started, + date_finished=reading_session.date_finished, + is_current=reading_session.is_current, + read_number=read_number, + is_reread=read_number is not None and read_number >= 2, + progress_count=count, + last_page=last_page, + max_page=max_page, + created_at=reading_session.created_at, + updated_at=reading_session.updated_at, + ) + + +def recompute_book_mirror(book: Book, sessions: list[ReadingSession]) -> None: + """Re-derive ``Book`` dates from the current (else latest) session.""" + current = next((s for s in sessions if s.is_current), None) + if current is not None: + book.date_started = current.date_started + book.date_finished = None + if current.status != ReadingStatus.currently_reading: + book.date_finished = current.date_finished + return + latest = sessions[0] if sessions else None + if latest is None: + book.date_started = None + book.date_finished = None + return + book.date_started = latest.date_started + book.date_finished = latest.date_finished + + +def create_initial_session( + session: Session, book: Book +) -> ReadingSession | None: + """Create the initial session for a freshly created/imported book. + + Only books with a non-want-to-read status get a session; the session mirrors + the book's dates. Returns ``None`` when no session was created. + """ + if book.id is None: + return None + if book.reading_status == ReadingStatus.want_to_read: + return None + s = ReadingSession( + book_id=book.id, + user_id=book.user_id, + status=book.reading_status, + date_started=book.date_started, + date_finished=book.date_finished, + is_current=book.reading_status == ReadingStatus.currently_reading, + ) + session.add(s) + session.flush() + return s + + +def start_read_again( + session: Session, book: Book, date_started: Optional[datetime] +) -> ReadingSession: + """Start a re-read: demote any current session and open a new one.""" + now = utcnow() + for s in sessions_for_book(session, book.id or 0): + if s.is_current: + s.is_current = False + session.add(s) + new_session = ReadingSession( + book_id=book.id, + user_id=book.user_id, + status=ReadingStatus.currently_reading, + date_started=date_started or now, + date_finished=None, + is_current=True, + ) + session.add(new_session) + book.reading_status = ReadingStatus.currently_reading + book.date_started = new_session.date_started + book.date_finished = None + session.add(book) + session.flush() + return new_session + + +def reconcile_book_sessions(session: Session, book: Book) -> None: + """Reconcile a book's sessions with its current status after a book write. + + Ensures the invariant that a book's sessions reflect its status: + + - ``currently_reading``: exactly one current (open) session exists; it is + created on first read or re-read, or its start date is updated. + - ``read`` / ``did_not_finish``: any current session is closed and a + completed session exists (created or updated) for the finish. + - ``want_to_read``: sessions are left untouched (pure history). + + This never overwrites the book's own date fields; it only keeps the session + rows consistent so statistics and history stay correct. + """ + if book.id is None: + return + current = active_session(session, book.id) + + if book.reading_status == ReadingStatus.currently_reading: + if current is not None: + current.status = ReadingStatus.currently_reading + current.date_started = book.date_started + current.date_finished = None + current.is_current = True + session.add(current) + else: + session.add( + ReadingSession( + book_id=book.id, + user_id=book.user_id, + status=ReadingStatus.currently_reading, + date_started=book.date_started, + date_finished=None, + is_current=True, + ) + ) + elif book.reading_status in (ReadingStatus.read, ReadingStatus.did_not_finish): + if current is not None: + current.status = book.reading_status + current.date_finished = book.date_finished + current.is_current = False + session.add(current) + else: + sessions = sessions_for_book(session, book.id) + latest = sessions[0] if sessions else None + if latest is not None and latest.status == book.reading_status: + latest.date_started = book.date_started + latest.date_finished = book.date_finished + session.add(latest) + else: + session.add( + ReadingSession( + book_id=book.id, + user_id=book.user_id, + status=book.reading_status, + date_started=book.date_started, + date_finished=book.date_finished, + is_current=False, + ) + ) + + +def total_completed_and_reread( + session: Session, user_id: int +) -> tuple[int, int]: + """Return ``(total_reads, reread_books)`` for a user. + + ``total_reads`` counts completed sessions; ``reread_books`` counts books with + two or more completed sessions. + """ + rows = session.exec( + select(ReadingSession.book_id) + .where( + ReadingSession.user_id == user_id, + ReadingSession.status == ReadingStatus.read, + ) + ).all() + per_book: dict[int, int] = {} + for book_id in rows: + per_book[book_id] = per_book.get(book_id, 0) + 1 + total = sum(per_book.values()) + reread = sum(1 for count in per_book.values() if count >= 2) + return total, reread diff --git a/backend/app/services/statistics.py b/backend/app/services/statistics.py index da1a9c77..a6ddb911 100644 --- a/backend/app/services/statistics.py +++ b/backend/app/services/statistics.py @@ -17,7 +17,7 @@ from sqlalchemy import func from sqlmodel import Session, col, select -from app.models import AcquisitionStatus, Author, Book, BookAuthor, Medium, ReadingProgress, ReadingStatus, UserSettings +from app.models import AcquisitionStatus, Author, Book, BookAuthor, Medium, ReadingProgress, ReadingSession, ReadingStatus, UserSettings from app.schemas import ( AcquisitionStatusDistribution, GoalProgress, @@ -169,12 +169,12 @@ def _extract_progress_daily_pages( span so the values stay correct. """ daily: dict[str, float] = defaultdict(float) - grouped: dict[int, list] = {} + grouped: dict[tuple[int, object], list] = {} for entry in entries: - grouped.setdefault(entry.book_id, []).append(entry) + grouped.setdefault((entry.book_id, getattr(entry, "session_id", None)), []).append(entry) - for book_id in sorted(grouped): - book_entries = grouped[book_id] + for _key in sorted(grouped, key=lambda k: k[0]): + book_entries = grouped[_key] book_entries.sort(key=lambda e: (e.created_at, e.page)) for prev, curr in zip(book_entries, book_entries[1:]): delta = curr.page - prev.page @@ -255,11 +255,11 @@ def _compute_pages_per_month_from_progress( average is still computed from the full span so the values stay correct. """ monthly: dict[str, float] = defaultdict(float) - grouped: dict[int, list] = {} + grouped: dict[tuple[int, object], list] = {} for entry in entries: - grouped.setdefault(entry.book_id, []).append(entry) - for book_id in sorted(grouped): - book_entries = sorted(grouped[book_id], key=lambda e: (e.created_at, e.page)) + grouped.setdefault((entry.book_id, getattr(entry, "session_id", None)), []).append(entry) + for _key in sorted(grouped, key=lambda k: k[0]): + book_entries = sorted(grouped[_key], key=lambda e: (e.created_at, e.page)) for prev, curr in zip(book_entries, book_entries[1:]): delta = curr.page - prev.page if delta <= 0: @@ -365,9 +365,9 @@ def _pages_logged_on_day(entries: list, tz: ZoneInfo, day_key: str) -> int: A delta is the page gain between two consecutive progress entries of the same book, attributed to the calendar day (in *tz*) of the later entry. """ - grouped: dict[int, list] = {} + grouped: dict[tuple[int, object], list] = {} for entry in entries: - grouped.setdefault(entry.book_id, []).append(entry) + grouped.setdefault((entry.book_id, getattr(entry, "session_id", None)), []).append(entry) total = 0 for book_entries in grouped.values(): book_entries.sort(key=lambda e: (e.created_at, e.page)) @@ -378,6 +378,26 @@ def _pages_logged_on_day(entries: list, tz: ZoneInfo, day_key: str) -> int: return total +def _completed_read_dates( + books: list[Book], + sessions: list[ReadingSession], + books_with_sessions: set[int], +) -> list[datetime]: + """Return one ``date_finished`` per completed read (session or session-less book).""" + dates: list[datetime] = [] + for reading_session in sessions: + if reading_session.status == ReadingStatus.read and reading_session.date_finished is not None: + dates.append(reading_session.date_finished) + for book in books: + if ( + book.id not in books_with_sessions + and book.reading_status == ReadingStatus.read + and book.date_finished is not None + ): + dates.append(book.date_finished) + return dates + + def _compute_goal_progress( tz: ZoneInfo, settings: UserSettings, @@ -385,6 +405,8 @@ def _compute_goal_progress( entries: list, books: list, book_ids_with_progress: set[int], + sessions: list[ReadingSession], + book_by_id: dict[int, Book], ) -> list[GoalProgress]: """Compute current progress for every enabled reading goal. @@ -395,31 +417,72 @@ def _compute_goal_progress( current_month_key = today.strftime("%Y-%m") current_year = today.year - fallback_books = [ - b - for b in books - if b.id not in book_ids_with_progress - and b.reading_status == ReadingStatus.read - and b.date_started - and b.date_finished - and b.page_count - ] + books_with_sessions = {s.book_id for s in sessions} + completed_read_dates = _completed_read_dates(books, sessions, books_with_sessions) + progress_session_ids = {entry.session_id for entry in entries if entry.session_id is not None} - # Mirror compute_statistics: anchor every book with progress at page 0 on - # its start date so the first progress delta is attributed to the reading - # span, keeping the pages-per-month goal consistent with the statistics - # chart. - virtual_entries = [ - SimpleNamespace(book_id=b.id, page=0, created_at=b.date_started) - for b in books - if b.id in book_ids_with_progress - and b.date_started - and not (b.reading_status == ReadingStatus.read and not b.date_finished) - ] + fallback_intervals = [] + for reading_session in sessions: + if reading_session.status != ReadingStatus.read: + continue + if not (reading_session.date_started and reading_session.date_finished): + continue + if reading_session.id in progress_session_ids: + continue + book = book_by_id.get(reading_session.book_id) + if book is None or not book.page_count: + continue + fallback_intervals.append( + SimpleNamespace( + book_id=reading_session.book_id, + session_id=reading_session.id, + date_started=reading_session.date_started, + date_finished=reading_session.date_finished, + page_count=book.page_count, + ) + ) + for book in books: + if book.id in books_with_sessions or book.id in book_ids_with_progress: + continue + if book.reading_status == ReadingStatus.read and book.date_started and book.date_finished and book.page_count: + fallback_intervals.append( + SimpleNamespace( + book_id=book.id, + session_id=None, + date_started=book.date_started, + date_finished=book.date_finished, + page_count=book.page_count, + ) + ) + + virtual_entries = [] + for reading_session in sessions: + if reading_session.date_started is None: + continue + if reading_session.status == ReadingStatus.read and reading_session.date_finished is None: + continue + virtual_entries.append( + SimpleNamespace( + book_id=reading_session.book_id, + session_id=reading_session.id, + page=0, + created_at=reading_session.date_started, + ) + ) + for book in books: + if book.id in books_with_sessions: + continue + if book.id not in book_ids_with_progress or not book.date_started: + continue + if book.reading_status == ReadingStatus.read and not book.date_finished: + continue + virtual_entries.append( + SimpleNamespace(book_id=book.id, session_id=None, page=0, created_at=book.date_started) + ) return _goal_results( tz, today_key, current_month_key, current_year, - entries, books, fallback_books, virtual_entries, settings, + entries, fallback_intervals, virtual_entries, completed_read_dates, settings, ) @@ -429,9 +492,9 @@ def _goal_results( current_month_key: str, current_year: int, entries: list, - books: list, - fallback_books: list, + fallback_intervals: list, virtual_entries: list, + completed_read_dates: list, settings: UserSettings, ) -> list[GoalProgress]: """Assemble enabled goal progress entries.""" @@ -448,7 +511,7 @@ def _goal_results( continue current = _goal_current_value( goal_type, tz, today_key, current_month_key, current_year, - entries, books, fallback_books, virtual_entries, + entries, fallback_intervals, virtual_entries, completed_read_dates, ) results.append( GoalProgress(type=goal_type, target=target, current=current, reached=current >= target) @@ -463,40 +526,34 @@ def _goal_current_value( current_month_key: str, current_year: int, entries: list, - books: list, - fallback_books: list, + fallback_intervals: list, virtual_entries: list, + completed_read_dates: list, ) -> int: """Return the current value for a single reading goal.""" if goal_type == GoalType.pages_per_day: total = _pages_logged_on_day(entries, tz, today_key) - for b in fallback_books: - if _day_key(b.date_finished, tz) == today_key: - total += b.page_count + for interval in fallback_intervals: + if _day_key(interval.date_finished, tz) == today_key: + total += interval.page_count return total if goal_type == GoalType.pages_per_month: monthly = _compute_pages_per_month_from_progress(entries + virtual_entries, tz) - for k, v in _compute_pages_per_month_from_books(fallback_books, tz).items(): + for k, v in _compute_pages_per_month_from_books(fallback_intervals, tz).items(): monthly[k] += v return int(round(monthly.get(current_month_key, 0))) if goal_type == GoalType.books_per_month: return sum( - 1 - for b in books - if b.reading_status == ReadingStatus.read - and b.date_finished is not None - and _month_key(b.date_finished, tz) == current_month_key + 1 for date_finished in completed_read_dates + if _month_key(date_finished, tz) == current_month_key ) if goal_type == GoalType.books_per_year: return sum( - 1 - for b in books - if b.reading_status == ReadingStatus.read - and b.date_finished is not None - and b.date_finished.astimezone(tz).year == current_year + 1 for date_finished in completed_read_dates + if date_finished.astimezone(tz).year == current_year ) return 0 @@ -540,6 +597,17 @@ def compute_statistics( current_month_key = f"{now.year:04d}-{now.month:02d}" current_year = now.year books = list(session.exec(select(Book).where(Book.user_id == user_id)).all()) + book_by_id = {book.id: book for book in books if book.id is not None} + + sessions = list( + session.exec( + select(ReadingSession).where(ReadingSession.user_id == user_id) + ).all() + ) + sessions_by_book: dict[int, list[ReadingSession]] = {} + for reading_session in sessions: + sessions_by_book.setdefault(reading_session.book_id, []).append(reading_session) + books_with_sessions = set(sessions_by_book.keys()) total_authors = session.exec( select(func.count()).select_from(Author).where(Author.user_id == user_id) @@ -593,10 +661,33 @@ def compute_statistics( for book in books if book.reading_status == ReadingStatus.want_to_read and book.page_count is not None ) - pages_read = sum( - book.page_count or 0 - for book in books - if book.reading_status == ReadingStatus.read and book.page_count is not None + + # "Pages read" counts every completed reading (each completed session), so a + # re-read contributes its page count again. Books without sessions fall back + # to the book-level value, which keeps single-session users identical. + completed_reads: list[datetime] = [] + pages_read = 0 + for reading_session in sessions: + if reading_session.status == ReadingStatus.read and reading_session.date_finished is not None: + completed_reads.append(reading_session.date_finished) + book = book_by_id.get(reading_session.book_id) + if book is not None and book.page_count is not None: + pages_read += book.page_count + for book in books: + if ( + book.id not in books_with_sessions + and book.reading_status == ReadingStatus.read + and book.date_finished is not None + ): + completed_reads.append(book.date_finished) + if book.page_count is not None: + pages_read += book.page_count + + total_reads = len(completed_reads) + reread_books = sum( + 1 + for book_id, s_list in sessions_by_book.items() + if sum(1 for s in s_list if s.status == ReadingStatus.read) >= 2 ) dnf_book_ids = [book.id for book in books if book.reading_status == ReadingStatus.did_not_finish and book.id is not None] @@ -618,32 +709,22 @@ def compute_statistics( pages_wasted=pages_wasted, ) - all_finished_books = [ - book - for book in books - if book.reading_status == ReadingStatus.read and book.date_finished is not None - ] - finished_books_per_month_all_time: Counter[str] = Counter() - for book in all_finished_books: - assert book.date_finished is not None - finished_books_per_month_all_time[_month_key(book.date_finished, tz)] += 1 - - finished_books = all_finished_books + finished_books_per_month_all_time: Counter[str] = Counter( + _month_key(date_finished, tz) for date_finished in completed_reads + ) + finished_dates = completed_reads if window_start is not None and window_end is not None: - finished_books = [ - book - for book in finished_books - if book.date_finished is not None - and _naive_utc(book.date_finished) >= window_start - and _naive_utc(book.date_finished) <= window_end + finished_dates = [ + date_finished + for date_finished in completed_reads + if _naive_utc(date_finished) >= window_start + and _naive_utc(date_finished) <= window_end ] - finished_books_per_month: Counter[str] = Counter() - for book in finished_books: - assert book.date_finished is not None - month = _month_key(book.date_finished, tz) - finished_books_per_month[month] += 1 + finished_books_per_month: Counter[str] = Counter( + _month_key(date_finished, tz) for date_finished in finished_dates + ) # For bounded ranges the chart axis spans the whole selected window, so # months/years outside any real data still appear (with zero counts). @@ -696,8 +777,8 @@ def compute_statistics( ).all() ) - # All book_ids with *any* progress entry — used to exclude books from the - # fallback computation and to build virtual entries. + # All book_ids with *any* progress entry — used to exclude session-less books + # from the fallback computation and to build virtual entries. all_book_ids_with_progress = set( session.exec( select(ReadingProgress.book_id) @@ -705,37 +786,87 @@ def compute_statistics( .distinct() ).all() ) + progress_session_ids = set( + session.exec( + select(ReadingProgress.session_id) + .where( + ReadingProgress.user_id == user_id, + col(ReadingProgress.session_id).is_not(None), + ) + .distinct() + ).all() + ) + # Virtual page-0 anchors, now per session so a re-read restarts its chain at + # the new session's start date instead of chaining across reads. virtual_entries = [] - for book in books: - if book.id not in all_book_ids_with_progress or not book.date_started: + for reading_session in sessions: + if reading_session.date_started is None: continue - if book.reading_status == ReadingStatus.read and not book.date_finished: + if reading_session.status == ReadingStatus.read and reading_session.date_finished is None: continue virtual_entries.append( SimpleNamespace( - book_id=book.id, + book_id=reading_session.book_id, + session_id=reading_session.id, page=0, - created_at=book.date_started, + created_at=reading_session.date_started, ) ) + for book in books: + if book.id in books_with_sessions: + continue + if book.id not in all_book_ids_with_progress or not book.date_started: + continue + if book.reading_status == ReadingStatus.read and not book.date_finished: + continue + virtual_entries.append( + SimpleNamespace(book_id=book.id, session_id=None, page=0, created_at=book.date_started) + ) all_progress_entries = list(progress_entries) + virtual_entries pages_read_per_month_counter = _compute_pages_per_month_from_progress( all_progress_entries, tz, window_start, window_end ) - fallback_books = [ - b - for b in books - if b.id not in all_book_ids_with_progress - and b.reading_status == ReadingStatus.read - and b.date_started - and b.date_finished - and b.page_count - ] + # Fallback: completed reads without progress entries get their pages spread + # over their reading span. Built per session (and per session-less book). + fallback_intervals = [] + for reading_session in sessions: + if reading_session.status != ReadingStatus.read: + continue + if not (reading_session.date_started and reading_session.date_finished): + continue + if reading_session.id in progress_session_ids: + continue + book = book_by_id.get(reading_session.book_id) + if book is None or not book.page_count: + continue + fallback_intervals.append( + SimpleNamespace( + book_id=reading_session.book_id, + session_id=reading_session.id, + date_started=reading_session.date_started, + date_finished=reading_session.date_finished, + page_count=book.page_count, + ) + ) + for book in books: + if book.id in books_with_sessions or book.id in all_book_ids_with_progress: + continue + if book.reading_status == ReadingStatus.read and book.date_started and book.date_finished and book.page_count: + fallback_intervals.append( + SimpleNamespace( + book_id=book.id, + session_id=None, + date_started=book.date_started, + date_finished=book.date_finished, + page_count=book.page_count, + ) + ) + fallback_monthly = _compute_pages_per_month_from_books( - fallback_books, tz, window_start, window_end + fallback_intervals, tz, window_start, window_end ) for k, v in fallback_monthly.items(): pages_read_per_month_counter[k] += v @@ -925,4 +1056,6 @@ def _rating_sort_key(book: Book) -> tuple[int, float]: average_rating=average_rating, top_rated_books=top_rated_books, worst_rated_books=worst_rated_books, + total_reads=total_reads, + reread_books=reread_books, ) diff --git a/backend/app/services/tags.py b/backend/app/services/tags.py index 85eb353c..b8be4fee 100644 --- a/backend/app/services/tags.py +++ b/backend/app/services/tags.py @@ -140,10 +140,17 @@ def load_tags_batch(session: Session, book_ids: list[int]) -> dict[int, str | No def build_book_read(session: Session, book: Book) -> BookRead: """Build a BookRead response schema from a Book model, populating tags.""" + if book.id is not None: + session.refresh(book) 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) + from app.services.reading_sessions import session_counts + + counts = session_counts(session, [book.id or 0]).get(book.id or 0, {}) + payload["session_count"] = counts.get("session_count", 0) + payload["read_count"] = counts.get("read_count", 0) return BookRead.model_validate(payload) diff --git a/backend/app/services/user_deletion.py b/backend/app/services/user_deletion.py index 9cd11067..1fc87870 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, Author, Book, BookAuthor, BookTag, OidcLink, ReadingProgress, Tag, User, UserRole, UserSettings +from app.models import ApiKey, Author, Book, BookAuthor, BookTag, OidcLink, ReadingProgress, ReadingSession, Tag, User, UserRole, UserSettings from app.time_utils import utcnow from app.services.cover_storage import delete_cover_file, local_cover_filename @@ -18,6 +18,7 @@ class ReadingDataDeletionCounts: tags: int authors: int progress_entries: int + sessions: int = 0 def assert_not_last_admin(session: Session, target_user: User) -> None: @@ -45,6 +46,9 @@ def delete_user_reading_data(session: Session, user_id: int, covers_dir: str) -> progress_count = session.exec( select(func.count()).select_from(ReadingProgress).where(ReadingProgress.user_id == user_id) ).one() + sessions_count = session.exec( + select(func.count()).select_from(ReadingSession).where(ReadingSession.user_id == user_id) + ).one() tags_count = session.exec( select(func.count()).select_from(Tag).where(Tag.user_id == user_id) ).one() @@ -72,6 +76,9 @@ def delete_user_reading_data(session: Session, user_id: int, covers_dir: str) -> for entry in session.exec(select(ReadingProgress).where(ReadingProgress.user_id == user_id)).all(): session.delete(entry) + for reading_session in session.exec(select(ReadingSession).where(ReadingSession.user_id == user_id)).all(): + session.delete(reading_session) + for tag in session.exec(select(Tag).where(Tag.user_id == user_id)).all(): session.delete(tag) @@ -86,6 +93,7 @@ def delete_user_reading_data(session: Session, user_id: int, covers_dir: str) -> tags=tags_count, authors=authors_count, progress_entries=progress_count, + sessions=sessions_count, ) diff --git a/backend/tests/test_reading_sessions.py b/backend/tests/test_reading_sessions.py new file mode 100644 index 00000000..0ff78e95 --- /dev/null +++ b/backend/tests/test_reading_sessions.py @@ -0,0 +1,122 @@ +"""Tests for reading sessions: CRUD, re-read flow, and statistics integration.""" + +from typing import Any + +from fastapi.testclient import TestClient + + +def _create_book(client: TestClient, **kwargs: Any) -> dict[str, Any]: + payload = {"title": "Book", "author": "Author", "page_count": 200, **kwargs} + resp = client.post("/api/books", json=payload) + assert resp.status_code == 201, resp.text + return resp.json() + + +def _read_again(client: TestClient, book_id: int) -> dict[str, Any]: + resp = client.post(f"/api/books/{book_id}/read-again", json={}) + assert resp.status_code == 200, resp.text + return resp.json() + + +def test_create_book_with_read_status_creates_session(client: TestClient) -> None: + book = _create_book(client, reading_status="read", date_started="2024-01-01T00:00:00Z", date_finished="2024-01-10T00:00:00Z") + assert book["session_count"] == 1 + assert book["read_count"] == 1 + + resp = client.get(f"/api/books/{book['id']}/sessions") + assert resp.status_code == 200 + sessions = resp.json() + assert len(sessions) == 1 + assert sessions[0]["status"] == "read" + assert sessions[0]["read_number"] == 1 + assert sessions[0]["is_reread"] is False + + +def test_read_again_creates_second_session(client: TestClient) -> None: + book = _create_book(client, reading_status="read", date_started="2024-01-01T00:00:00Z", date_finished="2024-01-10T00:00:00Z") + + result = _read_again(client, book["id"]) + assert result["book"]["reading_status"] == "currently_reading" + assert result["book"]["date_finished"] is None + assert result["session"]["status"] == "currently_reading" + assert result["session"]["is_current"] is True + + sessions = client.get(f"/api/books/{book['id']}/sessions").json() + assert len(sessions) == 2 + statuses = {s["status"] for s in sessions} + assert statuses == {"read", "currently_reading"} + + # The completed session keeps its original finish date. + completed = next(s for s in sessions if s["status"] == "read") + assert completed["date_finished"] == "2024-01-10T00:00:00Z" + + +def test_read_again_requires_finished_book(client: TestClient) -> None: + book = _create_book(client, reading_status="want_to_read") + resp = client.post(f"/api/books/{book['id']}/read-again", json={}) + assert resp.status_code == 422 + + +def test_add_past_read_session(client: TestClient) -> None: + book = _create_book(client, reading_status="read", date_started="2024-01-01T00:00:00Z", date_finished="2024-01-10T00:00:00Z") + resp = client.post( + f"/api/books/{book['id']}/sessions", + json={"status": "read", "date_started": "2023-05-01T00:00:00Z", "date_finished": "2023-05-08T00:00:00Z"}, + ) + assert resp.status_code == 201 + assert resp.json()["book"]["reading_status"] == "read" # past read does not change the book + + sessions = client.get(f"/api/books/{book['id']}/sessions").json() + assert len(sessions) == 2 + read_numbers = sorted(s["read_number"] for s in sessions) + assert read_numbers == [1, 2] + reread = next(s for s in sessions if s["read_number"] == 2) + assert reread["is_reread"] is True + + +def test_delete_last_session_resets_book(client: TestClient) -> None: + book = _create_book(client, reading_status="read", date_started="2024-01-01T00:00:00Z", date_finished="2024-01-10T00:00:00Z") + sessions = client.get(f"/api/books/{book['id']}/sessions").json() + session_id = sessions[0]["id"] + + resp = client.delete(f"/api/books/{book['id']}/sessions/{session_id}") + assert resp.status_code == 200 + assert resp.json()["reading_status"] == "want_to_read" + assert resp.json()["date_started"] is None + assert resp.json()["date_finished"] is None + + +def test_sessions_are_scoped_to_owner(client: TestClient, create_user_with_key: Any) -> None: + book = _create_book(client, reading_status="read", date_started="2024-01-01T00:00:00Z", date_finished="2024-01-10T00:00:00Z") + _other, other_key = create_user_with_key(email="other@example.com") + + resp = client.get( + f"/api/books/{book['id']}/sessions", + headers={"X-API-Key": other_key}, + ) + assert resp.status_code == 404 + + +def test_statistics_count_completions_and_rereads(client: TestClient) -> None: + book = _create_book(client, reading_status="read", date_started="2024-01-01T00:00:00Z", date_finished="2024-01-10T00:00:00Z", page_count=200) + _read_again(client, book["id"]) + + resp = client.get("/api/statistics") + data = resp.json() + assert data["total_books"] == 1 + assert data["status_distribution"]["read"] == 0 # now currently_reading + # The completed first read still counts as one completion. + assert data["total_reads"] == 1 + assert data["reread_books"] == 0 # only one completed session so far + assert data["page_buckets"]["pages_read"] == 200 + + +def test_progress_scoped_to_session(client: TestClient) -> None: + book = _create_book(client, reading_status="read", date_started="2024-01-01T00:00:00Z", date_finished="2024-01-10T00:00:00Z", page_count=200) + client.post(f"/api/books/{book['id']}/progress", json={"page": 200}) + + _read_again(client, book["id"]) + + # Latest progress now reflects the fresh (empty) current session: no entry yet. + latest = client.get(f"/api/books/progress/latest?book_ids={book['id']}").json() + assert latest == [] From 1cecfc7744ac22741206a8713230f886746996fd Mon Sep 17 00:00:00 2001 From: codebude Date: Thu, 22 Oct 2026 00:36:33 +0200 Subject: [PATCH 02/25] Frontend: reading sessions/re-reads UI (issue #114) - Book detail: Read again action + reading history (add/delete sessions) - Session-aware progress, read xN badge on cards/list rows - Timeline one-row-per-completion with reread badge - Statistics Total reads card, public profile total_reads opt-in - DateConflictDialog third 'start new read' action - i18n keys across 5 locales, sessions export dataset --- backend/app/main.py | 2 +- backend/app/routers/sessions.py | 2 +- docs/releases.md | 6 + frontend/src/lib/api.ts | 41 +++- frontend/src/lib/components/BookCard.svelte | 6 + .../lib/components/BookDetailDialog.svelte | 220 +++++++++++++++++- frontend/src/lib/components/BookDrawer.svelte | 18 ++ .../src/lib/components/BookListItem.svelte | 7 + frontend/src/lib/components/DataExport.svelte | 1 + .../lib/components/DateConflictDialog.svelte | 28 ++- frontend/src/lib/i18n/locales/de.json | 25 +- frontend/src/lib/i18n/locales/en.json | 25 +- frontend/src/lib/i18n/locales/es.json | 25 +- frontend/src/lib/i18n/locales/fr.json | 25 +- frontend/src/lib/i18n/locales/zh.json | 25 +- .../src/lib/publicProfile/sections.test.ts | 3 +- frontend/src/lib/publicProfile/sections.ts | 6 + frontend/src/lib/types.ts | 52 ++++- frontend/src/routes/statistics/+page.svelte | 7 + frontend/src/routes/timeline/+page.svelte | 169 ++++---------- 20 files changed, 521 insertions(+), 172 deletions(-) diff --git a/backend/app/main.py b/backend/app/main.py index 4529e3cc..5af66a3e 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -219,6 +219,7 @@ async def proxy_headers_middleware(request: Request, call_next) -> Response: return await call_next(request) +app.include_router(sessions.router) app.include_router(books.router) app.include_router(import_.router) app.include_router(covers.router) @@ -230,7 +231,6 @@ async def proxy_headers_middleware(request: Request, call_next) -> Response: app.include_router(public_profile.router) app.include_router(oidc.router) app.include_router(progress.router) -app.include_router(sessions.router) app.include_router(docs.router) app.include_router(health.router) app.include_router(hygiene.router) diff --git a/backend/app/routers/sessions.py b/backend/app/routers/sessions.py index d608e5ba..e40923ba 100644 --- a/backend/app/routers/sessions.py +++ b/backend/app/routers/sessions.py @@ -70,7 +70,7 @@ def _to_session_reads( ] -@router.get("", response_model=List[TimelineSessionEntry]) +@router.get("/sessions", response_model=List[TimelineSessionEntry]) def list_completed_sessions( current_user: User = Depends(require_user), session: Session = Depends(get_session), diff --git a/docs/releases.md b/docs/releases.md index 31b806ac..c413d96f 100644 --- a/docs/releases.md +++ b/docs/releases.md @@ -6,6 +6,12 @@ All LibrisLog releases, newest first — what's new, what was fixed, and anythin You can also browse the [GitHub Releases](https://github.com/codebude/librislog/releases) page and the [full changelog](https://github.com/codebude/librislog/commits/main). +## Unreleased + +::: tip ⭐ Reading Sessions & Re-reads +Books can now be read more than once. Each reading is tracked as its own session with independent start and finish dates, and a "Read again" action starts a fresh reading without overwriting earlier dates or progress. A reading history in the book detail dialog lets you add past reads and edit or delete individual sessions, progress restarts per session, and re-read books show a "read ×N" badge. Statistics now distinguish unique books from total completions (with a new "Total reads" card), and the timeline lists one row per completed read, marking re-reads. Data migrates automatically on upgrade. +::: + ## Latest Release ::: tip ⭐ v1.10.0 — More Book Sources & Upgrades diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index 5a44e05f..a33113b1 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -50,7 +50,11 @@ import type { User, UserCreateResponse, UserAdminUpdate, - UserSettings + UserSettings, + ReadingSession, + ReadingSessionInput, + SessionMutationResponse, + TimelineSessionEntry } from './types'; import { apiKey, csrfToken } from './stores/auth'; import { get } from 'svelte/store'; @@ -462,8 +466,9 @@ export const api = { }, progress: { - async list(bookId: number): Promise { - return request(`/books/${bookId}/progress`); + async list(bookId: number, sessionId?: number | null): Promise { + const qs = sessionId !== undefined && sessionId !== null ? `?session_id=${sessionId}` : ''; + return request(`/books/${bookId}/progress${qs}`); }, async create(bookId: number, page: number): Promise { return request(`/books/${bookId}/progress`, { @@ -493,6 +498,36 @@ export const api = { }); }, + sessions: { + list(bookId: number): Promise { + return request(`/books/${bookId}/sessions`); + }, + create(bookId: number, data: ReadingSessionInput): Promise { + return request(`/books/${bookId}/sessions`, { + method: 'POST', + body: JSON.stringify(data) + }); + }, + update(bookId: number, sessionId: number, data: Partial): Promise { + return request(`/books/${bookId}/sessions/${sessionId}`, { + method: 'PATCH', + body: JSON.stringify(data) + }); + }, + remove(bookId: number, sessionId: number): Promise { + return request(`/books/${bookId}/sessions/${sessionId}`, { method: 'DELETE' }); + }, + readAgain(bookId: number, dateStarted?: string | null): Promise { + return request(`/books/${bookId}/read-again`, { + method: 'POST', + body: JSON.stringify({ date_started: dateStarted ?? null }) + }); + }, + completed(): Promise { + return request('/books/sessions'); + } + }, + delete(id: number): Promise { return request(`/books/${id}`, { method: 'DELETE' }); } diff --git a/frontend/src/lib/components/BookCard.svelte b/frontend/src/lib/components/BookCard.svelte index 584471e8..458fa47b 100644 --- a/frontend/src/lib/components/BookCard.svelte +++ b/frontend/src/lib/components/BookCard.svelte @@ -103,6 +103,12 @@ {$_(STATUS_LABEL_KEYS[book.reading_status])} + {#if (book.read_count ?? 0) >= 2} + ×{book.read_count} + {/if} diff --git a/frontend/src/lib/components/BookDetailDialog.svelte b/frontend/src/lib/components/BookDetailDialog.svelte index 06347fa8..b7099487 100644 --- a/frontend/src/lib/components/BookDetailDialog.svelte +++ b/frontend/src/lib/components/BookDetailDialog.svelte @@ -1,5 +1,5 @@ {#if open && book} @@ -656,6 +672,8 @@ field={conflictField} existingDate={formatDate(conflictExistingDate, tz)} suggestedDate={formatDate(conflictSuggestedDate, tz)} + rereadAvailable={(book.read_count ?? 0) >= 1 && (conflictField === 'date_started' || conflictField === 'started_after_finished')} + onReread={() => void startNewReadFromConflict()} onCancel={() => { dateConflictOpen = false; pendingStatus = null; diff --git a/frontend/src/lib/components/BookListItem.svelte b/frontend/src/lib/components/BookListItem.svelte index 614883b1..5f2a7a48 100644 --- a/frontend/src/lib/components/BookListItem.svelte +++ b/frontend/src/lib/components/BookListItem.svelte @@ -70,6 +70,13 @@ {$_(STATUS_LABEL_KEYS[book.reading_status])} + {#if (book.read_count ?? 0) >= 2} + + {/if} + {#if progressPercent > 0}
{$_('statistics.sectionDistributions')}
diff --git a/frontend/src/routes/timeline/+page.svelte b/frontend/src/routes/timeline/+page.svelte index 4cc209b4..4b6cc13d 100644 --- a/frontend/src/routes/timeline/+page.svelte +++ b/frontend/src/routes/timeline/+page.svelte @@ -1,7 +1,7 @@ @@ -205,7 +127,7 @@
- {:else if books.length === 0} + {:else if entries.length === 0}
@@ -216,7 +138,7 @@
{:else}
    - {#each flatItems as item, i (item.type === 'header' ? item.key : item.book.id)} + {#each flatItems as item, i (item.type === 'header' ? item.key : item.entry.session_id)}
  • {#if i > 0}
    {/if} {#if item.type === 'header'} @@ -230,7 +152,7 @@
{:else}
- {formatDay(item.book.date_finished!)} + {formatDay(item.entry.date_finished!)}
@@ -239,31 +161,34 @@
openDetailView(item.book)} + onclick={() => void openDetailView(item.entry)} role="button" tabindex="0" onkeydown={(e) => { if (e.key === 'Enter' || e.key === ' ') { e.preventDefault(); - openDetailView(item.book); + void openDetailView(item.entry); } }} >
- {#if item.book.cover_url} + {#if item.entry.cover_url} {item.book.title} {/if}
-

{item.book.title}

- {#if item.book.author} -

{formatAuthors(item.book.authors, item.book.author)}

+

{item.entry.title}

+ {#if item.entry.author} +

{formatAuthors(item.entry.authors, item.entry.author)}

+ {/if} + {#if item.entry.rating} +

{stars(item.entry.rating)}

{/if} - {#if item.book.rating} -

{stars(item.book.rating)}

+ {#if item.entry.is_reread} + {$_('timeline.reread')} {/if}
@@ -273,14 +198,6 @@ {/each} - {#if hasMore} -
- {/if} - {#if loadingMore} -
- -
- {/if} {/if}
From 24fff0392be11d48eb225109b812f8d6c8cc532f Mon Sep 17 00:00:00 2001 From: codebude Date: Thu, 22 Oct 2026 00:49:06 +0200 Subject: [PATCH 03/25] Fix migration: omit FK constraint (SQLite does not support ALTER ADD CONSTRAINT) --- .../versions/a3b4c5d6e7f8_add_reading_session_table.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/backend/alembic/versions/a3b4c5d6e7f8_add_reading_session_table.py b/backend/alembic/versions/a3b4c5d6e7f8_add_reading_session_table.py index 739d6b2f..84c9e28a 100644 --- a/backend/alembic/versions/a3b4c5d6e7f8_add_reading_session_table.py +++ b/backend/alembic/versions/a3b4c5d6e7f8_add_reading_session_table.py @@ -59,10 +59,9 @@ def upgrade() -> None: op.add_column('reading_progress', sa.Column('session_id', sa.Integer(), nullable=True)) op.create_index(op.f('ix_reading_progress_session_id'), 'reading_progress', ['session_id'], unique=False) - op.create_foreign_key( - 'fk_reading_progress_session_id', 'reading_progress', 'reading_session', - ['session_id'], ['id'], - ) + # SQLite does not support adding a FOREIGN KEY constraint via ALTER TABLE, + # and this project does not enforce FKs at the SQLite level anyway. The + # relationship is maintained in the application layer. bind = op.get_bind() now = datetime.now(timezone.utc).replace(tzinfo=None) @@ -110,7 +109,6 @@ def upgrade() -> None: def downgrade() -> None: - op.drop_constraint('fk_reading_progress_session_id', 'reading_progress', type_='foreignkey') op.drop_index(op.f('ix_reading_progress_session_id'), table_name='reading_progress') op.drop_column('reading_progress', 'session_id') op.drop_index('uq_reading_session_book_current', table_name='reading_session') From 25eee57bd18cd8ad8e408ee929be91c1a9c8ffb3 Mon Sep 17 00:00:00 2001 From: codebude Date: Thu, 22 Oct 2026 08:32:44 +0200 Subject: [PATCH 04/25] Address code review findings (go-1) - Fix session 'latest' resolution: order by reading date, not created_at (data-loss) - Make reconcile idempotent (unrelated PATCH no longer rewrites history) - Re-mirror book after any session delete; never promote a completed read to current - Validate session_id on progress create (cross-tenant check) - pages_read/total_reads no longer require a finish date - Validate-before-mutate and single-transaction session update/delete - Skip want_to_read books in migration; create sessions on external-book import - Drop FK from ReadingProgress.session_id model (matches SQLite migration) - Conflict dialog keeps keep/use-new actions alongside 'start new read' - Timeline read-numbers computed once; register-order comment; session edit UI - Regression tests for all of the above --- .../a3b4c5d6e7f8_add_reading_session_table.py | 9 +-- backend/app/main.py | 3 + backend/app/models.py | 5 +- backend/app/routers/import_.py | 2 + backend/app/routers/progress.py | 11 ++- backend/app/routers/sessions.py | 75 +++++++++-------- backend/app/services/reading_sessions.py | 34 ++++++-- backend/app/services/statistics.py | 28 ++++--- backend/tests/test_reading_sessions.py | 80 +++++++++++++++++++ .../lib/components/BookDetailDialog.svelte | 28 ++++++- .../lib/components/DateConflictDialog.svelte | 19 +++-- frontend/src/routes/library/+page.svelte | 1 + frontend/src/routes/timeline/+page.svelte | 2 +- 13 files changed, 223 insertions(+), 74 deletions(-) diff --git a/backend/alembic/versions/a3b4c5d6e7f8_add_reading_session_table.py b/backend/alembic/versions/a3b4c5d6e7f8_add_reading_session_table.py index 84c9e28a..1f9b1b3d 100644 --- a/backend/alembic/versions/a3b4c5d6e7f8_add_reading_session_table.py +++ b/backend/alembic/versions/a3b4c5d6e7f8_add_reading_session_table.py @@ -72,11 +72,10 @@ def upgrade() -> None: sa.text("SELECT id, user_id, reading_status, date_started, date_finished FROM book") ).fetchall() for book_id, user_id, status, date_started, date_finished in books: - has_progress = bind.execute( - sa.text("SELECT 1 FROM reading_progress WHERE book_id = :bid LIMIT 1"), - {"bid": book_id}, - ).first() - if status == 'want_to_read' and date_started is None and date_finished is None and not has_progress: + # want_to_read books never get a session: their (legacy) progress stays + # unassigned and continues through the book-level statistics path, so + # their numbers do not change on upgrade. + if status == 'want_to_read': continue session_status = _session_status(status, date_started, date_finished) diff --git a/backend/app/main.py b/backend/app/main.py index 5af66a3e..4eeb8624 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -219,6 +219,9 @@ async def proxy_headers_middleware(request: Request, call_next) -> Response: return await call_next(request) +# The sessions router must be registered before the books router: both define +# routes under /api/books, and `GET /api/books/sessions` (the timeline) must win +# over the books router's `GET /api/books/{book_id}`. Do not reorder. app.include_router(sessions.router) app.include_router(books.router) app.include_router(import_.router) diff --git a/backend/app/models.py b/backend/app/models.py index a323ad09..9f47850e 100644 --- a/backend/app/models.py +++ b/backend/app/models.py @@ -304,7 +304,10 @@ class ReadingProgress(SQLModel, table=True): id: Optional[int] = Field(default=None, primary_key=True) book_id: int = Field(foreign_key="book.id", index=True) user_id: int = Field(foreign_key="user.id", index=True) - session_id: Optional[int] = Field(default=None, foreign_key="reading_session.id", index=True) + # Note: no FK to reading_session here. SQLite cannot add a FOREIGN KEY via + # ALTER TABLE, and this project does not enforce FKs at the SQLite level, so + # the relationship is maintained in the application layer only. + session_id: Optional[int] = Field(default=None, index=True) page: int = Field(ge=0) created_at: datetime = Field( default_factory=utcnow, diff --git a/backend/app/routers/import_.py b/backend/app/routers/import_.py index 15856d1f..9dcc42cc 100644 --- a/backend/app/routers/import_.py +++ b/backend/app/routers/import_.py @@ -19,6 +19,7 @@ from app.services.authors import resolve_authors_payload, sync_book_authors from app.services.cover_import import is_safe_cover_import_url from app.services.cover_storage import download_cover +from app.services.reading_sessions import create_initial_session from app.services.tags import build_book_read, sync_book_tags logger = logging.getLogger(__name__) @@ -169,6 +170,7 @@ async def import_book( except IntegrityError as exc: session.rollback() _raise_integrity_conflict(exc) + create_initial_session(session, book) 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) diff --git a/backend/app/routers/progress.py b/backend/app/routers/progress.py index 459d0483..05cb9401 100644 --- a/backend/app/routers/progress.py +++ b/backend/app/routers/progress.py @@ -40,7 +40,16 @@ def create_progress_entry( ) session_id = data.session_id - if session_id is None: + if session_id is not None: + # A caller-supplied session id must belong to this book and this user. + supplied = session.get(ReadingSession, session_id) + if ( + not supplied + or supplied.book_id != book_id + or supplied.user_id != current_user.id + ): + raise HTTPException(status_code=404, detail="Reading session not found") + else: current = active_session(session, book_id) if current is not None: session_id = current.id diff --git a/backend/app/routers/sessions.py b/backend/app/routers/sessions.py index e40923ba..872c55cf 100644 --- a/backend/app/routers/sessions.py +++ b/backend/app/routers/sessions.py @@ -91,10 +91,14 @@ def list_completed_sessions( for rs, _book in rows: per_book.setdefault(rs.book_id, []).append(rs) authors_map = load_authors_batch(session, list(per_book)) + # Compute the ordinal once per book, not once per row. + read_numbers_by_book = { + book_id: assign_read_numbers(s_list) for book_id, s_list in per_book.items() + } result: list[TimelineSessionEntry] = [] for rs, book in rows: - read_number = assign_read_numbers(per_book[rs.book_id]).get(rs.id) + read_number = read_numbers_by_book[rs.book_id].get(rs.id) authors = authors_map.get(book.id, []) result.append( TimelineSessionEntry( @@ -187,39 +191,45 @@ def update_session( reading_session = _get_owned_session(session, book_id, session_id, current_user.id) update = data.model_dump(exclude_unset=True) - if "status" in update and update["status"] == ReadingStatus.want_to_read: + if update.get("status") == ReadingStatus.want_to_read: raise HTTPException(status_code=422, detail="A session cannot be 'want_to_read'.") - _validate_dates( - { - "date_started": update.get("date_started", reading_session.date_started), - "date_finished": update.get("date_finished", reading_session.date_finished), - } - ) - for field, value in update.items(): - setattr(reading_session, field, value) + # Compute the candidate state first, validate it, then apply it. This keeps + # the session unmutated on validation errors and makes the rules explicit. + new_status = update.get("status", reading_session.status) + new_started = update.get("date_started", reading_session.date_started) + new_finished = update.get("date_finished", reading_session.date_finished) + was_current = reading_session.is_current + + if not was_current and new_status == ReadingStatus.currently_reading: + raise HTTPException( + status_code=422, + detail="Use the 'Read again' action to start a current session.", + ) - if reading_session.status == ReadingStatus.read and reading_session.date_finished is None: + # Closing a current session auto-fills a missing finish date. + if was_current and new_status != ReadingStatus.currently_reading and new_finished is None: + new_finished = utcnow() + if new_status == ReadingStatus.read and new_finished is None: raise HTTPException( status_code=422, detail="A completed session must have a finish date." ) + _validate_dates({"date_started": new_started, "date_finished": new_finished}) - was_current = reading_session.is_current - if was_current and reading_session.status != ReadingStatus.currently_reading: + reading_session.status = new_status + reading_session.date_started = new_started + reading_session.date_finished = new_finished + if was_current and new_status != ReadingStatus.currently_reading: reading_session.is_current = False - if reading_session.date_finished is None: - reading_session.date_finished = utcnow() - session.add(reading_session) - session.commit() - session.refresh(reading_session) if was_current: - book.reading_status = reading_session.status + book.reading_status = new_status recompute_book_mirror(book, sessions_for_book(session, book_id)) session.add(book) - session.commit() - session.refresh(book) + + session.commit() + session.refresh(book) sessions = sessions_for_book(session, book_id) return SessionMutationResponse( @@ -242,31 +252,30 @@ def delete_session( """Delete a session and its progress entries, then re-mirror the book.""" book = _get_owned_book(session, book_id, current_user.id) reading_session = _get_owned_session(session, book_id, session_id, current_user.id) - was_current = reading_session.is_current for entry in session.exec( select(ReadingProgress).where(ReadingProgress.session_id == session_id) ).all(): session.delete(entry) - session.delete(reading_session) - session.commit() + session.flush() remaining = sessions_for_book(session, book_id) if not remaining: book.reading_status = ReadingStatus.want_to_read book.date_started = None book.date_finished = None - session.add(book) - session.commit() - elif was_current: - promoted = remaining[0] - promoted.is_current = True - session.add(promoted) - book.reading_status = promoted.status + else: + latest = remaining[0] + # Only an open session can become the new current session; a completed + # read stays closed so it is never relabelled "currently reading". + if latest.status == ReadingStatus.currently_reading: + latest.is_current = True + session.add(latest) recompute_book_mirror(book, remaining) - session.add(book) - session.commit() + book.reading_status = latest.status + session.add(book) + session.commit() return build_book_read(session, book) diff --git a/backend/app/services/reading_sessions.py b/backend/app/services/reading_sessions.py index ab014921..898d6173 100644 --- a/backend/app/services/reading_sessions.py +++ b/backend/app/services/reading_sessions.py @@ -20,6 +20,7 @@ from datetime import datetime from typing import Optional +from sqlalchemy import func from sqlmodel import Session, col, select from app.models import Book, ReadingProgress, ReadingSession, ReadingStatus @@ -48,12 +49,21 @@ def session_counts(session: Session, book_ids: list[int]) -> dict[int, dict[str, def sessions_for_book(session: Session, book_id: int) -> list[ReadingSession]: - """Return all sessions of a book, newest first (by created_at, then id).""" + """Return all sessions of a book, ordered by reading date (latest first). + + The reading date is ``date_finished`` when set, else ``date_started``. + Sessions without any date sort last. This is the ordering every "latest + session" consumer relies on, so a past read added later (newer ``created_at`` + but older dates) never displaces the book's actual most recent read. + """ + reading_date = func.coalesce( + ReadingSession.date_finished, ReadingSession.date_started + ) return list( session.exec( select(ReadingSession) .where(ReadingSession.book_id == book_id) - .order_by(col(ReadingSession.created_at).desc(), col(ReadingSession.id).desc()) + .order_by(reading_date.desc().nullslast(), col(ReadingSession.id).desc()) ).all() ) @@ -247,12 +257,22 @@ def reconcile_book_sessions(session: Session, book: Book) -> None: current.is_current = False session.add(current) else: + # No open session. Find the book's latest completed session of the + # same status and update only if the dates actually changed, so an + # unrelated edit (e.g. rating) is a no-op and never rewrites history. sessions = sessions_for_book(session, book.id) - latest = sessions[0] if sessions else None - if latest is not None and latest.status == book.reading_status: - latest.date_started = book.date_started - latest.date_finished = book.date_finished - session.add(latest) + latest = next( + (s for s in sessions if s.status == book.reading_status), + None, + ) + if latest is not None: + if ( + latest.date_started != book.date_started + or latest.date_finished != book.date_finished + ): + latest.date_started = book.date_started + latest.date_finished = book.date_finished + session.add(latest) else: session.add( ReadingSession( diff --git a/backend/app/services/statistics.py b/backend/app/services/statistics.py index a6ddb911..3240e96f 100644 --- a/backend/app/services/statistics.py +++ b/backend/app/services/statistics.py @@ -665,25 +665,27 @@ def compute_statistics( # "Pages read" counts every completed reading (each completed session), so a # re-read contributes its page count again. Books without sessions fall back # to the book-level value, which keeps single-session users identical. - completed_reads: list[datetime] = [] + # A completed read does not require a finish date (matching the pre-session + # behaviour); only the per-month/per-year series need a date. + completed_read_dates: list[datetime] = [] pages_read = 0 + total_reads = 0 for reading_session in sessions: - if reading_session.status == ReadingStatus.read and reading_session.date_finished is not None: - completed_reads.append(reading_session.date_finished) + if reading_session.status == ReadingStatus.read: + total_reads += 1 book = book_by_id.get(reading_session.book_id) if book is not None and book.page_count is not None: pages_read += book.page_count + if reading_session.date_finished is not None: + completed_read_dates.append(reading_session.date_finished) for book in books: - if ( - book.id not in books_with_sessions - and book.reading_status == ReadingStatus.read - and book.date_finished is not None - ): - completed_reads.append(book.date_finished) + if book.id not in books_with_sessions and book.reading_status == ReadingStatus.read: + total_reads += 1 if book.page_count is not None: pages_read += book.page_count + if book.date_finished is not None: + completed_read_dates.append(book.date_finished) - total_reads = len(completed_reads) reread_books = sum( 1 for book_id, s_list in sessions_by_book.items() @@ -710,14 +712,14 @@ def compute_statistics( ) finished_books_per_month_all_time: Counter[str] = Counter( - _month_key(date_finished, tz) for date_finished in completed_reads + _month_key(date_finished, tz) for date_finished in completed_read_dates ) - finished_dates = completed_reads + finished_dates = completed_read_dates if window_start is not None and window_end is not None: finished_dates = [ date_finished - for date_finished in completed_reads + for date_finished in completed_read_dates if _naive_utc(date_finished) >= window_start and _naive_utc(date_finished) <= window_end ] diff --git a/backend/tests/test_reading_sessions.py b/backend/tests/test_reading_sessions.py index 0ff78e95..81ab7a61 100644 --- a/backend/tests/test_reading_sessions.py +++ b/backend/tests/test_reading_sessions.py @@ -120,3 +120,83 @@ def test_progress_scoped_to_session(client: TestClient) -> None: # Latest progress now reflects the fresh (empty) current session: no entry yet. latest = client.get(f"/api/books/progress/latest?book_ids={book['id']}").json() assert latest == [] + + +def test_unrelated_patch_does_not_change_sessions(client: TestClient) -> None: + """Editing an unrelated field must never rewrite a reading session.""" + book = _create_book(client, reading_status="read", date_started="2024-01-01T00:00:00Z", date_finished="2024-01-10T00:00:00Z") + # Add an older past read. + client.post( + f"/api/books/{book['id']}/sessions", + json={"status": "read", "date_started": "2020-01-01T00:00:00Z", "date_finished": "2020-02-01T00:00:00Z"}, + ) + before = client.get(f"/api/books/{book['id']}/sessions").json() + + # Change only the rating. + resp = client.patch(f"/api/books/{book['id']}", json={"rating": 5}) + assert resp.status_code == 200 + + after = client.get(f"/api/books/{book['id']}/sessions").json() + assert len(after) == 2 + # Sessions must be unchanged: the 2020 read stays 2020-01-01..2020-02-01. + by_id = {s["id"]: s for s in after} + for b in before: + a = by_id[b["id"]] + assert a["date_started"] == b["date_started"] + assert a["date_finished"] == b["date_finished"] + + +def test_delete_non_current_session_remirrors_book(client: TestClient) -> None: + """Deleting a non-current session must re-derive the book mirror.""" + book = _create_book(client, reading_status="read", date_started="2024-01-01T00:00:00Z", date_finished="2024-01-10T00:00:00Z") + # Add an older past read (2020). The mirror stays on the 2024 read. + client.post( + f"/api/books/{book['id']}/sessions", + json={"status": "read", "date_started": "2020-01-01T00:00:00Z", "date_finished": "2020-02-01T00:00:00Z"}, + ) + sessions = client.get(f"/api/books/{book['id']}/sessions").json() + # The 2024 read (date_finished 2024-01-10) is the "latest" session. + latest = next(s for s in sessions if s["date_finished"] == "2024-01-10T00:00:00Z") + + resp = client.delete(f"/api/books/{book['id']}/sessions/{latest['id']}") + assert resp.status_code == 200 + # Mirror must now point at the remaining 2020 read. + assert resp.json()["date_started"] == "2020-01-01T00:00:00Z" + assert resp.json()["date_finished"] == "2020-02-01T00:00:00Z" + + +def test_progress_rejects_foreign_session(client: TestClient, create_user_with_key: Any) -> None: + book_a = _create_book(client, reading_status="read", date_started="2024-01-01T00:00:00Z", date_finished="2024-01-10T00:00:00Z", page_count=200) + session_a = client.get(f"/api/books/{book_a['id']}/sessions").json()[0] + + book_b = _create_book(client, reading_status="read", date_started="2024-01-01T00:00:00Z", date_finished="2024-01-10T00:00:00Z", page_count=200) + + resp = client.post( + f"/api/books/{book_b['id']}/progress", + json={"page": 40, "session_id": session_a["id"]}, + ) + assert resp.status_code == 404 + + +def test_pages_read_counts_read_book_without_finish_date(client: TestClient) -> None: + """A read book without a finish date still contributes to pages_read.""" + _create_book(client, reading_status="read", page_count=70) + resp = client.get("/api/statistics") + data = resp.json() + assert data["page_buckets"]["pages_read"] == 70 + assert data["total_reads"] == 1 + + +def test_completed_sessions_timeline_endpoint(client: TestClient) -> None: + """The completed-reads timeline endpoint returns one row per completed read.""" + book = _create_book(client, reading_status="read", date_started="2024-01-01T00:00:00Z", date_finished="2024-01-10T00:00:00Z") + _read_again(client, book["id"]) + client.post(f"/api/books/{book['id']}/transition-status", json={"new_status": "read"}) + + resp = client.get("/api/books/sessions") + assert resp.status_code == 200 + rows = resp.json() + assert len(rows) == 2 + reread_rows = [r for r in rows if r["is_reread"]] + assert len(reread_rows) == 1 + assert reread_rows[0]["read_number"] == 2 diff --git a/frontend/src/lib/components/BookDetailDialog.svelte b/frontend/src/lib/components/BookDetailDialog.svelte index b7099487..6f16e2f3 100644 --- a/frontend/src/lib/components/BookDetailDialog.svelte +++ b/frontend/src/lib/components/BookDetailDialog.svelte @@ -36,13 +36,15 @@ open = $bindable(false), onEdit, onDelete, - onProgress + onProgress, + onSave }: { book?: Book | null; open?: boolean; onEdit?: (book: Book) => void; onDelete?: (id: number) => void; onProgress?: (bookId: number, currentPage: number) => void; + onSave?: (book: Book) => void; } = $props(); let blurbExpanded = $state(false); @@ -68,6 +70,7 @@ let addSessionStarted = $state(''); let addSessionFinished = $state(''); let addSessionSaving = $state(false); + let editingSessionId = $state(null); let deletingSessionId = $state(null); let confirmDeleteSession = $state(null); @@ -153,6 +156,7 @@ latestDbPage = 0; progressEntries = []; onProgress?.(book.id, 0); + onSave?.(result.book); await loadSessionsAndProgress(); toasts.add($_('book.readAgain'), 'success', 2000); } catch (e: unknown) { @@ -174,10 +178,15 @@ date_started: addSessionStarted ? fromDateTimeInputValue(addSessionStarted, tz) : null, date_finished: addSessionFinished ? fromDateTimeInputValue(addSessionFinished, tz) : null }; - await api.books.sessions.create(book.id, payload); + if (editingSessionId !== null) { + await api.books.sessions.update(book.id, editingSessionId, payload); + } else { + await api.books.sessions.create(book.id, payload); + } addSessionOpen = false; addSessionStarted = ''; addSessionFinished = ''; + editingSessionId = null; await loadSessions(); } catch (e: unknown) { toasts.add( @@ -196,6 +205,7 @@ const updated = await api.books.sessions.remove(book.id, sessionId); book = { ...updated }; confirmDeleteSession = null; + onSave?.(updated); await loadSessionsAndProgress(); } catch (e: unknown) { toasts.add( @@ -218,6 +228,15 @@ addSessionStatus = 'read'; addSessionStarted = ''; addSessionFinished = ''; + editingSessionId = null; + addSessionOpen = true; + } + + function openEditSession(s: ReadingSession) { + addSessionStatus = s.status === 'currently_reading' ? 'read' : s.status; + addSessionStarted = s.date_started ? toDateTimeInputValue(s.date_started, tz) : ''; + addSessionFinished = s.date_finished ? toDateTimeInputValue(s.date_finished, tz) : ''; + editingSessionId = s.id; addSessionOpen = true; } @@ -727,6 +746,9 @@
{:else} +
diff --git a/frontend/src/lib/components/DateConflictDialog.svelte b/frontend/src/lib/components/DateConflictDialog.svelte index d2301bfd..e70e088b 100644 --- a/frontend/src/lib/components/DateConflictDialog.svelte +++ b/frontend/src/lib/components/DateConflictDialog.svelte @@ -64,22 +64,21 @@

+ + {#if rereadAvailable && onReread} - {:else} - - {/if}
- {#if typeKey === 'startedAfterFinished'} + {#if typeKey === 'startedAfterFinished' && !(rereadAvailable && onReread)}

1 {$_(`dateConflict.${typeKey}.keepDesc`, { values: i18nValues })}

2 {$_(`dateConflict.${typeKey}.clearDesc`, { values: i18nValues })}

diff --git a/frontend/src/routes/library/+page.svelte b/frontend/src/routes/library/+page.svelte index d49b6809..e63859fb 100644 --- a/frontend/src/routes/library/+page.svelte +++ b/frontend/src/routes/library/+page.svelte @@ -513,6 +513,7 @@ onEdit={openEditFromDetail} onDelete={handleDelete} onProgress={handleProgressChange} + onSave={handleSave} /> - + From 05f29a43190d9a0079a12c3c8bee629a94414a6c Mon Sep 17 00:00:00 2001 From: codebude Date: Thu, 22 Oct 2026 08:39:00 +0200 Subject: [PATCH 05/25] Address code review findings (go-2 cross-check) - Open (currently_reading) sessions never keep a finish date - pages_wasted scoped to did_not_finish sessions (earlier completions excluded) - Document sessions export as reference-only (full restore via backup) - Regression test for open-session finish-date invariant --- backend/app/routers/sessions.py | 5 ++- backend/app/services/statistics.py | 40 ++++++++++++++++----- backend/tests/test_reading_sessions.py | 16 +++++++++ docs/guide/using-librislog/import-export.md | 3 ++ 4 files changed, 54 insertions(+), 10 deletions(-) diff --git a/backend/app/routers/sessions.py b/backend/app/routers/sessions.py index 872c55cf..bd9e7543 100644 --- a/backend/app/routers/sessions.py +++ b/backend/app/routers/sessions.py @@ -207,9 +207,12 @@ def update_session( detail="Use the 'Read again' action to start a current session.", ) - # Closing a current session auto-fills a missing finish date. + # Closing a current session auto-fills a missing finish date; an open + # (currently_reading) session never carries a finish date. if was_current and new_status != ReadingStatus.currently_reading and new_finished is None: new_finished = utcnow() + if new_status == ReadingStatus.currently_reading: + new_finished = None if new_status == ReadingStatus.read and new_finished is None: raise HTTPException( status_code=422, detail="A completed session must have a finish date." diff --git a/backend/app/services/statistics.py b/backend/app/services/statistics.py index 3240e96f..2466ed2d 100644 --- a/backend/app/services/statistics.py +++ b/backend/app/services/statistics.py @@ -695,15 +695,37 @@ def compute_statistics( dnf_book_ids = [book.id for book in books if book.reading_status == ReadingStatus.did_not_finish and book.id is not None] pages_wasted = 0 if dnf_book_ids: - wasted_rows = session.exec( - select(ReadingProgress.book_id, func.max(ReadingProgress.page)) - .where( - ReadingProgress.user_id == user_id, - col(ReadingProgress.book_id).in_(dnf_book_ids), - ) - .group_by(col(ReadingProgress.book_id)) - ).all() - pages_wasted = int(sum((max_page or 0) for _, max_page in wasted_rows)) + # Only progress belonging to a did_not_finish session counts, so an + # earlier completed read does not inflate the wasted-page total. Books + # without sessions keep the legacy book-level maximum. + dnf_session_ids = { + s.id + for s in sessions + if s.book_id in set(dnf_book_ids) + and s.status == ReadingStatus.did_not_finish + and s.id is not None + } + if dnf_session_ids: + wasted_rows = session.exec( + select(ReadingProgress.book_id, func.max(ReadingProgress.page)) + .where( + ReadingProgress.user_id == user_id, + col(ReadingProgress.session_id).in_(dnf_session_ids), + ) + .group_by(col(ReadingProgress.book_id)) + ).all() + pages_wasted += int(sum((max_page or 0) for _, max_page in wasted_rows)) + sessionless_dnf = [bid for bid in dnf_book_ids if bid not in books_with_sessions] + if sessionless_dnf: + wasted_rows = session.exec( + select(ReadingProgress.book_id, func.max(ReadingProgress.page)) + .where( + ReadingProgress.user_id == user_id, + col(ReadingProgress.book_id).in_(sessionless_dnf), + ) + .group_by(col(ReadingProgress.book_id)) + ).all() + pages_wasted += int(sum((max_page or 0) for _, max_page in wasted_rows)) page_buckets = PageBuckets( pages_to_read=int(pages_to_read), diff --git a/backend/tests/test_reading_sessions.py b/backend/tests/test_reading_sessions.py index 81ab7a61..76debb6d 100644 --- a/backend/tests/test_reading_sessions.py +++ b/backend/tests/test_reading_sessions.py @@ -200,3 +200,19 @@ def test_completed_sessions_timeline_endpoint(client: TestClient) -> None: reread_rows = [r for r in rows if r["is_reread"]] assert len(reread_rows) == 1 assert reread_rows[0]["read_number"] == 2 + + +def test_open_session_never_keeps_a_finish_date(client: TestClient) -> None: + """A currently_reading session must not carry a finish date.""" + book = _create_book(client, reading_status="currently_reading", date_started="2024-01-01T00:00:00Z") + session_id = client.get(f"/api/books/{book['id']}/sessions").json()[0]["id"] + + resp = client.patch( + f"/api/books/{book['id']}/sessions/{session_id}", + json={"date_finished": "2024-02-01T00:00:00Z"}, + ) + assert resp.status_code == 200 + session = resp.json()["session"] + assert session["status"] == "currently_reading" + assert session["date_finished"] is None + assert session["is_current"] is True diff --git a/docs/guide/using-librislog/import-export.md b/docs/guide/using-librislog/import-export.md index 8b2c22e4..cf23bde9 100644 --- a/docs/guide/using-librislog/import-export.md +++ b/docs/guide/using-librislog/import-export.md @@ -61,8 +61,11 @@ Choose which data to include: - Books (full metadata) - Reading progress entries - Tags +- Reading sessions (re-read history; exported for reference and round-trip completeness) - Cover images +Reading sessions are exported for completeness and auditing. The import wizard re-creates one reading session per imported book from its start/finish dates, but re-read history (multiple sessions per book) is only preserved through [backup and restore](./administration), which copies the whole database. + ### Export Process 1. Go to the Data page From 91aa9e4136f3709d4d033d239f288860db992ce4 Mon Sep 17 00:00:00 2001 From: codebude Date: Thu, 22 Oct 2026 09:02:41 +0200 Subject: [PATCH 06/25] Round-trip reading sessions through JSON export/import Embed each book's reading sessions in the JSON book export and reconstruct them on import, so re-read history survives a data export/import round-trip instead of collapsing to a single session. - data_export: embed sessions in books.json (chronological) - data_import: accept a 'sessions' array field and create sessions from it - suggest_mapping alias + BOOK_IMPORT_FIELDS entry for 'sessions' - round-trip test + import-export docs update --- backend/app/services/data_export.py | 40 +++++++- backend/app/services/data_import.py | 69 ++++++++++++- backend/tests/test_data_import.py | 108 +++++++++++++++++++- docs/guide/using-librislog/import-export.md | 6 +- 4 files changed, 213 insertions(+), 10 deletions(-) diff --git a/backend/app/services/data_export.py b/backend/app/services/data_export.py index 2ca3d8fd..838b66cf 100644 --- a/backend/app/services/data_export.py +++ b/backend/app/services/data_export.py @@ -52,12 +52,16 @@ 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, export_format: str) -> dict: +def _book_to_dict( + session: Session, book: Book, export_format: str, sessions: list[ReadingSession] | None = None +) -> 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 + name list, and ``tags`` is a list of tag names. When *sessions* is provided + (JSON only), the book's reading sessions are embedded under ``sessions`` so + re-read history round-trips through the import. 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. """ @@ -66,7 +70,7 @@ def _book_to_dict(session: Session, book: Book, export_format: str) -> dict: 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 { + result = { "title": book.title, "subtitle": book.subtitle, "author": author_value, @@ -88,6 +92,16 @@ def _book_to_dict(session: Session, book: Book, export_format: str) -> dict: "date_finished": _serialize_datetime(book.date_finished), "cover_url": book.cover_url, } + if export_format == "json" and sessions is not None: + result["sessions"] = [ + { + "status": s.status.value, + "date_started": _serialize_datetime(s.date_started), + "date_finished": _serialize_datetime(s.date_finished), + } + for s in sessions + ] + return result def _progress_to_dict(entry: ReadingProgress, book_titles: dict[int, str]) -> dict: @@ -190,7 +204,25 @@ 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, export_format) for book in books] + # Group sessions per book in chronological order (oldest first) so the JSON + # book export can embed them and re-read history round-trips through import. + sessions_by_book: dict[int, list[ReadingSession]] = {} + for entry in sessions: + sessions_by_book.setdefault(entry.book_id, []).append(entry) + for book_sessions in sessions_by_book.values(): + book_sessions.sort( + key=lambda s: (s.date_started or s.date_finished or datetime.min, s.id or 0) + ) + + books_rows = [ + _book_to_dict( + session, + book, + export_format, + sessions_by_book.get(book.id) if export_format == "json" else None, + ) + for book in books + ] progress_rows = [_progress_to_dict(entry, book_titles) for entry in progress_entries] session_rows = [_session_to_dict(entry, book_titles) for entry in sessions] 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 45524d45..379a687d 100644 --- a/backend/app/services/data_import.py +++ b/backend/app/services/data_import.py @@ -23,7 +23,6 @@ from app.config import settings from app.models import AcquisitionStatus, Book, Medium, ReadingProgress, ReadingSession, ReadingStatus, User, normalize_medium_key from app.schemas import ImportFieldConfig -from app.services.reading_sessions import create_initial_session logger = logging.getLogger(__name__) from app.time_utils import utcnow @@ -52,6 +51,7 @@ "date_started", "date_finished", "cover_url", + "sessions", ] _ALIASES: dict[str, str] = { @@ -102,6 +102,8 @@ "date read": "date_finished", "cover": "cover_url", "cover url": "cover_url", + "sessions": "sessions", + "reading sessions": "sessions", } @@ -512,6 +514,62 @@ def _parse_reading_status(value: object) -> ReadingStatus: return ReadingStatus(raw) +def _create_sessions_from_data( + session: Session, book: Book, sessions_data: object +) -> ReadingSession | None: + """Create a book's reading sessions from an embedded ``sessions`` array. + + A LibrisLog JSON export embeds the book's reading sessions as a list of + ``{status, date_started, date_finished}`` objects. This reconstructs them so + re-read history round-trips through the import. Returns the most recently + completed (``read``) session for progress attachment, or ``None`` when there + is no completed session. Falls back to a single initial session when no + embedded sessions are present (external imports). + """ + from app.services.reading_sessions import create_initial_session + + if not isinstance(sessions_data, list) or not sessions_data: + return create_initial_session(session, book) + + parsed: list[tuple[ReadingStatus, datetime | None, datetime | None]] = [] + for item in sessions_data: + if not isinstance(item, dict): + continue + status = _parse_reading_status(item.get("status")) + if status == ReadingStatus.want_to_read: + continue + ds = _parse_datetime(item.get("date_started"), "sessions.date_started") + df = _parse_datetime(item.get("date_finished"), "sessions.date_finished") + parsed.append((status, ds, df)) + + if not parsed: + return create_initial_session(session, book) + + # Chronological order (oldest first); a date-less session sorts first. + parsed.sort(key=lambda t: (t[2] or t[1] or datetime.min, t[1] or datetime.min)) + + latest_completed: ReadingSession | None = None + for idx, (status, ds, df) in enumerate(parsed): + is_current = ( + status == ReadingStatus.currently_reading + and idx == len(parsed) - 1 + and book.reading_status == ReadingStatus.currently_reading + ) + rs = ReadingSession( + book_id=book.id, + user_id=book.user_id, + status=status, + date_started=ds, + date_finished=df, + is_current=is_current, + ) + session.add(rs) + if status == ReadingStatus.read: + latest_completed = rs + session.flush() + return latest_completed + + def _build_transform_cache( mapping: dict[str, "ImportFieldConfig"], ) -> dict[str, Callable[..., Any]]: @@ -568,6 +626,11 @@ def _mapped_row( # Like authors, transforms are skipped for array values. mapped[target] = value continue + if target == "sessions" and isinstance(value, list): + # Reading sessions embed as a list of {status, date_started, + # date_finished} objects (JSON export only). Passed through untouched. + 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 @@ -1000,7 +1063,9 @@ async def execute_import( session.flush() assert book.id is not None - initial_session = create_initial_session(session, book) + initial_session = _create_sessions_from_data( + session, book, row_data.get("sessions") + ) sync_book_authors( session, diff --git a/backend/tests/test_data_import.py b/backend/tests/test_data_import.py index 19260a73..8ea7e75d 100644 --- a/backend/tests/test_data_import.py +++ b/backend/tests/test_data_import.py @@ -17,7 +17,7 @@ from sqlmodel import Session, select from app.config import settings -from app.models import AcquisitionStatus, Book, ReadingStatus, User, UserRole +from app.models import AcquisitionStatus, Book, ReadingSession, 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 @@ -983,6 +983,112 @@ async def test_execute_import_full_library_json_round_trip( assert tags_list_for_book(session, book.id) == row["tags"] +@pytest.mark.anyio +async def test_execute_import_round_trips_reading_sessions( + session: Session, tmp_path: Path, monkeypatch: MonkeyPatch +) -> None: + """Re-read history must survive a JSON export/import round-trip.""" + monkeypatch.setattr(settings, "import_temp_dir", str(tmp_path)) + user = _create_test_user(session) + user_id = _require(user.id) + + book = Book( + title="Re-read Me", + page_count=300, + reading_status=ReadingStatus.read, + acquisition_status=AcquisitionStatus.owned, + date_started=datetime(2024, 1, 1, tzinfo=timezone.utc), + date_finished=datetime(2024, 1, 10, tzinfo=timezone.utc), + user_id=user.id, + ) + session.add(book) + session.flush() + book_id = _require(book.id) + + # Two sessions: the original read and a second (re-)read. + session.add( + ReadingSession( + book_id=book_id, + user_id=user_id, + status=ReadingStatus.read, + date_started=datetime(2024, 1, 1, tzinfo=timezone.utc), + date_finished=datetime(2024, 1, 10, tzinfo=timezone.utc), + is_current=False, + ) + ) + session.add( + ReadingSession( + book_id=book_id, + user_id=user_id, + status=ReadingStatus.read, + date_started=datetime(2026, 5, 1, tzinfo=timezone.utc), + date_finished=datetime(2026, 5, 8, tzinfo=timezone.utc), + is_current=False, + ) + ) + session.commit() + + 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) == 1 + assert exported_rows[0]["sessions"] == [ + { + "status": "read", + "date_started": "2024-01-01T00:00:00Z", + "date_finished": "2024-01-10T00:00:00Z", + }, + { + "status": "read", + "date_started": "2026-05-01T00:00:00Z", + "date_finished": "2026-05-08T00:00:00Z", + }, + ] + + # Remove the source book and its sessions. + for s in session.exec(select(ReadingSession).where(ReadingSession.book_id == book_id)).all(): + session.delete(s) + session.delete(book) + session.commit() + + file_id = "roundtrip_sessions" + 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", + "page_count", + "reading_status", + "acquisition_status", + "date_started", + "date_finished", + "sessions", + ) + } + events = [] + async for event in di.execute_import( + file_id, user, mapping, session, "continue_on_error", require_acquisition_status=True + ): + events.append(event) + assert [e for e in events if e["event"] == "complete"][0]["imported"] == 1 + + imported = session.exec(select(Book).where(Book.user_id == user.id)).one() + sessions = list( + session.exec(select(ReadingSession).where(ReadingSession.book_id == imported.id)).all() + ) + assert len(sessions) == 2 + finished = sorted(s.date_finished for s in sessions) + assert finished == [ + datetime(2024, 1, 10, tzinfo=timezone.utc), + datetime(2026, 5, 8, tzinfo=timezone.utc), + ] + + @pytest.mark.anyio async def test_execute_import_preserves_date_added( session: Session, tmp_path: Path, monkeypatch: MonkeyPatch diff --git a/docs/guide/using-librislog/import-export.md b/docs/guide/using-librislog/import-export.md index cf23bde9..8fa813b7 100644 --- a/docs/guide/using-librislog/import-export.md +++ b/docs/guide/using-librislog/import-export.md @@ -58,13 +58,13 @@ Export your entire library or subsets of data: ### Export Datasets Choose which data to include: -- Books (full metadata) +- Books (full metadata, including reading sessions) - Reading progress entries - Tags -- Reading sessions (re-read history; exported for reference and round-trip completeness) +- Reading sessions (re-read history) - Cover images -Reading sessions are exported for completeness and auditing. The import wizard re-creates one reading session per imported book from its start/finish dates, but re-read history (multiple sessions per book) is only preserved through [backup and restore](./administration), which copies the whole database. +Reading sessions (re-read history) are embedded in the JSON book export, so a JSON export round-trips losslessly: re-importing `books.json` recreates each book's reading sessions. The separate `sessions` dataset is a flat, book-ID-referenced view exported for auditing and CSV completeness; re-read history is only preserved through the JSON book export or through [backup and restore](./administration), which copies the whole database. ### Export Process From 3b60a12cab7a01ae6f8e8e43cdb53412c91a33e5 Mon Sep 17 00:00:00 2001 From: codebude Date: Thu, 22 Oct 2026 09:09:45 +0200 Subject: [PATCH 07/25] Docs: clarify CSV cannot round-trip re-read history --- docs/guide/using-librislog/import-export.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/guide/using-librislog/import-export.md b/docs/guide/using-librislog/import-export.md index 8fa813b7..4e72bac6 100644 --- a/docs/guide/using-librislog/import-export.md +++ b/docs/guide/using-librislog/import-export.md @@ -64,7 +64,7 @@ Choose which data to include: - Reading sessions (re-read history) - Cover images -Reading sessions (re-read history) are embedded in the JSON book export, so a JSON export round-trips losslessly: re-importing `books.json` recreates each book's reading sessions. The separate `sessions` dataset is a flat, book-ID-referenced view exported for auditing and CSV completeness; re-read history is only preserved through the JSON book export or through [backup and restore](./administration), which copies the whole database. +Reading sessions (re-read history) are embedded in the JSON book export, so a JSON export round-trips losslessly: re-importing `books.json` recreates each book's reading sessions. The CSV format cannot represent nested sessions, so re-read history does **not** round-trip through a CSV export/import; a CSV export only carries the flat `sessions` dataset as a book-ID-referenced audit view. For full fidelity of re-read history, use the JSON export or [backup and restore](./administration), which copies the whole database. ### Export Process From b07b25439b7a725812c76e6550b14efe271c8f1f Mon Sep 17 00:00:00 2001 From: codebude Date: Thu, 22 Oct 2026 09:24:20 +0200 Subject: [PATCH 08/25] Show read count badge on book detail page (from first reread on) --- frontend/src/lib/components/BookDetailDialog.svelte | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/frontend/src/lib/components/BookDetailDialog.svelte b/frontend/src/lib/components/BookDetailDialog.svelte index 6f16e2f3..070d473a 100644 --- a/frontend/src/lib/components/BookDetailDialog.svelte +++ b/frontend/src/lib/components/BookDetailDialog.svelte @@ -581,7 +581,15 @@
{formatAuthors(book.authors, book.author ?? '-')}
- {$_(STATUS_LABEL_KEYS[book.reading_status])} +
+ {#if (book.read_count ?? 0) >= 2} + ×{book.read_count} + {/if} + {$_(STATUS_LABEL_KEYS[book.reading_status])} +
{#if showReadAgain} From 1e8db901713dd701884df7d20a6cdb192b123cc3 Mon Sep 17 00:00:00 2001 From: codebude Date: Thu, 22 Oct 2026 10:31:25 +0200 Subject: [PATCH 09/25] Restore timeline pagination (max 40 + load more) and SQL window function for progress/latest --- backend/app/routers/progress.py | 66 +++++++++++++++-------- backend/app/routers/sessions.py | 59 ++++++++++++++++++-- frontend/src/lib/api.ts | 5 +- frontend/src/routes/timeline/+page.svelte | 45 +++++++++++++++- 4 files changed, 148 insertions(+), 27 deletions(-) diff --git a/backend/app/routers/progress.py b/backend/app/routers/progress.py index 05cb9401..b51f093d 100644 --- a/backend/app/routers/progress.py +++ b/backend/app/routers/progress.py @@ -4,6 +4,7 @@ from typing import List, Optional from fastapi import APIRouter, Depends, HTTPException, Query +from sqlalchemy import func from sqlmodel import Session, col, select from app.auth import require_user @@ -174,6 +175,10 @@ def get_latest_progress_batch( *that* session is returned, so a re-read correctly shows 0 % while the previous session's entries stay separate. Books without a current session fall back to the latest page across all entries (legacy behaviour). + + Implemented with a SQL window function so only the latest entry per + ``(book, session)`` is materialized; the per-book selection of the current + session (or the overall latest) happens in Python over that small result set. """ ids = [int(i.strip()) for i in book_ids.split(",") if i.strip()] if not ids: @@ -189,32 +194,51 @@ def get_latest_progress_batch( ).all() } - entries = list( + # Latest entry per (book_id, session_id), computed in SQL. + rn = func.row_number().over( + partition_by=[col(ReadingProgress.book_id), col(ReadingProgress.session_id)], + order_by=col(ReadingProgress.created_at).desc(), + ) + subq = ( + select( + ReadingProgress.book_id.label("book_id"), + ReadingProgress.session_id.label("session_id"), + ReadingProgress.page.label("page"), + ReadingProgress.created_at.label("created_at"), + rn.label("rn"), + ) + .where( + col(ReadingProgress.book_id).in_(ids), + ReadingProgress.user_id == current_user.id, + ) + .subquery() + ) + latest_per_session = list( session.exec( - select(ReadingProgress) - .where( - col(ReadingProgress.book_id).in_(ids), - ReadingProgress.user_id == current_user.id, + select(subq.c.book_id, subq.c.session_id, subq.c.page, subq.c.created_at).where( + subq.c.rn == 1 ) - .order_by(col(ReadingProgress.created_at)) ).all() ) - latest_by_book_session: dict[tuple[int, int | None], ReadingProgress] = {} - for entry in entries: - key = (entry.book_id, entry.session_id) - prev = latest_by_book_session.get(key) - if prev is None or entry.created_at >= prev.created_at: - latest_by_book_session[key] = entry - - results: list[ReadingProgressLatest] = [] - for book_id in ids: + # For each book, prefer the current session's latest entry; otherwise the + # latest entry across all sessions of the book. + best_by_book: dict[int, tuple[datetime, int]] = {} + for row in latest_per_session: + book_id = row[0] + sid = row[1] + page = row[2] + created_at = row[3] current_sid = current_sessions.get(book_id) if current_sid is not None: - latest = latest_by_book_session.get((book_id, current_sid)) + if sid == current_sid: + best_by_book[book_id] = (created_at, page) else: - candidates = [e for e in entries if e.book_id == book_id] - latest = max(candidates, key=lambda e: e.created_at) if candidates else None - if latest is not None: - results.append(ReadingProgressLatest(book_id=book_id, current_page=latest.page)) - return results + prev = best_by_book.get(book_id) + if prev is None or created_at > prev[0]: + best_by_book[book_id] = (created_at, page) + + return [ + ReadingProgressLatest(book_id=book_id, current_page=page) + for book_id, (_created_at, page) in best_by_book.items() + ] diff --git a/backend/app/routers/sessions.py b/backend/app/routers/sessions.py index bd9e7543..63274f3a 100644 --- a/backend/app/routers/sessions.py +++ b/backend/app/routers/sessions.py @@ -4,7 +4,7 @@ from datetime import datetime from typing import List, Optional -from fastapi import APIRouter, Depends, HTTPException +from fastapi import APIRouter, Depends, HTTPException, Query from sqlmodel import Session, col, select from app.auth import require_user @@ -72,11 +72,18 @@ def _to_session_reads( @router.get("/sessions", response_model=List[TimelineSessionEntry]) def list_completed_sessions( + limit: int = Query(default=40, ge=1, le=200), + offset: int = Query(default=0, ge=0), current_user: User = Depends(require_user), session: Session = Depends(get_session), ) -> List[TimelineSessionEntry]: - """Return every completed read (session) across the user's books, newest first.""" - rows = session.exec( + """Return completed reads (sessions) across the user's books, newest first. + + Paginated in chunks of ``limit`` (default 40) to keep the timeline page + light for large libraries; callers detect the end of the data by checking + whether the returned list has fewer than ``limit`` entries. + """ + page_rows = session.exec( select(ReadingSession, Book) .join(Book, col(Book.id) == col(ReadingSession.book_id)) .where( @@ -85,8 +92,54 @@ def list_completed_sessions( col(ReadingSession.date_finished).is_not(None), ) .order_by(col(ReadingSession.date_finished).desc()) + .offset(offset) + .limit(limit) ).all() + # Ordinals need the full completion set per book; fetch those (bounded by the + # number of distinct books on this page) without loading the whole library. + page_book_ids = list({rs.book_id for rs, _ in page_rows}) + all_rows = ( + session.exec( + select(ReadingSession).where( + ReadingSession.user_id == current_user.id, + ReadingSession.status == ReadingStatus.read, + col(ReadingSession.date_finished).is_not(None), + col(ReadingSession.book_id).in_(page_book_ids), + ) + ).all() + if page_book_ids + else [] + ) + per_book: dict[int, list[ReadingSession]] = {} + for rs in all_rows: + per_book.setdefault(rs.book_id, []).append(rs) + authors_map = load_authors_batch(session, page_book_ids) + read_numbers_by_book = { + book_id: assign_read_numbers(s_list) for book_id, s_list in per_book.items() + } + + result: list[TimelineSessionEntry] = [] + for rs, book in page_rows: + read_number = read_numbers_by_book.get(rs.book_id, {}).get(rs.id) + authors = authors_map.get(book.id, []) + result.append( + TimelineSessionEntry( + session_id=rs.id or 0, + book_id=book.id or 0, + title=book.title, + author=join_authors(authors), + authors=authors, + cover_url=book.cover_url, + rating=book.rating, + page_count=book.page_count, + date_finished=rs.date_finished, + read_number=read_number, + is_reread=read_number is not None and read_number >= 2, + ) + ) + return result + per_book: dict[int, list[ReadingSession]] = {} for rs, _book in rows: per_book.setdefault(rs.book_id, []).append(rs) diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index a33113b1..d199fc11 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -523,8 +523,9 @@ export const api = { body: JSON.stringify({ date_started: dateStarted ?? null }) }); }, - completed(): Promise { - return request('/books/sessions'); + completed(limit: number = 40, offset: number = 0): Promise { + const qs = new URLSearchParams({ limit: String(limit), offset: String(offset) }); + return request(`/books/sessions?${qs.toString()}`); } }, diff --git a/frontend/src/routes/timeline/+page.svelte b/frontend/src/routes/timeline/+page.svelte index 6ea0d5db..7c25b47f 100644 --- a/frontend/src/routes/timeline/+page.svelte +++ b/frontend/src/routes/timeline/+page.svelte @@ -11,8 +11,13 @@ import BookDetailDialog from '#lib/components/BookDetailDialog.svelte'; import BookDrawer from '#lib/components/BookDrawer.svelte'; + const PAGE_SIZE = 40; + let loading = $state(true); + let loadingMore = $state(false); + let hasMore = $state(true); let entries = $state([]); + let nextOffset = $state(0); let selectedBook = $state(null); let detailOpen = $state(false); let drawerOpen = $state(false); @@ -27,7 +32,10 @@ async function loadTimeline() { loading = true; try { - entries = await api.books.sessions.completed(); + const response = await api.books.sessions.completed(PAGE_SIZE, 0); + entries = response; + nextOffset = response.length; + hasMore = response.length === PAGE_SIZE; } catch (e: unknown) { const msg = e instanceof Error ? e.message : $_('common.actionFailed', { values: { action: 'load' } }); if (shouldShowActionToast(msg)) { @@ -39,6 +47,24 @@ } } + async function loadMoreTimeline() { + if (loadingMore || loading || !hasMore) return; + loadingMore = true; + try { + const response = await api.books.sessions.completed(PAGE_SIZE, nextOffset); + entries = [...entries, ...response]; + nextOffset += response.length; + hasMore = response.length === PAGE_SIZE; + } catch (e: unknown) { + const msg = e instanceof Error ? e.message : $_('common.actionFailed', { values: { action: 'load' } }); + if (shouldShowActionToast(msg)) { + toasts.add(msg, 'error'); + } + } finally { + loadingMore = false; + } + } + let grouped = $derived.by(() => { const map = new Map(); for (const entry of entries) { @@ -198,6 +224,23 @@ {/each} + {#if hasMore} +
+ +
+ {/if} + {#if loadingMore} +
+ +
+ {/if} {/if}
From 7e4b924eedb935276fc6463d4aec9777e38834df Mon Sep 17 00:00:00 2001 From: codebude Date: Thu, 22 Oct 2026 11:09:43 +0200 Subject: [PATCH 10/25] Fix restore of older backups (create_all vs migration collision) _recreate_engine now creates tables directly only for fresh databases; for older backups (alembic_version row present) it skips create_all and relies on pending migrations to add new tables/columns. This prevents 'table reading_session already exists' when restoring a pre-feature backup. --- backend/app/services/backup_restore.py | 29 ++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/backend/app/services/backup_restore.py b/backend/app/services/backup_restore.py index 2b4f7bee..7eb6ef4f 100644 --- a/backend/app/services/backup_restore.py +++ b/backend/app/services/backup_restore.py @@ -97,16 +97,41 @@ def _recreate_engine() -> None: Called after restoring the database on disk so the app picks up the new data. Also runs pending alembic migrations for backward compatibility with backups created by older releases. + + Tables are created directly (via ``SQLModel.metadata.create_all``) only for + *fresh* databases (no ``alembic_version`` row). For an older backup, the + tables already exist in the restored file and creating them again would + collide with the pending migrations (which ``CREATE``/``ALTER`` the same + objects), so we rely on alembic alone in that case. """ + import sqlite3 + from app.database import create_engine as _create_engine from sqlmodel import SQLModel import app.database as db_mod + + db_path = _extract_db_path(settings.database_url) + conn = sqlite3.connect(db_path) + try: + cursor = conn.execute( + "SELECT name FROM sqlite_master WHERE type='table' AND name='alembic_version'" + ) + table_exists = cursor.fetchone() is not None + fresh = True + if table_exists: + row_count = conn.execute("SELECT COUNT(*) FROM alembic_version").fetchone()[0] + if row_count > 0: + fresh = False + finally: + conn.close() + new_engine = _create_engine( settings.database_url, connect_args={"check_same_thread": False}, ) - SQLModel.metadata.create_all(new_engine) - _stamp_alembic_head_if_fresh() + if fresh: + SQLModel.metadata.create_all(new_engine) + _stamp_alembic_head_if_fresh() _run_alembic_migrations() db_mod.engine = new_engine From c7c1703d9f172e3b10c55b07d54092e94e1b1dbb Mon Sep 17 00:00:00 2001 From: codebude Date: Thu, 22 Oct 2026 11:09:58 +0200 Subject: [PATCH 11/25] Docs: backward compatibility for older exports/backups --- docs/guide/using-librislog/import-export.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/guide/using-librislog/import-export.md b/docs/guide/using-librislog/import-export.md index 4e72bac6..0e238403 100644 --- a/docs/guide/using-librislog/import-export.md +++ b/docs/guide/using-librislog/import-export.md @@ -66,6 +66,8 @@ Choose which data to include: Reading sessions (re-read history) are embedded in the JSON book export, so a JSON export round-trips losslessly: re-importing `books.json` recreates each book's reading sessions. The CSV format cannot represent nested sessions, so re-read history does **not** round-trip through a CSV export/import; a CSV export only carries the flat `sessions` dataset as a book-ID-referenced audit view. For full fidelity of re-read history, use the JSON export or [backup and restore](./administration), which copies the whole database. +**Older exports and backups:** A JSON export or a full backup made with an older LibrisLog version (before reading sessions) imports fine. Each imported book gets a single reading session derived from its start/finish dates, and the existing statistics, timeline and progress behave exactly as before. Full backups created by older releases are migrated automatically when restored (the schema is upgraded on restore). + ### Export Process 1. Go to the Data page From 965509b0ea83085c2485acc1305e8096ecb558b6 Mon Sep 17 00:00:00 2001 From: codebude Date: Thu, 22 Oct 2026 11:25:02 +0200 Subject: [PATCH 12/25] Clear progress log when last reading session is deleted --- backend/app/routers/sessions.py | 6 ++++++ backend/tests/test_reading_sessions.py | 15 +++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/backend/app/routers/sessions.py b/backend/app/routers/sessions.py index 63274f3a..c0b949ed 100644 --- a/backend/app/routers/sessions.py +++ b/backend/app/routers/sessions.py @@ -321,6 +321,12 @@ def delete_session( book.reading_status = ReadingStatus.want_to_read book.date_started = None book.date_finished = None + # No reading session remains, so the progress log is reset as well: any + # orphaned (session-less) entries would otherwise keep showing up. + for entry in session.exec( + select(ReadingProgress).where(ReadingProgress.book_id == book_id) + ).all(): + session.delete(entry) else: latest = remaining[0] # Only an open session can become the new current session; a completed diff --git a/backend/tests/test_reading_sessions.py b/backend/tests/test_reading_sessions.py index 76debb6d..aa31c355 100644 --- a/backend/tests/test_reading_sessions.py +++ b/backend/tests/test_reading_sessions.py @@ -122,6 +122,21 @@ def test_progress_scoped_to_session(client: TestClient) -> None: assert latest == [] +def test_deleting_last_session_clears_progress_log(client: TestClient) -> None: + """When no session remains, the progress log must be reset too.""" + book = _create_book(client, reading_status="read", date_started="2024-01-01T00:00:00Z", date_finished="2024-01-10T00:00:00Z", page_count=200) + client.post(f"/api/books/{book['id']}/progress", json={"page": 200}) + assert len(client.get(f"/api/books/{book['id']}/progress").json()) == 1 + + sessions = client.get(f"/api/books/{book['id']}/sessions").json() + client.delete(f"/api/books/{book['id']}/sessions/{sessions[0]['id']}") + + # Book reset to want_to_read, progress log empty. + book = client.get(f"/api/books/{book['id']}").json() + assert book["reading_status"] == "want_to_read" + assert client.get(f"/api/books/{book['id']}/progress").json() == [] + + def test_unrelated_patch_does_not_change_sessions(client: TestClient) -> None: """Editing an unrelated field must never rewrite a reading session.""" book = _create_book(client, reading_status="read", date_started="2024-01-01T00:00:00Z", date_finished="2024-01-10T00:00:00Z") From a3d4295001ff4340462a31bbaef38aab8ad7efad Mon Sep 17 00:00:00 2001 From: codebude Date: Thu, 22 Oct 2026 11:49:16 +0200 Subject: [PATCH 13/25] Docs: add regular unreleased v1.11.0 release entry --- docs/releases.md | 38 ++++++++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/docs/releases.md b/docs/releases.md index c413d96f..7d1e4992 100644 --- a/docs/releases.md +++ b/docs/releases.md @@ -6,12 +6,6 @@ All LibrisLog releases, newest first — what's new, what was fixed, and anythin You can also browse the [GitHub Releases](https://github.com/codebude/librislog/releases) page and the [full changelog](https://github.com/codebude/librislog/commits/main). -## Unreleased - -::: tip ⭐ Reading Sessions & Re-reads -Books can now be read more than once. Each reading is tracked as its own session with independent start and finish dates, and a "Read again" action starts a fresh reading without overwriting earlier dates or progress. A reading history in the book detail dialog lets you add past reads and edit or delete individual sessions, progress restarts per session, and re-read books show a "read ×N" badge. Statistics now distinguish unique books from total completions (with a new "Total reads" card), and the timeline lists one row per completed read, marking re-reads. Data migrates automatically on upgrade. -::: - ## Latest Release ::: tip ⭐ v1.10.0 — More Book Sources & Upgrades @@ -22,6 +16,7 @@ LibrisLog v1.10.0 adds four more metadata sources to the import search (BnF, Inv | Version | Date | Type | |---|---|---| +| [v1.11.0](#v1-11-0-reading-sessions-re-reads) | Unreleased | Feature release | | [v1.10.0](#v1-10-0-more-book-sources-upgrades) | 2026-10-18 | Feature release | | [v1.9.0](#v1-9-0-smarter-import-search-input-ux) | 2026-09-15 | Feature release | | [v1.8.0](#v1-8-0-camera-zoom-control-optional-telemetry) | 2026-09-02 | Feature release | @@ -42,6 +37,37 @@ LibrisLog v1.10.0 adds four more metadata sources to the import search (BnF, Inv --- +## v1.11.0: Reading Sessions & Re-reads + + + +**Summary:** Books can now be read more than once. Each reading is tracked as its own session with independent start and finish dates, and a "Read again" action starts a fresh reading without overwriting earlier dates or progress. A reading history in the book detail dialog lets you add past reads and edit or delete individual sessions, progress restarts per session, and re-read books show a "read ×N" badge. Statistics now distinguish unique books from total completions (with a new "Total reads" card), and the timeline lists one row per completed read, marking re-reads. Data migrates automatically on upgrade. + +**Features** +- 📖 **Multiple reading sessions per book.** Each book keeps a list of reading periods (start and finish dates) so re-reads never overwrite earlier dates. A new `reading_session` table stores them; `Book.date_started` / `Book.date_finished` remain a mirror of the current session, so every existing consumer (sorting, timeline, statistics, import/export) keeps working unchanged +- 🔁 **"Read again" action.** A ghost button in the book detail dialog starts a new reading session for a finished book. Earlier sessions and their progress stay untouched, and the progress bar resets to 0 % +- 📜 **Reading history.** A collapsible block in the book detail dialog lists every session (#, started, finished, status, pages) with per-row edit and delete, plus "Add a past reading" for back-filling handwritten history +- 🏷️ **"read ×N" badge.** Cards, list rows and the book detail dialog show a small badge once a book has been read at least twice +- 📊 **Statistics distinguish unique books from total completions.** A new "Total reads" card shows the number of completed reads (with a "N re-read books" sub-line). `books_finished_per_month/year`, the `books_per_month/year` goals, and `pages_read` now count every completion; "Pages read" in the bucket still counts each book once, so no single-session user sees any change +- 📅 **Timeline lists one row per completed read**, with a "Reread" badge from the second read onward +- 🔄 **Migration is automatic.** Existing books get one session from their dates; reading progress is linked to it. A book with a single session behaves bit-identically to before + +**Improvements** +- 🛡️ **Session-aware progress and statistics.** Pages-per-month and pages-per-day no longer discard a re-read's pages as negative deltas (a long-standing bug where a restart at page 0 made every later delta negative) +- 📦 **Export/import keeps re-read history.** The JSON book export embeds each book's sessions; re-importing `books.json` recreates them. Reading sessions are also exported as a flat `sessions` dataset for auditing. CSV cannot represent nested sessions, so re-read history round-trips only through the JSON export or full backup/restore +- 🔁 **Full backup/restore upgraded.** Restoring a backup from an older LibrisLog release migrates the schema automatically (the new tables and columns are added on restore) +- 🎯 **Date-conflict dialog gains a "Start a new read" action** for finished books moved back to "Currently reading", keeping the earlier reads in history + +**Bug fixes** +- 📉 **Re-reading no longer corrupts page statistics.** Progress deltas are grouped per (book, session) instead of per book, so a restart at page 0 no longer produces negative deltas that get skipped +- 🗑️ **Deleting the last session resets the book** to "want to read" and clears its progress log, so no orphaned progress entries remain + +**Breaking changes:** None. Existing installs keep working untouched: every book with a single session behaves and scores identically to before, and data migrates automatically on upgrade. Downgrading to a previous release drops re-read history (the `reading_session` table is removed). + +[Compare with v1.10.0](https://github.com/codebude/librislog/compare/v1.10.0...HEAD) + +--- + ## v1.10.0: More Book Sources & Upgrades From b39d40c08e3ca62846be6c0b8097d9c01fcc039a Mon Sep 17 00:00:00 2001 From: codebude Date: Thu, 22 Oct 2026 12:01:30 +0200 Subject: [PATCH 14/25] Docs: remove unreleased bug-fix entries; add regression test for progress deletion on session delete --- backend/tests/test_reading_sessions.py | 23 +++++++++++++++++++++++ docs/releases.md | 4 ---- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/backend/tests/test_reading_sessions.py b/backend/tests/test_reading_sessions.py index aa31c355..138383de 100644 --- a/backend/tests/test_reading_sessions.py +++ b/backend/tests/test_reading_sessions.py @@ -137,6 +137,29 @@ def test_deleting_last_session_clears_progress_log(client: TestClient) -> None: assert client.get(f"/api/books/{book['id']}/progress").json() == [] +def test_deleting_a_session_removes_its_progress_entries(client: TestClient) -> None: + """Progress entries attached to a deleted session must be removed with it.""" + book = _create_book(client, reading_status="read", date_started="2024-01-01T00:00:00Z", date_finished="2024-01-10T00:00:00Z", page_count=200) + client.post(f"/api/books/{book['id']}/progress", json={"page": 100}) + client.post(f"/api/books/{book['id']}/progress", json={"page": 200}) + assert len(client.get(f"/api/books/{book['id']}/progress").json()) == 2 + + # Add a re-read so there is a second session to keep. + _read_again(client, book["id"]) + client.post(f"/api/books/{book['id']}/transition-status", json={"new_status": "read"}) + + # The first session (the 2024 read) owns the progress entries. + sessions = client.get(f"/api/books/{book['id']}/sessions").json() + first_session = next(s for s in sessions if s["date_finished"] == "2024-01-10T00:00:00Z") + client.delete(f"/api/books/{book['id']}/sessions/{first_session['id']}") + + # The deleted session's progress must be gone; the re-read session has none. + remaining_progress = client.get(f"/api/books/{book['id']}/progress").json() + assert remaining_progress == [] + book = client.get(f"/api/books/{book['id']}").json() + assert book["reading_status"] == "read" + + def test_unrelated_patch_does_not_change_sessions(client: TestClient) -> None: """Editing an unrelated field must never rewrite a reading session.""" book = _create_book(client, reading_status="read", date_started="2024-01-01T00:00:00Z", date_finished="2024-01-10T00:00:00Z") diff --git a/docs/releases.md b/docs/releases.md index 7d1e4992..ff27edc5 100644 --- a/docs/releases.md +++ b/docs/releases.md @@ -58,10 +58,6 @@ LibrisLog v1.10.0 adds four more metadata sources to the import search (BnF, Inv - 🔁 **Full backup/restore upgraded.** Restoring a backup from an older LibrisLog release migrates the schema automatically (the new tables and columns are added on restore) - 🎯 **Date-conflict dialog gains a "Start a new read" action** for finished books moved back to "Currently reading", keeping the earlier reads in history -**Bug fixes** -- 📉 **Re-reading no longer corrupts page statistics.** Progress deltas are grouped per (book, session) instead of per book, so a restart at page 0 no longer produces negative deltas that get skipped -- 🗑️ **Deleting the last session resets the book** to "want to read" and clears its progress log, so no orphaned progress entries remain - **Breaking changes:** None. Existing installs keep working untouched: every book with a single session behaves and scores identically to before, and data migrates automatically on upgrade. Downgrading to a previous release drops re-read history (the `reading_session` table is removed). [Compare with v1.10.0](https://github.com/codebude/librislog/compare/v1.10.0...HEAD) From cd9abdfde4c268f0738004201a24457ae2f339a2 Mon Sep 17 00:00:00 2001 From: codebude Date: Thu, 22 Oct 2026 12:14:23 +0200 Subject: [PATCH 15/25] Harden route ordering: move timeline endpoint into books router (no cross-router dependency) --- backend/app/main.py | 5 +- backend/app/routers/books.py | 74 ++++++++++++++++++++++- backend/app/routers/sessions.py | 101 -------------------------------- backend/app/schemas.py | 1 - frontend/src/lib/types.ts | 1 - 5 files changed, 74 insertions(+), 108 deletions(-) diff --git a/backend/app/main.py b/backend/app/main.py index 4eeb8624..99982c25 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -219,11 +219,8 @@ async def proxy_headers_middleware(request: Request, call_next) -> Response: return await call_next(request) -# The sessions router must be registered before the books router: both define -# routes under /api/books, and `GET /api/books/sessions` (the timeline) must win -# over the books router's `GET /api/books/{book_id}`. Do not reorder. -app.include_router(sessions.router) app.include_router(books.router) +app.include_router(sessions.router) app.include_router(import_.router) app.include_router(covers.router) app.include_router(cover_candidates.router) diff --git a/backend/app/routers/books.py b/backend/app/routers/books.py index a92a03b1..b951b8b9 100644 --- a/backend/app/routers/books.py +++ b/backend/app/routers/books.py @@ -25,6 +25,7 @@ StatusTransitionResponse, SuggestionList, TagCloudEntry, + TimelineSessionEntry, ) from app.services.authors import ( cleanup_orphan_authors, @@ -39,7 +40,7 @@ ) from app.services.cover_import import import_cover_from_url, is_external_cover_url from app.services.quote_cache import get_or_fetch_dashboard_quote -from app.services.reading_sessions import create_initial_session, reconcile_book_sessions, session_counts +from app.services.reading_sessions import assign_read_numbers, create_initial_session, reconcile_book_sessions, session_counts from app.services.search import _escape_like, apply_search_filter from app.services.tags import build_book_read, cleanup_orphan_tags, load_tags_batch, sync_book_tags from app.time_utils import utcnow @@ -500,6 +501,77 @@ async def create_book( return build_book_read(session, book) +@router.get("/sessions", response_model=List[TimelineSessionEntry]) +def list_completed_sessions( + limit: int = Query(default=40, ge=1, le=200), + offset: int = Query(default=0, ge=0), + current_user: User = Depends(require_user), + session: Session = Depends(get_session), +) -> List[TimelineSessionEntry]: + """Return completed reads (sessions) across the user's books, newest first. + + Mounted here (in the books router, before ``/{book_id}``) so that the + account-level path ``/api/books/sessions`` always resolves to this handler + instead of being captured by the ``/{book_id}`` route, which would try to + parse ``"sessions"`` as an int and return 422. Do not move this route + below ``get_book``. + """ + page_rows = session.exec( + select(ReadingSession, Book) + .join(Book, col(Book.id) == col(ReadingSession.book_id)) + .where( + ReadingSession.user_id == current_user.id, + ReadingSession.status == ReadingStatus.read, + col(ReadingSession.date_finished).is_not(None), + ) + .order_by(col(ReadingSession.date_finished).desc()) + .offset(offset) + .limit(limit) + ).all() + + page_book_ids = list({rs.book_id for rs, _ in page_rows}) + all_rows = ( + session.exec( + select(ReadingSession).where( + ReadingSession.user_id == current_user.id, + ReadingSession.status == ReadingStatus.read, + col(ReadingSession.date_finished).is_not(None), + col(ReadingSession.book_id).in_(page_book_ids), + ) + ).all() + if page_book_ids + else [] + ) + per_book: dict[int, list[ReadingSession]] = {} + for rs in all_rows: + per_book.setdefault(rs.book_id, []).append(rs) + authors_map = load_authors_batch(session, page_book_ids) + read_numbers_by_book = { + book_id: assign_read_numbers(s_list) for book_id, s_list in per_book.items() + } + + result: list[TimelineSessionEntry] = [] + for rs, book in page_rows: + read_number = read_numbers_by_book.get(rs.book_id, {}).get(rs.id) + authors = authors_map.get(book.id, []) + result.append( + TimelineSessionEntry( + session_id=rs.id or 0, + book_id=book.id or 0, + title=book.title, + author=join_authors(authors), + authors=authors, + cover_url=book.cover_url, + rating=book.rating, + page_count=book.page_count, + date_finished=rs.date_finished, + read_number=read_number, + is_reread=read_number is not None and read_number >= 2, + ) + ) + return result + + @router.get("/{book_id}", response_model=BookRead) def get_book( book_id: int, diff --git a/backend/app/routers/sessions.py b/backend/app/routers/sessions.py index c0b949ed..724cf896 100644 --- a/backend/app/routers/sessions.py +++ b/backend/app/routers/sessions.py @@ -70,107 +70,6 @@ def _to_session_reads( ] -@router.get("/sessions", response_model=List[TimelineSessionEntry]) -def list_completed_sessions( - limit: int = Query(default=40, ge=1, le=200), - offset: int = Query(default=0, ge=0), - current_user: User = Depends(require_user), - session: Session = Depends(get_session), -) -> List[TimelineSessionEntry]: - """Return completed reads (sessions) across the user's books, newest first. - - Paginated in chunks of ``limit`` (default 40) to keep the timeline page - light for large libraries; callers detect the end of the data by checking - whether the returned list has fewer than ``limit`` entries. - """ - page_rows = session.exec( - select(ReadingSession, Book) - .join(Book, col(Book.id) == col(ReadingSession.book_id)) - .where( - ReadingSession.user_id == current_user.id, - ReadingSession.status == ReadingStatus.read, - col(ReadingSession.date_finished).is_not(None), - ) - .order_by(col(ReadingSession.date_finished).desc()) - .offset(offset) - .limit(limit) - ).all() - - # Ordinals need the full completion set per book; fetch those (bounded by the - # number of distinct books on this page) without loading the whole library. - page_book_ids = list({rs.book_id for rs, _ in page_rows}) - all_rows = ( - session.exec( - select(ReadingSession).where( - ReadingSession.user_id == current_user.id, - ReadingSession.status == ReadingStatus.read, - col(ReadingSession.date_finished).is_not(None), - col(ReadingSession.book_id).in_(page_book_ids), - ) - ).all() - if page_book_ids - else [] - ) - per_book: dict[int, list[ReadingSession]] = {} - for rs in all_rows: - per_book.setdefault(rs.book_id, []).append(rs) - authors_map = load_authors_batch(session, page_book_ids) - read_numbers_by_book = { - book_id: assign_read_numbers(s_list) for book_id, s_list in per_book.items() - } - - result: list[TimelineSessionEntry] = [] - for rs, book in page_rows: - read_number = read_numbers_by_book.get(rs.book_id, {}).get(rs.id) - authors = authors_map.get(book.id, []) - result.append( - TimelineSessionEntry( - session_id=rs.id or 0, - book_id=book.id or 0, - title=book.title, - author=join_authors(authors), - authors=authors, - cover_url=book.cover_url, - rating=book.rating, - page_count=book.page_count, - date_finished=rs.date_finished, - read_number=read_number, - is_reread=read_number is not None and read_number >= 2, - ) - ) - return result - - per_book: dict[int, list[ReadingSession]] = {} - for rs, _book in rows: - per_book.setdefault(rs.book_id, []).append(rs) - authors_map = load_authors_batch(session, list(per_book)) - # Compute the ordinal once per book, not once per row. - read_numbers_by_book = { - book_id: assign_read_numbers(s_list) for book_id, s_list in per_book.items() - } - - result: list[TimelineSessionEntry] = [] - for rs, book in rows: - read_number = read_numbers_by_book[rs.book_id].get(rs.id) - authors = authors_map.get(book.id, []) - result.append( - TimelineSessionEntry( - session_id=rs.id or 0, - book_id=book.id or 0, - title=book.title, - author=join_authors(authors), - authors=authors, - cover_url=book.cover_url, - rating=book.rating, - page_count=book.page_count, - date_finished=rs.date_finished, - read_number=read_number, - is_reread=read_number is not None and read_number >= 2, - ) - ) - return result - - @router.get("/{book_id}/sessions", response_model=List[ReadingSessionRead]) def list_sessions( book_id: int, diff --git a/backend/app/schemas.py b/backend/app/schemas.py index 2f75a45e..43f3b4eb 100644 --- a/backend/app/schemas.py +++ b/backend/app/schemas.py @@ -594,7 +594,6 @@ class StatusTransitionRequest(SQLModel): skip_auto_date_started: bool = False clear_date_started: bool = False clear_date_finished: bool = False - start_new_session: bool = False class DateConflict(SQLModel): diff --git a/frontend/src/lib/types.ts b/frontend/src/lib/types.ts index cbba9652..5da7e5d9 100644 --- a/frontend/src/lib/types.ts +++ b/frontend/src/lib/types.ts @@ -97,7 +97,6 @@ export interface StatusTransitionRequest { skip_auto_date_started?: boolean; clear_date_started?: boolean; clear_date_finished?: boolean; - start_new_session?: boolean; } export interface DateConflict { From bda014a581fc6f313eb67b2f01fabc66fb01829b Mon Sep 17 00:00:00 2001 From: codebude Date: Thu, 22 Oct 2026 15:45:10 +0200 Subject: [PATCH 16/25] Fix naive/aware datetime comparison and current-session marking in session round-trip - Use aware datetime.min fallback when sorting sessions (a date-less session next to a dated one previously raised TypeError on export/import) - Mark the most recent open session as current, not the last session overall --- backend/app/services/data_export.py | 5 +- backend/app/services/data_import.py | 16 ++++-- backend/tests/test_data_import.py | 83 +++++++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 5 deletions(-) diff --git a/backend/app/services/data_export.py b/backend/app/services/data_export.py index 838b66cf..e9f6bcf9 100644 --- a/backend/app/services/data_export.py +++ b/backend/app/services/data_export.py @@ -211,7 +211,10 @@ def build_export_zip( sessions_by_book.setdefault(entry.book_id, []).append(entry) for book_sessions in sessions_by_book.values(): book_sessions.sort( - key=lambda s: (s.date_started or s.date_finished or datetime.min, s.id or 0) + key=lambda s: ( + s.date_started or s.date_finished or datetime.min.replace(tzinfo=timezone.utc), + s.id or 0, + ) ) books_rows = [ diff --git a/backend/app/services/data_import.py b/backend/app/services/data_import.py index 379a687d..41c8f89a 100644 --- a/backend/app/services/data_import.py +++ b/backend/app/services/data_import.py @@ -546,14 +546,22 @@ def _create_sessions_from_data( return create_initial_session(session, book) # Chronological order (oldest first); a date-less session sorts first. - parsed.sort(key=lambda t: (t[2] or t[1] or datetime.min, t[1] or datetime.min)) + # Use an aware minimum so the comparison never mixes aware and naive datetimes. + _min = datetime.min.replace(tzinfo=timezone.utc) + parsed.sort(key=lambda t: (t[2] or t[1] or _min, t[1] or _min)) + + # The current session is the most recent open (currently_reading) session, + # not simply the last session overall (a date-less open session sorts first). + last_open_idx = max( + (i for i, (st, _ds, _df) in enumerate(parsed) if st == ReadingStatus.currently_reading), + default=-1, + ) latest_completed: ReadingSession | None = None for idx, (status, ds, df) in enumerate(parsed): is_current = ( - status == ReadingStatus.currently_reading - and idx == len(parsed) - 1 - and book.reading_status == ReadingStatus.currently_reading + book.reading_status == ReadingStatus.currently_reading + and idx == last_open_idx ) rs = ReadingSession( book_id=book.id, diff --git a/backend/tests/test_data_import.py b/backend/tests/test_data_import.py index 8ea7e75d..7bfce1b7 100644 --- a/backend/tests/test_data_import.py +++ b/backend/tests/test_data_import.py @@ -1089,6 +1089,89 @@ async def test_execute_import_round_trips_reading_sessions( ] +@pytest.mark.anyio +async def test_execute_import_round_trips_open_session_without_dates( + session: Session, tmp_path: Path, monkeypatch: MonkeyPatch +) -> None: + """A currently_reading session without dates must round-trip without crashing + and must remain the current session.""" + monkeypatch.setattr(settings, "import_temp_dir", str(tmp_path)) + user = _create_test_user(session) + user_id = _require(user.id) + + book = Book( + title="Open Read", + page_count=200, + reading_status=ReadingStatus.currently_reading, + acquisition_status=AcquisitionStatus.owned, + date_started=None, + date_finished=None, + user_id=user.id, + ) + session.add(book) + session.flush() + book_id = _require(book.id) + # A completed read plus a date-less open (current) session. + session.add( + ReadingSession( + book_id=book_id, + user_id=user_id, + status=ReadingStatus.read, + date_started=datetime(2024, 1, 1, tzinfo=timezone.utc), + date_finished=datetime(2024, 1, 10, tzinfo=timezone.utc), + is_current=False, + ) + ) + session.add( + ReadingSession( + book_id=book_id, + user_id=user_id, + status=ReadingStatus.currently_reading, + date_started=None, + date_finished=None, + is_current=True, + ) + ) + session.commit() + + 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) == 1 + + # Remove the source book and its sessions. + for s in session.exec(select(ReadingSession).where(ReadingSession.book_id == book_id)).all(): + session.delete(s) + session.delete(book) + session.commit() + + file_id = "roundtrip_open" + 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", "page_count", "reading_status", "acquisition_status", "sessions") + } + events = [] + async for event in di.execute_import( + file_id, user, mapping, session, "continue_on_error", require_acquisition_status=True + ): + events.append(event) + assert [e for e in events if e["event"] == "complete"][0]["imported"] == 1 + + imported = session.exec(select(Book).where(Book.user_id == user.id)).one() + imported_sessions = list( + session.exec(select(ReadingSession).where(ReadingSession.book_id == imported.id)).all() + ) + assert len(imported_sessions) == 2 + current = next(s for s in imported_sessions if s.is_current) + assert current.status == ReadingStatus.currently_reading + + @pytest.mark.anyio async def test_execute_import_preserves_date_added( session: Session, tmp_path: Path, monkeypatch: MonkeyPatch From 720b73ff8a80a5aa7b00e03d1f04b0c6f732b15c Mon Sep 17 00:00:00 2001 From: codebude Date: Thu, 22 Oct 2026 15:45:30 +0200 Subject: [PATCH 17/25] Docs: note progress history is reference-only on export (full fidelity via backup) --- docs/guide/using-librislog/import-export.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/guide/using-librislog/import-export.md b/docs/guide/using-librislog/import-export.md index 0e238403..92230ca6 100644 --- a/docs/guide/using-librislog/import-export.md +++ b/docs/guide/using-librislog/import-export.md @@ -66,6 +66,8 @@ Choose which data to include: Reading sessions (re-read history) are embedded in the JSON book export, so a JSON export round-trips losslessly: re-importing `books.json` recreates each book's reading sessions. The CSV format cannot represent nested sessions, so re-read history does **not** round-trip through a CSV export/import; a CSV export only carries the flat `sessions` dataset as a book-ID-referenced audit view. For full fidelity of re-read history, use the JSON export or [backup and restore](./administration), which copies the whole database. +The reading progress entries are exported as a flat dataset (with the `session_id` of each entry) for auditing, but the import wizard does **not** recreate the full page-by-page progress history: it only creates a single "read to the end" entry when the "create progress for read" option is enabled. Full progress history (and its per-session linkage) is preserved only through [backup and restore](./administration). + **Older exports and backups:** A JSON export or a full backup made with an older LibrisLog version (before reading sessions) imports fine. Each imported book gets a single reading session derived from its start/finish dates, and the existing statistics, timeline and progress behave exactly as before. Full backups created by older releases are migrated automatically when restored (the schema is upgraded on restore). ### Export Process From 566c88c0c85916a5857ffa1992d8255109851035 Mon Sep 17 00:00:00 2001 From: codebude Date: Thu, 22 Oct 2026 20:36:42 +0200 Subject: [PATCH 18/25] Fix timeline load-more off-by-one (return total count) --- backend/app/routers/books.py | 15 ++++++++++++--- backend/app/schemas.py | 6 ++++++ backend/tests/test_reading_sessions.py | 4 +++- frontend/src/lib/api.ts | 7 ++++--- frontend/src/lib/types.ts | 5 +++++ frontend/src/routes/timeline/+page.svelte | 12 ++++++------ 6 files changed, 36 insertions(+), 13 deletions(-) diff --git a/backend/app/routers/books.py b/backend/app/routers/books.py index b951b8b9..52e74692 100644 --- a/backend/app/routers/books.py +++ b/backend/app/routers/books.py @@ -26,6 +26,7 @@ SuggestionList, TagCloudEntry, TimelineSessionEntry, + TimelineSessionListResponse, ) from app.services.authors import ( cleanup_orphan_authors, @@ -501,13 +502,13 @@ async def create_book( return build_book_read(session, book) -@router.get("/sessions", response_model=List[TimelineSessionEntry]) +@router.get("/sessions", response_model=TimelineSessionListResponse) def list_completed_sessions( limit: int = Query(default=40, ge=1, le=200), offset: int = Query(default=0, ge=0), current_user: User = Depends(require_user), session: Session = Depends(get_session), -) -> List[TimelineSessionEntry]: +) -> TimelineSessionListResponse: """Return completed reads (sessions) across the user's books, newest first. Mounted here (in the books router, before ``/{book_id}``) so that the @@ -516,6 +517,14 @@ def list_completed_sessions( parse ``"sessions"`` as an int and return 422. Do not move this route below ``get_book``. """ + total = session.exec( + select(func.count()).select_from(ReadingSession).where( + ReadingSession.user_id == current_user.id, + ReadingSession.status == ReadingStatus.read, + col(ReadingSession.date_finished).is_not(None), + ) + ).one() + page_rows = session.exec( select(ReadingSession, Book) .join(Book, col(Book.id) == col(ReadingSession.book_id)) @@ -569,7 +578,7 @@ def list_completed_sessions( is_reread=read_number is not None and read_number >= 2, ) ) - return result + return TimelineSessionListResponse(entries=result, total=total) @router.get("/{book_id}", response_model=BookRead) diff --git a/backend/app/schemas.py b/backend/app/schemas.py index 43f3b4eb..0c3f49c6 100644 --- a/backend/app/schemas.py +++ b/backend/app/schemas.py @@ -235,6 +235,12 @@ class TimelineSessionEntry(SQLModel): is_reread: bool = False +class TimelineSessionListResponse(BaseModel): + """Paginated completed-reading-session list with a total count.""" + entries: list[TimelineSessionEntry] + total: int + + class BookListResponse(BaseModel): """Paginated book list with total count.""" books: list[BookRead] diff --git a/backend/tests/test_reading_sessions.py b/backend/tests/test_reading_sessions.py index 138383de..2eadc411 100644 --- a/backend/tests/test_reading_sessions.py +++ b/backend/tests/test_reading_sessions.py @@ -233,7 +233,9 @@ def test_completed_sessions_timeline_endpoint(client: TestClient) -> None: resp = client.get("/api/books/sessions") assert resp.status_code == 200 - rows = resp.json() + data = resp.json() + assert data["total"] == 2 + rows = data["entries"] assert len(rows) == 2 reread_rows = [r for r in rows if r["is_reread"]] assert len(reread_rows) == 1 diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index d199fc11..69664b09 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -54,7 +54,8 @@ import type { ReadingSession, ReadingSessionInput, SessionMutationResponse, - TimelineSessionEntry + TimelineSessionEntry, + TimelineSessionListResponse } from './types'; import { apiKey, csrfToken } from './stores/auth'; import { get } from 'svelte/store'; @@ -523,9 +524,9 @@ export const api = { body: JSON.stringify({ date_started: dateStarted ?? null }) }); }, - completed(limit: number = 40, offset: number = 0): Promise { + completed(limit: number = 40, offset: number = 0): Promise { const qs = new URLSearchParams({ limit: String(limit), offset: String(offset) }); - return request(`/books/sessions?${qs.toString()}`); + return request(`/books/sessions?${qs.toString()}`); } }, diff --git a/frontend/src/lib/types.ts b/frontend/src/lib/types.ts index 5da7e5d9..5fd459f5 100644 --- a/frontend/src/lib/types.ts +++ b/frontend/src/lib/types.ts @@ -403,6 +403,11 @@ export interface TimelineSessionEntry { is_reread: boolean; } +export interface TimelineSessionListResponse { + entries: TimelineSessionEntry[]; + total: number; +} + /** Search sources the backend can report progress for. */ export type ImportSource = | 'open_library' diff --git a/frontend/src/routes/timeline/+page.svelte b/frontend/src/routes/timeline/+page.svelte index 7c25b47f..6cf34ae5 100644 --- a/frontend/src/routes/timeline/+page.svelte +++ b/frontend/src/routes/timeline/+page.svelte @@ -33,9 +33,9 @@ loading = true; try { const response = await api.books.sessions.completed(PAGE_SIZE, 0); - entries = response; - nextOffset = response.length; - hasMore = response.length === PAGE_SIZE; + entries = response.entries; + nextOffset = response.entries.length; + hasMore = nextOffset < response.total; } catch (e: unknown) { const msg = e instanceof Error ? e.message : $_('common.actionFailed', { values: { action: 'load' } }); if (shouldShowActionToast(msg)) { @@ -52,9 +52,9 @@ loadingMore = true; try { const response = await api.books.sessions.completed(PAGE_SIZE, nextOffset); - entries = [...entries, ...response]; - nextOffset += response.length; - hasMore = response.length === PAGE_SIZE; + entries = [...entries, ...response.entries]; + nextOffset += response.entries.length; + hasMore = nextOffset < response.total; } catch (e: unknown) { const msg = e instanceof Error ? e.message : $_('common.actionFailed', { values: { action: 'load' } }); if (shouldShowActionToast(msg)) { From 22837296b23a7fba13183c639b6d1ec2a6e4f2e9 Mon Sep 17 00:00:00 2001 From: codebude Date: Thu, 22 Oct 2026 22:33:28 +0200 Subject: [PATCH 19/25] Support uploading LibrisLog export ZIP directly to import wizard --- backend/app/routers/data.py | 4 +++- backend/app/services/data_import.py | 24 +++++++++++++++++++++ backend/tests/test_data_import.py | 24 +++++++++++++++++++++ docs/guide/using-librislog/import-export.md | 1 + 4 files changed, 52 insertions(+), 1 deletion(-) diff --git a/backend/app/routers/data.py b/backend/app/routers/data.py index 65d9afef..061b52b2 100644 --- a/backend/app/routers/data.py +++ b/backend/app/routers/data.py @@ -105,9 +105,11 @@ async def parse_import_file( "text/plain", "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", "application/vnd.ms-excel.sheet.macroEnabled.12", + "application/zip", + "application/x-zip-compressed", } filename = file.filename or "upload" - allowed_extensions = (".csv", ".json", ".xlsx", ".xlsm") + allowed_extensions = (".csv", ".json", ".xlsx", ".xlsm", ".zip") extension_ok = filename.lower().endswith(allowed_extensions) if file.content_type and file.content_type not in allowed_content_types and not extension_ok: raise HTTPException( diff --git a/backend/app/services/data_import.py b/backend/app/services/data_import.py index 41c8f89a..cd599dcf 100644 --- a/backend/app/services/data_import.py +++ b/backend/app/services/data_import.py @@ -266,7 +266,31 @@ def parse_upload(content: bytes, filename: str, user_id: int, delimiter: str = " if len(content) > settings.max_import_file_size_mb * 1024 * 1024: raise ValueError("error.importFileTooLarge") + # A LibrisLog export ZIP wraps books.json (or books.csv) plus other + # datasets and covers. Unpack it and import the books dataset so users can + # re-import a whole export archive instead of extracting the file first. lower = filename.lower() + if lower.endswith(".zip"): + import io + import zipfile + + try: + with zipfile.ZipFile(io.BytesIO(content)) as zf: + names = zf.namelist() + inner_name = None + if "books.json" in names: + inner_name = "books.json" + elif "books.csv" in names: + inner_name = "books.csv" + else: + raise ValueError("error.importZipMissingBooksDataset") + inner_content = zf.read(inner_name) + except zipfile.BadZipFile as exc: + raise ValueError("error.importInvalidZipFile") from exc + content = inner_content + filename = inner_name + lower = filename.lower() + sheet: str | None = None if lower.endswith(".csv"): if len(delimiter) != 1: diff --git a/backend/tests/test_data_import.py b/backend/tests/test_data_import.py index 7bfce1b7..102a1292 100644 --- a/backend/tests/test_data_import.py +++ b/backend/tests/test_data_import.py @@ -2057,3 +2057,27 @@ def test_bookstats_predefined_mapping_transforms() -> None: assert mapped["date_started"] == "2020-12-28" assert mapped["date_finished"] == "" assert mapped["date_added"] == "2020-12-28" + + +def test_parse_upload_accepts_librislog_zip(client: TestClient, monkeypatch: MonkeyPatch, tmp_path: Path) -> None: + """A LibrisLog export ZIP can be uploaded directly and unpacks books.json.""" + import io + import zipfile + from app.services.data_export import build_export_zip + from app.models import User, Book, ReadingStatus + from sqlmodel import Session, select + + # Build a minimal ZIP via the export service using a fresh session-less user. + # Reuse the test client's session by creating a book first. + resp = client.post("/api/books", json={"title": "Zip Book", "author": "A", "page_count": 100}) + assert resp.status_code == 201 + + # Export via the API to get a real ZIP. + export_resp = client.post("/api/data/export", json={"datasets": ["books"], "format": "json"}) + assert export_resp.status_code == 200 + + monkeypatch.setattr(settings, "import_temp_dir", str(tmp_path)) + parsed = di.parse_upload(export_resp.content, "export.zip", 1) + assert parsed["format"] == "json" + assert "title" in parsed["source_fields"] + assert parsed["row_count"] == 1 diff --git a/docs/guide/using-librislog/import-export.md b/docs/guide/using-librislog/import-export.md index 92230ca6..dad2a56f 100644 --- a/docs/guide/using-librislog/import-export.md +++ b/docs/guide/using-librislog/import-export.md @@ -88,6 +88,7 @@ Import data from external sources: - **JSON** — LibrisLog export format - **CSV** — Custom field mapping supported - **Excel (XLSX)**: Custom field mapping supported +- **ZIP** — A LibrisLog export archive; the wizard unpacks `books.json` (or `books.csv`) automatically, so a whole export can be uploaded without extracting it first 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. From aaab107f4b3746a7cf7182906378811308a6c6e8 Mon Sep 17 00:00:00 2001 From: codebude Date: Thu, 22 Oct 2026 22:46:48 +0200 Subject: [PATCH 20/25] Revert "Support uploading LibrisLog export ZIP directly to import wizard" This reverts commit 22837296b23a7fba13183c639b6d1ec2a6e4f2e9. --- backend/app/routers/data.py | 4 +--- backend/app/services/data_import.py | 24 --------------------- backend/tests/test_data_import.py | 24 --------------------- docs/guide/using-librislog/import-export.md | 1 - 4 files changed, 1 insertion(+), 52 deletions(-) diff --git a/backend/app/routers/data.py b/backend/app/routers/data.py index 061b52b2..65d9afef 100644 --- a/backend/app/routers/data.py +++ b/backend/app/routers/data.py @@ -105,11 +105,9 @@ async def parse_import_file( "text/plain", "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", "application/vnd.ms-excel.sheet.macroEnabled.12", - "application/zip", - "application/x-zip-compressed", } filename = file.filename or "upload" - allowed_extensions = (".csv", ".json", ".xlsx", ".xlsm", ".zip") + allowed_extensions = (".csv", ".json", ".xlsx", ".xlsm") extension_ok = filename.lower().endswith(allowed_extensions) if file.content_type and file.content_type not in allowed_content_types and not extension_ok: raise HTTPException( diff --git a/backend/app/services/data_import.py b/backend/app/services/data_import.py index cd599dcf..41c8f89a 100644 --- a/backend/app/services/data_import.py +++ b/backend/app/services/data_import.py @@ -266,31 +266,7 @@ def parse_upload(content: bytes, filename: str, user_id: int, delimiter: str = " if len(content) > settings.max_import_file_size_mb * 1024 * 1024: raise ValueError("error.importFileTooLarge") - # A LibrisLog export ZIP wraps books.json (or books.csv) plus other - # datasets and covers. Unpack it and import the books dataset so users can - # re-import a whole export archive instead of extracting the file first. lower = filename.lower() - if lower.endswith(".zip"): - import io - import zipfile - - try: - with zipfile.ZipFile(io.BytesIO(content)) as zf: - names = zf.namelist() - inner_name = None - if "books.json" in names: - inner_name = "books.json" - elif "books.csv" in names: - inner_name = "books.csv" - else: - raise ValueError("error.importZipMissingBooksDataset") - inner_content = zf.read(inner_name) - except zipfile.BadZipFile as exc: - raise ValueError("error.importInvalidZipFile") from exc - content = inner_content - filename = inner_name - lower = filename.lower() - sheet: str | None = None if lower.endswith(".csv"): if len(delimiter) != 1: diff --git a/backend/tests/test_data_import.py b/backend/tests/test_data_import.py index 102a1292..7bfce1b7 100644 --- a/backend/tests/test_data_import.py +++ b/backend/tests/test_data_import.py @@ -2057,27 +2057,3 @@ def test_bookstats_predefined_mapping_transforms() -> None: assert mapped["date_started"] == "2020-12-28" assert mapped["date_finished"] == "" assert mapped["date_added"] == "2020-12-28" - - -def test_parse_upload_accepts_librislog_zip(client: TestClient, monkeypatch: MonkeyPatch, tmp_path: Path) -> None: - """A LibrisLog export ZIP can be uploaded directly and unpacks books.json.""" - import io - import zipfile - from app.services.data_export import build_export_zip - from app.models import User, Book, ReadingStatus - from sqlmodel import Session, select - - # Build a minimal ZIP via the export service using a fresh session-less user. - # Reuse the test client's session by creating a book first. - resp = client.post("/api/books", json={"title": "Zip Book", "author": "A", "page_count": 100}) - assert resp.status_code == 201 - - # Export via the API to get a real ZIP. - export_resp = client.post("/api/data/export", json={"datasets": ["books"], "format": "json"}) - assert export_resp.status_code == 200 - - monkeypatch.setattr(settings, "import_temp_dir", str(tmp_path)) - parsed = di.parse_upload(export_resp.content, "export.zip", 1) - assert parsed["format"] == "json" - assert "title" in parsed["source_fields"] - assert parsed["row_count"] == 1 diff --git a/docs/guide/using-librislog/import-export.md b/docs/guide/using-librislog/import-export.md index dad2a56f..92230ca6 100644 --- a/docs/guide/using-librislog/import-export.md +++ b/docs/guide/using-librislog/import-export.md @@ -88,7 +88,6 @@ Import data from external sources: - **JSON** — LibrisLog export format - **CSV** — Custom field mapping supported - **Excel (XLSX)**: Custom field mapping supported -- **ZIP** — A LibrisLog export archive; the wizard unpacks `books.json` (or `books.csv`) automatically, so a whole export can be uploaded without extracting it first 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. From 49145d9698ec684596653fcec95539c93b8601d8 Mon Sep 17 00:00:00 2001 From: codebude Date: Thu, 22 Oct 2026 22:46:51 +0200 Subject: [PATCH 21/25] Revert "Fix naive/aware datetime comparison and current-session marking in session round-trip" This reverts commit bda014a581fc6f313eb67b2f01fabc66fb01829b. --- backend/app/services/data_export.py | 5 +- backend/app/services/data_import.py | 16 ++---- backend/tests/test_data_import.py | 83 ----------------------------- 3 files changed, 5 insertions(+), 99 deletions(-) diff --git a/backend/app/services/data_export.py b/backend/app/services/data_export.py index e9f6bcf9..838b66cf 100644 --- a/backend/app/services/data_export.py +++ b/backend/app/services/data_export.py @@ -211,10 +211,7 @@ def build_export_zip( sessions_by_book.setdefault(entry.book_id, []).append(entry) for book_sessions in sessions_by_book.values(): book_sessions.sort( - key=lambda s: ( - s.date_started or s.date_finished or datetime.min.replace(tzinfo=timezone.utc), - s.id or 0, - ) + key=lambda s: (s.date_started or s.date_finished or datetime.min, s.id or 0) ) books_rows = [ diff --git a/backend/app/services/data_import.py b/backend/app/services/data_import.py index 41c8f89a..379a687d 100644 --- a/backend/app/services/data_import.py +++ b/backend/app/services/data_import.py @@ -546,22 +546,14 @@ def _create_sessions_from_data( return create_initial_session(session, book) # Chronological order (oldest first); a date-less session sorts first. - # Use an aware minimum so the comparison never mixes aware and naive datetimes. - _min = datetime.min.replace(tzinfo=timezone.utc) - parsed.sort(key=lambda t: (t[2] or t[1] or _min, t[1] or _min)) - - # The current session is the most recent open (currently_reading) session, - # not simply the last session overall (a date-less open session sorts first). - last_open_idx = max( - (i for i, (st, _ds, _df) in enumerate(parsed) if st == ReadingStatus.currently_reading), - default=-1, - ) + parsed.sort(key=lambda t: (t[2] or t[1] or datetime.min, t[1] or datetime.min)) latest_completed: ReadingSession | None = None for idx, (status, ds, df) in enumerate(parsed): is_current = ( - book.reading_status == ReadingStatus.currently_reading - and idx == last_open_idx + status == ReadingStatus.currently_reading + and idx == len(parsed) - 1 + and book.reading_status == ReadingStatus.currently_reading ) rs = ReadingSession( book_id=book.id, diff --git a/backend/tests/test_data_import.py b/backend/tests/test_data_import.py index 7bfce1b7..8ea7e75d 100644 --- a/backend/tests/test_data_import.py +++ b/backend/tests/test_data_import.py @@ -1089,89 +1089,6 @@ async def test_execute_import_round_trips_reading_sessions( ] -@pytest.mark.anyio -async def test_execute_import_round_trips_open_session_without_dates( - session: Session, tmp_path: Path, monkeypatch: MonkeyPatch -) -> None: - """A currently_reading session without dates must round-trip without crashing - and must remain the current session.""" - monkeypatch.setattr(settings, "import_temp_dir", str(tmp_path)) - user = _create_test_user(session) - user_id = _require(user.id) - - book = Book( - title="Open Read", - page_count=200, - reading_status=ReadingStatus.currently_reading, - acquisition_status=AcquisitionStatus.owned, - date_started=None, - date_finished=None, - user_id=user.id, - ) - session.add(book) - session.flush() - book_id = _require(book.id) - # A completed read plus a date-less open (current) session. - session.add( - ReadingSession( - book_id=book_id, - user_id=user_id, - status=ReadingStatus.read, - date_started=datetime(2024, 1, 1, tzinfo=timezone.utc), - date_finished=datetime(2024, 1, 10, tzinfo=timezone.utc), - is_current=False, - ) - ) - session.add( - ReadingSession( - book_id=book_id, - user_id=user_id, - status=ReadingStatus.currently_reading, - date_started=None, - date_finished=None, - is_current=True, - ) - ) - session.commit() - - 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) == 1 - - # Remove the source book and its sessions. - for s in session.exec(select(ReadingSession).where(ReadingSession.book_id == book_id)).all(): - session.delete(s) - session.delete(book) - session.commit() - - file_id = "roundtrip_open" - 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", "page_count", "reading_status", "acquisition_status", "sessions") - } - events = [] - async for event in di.execute_import( - file_id, user, mapping, session, "continue_on_error", require_acquisition_status=True - ): - events.append(event) - assert [e for e in events if e["event"] == "complete"][0]["imported"] == 1 - - imported = session.exec(select(Book).where(Book.user_id == user.id)).one() - imported_sessions = list( - session.exec(select(ReadingSession).where(ReadingSession.book_id == imported.id)).all() - ) - assert len(imported_sessions) == 2 - current = next(s for s in imported_sessions if s.is_current) - assert current.status == ReadingStatus.currently_reading - - @pytest.mark.anyio async def test_execute_import_preserves_date_added( session: Session, tmp_path: Path, monkeypatch: MonkeyPatch From 8ead95f89c41351764d141b4168dbeeaaa13a1b4 Mon Sep 17 00:00:00 2001 From: codebude Date: Thu, 22 Oct 2026 22:46:58 +0200 Subject: [PATCH 22/25] Revert "Round-trip reading sessions through JSON export/import" This reverts commit 91aa9e4136f3709d4d033d239f288860db992ce4. --- backend/app/services/data_export.py | 40 ++--------- backend/app/services/data_import.py | 69 +----------------- backend/tests/test_data_import.py | 108 +--------------------------- 3 files changed, 7 insertions(+), 210 deletions(-) diff --git a/backend/app/services/data_export.py b/backend/app/services/data_export.py index 838b66cf..2ca3d8fd 100644 --- a/backend/app/services/data_export.py +++ b/backend/app/services/data_export.py @@ -52,16 +52,12 @@ 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, export_format: str, sessions: list[ReadingSession] | None = None -) -> 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. When *sessions* is provided - (JSON only), the book's reading sessions are embedded under ``sessions`` so - re-read history round-trips through the import. CSV has no list type: both + 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. """ @@ -70,7 +66,7 @@ def _book_to_dict( 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) - result = { + return { "title": book.title, "subtitle": book.subtitle, "author": author_value, @@ -92,16 +88,6 @@ def _book_to_dict( "date_finished": _serialize_datetime(book.date_finished), "cover_url": book.cover_url, } - if export_format == "json" and sessions is not None: - result["sessions"] = [ - { - "status": s.status.value, - "date_started": _serialize_datetime(s.date_started), - "date_finished": _serialize_datetime(s.date_finished), - } - for s in sessions - ] - return result def _progress_to_dict(entry: ReadingProgress, book_titles: dict[int, str]) -> dict: @@ -204,25 +190,7 @@ def build_export_zip( book_titles = {book.id: book.title for book in books if book.id is not None} - # Group sessions per book in chronological order (oldest first) so the JSON - # book export can embed them and re-read history round-trips through import. - sessions_by_book: dict[int, list[ReadingSession]] = {} - for entry in sessions: - sessions_by_book.setdefault(entry.book_id, []).append(entry) - for book_sessions in sessions_by_book.values(): - book_sessions.sort( - key=lambda s: (s.date_started or s.date_finished or datetime.min, s.id or 0) - ) - - books_rows = [ - _book_to_dict( - session, - book, - export_format, - sessions_by_book.get(book.id) if export_format == "json" else None, - ) - 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] session_rows = [_session_to_dict(entry, book_titles) for entry in sessions] 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 379a687d..45524d45 100644 --- a/backend/app/services/data_import.py +++ b/backend/app/services/data_import.py @@ -23,6 +23,7 @@ from app.config import settings from app.models import AcquisitionStatus, Book, Medium, ReadingProgress, ReadingSession, ReadingStatus, User, normalize_medium_key from app.schemas import ImportFieldConfig +from app.services.reading_sessions import create_initial_session logger = logging.getLogger(__name__) from app.time_utils import utcnow @@ -51,7 +52,6 @@ "date_started", "date_finished", "cover_url", - "sessions", ] _ALIASES: dict[str, str] = { @@ -102,8 +102,6 @@ "date read": "date_finished", "cover": "cover_url", "cover url": "cover_url", - "sessions": "sessions", - "reading sessions": "sessions", } @@ -514,62 +512,6 @@ def _parse_reading_status(value: object) -> ReadingStatus: return ReadingStatus(raw) -def _create_sessions_from_data( - session: Session, book: Book, sessions_data: object -) -> ReadingSession | None: - """Create a book's reading sessions from an embedded ``sessions`` array. - - A LibrisLog JSON export embeds the book's reading sessions as a list of - ``{status, date_started, date_finished}`` objects. This reconstructs them so - re-read history round-trips through the import. Returns the most recently - completed (``read``) session for progress attachment, or ``None`` when there - is no completed session. Falls back to a single initial session when no - embedded sessions are present (external imports). - """ - from app.services.reading_sessions import create_initial_session - - if not isinstance(sessions_data, list) or not sessions_data: - return create_initial_session(session, book) - - parsed: list[tuple[ReadingStatus, datetime | None, datetime | None]] = [] - for item in sessions_data: - if not isinstance(item, dict): - continue - status = _parse_reading_status(item.get("status")) - if status == ReadingStatus.want_to_read: - continue - ds = _parse_datetime(item.get("date_started"), "sessions.date_started") - df = _parse_datetime(item.get("date_finished"), "sessions.date_finished") - parsed.append((status, ds, df)) - - if not parsed: - return create_initial_session(session, book) - - # Chronological order (oldest first); a date-less session sorts first. - parsed.sort(key=lambda t: (t[2] or t[1] or datetime.min, t[1] or datetime.min)) - - latest_completed: ReadingSession | None = None - for idx, (status, ds, df) in enumerate(parsed): - is_current = ( - status == ReadingStatus.currently_reading - and idx == len(parsed) - 1 - and book.reading_status == ReadingStatus.currently_reading - ) - rs = ReadingSession( - book_id=book.id, - user_id=book.user_id, - status=status, - date_started=ds, - date_finished=df, - is_current=is_current, - ) - session.add(rs) - if status == ReadingStatus.read: - latest_completed = rs - session.flush() - return latest_completed - - def _build_transform_cache( mapping: dict[str, "ImportFieldConfig"], ) -> dict[str, Callable[..., Any]]: @@ -626,11 +568,6 @@ def _mapped_row( # Like authors, transforms are skipped for array values. mapped[target] = value continue - if target == "sessions" and isinstance(value, list): - # Reading sessions embed as a list of {status, date_started, - # date_finished} objects (JSON export only). Passed through untouched. - 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 @@ -1063,9 +1000,7 @@ async def execute_import( session.flush() assert book.id is not None - initial_session = _create_sessions_from_data( - session, book, row_data.get("sessions") - ) + initial_session = create_initial_session(session, book) sync_book_authors( session, diff --git a/backend/tests/test_data_import.py b/backend/tests/test_data_import.py index 8ea7e75d..19260a73 100644 --- a/backend/tests/test_data_import.py +++ b/backend/tests/test_data_import.py @@ -17,7 +17,7 @@ from sqlmodel import Session, select from app.config import settings -from app.models import AcquisitionStatus, Book, ReadingSession, 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 @@ -983,112 +983,6 @@ async def test_execute_import_full_library_json_round_trip( assert tags_list_for_book(session, book.id) == row["tags"] -@pytest.mark.anyio -async def test_execute_import_round_trips_reading_sessions( - session: Session, tmp_path: Path, monkeypatch: MonkeyPatch -) -> None: - """Re-read history must survive a JSON export/import round-trip.""" - monkeypatch.setattr(settings, "import_temp_dir", str(tmp_path)) - user = _create_test_user(session) - user_id = _require(user.id) - - book = Book( - title="Re-read Me", - page_count=300, - reading_status=ReadingStatus.read, - acquisition_status=AcquisitionStatus.owned, - date_started=datetime(2024, 1, 1, tzinfo=timezone.utc), - date_finished=datetime(2024, 1, 10, tzinfo=timezone.utc), - user_id=user.id, - ) - session.add(book) - session.flush() - book_id = _require(book.id) - - # Two sessions: the original read and a second (re-)read. - session.add( - ReadingSession( - book_id=book_id, - user_id=user_id, - status=ReadingStatus.read, - date_started=datetime(2024, 1, 1, tzinfo=timezone.utc), - date_finished=datetime(2024, 1, 10, tzinfo=timezone.utc), - is_current=False, - ) - ) - session.add( - ReadingSession( - book_id=book_id, - user_id=user_id, - status=ReadingStatus.read, - date_started=datetime(2026, 5, 1, tzinfo=timezone.utc), - date_finished=datetime(2026, 5, 8, tzinfo=timezone.utc), - is_current=False, - ) - ) - session.commit() - - 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) == 1 - assert exported_rows[0]["sessions"] == [ - { - "status": "read", - "date_started": "2024-01-01T00:00:00Z", - "date_finished": "2024-01-10T00:00:00Z", - }, - { - "status": "read", - "date_started": "2026-05-01T00:00:00Z", - "date_finished": "2026-05-08T00:00:00Z", - }, - ] - - # Remove the source book and its sessions. - for s in session.exec(select(ReadingSession).where(ReadingSession.book_id == book_id)).all(): - session.delete(s) - session.delete(book) - session.commit() - - file_id = "roundtrip_sessions" - 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", - "page_count", - "reading_status", - "acquisition_status", - "date_started", - "date_finished", - "sessions", - ) - } - events = [] - async for event in di.execute_import( - file_id, user, mapping, session, "continue_on_error", require_acquisition_status=True - ): - events.append(event) - assert [e for e in events if e["event"] == "complete"][0]["imported"] == 1 - - imported = session.exec(select(Book).where(Book.user_id == user.id)).one() - sessions = list( - session.exec(select(ReadingSession).where(ReadingSession.book_id == imported.id)).all() - ) - assert len(sessions) == 2 - finished = sorted(s.date_finished for s in sessions) - assert finished == [ - datetime(2024, 1, 10, tzinfo=timezone.utc), - datetime(2026, 5, 8, tzinfo=timezone.utc), - ] - - @pytest.mark.anyio async def test_execute_import_preserves_date_added( session: Session, tmp_path: Path, monkeypatch: MonkeyPatch From 30654221f1fe78b4dad8b0bebe8e4f3c912dbb40 Mon Sep 17 00:00:00 2001 From: codebude Date: Thu, 22 Oct 2026 22:46:59 +0200 Subject: [PATCH 23/25] Revert "Docs: note progress history is reference-only on export (full fidelity via backup)" This reverts commit 720b73ff8a80a5aa7b00e03d1f04b0c6f732b15c. --- docs/guide/using-librislog/import-export.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/docs/guide/using-librislog/import-export.md b/docs/guide/using-librislog/import-export.md index 92230ca6..0e238403 100644 --- a/docs/guide/using-librislog/import-export.md +++ b/docs/guide/using-librislog/import-export.md @@ -66,8 +66,6 @@ Choose which data to include: Reading sessions (re-read history) are embedded in the JSON book export, so a JSON export round-trips losslessly: re-importing `books.json` recreates each book's reading sessions. The CSV format cannot represent nested sessions, so re-read history does **not** round-trip through a CSV export/import; a CSV export only carries the flat `sessions` dataset as a book-ID-referenced audit view. For full fidelity of re-read history, use the JSON export or [backup and restore](./administration), which copies the whole database. -The reading progress entries are exported as a flat dataset (with the `session_id` of each entry) for auditing, but the import wizard does **not** recreate the full page-by-page progress history: it only creates a single "read to the end" entry when the "create progress for read" option is enabled. Full progress history (and its per-session linkage) is preserved only through [backup and restore](./administration). - **Older exports and backups:** A JSON export or a full backup made with an older LibrisLog version (before reading sessions) imports fine. Each imported book gets a single reading session derived from its start/finish dates, and the existing statistics, timeline and progress behave exactly as before. Full backups created by older releases are migrated automatically when restored (the schema is upgraded on restore). ### Export Process From ef7e6c90ca5761cfd2cd16e31570d42b88dd8a63 Mon Sep 17 00:00:00 2001 From: codebude Date: Thu, 22 Oct 2026 22:46:59 +0200 Subject: [PATCH 24/25] Revert "Docs: clarify CSV cannot round-trip re-read history" This reverts commit 3b60a12cab7a01ae6f8e8e43cdb53412c91a33e5. --- docs/guide/using-librislog/import-export.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/guide/using-librislog/import-export.md b/docs/guide/using-librislog/import-export.md index 0e238403..2a3e8824 100644 --- a/docs/guide/using-librislog/import-export.md +++ b/docs/guide/using-librislog/import-export.md @@ -64,7 +64,7 @@ Choose which data to include: - Reading sessions (re-read history) - Cover images -Reading sessions (re-read history) are embedded in the JSON book export, so a JSON export round-trips losslessly: re-importing `books.json` recreates each book's reading sessions. The CSV format cannot represent nested sessions, so re-read history does **not** round-trip through a CSV export/import; a CSV export only carries the flat `sessions` dataset as a book-ID-referenced audit view. For full fidelity of re-read history, use the JSON export or [backup and restore](./administration), which copies the whole database. +Reading sessions (re-read history) are embedded in the JSON book export, so a JSON export round-trips losslessly: re-importing `books.json` recreates each book's reading sessions. The separate `sessions` dataset is a flat, book-ID-referenced view exported for auditing and CSV completeness; re-read history is only preserved through the JSON book export or through [backup and restore](./administration), which copies the whole database. **Older exports and backups:** A JSON export or a full backup made with an older LibrisLog version (before reading sessions) imports fine. Each imported book gets a single reading session derived from its start/finish dates, and the existing statistics, timeline and progress behave exactly as before. Full backups created by older releases are migrated automatically when restored (the schema is upgraded on restore). From 14353b67ad0fb3ff919fd934c67b2bc3b3e55915 Mon Sep 17 00:00:00 2001 From: codebude Date: Thu, 22 Oct 2026 22:49:23 +0200 Subject: [PATCH 25/25] Rollback import/export round-trip changes (sessions/progress) Revert the misunderstanding that export/import should round-trip sessions. Export is for third-party apps; import is for flexible import from third-party apps. Full fidelity round-trip is via backup/restore (admin section). - Remove embedded 'sessions' from books.json export - Remove 'sessions' field from import (BOOK_IMPORT_FIELDS, aliases, mapping) - Revert _create_sessions_from_data to create_initial_session (single session per book) - Remove ZIP upload support from parse_upload and data.py - Remove round-trip regression tests - Update docs to reflect reference-only sessions dataset --- docs/guide/using-librislog/import-export.md | 6 +++--- docs/releases.md | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/guide/using-librislog/import-export.md b/docs/guide/using-librislog/import-export.md index 2a3e8824..0b18c9d2 100644 --- a/docs/guide/using-librislog/import-export.md +++ b/docs/guide/using-librislog/import-export.md @@ -58,13 +58,13 @@ Export your entire library or subsets of data: ### Export Datasets Choose which data to include: -- Books (full metadata, including reading sessions) +- Books (full metadata) - Reading progress entries - Tags -- Reading sessions (re-read history) +- Reading sessions (re-read history, flat view) - Cover images -Reading sessions (re-read history) are embedded in the JSON book export, so a JSON export round-trips losslessly: re-importing `books.json` recreates each book's reading sessions. The separate `sessions` dataset is a flat, book-ID-referenced view exported for auditing and CSV completeness; re-read history is only preserved through the JSON book export or through [backup and restore](./administration), which copies the whole database. +Reading sessions (re-read history) are exported as a flat `sessions` dataset (book-ID-referenced) for auditing. The import wizard does not restore re-read history: each imported book gets a single reading session derived from its start/finish dates. Full re-read history is preserved only through [backup and restore](./administration), which copies the whole database. **Older exports and backups:** A JSON export or a full backup made with an older LibrisLog version (before reading sessions) imports fine. Each imported book gets a single reading session derived from its start/finish dates, and the existing statistics, timeline and progress behave exactly as before. Full backups created by older releases are migrated automatically when restored (the schema is upgraded on restore). diff --git a/docs/releases.md b/docs/releases.md index ff27edc5..8e6d70f1 100644 --- a/docs/releases.md +++ b/docs/releases.md @@ -54,7 +54,7 @@ LibrisLog v1.10.0 adds four more metadata sources to the import search (BnF, Inv **Improvements** - 🛡️ **Session-aware progress and statistics.** Pages-per-month and pages-per-day no longer discard a re-read's pages as negative deltas (a long-standing bug where a restart at page 0 made every later delta negative) -- 📦 **Export/import keeps re-read history.** The JSON book export embeds each book's sessions; re-importing `books.json` recreates them. Reading sessions are also exported as a flat `sessions` dataset for auditing. CSV cannot represent nested sessions, so re-read history round-trips only through the JSON export or full backup/restore +- 📦 **Export includes a `sessions` dataset.** Re-read history is exported as a flat, book-ID-referenced `sessions` dataset (for auditing and third-party tools). Full re-read history is preserved through [backup and restore](/guide/using-librislog/administration), which copies the whole database - 🔁 **Full backup/restore upgraded.** Restoring a backup from an older LibrisLog release migrates the schema automatically (the new tables and columns are added on restore) - 🎯 **Date-conflict dialog gains a "Start a new read" action** for finished books moved back to "Currently reading", keeping the earlier reads in history