From c2747fbd81b3da5143c6bd13f005a907eb9d6fe4 Mon Sep 17 00:00:00 2001 From: Sylvain Cau Date: Mon, 28 Sep 2026 09:10:52 -0700 Subject: [PATCH] feat(api): state scoped imports as coverage, and carry want-to-read across The first real import of a split read like a failure: "32 matched, 244 unmatched", for a destination that holds 32 series, every one of which matched. A split exports the whole old library and imports it into one of several new ones, so most file series are meant to miss, and calling them unmatched said the opposite of what happened. A scoped import now reports coverage of the destination: how many series and books the selected libraries hold, and how many of those received state. The matched side counts distinct destination series and books, because two file series can resolve to one destination series by name and counting file matches can report more than the destination holds. The totals go through the same visibility filter as matching, so a series hidden from the reader does not make the import look incomplete. Unscoped there is no destination to measure against, so the plain counts stay and "unmatched" keeps meaning nothing was found. The book count needed splitting, not just relabelling. `booksUnmatched` mixes books whose whole series belongs elsewhere with books missed inside a series that did match. Calling all of it "other libraries" would have hidden real misses behind the reassuring wording, so the summary counts the first group separately and the page shows the remainder as missed. Want-to-read now travels in the file, on the series or book it flags. The export seeds queued series and books into its sets: a queued series is usually one never started, and with no reading state nothing else would have put it in the file. On import the queue is global but the file is grouped by series and applied a series at a time, so every entry is ranked by its original position once, up front, and offset past the reader's existing queue. Each series transaction then writes entries whose positions are already fixed, so restored entries follow the existing queue in their original order, and nothing already queued moves. The original queued date is kept, so the newest and oldest sorts still mean what they did. A checkbox turns it off. The settings test fixture had stayed in snake_case since the wire format became camelCase. Test files are excluded from the type check and nothing asserted a rendered count, so it passed while the component read undefined. It is rebuilt as a typed builder, and the new tests assert the numbers that render. --- .../v1/handlers/reading_progress_transfer.rs | 1 + .../src/reading_transfer/export.rs | 45 +- .../src/reading_transfer/import.rs | 278 ++++++++++- .../src/reading_transfer/matching.rs | 42 +- .../src/reading_transfer/model.rs | 61 +++ docs/api/openapi.json | 78 ++- .../reading-progress-transfer.md | 47 +- tests/api/reading_progress_transfer.rs | 443 +++++++++++++++++- web/openapi.json | 78 ++- .../ReadingProgressTransferSettings.test.tsx | 163 +++++-- .../ReadingProgressTransferSettings.tsx | 133 ++++-- web/src/types/api.generated.ts | 77 +++ 12 files changed, 1373 insertions(+), 73 deletions(-) diff --git a/crates/codex-api/src/routes/v1/handlers/reading_progress_transfer.rs b/crates/codex-api/src/routes/v1/handlers/reading_progress_transfer.rs index 93aa2c91..51fc593f 100644 --- a/crates/codex-api/src/routes/v1/handlers/reading_progress_transfer.rs +++ b/crates/codex-api/src/routes/v1/handlers/reading_progress_transfer.rs @@ -140,6 +140,7 @@ pub async fn import_reading_progress( reattach_sessions: request.reattach_sessions, accept_stem_matches: request.accept_stem_matches, library_ids: request.library_ids, + restore_want_to_read: request.restore_want_to_read, }; let response = diff --git a/crates/codex-services/src/reading_transfer/export.rs b/crates/codex-services/src/reading_transfer/export.rs index 19a2e547..f3163fef 100644 --- a/crates/codex-services/src/reading_transfer/export.rs +++ b/crates/codex-services/src/reading_transfer/export.rs @@ -18,7 +18,7 @@ use sea_orm::{ColumnTrait, DatabaseConnection, EntityTrait, QueryFilter, QueryOr use std::collections::{HashMap, HashSet}; use uuid::Uuid; -use codex_db::entities::{books, read_completions, read_progress, reading_sessions}; +use codex_db::entities::{books, read_completions, read_progress, reading_sessions, want_to_read}; use codex_db::repositories::{ LibraryRepository, ReadProgressRepository, SeriesExternalIdRepository, SeriesRepository, UserSeriesRatingRepository, @@ -26,7 +26,7 @@ use codex_db::repositories::{ use super::model::{ ExportBookDto, ExportCompletionDto, ExportExternalIdDto, ExportProgressDto, ExportSeriesDto, - ExportSessionDto, READING_PROGRESS_FORMAT, READING_PROGRESS_VERSION, + ExportSessionDto, ExportWantToReadDto, READING_PROGRESS_FORMAT, READING_PROGRESS_VERSION, ReadingProgressExportDocument, }; use super::series_relative_book_path; @@ -95,6 +95,13 @@ async fn sessions_for_user( /// `include_sessions = false` omits the `sessions` key entirely on every book /// rather than emitting empty arrays, so a client can tell "not exported" /// apart from "exported, and there were none". +fn to_want_to_read(entry: &want_to_read::Model) -> ExportWantToReadDto { + ExportWantToReadDto { + position: entry.position, + added_at: entry.added_at, + } +} + /// `library_ids` of `None` exports everything the reader has state for. /// `Some` narrows to those libraries, which is what a split wants: carrying /// an entire reading history when only one library is being reorganised makes @@ -113,6 +120,10 @@ pub async fn export_reading_progress( Vec::new() }; let ratings = UserSeriesRatingRepository::get_all_for_user(db, user_id).await?; + let queue = want_to_read::Entity::find() + .filter(want_to_read::Column::UserId.eq(user_id)) + .all(db) + .await?; let mut book_id_set: HashSet = HashSet::new(); for p in &progress_rows { @@ -128,6 +139,11 @@ pub async fn export_reading_progress( book_id_set.insert(id); } } + for entry in &queue { + if let Some(id) = entry.book_id { + book_id_set.insert(id); + } + } let book_ids: Vec = book_id_set.into_iter().collect(); let books = books_including_soft_deleted(db, &book_ids).await?; @@ -135,6 +151,19 @@ pub async fn export_reading_progress( for r in &ratings { series_id_set.insert(r.series_id); } + for entry in &queue { + if let Some(id) = entry.series_id { + series_id_set.insert(id); + } + } + let queued_series: HashMap = queue + .iter() + .filter_map(|e| e.series_id.map(|id| (id, e))) + .collect(); + let queued_books: HashMap = queue + .iter() + .filter_map(|e| e.book_id.map(|id| (id, e))) + .collect(); let series_ids: Vec = series_id_set.into_iter().collect(); let mut series_rows = SeriesRepository::get_by_ids(db, &series_ids).await?; @@ -270,7 +299,12 @@ pub async fn export_reading_progress( }; let has_sessions = sessions.as_ref().is_some_and(|s| !s.is_empty()); - if progress.is_none() && completions.is_empty() && !has_sessions { + let want_to_read = queued_books.get(&book.id).map(|e| to_want_to_read(e)); + if progress.is_none() + && completions.is_empty() + && !has_sessions + && want_to_read.is_none() + { // Nothing to say about this book for this user. continue; } @@ -283,11 +317,13 @@ pub async fn export_reading_progress( progress, completions, sessions, + want_to_read, }); } } - if rating_row.is_none() && book_docs.is_empty() { + let series_want_to_read = queued_series.get(&series.id).map(|e| to_want_to_read(e)); + if rating_row.is_none() && book_docs.is_empty() && series_want_to_read.is_none() { // Nothing recorded against this series for this user. continue; } @@ -302,6 +338,7 @@ pub async fn export_reading_progress( notes: rating_row.and_then(|r| r.notes.clone()), rating_updated_at: rating_row.map(|r| r.updated_at), books: book_docs, + want_to_read: series_want_to_read, }); } diff --git a/crates/codex-services/src/reading_transfer/import.rs b/crates/codex-services/src/reading_transfer/import.rs index a9c14cf7..0f036a1d 100644 --- a/crates/codex-services/src/reading_transfer/import.rs +++ b/crates/codex-services/src/reading_transfer/import.rs @@ -23,7 +23,7 @@ use std::fmt; use uuid::Uuid; use codex_db::entities::{ - books, read_completions, read_progress, reading_sessions, user_series_ratings, + books, read_completions, read_progress, reading_sessions, user_series_ratings, want_to_read, }; use codex_db::repositories::{ BookRepository, LibraryRepository, SeriesRepository, UserSeriesRatingRepository, @@ -54,6 +54,8 @@ pub struct ImportOptions { /// the reader can see; `Some` narrows the search, which is how an import /// can run while the old copy of a series is still present. pub library_ids: Option>, + /// See [`ImportReadingProgressRequest::restore_want_to_read`]. + pub restore_want_to_read: bool, } /// A rejection worth a 400, versus every other failure which is a 500. @@ -341,6 +343,120 @@ struct PlannedState { ratings: HashMap, /// Completion and session ids already claimed by an earlier entry. rows: HashSet, + /// Series and books already in the reader's queue, or queued by an earlier + /// entry in this import. Two file series can resolve to one destination + /// series, and the second must not queue it again. + queued_series: HashSet, + queued_books: HashSet, +} + +/// Which queue a restored want-to-read entry flags. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum QueueTarget { + Series(Uuid), + Book(Uuid), +} + +/// A want-to-read entry the import will write, with its position already +/// fixed. See [`rank_queue_entries`] for why positions are decided before any +/// series is applied. +#[derive(Debug, Clone)] +struct QueueInsert { + target: QueueTarget, + position: i32, + added_at: DateTime, +} + +/// Where a want-to-read entry sits in the file: its series' index, and its +/// book's index within that series, or `None` for a series-level entry. +type QueueKey = (usize, Option); + +/// Rank every want-to-read entry in the file by its original queue position. +/// +/// The queue is ordered globally across series and books, but the file groups +/// entries by series and the import applies one series per transaction. So +/// positions are decided here, once, for the whole file: each entry's rank in +/// its original order, which the caller offsets past the reader's existing +/// queue. Each series transaction then writes entries whose positions are +/// already fixed, and the original relative order survives regardless of the +/// order series happen to be applied in. Entries that are later skipped leave +/// gaps, which the custom sort does not mind. +/// +/// Keyed by (series index, book index), `None` for a series-level entry. +fn rank_queue_entries(document: &ReadingProgressExportDocument) -> HashMap { + let mut entries: Vec<(QueueKey, i32, DateTime)> = Vec::new(); + for (series_index, series_doc) in document.series.iter().enumerate() { + if let Some(entry) = &series_doc.want_to_read { + entries.push(((series_index, None), entry.position, entry.added_at)); + } + for (book_index, book_doc) in series_doc.books.iter().enumerate() { + if let Some(entry) = &book_doc.want_to_read { + entries.push(( + (series_index, Some(book_index)), + entry.position, + entry.added_at, + )); + } + } + } + // Ties in position fall back to when the entry was queued, then to the + // file's own order, so the result never depends on hash iteration. + entries.sort_by(|a, b| a.1.cmp(&b.1).then(a.2.cmp(&b.2)).then(a.0.cmp(&b.0))); + entries + .into_iter() + .enumerate() + .map(|(rank, (key, _, _))| (key, rank as i32)) + .collect() +} + +/// The queue entries one matched series will write. +/// +/// An entry is written only onto a series that matched, or a book whose match +/// was applied, and only when that series or book is not already queued. +/// Already-queued entries keep their place: moving them would reshuffle a +/// queue the reader ordered by hand. +fn plan_queue( + series_index: usize, + series_id: Uuid, + series_doc: &ExportSeriesDto, + plans: &[BookPlan<'_>], + ranks: &HashMap, + base_position: i32, + planned: &mut PlannedState, +) -> Vec { + let mut inserts = Vec::new(); + + // Every entry in the file is ranked, so a missing rank would be a bug; + // skipping it is better than panicking a request over it. + if let Some(entry) = &series_doc.want_to_read + && let Some(rank) = ranks.get(&(series_index, None)) + && planned.queued_series.insert(series_id) + { + inserts.push(QueueInsert { + target: QueueTarget::Series(series_id), + position: base_position + rank, + added_at: entry.added_at, + }); + } + + for (book_index, plan) in plans.iter().enumerate() { + let (Some(entry), Some(book_id)) = (&plan.book_doc.want_to_read, plan.matched_book_id) + else { + continue; + }; + let Some(rank) = ranks.get(&(series_index, Some(book_index))) else { + continue; + }; + if plan.applied && planned.queued_books.insert(book_id) { + inserts.push(QueueInsert { + target: QueueTarget::Book(book_id), + position: base_position + rank, + added_at: entry.added_at, + }); + } + } + + inserts } /// Bind-parameter-safe batch size for `IN (...)` lookups. @@ -853,6 +969,7 @@ async fn apply_series( series_id: Uuid, plans: &[BookPlan<'_>], rating_decision: &RatingDecision, + queue_inserts: &[QueueInsert], ) -> Result<()> { let txn = db.begin().await?; @@ -877,6 +994,26 @@ async fn apply_series( apply_rating(&txn, user_id, series_id, rating_decision).await?; + // Written with an explicit position and date rather than through + // `WantToReadRepository::add_series`, which needs a plain connection, + // stamps the current time, and rescans the whole queue for a position. + for insert in queue_inserts { + let (series, book) = match insert.target { + QueueTarget::Series(id) => (Some(id), None), + QueueTarget::Book(id) => (None, Some(id)), + }; + want_to_read::ActiveModel { + id: Set(Uuid::new_v4()), + user_id: Set(user_id), + series_id: Set(series), + book_id: Set(book), + added_at: Set(insert.added_at), + position: Set(insert.position), + } + .insert(&txn) + .await?; + } + txn.commit().await?; Ok(()) } @@ -1055,7 +1192,25 @@ pub async fn import_reading_progress( let mut series_reports = Vec::with_capacity(document.series.len()); let mut planned = PlannedState::default(); - for series_doc in &document.series { + // Decided once for the whole file; see `rank_queue_entries`. + let queue_ranks = rank_queue_entries(document); + let mut queue_base = 0; + if options.restore_want_to_read && !queue_ranks.is_empty() { + let existing = want_to_read::Entity::find() + .filter(want_to_read::Column::UserId.eq(user_id)) + .all(db) + .await + .map_err(|e| ImportError::Database(e.into()))?; + queue_base = existing + .iter() + .map(|e| e.position) + .max() + .map_or(0, |m| m + 1); + planned.queued_series = existing.iter().filter_map(|e| e.series_id).collect(); + planned.queued_books = existing.iter().filter_map(|e| e.book_id).collect(); + } + + for (series_index, series_doc) in document.series.iter().enumerate() { summary.series_total += 1; // A failure in one series is reported on that series and the import @@ -1134,12 +1289,33 @@ pub async fn import_reading_progress( } }; + let queue_inserts = if options.restore_want_to_read { + plan_queue( + series_index, + series_id, + series_doc, + &plans, + &queue_ranks, + queue_base, + &mut planned, + ) + } else { + Vec::new() + }; + let outcome = if options.dry_run { Ok(false) } else { - apply_series(db, user_id, series_id, &plans, &rating_decision) - .await - .map(|()| true) + apply_series( + db, + user_id, + series_id, + &plans, + &rating_decision, + &queue_inserts, + ) + .await + .map(|()| true) }; let books: Vec = plans.iter().map(book_report_from_plan).collect(); @@ -1152,6 +1328,7 @@ pub async fn import_reading_progress( tally_book_report(&mut summary, report, true); } tally_rating_write(&mut summary, &rating_decision); + summary.want_to_read_restored += queue_inserts.len() as u32; series_reports.push(ImportSeriesReport { library_relative_path: series_doc.library_relative_path.clone(), name: series_doc.name.clone(), @@ -1184,6 +1361,36 @@ pub async fn import_reading_progress( } } + // Coverage of the destination, counted in destination terms. File series + // can resolve to the same destination series by name, so counting file + // matches can report more than the destination holds. + summary.series_matched_distinct = series_reports + .iter() + .filter_map(|report| report.matched_series_id) + .collect::>() + .len() as u32; + summary.books_matched_distinct = series_reports + .iter() + .flat_map(|report| report.books.iter()) + .filter(|book| book.disposition == BookDisposition::Matched) + .filter_map(|book| book.matched_book_id) + .collect::>() + .len() as u32; + + summary.books_in_unmatched_series = series_reports + .iter() + .filter(|report| report.disposition == SeriesDisposition::Unmatched) + .map(|report| report.books.len() as u32) + .sum(); + + if let Some(library_ids) = options.library_ids.as_deref() { + let (series, books) = matching::scope_totals(db, &content_filter, library_ids) + .await + .map_err(ImportError::Database)?; + summary.series_in_selected_libraries = Some(series); + summary.books_in_selected_libraries = Some(books); + } + // A stranded row means the progress moved and the reading history did // not, which the per-book counts show but the summary otherwise reads as // success. Say it plainly: the reader can still fix it by removing or @@ -1210,7 +1417,9 @@ pub async fn import_reading_progress( #[cfg(test)] mod tests { use super::*; - use crate::reading_transfer::model::ExportProgressDto; + use crate::reading_transfer::model::{ + ExportProgressDto, ExportWantToReadDto, READING_PROGRESS_FORMAT, READING_PROGRESS_VERSION, + }; fn progress_row( current_page: i32, @@ -1343,6 +1552,7 @@ mod tests { notes: None, rating_updated_at: when, books: vec![], + want_to_read: None, } } @@ -1560,6 +1770,62 @@ mod tests { assert_eq!(decision, RowDecision::Skip); } + /// The queue is global but the file is grouped by series, so ranking has + /// to reconstruct the original order across series and books, and settle + /// ties the same way every time. + #[test] + fn queue_entries_rank_by_original_position_across_series_and_books() { + let when = Utc::now(); + let entry = |position| ExportWantToReadDto { + position, + added_at: when, + }; + let book = |position: Option| ExportBookDto { + path: "v01.cbz".into(), + file_name: "v01.cbz".into(), + file_hash: String::new(), + partial_hash: String::new(), + progress: None, + completions: vec![], + sessions: None, + want_to_read: position.map(entry), + }; + let series = |position: Option, books| ExportSeriesDto { + external_ids: vec![], + library_relative_path: "s".into(), + name: "s".into(), + rating: None, + notes: None, + rating_updated_at: None, + books, + want_to_read: position.map(entry), + }; + let document = ReadingProgressExportDocument { + format: READING_PROGRESS_FORMAT.into(), + version: READING_PROGRESS_VERSION, + exported_at: when, + includes_sessions: false, + series: vec![ + series(Some(7), vec![book(Some(2)), book(None)]), + series(None, vec![book(Some(5))]), + series(Some(2), vec![]), + ], + }; + + let ranks = rank_queue_entries(&document); + + // Position 2 twice: the tie goes to file order, so series 0's book + // (0, Some(0)) comes before series 2 (2, None). + assert_eq!(ranks[&(0, Some(0))], 0); + assert_eq!(ranks[&(2, None)], 1); + assert_eq!(ranks[&(1, Some(0))], 2); + assert_eq!(ranks[&(0, None)], 3); + assert!( + !ranks.contains_key(&(0, Some(1))), + "an unqueued book has no rank" + ); + } + #[test] fn another_users_row_is_never_touched() { let mut planned = PlannedState::default(); diff --git a/crates/codex-services/src/reading_transfer/matching.rs b/crates/codex-services/src/reading_transfer/matching.rs index 6dbf1338..08e5e472 100644 --- a/crates/codex-services/src/reading_transfer/matching.rs +++ b/crates/codex-services/src/reading_transfer/matching.rs @@ -10,7 +10,7 @@ //! would confirm it exists. use anyhow::Result; -use sea_orm::{ColumnTrait, DatabaseConnection, EntityTrait, QueryFilter}; +use sea_orm::{ColumnTrait, DatabaseConnection, EntityTrait, QueryFilter, QuerySelect}; use std::collections::HashSet; use uuid::Uuid; @@ -110,6 +110,44 @@ async fn live_candidates( Ok(visible.into_iter().filter(|id| live.contains(id)).collect()) } +/// How many live series and books the reader can see in these libraries. +/// +/// The denominator for a scoped import's coverage. It goes through the same +/// visibility filter as matching: a series hidden from the reader could never +/// have received their state, so counting it would report an import as less +/// complete than it was. "Live" means at least one book not marked deleted, +/// the same test [`live_candidates`] applies. +pub async fn scope_totals( + db: &DatabaseConnection, + content_filter: &ContentFilter, + library_ids: &[Uuid], +) -> Result<(u32, u32)> { + // Only the series id is needed per book; a full row would drag in the + // EPUB position blobs for every book in the library. + let series_per_book: Vec = books::Entity::find() + .select_only() + .column(books::Column::SeriesId) + .filter(books::Column::LibraryId.is_in(library_ids.to_vec())) + .filter(books::Column::Deleted.eq(false)) + .into_tuple() + .all(db) + .await?; + + let distinct: Vec = series_per_book + .iter() + .copied() + .collect::>() + .into_iter() + .collect(); + let visible: HashSet = visible_ids(content_filter, distinct).into_iter().collect(); + + let books = series_per_book + .iter() + .filter(|series_id| visible.contains(series_id)) + .count() as u32; + Ok((visible.len() as u32, books)) +} + async fn series_ids_by_external_id( db: &DatabaseConnection, source: &str, @@ -345,6 +383,7 @@ mod tests { progress: None, completions: vec![], sessions: None, + want_to_read: None, } } @@ -537,6 +576,7 @@ mod tests { notes: None, rating_updated_at: None, books: vec![], + want_to_read: None, } } diff --git a/crates/codex-services/src/reading_transfer/model.rs b/crates/codex-services/src/reading_transfer/model.rs index 59ed8181..8b944beb 100644 --- a/crates/codex-services/src/reading_transfer/model.rs +++ b/crates/codex-services/src/reading_transfer/model.rs @@ -121,6 +121,24 @@ pub struct ExportBookDto { /// `includeSessions=false`. #[serde(default, skip_serializing_if = "Option::is_none")] pub sessions: Option>, + /// Present when this book, rather than its whole series, is queued. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub want_to_read: Option, +} + +/// A want-to-read queue entry, attached to the series or book it flags. +/// +/// `position` is carried so imported entries keep their original relative +/// order: the queue is ordered globally across series and books, and that +/// order is otherwise lost when the file groups entries by series. +/// `added_at` is carried because the queue's newest and oldest sorts read it, +/// and stamping the import time would reorder both views by when the import +/// happened to run. +#[derive(Debug, Clone, Serialize, Deserialize, ToSchema)] +#[serde(rename_all = "camelCase")] +pub struct ExportWantToReadDto { + pub position: i32, + pub added_at: DateTime, } /// One series and everything the exporting user recorded against its books. @@ -145,6 +163,10 @@ pub struct ExportSeriesDto { pub rating_updated_at: Option>, #[serde(default, skip_serializing_if = "Vec::is_empty")] pub books: Vec, + /// Present when the whole series is queued. A queued series is often one + /// the reader never started, so it can appear with no books at all. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub want_to_read: Option, } /// The whole export: one user's reading state, self-describing enough to be @@ -196,6 +218,10 @@ fn default_reattach_sessions() -> bool { true } +fn default_restore_want_to_read() -> bool { + true +} + /// `POST /api/v1/reading-progress/import` request body. #[derive(Debug, Clone, Serialize, Deserialize, ToSchema)] #[serde(rename_all = "camelCase")] @@ -236,6 +262,12 @@ pub struct ImportReadingProgressRequest { /// `ambiguous`. #[serde(default)] pub library_ids: Option>, + /// Put queued series and books back into want-to-read. On by default: + /// carrying the queue across a split is the reason it is exported. + /// Restored entries land after anything already queued, in their original + /// relative order, and an entry already queued is left where it is. + #[serde(default = "default_restore_want_to_read")] + pub restore_want_to_read: bool, pub file: ReadingProgressExportDocument, } @@ -365,6 +397,35 @@ pub struct ImportSummary { /// Rows left behind on a live book in another library. Non-zero means the /// import moved less than it appears to have. pub rows_stranded: u32, + /// Distinct destination series that at least one file series resolved to. + /// + /// `series_matched` counts file series, and two of those can resolve to + /// one destination series by name, so it can exceed what the destination + /// holds. Coverage of the destination has to count this instead. + pub series_matched_distinct: u32, + /// Distinct destination books matched, for the same reason. + pub books_matched_distinct: u32, + /// Live series in the libraries the import was scoped to. `None` when the + /// import was not scoped: the denominator would then be every series the + /// reader can see, which measures nothing. + /// + /// With this set, a file series that did not match is almost always one + /// that belongs to another library, which is expected when a split + /// imports into one of several new libraries, not a matching failure. + pub series_in_selected_libraries: Option, + /// Live books in the scoped libraries. `None` when not scoped. + pub books_in_selected_libraries: Option, + /// Books counted in `books_unmatched` only because their whole series did + /// not match. + /// + /// `books_unmatched` mixes two outcomes. In a scoped import these are + /// books belonging to other libraries, which is expected. The remainder, + /// `books_unmatched - books_in_unmatched_series`, are books missed inside + /// a series that *did* match, which is worth a reader's attention and + /// must not be hidden under the reassuring label. + pub books_in_unmatched_series: u32, + /// Queue entries put back into want-to-read. + pub want_to_read_restored: u32, } /// The response for both a real import and a dry run: the shape is identical diff --git a/docs/api/openapi.json b/docs/api/openapi.json index 3b9e31fd..1c8688c9 100644 --- a/docs/api/openapi.json +++ b/docs/api/openapi.json @@ -30709,6 +30709,10 @@ "$ref": "#/components/schemas/ExportSessionDto" }, "description": "Omitted entirely (not an empty array) when the export was taken with\n`includeSessions=false`." + }, + "wantToRead": { + "$ref": "#/components/schemas/ExportWantToReadDto", + "description": "Present when this book, rather than its whole series, is queued." } } }, @@ -30948,6 +30952,10 @@ ], "format": "date-time", "description": "When the rating was last changed. The `newest` conflict policy needs it\nto tell a stale rating from a fresh one; a file without it never\noverwrites an existing rating except under `overwrite`." + }, + "wantToRead": { + "$ref": "#/components/schemas/ExportWantToReadDto", + "description": "Present when the whole series is queued. A queued series is often one\nthe reader never started, so it can appear with no books at all." } } }, @@ -31034,6 +31042,24 @@ } } }, + "ExportWantToReadDto": { + "type": "object", + "description": "A want-to-read queue entry, attached to the series or book it flags.\n\n`position` is carried so imported entries keep their original relative\norder: the queue is ordered globally across series and books, and that\norder is otherwise lost when the file groups entries by series.\n`added_at` is carried because the queue's newest and oldest sorts read it,\nand stamping the import time would reorder both views by when the import\nhappened to run.", + "required": [ + "position", + "addedAt" + ], + "properties": { + "addedAt": { + "type": "string", + "format": "date-time" + }, + "position": { + "type": "integer", + "format": "int32" + } + } + }, "ExternalIdContextDto": { "type": "object", "description": "External ID context for template evaluation.\n\nRepresents an external ID from a metadata provider (plugin, comicinfo, etc.)\nin a simplified format suitable for template access.", @@ -32594,6 +32620,10 @@ "type": "boolean", "description": "When a session or completion in the file already exists as the\nimporter's own row but is not on a live book (its book was hard-deleted,\nleaving `book_id` null, or the scanner marked it deleted after the file\nmoved), move it onto the matched book instead of skipping it." }, + "restoreWantToRead": { + "type": "boolean", + "description": "Put queued series and books back into want-to-read. On by default:\ncarrying the queue across a split is the reason it is exported.\nRestored entries land after anything already queued, in their original\nrelative order, and an entry already queued is left where it is." + }, "sourcePreference": { "type": [ "array", @@ -32714,7 +32744,11 @@ "completionsReattached", "sessionsInserted", "sessionsReattached", - "rowsStranded" + "rowsStranded", + "seriesMatchedDistinct", + "booksMatchedDistinct", + "booksInUnmatchedSeries", + "wantToReadRestored" ], "properties": { "booksAmbiguous": { @@ -32727,11 +32761,32 @@ "format": "int32", "minimum": 0 }, + "booksInSelectedLibraries": { + "type": [ + "integer", + "null" + ], + "format": "int32", + "description": "Live books in the scoped libraries. `None` when not scoped.", + "minimum": 0 + }, + "booksInUnmatchedSeries": { + "type": "integer", + "format": "int32", + "description": "Books counted in `books_unmatched` only because their whole series did\nnot match.\n\n`books_unmatched` mixes two outcomes. In a scoped import these are\nbooks belonging to other libraries, which is expected. The remainder,\n`books_unmatched - books_in_unmatched_series`, are books missed inside\na series that *did* match, which is worth a reader's attention and\nmust not be hidden under the reassuring label.", + "minimum": 0 + }, "booksMatched": { "type": "integer", "format": "int32", "minimum": 0 }, + "booksMatchedDistinct": { + "type": "integer", + "format": "int32", + "description": "Distinct destination books matched, for the same reason.", + "minimum": 0 + }, "booksStemMatched": { "type": "integer", "format": "int32", @@ -32783,11 +32838,26 @@ "format": "int32", "minimum": 0 }, + "seriesInSelectedLibraries": { + "type": [ + "integer", + "null" + ], + "format": "int32", + "description": "Live series in the libraries the import was scoped to. `None` when the\nimport was not scoped: the denominator would then be every series the\nreader can see, which measures nothing.\n\nWith this set, a file series that did not match is almost always one\nthat belongs to another library, which is expected when a split\nimports into one of several new libraries, not a matching failure.", + "minimum": 0 + }, "seriesMatched": { "type": "integer", "format": "int32", "minimum": 0 }, + "seriesMatchedDistinct": { + "type": "integer", + "format": "int32", + "description": "Distinct destination series that at least one file series resolved to.\n\n`series_matched` counts file series, and two of those can resolve to\none destination series by name, so it can exceed what the destination\nholds. Coverage of the destination has to count this instead.", + "minimum": 0 + }, "seriesTotal": { "type": "integer", "format": "int32", @@ -32807,6 +32877,12 @@ "type": "integer", "format": "int32", "minimum": 0 + }, + "wantToReadRestored": { + "type": "integer", + "format": "int32", + "description": "Queue entries put back into want-to-read.", + "minimum": 0 } } }, diff --git a/docs/docs/backup-migration/reading-progress-transfer.md b/docs/docs/backup-migration/reading-progress-transfer.md index 49676453..edda0dc6 100644 --- a/docs/docs/backup-migration/reading-progress-transfer.md +++ b/docs/docs/backup-migration/reading-progress-transfer.md @@ -6,8 +6,9 @@ Reorganising a library, whether splitting one root into several, moving files to a new path, or moving your whole collection to a different Codex instance, mints new series and book ids. Nothing about your reading history follows -automatically: `read_progress`, `read_completions`, `reading_sessions`, and -your series ratings are all keyed on the ids the old scan created. +automatically: `read_progress`, `read_completions`, `reading_sessions`, your +series ratings, and your want-to-read queue are all keyed on the ids the old +scan created. `GET /api/v1/reading-progress/export` and `POST /api/v1/reading-progress/import` exist to carry that state across the move. Export writes one JSON file for your @@ -104,6 +105,7 @@ that the content exists. | `conflictPolicy` | `newest` | How to resolve a book/rating that already has a value on this side: `newest` (later `updatedAt` wins), `furthest` (further into the book wins; a finished read always beats a partial one), `skip_existing`, or `overwrite`. A rating has no position, so `furthest` behaves like `newest` for ratings, and a file without a rating timestamp never replaces an existing rating except under `overwrite` | | `reattachSessions` | `true` | When a session or completion in the file already exists as your own row but is not on a live book (its book was deleted, or the scanner marked it deleted after the file moved), move it onto the matched book instead of skipping it. A no-op, reported as such, when the file carries no sessions | | `acceptStemMatches` | `false` | Apply a book match found only by filename stem | +| `restoreWantToRead` | `true` | Put queued series and books back into want-to-read. See [Want to read](#want-to-read) | | `libraryIds` | all libraries | Which libraries a series may match into. Naming the target library is what lets an import run before the old library has been rescanned: otherwise both copies of a series are live, both match, and the series is reported `ambiguous` | `GET /api/v1/reading-progress/export` takes two query parameters. @@ -139,6 +141,22 @@ the history does not. The fix is the notice's advice: delete or rescan the other library so its books are no longer live, then import again. The reused row ids make that second import safe to run. +## Want to read + +Your want-to-read queue travels with the export: a whole series you queued, +and a single book you queued on its own. A series you queued but never +started is included too, which matters, because that is the usual reason +anything is on the list, and it has no reading state to pull it in otherwise. + +On import, restored entries go **after** anything already in your queue, in +the order they held on the old library. Anything already queued keeps its +place: nothing you arranged by hand is moved. Each entry keeps the date it was +first queued, so sorting the list by newest or oldest still means what it +did. Importing the same file twice adds nothing the second time. + +Entries are restored only onto a series that matched, or a book whose match +was applied, so a stem match you did not accept restores nothing. + ## The response The response is the same shape whether or not `dryRun` is set: counts, plus a @@ -147,6 +165,27 @@ per-series and per-book breakdown of what matched, what did not, and what was series does not cost every other series in the file its progress; the per-series `committed` field says which ones actually landed. +### Reading the counts after a scoped import + +When the import names `libraryIds`, most unmatched series are not a problem: +splitting a library exports the whole old library and imports it into one of +several new ones, so series belonging to the others are *meant* to miss. The +summary therefore also states coverage of the destination: + +| Field | Meaning | +|---|---| +| `seriesInSelectedLibraries`, `booksInSelectedLibraries` | What the selected libraries hold (only what you can see). Absent when the import was not scoped | +| `seriesMatchedDistinct`, `booksMatchedDistinct` | How many of those received state. Distinct, because two series in the file can resolve to one here | +| `booksInUnmatchedSeries` | Books whose whole series did not match. After a scoped import, these belong to other libraries | + +`booksUnmatched - booksInUnmatchedSeries` is the number worth looking at: +books missed inside a series that **did** match. The Settings page shows +these as "missed" and sets the rest aside, reading, for example, *32 of the +32 series in Shonen matched*. + +Without `libraryIds` there is no destination to measure against, so these +fields are absent and an unmatched series means what it says. + Ratings must be between 1 and 100, the same range the rating endpoint enforces; a file with any other value is rejected with a 400 naming the series. Imports up to 64 MB are accepted, which is room for tens of thousands @@ -155,7 +194,9 @@ of books with their sessions. ## Limitations - Metadata, collections, read lists, and covers are not carried; this moves - reading state only. See [Data Exports](../exports) for a metadata export, and + reading state and your want-to-read queue only. Read lists have their own + ordering and collections can be rule-driven, so neither follows the series + match cleanly. See [Data Exports](../exports) for a metadata export, and [`codex export`](./export-import-copy.md) for a full instance backup. - A series with no external ids whose name and path both changed will not match. Give it an external id (or a manual one) before the move if you can. diff --git a/tests/api/reading_progress_transfer.rs b/tests/api/reading_progress_transfer.rs index 0e3b15f7..313fd9aa 100644 --- a/tests/api/reading_progress_transfer.rs +++ b/tests/api/reading_progress_transfer.rs @@ -14,8 +14,8 @@ mod common; use chrono::{Duration, Utc}; use codex::api::routes::v1::dto::{ BookDisposition, ConflictPolicy, ExportBookDto, ExportCompletionDto, ExportExternalIdDto, - ExportProgressDto, ExportSeriesDto, ExportSessionDto, FieldOutcome, HashMode, - ImportReadingProgressRequest, ImportReadingProgressResponse, READING_PROGRESS_FORMAT, + ExportProgressDto, ExportSeriesDto, ExportSessionDto, ExportWantToReadDto, FieldOutcome, + HashMode, ImportReadingProgressRequest, ImportReadingProgressResponse, READING_PROGRESS_FORMAT, READING_PROGRESS_VERSION, ReadingProgressExportDocument, SeriesDisposition, }; use codex::db::ScanningStrategy; @@ -127,6 +127,7 @@ fn minimal_book_doc(path: &str, file_name: &str, hash: &str, current_page: i32) }), completions: vec![], sessions: None, + want_to_read: None, } } @@ -155,6 +156,7 @@ fn import_request( reattach_sessions: true, accept_stem_matches: false, library_ids: None, + restore_want_to_read: true, file, } } @@ -474,6 +476,7 @@ async fn exercise_idempotent_import(db: &DatabaseConnection) { notes: None, rating_updated_at: None, books: vec![book_doc], + want_to_read: None, }; let doc = document(vec![series_doc], true); @@ -549,6 +552,7 @@ async fn dry_run_reports_matches_but_writes_nothing() { notes: None, rating_updated_at: None, books: vec![minimal_book_doc("v01.cbz", "v01.cbz", "h1", 9)], + want_to_read: None, }; let doc = document(vec![series_doc], false); @@ -630,6 +634,7 @@ async fn exercise_visibility_denies_unmatched(db: &DatabaseConnection) { notes: None, rating_updated_at: None, books: vec![minimal_book_doc("v01.cbz", "v01.cbz", "h1", 1)], + want_to_read: None, }; let doc = document(vec![series_doc], false); @@ -845,6 +850,7 @@ fn series_doc(path: &str, name: &str, books: Vec) -> ExportSeries notes: None, rating_updated_at: None, books, + want_to_read: None, } } @@ -1199,6 +1205,7 @@ fn series_doc_with_ids(ids: Vec<(&str, &str)>) -> ExportSeriesDto { notes: None, rating_updated_at: Some(Utc::now()), books: vec![], + want_to_read: None, } } @@ -1420,6 +1427,7 @@ async fn scoping_the_import_to_a_library_resolves_a_duplicate_that_is_otherwise_ notes: None, rating_updated_at: Some(Utc::now()), books: vec![], + want_to_read: None, }; // Unscoped: both copies compete. @@ -1611,7 +1619,9 @@ async fn rows_on_a_live_book_elsewhere_are_reported_as_stranded() { client_ended_at: now, server_recorded_at: now, }]), + want_to_read: None, }], + want_to_read: None, }; let app = create_test_router(state.clone()).await; @@ -1633,3 +1643,432 @@ async fn rows_on_a_live_book_elsewhere_are_reported_as_stranded() { report.notices ); } + +// --------------------------------------------------------------------------- +// Reporting coverage of the destination. +// +// A split exports a whole old library and imports it into one of several new +// ones, so most file series are *meant* to miss. Counting them as unmatched +// made a complete import read like a failure. A scoped import instead reports +// how much of the destination it covered. +// --------------------------------------------------------------------------- + +/// A live series with one live book, in the given library. +async fn live_series(db: &DatabaseConnection, library_id: Uuid, root: &str, name: &str) -> Uuid { + let series = SeriesRepository::create(db, library_id, name, None) + .await + .unwrap(); + BookRepository::create( + db, + &book_model( + series.id, + library_id, + &format!("/{root}/{name}/v01.cbz"), + "v01.cbz", + "", + ), + None, + ) + .await + .unwrap(); + series.id +} + +fn named_series_doc(name: &str, relative_path: &str) -> ExportSeriesDto { + ExportSeriesDto { + external_ids: vec![], + library_relative_path: relative_path.to_string(), + name: name.to_string(), + rating: None, + notes: None, + rating_updated_at: None, + books: vec![], + want_to_read: None, + } +} + +async fn preview( + state: &std::sync::Arc, + token: &str, + series: Vec, + library_ids: Option>, +) -> ImportReadingProgressResponse { + let app = create_test_router(state.clone()).await; + let mut body = import_request(document(series, false), true); + body.library_ids = library_ids; + let request = post_json_request_with_auth("/api/v1/reading-progress/import", &body, token); + let (status, response): (StatusCode, Option) = + make_json_request(app, request).await; + assert_eq!(status, StatusCode::OK); + response.expect("import response") +} + +/// The shape of the user's first real import: the file covers far more than +/// the destination, and every destination series matched. +#[tokio::test] +async fn a_scoped_import_reports_coverage_of_the_destination() { + let (db, _tmp) = setup_test_db().await; + let state = create_test_auth_state(db.clone()).await; + let (_user, token) = admin_and_token(&db, &state, "coverage").await; + + let shonen = LibraryRepository::create(&db, "Shonen", "/shonen", ScanningStrategy::Default) + .await + .unwrap(); + let other = LibraryRepository::create(&db, "Other", "/other", ScanningStrategy::Default) + .await + .unwrap(); + live_series(&db, shonen.id, "shonen", "Naruto").await; + live_series(&db, shonen.id, "shonen", "Bleach").await; + live_series(&db, other.id, "other", "Monster").await; + + let report = preview( + &state, + &token, + vec![ + named_series_doc("Naruto", "Naruto"), + named_series_doc("Bleach", "Bleach"), + named_series_doc("Monster", "Monster"), + named_series_doc("Nowhere At All", "Nowhere At All"), + ], + Some(vec![shonen.id]), + ) + .await; + + assert_eq!(report.summary.series_in_selected_libraries, Some(2)); + assert_eq!(report.summary.series_matched_distinct, 2); + assert_eq!(report.summary.books_in_selected_libraries, Some(2)); +} + +/// Two file series can resolve to one destination series by name. Counting +/// file matches would then report more matches than the destination holds, +/// so coverage counts distinct destination series. +#[tokio::test] +async fn two_file_series_landing_on_one_destination_series_count_once() { + let (db, _tmp) = setup_test_db().await; + let state = create_test_auth_state(db.clone()).await; + let (_user, token) = admin_and_token(&db, &state, "distinct").await; + + let lib = LibraryRepository::create(&db, "Lib", "/lib", ScanningStrategy::Default) + .await + .unwrap(); + live_series(&db, lib.id, "lib", "Naruto").await; + + let report = preview( + &state, + &token, + vec![ + named_series_doc("Naruto", "shonen/Naruto"), + named_series_doc("Naruto", "old/Naruto"), + ], + Some(vec![lib.id]), + ) + .await; + + assert_eq!(report.summary.series_matched, 2); + assert_eq!(report.summary.series_matched_distinct, 1); + assert_eq!(report.summary.series_in_selected_libraries, Some(1)); +} + +/// Unscoped, there is no meaningful destination to measure against: the +/// denominator would be every series the reader can see. +#[tokio::test] +async fn an_unscoped_import_has_no_destination_totals() { + let (db, _tmp) = setup_test_db().await; + let state = create_test_auth_state(db.clone()).await; + let (_user, token) = admin_and_token(&db, &state, "unscoped").await; + + let lib = LibraryRepository::create(&db, "Lib", "/lib", ScanningStrategy::Default) + .await + .unwrap(); + live_series(&db, lib.id, "lib", "Naruto").await; + + let report = preview( + &state, + &token, + vec![named_series_doc("Naruto", "Naruto")], + None, + ) + .await; + + assert_eq!(report.summary.series_in_selected_libraries, None); + assert_eq!(report.summary.books_in_selected_libraries, None); + assert_eq!(report.summary.series_matched_distinct, 1); +} + +// --------------------------------------------------------------------------- +// Want-to-read. +// +// The queue is per-user and keyed by series or book, so it rides the match the +// import already performs. The case that matters most is the one that is easy +// to lose: a series queued but never started has no reading state at all, so +// an export seeded only from reading state would never carry it. +// --------------------------------------------------------------------------- + +use codex::db::entities::want_to_read; +use codex::db::repositories::WantToReadRepository; +use codex::models::sort::WantToReadSort; + +async fn queue_for(db: &DatabaseConnection, user: Uuid) -> Vec { + WantToReadRepository::list(db, user, WantToReadSort::Custom) + .await + .unwrap() +} + +async fn export_doc( + state: &std::sync::Arc, + token: &str, +) -> ReadingProgressExportDocument { + let app = create_test_router(state.clone()).await; + let request = get_request_with_auth("/api/v1/reading-progress/export", token); + let (status, doc): (StatusCode, Option) = + make_json_request(app, request).await; + assert_eq!(status, StatusCode::OK); + doc.expect("export document") +} + +async fn import_doc( + state: &std::sync::Arc, + token: &str, + doc: ReadingProgressExportDocument, + dry_run: bool, + restore: bool, +) -> ImportReadingProgressResponse { + let app = create_test_router(state.clone()).await; + let mut body = import_request(doc, dry_run); + body.restore_want_to_read = restore; + let request = post_json_request_with_auth("/api/v1/reading-progress/import", &body, token); + let (status, response): (StatusCode, Option) = + make_json_request(app, request).await; + assert_eq!(status, StatusCode::OK); + response.expect("import response") +} + +/// The ordinary want-to-read case: queued, never opened. With no progress, +/// completion, session or rating, nothing else would put it in the file. +#[tokio::test] +async fn a_queued_series_with_no_reading_state_is_exported() { + let (db, _tmp) = setup_test_db().await; + let state = create_test_auth_state(db.clone()).await; + let (user, token) = admin_and_token(&db, &state, "wtr-series").await; + let lib = LibraryRepository::create(&db, "Lib", "/lib", ScanningStrategy::Default) + .await + .unwrap(); + let series = live_series(&db, lib.id, "lib", "Unstarted").await; + WantToReadRepository::add_series(&db, user, series) + .await + .unwrap(); + + let doc = export_doc(&state, &token).await; + + let exported = doc + .series + .iter() + .find(|s| s.name == "Unstarted") + .expect("a queued series with no reading state must still be exported"); + assert!(exported.want_to_read.is_some()); +} + +/// A book can be queued on its own. It has to travel with its series so the +/// import can match it, even though nothing else was recorded against either. +#[tokio::test] +async fn a_queued_book_is_exported_with_its_series() { + let (db, _tmp) = setup_test_db().await; + let state = create_test_auth_state(db.clone()).await; + let (user, token) = admin_and_token(&db, &state, "wtr-book").await; + let lib = LibraryRepository::create(&db, "Lib", "/lib", ScanningStrategy::Default) + .await + .unwrap(); + let series = live_series(&db, lib.id, "lib", "Anthology").await; + let book = books::Entity::find() + .filter(books::Column::SeriesId.eq(series)) + .one(&db) + .await + .unwrap() + .expect("the series has a book"); + WantToReadRepository::add_book(&db, user, book.id) + .await + .unwrap(); + + let doc = export_doc(&state, &token).await; + + let exported = doc + .series + .iter() + .find(|s| s.name == "Anthology") + .expect("the queued book's series must be exported"); + assert!( + exported.want_to_read.is_none(), + "the book was queued, not the series" + ); + assert_eq!(exported.books.len(), 1); + assert!(exported.books[0].want_to_read.is_some()); +} + +/// Imported entries go after the reader's existing queue, in their original +/// relative order, and nothing already queued moves. +#[tokio::test] +async fn restored_entries_follow_the_existing_queue_in_original_order() { + let (db, _tmp) = setup_test_db().await; + let state = create_test_auth_state(db.clone()).await; + let (user, token) = admin_and_token(&db, &state, "wtr-order").await; + let lib = LibraryRepository::create(&db, "Lib", "/lib", ScanningStrategy::Default) + .await + .unwrap(); + let already_queued = live_series(&db, lib.id, "lib", "Already Queued").await; + let second = live_series(&db, lib.id, "lib", "Second").await; + let first = live_series(&db, lib.id, "lib", "First").await; + WantToReadRepository::add_series(&db, user, already_queued) + .await + .unwrap(); + + // Listed out of order in the file; the original positions say First comes + // before Second. + let queued_on = Utc::now() - Duration::days(30); + let mut second_doc = named_series_doc("Second", "Second"); + second_doc.want_to_read = Some(ExportWantToReadDto { + position: 9, + added_at: queued_on, + }); + let mut first_doc = named_series_doc("First", "First"); + first_doc.want_to_read = Some(ExportWantToReadDto { + position: 4, + added_at: queued_on, + }); + + let report = import_doc( + &state, + &token, + document(vec![second_doc, first_doc], false), + false, + true, + ) + .await; + assert_eq!(report.summary.want_to_read_restored, 2); + + let queue = queue_for(&db, user).await; + let order: Vec = queue.iter().filter_map(|e| e.series_id).collect(); + assert_eq!(order, vec![already_queued, first, second]); + + // The original date survives, so the newest/oldest sorts stay truthful. + let restored = queue.iter().find(|e| e.series_id == Some(first)).unwrap(); + assert_eq!(restored.added_at.timestamp(), queued_on.timestamp()); +} + +/// Row ids are not reused for the queue, so idempotency comes from the entry +/// itself: a series already queued is left exactly where it is. +#[tokio::test] +async fn reimporting_does_not_duplicate_the_queue() { + let (db, _tmp) = setup_test_db().await; + let state = create_test_auth_state(db.clone()).await; + let (user, token) = admin_and_token(&db, &state, "wtr-twice").await; + let lib = LibraryRepository::create(&db, "Lib", "/lib", ScanningStrategy::Default) + .await + .unwrap(); + live_series(&db, lib.id, "lib", "Once").await; + + let mut doc_series = named_series_doc("Once", "Once"); + doc_series.want_to_read = Some(ExportWantToReadDto { + position: 0, + added_at: Utc::now(), + }); + let doc = document(vec![doc_series], false); + + import_doc(&state, &token, doc.clone(), false, true).await; + let second = import_doc(&state, &token, doc, false, true).await; + + assert_eq!(queue_for(&db, user).await.len(), 1); + assert_eq!(second.summary.want_to_read_restored, 0); +} + +#[tokio::test] +async fn restoring_want_to_read_can_be_switched_off() { + let (db, _tmp) = setup_test_db().await; + let state = create_test_auth_state(db.clone()).await; + let (user, token) = admin_and_token(&db, &state, "wtr-off").await; + let lib = LibraryRepository::create(&db, "Lib", "/lib", ScanningStrategy::Default) + .await + .unwrap(); + live_series(&db, lib.id, "lib", "Skipped").await; + + let mut doc_series = named_series_doc("Skipped", "Skipped"); + doc_series.want_to_read = Some(ExportWantToReadDto { + position: 0, + added_at: Utc::now(), + }); + + let report = import_doc( + &state, + &token, + document(vec![doc_series], false), + false, + false, + ) + .await; + + assert!(queue_for(&db, user).await.is_empty()); + assert_eq!(report.summary.want_to_read_restored, 0); +} + +/// A dry run reports what it would restore and writes nothing, like every +/// other count in the report. +#[tokio::test] +async fn a_dry_run_reports_want_to_read_but_writes_nothing() { + let (db, _tmp) = setup_test_db().await; + let state = create_test_auth_state(db.clone()).await; + let (user, token) = admin_and_token(&db, &state, "wtr-dry").await; + let lib = LibraryRepository::create(&db, "Lib", "/lib", ScanningStrategy::Default) + .await + .unwrap(); + live_series(&db, lib.id, "lib", "Previewed").await; + + let mut doc_series = named_series_doc("Previewed", "Previewed"); + doc_series.want_to_read = Some(ExportWantToReadDto { + position: 0, + added_at: Utc::now(), + }); + + let report = import_doc( + &state, + &token, + document(vec![doc_series], false), + true, + true, + ) + .await; + + assert_eq!(report.summary.want_to_read_restored, 1); + assert!(queue_for(&db, user).await.is_empty()); +} + +/// `booksUnmatched` mixes books whose whole series belongs elsewhere with +/// books missed inside a series that matched. Only the first is reassuring, +/// so the report has to keep them apart. +#[tokio::test] +async fn books_missed_inside_a_matched_series_are_kept_apart_from_other_libraries() { + let (db, _tmp) = setup_test_db().await; + let state = create_test_auth_state(db.clone()).await; + let (_user, token) = admin_and_token(&db, &state, "book-split").await; + let lib = LibraryRepository::create(&db, "Lib", "/lib", ScanningStrategy::Default) + .await + .unwrap(); + live_series(&db, lib.id, "lib", "Here").await; + + let book = |path: &str| minimal_book_doc(path, path, "", 1); + + // A matched series: one book matches, one is genuinely missing. + let mut here = named_series_doc("Here", "Here"); + here.books = vec![book("v01.cbz"), book("v99-missing.cbz")]; + // A series that belongs to another library entirely. + let mut elsewhere = named_series_doc("Elsewhere", "Elsewhere"); + elsewhere.books = vec![book("a.cbz"), book("b.cbz")]; + + let report = preview(&state, &token, vec![here, elsewhere], Some(vec![lib.id])).await; + + assert_eq!(report.summary.books_unmatched, 3); + assert_eq!(report.summary.books_in_unmatched_series, 2); + // What is left is the one that deserves attention. + assert_eq!( + report.summary.books_unmatched - report.summary.books_in_unmatched_series, + 1 + ); +} diff --git a/web/openapi.json b/web/openapi.json index 3b9e31fd..1c8688c9 100644 --- a/web/openapi.json +++ b/web/openapi.json @@ -30709,6 +30709,10 @@ "$ref": "#/components/schemas/ExportSessionDto" }, "description": "Omitted entirely (not an empty array) when the export was taken with\n`includeSessions=false`." + }, + "wantToRead": { + "$ref": "#/components/schemas/ExportWantToReadDto", + "description": "Present when this book, rather than its whole series, is queued." } } }, @@ -30948,6 +30952,10 @@ ], "format": "date-time", "description": "When the rating was last changed. The `newest` conflict policy needs it\nto tell a stale rating from a fresh one; a file without it never\noverwrites an existing rating except under `overwrite`." + }, + "wantToRead": { + "$ref": "#/components/schemas/ExportWantToReadDto", + "description": "Present when the whole series is queued. A queued series is often one\nthe reader never started, so it can appear with no books at all." } } }, @@ -31034,6 +31042,24 @@ } } }, + "ExportWantToReadDto": { + "type": "object", + "description": "A want-to-read queue entry, attached to the series or book it flags.\n\n`position` is carried so imported entries keep their original relative\norder: the queue is ordered globally across series and books, and that\norder is otherwise lost when the file groups entries by series.\n`added_at` is carried because the queue's newest and oldest sorts read it,\nand stamping the import time would reorder both views by when the import\nhappened to run.", + "required": [ + "position", + "addedAt" + ], + "properties": { + "addedAt": { + "type": "string", + "format": "date-time" + }, + "position": { + "type": "integer", + "format": "int32" + } + } + }, "ExternalIdContextDto": { "type": "object", "description": "External ID context for template evaluation.\n\nRepresents an external ID from a metadata provider (plugin, comicinfo, etc.)\nin a simplified format suitable for template access.", @@ -32594,6 +32620,10 @@ "type": "boolean", "description": "When a session or completion in the file already exists as the\nimporter's own row but is not on a live book (its book was hard-deleted,\nleaving `book_id` null, or the scanner marked it deleted after the file\nmoved), move it onto the matched book instead of skipping it." }, + "restoreWantToRead": { + "type": "boolean", + "description": "Put queued series and books back into want-to-read. On by default:\ncarrying the queue across a split is the reason it is exported.\nRestored entries land after anything already queued, in their original\nrelative order, and an entry already queued is left where it is." + }, "sourcePreference": { "type": [ "array", @@ -32714,7 +32744,11 @@ "completionsReattached", "sessionsInserted", "sessionsReattached", - "rowsStranded" + "rowsStranded", + "seriesMatchedDistinct", + "booksMatchedDistinct", + "booksInUnmatchedSeries", + "wantToReadRestored" ], "properties": { "booksAmbiguous": { @@ -32727,11 +32761,32 @@ "format": "int32", "minimum": 0 }, + "booksInSelectedLibraries": { + "type": [ + "integer", + "null" + ], + "format": "int32", + "description": "Live books in the scoped libraries. `None` when not scoped.", + "minimum": 0 + }, + "booksInUnmatchedSeries": { + "type": "integer", + "format": "int32", + "description": "Books counted in `books_unmatched` only because their whole series did\nnot match.\n\n`books_unmatched` mixes two outcomes. In a scoped import these are\nbooks belonging to other libraries, which is expected. The remainder,\n`books_unmatched - books_in_unmatched_series`, are books missed inside\na series that *did* match, which is worth a reader's attention and\nmust not be hidden under the reassuring label.", + "minimum": 0 + }, "booksMatched": { "type": "integer", "format": "int32", "minimum": 0 }, + "booksMatchedDistinct": { + "type": "integer", + "format": "int32", + "description": "Distinct destination books matched, for the same reason.", + "minimum": 0 + }, "booksStemMatched": { "type": "integer", "format": "int32", @@ -32783,11 +32838,26 @@ "format": "int32", "minimum": 0 }, + "seriesInSelectedLibraries": { + "type": [ + "integer", + "null" + ], + "format": "int32", + "description": "Live series in the libraries the import was scoped to. `None` when the\nimport was not scoped: the denominator would then be every series the\nreader can see, which measures nothing.\n\nWith this set, a file series that did not match is almost always one\nthat belongs to another library, which is expected when a split\nimports into one of several new libraries, not a matching failure.", + "minimum": 0 + }, "seriesMatched": { "type": "integer", "format": "int32", "minimum": 0 }, + "seriesMatchedDistinct": { + "type": "integer", + "format": "int32", + "description": "Distinct destination series that at least one file series resolved to.\n\n`series_matched` counts file series, and two of those can resolve to\none destination series by name, so it can exceed what the destination\nholds. Coverage of the destination has to count this instead.", + "minimum": 0 + }, "seriesTotal": { "type": "integer", "format": "int32", @@ -32807,6 +32877,12 @@ "type": "integer", "format": "int32", "minimum": 0 + }, + "wantToReadRestored": { + "type": "integer", + "format": "int32", + "description": "Queue entries put back into want-to-read.", + "minimum": 0 } } }, diff --git a/web/src/pages/settings/ReadingProgressTransferSettings.test.tsx b/web/src/pages/settings/ReadingProgressTransferSettings.test.tsx index 31a07d12..a0760dcc 100644 --- a/web/src/pages/settings/ReadingProgressTransferSettings.test.tsx +++ b/web/src/pages/settings/ReadingProgressTransferSettings.test.tsx @@ -1,10 +1,14 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import type { ImportReadingProgressResponse, + ImportSummary, ReadingProgressExportDocument, } from "@/api/readingProgressTransfer"; import { renderWithProviders, screen, userEvent, waitFor } from "@/test/utils"; -import { ReadingProgressTransferSettings } from "./ReadingProgressTransferSettings"; +import { + ReadingProgressTransferSettings, + SummaryLine, +} from "./ReadingProgressTransferSettings"; const exportProgress = vi.fn(); const importProgress = vi.fn(); @@ -37,7 +41,7 @@ const exportDocument: ReadingProgressExportDocument = { path: "v01.cbz", fileName: "v01.cbz", fileHash: "", - partial_hash: "", + partialHash: "", completions: [], }, ], @@ -45,36 +49,57 @@ const exportDocument: ReadingProgressExportDocument = { ], }; -function dryRunResponse(): ImportReadingProgressResponse { +/** + * Builds a summary with every field present. Test files are excluded from + * `tsc -b`, so a stale key here would not fail the type check: this fixture + * sat in snake_case for a release after the wire format became camelCase, + * and passed only because nothing asserted a rendered count. The tests below + * assert rendered numbers, which is what catches that at run time. + */ +function summary(overrides: Partial = {}): ImportSummary { + return { + seriesTotal: 1, + seriesMatched: 1, + seriesAmbiguous: 0, + seriesUnmatched: 0, + seriesCommitted: 0, + booksTotal: 1, + booksMatched: 1, + booksStemMatched: 0, + booksAmbiguous: 0, + booksUnmatched: 0, + booksHashMismatch: 0, + progressWritten: 1, + ratingsWritten: 0, + completionsInserted: 0, + completionsReattached: 0, + sessionsInserted: 0, + sessionsReattached: 0, + rowsStranded: 0, + seriesMatchedDistinct: 1, + booksMatchedDistinct: 1, + seriesInSelectedLibraries: null, + booksInSelectedLibraries: null, + booksInUnmatchedSeries: 0, + wantToReadRestored: 0, + ...overrides, + }; +} + +function dryRunResponse( + overrides: Partial = {}, +): ImportReadingProgressResponse { return { dryRun: true, - sessions_in_file: true, + sessionsInFile: true, notices: [], - summary: { - series_total: 1, - series_matched: 1, - series_ambiguous: 0, - series_unmatched: 0, - series_committed: 0, - books_total: 1, - books_matched: 1, - books_stem_matched: 0, - books_ambiguous: 0, - books_unmatched: 0, - books_hash_mismatch: 0, - progress_written: 1, - ratings_written: 0, - completions_inserted: 0, - completions_reattached: 0, - sessions_inserted: 0, - sessions_reattached: 0, - }, + summary: summary(overrides), series: [ { libraryRelativePath: "Naruto", name: "Naruto", disposition: "matched", - matched_series_id: "11111111-1111-1111-1111-111111111111", + matchedSeriesId: "11111111-1111-1111-1111-111111111111", attempted: true, committed: false, books: [ @@ -82,11 +107,16 @@ function dryRunResponse(): ImportReadingProgressResponse { path: "v01.cbz", fileName: "v01.cbz", disposition: "matched", - matched_book_id: "22222222-2222-2222-2222-222222222222", + matchedBookId: "22222222-2222-2222-2222-222222222222", applied: true, progress: "inserted", - completions: { inserted: 0, reattached: 0, skipped: 0 }, - sessions: { inserted: 0, reattached: 0, skipped: 0 }, + completions: { + inserted: 0, + reattached: 0, + skipped: 0, + stranded: 0, + }, + sessions: { inserted: 0, reattached: 0, skipped: 0, stranded: 0 }, }, ], }, @@ -315,4 +345,83 @@ describe("ReadingProgressTransferSettings", () => { screen.getByRole("button", { name: /apply import/i }), ).toBeDisabled(); }); + + describe("report wording", () => { + // The shape of the first real import: the file covered far more than the + // destination, and every destination series matched. + const scoped = dryRunResponse({ + seriesMatched: 32, + seriesMatchedDistinct: 32, + seriesUnmatched: 244, + seriesInSelectedLibraries: 32, + booksMatched: 1838, + booksMatchedDistinct: 1838, + booksUnmatched: 3183, + booksInUnmatchedSeries: 3121, + booksInSelectedLibraries: 1900, + }); + + it("states a scoped import as coverage of the destination", () => { + renderWithProviders(); + const text = document.body.textContent ?? ""; + + expect(text).toContain("Series: 32 of the 32 in Shonen matched"); + expect(text).toContain("Books: 1838 of the 1900 in Shonen matched"); + expect(text).toContain( + "244 series (3121 books) that belong to other libraries", + ); + expect(text).not.toMatch(/unmatched/); + }); + + it("keeps books missed inside a matched series visible", () => { + // 3183 unmatched, 3121 of them in series that live elsewhere: the other + // 62 were missed inside series that did match, and must not be folded + // into the reassuring note. + renderWithProviders(); + + expect(document.body.textContent).toContain("62 missed"); + }); + + it("keeps the plain counts when the import was not scoped", () => { + const unscoped = dryRunResponse({ seriesUnmatched: 4 }); + renderWithProviders(); + const text = document.body.textContent ?? ""; + + expect(text).toContain("4 unmatched"); + expect(text).not.toMatch(/belong to other libraries/); + }); + }); + + describe("restoring want to read", () => { + it("is requested by default", async () => { + importProgress.mockResolvedValue(dryRunResponse()); + const user = userEvent.setup(); + renderWithProviders(); + + await uploadDocument(user); + await user.click( + screen.getByRole("button", { name: /preview \(dry run\)/i }), + ); + + await waitFor(() => expect(importProgress).toHaveBeenCalled()); + expect(importProgress.mock.calls[0][0].restoreWantToRead).toBe(true); + }); + + it("can be switched off", async () => { + importProgress.mockResolvedValue(dryRunResponse()); + const user = userEvent.setup(); + renderWithProviders(); + + await uploadDocument(user); + await user.click( + screen.getByRole("checkbox", { name: /restore want to read/i }), + ); + await user.click( + screen.getByRole("button", { name: /preview \(dry run\)/i }), + ); + + await waitFor(() => expect(importProgress).toHaveBeenCalled()); + expect(importProgress.mock.calls[0][0].restoreWantToRead).toBe(false); + }); + }); }); diff --git a/web/src/pages/settings/ReadingProgressTransferSettings.tsx b/web/src/pages/settings/ReadingProgressTransferSettings.tsx index 24e75e73..c7ac62af 100644 --- a/web/src/pages/settings/ReadingProgressTransferSettings.tsx +++ b/web/src/pages/settings/ReadingProgressTransferSettings.tsx @@ -97,36 +97,92 @@ function bookDispositionColor(disposition: BookDisposition): string { } } -function SummaryLine({ report }: { report: ImportReadingProgressResponse }) { +/** + * The import's headline counts. + * + * Scoped, the counts are stated in terms of the destination: a split exports a + * whole old library and imports it into one of several new ones, so most file + * series are meant to miss, and reporting them as "unmatched" made a complete + * import read like a failure. Unscoped there is no destination to measure + * against, and an unmatched series really was not found, so the plain counts + * stay. + */ +export function SummaryLine({ + report, + scopeLabel, +}: { + report: ImportReadingProgressResponse; + /** Name of the one selected library, or a phrase for several; null if unscoped. */ + scopeLabel: string | null; +}) { const { summary } = report; + const committed = !report.dryRun && ( + <> + , {summary.seriesCommitted} committed + + ); + const writes = ( + + Progress written: {summary.progressWritten} · Completions:{" "} + {summary.completionsInserted} inserted /{" "} + {summary.completionsReattached} reattached · Sessions:{" "} + {summary.sessionsInserted} inserted /{" "} + {summary.sessionsReattached} reattached · Ratings:{" "} + {summary.ratingsWritten} · Want to read:{" "} + {summary.wantToReadRestored} + + ); + + const seriesInScope = summary.seriesInSelectedLibraries; + const booksInScope = summary.booksInSelectedLibraries; + if (scopeLabel === null || seriesInScope == null || booksInScope == null) { + return ( + + + Series: {summary.seriesMatched} matched,{" "} + {summary.seriesAmbiguous} ambiguous,{" "} + {summary.seriesUnmatched} unmatched + {committed} + + + Books: {summary.booksMatched} matched,{" "} + {summary.booksStemMatched} stem match,{" "} + {summary.booksAmbiguous} ambiguous,{" "} + {summary.booksUnmatched} unmatched,{" "} + {summary.booksHashMismatch} hash mismatch + + {writes} + + ); + } + + // Books missed inside a series that did match are a real gap and stay + // visible; only books whose whole series lives elsewhere are set aside. + const booksMissed = summary.booksUnmatched - summary.booksInUnmatchedSeries; return ( - - - Series: {summary.seriesMatched} matched,{" "} - {summary.seriesAmbiguous} ambiguous,{" "} - {summary.seriesUnmatched} unmatched - {!report.dryRun && ( - <> - , {summary.seriesCommitted} committed - - )} - - - Books: {summary.booksMatched} matched,{" "} - {summary.booksStemMatched} stem match,{" "} - {summary.booksAmbiguous} ambiguous,{" "} - {summary.booksUnmatched} unmatched,{" "} - {summary.booksHashMismatch} hash mismatch - - - Progress written: {summary.progressWritten} · Completions:{" "} - {summary.completionsInserted} inserted /{" "} - {summary.completionsReattached} reattached · Sessions:{" "} - {summary.sessionsInserted} inserted /{" "} - {summary.sessionsReattached} reattached · Ratings:{" "} - {summary.ratingsWritten} + + + + Series: {summary.seriesMatchedDistinct} of the{" "} + {seriesInScope} in {scopeLabel} matched,{" "} + {summary.seriesAmbiguous} ambiguous + {committed} + + + Books: {summary.booksMatchedDistinct} of the{" "} + {booksInScope} in {scopeLabel} matched, {booksMissed}{" "} + missed, {summary.booksStemMatched} stem match,{" "} + {summary.booksAmbiguous} ambiguous,{" "} + {summary.booksHashMismatch} hash mismatch + + {writes} + + + The file also holds {summary.seriesUnmatched} series ( + {summary.booksInUnmatchedSeries} books) that belong to other libraries. + That is expected when importing part of a split. - + ); } @@ -222,6 +278,7 @@ export function ReadingProgressTransferSettings() { const [exportLibraryIds, setExportLibraryIds] = useState([]); const [importLibraryIds, setImportLibraryIds] = useState([]); const [sourcePreference, setSourcePreference] = useState([]); + const [restoreWantToRead, setRestoreWantToRead] = useState(true); const [file, setFile] = useState(null); const [parsedDocument, setParsedDocument] = @@ -256,6 +313,14 @@ export function ReadingProgressTransferSettings() { return [...seen]; }, [parsedDocument]); + const scopeLabel = + importLibraryIds.length === 0 + ? null + : importLibraryIds.length === 1 + ? (libraries.find((library) => library.id === importLibraryIds[0]) + ?.name ?? "the selected library") + : "the selected libraries"; + const exportMutation = useExportReadingProgress(); const importMutation = useImportReadingProgress(); @@ -291,6 +356,7 @@ export function ReadingProgressTransferSettings() { sourcePreference: sourcePreference.length > 0 ? sourcePreference : undefined, libraryIds: importLibraryIds.length > 0 ? importLibraryIds : undefined, + restoreWantToRead, conflictPolicy, reattachSessions, acceptStemMatches, @@ -507,6 +573,17 @@ export function ReadingProgressTransferSettings() { }} /> + { + setRestoreWantToRead(event.currentTarget.checked); + clearPreview(); + }} + /> + {importError && ( ))} - + )} diff --git a/web/src/types/api.generated.ts b/web/src/types/api.generated.ts index a7683e09..1a7202c6 100644 --- a/web/src/types/api.generated.ts +++ b/web/src/types/api.generated.ts @@ -12310,6 +12310,8 @@ export interface components { * `includeSessions=false`. */ sessions?: components["schemas"]["ExportSessionDto"][] | null; + /** @description Present when this book, rather than its whole series, is queued. */ + wantToRead?: components["schemas"]["ExportWantToReadDto"]; }; /** * @description One finished read-through. Keeps its original id so re-importing the same @@ -12418,6 +12420,11 @@ export interface components { * overwrites an existing rating except under `overwrite`. */ ratingUpdatedAt?: string | null; + /** + * @description Present when the whole series is queued. A queued series is often one + * the reader never started, so it can appear with no books at all. + */ + wantToRead?: components["schemas"]["ExportWantToReadDto"]; }; /** * @description One row from the reading-session log. `r2_progression` is deliberately @@ -12456,6 +12463,22 @@ export interface components { /** Format: double */ toPercentage?: number | null; }; + /** + * @description A want-to-read queue entry, attached to the series or book it flags. + * + * `position` is carried so imported entries keep their original relative + * order: the queue is ordered globally across series and books, and that + * order is otherwise lost when the file groups entries by series. + * `added_at` is carried because the queue's newest and oldest sorts read it, + * and stamping the import time would reorder both views by when the import + * happened to run. + */ + ExportWantToReadDto: { + /** Format: date-time */ + addedAt: string; + /** Format: int32 */ + position: number; + }; /** * @description External ID context for template evaluation. * @@ -13325,6 +13348,13 @@ export interface components { * moved), move it onto the matched book instead of skipping it. */ reattachSessions?: boolean; + /** + * @description Put queued series and books back into want-to-read. On by default: + * carrying the queue across a split is the reason it is exported. + * Restored entries land after anything already queued, in their original + * relative order, and an entry already queued is left where it is. + */ + restoreWantToRead?: boolean; /** * @description External-id sources to try, in order, before falling back to path and * then normalized name. @@ -13381,8 +13411,30 @@ export interface components { booksAmbiguous: number; /** Format: int32 */ booksHashMismatch: number; + /** + * Format: int32 + * @description Live books in the scoped libraries. `None` when not scoped. + */ + booksInSelectedLibraries?: number | null; + /** + * Format: int32 + * @description Books counted in `books_unmatched` only because their whole series did + * not match. + * + * `books_unmatched` mixes two outcomes. In a scoped import these are + * books belonging to other libraries, which is expected. The remainder, + * `books_unmatched - books_in_unmatched_series`, are books missed inside + * a series that *did* match, which is worth a reader's attention and + * must not be hidden under the reassuring label. + */ + booksInUnmatchedSeries: number; /** Format: int32 */ booksMatched: number; + /** + * Format: int32 + * @description Distinct destination books matched, for the same reason. + */ + booksMatchedDistinct: number; /** Format: int32 */ booksStemMatched: number; /** Format: int32 */ @@ -13407,8 +13459,28 @@ export interface components { seriesAmbiguous: number; /** Format: int32 */ seriesCommitted: number; + /** + * Format: int32 + * @description Live series in the libraries the import was scoped to. `None` when the + * import was not scoped: the denominator would then be every series the + * reader can see, which measures nothing. + * + * With this set, a file series that did not match is almost always one + * that belongs to another library, which is expected when a split + * imports into one of several new libraries, not a matching failure. + */ + seriesInSelectedLibraries?: number | null; /** Format: int32 */ seriesMatched: number; + /** + * Format: int32 + * @description Distinct destination series that at least one file series resolved to. + * + * `series_matched` counts file series, and two of those can resolve to + * one destination series by name, so it can exceed what the destination + * holds. Coverage of the destination has to count this instead. + */ + seriesMatchedDistinct: number; /** Format: int32 */ seriesTotal: number; /** Format: int32 */ @@ -13417,6 +13489,11 @@ export interface components { sessionsInserted: number; /** Format: int32 */ sessionsReattached: number; + /** + * Format: int32 + * @description Queue entries put back into want-to-read. + */ + wantToReadRestored: number; }; /** * @description Which layer supplied a value the user is inheriting.