Add Deleted error and disable all bad browser rows (#1742)

* Add Deleted error and disable all bad browser rows

* Avoid error when opening the browse screen to a card with a missing note (dae)

* In cards mode, a missing note is NotFound, not Deleted (dae)

So we distinguish between referential integrity error, and explicit
deletion.

* Remove redundant try block

proto/anki/backend.proto

@@ -58,6 +58,7 @@ message BackendError {
     SEARCH_ERROR = 14;
     CUSTOM_STUDY_ERROR = 15;
     IMPORT_ERROR = 16;
+    DELETED = 17;
   }
 
   // localized error description suitable for displaying to the user

pylib/anki/errors.py

@@ -56,6 +56,10 @@ class NotFoundError(Exception):
     pass
 
 
+class DeletedError(LocalizedError):
+    pass
+
+
 class ExistsError(Exception):
     pass
 

qt/aqt/browser/table/__init__.py

@@ -37,7 +37,7 @@ class Cell:
 
 
 class CellRow:
-    is_deleted: bool = False
+    is_disabled: bool = False
 
     def __init__(
         self,
@@ -69,9 +69,9 @@ class CellRow:
         return CellRow.generic(length, "...")
 
     @staticmethod
-    def deleted(length: int) -> CellRow:
+    def disabled(length: int, cell_text: str) -> CellRow:
-        row = CellRow.generic(length, tr.browsing_row_deleted())
+        row = CellRow.generic(length, cell_text)
-        row.is_deleted = True
+        row.is_disabled = True
         return row
 
 

qt/aqt/browser/table/model.py

@@ -11,7 +11,7 @@ from anki.cards import Card, CardId
 from anki.collection import BrowserColumns as Columns
 from anki.collection import Collection
 from anki.consts import *
-from anki.errors import NotFoundError
+from anki.errors import LocalizedError, NotFoundError
 from anki.notes import Note, NoteId
 from aqt import gui_hooks
 from aqt.browser.table import Cell, CellRow, Column, ItemId, SearchContext
@@ -87,24 +87,26 @@ class DataModel(QAbstractTableModel):
         # row state has changed if existence of cached and fetched counterparts differ
         # if the row was previously uncached, it is assumed to have existed
         state_change = (
-            new_row.is_deleted
+            new_row.is_disabled
             if old_row is None
-            else old_row.is_deleted != new_row.is_deleted
+            else old_row.is_disabled != new_row.is_disabled
         )
         if state_change:
-            self._on_row_state_will_change(index, not new_row.is_deleted)
+            self._on_row_state_will_change(index, not new_row.is_disabled)
         self._rows[item] = new_row
         if state_change:
-            self._on_row_state_changed(index, not new_row.is_deleted)
+            self._on_row_state_changed(index, not new_row.is_disabled)
         return self._rows[item]
 
     def _fetch_row_from_backend(self, item: ItemId) -> CellRow:
         try:
             row = CellRow(*self.col.browser_row_for_id(item))
-        except NotFoundError:
+        except LocalizedError as e:
-            return CellRow.deleted(self.len_columns())
+            return CellRow.disabled(self.len_columns(), str(e))
         except Exception as e:
-            return CellRow.generic(self.len_columns(), str(e))
+            return CellRow.disabled(
+                self.len_columns(), tr.errors_please_check_database()
+            )
         except BaseException as e:
             # fatal error like a panic in the backend - dump it to the
             # console so it gets picked up by the error handler
@@ -214,10 +216,11 @@ class DataModel(QAbstractTableModel):
         """Try to return the indicated, possibly deleted card."""
         if not index.isValid():
             return None
-        try:
+        # The browser code will be calling .note() on the returned card.
-            return self._state.get_card(self.get_item(index))
+        # This implicitly ensures both the card and its note exist.
-        except NotFoundError:
+        if self.get_row(index).is_disabled:
             return None
+        return self._state.get_card(self.get_item(index))
 
     def get_note(self, index: QModelIndex) -> Note | None:
         """Try to return the indicated, possibly deleted note."""
@@ -341,7 +344,7 @@ class DataModel(QAbstractTableModel):
     def flags(self, index: QModelIndex) -> Qt.ItemFlag:
         # shortcut for large selections (Ctrl+A) to avoid fetching large numbers of rows at once
         if row := self.get_cached_row(index):
-            if row.is_deleted:
+            if row.is_disabled:
                 return Qt.ItemFlag(Qt.ItemFlag.NoItemFlags)
         return Qt.ItemFlag.ItemIsEnabled | Qt.ItemFlag.ItemIsSelectable
 

qt/aqt/browser/table/table.py

@@ -225,7 +225,7 @@ class Table:
         bottom = max(r.row() for r in self._selected()) + 1
         for row in range(bottom, self.len()):
             index = self._model.index(row, 0)
-            if self._model.get_row(index).is_deleted:
+            if self._model.get_row(index).is_disabled:
                 continue
             if self._model.get_note_id(index) in nids:
                 continue
@@ -235,7 +235,7 @@ class Table:
         top = min(r.row() for r in self._selected()) - 1
         for row in range(top, -1, -1):
             index = self._model.index(row, 0)
-            if self._model.get_row(index).is_deleted:
+            if self._model.get_row(index).is_disabled:
                 continue
             if self._model.get_note_id(index) in nids:
                 continue

rslib/src/backend/error.rs

@@ -24,6 +24,7 @@ impl AnkiError {
             AnkiError::JsonError(_) => Kind::JsonError,
             AnkiError::ProtoError(_) => Kind::ProtoError,
             AnkiError::NotFound => Kind::NotFoundError,
+            AnkiError::Deleted => Kind::Deleted,
             AnkiError::Existing => Kind::Exists,
             AnkiError::FilteredDeckError(_) => Kind::FilteredDeckError,
             AnkiError::SearchError(_) => Kind::SearchError,

rslib/src/browser_table.rs

@@ -290,7 +290,15 @@ impl RowContext {
         let cards;
         let note;
         if notes_mode {
-            note = col.get_note_maybe_with_fields(NoteId(id), with_card_render)?;
+            note = col
+                .get_note_maybe_with_fields(NoteId(id), with_card_render)
+                .map_err(|e| {
+                    if e == AnkiError::NotFound {
+                        AnkiError::Deleted
+                    } else {
+                        e
+                    }
+                })?;
             cards = col.storage.all_cards_of_note(note.id)?;
             if cards.is_empty() {
                 return Err(AnkiError::DatabaseCheckRequired);
@@ -299,7 +307,7 @@ impl RowContext {
             cards = vec![col
                 .storage
                 .get_card(CardId(id))?
-                .ok_or(AnkiError::NotFound)?];
+                .ok_or(AnkiError::Deleted)?];
             note = col.get_note_maybe_with_fields(cards[0].note_id, with_card_render)?;
         }
         let notetype = col

rslib/src/error/mod.rs

@@ -35,6 +35,10 @@ pub enum AnkiError {
     CollectionNotOpen,
     CollectionAlreadyOpen,
     NotFound,
+    /// Indicates an absent card or note, but (unlike [AnkiError::NotFound]) in
+    /// a non-critical context like the browser table, where deleted ids are
+    /// deliberately not removed.
+    Deleted,
     Existing,
     FilteredDeckError(FilteredDeckError),
     SearchError(SearchErrorKind),
@@ -101,6 +105,7 @@ impl AnkiError {
             AnkiError::MediaCheckRequired => tr.errors_please_check_media().into(),
             AnkiError::CustomStudyError(err) => err.localized_description(tr),
             AnkiError::ImportError(err) => err.localized_description(tr),
+            AnkiError::Deleted => tr.browsing_row_deleted().into(),
             AnkiError::IoError(_)
             | AnkiError::JsonError(_)
             | AnkiError::ProtoError(_)