From 1df86b28e8231f5c7089bdd361a9b25bd5a504d9 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Wed, 28 Apr 2021 21:08:09 +1000 Subject: [PATCH] sort deck on config update; fix id not being updated after deletion --- rslib/src/backend/scheduler/mod.rs | 2 +- rslib/src/deckconfig/update.rs | 75 +++++++++++++++++++++++------- rslib/src/scheduler/new.rs | 49 +++++++++++++------ 3 files changed, 92 insertions(+), 34 deletions(-) diff --git a/rslib/src/backend/scheduler/mod.rs b/rslib/src/backend/scheduler/mod.rs index a90f01265..1556f78fe 100644 --- a/rslib/src/backend/scheduler/mod.rs +++ b/rslib/src/backend/scheduler/mod.rs @@ -147,7 +147,7 @@ impl SchedulingService for Backend { fn sort_deck(&self, input: pb::SortDeckIn) -> Result { self.with_col(|col| { - col.sort_deck(input.deck_id.into(), input.randomize) + col.sort_deck_legacy(input.deck_id.into(), input.randomize) .map(Into::into) }) } diff --git a/rslib/src/deckconfig/update.rs b/rslib/src/deckconfig/update.rs index 0f6912096..dac3faf67 100644 --- a/rslib/src/deckconfig/update.rs +++ b/rslib/src/deckconfig/update.rs @@ -1,9 +1,12 @@ // Copyright: Ankitects Pty Ltd and contributors // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html -//! Updating configs in bulk, from the deck config screen. +//! Updating configs in bulk, from the deck options screen. -use std::collections::{HashMap, HashSet}; +use std::{ + collections::{HashMap, HashSet}, + iter, +}; use crate::{ backend_proto as pb, @@ -104,34 +107,70 @@ impl Collection { if input.configs.is_empty() { return Err(AnkiError::invalid_input("config not provided")); } + let configs_before_update = self.storage.get_deck_config_map()?; + let mut configs_after_update = configs_before_update.clone(); // handle removals first for dcid in &input.removed_config_ids { self.remove_deck_config_inner(*dcid)?; + configs_after_update.remove(dcid); } // add/update provided configs for conf in &mut input.configs { self.add_or_update_deck_config(conf, false)?; + configs_after_update.insert(conf.id, conf.clone()); } - // point current deck to last config - let usn = self.usn()?; - let config_id = input.configs.last().unwrap().id; - let mut deck = self - .storage - .get_deck(input.target_deck_id)? - .ok_or(AnkiError::NotFound)?; - let original = deck.clone(); - deck.normal_mut()?.config_id = config_id.0; - self.update_deck_inner(&mut deck, original, usn)?; + // get selected deck and possibly children + let selected_deck_ids: HashSet<_> = if input.apply_to_children { + let deck = self + .storage + .get_deck(input.target_deck_id)? + .ok_or(AnkiError::NotFound)?; + self.storage + .child_decks(&deck)? + .iter() + .chain(iter::once(&deck)) + .map(|d| d.id) + .collect() + } else { + [input.target_deck_id].iter().cloned().collect() + }; - if input.apply_to_children { - for mut child_deck in self.storage.child_decks(&deck)? { - let child_original = child_deck.clone(); - if let Ok(normal) = child_deck.normal_mut() { - normal.config_id = config_id.0; - self.update_deck_inner(&mut child_deck, child_original, usn)?; + // loop through all normal decks + let usn = self.usn()?; + let selected_config = input.configs.last().unwrap(); + for deck in self.storage.get_all_decks()? { + if let Ok(normal) = deck.normal() { + let deck_id = deck.id; + + // previous order + let previous_config_id = DeckConfId(normal.config_id); + let previous_order = configs_before_update + .get(&previous_config_id) + .map(|c| c.inner.new_card_order()) + .unwrap_or_default(); + + // if a selected (sub)deck, or its old config was removed, update deck to point to new config + let current_config_id = if selected_deck_ids.contains(&deck.id) + || !configs_after_update.contains_key(&previous_config_id) + { + let mut updated = deck.clone(); + updated.normal_mut()?.config_id = selected_config.id.0; + self.update_deck_inner(&mut updated, deck, usn)?; + selected_config.id + } else { + previous_config_id + }; + + // if new order differs, deck needs re-sorting + let current_order = configs_after_update + .get(¤t_config_id) + .map(|c| c.inner.new_card_order()) + .unwrap_or_default(); + if previous_order != current_order { + self.sort_deck(deck_id, current_order, usn)?; } } } diff --git a/rslib/src/scheduler/new.rs b/rslib/src/scheduler/new.rs index 7c8223195..8b32f8dde 100644 --- a/rslib/src/scheduler/new.rs +++ b/rslib/src/scheduler/new.rs @@ -6,14 +6,10 @@ use std::collections::{HashMap, HashSet}; use rand::seq::SliceRandom; use crate::{ - card::{Card, CardId, CardQueue, CardType}, - collection::Collection, - decks::DeckId, - error::Result, - notes::NoteId, + card::{CardQueue, CardType}, + deckconfig::NewCardOrder, prelude::*, search::SortMode, - types::Usn, }; impl Card { @@ -168,16 +164,30 @@ impl Collection { Ok(count) } - /// This creates a transaction - we probably want to split it out - /// in the future if calling it as part of a deck options update. - pub fn sort_deck(&mut self, deck: DeckId, random: bool) -> Result> { + /// This is handled by update_deck_configs() now; this function has been kept around + /// for now to support the old deck config screen. + pub fn sort_deck_legacy(&mut self, deck: DeckId, random: bool) -> Result> { + self.transact(Op::SortCards, |col| { + col.sort_deck( + deck, + if random { + NewCardOrder::Random + } else { + NewCardOrder::Due + }, + col.usn()?, + ) + }) + } + + pub(crate) fn sort_deck( + &mut self, + deck: DeckId, + order: NewCardOrder, + usn: Usn, + ) -> Result { let cids = self.search_cards(&format!("did:{} is:new", deck), SortMode::NoOrder)?; - let order = if random { - NewCardSortOrder::Random - } else { - NewCardSortOrder::NoteId - }; - self.sort_cards(&cids, 1, 1, order, false) + self.sort_cards_inner(&cids, 1, 1, order.into(), false, usn) } fn shift_existing_cards(&mut self, start: u32, by: u32, usn: Usn) -> Result<()> { @@ -230,3 +240,12 @@ mod test { unreachable!("not random"); } } + +impl From for NewCardSortOrder { + fn from(o: NewCardOrder) -> Self { + match o { + NewCardOrder::Due => NewCardSortOrder::NoteId, + NewCardOrder::Random => NewCardSortOrder::Random, + } + } +}