fix: make 'changeNoteType' undoable

* return OpChanges from notetypes.change * inline changeNoteTypeWithErrorHandling * handle suspend routines in withErrorHandling * wrap `OpChanges` with `undoableOp` * use `withCol` in `withErrorHandling` fixes 14885
ankidroid · Dec 8, 2023 · befa32a · befa32a
1 parent 42ca2b5
commit befa32a
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 18 deletions.
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/NoteEditor.kt b/AnkiDroid/src/main/java/com/ichi2/anki/NoteEditor.kt
@@ -91,7 +91,6 @@ import com.ichi2.libanki.Decks.Companion.CURRENT_DECK
 import com.ichi2.libanki.Note.ClozeUtils
 import com.ichi2.libanki.Note.DupeOrEmpty
 import com.ichi2.libanki.Notetypes.Companion.NOT_FOUND_NOTE_TYPE
-import com.ichi2.libanki.exception.ConfirmModSchemaException
 import com.ichi2.utils.*
 import com.ichi2.widget.WidgetStatus
 import org.json.JSONArray
@@ -765,6 +764,7 @@ class NoteEditor : AnkiActivity(), DeckSelectionListener, SubtitleListener, Tags
 
     @VisibleForTesting
     @NeedsTest("14664: 'first field must not be empty' no longer applies after saving the note")
+    @KotlinCleanup("fix !! on oldModel/newModel")
     suspend fun saveNote() {
         val res = resources
         if (mSelectedTags == null) {
@@ -824,13 +824,13 @@ class NoteEditor : AnkiActivity(), DeckSelectionListener, SubtitleListener, Tags
                     dialog.setArgs(res.getString(R.string.confirm_map_cards_to_nothing))
                     val confirm = Runnable {
                         // Bypass the check once the user confirms
-                        changeNoteTypeWithErrorHandling(oldModel, newModel)
+                        changeNoteType(oldModel!!, newModel!!)
                     }
                     dialog.setConfirm(confirm)
                     showDialogFragment(dialog)
                 } else {
                     // Otherwise go straight to changing note type
-                    changeNoteTypeWithErrorHandling(oldModel, newModel)
+                    changeNoteType(oldModel!!, newModel!!)
                 }
                 return
             }
@@ -869,20 +869,13 @@ class NoteEditor : AnkiActivity(), DeckSelectionListener, SubtitleListener, Tags
     /**
      * Change the note type from oldModel to newModel, handling the case where a full sync will be required
      */
-    private fun changeNoteTypeWithErrorHandling(oldNotetype: NotetypeJson?, newNotetype: NotetypeJson?) = launchCatchingTask {
-        if (userAcceptsSchemaChange()) {
-            changeNoteType(oldNotetype, newNotetype)
-        }
-    }
+    private fun changeNoteType(oldNotetype: NotetypeJson, newNotetype: NotetypeJson) = launchCatchingTask {
+        if (!userAcceptsSchemaChange()) return@launchCatchingTask
 
-    /**
-     * Change the note type from oldModel to newModel
-     * @throws ConfirmModSchemaException If a full sync will be required
-     */
-    @Throws(ConfirmModSchemaException::class)
-    private fun changeNoteType(oldNotetype: NotetypeJson?, newNotetype: NotetypeJson?) {
         val noteId = mEditorNote!!.id
-        getColUnsafe.notetypes.change(oldNotetype!!, noteId, newNotetype!!, mModelChangeFieldMap!!, mModelChangeCardMap!!)
+        undoableOp {
+            notetypes.change(oldNotetype, noteId, newNotetype, mModelChangeFieldMap!!, mModelChangeCardMap!!)
+        }
         // refresh the note object to reflect the database changes
         mEditorNote!!.load()
         // close note editor

diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/Notetypes.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/Notetypes.kt
@@ -513,7 +513,10 @@ class Notetypes(val col: Collection) {
     # - newModel should be same as m if model is not changing
      */
 
-    /** A compatibility wrapper that converts legacy-style arguments and
+    /**
+     * Modifies the backend schema. Ask the user to confirm schema changes before calling
+     *
+     * A compatibility wrapper that converts legacy-style arguments and
      * feeds them into a backend request, so that AnkiDroid's editor-bound
      * notetype changing can be used. Changing the notetype via the editor is
      * not ideal: it doesn't let users re-order fields in a 2 element note,
@@ -533,7 +536,7 @@ class Notetypes(val col: Collection) {
         newModel: NotetypeJson,
         fmap: Map<Int, Int?>,
         cmap: Map<Int, Int?>
-    ) {
+    ): OpChanges {
         val fieldMap = convertLegacyMap(fmap, newModel.fieldsNames.size)
         val templateMap =
             if (cmap.isEmpty() || m.type == MODEL_CLOZE || newModel.type == MODEL_CLOZE) {
@@ -542,7 +545,7 @@ class Notetypes(val col: Collection) {
                 convertLegacyMap(cmap, newModel.templatesNames.size)
             }
         val isCloze = newModel.isCloze || m.isCloze
-        col.backend.changeNotetype(
+        return col.backend.changeNotetype(
             noteIds = listOf(nid),
             newFields = fieldMap,
             newTemplates = templateMap,