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.