diff --git a/crates/codex-api/src/routes/v1/dto/reading_progress_transfer.rs b/crates/codex-api/src/routes/v1/dto/reading_progress_transfer.rs index 7ca26bb4..5a21cea0 100644 --- a/crates/codex-api/src/routes/v1/dto/reading_progress_transfer.rs +++ b/crates/codex-api/src/routes/v1/dto/reading_progress_transfer.rs @@ -10,6 +10,7 @@ pub use codex_services::reading_transfer::model::*; use serde::Deserialize; use utoipa::ToSchema; +use uuid::Uuid; fn default_true() -> bool { true @@ -24,12 +25,39 @@ pub struct ExportReadingProgressQuery { /// notice until the numbers are gone. #[serde(default = "default_true")] pub include_sessions: bool, + + /// Comma-separated library ids to export, e.g. + /// `?libraryIds=,`. Omitted exports every library the reader + /// has state for. + /// + /// Comma-separated rather than a repeated key because axum's `Query` + /// extractor deserializes with `serde_urlencoded`, which collapses a + /// repeated key instead of collecting it into a `Vec`. + pub library_ids: Option, +} + +impl ExportReadingProgressQuery { + /// Parse `library_ids`, rejecting anything that is not a uuid rather than + /// silently exporting more than the caller asked for. + pub fn parsed_library_ids(&self) -> Result>, uuid::Error> { + let Some(raw) = self.library_ids.as_deref() else { + return Ok(None); + }; + let ids = raw + .split(',') + .map(str::trim) + .filter(|part| !part.is_empty()) + .map(Uuid::parse_str) + .collect::, _>>()?; + Ok(Some(ids)) + } } impl Default for ExportReadingProgressQuery { fn default() -> Self { Self { include_sessions: true, + library_ids: None, } } } 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 4da111cf..93aa2c91 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 @@ -40,7 +40,8 @@ use codex_services::reading_transfer::import::{ImportError, ImportOptions}; get, path = "/api/v1/reading-progress/export", params( - ("includeSessions" = Option, Query, description = "Include the reading-session log (default: true). Sessions are the only source of every reading statistic, so this is opt-out rather than opt-in.") + ("includeSessions" = Option, Query, description = "Include the reading-session log (default: true). Sessions are the only source of every reading statistic, so this is opt-out rather than opt-in."), + ("libraryIds" = Option, Query, description = "Comma-separated library ids to export. Omitted exports every library the reader has state for.") ), responses( (status = 200, description = "The export document", body = codex_services::reading_transfer::model::ReadingProgressExportDocument), @@ -60,10 +61,18 @@ pub async fn export_reading_progress( ) -> Result { auth.require_permission(&Permission::ProgressRead)?; - let document = - codex_services::export_reading_progress(&state.db, auth.user_id, query.include_sessions) - .await - .map_err(|e| ApiError::Internal(format!("Failed to export reading progress: {e}")))?; + let library_ids = query + .parsed_library_ids() + .map_err(|e| ApiError::BadRequest(format!("Invalid libraryIds: {e}")))?; + + let document = codex_services::export_reading_progress( + &state.db, + auth.user_id, + query.include_sessions, + library_ids.as_deref(), + ) + .await + .map_err(|e| ApiError::Internal(format!("Failed to export reading progress: {e}")))?; let filename = format!( "codex-reading-progress-{}.json", @@ -130,6 +139,7 @@ pub async fn import_reading_progress( conflict_policy: request.conflict_policy, reattach_sessions: request.reattach_sessions, accept_stem_matches: request.accept_stem_matches, + library_ids: request.library_ids, }; let response = diff --git a/crates/codex-services/src/reading_transfer/export.rs b/crates/codex-services/src/reading_transfer/export.rs index 601d70c7..19a2e547 100644 --- a/crates/codex-services/src/reading_transfer/export.rs +++ b/crates/codex-services/src/reading_transfer/export.rs @@ -95,10 +95,15 @@ 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". +/// `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 +/// the file larger and the import's matching job harder for no gain. pub async fn export_reading_progress( db: &DatabaseConnection, user_id: Uuid, include_sessions: bool, + library_ids: Option<&[Uuid]>, ) -> Result { let progress_rows = ReadProgressRepository::get_by_user(db, user_id).await?; let completion_rows = completions_for_user(db, user_id).await?; @@ -133,8 +138,15 @@ pub async fn export_reading_progress( let series_ids: Vec = series_id_set.into_iter().collect(); let mut series_rows = SeriesRepository::get_by_ids(db, &series_ids).await?; + if let Some(wanted) = library_ids { + series_rows.retain(|s| wanted.contains(&s.library_id)); + } series_rows.sort_by(|a, b| a.path.cmp(&b.path).then_with(|| a.id.cmp(&b.id))); + // Re-derived after the filter so nothing downstream loads or emits a + // series the caller excluded. + let series_ids: Vec = series_rows.iter().map(|s| s.id).collect(); + let library_ids: Vec = series_rows .iter() .map(|s| s.library_id) @@ -381,7 +393,9 @@ mod tests { .await .unwrap(); - let doc = export_reading_progress(conn, user, true).await.unwrap(); + let doc = export_reading_progress(conn, user, true, None) + .await + .unwrap(); assert_eq!(doc.format, READING_PROGRESS_FORMAT); assert_eq!(doc.series.len(), 1); @@ -449,7 +463,9 @@ mod tests { missing.deleted = Set(true); missing.update(conn).await.unwrap(); - let doc = export_reading_progress(conn, user, true).await.unwrap(); + let doc = export_reading_progress(conn, user, true, None) + .await + .unwrap(); assert_eq!( doc.series.len(), @@ -520,7 +536,9 @@ mod tests { .await .unwrap(); - let doc_without = export_reading_progress(conn, user, false).await.unwrap(); + let doc_without = export_reading_progress(conn, user, false, None) + .await + .unwrap(); assert!(!doc_without.includes_sessions); let book_doc = &doc_without.series[0].books[0]; assert!(book_doc.sessions.is_none()); @@ -528,7 +546,9 @@ mod tests { assert_eq!(book_doc.completions.len(), 1); assert!(book_doc.progress.is_some()); - let doc_with = export_reading_progress(conn, user, true).await.unwrap(); + let doc_with = export_reading_progress(conn, user, true, None) + .await + .unwrap(); assert!(doc_with.includes_sessions); let book_doc = &doc_with.series[0].books[0]; assert!(book_doc.sessions.is_some()); @@ -684,7 +704,9 @@ mod tests { .unwrap(); } - let doc = export_reading_progress(conn, user, true).await.unwrap(); + let doc = export_reading_progress(conn, user, true, None) + .await + .unwrap(); assert_eq!(doc.series.len(), SERIES_COUNT); let json = serde_json::to_vec(&doc).unwrap(); @@ -746,7 +768,9 @@ mod tests { .await .unwrap(); - let doc = export_reading_progress(conn, user_a, true).await.unwrap(); + let doc = export_reading_progress(conn, user_a, true, None) + .await + .unwrap(); assert_eq!(doc.series.len(), 1); assert_eq!(doc.series[0].books.len(), 1); assert_eq!( diff --git a/crates/codex-services/src/reading_transfer/import.rs b/crates/codex-services/src/reading_transfer/import.rs index d164ac7a..a9c14cf7 100644 --- a/crates/codex-services/src/reading_transfer/import.rs +++ b/crates/codex-services/src/reading_transfer/import.rs @@ -44,10 +44,16 @@ use super::model::{ pub struct ImportOptions { pub dry_run: bool, pub hash_mode: HashMode, - pub source_preference: Vec, + /// `None` means every source the exported series carries. See + /// [`ImportReadingProgressRequest::source_preference`]. + pub source_preference: Option>, pub conflict_policy: ConflictPolicy, pub reattach_sessions: bool, pub accept_stem_matches: bool, + /// Which libraries a series may match into. `None` searches every library + /// 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>, } /// A rejection worth a 400, versus every other failure which is a 500. @@ -304,6 +310,11 @@ enum RowDecision { /// the file, or belongs to someone else (a UUID collision that should /// never happen, handled by leaving it alone). Skip, + /// On a *different* live book, in a library that still has the file. The + /// write is the same no-op as [`Self::Skip`], but it means something very + /// different: the progress moved and the reading history did not. Counted + /// apart so the report can say so. + SkipStranded, } /// An existing history row, as much as a decision needs. @@ -440,12 +451,15 @@ fn decide_row( match existing { None => RowDecision::Insert, Some(row) if row.user_id == user_id => { + let on_another_live_book = matches!(row.book_id, Some(book) if book != target_book && !soft_deleted.contains(&book)); let off_a_live_book = match row.book_id { None => true, Some(book) => book != target_book && soft_deleted.contains(&book), }; if off_a_live_book && reattach { RowDecision::Reattach + } else if on_another_live_book { + RowDecision::SkipStranded } else { RowDecision::Skip } @@ -591,6 +605,10 @@ fn book_report_from_plan(plan: &BookPlan<'_>) -> ImportBookReport { RowDecision::Insert => completions.inserted += 1, RowDecision::Reattach => completions.reattached += 1, RowDecision::Skip => completions.skipped += 1, + RowDecision::SkipStranded => { + completions.skipped += 1; + completions.stranded += 1; + } } } let mut sessions = WriteCounts::default(); @@ -599,6 +617,10 @@ fn book_report_from_plan(plan: &BookPlan<'_>) -> ImportBookReport { RowDecision::Insert => sessions.inserted += 1, RowDecision::Reattach => sessions.reattached += 1, RowDecision::Skip => sessions.skipped += 1, + RowDecision::SkipStranded => { + sessions.skipped += 1; + sessions.stranded += 1; + } } } @@ -651,6 +673,7 @@ fn tally_book_report(summary: &mut ImportSummary, report: &ImportBookReport, cou summary.completions_reattached += report.completions.reattached; summary.sessions_inserted += report.sessions.inserted; summary.sessions_reattached += report.sessions.reattached; + summary.rows_stranded += report.completions.stranded + report.sessions.stranded; } // --------------------------------------------------------------------------- @@ -710,7 +733,7 @@ async fn apply_completion( decision: &RowDecision, ) -> Result<()> { match decision { - RowDecision::Skip => Ok(()), + RowDecision::Skip | RowDecision::SkipStranded => Ok(()), RowDecision::Insert => { read_completions::ActiveModel { id: Set(doc.id), @@ -743,7 +766,7 @@ async fn apply_session( decision: &RowDecision, ) -> Result<()> { match decision { - RowDecision::Skip => Ok(()), + RowDecision::Skip | RowDecision::SkipStranded => Ok(()), RowDecision::Insert => { reading_sessions::ActiveModel { id: Set(doc.id), @@ -1042,7 +1065,8 @@ pub async fn import_reading_progress( db, &content_filter, series_doc, - &options.source_preference, + options.source_preference.as_deref(), + options.library_ids.as_deref(), ) .await { @@ -1160,6 +1184,20 @@ pub async fn import_reading_progress( } } + // 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 + // rescanning the other library and importing again. + if summary.rows_stranded > 0 { + notices.push(format!( + "{} session/completion rows were left behind: their books are still \ + live in another library. Progress moved but reading history did not. \ + Delete or rescan that library so its books are no longer on disk, \ + then import again to bring the history across.", + summary.rows_stranded + )); + } + Ok(ImportReadingProgressResponse { dry_run: options.dry_run, sessions_in_file: document.includes_sessions, @@ -1481,8 +1519,13 @@ mod tests { assert_eq!(decision, RowDecision::Reattach); } + /// Left alone, but not for the harmless reason a plain `Skip` means. + /// The row is on a live book in another library: reattaching would strip + /// history from a library the reader may still be using, and skipping it + /// silently would hide that the progress moved without it. Hence its own + /// variant, which the report turns into a notice. #[test] - fn row_on_a_live_book_is_left_alone() { + fn row_on_a_live_book_elsewhere_is_left_alone_but_flagged() { let user = Uuid::new_v4(); let live = Uuid::new_v4(); let mut planned = PlannedState::default(); @@ -1495,6 +1538,25 @@ mod tests { true, &mut planned, ); + assert_eq!(decision, RowDecision::SkipStranded); + } + + /// The genuinely harmless skip: the row is already on the book being + /// imported onto, which is what makes re-importing the same file a no-op. + #[test] + fn row_already_on_the_target_book_is_a_plain_skip() { + let user = Uuid::new_v4(); + let target = Uuid::new_v4(); + let mut planned = PlannedState::default(); + let decision = decide_row( + Uuid::new_v4(), + Some(&row(user, Some(target))), + target, + user, + &HashSet::new(), + true, + &mut planned, + ); assert_eq!(decision, RowDecision::Skip); } diff --git a/crates/codex-services/src/reading_transfer/matching.rs b/crates/codex-services/src/reading_transfer/matching.rs index 88e5f64c..6dbf1338 100644 --- a/crates/codex-services/src/reading_transfer/matching.rs +++ b/crates/codex-services/src/reading_transfer/matching.rs @@ -81,18 +81,27 @@ fn visible_ids(content_filter: &ContentFilter, ids: Vec) -> Vec { /// export exactly by path, and matching it would stop the search there with /// no book to write onto, never reaching the new series. A series with nothing /// on disk is not somewhere reading state can land, so it is not a candidate. +/// Every matching step funnels through here, so scoping to a library is done +/// once rather than in each lookup. That scope is what lets an import run +/// while the old copy of a series is still on disk: without it the old and +/// new series both match the same name and the step reports `Ambiguous`. async fn live_candidates( db: &DatabaseConnection, content_filter: &ContentFilter, ids: Vec, + library_ids: Option<&[Uuid]>, ) -> Result> { let visible = visible_ids(content_filter, ids); if visible.is_empty() { return Ok(visible); } - let live: HashSet = books::Entity::find() + let mut query = books::Entity::find() .filter(books::Column::SeriesId.is_in(visible.clone())) - .filter(books::Column::Deleted.eq(false)) + .filter(books::Column::Deleted.eq(false)); + if let Some(wanted) = library_ids { + query = query.filter(books::Column::LibraryId.is_in(wanted.to_vec())); + } + let live: HashSet = query .all(db) .await? .into_iter() @@ -149,19 +158,35 @@ async fn series_ids_by_normalized_name( /// moving to the next preferred source when a tried one yields no visible /// candidate), then `series.path`, then `series.normalized_name`. Stops at /// the first step that produces any visible candidate. +/// +/// `source_preference` of `None` means every source the exported series +/// carries, in document order. That is the useful default: an external id is +/// the only key that survives both a rename and a move, so a caller who names +/// no sources should still get it rather than falling through to the two +/// weakest steps. `Some(&[])` skips the id steps outright. pub async fn resolve_series( db: &DatabaseConnection, content_filter: &ContentFilter, exported: &ExportSeriesDto, - source_preference: &[String], + source_preference: Option<&[String]>, + library_ids: Option<&[Uuid]>, ) -> Result { - for source in source_preference { - let Some(external) = exported.external_ids.iter().find(|e| &e.source == source) else { + let sources: Vec<&str> = match source_preference { + Some(preferred) => preferred.iter().map(String::as_str).collect(), + None => exported + .external_ids + .iter() + .map(|e| e.source.as_str()) + .collect(), + }; + + for source in sources { + let Some(external) = exported.external_ids.iter().find(|e| e.source == source) else { continue; }; let candidates = series_ids_by_external_id(db, source, &external.id).await?; - match live_candidates(db, content_filter, candidates) + match live_candidates(db, content_filter, candidates, library_ids) .await? .as_slice() { @@ -172,7 +197,7 @@ pub async fn resolve_series( } let by_path = series_ids_by_path(db, &exported.library_relative_path).await?; - match live_candidates(db, content_filter, by_path) + match live_candidates(db, content_filter, by_path, library_ids) .await? .as_slice() { @@ -183,7 +208,7 @@ pub async fn resolve_series( let normalized = SeriesRepository::normalize_name(&exported.name); let by_name = series_ids_by_normalized_name(db, &normalized).await?; - match live_candidates(db, content_filter, by_name) + match live_candidates(db, content_filter, by_name, library_ids) .await? .as_slice() { @@ -596,7 +621,9 @@ mod tests { let filter = ContentFilter::for_user(conn, user).await.unwrap(); let exported = doc_series("Naruto", "shonen/Naruto", None); - let result = resolve_series(conn, &filter, &exported, &[]).await.unwrap(); + let result = resolve_series(conn, &filter, &exported, None, None) + .await + .unwrap(); assert_eq!(result, SeriesMatch::Matched(moved.id)); } @@ -615,7 +642,9 @@ mod tests { let filter = ContentFilter::for_user(conn, user).await.unwrap(); let doc = doc_series("Naruto", "shonen/Naruto", None); - let result = resolve_series(conn, &filter, &doc, &[]).await.unwrap(); + let result = resolve_series(conn, &filter, &doc, None, None) + .await + .unwrap(); assert_eq!(result, SeriesMatch::Matched(series.id)); } @@ -632,7 +661,9 @@ mod tests { let filter = ContentFilter::for_user(conn, user).await.unwrap(); // A path that does not exist anywhere; only the name matches. let doc = doc_series("One Piece", "moved/somewhere/else", None); - let result = resolve_series(conn, &filter, &doc, &[]).await.unwrap(); + let result = resolve_series(conn, &filter, &doc, None, None) + .await + .unwrap(); assert_eq!(result, SeriesMatch::Matched(series.id)); } @@ -677,7 +708,8 @@ mod tests { conn, &filter, &doc, - &["plugin:mangabaka".to_string(), "plugin:anilist".to_string()], + Some(&["plugin:mangabaka".to_string(), "plugin:anilist".to_string()]), + None, ) .await .unwrap(); @@ -687,7 +719,8 @@ mod tests { conn, &filter, &doc, - &["plugin:anilist".to_string(), "plugin:mangabaka".to_string()], + Some(&["plugin:anilist".to_string(), "plugin:mangabaka".to_string()]), + None, ) .await .unwrap(); @@ -712,7 +745,9 @@ mod tests { let filter = ContentFilter::for_user(conn, user).await.unwrap(); let doc = doc_series("Duplicate", "nowhere/matching", None); - let result = resolve_series(conn, &filter, &doc, &[]).await.unwrap(); + let result = resolve_series(conn, &filter, &doc, None, None) + .await + .unwrap(); assert_eq!(result, SeriesMatch::Ambiguous); } @@ -740,7 +775,9 @@ mod tests { let filter = ContentFilter::for_user(conn, user).await.unwrap(); let doc = doc_series("Hidden", &series.path, None); - let result = resolve_series(conn, &filter, &doc, &[]).await.unwrap(); + let result = resolve_series(conn, &filter, &doc, None, None) + .await + .unwrap(); assert_eq!(result, SeriesMatch::Unmatched); } } diff --git a/crates/codex-services/src/reading_transfer/model.rs b/crates/codex-services/src/reading_transfer/model.rs index b6b1f045..59ed8181 100644 --- a/crates/codex-services/src/reading_transfer/model.rs +++ b/crates/codex-services/src/reading_transfer/model.rs @@ -206,9 +206,16 @@ pub struct ImportReadingProgressRequest { #[serde(default)] pub hash_mode: HashMode, /// External-id sources to try, in order, before falling back to path and - /// then normalized name. An empty list skips straight to path matching. + /// then normalized name. + /// + /// Omitted means every source the exported series carries, in the order + /// the document lists them. An external id survives a rename and a move + /// where neither the path nor the name does, so defaulting this to + /// nothing silently downgrades every import to the two weakest steps. + /// An explicit empty list still skips straight to path matching, for a + /// caller that wants exactly that. #[serde(default)] - pub source_preference: Vec, + pub source_preference: Option>, #[serde(default)] pub conflict_policy: ConflictPolicy, /// When a session or completion in the file already exists as the @@ -222,6 +229,13 @@ pub struct ImportReadingProgressRequest { /// applying it silently risks writing progress onto the wrong one. #[serde(default)] pub accept_stem_matches: bool, + /// Which libraries a series may match into. Omitted searches every + /// library the reader can see. Narrowing to the target library is what + /// lets an import run before the old library has been rescanned: two + /// copies of one series would otherwise both match and report + /// `ambiguous`. + #[serde(default)] + pub library_ids: Option>, pub file: ReadingProgressExportDocument, } @@ -275,7 +289,15 @@ pub struct WriteCounts { /// new one. pub reattached: u32, /// Already present with the same book attached; re-importing is a no-op. + /// Includes `stranded`, so `inserted + reattached + skipped` still totals + /// every row in the file. pub skipped: u32, + /// Skipped because the row sits on a book that is still live in another + /// library, which is not the same thing as a harmless re-import: the + /// reading history stays behind while the progress moves. Reattaching it + /// would strip a library the reader may still be using, so the import + /// reports it instead of guessing. + pub stranded: u32, } /// The outcome for one book in the import file. @@ -340,6 +362,9 @@ pub struct ImportSummary { pub completions_reattached: u32, pub sessions_inserted: u32, pub sessions_reattached: u32, + /// 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, } /// 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 0bb090e6..72077160 100644 --- a/docs/api/openapi.json +++ b/docs/api/openapi.json @@ -10254,6 +10254,15 @@ "schema": { "type": "boolean" } + }, + { + "name": "libraryIds", + "in": "query", + "description": "Comma-separated library ids to export. Omitted exports every library the reader has state for.", + "required": false, + "schema": { + "type": "string" + } } ], "responses": { @@ -30880,6 +30889,13 @@ "includeSessions": { "type": "boolean", "description": "Sessions are opt-out: they are the only source of every reading\nstatistic, so leaving them out is easy to do by accident and hard to\nnotice until the numbers are gone." + }, + "libraryIds": { + "type": [ + "string", + "null" + ], + "description": "Comma-separated library ids to export, e.g.\n`?libraryIds=,`. Omitted exports every library the reader\nhas state for.\n\nComma-separated rather than a repeated key because axum's `Query`\nextractor deserializes with `serde_urlencoded`, which collapses a\nrepeated key instead of collecting it into a `Vec`." } } }, @@ -32563,16 +32579,30 @@ "hashMode": { "$ref": "#/components/schemas/HashMode" }, + "libraryIds": { + "type": [ + "array", + "null" + ], + "items": { + "type": "string", + "format": "uuid" + }, + "description": "Which libraries a series may match into. Omitted searches every\nlibrary the reader can see. Narrowing to the target library is what\nlets an import run before the old library has been rescanned: two\ncopies of one series would otherwise both match and report\n`ambiguous`." + }, "reattachSessions": { "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." }, "sourcePreference": { - "type": "array", + "type": [ + "array", + "null" + ], "items": { "type": "string" }, - "description": "External-id sources to try, in order, before falling back to path and\nthen normalized name. An empty list skips straight to path matching." + "description": "External-id sources to try, in order, before falling back to path and\nthen normalized name.\n\nOmitted means every source the exported series carries, in the order\nthe document lists them. An external id survives a rename and a move\nwhere neither the path nor the name does, so defaulting this to\nnothing silently downgrades every import to the two weakest steps.\nAn explicit empty list still skips straight to path matching, for a\ncaller that wants exactly that." } } }, @@ -32683,7 +32713,8 @@ "completionsInserted", "completionsReattached", "sessionsInserted", - "sessionsReattached" + "sessionsReattached", + "rowsStranded" ], "properties": { "booksAmbiguous": { @@ -32736,6 +32767,12 @@ "format": "int32", "minimum": 0 }, + "rowsStranded": { + "type": "integer", + "format": "int32", + "description": "Rows left behind on a live book in another library. Non-zero means the\nimport moved less than it appears to have.", + "minimum": 0 + }, "seriesAmbiguous": { "type": "integer", "format": "int32", @@ -48125,7 +48162,8 @@ "required": [ "inserted", "reattached", - "skipped" + "skipped", + "stranded" ], "properties": { "inserted": { @@ -48142,7 +48180,13 @@ "skipped": { "type": "integer", "format": "int32", - "description": "Already present with the same book attached; re-importing is a no-op.", + "description": "Already present with the same book attached; re-importing is a no-op.\nIncludes `stranded`, so `inserted + reattached + skipped` still totals\nevery row in the file.", + "minimum": 0 + }, + "stranded": { + "type": "integer", + "format": "int32", + "description": "Skipped because the row sits on a book that is still live in another\nlibrary, which is not the same thing as a harmless re-import: the\nreading history stays behind while the progress moves. Reattaching it\nwould strip a library the reader may still be using, so the import\nreports it instead of guessing.", "minimum": 0 } } diff --git a/docs/docs/backup-migration/reading-progress-transfer.md b/docs/docs/backup-migration/reading-progress-transfer.md index 682c312f..49676453 100644 --- a/docs/docs/backup-migration/reading-progress-transfer.md +++ b/docs/docs/backup-migration/reading-progress-transfer.md @@ -100,17 +100,45 @@ that the content exists. |---|---|---| | `dryRun` | `false` | Report the outcome without writing anything | | `hashMode` | `verify` | `off` ignores hashes; `verify` rejects a path/name match whose `fileHash` disagrees; `match` additionally uses `fileHash`/`partialHash` to find a book when path and name both fail (rescues a bulk rename) | -| `sourcePreference` | `[]` | External-id sources to try, in order, before falling back to path and name | +| `sourcePreference` | every source in the file | External-id sources to try, in order, before falling back to path and name. Omit it to try each source the exported series carries, in document order. An explicit `[]` skips external ids entirely and goes straight to path matching | | `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 | +| `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. -`GET /api/v1/reading-progress/export` takes one query parameter, `includeSessions` (default `true`). Turn it off only if you specifically want a smaller file: sessions are the only source of every reading statistic, so leaving them out is easy to do by accident and easy not to notice until the numbers are gone. +`libraryIds`, a comma-separated list (for example +`?libraryIds=,`). Omitted, the export carries every library you +have reading state for. Narrowing it to the library you are reorganising +keeps the file small and gives the import less to match against. A value that +is not a uuid is a `400` rather than a quietly wider export. + +## When history is left behind + +Reattachment moves a session or completion onto the matched book when its own +book is gone (`bookId` is null after a hard delete) or the scanner has marked +it deleted because the file moved. Both are the normal shapes of a library +split, so the usual sequence needs no special care: export, delete the old +library, import into the new one. + +It does **not** move a row whose book is still live somewhere. Reattaching +then would strip reading history out of a library you may still be using, so +the import leaves it alone and counts it as `stranded`, both per book and as +`rowsStranded` in the summary, with a notice on the report. + +This is worth watching for when you scope an import with `libraryIds` while +the old copy of a series is still on disk. The scope makes the series match +where it would otherwise be reported `ambiguous`, so the progress moves and +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. + ## The response The response is the same shape whether or not `dryRun` is set: counts, plus a diff --git a/tests/api/reading_progress_transfer.rs b/tests/api/reading_progress_transfer.rs index c30c6669..0e3b15f7 100644 --- a/tests/api/reading_progress_transfer.rs +++ b/tests/api/reading_progress_transfer.rs @@ -13,10 +13,10 @@ mod common; use chrono::{Duration, Utc}; use codex::api::routes::v1::dto::{ - BookDisposition, ConflictPolicy, ExportBookDto, ExportCompletionDto, ExportProgressDto, - ExportSeriesDto, ExportSessionDto, FieldOutcome, HashMode, ImportReadingProgressRequest, - ImportReadingProgressResponse, READING_PROGRESS_FORMAT, READING_PROGRESS_VERSION, - ReadingProgressExportDocument, SeriesDisposition, + BookDisposition, ConflictPolicy, ExportBookDto, ExportCompletionDto, ExportExternalIdDto, + ExportProgressDto, ExportSeriesDto, ExportSessionDto, FieldOutcome, HashMode, + ImportReadingProgressRequest, ImportReadingProgressResponse, READING_PROGRESS_FORMAT, + READING_PROGRESS_VERSION, ReadingProgressExportDocument, SeriesDisposition, }; use codex::db::ScanningStrategy; use codex::db::entities::reading_sessions::SessionKind; @@ -26,8 +26,8 @@ use codex::db::entities::{ }; use codex::db::repositories::{ BookRepository, LibraryRepository, NewSession, ReadCompletionRepository, - ReadProgressRepository, SeriesRepository, SharingTagRepository, UserRepository, - UserSeriesRatingRepository, + ReadProgressRepository, SeriesExternalIdRepository, SeriesRepository, SharingTagRepository, + UserRepository, UserSeriesRatingRepository, }; use codex::utils::password; use common::*; @@ -150,10 +150,11 @@ fn import_request( ImportReadingProgressRequest { dry_run, hash_mode: HashMode::Verify, - source_preference: vec![], + source_preference: None, conflict_policy: ConflictPolicy::Overwrite, reattach_sessions: true, accept_stem_matches: false, + library_ids: None, file, } } @@ -1141,3 +1142,494 @@ async fn reading_progress_transfer_postgres() { exercise_idempotent_import(&db).await; exercise_visibility_denies_unmatched(&db).await; } + +// --------------------------------------------------------------------------- +// Series matching by external id. +// +// This is the step the format exists for: an id survives a rename and a move, +// which neither the relative path nor the normalized name does. Everything +// below builds a target whose path *and* name differ from the export, so the +// two weaker steps cannot succeed and only the id can explain a match. +// --------------------------------------------------------------------------- + +/// A series in the library, with an external id attached, whose path and name +/// deliberately differ from whatever the export will carry. +async fn series_with_external_id( + db: &DatabaseConnection, + library_id: Uuid, + name: &str, + source: &str, + external_id: &str, +) -> Uuid { + let series = SeriesRepository::create(db, library_id, name, None) + .await + .unwrap(); + SeriesExternalIdRepository::create(db, series.id, source, external_id, None, None) + .await + .unwrap(); + BookRepository::create( + db, + &book_model( + series.id, + library_id, + &format!("/lib/{name}/v01.cbz"), + "v01.cbz", + "", + ), + None, + ) + .await + .unwrap(); + series.id +} + +/// An exported series carrying ids but a path and name that match nothing. +fn series_doc_with_ids(ids: Vec<(&str, &str)>) -> ExportSeriesDto { + ExportSeriesDto { + external_ids: ids + .into_iter() + .map(|(source, id)| ExportExternalIdDto { + source: source.to_string(), + id: id.to_string(), + }) + .collect(), + library_relative_path: "a/path/that/matches/nothing".to_string(), + name: "A Name That Matches Nothing".to_string(), + rating: Some(64), + notes: None, + rating_updated_at: Some(Utc::now()), + books: vec![], + } +} + +async fn import_with( + state: &std::sync::Arc, + token: &str, + doc: ReadingProgressExportDocument, + sources: Option>, +) -> ImportReadingProgressResponse { + let app = create_test_router(state.clone()).await; + let mut body = import_request(doc, true); + body.source_preference = sources; + 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 defect this change fixes: a caller that does not name any source still +/// gets id matching. An empty default silently downgraded every import to the +/// two weakest steps, and no caller in the tree ever passed anything else. +#[tokio::test] +async fn omitting_the_source_preference_still_matches_on_external_id() { + let (db, _tmp) = setup_test_db().await; + let state = create_test_auth_state(db.clone()).await; + let (_user, token) = admin_and_token(&db, &state, "id-default").await; + let library = LibraryRepository::create(&db, "Lib", "/lib", ScanningStrategy::Default) + .await + .unwrap(); + + let target = series_with_external_id( + &db, + library.id, + "Renamed Series", + "plugin:mangabaka", + "12345", + ) + .await; + + let doc = document( + vec![series_doc_with_ids(vec![("plugin:mangabaka", "12345")])], + false, + ); + let report = import_with(&state, &token, doc, None).await; + + assert_eq!(report.summary.series_matched, 1); + assert_eq!(report.series[0].disposition, SeriesDisposition::Matched); + assert_eq!(report.series[0].matched_series_id, Some(target)); +} + +/// An explicit empty list still means "skip ids", so a caller can deliberately +/// fall through to path and name. +#[tokio::test] +async fn an_explicit_empty_source_preference_skips_external_ids() { + let (db, _tmp) = setup_test_db().await; + let state = create_test_auth_state(db.clone()).await; + let (_user, token) = admin_and_token(&db, &state, "id-optout").await; + let library = LibraryRepository::create(&db, "Lib", "/lib", ScanningStrategy::Default) + .await + .unwrap(); + + series_with_external_id( + &db, + library.id, + "Renamed Series", + "plugin:mangabaka", + "12345", + ) + .await; + + let doc = document( + vec![series_doc_with_ids(vec![("plugin:mangabaka", "12345")])], + false, + ); + let report = import_with(&state, &token, doc, Some(vec![])).await; + + assert_eq!(report.summary.series_unmatched, 1); +} + +/// Priority order is the point of a *list*: when the file carries two ids that +/// resolve to different series, the order decides which one wins. +#[tokio::test] +async fn the_preference_order_decides_between_two_matching_sources() { + let (db, _tmp) = setup_test_db().await; + let state = create_test_auth_state(db.clone()).await; + let (_user, token) = admin_and_token(&db, &state, "id-order").await; + let library = LibraryRepository::create(&db, "Lib", "/lib", ScanningStrategy::Default) + .await + .unwrap(); + + let baka = + series_with_external_id(&db, library.id, "Via Mangabaka", "plugin:mangabaka", "111").await; + let anilist = + series_with_external_id(&db, library.id, "Via Anilist", "plugin:anilist", "222").await; + + let ids = vec![("plugin:mangabaka", "111"), ("plugin:anilist", "222")]; + + let baka_first = import_with( + &state, + &token, + document(vec![series_doc_with_ids(ids.clone())], false), + Some(vec!["plugin:mangabaka".into(), "plugin:anilist".into()]), + ) + .await; + assert_eq!(baka_first.series[0].matched_series_id, Some(baka)); + + let anilist_first = import_with( + &state, + &token, + document(vec![series_doc_with_ids(ids)], false), + Some(vec!["plugin:anilist".into(), "plugin:mangabaka".into()]), + ) + .await; + assert_eq!(anilist_first.series[0].matched_series_id, Some(anilist)); +} + +/// A preferred source the file does not carry is skipped rather than ending +/// the search, so naming a source order costs nothing when a file is sparse. +#[tokio::test] +async fn a_source_absent_from_the_file_falls_through_to_the_next() { + let (db, _tmp) = setup_test_db().await; + let state = create_test_auth_state(db.clone()).await; + let (_user, token) = admin_and_token(&db, &state, "id-sparse").await; + let library = LibraryRepository::create(&db, "Lib", "/lib", ScanningStrategy::Default) + .await + .unwrap(); + + let target = + series_with_external_id(&db, library.id, "Only Anilist", "plugin:anilist", "222").await; + + let doc = document( + vec![series_doc_with_ids(vec![("plugin:anilist", "222")])], + false, + ); + let report = import_with( + &state, + &token, + doc, + Some(vec!["plugin:mangabaka".into(), "plugin:anilist".into()]), + ) + .await; + + assert_eq!(report.series[0].matched_series_id, Some(target)); +} + +/// Two series sharing one id is a real state (a bad plugin match, or a split +/// that duplicated a series). Guessing between them would write a reader's +/// history onto the wrong book, so it reports and writes nothing. +#[tokio::test] +async fn two_series_sharing_an_external_id_are_ambiguous() { + let (db, _tmp) = setup_test_db().await; + let state = create_test_auth_state(db.clone()).await; + let (_user, token) = admin_and_token(&db, &state, "id-ambiguous").await; + let library = LibraryRepository::create(&db, "Lib", "/lib", ScanningStrategy::Default) + .await + .unwrap(); + + series_with_external_id(&db, library.id, "First Copy", "plugin:mangabaka", "12345").await; + series_with_external_id(&db, library.id, "Second Copy", "plugin:mangabaka", "12345").await; + + let doc = document( + vec![series_doc_with_ids(vec![("plugin:mangabaka", "12345")])], + false, + ); + let report = import_with(&state, &token, doc, None).await; + + assert_eq!(report.series[0].disposition, SeriesDisposition::Ambiguous); + assert_eq!(report.summary.series_ambiguous, 1); +} + +// --------------------------------------------------------------------------- +// Library scoping. +// --------------------------------------------------------------------------- + +/// Scoping the import to the target library is what lets it run while the old +/// copy of a series is still on disk. Unscoped, both copies are live, both +/// match the same normalized name, and the series reports `ambiguous` with +/// nothing written. +#[tokio::test] +async fn scoping_the_import_to_a_library_resolves_a_duplicate_that_is_otherwise_ambiguous() { + let (db, _tmp) = setup_test_db().await; + let state = create_test_auth_state(db.clone()).await; + let (_user, token) = admin_and_token(&db, &state, "lib-scope").await; + + let old_lib = LibraryRepository::create(&db, "Old", "/old", ScanningStrategy::Default) + .await + .unwrap(); + let new_lib = LibraryRepository::create(&db, "New", "/new", ScanningStrategy::Default) + .await + .unwrap(); + + // The same series, live in both libraries: the files were copied rather + // than moved, or the old library has not been rescanned yet. + for (lib, root) in [(old_lib.id, "old"), (new_lib.id, "new")] { + let series = SeriesRepository::create(&db, lib, "Naruto", None) + .await + .unwrap(); + BookRepository::create( + &db, + &book_model( + series.id, + lib, + &format!("/{root}/Naruto/v01.cbz"), + "v01.cbz", + "", + ), + None, + ) + .await + .unwrap(); + } + + let series_doc = ExportSeriesDto { + external_ids: vec![], + library_relative_path: "Naruto".to_string(), + name: "Naruto".to_string(), + rating: Some(50), + notes: None, + rating_updated_at: Some(Utc::now()), + books: vec![], + }; + + // Unscoped: both copies compete. + let app = create_test_router(state.clone()).await; + let body = import_request(document(vec![series_doc.clone()], false), true); + let request = post_json_request_with_auth("/api/v1/reading-progress/import", &body, &token); + let (_status, unscoped): (StatusCode, Option) = + make_json_request(app, request).await; + assert_eq!( + unscoped.expect("report").series[0].disposition, + SeriesDisposition::Ambiguous + ); + + // Scoped to the new library: only one candidate remains. + let app = create_test_router(state.clone()).await; + let mut body = import_request(document(vec![series_doc], false), true); + body.library_ids = Some(vec![new_lib.id]); + let request = post_json_request_with_auth("/api/v1/reading-progress/import", &body, &token); + let (_status, scoped): (StatusCode, Option) = + make_json_request(app, request).await; + assert_eq!( + scoped.expect("report").series[0].disposition, + SeriesDisposition::Matched + ); +} + +/// Exporting a subset carries only that library's series. +#[tokio::test] +async fn exporting_with_library_ids_omits_the_other_libraries() { + let (db, _tmp) = setup_test_db().await; + let state = create_test_auth_state(db.clone()).await; + let (user_id, token) = admin_and_token(&db, &state, "lib-export").await; + + let mut wanted_library = None; + for name in ["Kept", "Excluded"] { + let lib = + LibraryRepository::create(&db, name, &format!("/{name}"), ScanningStrategy::Default) + .await + .unwrap(); + let series = SeriesRepository::create(&db, lib.id, name, None) + .await + .unwrap(); + let book = BookRepository::create( + &db, + &book_model( + series.id, + lib.id, + &format!("/{name}/{name}/v01.cbz"), + "v01.cbz", + "", + ), + None, + ) + .await + .unwrap(); + ReadProgressRepository::upsert(&db, user_id, book.id, 3, false) + .await + .unwrap(); + if name == "Kept" { + wanted_library = Some(lib.id); + } + } + + let app = create_test_router(state.clone()).await; + let request = get_request_with_auth( + &format!( + "/api/v1/reading-progress/export?libraryIds={}", + wanted_library.unwrap() + ), + &token, + ); + let (status, document): (StatusCode, Option) = + make_json_request(app, request).await; + assert_eq!(status, StatusCode::OK); + + let document = document.expect("export document"); + let names: Vec<&str> = document.series.iter().map(|s| s.name.as_str()).collect(); + assert_eq!(names, vec!["Kept"]); +} + +/// A malformed id is a 400, not a silently wider export. +#[tokio::test] +async fn a_malformed_library_id_is_rejected_with_400() { + let (db, _tmp) = setup_test_db().await; + let state = create_test_auth_state(db.clone()).await; + let (_user, token) = admin_and_token(&db, &state, "lib-bad").await; + + let app = create_test_router(state.clone()).await; + let request = get_request_with_auth( + "/api/v1/reading-progress/export?libraryIds=not-a-uuid", + &token, + ); + let (status, _body): (StatusCode, Option) = + make_json_request(app, request).await; + assert_eq!(status, StatusCode::BAD_REQUEST); +} + +/// Scoping makes a series match that would otherwise be ambiguous, which is +/// the point of it. But the reader's sessions still sit on the old library's +/// live books, so they are *not* moved: progress goes across and the reading +/// history stays behind. That is a half-migration, and the report has to say +/// so rather than reading as a clean success. +#[tokio::test] +async fn rows_on_a_live_book_elsewhere_are_reported_as_stranded() { + let (db, _tmp) = setup_test_db().await; + let state = create_test_auth_state(db.clone()).await; + let (user_id, token) = admin_and_token(&db, &state, "stranded").await; + + let old_lib = LibraryRepository::create(&db, "Old", "/old", ScanningStrategy::Default) + .await + .unwrap(); + let new_lib = LibraryRepository::create(&db, "New", "/new", ScanningStrategy::Default) + .await + .unwrap(); + + // The same series in both libraries, both live: the files were copied + // rather than moved, so the old library still has them on disk. + let mut old_book = None; + for (lib, root) in [(old_lib.id, "old"), (new_lib.id, "new")] { + let series = SeriesRepository::create(&db, lib, "Naruto", None) + .await + .unwrap(); + let book = BookRepository::create( + &db, + &book_model( + series.id, + lib, + &format!("/{root}/Naruto/v01.cbz"), + "v01.cbz", + "", + ), + None, + ) + .await + .unwrap(); + if root == "old" { + old_book = Some(book.id); + } + } + + // A session already banked against the old library's still-live book. + let session_id = Uuid::new_v4(); + let now = Utc::now(); + ReadProgressRepository::record_session( + &db, + NewSession::from_client( + session_id, + user_id, + old_book.unwrap(), + "device-1", + None, + SessionKind::Progress, + Some(60_000), + Some(5), + now - Duration::minutes(10), + now, + ) + .with_page(5), + ) + .await + .unwrap(); + + let series_doc = ExportSeriesDto { + external_ids: vec![], + library_relative_path: "Naruto".to_string(), + name: "Naruto".to_string(), + rating: None, + notes: None, + rating_updated_at: None, + books: vec![ExportBookDto { + path: "v01.cbz".to_string(), + file_name: "v01.cbz".to_string(), + file_hash: String::new(), + partial_hash: String::new(), + progress: None, + completions: vec![], + sessions: Some(vec![ExportSessionDto { + id: session_id, + device_id: "device-1".to_string(), + device_name: None, + pass: 1, + kind: "progress".to_string(), + to_page: Some(5), + to_percentage: None, + active_duration_ms: Some(60_000), + duration_source: "measured".to_string(), + pages_read: Some(5), + client_started_at: now - Duration::minutes(10), + client_ended_at: now, + server_recorded_at: now, + }]), + }], + }; + + let app = create_test_router(state.clone()).await; + let mut body = import_request(document(vec![series_doc], true), true); + body.library_ids = Some(vec![new_lib.id]); + 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); + let report = response.expect("import response"); + + // The series matched, so this reads as a success without the notice. + assert_eq!(report.series[0].disposition, SeriesDisposition::Matched); + assert_eq!(report.summary.rows_stranded, 1); + assert_eq!(report.series[0].books[0].sessions.stranded, 1); + assert!( + report.notices.iter().any(|n| n.contains("left behind")), + "a half-migration must be stated, got: {:?}", + report.notices + ); +} diff --git a/web/openapi.json b/web/openapi.json index 0bb090e6..72077160 100644 --- a/web/openapi.json +++ b/web/openapi.json @@ -10254,6 +10254,15 @@ "schema": { "type": "boolean" } + }, + { + "name": "libraryIds", + "in": "query", + "description": "Comma-separated library ids to export. Omitted exports every library the reader has state for.", + "required": false, + "schema": { + "type": "string" + } } ], "responses": { @@ -30880,6 +30889,13 @@ "includeSessions": { "type": "boolean", "description": "Sessions are opt-out: they are the only source of every reading\nstatistic, so leaving them out is easy to do by accident and hard to\nnotice until the numbers are gone." + }, + "libraryIds": { + "type": [ + "string", + "null" + ], + "description": "Comma-separated library ids to export, e.g.\n`?libraryIds=,`. Omitted exports every library the reader\nhas state for.\n\nComma-separated rather than a repeated key because axum's `Query`\nextractor deserializes with `serde_urlencoded`, which collapses a\nrepeated key instead of collecting it into a `Vec`." } } }, @@ -32563,16 +32579,30 @@ "hashMode": { "$ref": "#/components/schemas/HashMode" }, + "libraryIds": { + "type": [ + "array", + "null" + ], + "items": { + "type": "string", + "format": "uuid" + }, + "description": "Which libraries a series may match into. Omitted searches every\nlibrary the reader can see. Narrowing to the target library is what\nlets an import run before the old library has been rescanned: two\ncopies of one series would otherwise both match and report\n`ambiguous`." + }, "reattachSessions": { "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." }, "sourcePreference": { - "type": "array", + "type": [ + "array", + "null" + ], "items": { "type": "string" }, - "description": "External-id sources to try, in order, before falling back to path and\nthen normalized name. An empty list skips straight to path matching." + "description": "External-id sources to try, in order, before falling back to path and\nthen normalized name.\n\nOmitted means every source the exported series carries, in the order\nthe document lists them. An external id survives a rename and a move\nwhere neither the path nor the name does, so defaulting this to\nnothing silently downgrades every import to the two weakest steps.\nAn explicit empty list still skips straight to path matching, for a\ncaller that wants exactly that." } } }, @@ -32683,7 +32713,8 @@ "completionsInserted", "completionsReattached", "sessionsInserted", - "sessionsReattached" + "sessionsReattached", + "rowsStranded" ], "properties": { "booksAmbiguous": { @@ -32736,6 +32767,12 @@ "format": "int32", "minimum": 0 }, + "rowsStranded": { + "type": "integer", + "format": "int32", + "description": "Rows left behind on a live book in another library. Non-zero means the\nimport moved less than it appears to have.", + "minimum": 0 + }, "seriesAmbiguous": { "type": "integer", "format": "int32", @@ -48125,7 +48162,8 @@ "required": [ "inserted", "reattached", - "skipped" + "skipped", + "stranded" ], "properties": { "inserted": { @@ -48142,7 +48180,13 @@ "skipped": { "type": "integer", "format": "int32", - "description": "Already present with the same book attached; re-importing is a no-op.", + "description": "Already present with the same book attached; re-importing is a no-op.\nIncludes `stranded`, so `inserted + reattached + skipped` still totals\nevery row in the file.", + "minimum": 0 + }, + "stranded": { + "type": "integer", + "format": "int32", + "description": "Skipped because the row sits on a book that is still live in another\nlibrary, which is not the same thing as a harmless re-import: the\nreading history stays behind while the progress moves. Reattaching it\nwould strip a library the reader may still be using, so the import\nreports it instead of guessing.", "minimum": 0 } } diff --git a/web/src/api/readingProgressTransfer.ts b/web/src/api/readingProgressTransfer.ts index 6c4ffddc..9fbbfdbb 100644 --- a/web/src/api/readingProgressTransfer.ts +++ b/web/src/api/readingProgressTransfer.ts @@ -22,13 +22,25 @@ export type BookDisposition = components["schemas"]["BookDisposition"]; const IMPORT_TIMEOUT_MS = 10 * 60_000; export const readingProgressTransferApi = { - /** Export the current user's reading progress as a downloadable document. */ + /** + * Export the current user's reading progress as a downloadable document. + * An empty `libraryIds` exports every library the reader has state for. + */ exportProgress: async ( includeSessions = true, + libraryIds: string[] = [], ): Promise => { const response = await api.get( "/reading-progress/export", - { params: { includeSessions } }, + { + params: { + includeSessions, + // Comma-separated: the endpoint reads one value, not a repeated key. + ...(libraryIds.length > 0 + ? { libraryIds: libraryIds.join(",") } + : {}), + }, + }, ); return response.data; }, diff --git a/web/src/hooks/useReadingProgressTransfer.ts b/web/src/hooks/useReadingProgressTransfer.ts index 909ac550..d7d0050c 100644 --- a/web/src/hooks/useReadingProgressTransfer.ts +++ b/web/src/hooks/useReadingProgressTransfer.ts @@ -25,9 +25,17 @@ function errorMessage(error: ApiErrorLike, fallback: string): string { */ export function useExportReadingProgress() { return useMutation({ - mutationFn: async (includeSessions: boolean) => { - const exported = - await readingProgressTransferApi.exportProgress(includeSessions); + mutationFn: async ({ + includeSessions, + libraryIds, + }: { + includeSessions: boolean; + libraryIds: string[]; + }) => { + const exported = await readingProgressTransferApi.exportProgress( + includeSessions, + libraryIds, + ); const timestamp = new Date().toISOString().slice(0, 10); const filename = `codex-reading-progress-${timestamp}.json`; diff --git a/web/src/pages/settings/ReadingProgressTransferSettings.test.tsx b/web/src/pages/settings/ReadingProgressTransferSettings.test.tsx index 35f6a777..31a07d12 100644 --- a/web/src/pages/settings/ReadingProgressTransferSettings.test.tsx +++ b/web/src/pages/settings/ReadingProgressTransferSettings.test.tsx @@ -131,7 +131,9 @@ describe("ReadingProgressTransferSettings", () => { await user.click(screen.getByRole("button", { name: /download export/i })); await waitFor(() => { - expect(exportProgress).toHaveBeenCalledWith(true); + // No library selected means every library, which the client sends as + // an empty list rather than a libraryIds parameter. + expect(exportProgress).toHaveBeenCalledWith(true, []); }); }); @@ -159,6 +161,27 @@ describe("ReadingProgressTransferSettings", () => { ); }); + it("omits sourcePreference rather than sending an empty list", async () => { + // An empty list means "skip external ids" to the server, and an id is the + // only matching key that survives a rename. Sending [] here silently + // downgraded every import to path and name matching. + 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(); + }); + const request = importProgress.mock.calls[0][0]; + expect(request.sourcePreference).toBeUndefined(); + expect(request.libraryIds).toBeUndefined(); + }); + it("re-locks Apply when an option changes after the preview", async () => { importProgress.mockResolvedValue(dryRunResponse()); const user = userEvent.setup(); diff --git a/web/src/pages/settings/ReadingProgressTransferSettings.tsx b/web/src/pages/settings/ReadingProgressTransferSettings.tsx index e3c94f76..24e75e73 100644 --- a/web/src/pages/settings/ReadingProgressTransferSettings.tsx +++ b/web/src/pages/settings/ReadingProgressTransferSettings.tsx @@ -7,6 +7,7 @@ import { Divider, FileButton, Group, + MultiSelect, Select, Stack, Table, @@ -19,7 +20,9 @@ import { IconDownload, IconUpload, } from "@tabler/icons-react"; -import { useRef, useState } from "react"; +import { useQuery } from "@tanstack/react-query"; +import { useMemo, useRef, useState } from "react"; +import { librariesApi } from "@/api/libraries"; import type { BookDisposition, ConflictPolicy, @@ -206,7 +209,19 @@ function ReportTable({ report }: { report: ImportReadingProgressResponse }) { } export function ReadingProgressTransferSettings() { + const { data: libraries = [] } = useQuery({ + queryKey: ["libraries"], + queryFn: librariesApi.getAll, + }); + const libraryOptions = libraries.map((library) => ({ + value: library.id, + label: library.name, + })); + const [includeSessions, setIncludeSessions] = useState(true); + const [exportLibraryIds, setExportLibraryIds] = useState([]); + const [importLibraryIds, setImportLibraryIds] = useState([]); + const [sourcePreference, setSourcePreference] = useState([]); const [file, setFile] = useState(null); const [parsedDocument, setParsedDocument] = @@ -232,6 +247,15 @@ export function ReadingProgressTransferSettings() { ); const [importError, setImportError] = useState(null); + const availableSources = useMemo(() => { + const seen = new Set(); + for (const series of parsedDocument?.series ?? []) { + for (const external of series.externalIds ?? []) + seen.add(external.source); + } + return [...seen]; + }, [parsedDocument]); + const exportMutation = useExportReadingProgress(); const importMutation = useImportReadingProgress(); @@ -264,7 +288,9 @@ export function ReadingProgressTransferSettings() { { dryRun, hashMode, - sourcePreference: [], + sourcePreference: + sourcePreference.length > 0 ? sourcePreference : undefined, + libraryIds: importLibraryIds.length > 0 ? importLibraryIds : undefined, conflictPolicy, reattachSessions, acceptStemMatches, @@ -318,6 +344,19 @@ export function ReadingProgressTransferSettings() { Export + } loading={exportMutation.isPending} - onClick={() => exportMutation.mutate(includeSessions)} + onClick={() => + exportMutation.mutate({ + includeSessions, + libraryIds: exportLibraryIds, + }) + } > Download export @@ -368,6 +412,46 @@ export function ReadingProgressTransferSettings() { + { + setImportLibraryIds(value); + clearPreview(); + }} + clearable + searchable + /> + + {availableSources.length > 0 && ( + ({ + value: source, + label: source, + }))} + value={sourcePreference} + disabled={busy} + onChange={(value) => { + setSourcePreference(value); + clearPreview(); + }} + clearable + /> + )} +