From 45bb56808a555e5b3e92e9620a57a3809d5388db Mon Sep 17 00:00:00 2001 From: llama <100429699+iamllama@users.noreply.github.com> Date: Tue, 25 Mar 2025 01:45:09 +0800 Subject: [PATCH] Fix deck day limits incorrectly being carried over when importing (#3878) * re-export DayLimit * add and use DeckContext::maybe_correct_day_limits * update existing test * add test * small tweaks * refactor * refactor test --- rslib/src/decks/mod.rs | 1 + .../package/apkg/import/decks.rs | 69 ++++++++++++++++++- 2 files changed, 67 insertions(+), 3 deletions(-) diff --git a/rslib/src/decks/mod.rs b/rslib/src/decks/mod.rs index 91c1a2206..d16ebac49 100644 --- a/rslib/src/decks/mod.rs +++ b/rslib/src/decks/mod.rs @@ -20,6 +20,7 @@ use std::sync::Arc; pub use anki_proto::decks::deck::filtered::search_term::Order as FilteredSearchOrder; pub use anki_proto::decks::deck::filtered::SearchTerm as FilteredSearchTerm; pub use anki_proto::decks::deck::kind_container::Kind as DeckKind; +pub use anki_proto::decks::deck::normal::DayLimit as NormalDeckDayLimit; pub use anki_proto::decks::deck::Common as DeckCommon; pub use anki_proto::decks::deck::Filtered as FilteredDeck; pub use anki_proto::decks::deck::KindContainer as DeckKindContainer; diff --git a/rslib/src/import_export/package/apkg/import/decks.rs b/rslib/src/import_export/package/apkg/import/decks.rs index bba4505a6..38a62dc92 100644 --- a/rslib/src/import_export/package/apkg/import/decks.rs +++ b/rslib/src/import_export/package/apkg/import/decks.rs @@ -6,6 +6,7 @@ use std::mem; use super::Context; use crate::decks::NormalDeck; +use crate::decks::NormalDeckDayLimit; use crate::prelude::*; struct DeckContext<'d> { @@ -14,23 +15,25 @@ struct DeckContext<'d> { renamed_parents: Vec<(String, String)>, imported_decks: HashMap, unique_suffix: String, + source_col_today: u32, } impl<'d> DeckContext<'d> { - fn new<'a: 'd>(target_col: &'a mut Collection, usn: Usn) -> Self { + fn new<'a: 'd>(target_col: &'a mut Collection, usn: Usn, source_col_today: u32) -> Self { Self { target_col, usn, renamed_parents: Vec::new(), imported_decks: HashMap::new(), unique_suffix: TimestampSecs::now().to_string(), + source_col_today, } } } impl Context<'_> { pub(super) fn import_decks_and_configs(&mut self) -> Result> { - let mut ctx = DeckContext::new(self.target_col, self.usn); + let mut ctx = DeckContext::new(self.target_col, self.usn, self.data.days_elapsed); ctx.import_deck_configs(mem::take(&mut self.data.deck_configs))?; ctx.import_decks(mem::take(&mut self.data.decks))?; Ok(ctx.imported_decks) @@ -51,6 +54,7 @@ impl DeckContext<'_> { decks.sort_unstable_by_key(|deck| deck.level()); for deck in &mut decks { self.maybe_reparent(deck); + self.maybe_correct_day_limits(deck)?; self.import_deck(deck)?; } Ok(()) @@ -83,6 +87,29 @@ impl DeckContext<'_> { }) } + fn maybe_correct_day_limits(&mut self, deck: &mut Deck) -> Result<()> { + if let Ok(normal) = deck.normal_mut() { + let target_col_today = self.target_col.timing_today()?.days_elapsed; + let op = |mut limit: NormalDeckDayLimit| { + if limit.today == self.source_col_today { + // imported deck has an active today limit, map it to target col + limit.today = target_col_today; + Some(limit) + } else if target_col_today > 0 { + // imported deck's today limit was not active + limit.today = limit.today.min(target_col_today - 1); + Some(limit) + } else { + // edge case where target collection is new (day 0), clear saved limit + None + } + }; + normal.new_limit_today = normal.new_limit_today.and_then(op); + normal.review_limit_today = normal.review_limit_today.and_then(op); + } + Ok(()) + } + fn get_deck_by_name(&mut self, deck: &Deck) -> Result> { self.target_col .storage @@ -181,7 +208,7 @@ mod test { DeckAdder::new("filtered").filtered(true).add(&mut col); DeckAdder::new("PARENT").add(&mut col); - let mut ctx = DeckContext::new(&mut col, Usn(1)); + let mut ctx = DeckContext::new(&mut col, Usn(1), 0); ctx.unique_suffix = "★".to_string(); let imports = vec![ @@ -210,4 +237,40 @@ mod test { // the case of imported parents is matched, regardless of pass order assert!(existing_decks.contains("new parent::child")); } + + #[test] + fn day_limits_should_carry_over_correctly() { + let mut col = Collection::new(); + + let importing_col_today = col.timing_today().unwrap().days_elapsed; + let exporting_col_today = importing_col_today + 100; + let deck_name = "blah"; + + let mut exported_deck = DeckAdder::new(deck_name).filtered(false).deck(); + let normal = exported_deck.normal_mut().unwrap(); + + normal.new_limit_today = Some(NormalDeckDayLimit { + limit: 123, + today: exporting_col_today, + }); + normal.review_limit_today = Some(NormalDeckDayLimit { + limit: 456, + today: exporting_col_today - 100, + }); + + let mut ctx = DeckContext::new(&mut col, Usn(1), exporting_col_today); + ctx.import_decks(vec![exported_deck]).unwrap(); + + let imported_deck_id = ctx.target_col.get_deck_id(deck_name).unwrap().unwrap(); + let imported_deck = ctx.target_col.get_deck(imported_deck_id).unwrap().unwrap(); + + let imported_deck = imported_deck.normal().unwrap(); + + // active day limit should carry over regardless of collection age + assert!( + matches!(imported_deck.new_limit_today, Some(NormalDeckDayLimit { limit: 123, today }) if today == importing_col_today) + ); + // target_col's today is 0, expect the day limit to be cleared + assert!(imported_deck.review_limit_today.is_none()) + } }