fix: card repositioning doesn't work for suspended card(s)

AnkiDroid/src/main/java/com/ichi2/anki/browser/CardBrowserFragment.kt

@@ -67,7 +67,7 @@ import com.ichi2.anki.browser.CardBrowserViewModel.ToggleSelectionState
 import com.ichi2.anki.browser.CardBrowserViewModel.ToggleSelectionState.SELECT_ALL
 import com.ichi2.anki.browser.CardBrowserViewModel.ToggleSelectionState.SELECT_NONE
 import com.ichi2.anki.browser.RepositionCardFragment.Companion.REQUEST_REPOSITION_NEW_CARDS
-import com.ichi2.anki.browser.RepositionCardsRequest.ContainsNonNewCardsError
+import com.ichi2.anki.browser.RepositionCardsRequest.NoRepositionableCardsError
 import com.ichi2.anki.browser.RepositionCardsRequest.RepositionData
 import com.ichi2.anki.common.annotations.NeedsTest
 import com.ichi2.anki.common.utils.android.isRobolectric
@@ -719,8 +719,8 @@ class CardBrowserFragment :
         Timber.i("CardBrowser:: Reposition button pressed")
         launchCatchingTask {
             when (val repositionCardsResult = activityViewModel.prepareToRepositionCards()) {
-                is ContainsNonNewCardsError -> {
-                    // Only new cards may be repositioned (If any non-new found show error dialog and return false)
+                is NoRepositionableCardsError -> {
+                    // No selected cards can be repositioned
                     showDialogFragment(
                         SimpleMessageDialog.newInstance(
                             title = getString(R.string.vague_error),

AnkiDroid/src/main/java/com/ichi2/anki/browser/CardBrowserViewModel.kt

@@ -953,11 +953,24 @@ class CardBrowserViewModel(
     @NeedsTest("verify behavior for repositioning with 'Randomize order'")
     suspend fun prepareToRepositionCards(): RepositionCardsRequest {
         val selectedCardIds = queryAllSelectedCardIds()
-        // Only new cards may be repositioned (If any non-new found show error dialog and return false)
-        if (selectedCardIds.any { withCol { getCard(it).queue != QueueType.New } }) {
-            return RepositionCardsRequest.ContainsNonNewCardsError
+
+        // Separate repositionable and non-repositionable cards.
+        // TODO: Add a timeout for this card-by-card scan on very large selections.
+        val (repositionableIds, skippedIds) =
+            withCol {
+                selectedCardIds.partition { cardId ->
+                    canRepositionCard(getCard(cardId))
+                }
+            }
+
+        // If no cards can be repositioned, return error
+        if (repositionableIds.isEmpty()) {
+            return RepositionCardsRequest.NoRepositionableCardsError
         }
 
+        // The full partition already ran, so this value is exact for now.
+        val unsupportedCardCount = UnsupportedCardCount.Count(skippedIds.size)
+
         // query obtained from Anki Desktop
         // https://github.com/ankitects/anki/blob/1fb1cbbf85c48a54c05cb4442b1b424a529cac60/qt/aqt/operations/scheduling.py#L117
         try {
@@ -975,6 +988,7 @@ class CardBrowserViewModel(
                     max = max,
                     random = defaults.random,
                     shift = defaults.shift,
+                    unsupportedCardCount = unsupportedCardCount,
                 )
             }
         } catch (e: Exception) {
@@ -984,6 +998,7 @@ class CardBrowserViewModel(
             return RepositionData(
                 min = null,
                 max = null,
+                unsupportedCardCount = unsupportedCardCount,
             )
         }
     }
@@ -999,6 +1014,7 @@ class CardBrowserViewModel(
         shift: Boolean,
     ): Int {
         val ids = queryAllSelectedCardIds()
+
         Timber.d("repositioning %d cards to %d", ids.size, position)
         return undoableOp {
             sched.sortCards(cids = ids, position, step = step, shuffle = shuffle, shift = shift)
@@ -1495,16 +1511,41 @@ class IdsFile(
     }
 }
 
+/**
+ * Determines if a card can be repositioned.
+ *
+ * Mirrors Anki upstream logic in `set_new_position()`: https://github.com/ankitects/anki/blob/967992304627bb2bc690afd70b28760f09c2a021/rslib/src/scheduler/new.rs#L65-L80
+ * - if `card.type == CardType.New`, it's repositionable
+ * - otherwise, if `card.queue == QueueType.New`, it's repositionable
+ *
+ * @param card The card to check
+ * @return true if the card can be repositioned, false otherwise
+ */
+private fun canRepositionCard(card: Card): Boolean = card.type == CardType.New || card.queue == QueueType.New
+
+/** Count of selected cards that cannot be repositioned. */
+sealed interface UnsupportedCardCount {
+    data class Count(
+        val value: Int,
+    ) : UnsupportedCardCount
+
+    /** Used when we short-circuit the scan (for example, after timeout). */
+    data object Undetermined : UnsupportedCardCount
+}
+
 sealed class RepositionCardsRequest {
-    /** Only new cards may be repositioned */
-    data object ContainsNonNewCardsError : RepositionCardsRequest()
+    /** None of the selected cards can be repositioned */
+    data object NoRepositionableCardsError : RepositionCardsRequest()
 
-    /** Should contain queue top & bottom positions. Null on error */
+    /** Should contain queue top & bottom positions. Null on error.
+     * `unsupportedCardCount` uses [UnsupportedCardCount.Undetermined] when scan is short-circuited.
+     */
     class RepositionData(
         val min: Int?,
         val max: Int?,
         val random: Boolean = false,
         val shift: Boolean = false,
+        val unsupportedCardCount: UnsupportedCardCount = UnsupportedCardCount.Count(0),
     ) : RepositionCardsRequest() {
         val queueTop: Int?
         val queueBottom: Int?

AnkiDroid/src/test/java/com/ichi2/anki/browser/CardBrowserViewModelTest.kt

@@ -56,7 +56,7 @@ import com.ichi2.anki.browser.CardBrowserViewModel.Companion.STATE_MULTISELECT_V
 import com.ichi2.anki.browser.CardBrowserViewModel.RowSelection
 import com.ichi2.anki.browser.CardBrowserViewModel.ToggleSelectionState.SELECT_ALL
 import com.ichi2.anki.browser.CardBrowserViewModel.ToggleSelectionState.SELECT_NONE
-import com.ichi2.anki.browser.RepositionCardsRequest.ContainsNonNewCardsError
+import com.ichi2.anki.browser.RepositionCardsRequest.NoRepositionableCardsError
 import com.ichi2.anki.browser.RepositionCardsRequest.RepositionData
 import com.ichi2.anki.browser.search.SavedSearch
 import com.ichi2.anki.export.ExportDialogFragment
@@ -964,7 +964,17 @@ class CardBrowserViewModelTest : JvmTest() {
             assertThat("2 selected rows", selectedRows.size, equalTo(2))
 
             val repositionResult = prepareToRepositionCards()
-            assertInstanceOf<ContainsNonNewCardsError>(repositionResult, "new cards error")
+            assertInstanceOf<RepositionData>(repositionResult, "mixed selection should still return reposition data").apply {
+                val unsupported =
+                    assertInstanceOf<UnsupportedCardCount.Count>(
+                        unsupportedCardCount,
+                        "unsupported card count should be exact",
+                    )
+                assertThat("unsupported card count", unsupported.value, equalTo(1))
+            }
+
+            val count = repositionSelectedRows(position = 50, step = 1, shuffle = false, shift = false)
+            assertThat("only new cards should be repositioned", count, equalTo(1))
         }
     }
 
@@ -994,6 +1004,44 @@ class CardBrowserViewModelTest : JvmTest() {
         }
     }
 
+    @Test
+    fun `reposition - suspended new card`() {
+        addBasicNote("New").suspendAll()
+        addBasicNote("New")
+
+        runViewModelTest {
+            selectAll()
+
+            val cards = queryAllSelectedCardIds().map(col::getCard)
+            assertTrue("at least one card is suspended") { cards.any { it.queue == QueueType.Suspended } }
+            assertTrue("all suspended cards are still new type") {
+                cards.filter { it.queue == QueueType.Suspended }.all { it.type == CardType.New }
+            }
+
+            val repositionResult = prepareToRepositionCards()
+
+            // Should succeed because it's still a New card, even though suspended
+            assertInstanceOf<RepositionData>(repositionResult, "suspended new card should be repositionable").apply {
+                assertThat("queueTop", queueTop, equalTo(1))
+                assertThat("queueBottom", queueBottom, equalTo(2))
+            }
+        }
+    }
+
+    @Test
+    fun `reposition - all non new cards`() {
+        addRevBasicNoteDueToday("Review1", "Today")
+        addRevBasicNoteDueToday("Review2", "Today")
+
+        runViewModelTest {
+            selectAll()
+            assertThat("2 selected rows", selectedRows.size, equalTo(2))
+
+            val repositionResult = prepareToRepositionCards()
+            assertInstanceOf<NoRepositionableCardsError>(repositionResult, "all non-new cards error")
+        }
+    }
+
     @Test
     fun `preview - no notes`() {
         // add a card: a preview should

libanki/testutils/src/main/java/com/ichi2/anki/libanki/testutils/AnkiTest.kt

@@ -275,6 +275,12 @@ interface AnkiTest {
         return this
     }
 
+    /** Helper method to suspend all cards of a note */
+    fun Note.suspendAll(): Note {
+        col.sched.suspendCards(cardIds(col))
+        return this
+    }
+
     fun NotetypeJson.createClone(): NotetypeJson {
         val targetNotetype = requireNotNull(col.notetypes.byName(name)) { "could not find note type '$name'" }
         val newNotetype =