fix(editor): IMPL_15 - Add Item respects separator position in checklist

inventory69 · inventory69 · commit da371436cdd8 · 2026-02-11T21:34:18.000+01:00
Changes:
- NoteEditorViewModel.kt: Add calculateInsertIndexForNewItem() helper method
- NoteEditorViewModel.kt: Rewrite addChecklistItemAtEnd() to insert before
  first checked item (MANUAL/UNCHECKED_FIRST) instead of appending at list end
- NoteEditorViewModel.kt: Add cross-boundary guard to addChecklistItemAfter()
  preventing new unchecked items from being inserted inside checked section
- ChecklistSortingTest.kt: Add ChecklistSortOption import
- ChecklistSortingTest.kt: Add 10 IMPL_15 unit tests covering all sort modes,
  edge cases (empty list, all checked, no checked), and position stability

Root cause: addChecklistItemAtEnd() appended new unchecked items at the end
of the flat list, after checked items. The UI splits items by count
(subList(0, uncheckedCount)), not by isChecked state — causing checked items
to appear above the separator and new items below it.

Fix: Insert new items at the semantically correct position per sort mode.
MANUAL/UNCHECKED_FIRST: before first checked item (above separator).
All other modes: at list end (no separator visible, no visual issue).

All 19 unit tests pass (9 existing + 10 new). No UI changes required.
diff --git a/android/app/src/main/java/dev/dettmer/simplenotes/ui/editor/NoteEditorViewModel.kt b/android/app/src/main/java/dev/dettmer/simplenotes/ui/editor/NoteEditorViewModel.kt
@@ -244,13 +244,34 @@ class NoteEditorViewModel(
         }
     }
     
+    /**
+     * 🆕 v1.8.1 (IMPL_15): Fügt ein neues Item nach dem angegebenen Item ein.
+     *
+     * Guard: Bei MANUAL/UNCHECKED_FIRST wird sichergestellt, dass das neue (unchecked)
+     * Item nicht innerhalb der checked-Sektion eingefügt wird. Falls das Trigger-Item
+     * checked ist, wird stattdessen vor dem ersten checked Item eingefügt.
+     */
     fun addChecklistItemAfter(afterItemId: String): String {
         val newItem = ChecklistItemState.createEmpty(0)
         _checklistItems.update { items ->
             val index = items.indexOfFirst { it.id == afterItemId }
             if (index >= 0) {
+                val currentSort = _lastChecklistSortOption.value
+                val hasSeparator = currentSort == ChecklistSortOption.MANUAL ||
+                        currentSort == ChecklistSortOption.UNCHECKED_FIRST
+
+                // 🆕 v1.8.1 (IMPL_15): Wenn das Trigger-Item checked ist und ein Separator
+                // existiert, darf das neue unchecked Item nicht in die checked-Sektion.
+                // → Stattdessen vor dem ersten checked Item einfügen.
+                val effectiveIndex = if (hasSeparator && items[index].isChecked) {
+                    val firstCheckedIndex = items.indexOfFirst { it.isChecked }
+                    if (firstCheckedIndex >= 0) firstCheckedIndex else index + 1
+                } else {
+                    index + 1
+                }
+
                 val newList = items.toMutableList()
-                newList.add(index + 1, newItem)
+                newList.add(effectiveIndex, newItem)
                 // Update order values
                 newList.mapIndexed { i, item -> item.copy(order = i) }
             } else {
@@ -259,12 +280,46 @@ class NoteEditorViewModel(
         }
         return newItem.id
     }
-    
+
+    /**
+     * 🆕 v1.8.1 (IMPL_15): Fügt ein neues Item an der semantisch korrekten Position ein.
+     *
+     * Bei MANUAL/UNCHECKED_FIRST: Vor dem ersten checked Item (= direkt über dem Separator).
+     * Bei allen anderen Modi: Am Ende der Liste (kein Separator sichtbar).
+     *
+     * Verhindert, dass checked Items über den Separator springen oder das neue Item
+     * unter dem Separator erscheint.
+     */
     fun addChecklistItemAtEnd(): String {
-        val newItem = ChecklistItemState.createEmpty(_checklistItems.value.size)
-        _checklistItems.update { items -> items + newItem }
+        val newItem = ChecklistItemState.createEmpty(0)
+        _checklistItems.update { items ->
+            val insertIndex = calculateInsertIndexForNewItem(items)
+            val newList = items.toMutableList()
+            newList.add(insertIndex, newItem)
+            newList.mapIndexed { i, item -> item.copy(order = i) }
+        }
         return newItem.id
     }
+
+    /**
+     * 🆕 v1.8.1 (IMPL_15): Berechnet die korrekte Insert-Position für ein neues unchecked Item.
+     *
+     * - MANUAL / UNCHECKED_FIRST: Vor dem ersten checked Item (direkt über dem Separator)
+     * - Alle anderen Modi: Am Ende der Liste (kein Separator, kein visuelles Problem)
+     *
+     * Falls keine checked Items existieren, wird am Ende eingefügt.
+     */
+    private fun calculateInsertIndexForNewItem(items: List<ChecklistItemState>): Int {
+        val currentSort = _lastChecklistSortOption.value
+        return when (currentSort) {
+            ChecklistSortOption.MANUAL,
+            ChecklistSortOption.UNCHECKED_FIRST -> {
+                val firstCheckedIndex = items.indexOfFirst { it.isChecked }
+                if (firstCheckedIndex >= 0) firstCheckedIndex else items.size
+            }
+            else -> items.size
+        }
+    }
     
     fun deleteChecklistItem(itemId: String) {
         _checklistItems.update { items ->
diff --git a/android/app/src/test/java/dev/dettmer/simplenotes/ui/editor/ChecklistSortingTest.kt b/android/app/src/test/java/dev/dettmer/simplenotes/ui/editor/ChecklistSortingTest.kt
@@ -1,5 +1,6 @@
 package dev.dettmer.simplenotes.ui.editor
 
+import dev.dettmer.simplenotes.models.ChecklistSortOption
 import org.junit.Assert.*
 import org.junit.Test
 
@@ -174,4 +175,204 @@ class ChecklistSortingTest {
         assertEquals(1, sorted[1].order)
         assertEquals(2, sorted[2].order)
     }
+
+    // ═══════════════════════════════════════════════════════════════════════
+    // 🆕 v1.8.1 (IMPL_15): Tests für Add-Item Insert-Position
+    // ═══════════════════════════════════════════════════════════════════════
+
+    /**
+     * Simulates calculateInsertIndexForNewItem() from NoteEditorViewModel.
+     * Tests the insert position logic for new unchecked items.
+     */
+    private fun calculateInsertIndexForNewItem(
+        items: List<ChecklistItemState>,
+        sortOption: ChecklistSortOption
+    ): Int {
+        return when (sortOption) {
+            ChecklistSortOption.MANUAL,
+            ChecklistSortOption.UNCHECKED_FIRST -> {
+                val firstCheckedIndex = items.indexOfFirst { it.isChecked }
+                if (firstCheckedIndex >= 0) firstCheckedIndex else items.size
+            }
+            else -> items.size
+        }
+    }
+
+    /**
+     * Simulates the full addChecklistItemAtEnd() logic:
+     * 1. Calculate insert index
+     * 2. Insert new item
+     * 3. Reassign order values
+     */
+    private fun simulateAddItemAtEnd(
+        items: List<ChecklistItemState>,
+        sortOption: ChecklistSortOption
+    ): List<ChecklistItemState> {
+        val newItem = ChecklistItemState(id = "new", text = "", isChecked = false, order = 0)
+        val insertIndex = calculateInsertIndexForNewItem(items, sortOption)
+        val newList = items.toMutableList()
+        newList.add(insertIndex, newItem)
+        return newList.mapIndexed { i, item -> item.copy(order = i) }
+    }
+
+    @Test
+    fun `IMPL_15 - add item at end inserts before separator in MANUAL mode`() {
+        // Ausgangslage: 2 unchecked, 1 checked (sortiert)
+        val items = listOf(
+            item("a", checked = false, order = 0),
+            item("b", checked = false, order = 1),
+            item("c", checked = true,  order = 2)
+        )
+
+        val result = simulateAddItemAtEnd(items, ChecklistSortOption.MANUAL)
+
+        // Neues Item muss an Index 2 stehen (vor dem checked Item)
+        assertEquals(4, result.size)
+        assertEquals("a",   result[0].id)
+        assertEquals("b",   result[1].id)
+        assertEquals("new", result[2].id)  // ← Neues Item VOR Separator
+        assertFalse(result[2].isChecked)
+        assertEquals("c",   result[3].id)  // ← Checked Item bleibt UNTER Separator
+        assertTrue(result[3].isChecked)
+    }
+
+    @Test
+    fun `IMPL_15 - add item at end inserts before separator in UNCHECKED_FIRST mode`() {
+        val items = listOf(
+            item("a", checked = false, order = 0),
+            item("b", checked = true,  order = 1),
+            item("c", checked = true,  order = 2)
+        )
+
+        val result = simulateAddItemAtEnd(items, ChecklistSortOption.UNCHECKED_FIRST)
+
+        assertEquals(4, result.size)
+        assertEquals("a",   result[0].id)
+        assertEquals("new", result[1].id)  // ← Neues Item direkt nach letztem unchecked
+        assertFalse(result[1].isChecked)
+        assertEquals("b",   result[2].id)
+        assertEquals("c",   result[3].id)
+    }
+
+    @Test
+    fun `IMPL_15 - add item at end appends at end in CHECKED_FIRST mode`() {
+        val items = listOf(
+            item("a", checked = true,  order = 0),
+            item("b", checked = false, order = 1)
+        )
+
+        val result = simulateAddItemAtEnd(items, ChecklistSortOption.CHECKED_FIRST)
+
+        assertEquals(3, result.size)
+        assertEquals("a",   result[0].id)
+        assertEquals("b",   result[1].id)
+        assertEquals("new", result[2].id)  // ← Am Ende (kein Separator)
+    }
+
+    @Test
+    fun `IMPL_15 - add item at end appends at end in ALPHABETICAL_ASC mode`() {
+        val items = listOf(
+            item("a", checked = false, order = 0),
+            item("b", checked = true,  order = 1)
+        )
+
+        val result = simulateAddItemAtEnd(items, ChecklistSortOption.ALPHABETICAL_ASC)
+
+        assertEquals(3, result.size)
+        assertEquals("new", result[2].id)  // ← Am Ende
+    }
+
+    @Test
+    fun `IMPL_15 - add item at end appends at end in ALPHABETICAL_DESC mode`() {
+        val items = listOf(
+            item("a", checked = true,  order = 0),
+            item("b", checked = false, order = 1)
+        )
+
+        val result = simulateAddItemAtEnd(items, ChecklistSortOption.ALPHABETICAL_DESC)
+
+        assertEquals(3, result.size)
+        assertEquals("new", result[2].id)  // ← Am Ende
+    }
+
+    @Test
+    fun `IMPL_15 - add item with no checked items appends at end`() {
+        val items = listOf(
+            item("a", checked = false, order = 0),
+            item("b", checked = false, order = 1)
+        )
+
+        val result = simulateAddItemAtEnd(items, ChecklistSortOption.MANUAL)
+
+        assertEquals(3, result.size)
+        assertEquals("new", result[2].id)  // Kein checked Item → ans Ende
+    }
+
+    @Test
+    fun `IMPL_15 - add item with all checked items inserts at position 0`() {
+        val items = listOf(
+            item("a", checked = true, order = 0),
+            item("b", checked = true, order = 1)
+        )
+
+        val result = simulateAddItemAtEnd(items, ChecklistSortOption.MANUAL)
+
+        assertEquals(3, result.size)
+        assertEquals("new", result[0].id)  // ← Ganz oben (vor allen checked Items)
+        assertFalse(result[0].isChecked)
+        assertEquals("a",   result[1].id)
+        assertEquals("b",   result[2].id)
+    }
+
+    @Test
+    fun `IMPL_15 - add item to empty list in MANUAL mode`() {
+        val items = emptyList<ChecklistItemState>()
+
+        val result = simulateAddItemAtEnd(items, ChecklistSortOption.MANUAL)
+
+        assertEquals(1, result.size)
+        assertEquals("new", result[0].id)
+        assertEquals(0, result[0].order)
+    }
+
+    @Test
+    fun `IMPL_15 - order values are sequential after add item`() {
+        val items = listOf(
+            item("a", checked = false, order = 0),
+            item("b", checked = false, order = 1),
+            item("c", checked = true,  order = 2)
+        )
+
+        val result = simulateAddItemAtEnd(items, ChecklistSortOption.MANUAL)
+
+        result.forEachIndexed { index, item ->
+            assertEquals("Order at index $index should be $index", index, item.order)
+        }
+    }
+
+    @Test
+    fun `IMPL_15 - existing items do not change position after add item`() {
+        // Kernforderung: Kein Item darf sich verschieben
+        val items = listOf(
+            item("cashews",  checked = false, order = 0),
+            item("noodles",  checked = false, order = 1),
+            item("coffee",   checked = true,  order = 2)
+        )
+
+        val result = simulateAddItemAtEnd(items, ChecklistSortOption.MANUAL)
+
+        // Relative Reihenfolge der bestehenden Items prüfen
+        val existingIds = result.filter { it.id != "new" }.map { it.id }
+        assertEquals(listOf("cashews", "noodles", "coffee"), existingIds)
+
+        // Cashews und Noodles müssen VOR dem neuen Item sein
+        val cashewsIdx = result.indexOfFirst { it.id == "cashews" }
+        val noodlesIdx = result.indexOfFirst { it.id == "noodles" }
+        val newIdx     = result.indexOfFirst { it.id == "new" }
+        val coffeeIdx  = result.indexOfFirst { it.id == "coffee" }
+
+        assertTrue("Cashews before new", cashewsIdx < newIdx)
+        assertTrue("Noodles before new", noodlesIdx < newIdx)
+        assertTrue("New before Coffee",  newIdx < coffeeIdx)
+    }
 }
