make reposition undoable

ftl/core/browsing.ftl

@@ -146,3 +146,8 @@ browsing-removed-unused-tags-count =
         [one] Removed { $count } unused tag.
        *[other] Removed { $count } unused tags.
     }
+browsing-changed-new-position =
+    { $count ->
+        [one] Changed position of { $count } new card.
+       *[other] Changed position of { $count } new cards.
+    }

pylib/anki/scheduler/base.py

@@ -5,7 +5,7 @@ from __future__ import annotations
 
 import anki
 import anki._backend.backend_pb2 as _pb
-from anki.collection import OpChanges
+from anki.collection import OpChanges, OpChangesWithCount
 from anki.config import Config
 
 SchedTimingToday = _pb.SchedTimingTodayOut
@@ -167,20 +167,20 @@ select id from cards where did in %s and queue = {QUEUE_TYPE_REV} and due <= ? l
     # Repositioning new cards
     ##########################################################################
 
-    def sortCards(
+    def reposition_new_cards(
         self,
-        cids: List[int],
+        card_ids: Sequence[int],
-        start: int = 1,
+        starting_from: int,
-        step: int = 1,
+        step_size: int,
-        shuffle: bool = False,
+        randomize: bool,
-        shift: bool = False,
+        shift_existing: bool,
-    ) -> None:
+    ) -> OpChangesWithCount:
-        self.col._backend.sort_cards(
+        return self.col._backend.sort_cards(
-            card_ids=cids,
+            card_ids=card_ids,
-            starting_from=start,
+            starting_from=starting_from,
-            step_size=step,
+            step_size=step_size,
-            randomize=shuffle,
+            randomize=randomize,
-            shift_existing=shift,
+            shift_existing=shift_existing,
         )
 
     def randomizeCards(self, did: int) -> None:
@@ -204,3 +204,14 @@ select id from cards where did in %s and queue = {QUEUE_TYPE_REV} and due <= ? l
         # in order due?
         if conf["new"]["order"] == NEW_CARDS_RANDOM:
             self.randomizeCards(did)
+
+    # legacy
+    def sortCards(
+        self,
+        cids: List[int],
+        start: int = 1,
+        step: int = 1,
+        shuffle: bool = False,
+        shift: bool = False,
+    ) -> None:
+        self.reposition_new_cards(cids, start, step, shuffle, shift)

pylib/tests/test_schedv1.py

@@ -1023,62 +1023,6 @@ def test_deckFlow():
         col.sched.answerCard(c, 2)
 
 
-def test_reorder():
-    col = getEmptyCol()
-    # add a note with default deck
-    note = col.newNote()
-    note["Front"] = "one"
-    col.addNote(note)
-    note2 = col.newNote()
-    note2["Front"] = "two"
-    col.addNote(note2)
-    assert note2.cards()[0].due == 2
-    found = False
-    # 50/50 chance of being reordered
-    for i in range(20):
-        col.sched.randomizeCards(1)
-        if note.cards()[0].due != note.id:
-            found = True
-            break
-    assert found
-    col.sched.orderCards(1)
-    assert note.cards()[0].due == 1
-    # shifting
-    note3 = col.newNote()
-    note3["Front"] = "three"
-    col.addNote(note3)
-    note4 = col.newNote()
-    note4["Front"] = "four"
-    col.addNote(note4)
-    assert note.cards()[0].due == 1
-    assert note2.cards()[0].due == 2
-    assert note3.cards()[0].due == 3
-    assert note4.cards()[0].due == 4
-    col.sched.sortCards([note3.cards()[0].id, note4.cards()[0].id], start=1, shift=True)
-    assert note.cards()[0].due == 3
-    assert note2.cards()[0].due == 4
-    assert note3.cards()[0].due == 1
-    assert note4.cards()[0].due == 2
-
-
-def test_forget():
-    col = getEmptyCol()
-    note = col.newNote()
-    note["Front"] = "one"
-    col.addNote(note)
-    c = note.cards()[0]
-    c.queue = QUEUE_TYPE_REV
-    c.type = CARD_TYPE_REV
-    c.ivl = 100
-    c.due = 0
-    c.flush()
-    col.reset()
-    assert col.sched.counts() == (0, 0, 1)
-    col.sched.forgetCards([c.id])
-    col.reset()
-    assert col.sched.counts() == (1, 0, 0)
-
-
 def test_norelearn():
     col = getEmptyCol()
     # add a note

pylib/tests/test_schedv2.py

@@ -1211,7 +1211,13 @@ def test_reorder():
     assert note2.cards()[0].due == 2
     assert note3.cards()[0].due == 3
     assert note4.cards()[0].due == 4
-    col.sched.sortCards([note3.cards()[0].id, note4.cards()[0].id], start=1, shift=True)
+    col.sched.reposition_new_cards(
+        [note3.cards()[0].id, note4.cards()[0].id],
+        starting_from=1,
+        shift_existing=True,
+        step_size=1,
+        randomize=False,
+    )
     assert note.cards()[0].due == 3
     assert note2.cards()[0].due == 4
     assert note3.cards()[0].due == 1

qt/aqt/browser.py

@@ -37,6 +37,7 @@ from aqt.previewer import Previewer
 from aqt.qt import *
 from aqt.scheduling_ops import (
     forget_cards,
+    reposition_new_cards_dialog,
     set_due_date_dialog,
     suspend_cards,
     unsuspend_cards,
@@ -247,7 +248,7 @@ class DataModel(QAbstractTableModel):
         self.endReset()
 
     def saveSelection(self) -> None:
-        cards = self.browser.selectedCards()
+        cards = self.browser.selected_cards()
         self.selectedCards = {id: True for id in cards}
         if getattr(self.browser, "card", None):
             self.focusedCard = self.browser.card.id
@@ -1076,13 +1077,13 @@ QTableView {{ gridline-color: {grid} }}
     # Menu helpers
     ######################################################################
 
-    def selectedCards(self) -> List[int]:
+    def selected_cards(self) -> List[int]:
         return [
             self.model.cards[idx.row()]
             for idx in self.form.tableView.selectionModel().selectedRows()
         ]
 
-    def selectedNotes(self) -> List[int]:
+    def selected_notes(self) -> List[int]:
         return self.col.db.list(
             """
 select distinct nid from cards
@@ -1098,11 +1099,11 @@ where id in %s"""
     def selectedNotesAsCards(self) -> List[int]:
         return self.col.db.list(
             "select id from cards where nid in (%s)"
-            % ",".join([str(s) for s in self.selectedNotes()])
+            % ",".join([str(s) for s in self.selected_notes()])
         )
 
     def oneModelNotes(self) -> List[int]:
-        sf = self.selectedNotes()
+        sf = self.selected_notes()
         if not sf:
             return []
         mods = self.col.db.scalar(
@@ -1119,6 +1120,11 @@ where id in %s"""
     def onHelp(self) -> None:
         openHelp(HelpPage.BROWSING)
 
+    # legacy
+
+    selectedCards = selected_cards
+    selectedNotes = selected_notes
+
     # Misc menu options
     ######################################################################
 
@@ -1174,7 +1180,7 @@ where id in %s"""
             return
 
         # nothing selected?
-        nids = self.selectedNotes()
+        nids = self.selected_notes()
         if not nids:
             return
 
@@ -1198,7 +1204,7 @@ where id in %s"""
     def set_deck_of_selected_cards(self) -> None:
         from aqt.studydeck import StudyDeck
 
-        cids = self.selectedCards()
+        cids = self.selected_cards()
         if not cids:
             return
 
@@ -1235,7 +1241,7 @@ where id in %s"""
             tags := tags or self._prompt_for_tags(tr(TR.BROWSING_ENTER_TAGS_TO_ADD))
         ):
             return
-        add_tags(mw=self.mw, note_ids=self.selectedNotes(), space_separated_tags=tags)
+        add_tags(mw=self.mw, note_ids=self.selected_notes(), space_separated_tags=tags)
 
     @ensure_editor_saved_on_trigger
     def remove_tags_from_selected_notes(self, tags: Optional[str] = None) -> None:
@@ -1245,7 +1251,7 @@ where id in %s"""
         ):
             return
         remove_tags(
-            mw=self.mw, note_ids=self.selectedNotes(), space_separated_tags=tags
+            mw=self.mw, note_ids=self.selected_notes(), space_separated_tags=tags
         )
 
     def _prompt_for_tags(self, prompt: str) -> Optional[str]:
@@ -1272,7 +1278,7 @@ where id in %s"""
     @ensure_editor_saved_on_trigger
     def suspend_selected_cards(self) -> None:
         want_suspend = not self.current_card_is_suspended()
-        cids = self.selectedCards()
+        cids = self.selected_cards()
 
         if want_suspend:
             suspend_cards(mw=self.mw, card_ids=cids)
@@ -1300,7 +1306,7 @@ where id in %s"""
         if flag == self.card.user_flag():
             flag = 0
 
-        cids = self.selectedCards()
+        cids = self.selected_cards()
         set_card_flag(mw=self.mw, card_ids=cids, flag=flag)
 
     def _updateFlagsMenu(self) -> None:
@@ -1331,57 +1337,25 @@ where id in %s"""
     def isMarked(self) -> bool:
         return bool(self.card and self.card.note().has_tag("Marked"))
 
-    # Repositioning
+    # Scheduling
     ######################################################################
 
     @ensure_editor_saved_on_trigger
     def reposition(self) -> None:
-        cids = self.selectedCards()
+        if self.card and self.card.queue != QUEUE_TYPE_NEW:
-        cids2 = self.col.db.list(
+            showInfo(tr(TR.BROWSING_ONLY_NEW_CARDS_CAN_BE_REPOSITIONED), parent=self)
-            f"select id from cards where type = {CARD_TYPE_NEW} and id in "
-            + ids2str(cids)
-        )
-        if not cids2:
-            showInfo(tr(TR.BROWSING_ONLY_NEW_CARDS_CAN_BE_REPOSITIONED))
             return
-        d = QDialog(self)
-        disable_help_button(d)
-        d.setWindowModality(Qt.WindowModal)
-        frm = aqt.forms.reposition.Ui_Dialog()
-        frm.setupUi(d)
-        (pmin, pmax) = self.col.db.first(
-            f"select min(due), max(due) from cards where type={CARD_TYPE_NEW} and odid=0"
-        )
-        pmin = pmin or 0
-        pmax = pmax or 0
-        txt = tr(TR.BROWSING_QUEUE_TOP, val=pmin)
-        txt += "\n" + tr(TR.BROWSING_QUEUE_BOTTOM, val=pmax)
-        frm.label.setText(txt)
-        frm.start.selectAll()
-        if not d.exec_():
-            return
-        self.model.beginReset()
-        self.mw.checkpoint(tr(TR.ACTIONS_REPOSITION))
-        self.col.sched.sortCards(
-            cids,
-            start=frm.start.value(),
-            step=frm.step.value(),
-            shuffle=frm.randomize.isChecked(),
-            shift=frm.shift.isChecked(),
-        )
-        self.search()
-        self.mw.requireReset(reason=ResetReason.BrowserReposition, context=self)
-        self.model.endReset()
 
-    # Scheduling
+        reposition_new_cards_dialog(
-    ######################################################################
+            mw=self.mw, parent=self, card_ids=self.selected_cards()
+        )
 
     @ensure_editor_saved_on_trigger
     def set_due_date(self) -> None:
         set_due_date_dialog(
             mw=self.mw,
             parent=self,
-            card_ids=self.selectedCards(),
+            card_ids=self.selected_cards(),
             config_key=Config.String.SET_DUE_BROWSER,
         )
 
@@ -1390,7 +1364,7 @@ where id in %s"""
         forget_cards(
             mw=self.mw,
             parent=self,
-            card_ids=self.selectedCards(),
+            card_ids=self.selected_cards(),
         )
 
     # Edit: selection
@@ -1398,7 +1372,7 @@ where id in %s"""
 
     @ensure_editor_saved_on_trigger
     def selectNotes(self) -> None:
-        nids = self.selectedNotes()
+        nids = self.selected_notes()
         # clear the selection so we don't waste energy preserving it
         tv = self.form.tableView
         tv.selectionModel().clear()
@@ -1465,7 +1439,7 @@ where id in %s"""
 
     @ensure_editor_saved_on_trigger
     def onFindReplace(self) -> None:
-        nids = self.selectedNotes()
+        nids = self.selected_notes()
         if not nids:
             return
         import anki.find

qt/aqt/scheduling_ops.py

@@ -6,12 +6,12 @@ from __future__ import annotations
 from typing import List, Optional, Sequence
 
 import aqt
-from anki.collection import Config
+from anki.collection import CARD_TYPE_NEW, Config
 from anki.lang import TR
 from aqt import AnkiQt
 from aqt.main import PerformOpOptionalSuccessCallback
 from aqt.qt import *
-from aqt.utils import getText, tooltip, tr
+from aqt.utils import disable_help_button, getText, tooltip, tr
 
 
 def set_due_date_dialog(
@@ -63,6 +63,72 @@ def forget_cards(*, mw: aqt.AnkiQt, parent: QWidget, card_ids: List[int]) -> Non
     )
 
 
+def reposition_new_cards_dialog(
+    *, mw: AnkiQt, parent: QWidget, card_ids: Sequence[int]
+) -> None:
+    assert mw.col.db
+    row = mw.col.db.first(
+        f"select min(due), max(due) from cards where type={CARD_TYPE_NEW} and odid=0"
+    )
+    assert row
+    (min_position, max_position) = row
+    min_position = max(min_position or 0, 0)
+    max_position = max_position or 0
+
+    d = QDialog(parent)
+    disable_help_button(d)
+    d.setWindowModality(Qt.WindowModal)
+    frm = aqt.forms.reposition.Ui_Dialog()
+    frm.setupUi(d)
+
+    txt = tr(TR.BROWSING_QUEUE_TOP, val=min_position)
+    txt += "\n" + tr(TR.BROWSING_QUEUE_BOTTOM, val=max_position)
+    frm.label.setText(txt)
+
+    frm.start.selectAll()
+    if not d.exec_():
+        return
+
+    start = frm.start.value()
+    step = frm.step.value()
+    randomize = frm.randomize.isChecked()
+    shift = frm.shift.isChecked()
+
+    reposition_new_cards(
+        mw=mw,
+        parent=parent,
+        card_ids=card_ids,
+        starting_from=start,
+        step_size=step,
+        randomize=randomize,
+        shift_existing=shift,
+    )
+
+
+def reposition_new_cards(
+    *,
+    mw: AnkiQt,
+    parent: QWidget,
+    card_ids: Sequence[int],
+    starting_from: int,
+    step_size: int,
+    randomize: bool,
+    shift_existing: bool,
+) -> None:
+    mw.perform_op(
+        lambda: mw.col.sched.reposition_new_cards(
+            card_ids=card_ids,
+            starting_from=starting_from,
+            step_size=step_size,
+            randomize=randomize,
+            shift_existing=shift_existing,
+        ),
+        success=lambda out: tooltip(
+            tr(TR.BROWSING_CHANGED_NEW_POSITION, count=out.count), parent=parent
+        ),
+    )
+
+
 def suspend_cards(
     *,
     mw: AnkiQt,

rslib/backend.proto

@@ -120,8 +120,8 @@ service SchedulingService {
   rpc RebuildFilteredDeck(DeckID) returns (UInt32);
   rpc ScheduleCardsAsNew(ScheduleCardsAsNewIn) returns (OpChanges);
   rpc SetDueDate(SetDueDateIn) returns (OpChanges);
-  rpc SortCards(SortCardsIn) returns (Empty);
+  rpc SortCards(SortCardsIn) returns (OpChangesWithCount);
-  rpc SortDeck(SortDeckIn) returns (Empty);
+  rpc SortDeck(SortDeckIn) returns (OpChangesWithCount);
   rpc GetNextCardStates(CardID) returns (NextCardStates);
   rpc DescribeNextStates(NextCardStates) returns (StringList);
   rpc StateIsLeech(SchedulingState) returns (Bool);

rslib/src/backend/scheduler/mod.rs

@@ -118,7 +118,7 @@ impl SchedulingService for Backend {
         self.with_col(|col| col.set_due_date(&cids, &days, config).map(Into::into))
     }
 
-    fn sort_cards(&self, input: pb::SortCardsIn) -> Result<pb::Empty> {
+    fn sort_cards(&self, input: pb::SortCardsIn) -> Result<pb::OpChangesWithCount> {
         let cids: Vec<_> = input.card_ids.into_iter().map(CardID).collect();
         let (start, step, random, shift) = (
             input.starting_from,
@@ -137,7 +137,7 @@ impl SchedulingService for Backend {
         })
     }
 
-    fn sort_deck(&self, input: pb::SortDeckIn) -> Result<pb::Empty> {
+    fn sort_deck(&self, input: pb::SortDeckIn) -> Result<pb::OpChangesWithCount> {
         self.with_col(|col| {
             col.sort_deck(input.deck_id.into(), input.randomize)
                 .map(Into::into)

rslib/src/ops.rs

@@ -18,6 +18,7 @@ pub enum Op {
     SetDeck,
     SetDueDate,
     SetFlag,
+    SortCards,
     Suspend,
     UnburyUnsuspend,
     UpdateCard,
@@ -50,6 +51,7 @@ impl Op {
             Op::SetFlag => TR::UndoSetFlag,
             Op::FindAndReplace => TR::BrowsingFindAndReplace,
             Op::ClearUnusedTags => TR::BrowsingClearUnusedTags,
+            Op::SortCards => TR::BrowsingReschedule,
         };
 
         i18n.tr(key).to_string()

rslib/src/scheduler/new.rs

@@ -24,12 +24,14 @@ impl Card {
         self.ease_factor = 0;
     }
 
-    /// If the card is new, change its position.
+    /// If the card is new, change its position, and return true.
-    fn set_new_position(&mut self, position: u32) {
+    fn set_new_position(&mut self, position: u32) -> bool {
         if self.queue != CardQueue::New || self.ctype != CardType::New {
-            return;
+            false
-        }
+        } else {
             self.due = position as i32;
+            true
+        }
     }
 }
 pub(crate) struct NewCardSorter {
@@ -130,9 +132,9 @@ impl Collection {
         step: u32,
         order: NewCardSortOrder,
         shift: bool,
-    ) -> Result<()> {
+    ) -> Result<OpOutput<usize>> {
         let usn = self.usn()?;
-        self.transact_no_undo(|col| {
+        self.transact(Op::SortCards, |col| {
             col.sort_cards_inner(cids, starting_from, step, order, shift, usn)
         })
     }
@@ -145,24 +147,28 @@ impl Collection {
         order: NewCardSortOrder,
         shift: bool,
         usn: Usn,
-    ) -> Result<()> {
+    ) -> Result<usize> {
         if shift {
             self.shift_existing_cards(starting_from, step * cids.len() as u32, usn)?;
         }
         self.storage.set_search_table_to_card_ids(cids, true)?;
         let cards = self.storage.all_searched_cards_in_search_order()?;
         let sorter = NewCardSorter::new(&cards, starting_from, step, order);
+        let mut count = 0;
         for mut card in cards {
             let original = card.clone();
-            card.set_new_position(sorter.position(&card));
+            if card.set_new_position(sorter.position(&card)) {
+                count += 1;
                 self.update_card_inner(&mut card, original, usn)?;
             }
-        self.storage.clear_searched_cards_table()
+        }
+        self.storage.clear_searched_cards_table()?;
+        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<()> {
+    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