sort deck on config update; fix id not being updated after deletion

rslib/src/backend/scheduler/mod.rs

@@ -147,7 +147,7 @@ impl SchedulingService for Backend {
 
     fn sort_deck(&self, input: pb::SortDeckIn) -> Result<pb::OpChangesWithCount> {
         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)
         })
     }

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
+        // 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)?;
-        let original = deck.clone();
-        deck.normal_mut()?.config_id = config_id.0;
-        self.update_deck_inner(&mut deck, original, usn)?;
+            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(&current_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)?;
                 }
             }
         }

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<OpOutput<usize>> {
-        let cids = self.search_cards(&format!("did:{} is:new", deck), SortMode::NoOrder)?;
-        let order = if random {
-            NewCardSortOrder::Random
+    /// 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<OpOutput<usize>> {
+        self.transact(Op::SortCards, |col| {
+            col.sort_deck(
+                deck,
+                if random {
+                    NewCardOrder::Random
                 } else {
-            NewCardSortOrder::NoteId
-        };
-        self.sort_cards(&cids, 1, 1, order, false)
+                    NewCardOrder::Due
+                },
+                col.usn()?,
+            )
+        })
+    }
+
+    pub(crate) fn sort_deck(
+        &mut self,
+        deck: DeckId,
+        order: NewCardOrder,
+        usn: Usn,
+    ) -> Result<usize> {
+        let cids = self.search_cards(&format!("did:{} is:new", deck), SortMode::NoOrder)?;
+        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<NewCardOrder> for NewCardSortOrder {
+    fn from(o: NewCardOrder) -> Self {
+        match o {
+            NewCardOrder::Due => NewCardSortOrder::NoteId,
+            NewCardOrder::Random => NewCardSortOrder::Random,
+        }
+    }
+}