rework filtered deck screen & search errors

- Filtered deck creation now happens as an atomic operation, and is
undoable.
- The logic for initial search text, normalizing searches and so on
has been pushed into the backend.
- Use protobuf to pass the filtered deck to the updated dialog, so
we don't need to deal with untyped JSON.
- Change the "revise your search?" prompt to be a simple info box -
user has access to cancel and build buttons, and doesn't need a separate
prompt. Tweak the wording so the 'show excluded' button should be more
obvious.
- Filtered decks have a time appended to them instead of a number,
primarily because it's easier to implement. No objections going back to
the old behaviour if someone wants to contribute a clean patch.
The standard de-duplication will happen if two decks are created in the
same minute with the same name.
- Tweak the default sort order, and start with two searches. The UI
will still hide the second search by default, but by starting with two,
the frontend doesn't need logic for creating the starting text.
- Search errors now have their own error type, instead of using
InvalidInput, as that was intended mainly for bad API calls. The markdown
conversion is done when the error is converted from the backend, allowing
errors to printed as a string without any special handling by the calling
code.

TODO: when building a new filtered deck, update_active() is clobbering
the undo log when the overview is refreshed

ftl/core/decks.ftl

@@ -29,5 +29,5 @@ decks-repeat-failed-cards-after = Repeat failed cards after
 decks-reschedule-cards-based-on-my-answers = Reschedule cards based on my answers in this deck
 decks-study = Study
 decks-study-deck = Study Deck
-decks-the-provided-search-did-not-match = The provided search did not match any cards. Would you like to revise it?
+decks-filtered-deck-search-empty = No cards matched the provided search. Some cards may have been excluded because they are in a different filtered deck, or suspended.
 decks-unmovable-cards = Show any excluded cards

pylib/anki/decks.py

@@ -19,6 +19,7 @@ from anki.utils import from_json_bytes, ids2str, intTime, legacy_func, to_json_b
 # public exports
 DeckTreeNode = _pb.DeckTreeNode
 DeckNameID = _pb.DeckNameID
+FilteredDeckConfig = _pb.FilteredDeck
 
 # legacy code may pass this in as the type argument to .id()
 defaultDeck = 0
@@ -171,7 +172,16 @@ class DeckManager:
         return list(from_json_bytes(self.col._backend.get_all_decks_legacy()).values())
 
     def new_deck_legacy(self, filtered: bool) -> DeckDict:
-        return from_json_bytes(self.col._backend.new_deck_legacy(filtered))
+        deck = from_json_bytes(self.col._backend.new_deck_legacy(filtered))
+        if deck["dyn"]:
+            # Filtered decks are now created via a scheduler method, but old unit
+            # tests still use this method. Set the default values to what the tests
+            # expect: one empty search term, and ordering by oldest first.
+            del deck["terms"][1]
+            deck["terms"][0][0] = ""
+            deck["terms"][0][2] = 0
+
+        return deck
 
     def deck_tree(self) -> DeckTreeNode:
         return self.col._backend.deck_tree(top_deck_id=0, now=0)

pylib/anki/errors.py

@@ -3,6 +3,8 @@
 
 from __future__ import annotations
 
+from markdown import markdown
+
 import anki._backend.backend_pb2 as _pb
 
 # fixme: notfounderror etc need to be in rsbackend.py
@@ -59,10 +61,18 @@ class DeckIsFilteredError(StringError, DeckRenameError):
     pass
 
 
+class FilteredDeckEmpty(StringError):
+    pass
+
+
 class InvalidInput(StringError):
     pass
 
 
+class SearchError(StringError):
+    pass
+
+
 def backend_exception_to_pylib(err: _pb.BackendError) -> Exception:
     val = err.WhichOneof("value")
     if val == "interrupted":
@@ -87,8 +97,12 @@ def backend_exception_to_pylib(err: _pb.BackendError) -> Exception:
         return ExistsError()
     elif val == "deck_is_filtered":
         return DeckIsFilteredError(err.localized)
+    elif val == "filtered_deck_empty":
+        return FilteredDeckEmpty(err.localized)
     elif val == "proto_error":
         return StringError(err.localized)
+    elif val == "search_error":
+        return SearchError(markdown(err.localized))
     else:
         print("unhandled error type:", val)
         return StringError(err.localized)

pylib/anki/scheduler/__init__.py

@@ -10,6 +10,7 @@ import anki.scheduler.base as _base
 UnburyCurrentDeck = _base.UnburyCurrentDeck
 CongratsInfo = _base.CongratsInfo
 BuryOrSuspend = _base.BuryOrSuspend
+FilteredDeckForUpdate = _base.FilteredDeckForUpdate
 
 # add aliases to the legacy pathnames
 import anki.scheduler.v1

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, OpChangesWithCount
+from anki.collection import OpChanges, OpChangesWithCount, OpChangesWithID
 from anki.config import Config
 
 SchedTimingToday = _pb.SchedTimingTodayOut
@@ -14,13 +14,14 @@ SchedTimingToday = _pb.SchedTimingTodayOut
 from typing import List, Optional, Sequence
 
 from anki.consts import CARD_TYPE_NEW, NEW_CARDS_RANDOM, QUEUE_TYPE_NEW, QUEUE_TYPE_REV
-from anki.decks import DeckConfigDict, DeckTreeNode
+from anki.decks import DeckConfigDict, DeckID, DeckTreeNode
 from anki.notes import Note
 from anki.utils import ids2str, intTime
 
 CongratsInfo = _pb.CongratsInfoOut
 UnburyCurrentDeck = _pb.UnburyCardsInCurrentDeckIn
 BuryOrSuspend = _pb.BuryOrSuspendCardsIn
+FilteredDeckForUpdate = _pb.FilteredDeckForUpdate
 
 
 class SchedulerBase:
@@ -95,6 +96,14 @@ select id from cards where did in %s and queue = {QUEUE_TYPE_REV} and due <= ? l
     def empty_filtered_deck(self, deck_id: int) -> OpChanges:
         return self.col._backend.empty_filtered_deck(deck_id)
 
+    def get_or_create_filtered_deck(self, deck_id: DeckID) -> FilteredDeckForUpdate:
+        return self.col._backend.get_or_create_filtered_deck(deck_id)
+
+    def add_or_update_filtered_deck(
+        self, deck: FilteredDeckForUpdate
+    ) -> OpChangesWithID:
+        return self.col._backend.add_or_update_filtered_deck(deck)
+
     # Suspending & burying
     ##########################################################################
 

qt/aqt/browser.py

@@ -25,7 +25,7 @@ import aqt.forms
 from anki.cards import Card
 from anki.collection import BrowserRow, Collection, Config, OpChanges, SearchNode
 from anki.consts import *
-from anki.errors import InvalidInput, NotFoundError
+from anki.errors import NotFoundError
 from anki.lang import without_unicode_isolation
 from anki.models import NoteType
 from anki.stats import CardStats
@@ -76,8 +76,8 @@ from aqt.utils import (
     saveSplitter,
     saveState,
     shortcut,
-    show_invalid_search_error,
     showInfo,
+    showWarning,
     tooltip,
     tr,
 )
@@ -692,8 +692,8 @@ class Browser(QMainWindow):
         text = self.form.searchEdit.lineEdit().text()
         try:
             normed = self.col.build_search_string(text)
-        except InvalidInput as err:
+        except Exception as err:
-            show_invalid_search_error(err)
+            showWarning(str(err))
         else:
             self.search_for(normed)
             self.update_history()
@@ -718,7 +718,7 @@ class Browser(QMainWindow):
         try:
             self.model.search(self._lastSearchTxt)
         except Exception as err:
-            show_invalid_search_error(err)
+            showWarning(str(err))
         if not self.model.cards:
             # no row change will fire
             self.onRowChanged(None, None)
@@ -1371,8 +1371,7 @@ where id in %s"""
 
     def setupHooks(self) -> None:
         gui_hooks.undo_state_did_change.append(self.onUndoState)
-        # fixme: remove these once all items are using `operation_did_execute`
+        # fixme: remove this once all items are using `operation_did_execute`
-        gui_hooks.sidebar_should_refresh_decks.append(self.on_item_added)
         gui_hooks.sidebar_should_refresh_notetypes.append(self.on_item_added)
         gui_hooks.backend_will_block.append(self.on_backend_will_block)
         gui_hooks.backend_did_block.append(self.on_backend_did_block)
@@ -1381,20 +1380,15 @@ where id in %s"""
 
     def teardownHooks(self) -> None:
         gui_hooks.undo_state_did_change.remove(self.onUndoState)
-        gui_hooks.sidebar_should_refresh_decks.remove(self.on_item_added)
         gui_hooks.sidebar_should_refresh_notetypes.remove(self.on_item_added)
         gui_hooks.backend_will_block.remove(self.on_backend_will_block)
         gui_hooks.backend_did_block.remove(self.on_backend_will_block)
         gui_hooks.operation_did_execute.remove(self.on_operation_did_execute)
         gui_hooks.focus_did_change.remove(self.on_focus_change)
 
-    # covers the tag, note and deck case
     def on_item_added(self, item: Any = None) -> None:
         self.sidebar.refresh()
 
-    def on_tag_list_update(self) -> None:
-        self.sidebar.refresh()
-
     # Undo
     ######################################################################
 
@@ -1477,9 +1471,9 @@ where id in %s"""
         self.mw.progress.start()
         try:
             res = self.mw.col.findDupes(fname, search)
-        except InvalidInput as e:
+        except Exception as e:
             self.mw.progress.finish()
-            show_invalid_search_error(e)
+            showWarning(str(e))
             return
         if not self._dupesButton:
             self._dupesButton = b = frm.buttonBox.addButton(

qt/aqt/filtered_deck.py

@@ -1,73 +1,83 @@
 # Copyright: Ankitects Pty Ltd and contributors
 # License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
-from typing import Callable, List, Optional, Tuple
+
+from typing import List, Optional, Tuple
 
 import aqt
-from anki.collection import SearchNode
+from anki.collection import OpChangesWithCount, SearchNode
-from anki.decks import DeckDict
+from anki.decks import DeckDict, DeckID, FilteredDeckConfig
-from anki.errors import DeckIsFilteredError, InvalidInput
+from anki.errors import SearchError
 from anki.lang import without_unicode_isolation
-from aqt import AnkiQt, colors, gui_hooks
+from anki.scheduler import FilteredDeckForUpdate
+from aqt import AnkiQt, colors
 from aqt.qt import *
+from aqt.scheduling_ops import add_or_update_filtered_deck
 from aqt.theme import theme_manager
 from aqt.utils import (
     TR,
     HelpPage,
-    askUser,
     disable_help_button,
     openHelp,
     restoreGeom,
     saveGeom,
-    show_invalid_search_error,
     showWarning,
     tr,
 )
 
 
 class FilteredDeckConfigDialog(QDialog):
-    """Dialogue to modify and build a filtered deck."""
+    """Dialog to modify and (re)build a filtered deck."""
+
+    GEOMETRY_KEY = "dyndeckconf"
+    DIALOG_KEY = "FilteredDeckConfigDialog"
+    silentlyClose = True
 
     def __init__(
         self,
         mw: AnkiQt,
+        deck_id: DeckID = DeckID(0),
         search: Optional[str] = None,
         search_2: Optional[str] = None,
-        deck: Optional[DeckDict] = None,
     ) -> None:
-        """If 'deck' is an existing filtered deck, load and modify its settings.
+        """If 'deck_id' is non-zero, load and modify its settings.
-        Otherwise, build a new one and derive settings from the current deck.
+        Otherwise, build a new deck and derive settings from the current deck.
+
+        If search or search_2 are provided, they will be used as the default
+        search text.
         """
 
         QDialog.__init__(self, mw)
         self.mw = mw
         self.col = self.mw.col
-        self.did: Optional[int] = None
+        self._desired_search_1 = search
+        self._desired_search_2 = search_2
+
+        self._initial_dialog_setup()
+
+        # set on successful query
+        self.deck: FilteredDeckForUpdate
+
+        mw.query_op(
+            lambda: mw.col.sched.get_or_create_filtered_deck(deck_id=deck_id),
+            success=self.load_deck_and_show,
+            failure=self.on_fetch_error,
+        )
+
+    def on_fetch_error(self, exc: Exception) -> None:
+        showWarning(str(exc))
+        self.close()
+
+    def _initial_dialog_setup(self) -> None:
+        import anki.consts as cs
+
         self.form = aqt.forms.filtered_deck.Ui_Dialog()
         self.form.setupUi(self)
-        self.mw.checkpoint(tr(TR.ACTIONS_OPTIONS))
-        self.initialSetup()
-        self.old_deck = self.col.decks.current()
 
-        if deck and deck["dyn"]:
+        self.form.order.addItems(list(cs.dynOrderLabels(self.mw.col).values()))
-            # modify existing dyn deck
+        self.form.order_2.addItems(list(cs.dynOrderLabels(self.mw.col).values()))
-            label = tr(TR.ACTIONS_REBUILD)
+
-            self.deck = deck
+        qconnect(self.form.resched.stateChanged, self._onReschedToggled)
-            self.loadConf()
-        elif self.old_deck["dyn"]:
-            # create new dyn deck from other dyn deck
-            label = tr(TR.DECKS_BUILD)
-            self.loadConf(deck=self.old_deck)
-            self.new_dyn_deck()
-        else:
-            # create new dyn deck from regular deck
-            label = tr(TR.DECKS_BUILD)
-            self.new_dyn_deck()
-            self.loadConf()
-            self.set_default_searches(self.old_deck["name"])
 
-        self.form.name.setText(self.deck["name"])
-        self.form.name.setPlaceholderText(self.deck["name"])
-        self.set_custom_searches(search, search_2)
         qconnect(self.form.search_button.clicked, self.on_search_button)
         qconnect(self.form.search_button_2.clicked, self.on_search_button_2)
         qconnect(self.form.hint_button.clicked, self.on_hint_button)
@@ -84,16 +94,68 @@ class FilteredDeckConfigDialog(QDialog):
         qconnect(
             self.form.buttonBox.helpRequested, lambda: openHelp(HelpPage.FILTERED_DECK)
         )
-        self.setWindowTitle(
+
-            without_unicode_isolation(tr(TR.ACTIONS_OPTIONS_FOR, val=self.deck["name"]))
-        )
-        self.form.buttonBox.button(QDialogButtonBox.Ok).setText(label)
         if self.col.schedVer() == 1:
             self.form.secondFilter.setVisible(False)
-        restoreGeom(self, "dyndeckconf")
+        restoreGeom(self, self.GEOMETRY_KEY)
 
+    def load_deck_and_show(self, deck: FilteredDeckForUpdate) -> None:
+        self.deck = deck
+        self._load_deck()
         self.show()
 
+    def _load_deck(self) -> None:
+        form = self.form
+        deck = self.deck
+        config = deck.config
+
+        self.form.name.setText(deck.name)
+        self.form.name.setPlaceholderText(deck.name)
+        self.set_custom_searches(self._desired_search_1, self._desired_search_2)
+
+        existing = deck.id != 0
+        if existing:
+            build_label = tr(TR.ACTIONS_REBUILD)
+        else:
+            build_label = tr(TR.DECKS_BUILD)
+        self.form.buttonBox.button(QDialogButtonBox.Ok).setText(build_label)
+
+        form.resched.setChecked(config.reschedule)
+        self._onReschedToggled(0)
+
+        term1: FilteredDeckConfig.SearchTerm = config.search_terms[0]
+        form.search.setText(term1.search)
+        form.order.setCurrentIndex(term1.order)
+        form.limit.setValue(term1.limit)
+
+        if self.col.schedVer() == 1:
+            if config.delays:
+                form.steps.setText(self.listToUser(list(config.delays)))
+                form.stepsOn.setChecked(True)
+        else:
+            form.steps.setVisible(False)
+            form.stepsOn.setVisible(False)
+
+        form.previewDelay.setValue(config.preview_delay)
+
+        if len(config.search_terms) > 1:
+            term2: FilteredDeckConfig.SearchTerm = config.search_terms[1]
+            form.search_2.setText(term2.search)
+            form.order_2.setCurrentIndex(term2.order)
+            form.limit_2.setValue(term2.limit)
+            show_second = existing
+        else:
+            show_second = False
+            form.order_2.setCurrentIndex(5)
+            form.limit_2.setValue(20)
+
+        form.secondFilter.setChecked(show_second)
+        form.filter2group.setVisible(show_second)
+
+        self.setWindowTitle(
+            without_unicode_isolation(tr(TR.ACTIONS_OPTIONS_FOR, val=self.deck.name))
+        )
+
     def reopen(
         self,
         _mw: AnkiQt,
@@ -103,30 +165,6 @@ class FilteredDeckConfigDialog(QDialog):
     ) -> None:
         self.set_custom_searches(search, search_2)
 
-    def new_dyn_deck(self) -> None:
-        suffix: int = 1
-        while self.col.decks.id_for_name(
-            without_unicode_isolation(tr(TR.QT_MISC_FILTERED_DECK, val=suffix))
-        ):
-            suffix += 1
-        name: str = without_unicode_isolation(tr(TR.QT_MISC_FILTERED_DECK, val=suffix))
-        self.did = self.col.decks.new_filtered(name)
-        self.deck = self.col.decks.current()
-
-    def set_default_searches(self, deck_name: str) -> None:
-        self.form.search.setText(
-            self.col.build_search_string(
-                SearchNode(deck=deck_name),
-                SearchNode(card_state=SearchNode.CARD_STATE_DUE),
-            )
-        )
-        self.form.search_2.setText(
-            self.col.build_search_string(
-                SearchNode(deck=deck_name),
-                SearchNode(card_state=SearchNode.CARD_STATE_NEW),
-            )
-        )
-
     def set_custom_searches(
         self, search: Optional[str], search_2: Optional[str]
     ) -> None:
@@ -141,14 +179,6 @@ class FilteredDeckConfigDialog(QDialog):
             self.form.search_2.setFocus()
             self.form.search_2.selectAll()
 
-    def initialSetup(self) -> None:
-        import anki.consts as cs
-
-        self.form.order.addItems(list(cs.dynOrderLabels(self.mw.col).values()))
-        self.form.order_2.addItems(list(cs.dynOrderLabels(self.mw.col).values()))
-
-        qconnect(self.form.resched.stateChanged, self._onReschedToggled)
-
     def on_search_button(self) -> None:
         self._on_search_button(self.form.search)
 
@@ -158,10 +188,10 @@ class FilteredDeckConfigDialog(QDialog):
     def _on_search_button(self, line: QLineEdit) -> None:
         try:
             search = self.col.build_search_string(line.text())
-        except InvalidInput as err:
+        except SearchError as err:
             line.setFocus()
             line.selectAll()
-            show_invalid_search_error(err)
+            showWarning(str(err))
         else:
             aqt.dialogs.open("Browser", self.mw, search=(search,))
 
@@ -180,8 +210,8 @@ class FilteredDeckConfigDialog(QDialog):
         implicit_filter = self.col.group_searches(*implicit_filters, joiner="OR")
         try:
             search = self.col.build_search_string(manual_filter, implicit_filter)
-        except InvalidInput as err:
+        except Exception as err:
-            show_invalid_search_error(err)
+            showWarning(str(err))
         else:
             aqt.dialogs.open("Browser", self.mw, search=(search,))
 
@@ -195,11 +225,11 @@ class FilteredDeckConfigDialog(QDialog):
         If it's a rebuild, exclude cards from this filtered deck as those will be reset.
         """
         if self.col.schedVer() == 1:
-            if self.did is None:
+            if self.deck.id:
                 return (
                     self.col.group_searches(
                         SearchNode(card_state=SearchNode.CARD_STATE_LEARN),
-                        SearchNode(negated=SearchNode(deck=self.deck["name"])),
+                        SearchNode(negated=SearchNode(deck=self.deck.name)),
                     ),
                 )
             return (SearchNode(card_state=SearchNode.CARD_STATE_LEARN),)
@@ -208,11 +238,11 @@ class FilteredDeckConfigDialog(QDialog):
     def _filtered_search_node(self) -> Tuple[SearchNode]:
         """Return a search node that matches cards in filtered decks, if applicable excluding those
         in the deck being rebuild."""
-        if self.did is None:
+        if self.deck.id:
             return (
                 self.col.group_searches(
                     SearchNode(deck="filtered"),
-                    SearchNode(negated=SearchNode(deck=self.deck["name"])),
+                    SearchNode(negated=SearchNode(deck=self.deck.name)),
                 ),
             )
         return (SearchNode(deck="filtered"),)
@@ -222,111 +252,70 @@ class FilteredDeckConfigDialog(QDialog):
             not self.form.resched.isChecked() and self.col.schedVer() > 1
         )
 
-    def loadConf(self, deck: Optional[DeckDict] = None) -> None:
+    def _update_deck(self) -> bool:
-        f = self.form
+        """Update our stored deck with the details from the GUI.
-        d = deck or self.deck
+        If false, abort adding."""
+        form = self.form
+        deck = self.deck
+        config = deck.config
 
-        f.resched.setChecked(d["resched"])
+        deck.name = form.name.text()
-        self._onReschedToggled(0)
+        config.reschedule = form.resched.isChecked()
 
-        search, limit, order = d["terms"][0]
+        del config.delays[:]
-        f.search.setText(search)
+        if self.col.schedVer() == 1 and form.stepsOn.isChecked():
+            if (delays := self.userToList(form.steps)) is None:
+                return False
+            config.delays.extend(delays)
 
-        if self.col.schedVer() == 1:
+        terms = [
-            if d["delays"]:
+            FilteredDeckConfig.SearchTerm(
-                f.steps.setText(self.listToUser(d["delays"]))
+                search=form.search.text(),
-                f.stepsOn.setChecked(True)
+                limit=form.limit.value(),
-        else:
+                order=form.order.currentIndex(),
-            f.steps.setVisible(False)
+            )
-            f.stepsOn.setVisible(False)
+        ]
 
-        f.order.setCurrentIndex(order)
+        if form.secondFilter.isChecked():
-        f.limit.setValue(limit)
+            terms.append(
-        f.previewDelay.setValue(d.get("previewDelay", 10))
+                FilteredDeckConfig.SearchTerm(
+                    search=form.search_2.text(),
+                    limit=form.limit_2.value(),
+                    order=form.order_2.currentIndex(),
+                )
+            )
 
-        if len(d["terms"]) > 1:
+        del config.search_terms[:]
-            search, limit, order = d["terms"][1]
+        config.search_terms.extend(terms)
-            f.search_2.setText(search)
+        config.preview_delay = form.previewDelay.value()
-            f.order_2.setCurrentIndex(order)
-            f.limit_2.setValue(limit)
-            f.secondFilter.setChecked(True)
-            f.filter2group.setVisible(True)
-        else:
-            f.order_2.setCurrentIndex(5)
-            f.limit_2.setValue(20)
-            f.secondFilter.setChecked(False)
-            f.filter2group.setVisible(False)
 
-    def saveConf(self) -> None:
+        return True
-        f = self.form
-        d = self.deck
-
-        if f.name.text() and d["name"] != f.name.text():
-            self.col.decks.rename(d, f.name.text())
-            gui_hooks.sidebar_should_refresh_decks()
-
-        d["resched"] = f.resched.isChecked()
-        d["delays"] = None
-
-        if self.col.schedVer() == 1 and f.stepsOn.isChecked():
-            steps = self.userToList(f.steps)
-            if steps:
-                d["delays"] = steps
-            else:
-                d["delays"] = None
-
-        search = self.col.build_search_string(f.search.text())
-        terms = [[search, f.limit.value(), f.order.currentIndex()]]
-
-        if f.secondFilter.isChecked():
-            search_2 = self.col.build_search_string(f.search_2.text())
-            terms.append([search_2, f.limit_2.value(), f.order_2.currentIndex()])
-
-        d["terms"] = terms
-        d["previewDelay"] = f.previewDelay.value()
-
-        self.col.decks.save(d)
 
     def reject(self) -> None:
-        if self.did:
+        aqt.dialogs.markClosed(self.DIALOG_KEY)
-            self.col.decks.rem(self.did)
-            self.col.decks.select(self.old_deck["id"])
-            self.mw.reset()
-        saveGeom(self, "dyndeckconf")
         QDialog.reject(self)
-        aqt.dialogs.markClosed("FilteredDeckConfigDialog")
 
     def accept(self) -> None:
-        try:
+        if not self._update_deck():
-            self.saveConf()
+            return
-        except InvalidInput as err:
+
-            show_invalid_search_error(err)
+        def success(out: OpChangesWithCount) -> None:
-        except DeckIsFilteredError as err:
+            saveGeom(self, self.GEOMETRY_KEY)
-            showWarning(str(err))
+            aqt.dialogs.markClosed(self.DIALOG_KEY)
-        else:
-            if not self.col.sched.rebuild_filtered_deck(self.deck["id"]):
-                if askUser(tr(TR.DECKS_THE_PROVIDED_SEARCH_DID_NOT_MATCH)):
-                    return
-            saveGeom(self, "dyndeckconf")
-            self.mw.reset()
             QDialog.accept(self)
-            aqt.dialogs.markClosed("FilteredDeckConfigDialog")
 
-    def closeWithCallback(self, callback: Callable) -> None:
+        add_or_update_filtered_deck(mw=self.mw, deck=self.deck, success=success)
-        self.reject()
-        callback()
 
-    # Step load/save - fixme: share with std options screen
+    # Step load/save
     ########################################################
+    # fixme: remove once we drop support for v1
 
     def listToUser(self, values: List[Union[float, int]]) -> str:
         return " ".join(
             [str(int(val)) if int(val) == val else str(val) for val in values]
         )
 
-    def userToList(
+    def userToList(self, line: QLineEdit, minSize: int = 1) -> Optional[List[float]]:
-        self, line: QLineEdit, minSize: int = 1
-    ) -> Optional[List[Union[float, int]]]:
         items = str(line.text()).split(" ")
         ret = []
         for item in items:
@@ -335,8 +324,6 @@ class FilteredDeckConfigDialog(QDialog):
             try:
                 i = float(item)
                 assert i > 0
-                if i == int(i):
-                    i = int(i)
                 ret.append(i)
             except:
                 # invalid, don't update

qt/aqt/find_and_replace.py

@@ -22,7 +22,6 @@ from aqt.utils import (
     save_combo_index_for_session,
     save_is_checked,
     saveGeom,
-    show_invalid_search_error,
     tooltip,
     tr,
 )
@@ -52,7 +51,6 @@ def find_and_replace(
             tr(TR.FINDREPLACE_NOTES_UPDATED, changed=out.count, total=len(note_ids)),
             parent=parent,
         ),
-        failure=lambda exc: show_invalid_search_error(exc, parent=parent),
     )
 
 
@@ -78,7 +76,6 @@ def find_and_replace_tag(
             tr(TR.FINDREPLACE_NOTES_UPDATED, changed=out.count, total=len(note_ids)),
             parent=parent,
         ),
-        failure=lambda exc: show_invalid_search_error(exc, parent=parent),
     )
 
 

qt/aqt/main.py

@@ -107,7 +107,10 @@ ResultWithChanges = TypeVar(
     bound=Union[OpChanges, OpChangesWithCount, OpChangesWithID, HasChangesProperty],
 )
 
+T = TypeVar("T")
+
 PerformOpOptionalSuccessCallback = Optional[Callable[[ResultWithChanges], Any]]
+PerformOpOptionalFailureCallback = Optional[Callable[[Exception], Any]]
 
 install_pylib_legacy()
 
@@ -719,9 +722,9 @@ class AnkiQt(QMainWindow):
 
     def query_op(
         self,
-        op: Callable[[], Any],
+        op: Callable[[], T],
         *,
-        success: Callable[[Any], Any] = None,
+        success: Callable[[T], Any] = None,
         failure: Optional[Callable[[Exception], Any]] = None,
     ) -> None:
         """Run an operation that queries the DB on a background thread.
@@ -764,7 +767,7 @@ class AnkiQt(QMainWindow):
         op: Callable[[], ResultWithChanges],
         *,
         success: PerformOpOptionalSuccessCallback = None,
-        failure: Optional[Callable[[Exception], Any]] = None,
+        failure: PerformOpOptionalFailureCallback = None,
         after_hooks: Optional[Callable[[], None]] = None,
     ) -> None:
         """Run the provided operation on a background thread.
@@ -1325,7 +1328,7 @@ title="%s" %s>%s</button>""" % (
         if not deck:
             deck = self.col.decks.current()
         if deck["dyn"]:
-            aqt.dialogs.open("FilteredDeckConfigDialog", self, deck=deck)
+            aqt.dialogs.open("FilteredDeckConfigDialog", self, deck_id=deck["id"])
         else:
             aqt.deckconf.DeckConf(self, deck)
 

qt/aqt/scheduling_ops.py

@@ -9,6 +9,7 @@ import aqt
 from anki.collection import CARD_TYPE_NEW, Config
 from anki.decks import DeckID
 from anki.lang import TR
+from anki.scheduler import FilteredDeckForUpdate
 from aqt import AnkiQt
 from aqt.main import PerformOpOptionalSuccessCallback
 from aqt.qt import *
@@ -182,3 +183,15 @@ def rebuild_filtered_deck(*, mw: AnkiQt, deck_id: DeckID) -> None:
 
 def empty_filtered_deck(*, mw: AnkiQt, deck_id: DeckID) -> None:
     mw.perform_op(lambda: mw.col.sched.empty_filtered_deck(deck_id))
+
+
+def add_or_update_filtered_deck(
+    *,
+    mw: AnkiQt,
+    deck: FilteredDeckForUpdate,
+    success: PerformOpOptionalSuccessCallback,
+) -> None:
+    mw.perform_op(
+        lambda: mw.col.sched.add_or_update_filtered_deck(deck),
+        success=success,
+    )

qt/aqt/sidebar.py

@@ -9,7 +9,6 @@ from typing import Dict, Iterable, List, Optional, Tuple, cast
 import aqt
 from anki.collection import Config, OpChanges, SearchJoiner, SearchNode
 from anki.decks import DeckID, DeckTreeNode
-from anki.errors import InvalidInput
 from anki.notes import Note
 from anki.tags import TagTreeNode
 from anki.types import assert_exhaustive
@@ -25,7 +24,7 @@ from aqt.utils import (
     KeyboardModifiersPressed,
     askUser,
     getOnlyText,
-    show_invalid_search_error,
+    showWarning,
     tr,
 )
 
@@ -539,8 +538,8 @@ class SidebarTreeView(QTreeView):
                 search = self.col.join_searches(previous, current, "OR")
             else:
                 search = self.col.build_search_string(current)
-        except InvalidInput as e:
+        except Exception as e:
-            show_invalid_search_error(e)
+            showWarning(str(e))
         else:
             self.browser.search_for(search)
 
@@ -1228,8 +1227,8 @@ class SidebarTreeView(QTreeView):
             return self.col.build_search_string(
                 self.browser.form.searchEdit.lineEdit().text()
             )
-        except InvalidInput as e:
+        except Exception as e:
-            show_invalid_search_error(e)
+            showWarning(str(e))
             return None
 
     def _save_search(self, name: str, search: str, update: bool = False) -> None:

qt/aqt/utils.py

@@ -21,7 +21,6 @@ from typing import (
     cast,
 )
 
-from markdown import markdown
 from PyQt5.QtWidgets import (
     QAction,
     QDialog,
@@ -37,7 +36,6 @@ from PyQt5.QtWidgets import (
 import anki
 import aqt
 from anki import Collection
-from anki.errors import InvalidInput
 from anki.lang import TR  # pylint: disable=unused-import
 from anki.utils import invalidFilename, isMac, isWin, noBundledLibs, versionWithBuild
 from aqt.qt import *
@@ -139,14 +137,6 @@ def showCritical(
     return showInfo(text, parent, help, "critical", title=title, textFormat=textFormat)
 
 
-def show_invalid_search_error(err: Exception, parent: Optional[QWidget] = None) -> None:
-    "Render search errors in markdown, then display a warning."
-    text = str(err)
-    if isinstance(err, InvalidInput):
-        text = markdown(text)
-    showWarning(text, parent=parent)
-
-
 def showInfo(
     text: str,
     parent: Optional[QWidget] = None,

qt/tools/genhooks_gui.py

@@ -856,7 +856,7 @@ gui_hooks.webview_did_inject_style_into_page.append(mytest)
         legacy_hook="currentModelChanged",
         legacy_no_args=True,
     ),
-    Hook(name="sidebar_should_refresh_decks"),
+    Hook(name="sidebar_should_refresh_decks", doc="Legacy, do not use."),
     Hook(name="sidebar_should_refresh_notetypes"),
     Hook(
         name="deck_browser_will_show_options_menu",

rslib/backend.proto

@@ -389,7 +389,7 @@ message NormalDeck {
 message FilteredDeck {
   message SearchTerm {
     enum Order {
-      OLDEST_FIRST = 0;
+      OLDEST_REVIEWED_FIRST = 0;
       RANDOM = 1;
       INTERVALS_ASCENDING = 2;
       INTERVALS_DESCENDING = 3;
@@ -568,6 +568,7 @@ message BackendError {
     Empty exists = 12;
     Empty deck_is_filtered = 13;
     Empty filtered_deck_empty = 14;
+    Empty search_error = 15;
   }
 }
 
@@ -1529,9 +1530,9 @@ message RenameDeckIn {
 }
 
 message FilteredDeckForUpdate {
-  int64 deck_id = 1;
+  int64 id = 1;
   string name = 2;
-  FilteredDeck settings = 3;
+  FilteredDeck config = 3;
 }
 
 message SetFlagIn {

rslib/src/backend/decks.rs

@@ -6,6 +6,7 @@ use crate::{
     backend_proto::{self as pb},
     decks::{Deck, DeckID, DeckSchema11},
     prelude::*,
+    scheduler::filtered::FilteredDeckForUpdate,
 };
 pub(super) use pb::decks_service::Service as DecksService;
 
@@ -139,15 +140,18 @@ impl DecksService for Backend {
             .map(Into::into)
     }
 
-    fn get_or_create_filtered_deck(&self, _input: pb::DeckId) -> Result<pb::FilteredDeckForUpdate> {
+    fn get_or_create_filtered_deck(&self, input: pb::DeckId) -> Result<pb::FilteredDeckForUpdate> {
-        todo!()
+        self.with_col(|col| col.get_or_create_filtered_deck(input.into()))
+            .map(Into::into)
     }
 
     fn add_or_update_filtered_deck(
         &self,
-        _input: pb::FilteredDeckForUpdate,
+        input: pb::FilteredDeckForUpdate,
     ) -> Result<pb::OpChangesWithId> {
-        todo!()
+        self.with_col(|col| col.add_or_update_filtered_deck(input.into()))
+            .map(|out| out.map(i64::from))
+            .map(Into::into)
     }
 }
 
@@ -169,6 +173,26 @@ impl From<DeckID> for pb::DeckId {
     }
 }
 
+impl From<FilteredDeckForUpdate> for pb::FilteredDeckForUpdate {
+    fn from(deck: FilteredDeckForUpdate) -> Self {
+        pb::FilteredDeckForUpdate {
+            id: deck.id.into(),
+            name: deck.human_name,
+            config: Some(deck.config),
+        }
+    }
+}
+
+impl From<pb::FilteredDeckForUpdate> for FilteredDeckForUpdate {
+    fn from(deck: pb::FilteredDeckForUpdate) -> Self {
+        FilteredDeckForUpdate {
+            id: deck.id.into(),
+            human_name: deck.name,
+            config: deck.config.unwrap_or_default(),
+        }
+    }
+}
+
 // before we can switch to returning protobuf, we need to make sure we're converting the
 // deck separators
 

rslib/src/backend/err.rs

@@ -28,7 +28,7 @@ pub(super) fn anki_error_to_proto_error(err: AnkiError, i18n: &I18n) -> pb::Back
         AnkiError::NotFound => V::NotFoundError(pb::Empty {}),
         AnkiError::Existing => V::Exists(pb::Empty {}),
         AnkiError::DeckIsFiltered => V::DeckIsFiltered(pb::Empty {}),
-        AnkiError::SearchError(_) => V::InvalidInput(pb::Empty {}),
+        AnkiError::SearchError(_) => V::SearchError(pb::Empty {}),
         AnkiError::TemplateSaveError { .. } => V::TemplateParse(pb::Empty {}),
         AnkiError::ParseNumError => V::InvalidInput(pb::Empty {}),
         AnkiError::FilteredDeckEmpty => V::FilteredDeckEmpty(pb::Empty {}),

rslib/src/decks/filtered.rs

@@ -5,7 +5,7 @@ pub use crate::backend_proto::{
     deck_kind::Kind as DeckKind, Deck as DeckProto, DeckCommon, DeckKind as DeckKindProto,
     FilteredDeck, NormalDeck,
 };
-use crate::decks::FilteredSearchTerm;
+use crate::decks::{FilteredSearchOrder, FilteredSearchTerm};
 use crate::prelude::*;
 
 impl Deck {
@@ -14,7 +14,12 @@ impl Deck {
         filt.search_terms.push(FilteredSearchTerm {
             search: "".into(),
             limit: 100,
-            order: 0,
+            order: FilteredSearchOrder::Random as i32,
+        });
+        filt.search_terms.push(FilteredSearchTerm {
+            search: "".into(),
+            limit: 20,
+            order: FilteredSearchOrder::Due as i32,
         });
         filt.preview_delay = 10;
         filt.reschedule = true;

rslib/src/err.rs

@@ -212,6 +212,7 @@ impl AnkiError {
             }
             AnkiError::ParseNumError => i18n.tr(TR::ErrorsParseNumberFail).into(),
             AnkiError::DeckIsFiltered => i18n.tr(TR::ErrorsFilteredParentDeck).into(),
+            AnkiError::FilteredDeckEmpty => i18n.tr(TR::DecksFilteredDeckSearchEmpty).into(),
             _ => format!("{:?}", self),
         }
     }

rslib/src/scheduler/filtered/mod.rs

@@ -8,6 +8,7 @@ use std::convert::{TryFrom, TryInto};
 use crate::{
     config::ConfigKey,
     decks::{human_deck_name_to_native, FilteredDeck, FilteredSearchTerm},
+    search::writer::{deck_search, normalize_search},
 };
 use crate::{
     config::SchedulerVersion, prelude::*, search::SortMode,
@@ -19,7 +20,7 @@ use crate::{
 pub struct FilteredDeckForUpdate {
     pub id: DeckID,
     pub human_name: String,
-    pub settings: FilteredDeck,
+    pub config: FilteredDeck,
 }
 
 pub(crate) struct DeckFilterContext<'a> {
@@ -33,12 +34,12 @@ pub(crate) struct DeckFilterContext<'a> {
 impl Collection {
     /// Get an existing filtered deck, or create a new one if `deck_id` is 0. The new deck
     /// will not be added to the DB.
-    pub fn get_or_create_filtered_deck(&self, deck_id: DeckID) -> Result<FilteredDeckForUpdate> {
+    pub fn get_or_create_filtered_deck(
+        &mut self,
+        deck_id: DeckID,
+    ) -> Result<FilteredDeckForUpdate> {
         let deck = if deck_id.0 == 0 {
-            Deck {
+            self.new_filtered_deck_for_adding()?
-                name: self.get_next_filtered_deck_name()?,
-                ..Deck::new_filtered()
-            }
         } else {
             self.storage.get_deck(deck_id)?.ok_or(AnkiError::NotFound)?
         };
@@ -140,16 +141,23 @@ impl Collection {
     }
 
     fn get_next_filtered_deck_name(&self) -> Result<String> {
-        // fixme:
+        Ok(format!(
-        Ok("Filtered Deck 1".to_string())
+            "Filtered Deck {}",
+            TimestampSecs::now().time_string()
+        ))
     }
 
     fn add_or_update_filtered_deck_inner(
         &mut self,
-        update: FilteredDeckForUpdate,
+        mut update: FilteredDeckForUpdate,
     ) -> Result<DeckID> {
         let usn = self.usn()?;
 
+        // check the searches are valid, and normalize them
+        for term in &mut update.config.search_terms {
+            term.search = normalize_search(&term.search)?
+        }
+
         // add or update the deck
         let mut deck: Deck;
         if update.id.0 == 0 {
@@ -179,7 +187,7 @@ impl Collection {
         }
     }
 
-    pub fn rebuild_filtered_deck_inner(&mut self, deck: &Deck, usn: Usn) -> Result<usize> {
+    fn rebuild_filtered_deck_inner(&mut self, deck: &Deck, usn: Usn) -> Result<usize> {
         let config = deck.filtered()?;
         let ctx = DeckFilterContext {
             target_deck: deck.id,
@@ -192,6 +200,35 @@ impl Collection {
         self.return_all_cards_in_filtered_deck(deck.id)?;
         self.build_filtered_deck(ctx)
     }
+
+    fn new_filtered_deck_for_adding(&mut self) -> Result<Deck> {
+        let mut deck = Deck {
+            name: self.get_next_filtered_deck_name()?,
+            ..Deck::new_filtered()
+        };
+        if let Some(current) = self.get_deck(self.get_current_deck_id())? {
+            if !current.is_filtered() && current.id.0 != 0 {
+                // start with a search based on the selected deck name
+                let search = deck_search(&current.human_name());
+                let term1 = deck
+                    .filtered_mut()
+                    .unwrap()
+                    .search_terms
+                    .get_mut(0)
+                    .unwrap();
+                term1.search = format!(r#"{} AND "is:due""#, search);
+                let term2 = deck
+                    .filtered_mut()
+                    .unwrap()
+                    .search_terms
+                    .get_mut(1)
+                    .unwrap();
+                term2.search = format!(r#"{} AND "is:new""#, search);
+            }
+        }
+
+        Ok(deck)
+    }
 }
 
 impl TryFrom<Deck> for FilteredDeckForUpdate {
@@ -203,7 +240,7 @@ impl TryFrom<Deck> for FilteredDeckForUpdate {
             Ok(FilteredDeckForUpdate {
                 id: value.id,
                 human_name,
-                settings: filtered,
+                config: filtered,
             })
         } else {
             Err(AnkiError::invalid_input("not filtered"))
@@ -214,5 +251,5 @@ impl TryFrom<Deck> for FilteredDeckForUpdate {
 fn apply_update_to_filtered_deck(deck: &mut Deck, update: FilteredDeckForUpdate) {
     deck.id = update.id;
     deck.name = human_deck_name_to_native(&update.human_name);
-    deck.kind = DeckKind::Filtered(update.settings);
+    deck.kind = DeckKind::Filtered(update.config);
 }

rslib/src/scheduler/mod.rs

@@ -6,7 +6,7 @@ use crate::{collection::Collection, config::SchedulerVersion, err::Result, prelu
 pub mod answering;
 pub mod bury_and_suspend;
 pub(crate) mod congrats;
-mod filtered;
+pub(crate) mod filtered;
 mod learning;
 pub mod new;
 pub(crate) mod queue;

rslib/src/search/mod.rs

@@ -5,7 +5,7 @@ mod cards;
 mod notes;
 mod parser;
 mod sqlwriter;
-mod writer;
+pub(crate) mod writer;
 
 pub use cards::SortMode;
 pub use parser::{

rslib/src/search/writer.rs

@@ -4,7 +4,9 @@
 use crate::{
     decks::DeckID as DeckIDType,
     notetype::NoteTypeID as NoteTypeIDType,
-    search::parser::{Node, PropertyKind, RatingKind, SearchNode, StateKind, TemplateKind},
+    prelude::*,
+    search::parser::{parse, Node, PropertyKind, RatingKind, SearchNode, StateKind, TemplateKind},
+    text::escape_anki_wildcards,
 };
 use std::mem;
 
@@ -172,18 +174,22 @@ fn write_property(operator: &str, kind: &PropertyKind) -> String {
     }
 }
 
+pub(crate) fn deck_search(name: &str) -> String {
+    write_nodes(&[Node::Search(SearchNode::Deck(escape_anki_wildcards(name)))])
+}
+
+/// Take an Anki-style search string and convert it into an equivalent
+/// search string with normalized syntax.
+pub(crate) fn normalize_search(input: &str) -> Result<String> {
+    Ok(write_nodes(&parse(input)?))
+}
+
 #[cfg(test)]
 mod test {
     use super::*;
     use crate::err::Result;
     use crate::search::parse_search as parse;
 
-    /// Take an Anki-style search string and convert it into an equivalent
-    /// search string with normalized syntax.
-    fn normalize_search(input: &str) -> Result<String> {
-        Ok(write_nodes(&parse(input)?))
-    }
-
     #[test]
     fn normalizing() -> Result<()> {
         assert_eq!(r#""(" AND "-""#, normalize_search(r"\( \-").unwrap());

rslib/src/storage/card/filtered.rs

@@ -9,7 +9,7 @@ use crate::{
 pub(crate) fn order_and_limit_for_search(term: &FilteredSearchTerm, today: u32) -> String {
     let temp_string;
     let order = match term.order() {
-        FilteredSearchOrder::OldestFirst => "(select max(id) from revlog where cid=c.id)",
+        FilteredSearchOrder::OldestReviewedFirst => "(select max(id) from revlog where cid=c.id)",
         FilteredSearchOrder::Random => "random()",
         FilteredSearchOrder::IntervalsAscending => "ivl",
         FilteredSearchOrder::IntervalsDescending => "ivl desc",

rslib/src/timestamp.rs

@@ -28,6 +28,11 @@ impl TimestampSecs {
         Local.timestamp(self.0, 0).format("%Y-%m-%d").to_string()
     }
 
+    /// HH-MM
+    pub(crate) fn time_string(self) -> String {
+        Local.timestamp(self.0, 0).format("%H:%M").to_string()
+    }
+
     pub fn local_utc_offset(self) -> FixedOffset {
         *Local.timestamp(self.0, 0).offset()
     }