diff --git a/rslib/src/card/mod.rs b/rslib/src/card/mod.rs index 258ea8285..c3945fd5a 100644 --- a/rslib/src/card/mod.rs +++ b/rslib/src/card/mod.rs @@ -3,7 +3,7 @@ pub(crate) mod undo; -use std::collections::HashSet; +use std::collections::{hash_map::Entry, HashMap, HashSet}; use num_enum::TryFromPrimitive; use serde_repr::{Deserialize_repr, Serialize_repr}; @@ -145,9 +145,7 @@ impl Card { pub fn is_intraday_learning(&self) -> bool { matches!(self.queue, CardQueue::Learn | CardQueue::PreviewRepeat) } -} -impl Card { pub fn new(note_id: NoteId, template_idx: u16, deck_id: DeckId, due: i32) -> Self { Card { note_id, @@ -157,6 +155,27 @@ impl Card { ..Default::default() } } + + /// Remaining steps after configured steps have changed, disregarding "remaining today". + /// [None] if same as before. A step counts as remaining if the card has not passed a step + /// with the same or a greater delay, but output will be at least 1. + fn new_remaining_steps(&self, new_steps: &[f32], old_steps: &[f32]) -> Option { + let remaining = self.remaining_steps(); + let new_remaining = old_steps + .len() + .checked_sub(remaining as usize + 1) + .and_then(|last_index| { + new_steps + .iter() + .rev() + .position(|&step| step <= old_steps[last_index]) + }) + // no last delay or last delay is less than all new steps → all steps remain + .unwrap_or(new_steps.len()) + // (re)learning card must have at least 1 step remaining + .max(1) as u32; + (remaining != new_remaining).then(|| new_remaining) + } } impl Collection { @@ -250,9 +269,11 @@ impl Collection { pub fn set_deck(&mut self, cards: &[CardId], deck_id: DeckId) -> Result> { let deck = self.get_deck(deck_id)?.ok_or(AnkiError::NotFound)?; - if deck.is_filtered() { - return Err(FilteredDeckError::CanNotMoveCardsInto.into()); - } + let config_id = deck.config_id().ok_or(AnkiError::FilteredDeckError( + FilteredDeckError::CanNotMoveCardsInto, + ))?; + let config = self.get_deck_config(config_id, true)?.unwrap(); + let mut steps_adjuster = RemainingStepsAdjuster::new(&config); self.storage.set_search_table_to_card_ids(cards, false)?; let sched = self.scheduler_version(); let usn = self.usn()?; @@ -264,6 +285,7 @@ impl Collection { } count += 1; let original = card.clone(); + steps_adjuster.adjust_remaining_steps(col, &mut card)?; card.set_deck(deck_id, sched); col.update_card_inner(&mut card, original, usn)?; } @@ -306,4 +328,95 @@ impl Collection { Ok(DeckConfig::default()) } + + /// Adjust the remaining steps of the card according to the steps change. + /// Steps must be learning or relearning steps according to the card's type. + pub(crate) fn adjust_remaining_steps( + &mut self, + card: &mut Card, + old_steps: &[f32], + new_steps: &[f32], + usn: Usn, + ) -> Result<()> { + if let Some(new_remaining) = card.new_remaining_steps(new_steps, old_steps) { + let original = card.clone(); + card.remaining_steps = new_remaining; + self.update_card_inner(card, original, usn) + } else { + Ok(()) + } + } +} + +/// Adjusts the remaining steps of cards after their deck config has changed. +struct RemainingStepsAdjuster<'a> { + learn_steps: &'a [f32], + relearn_steps: &'a [f32], + configs: HashMap, +} + +impl<'a> RemainingStepsAdjuster<'a> { + fn new(new_config: &'a DeckConfig) -> Self { + RemainingStepsAdjuster { + learn_steps: &new_config.inner.learn_steps, + relearn_steps: &new_config.inner.relearn_steps, + configs: HashMap::new(), + } + } + + fn adjust_remaining_steps(&mut self, col: &mut Collection, card: &mut Card) -> Result<()> { + if let Some(remaining) = match card.ctype { + CardType::Learn => card.new_remaining_steps( + self.learn_steps, + &self.config_for_card(col, card)?.inner.learn_steps, + ), + CardType::Relearn => card.new_remaining_steps( + self.relearn_steps, + &self.config_for_card(col, card)?.inner.relearn_steps, + ), + _ => None, + } { + card.remaining_steps = remaining; + } + Ok(()) + } + + fn config_for_card(&mut self, col: &mut Collection, card: &Card) -> Result<&mut DeckConfig> { + Ok( + match self.configs.entry(card.original_or_current_deck_id()) { + Entry::Occupied(e) => e.into_mut(), + Entry::Vacant(e) => e.insert(col.deck_config_for_card(card)?), + }, + ) + } +} + +#[cfg(test)] +mod test { + use crate::tests::{ + open_test_collection_with_learning_card, open_test_collection_with_relearning_card, + DeckAdder, + }; + + #[test] + fn should_increase_remaining_learning_steps_if_new_deck_has_more_unpassed_ones() { + let mut col = open_test_collection_with_learning_card(); + let deck = DeckAdder::new("target") + .with_config(|config| config.inner.learn_steps.push(100.)) + .add(&mut col); + let card_id = col.get_first_card().id; + col.set_deck(&[card_id], deck.id).unwrap(); + assert_eq!(col.get_first_card().remaining_steps, 3); + } + + #[test] + fn should_increase_remaining_relearning_steps_if_new_deck_has_more_unpassed_ones() { + let mut col = open_test_collection_with_relearning_card(); + let deck = DeckAdder::new("target") + .with_config(|config| config.inner.relearn_steps.push(100.)) + .add(&mut col); + let card_id = col.get_first_card().id; + col.set_deck(&[card_id], deck.id).unwrap(); + assert_eq!(col.get_first_card().remaining_steps, 2); + } } diff --git a/rslib/src/deckconfig/update.rs b/rslib/src/deckconfig/update.rs index 3b464d3f9..36e5061d3 100644 --- a/rslib/src/deckconfig/update.rs +++ b/rslib/src/deckconfig/update.rs @@ -13,6 +13,7 @@ use crate::{ pb, pb::deck_configs_for_update::{ConfigWithExtra, CurrentDeck}, prelude::*, + search::{JoinSearches, SearchNode}, }; #[derive(Debug, Clone)] @@ -153,8 +154,8 @@ impl Collection { // previous order let previous_config_id = DeckConfigId(normal.config_id); - let previous_order = configs_before_update - .get(&previous_config_id) + let previous_config = configs_before_update.get(&previous_config_id); + let previous_order = previous_config .map(|c| c.inner.new_card_insert_order()) .unwrap_or_default(); @@ -171,13 +172,15 @@ impl Collection { }; // if new order differs, deck needs re-sorting - let current_order = configs_after_update - .get(¤t_config_id) + let current_config = configs_after_update.get(¤t_config_id); + let current_order = current_config .map(|c| c.inner.new_card_insert_order()) .unwrap_or_default(); if previous_order != current_order { self.sort_deck(deck_id, current_order, usn)?; } + + self.adjust_remaining_steps_in_deck(deck_id, previous_config, current_config, usn)?; } } @@ -185,12 +188,51 @@ impl Collection { Ok(()) } + + /// Adjust the remaining steps of cards in the given deck according to the config change. + fn adjust_remaining_steps_in_deck( + &mut self, + deck: DeckId, + previous_config: Option<&DeckConfig>, + current_config: Option<&DeckConfig>, + usn: Usn, + ) -> Result<()> { + if let (Some(old), Some(new)) = (previous_config, current_config) { + for (search, old_steps, new_steps) in [ + ( + SearchBuilder::learning_cards(), + &old.inner.learn_steps, + &new.inner.learn_steps, + ), + ( + SearchBuilder::relearning_cards(), + &old.inner.relearn_steps, + &new.inner.relearn_steps, + ), + ] { + if old_steps == new_steps { + continue; + } + let search = search.clone().and(SearchNode::from_deck_id(deck, false)); + for mut card in self.all_cards_for_search(search)? { + self.adjust_remaining_steps(&mut card, old_steps, new_steps, usn)?; + } + } + } + Ok(()) + } } #[cfg(test)] mod test { use super::*; - use crate::{collection::open_test_collection, deckconfig::NewCardInsertOrder}; + use crate::{ + collection::open_test_collection, + deckconfig::NewCardInsertOrder, + tests::{ + open_test_collection_with_learning_card, open_test_collection_with_relearning_card, + }, + }; #[test] fn updating() -> Result<()> { @@ -284,4 +326,66 @@ mod test { Ok(()) } + + #[test] + fn should_increase_remaining_learning_steps_if_unpassed_learning_step_added() { + let mut col = open_test_collection_with_learning_card(); + col.set_default_learn_steps(vec![1., 10., 100.]); + assert_eq!(col.get_first_card().remaining_steps, 3); + } + + #[test] + fn should_keep_remaining_learning_steps_if_unpassed_relearning_step_added() { + let mut col = open_test_collection_with_learning_card(); + col.set_default_relearn_steps(vec![1., 10., 100.]); + assert_eq!(col.get_first_card().remaining_steps, 2); + } + + #[test] + fn should_keep_remaining_learning_steps_if_passed_learning_step_added() { + let mut col = open_test_collection_with_learning_card(); + col.answer_good(); + col.set_default_learn_steps(vec![1., 1., 10.]); + assert_eq!(col.get_first_card().remaining_steps, 1); + } + + #[test] + fn should_keep_at_least_one_remaining_learning_step() { + let mut col = open_test_collection_with_learning_card(); + col.answer_good(); + col.set_default_learn_steps(vec![1.]); + assert_eq!(col.get_first_card().remaining_steps, 1); + } + + #[test] + fn should_increase_remaining_relearning_steps_if_unpassed_relearning_step_added() { + let mut col = open_test_collection_with_relearning_card(); + col.set_default_relearn_steps(vec![1., 10., 100.]); + assert_eq!(col.get_first_card().remaining_steps, 3); + } + + #[test] + fn should_keep_remaining_relearning_steps_if_unpassed_learning_step_added() { + let mut col = open_test_collection_with_relearning_card(); + col.set_default_learn_steps(vec![1., 10., 100.]); + assert_eq!(col.get_first_card().remaining_steps, 1); + } + + #[test] + fn should_keep_remaining_relearning_steps_if_passed_relearning_step_added() { + let mut col = open_test_collection_with_relearning_card(); + col.set_default_relearn_steps(vec![10., 100.]); + col.answer_good(); + col.set_default_relearn_steps(vec![1., 10., 100.]); + assert_eq!(col.get_first_card().remaining_steps, 1); + } + + #[test] + fn should_keep_at_least_one_remaining_relearning_step() { + let mut col = open_test_collection_with_relearning_card(); + col.set_default_relearn_steps(vec![10., 100.]); + col.answer_good(); + col.set_default_relearn_steps(vec![1.]); + assert_eq!(col.get_first_card().remaining_steps, 1); + } } diff --git a/rslib/src/decks/name.rs b/rslib/src/decks/name.rs index 2f8a30086..1b3058982 100644 --- a/rslib/src/decks/name.rs +++ b/rslib/src/decks/name.rs @@ -6,7 +6,7 @@ use itertools::Itertools; use crate::{prelude::*, text::normalize_to_nfc}; -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone, Default, PartialEq)] pub struct NativeDeckName(String); impl NativeDeckName { diff --git a/rslib/src/search/builder.rs b/rslib/src/search/builder.rs index dab57edfa..e17ccfe3f 100644 --- a/rslib/src/search/builder.rs +++ b/rslib/src/search/builder.rs @@ -113,6 +113,21 @@ impl SearchBuilder { pub fn write(&self) -> String { write_nodes(&self.0) } + + /// Construct [SearchBuilder] matching any given deck, excluding children. + pub fn from_decks(decks: &[DeckId]) -> Self { + Self::any(decks.iter().copied().map(SearchNode::DeckIdWithoutChildren)) + } + + /// Construct [SearchBuilder] matching learning, but not relearning cards. + pub fn learning_cards() -> Self { + StateKind::Learning.and(StateKind::Review.negated()) + } + + /// Construct [SearchBuilder] matching relearning cards. + pub fn relearning_cards() -> Self { + StateKind::Learning.and(StateKind::Review) + } } impl> From for SearchBuilder { diff --git a/rslib/src/search/mod.rs b/rslib/src/search/mod.rs index 721c57b80..dd17cb01d 100644 --- a/rslib/src/search/mod.rs +++ b/rslib/src/search/mod.rs @@ -18,7 +18,7 @@ pub use writer::replace_search_node; use crate::{ browser_table::Column, - card::{CardId, CardType}, + card::{Card, CardId, CardType}, collection::Collection, error::Result, notes::NoteId, @@ -216,6 +216,16 @@ impl Collection { .map_err(Into::into) } + pub(crate) fn all_cards_for_search(&mut self, search: N) -> Result> + where + N: TryIntoSearch, + { + self.search_cards_into_table(search, SortMode::NoOrder)?; + let cards = self.storage.all_searched_cards(); + self.storage.clear_searched_cards_table()?; + cards + } + /// Place the matched note ids into a temporary 'search_nids' table /// instead of returning them. Use clear_searched_notes() to remove it. /// Returns number of added notes. diff --git a/rslib/src/storage/card/mod.rs b/rslib/src/storage/card/mod.rs index 96b314486..0b5c364e2 100644 --- a/rslib/src/storage/card/mod.rs +++ b/rslib/src/storage/card/mod.rs @@ -653,6 +653,17 @@ impl super::SqliteStorage { Ok(()) } + + #[cfg(test)] + pub(crate) fn get_all_cards(&self) -> Vec { + self.db + .prepare("SELECT * FROM cards") + .unwrap() + .query_and_then([], row_to_card) + .unwrap() + .collect::>() + .unwrap() + } } #[derive(Clone, Copy)] diff --git a/rslib/src/tests.rs b/rslib/src/tests.rs index 93a8e3a4d..405e7c9e1 100644 --- a/rslib/src/tests.rs +++ b/rslib/src/tests.rs @@ -5,7 +5,12 @@ use tempfile::{tempdir, TempDir}; -use crate::{collection::CollectionBuilder, media::MediaManager, prelude::*}; +use crate::{ + collection::{open_test_collection, CollectionBuilder}, + deckconfig::UpdateDeckConfigsRequest, + media::MediaManager, + prelude::*, +}; pub(crate) fn open_fs_test_collection(name: &str) -> (Collection, TempDir) { let tempdir = tempdir().unwrap(); @@ -19,6 +24,26 @@ pub(crate) fn open_fs_test_collection(name: &str) -> (Collection, TempDir) { (col, tempdir) } +pub(crate) fn open_test_collection_with_learning_card() -> Collection { + let mut col = open_test_collection(); + col.add_new_note("basic"); + col.answer_again(); + col +} + +pub(crate) fn open_test_collection_with_relearning_card() -> Collection { + let mut col = open_test_collection(); + col.add_new_note("basic"); + col.answer_easy(); + col.storage + .db + .execute_batch("UPDATE cards SET due = 0") + .unwrap(); + col.clear_study_queues(); + col.answer_again(); + col +} + impl Collection { pub(crate) fn add_media(&self, media: &[(&str, &[u8])]) { let mgr = MediaManager::new(&self.media_folder, &self.media_db).unwrap(); @@ -57,6 +82,37 @@ impl Collection { self.add_deck_inner(&mut deck, Usn(1)).unwrap(); deck } + + pub(crate) fn get_first_card(&self) -> Card { + self.storage.get_all_cards().pop().unwrap() + } + + pub(crate) fn get_first_deck_config(&mut self) -> DeckConfig { + self.storage.all_deck_config().unwrap().pop().unwrap() + } + + pub(crate) fn set_default_learn_steps(&mut self, steps: Vec) { + let mut config = self.get_first_deck_config(); + config.inner.learn_steps = steps; + self.update_default_deck_config(config); + } + + pub(crate) fn set_default_relearn_steps(&mut self, steps: Vec) { + let mut config = self.get_first_deck_config(); + config.inner.relearn_steps = steps; + self.update_default_deck_config(config); + } + + pub(crate) fn update_default_deck_config(&mut self, config: DeckConfig) { + self.update_deck_configs(UpdateDeckConfigsRequest { + target_deck_id: DeckId(1), + configs: vec![config], + removed_config_ids: vec![], + apply_to_children: false, + card_state_customizer: "".to_string(), + }) + .unwrap(); + } } pub(crate) fn new_deck_with_machine_name(name: &str, filtered: bool) -> Deck { @@ -68,3 +124,49 @@ pub(crate) fn new_deck_with_machine_name(name: &str, filtered: bool) -> Deck { deck.name = NativeDeckName::from_native_str(name); deck } + +#[derive(Debug, Default, Clone)] +pub(crate) struct DeckAdder { + name: NativeDeckName, + filtered: bool, + config: Option, +} + +impl DeckAdder { + pub(crate) fn new(machine_name: impl Into) -> Self { + Self { + name: NativeDeckName::from_native_str(machine_name), + ..Default::default() + } + } + + #[allow(dead_code)] + pub(crate) fn filtered(mut self, filtered: bool) -> Self { + self.filtered = filtered; + self + } + + pub(crate) fn with_config(mut self, modifier: impl FnOnce(&mut DeckConfig)) -> Self { + let mut config = DeckConfig::default(); + modifier(&mut config); + self.config = Some(config); + self + } + + pub(crate) fn add(self, col: &mut Collection) -> Deck { + let mut deck = if self.filtered { + Deck::new_filtered() + } else { + Deck::new_normal() + }; + deck.name = self.name; + if let Some(mut config) = self.config { + col.add_or_update_deck_config(&mut config).unwrap(); + deck.normal_mut() + .expect("can't set config for filtered deck") + .config_id = config.id.0; + } + col.add_or_update_deck(&mut deck).unwrap(); + deck + } +}