Keep the list order with tasks.org and OpenTasks (#136) (#143)

CHANGELOG.md

@@ -20,6 +20,8 @@ All notable changes to this project are documented here. The format follows
   closed the whole series. With tasks.org and OpenTasks alike, the editor now
   shows the task's repeat rule instead of none, and an occurrence changed on
   its own stays part of its series (#135).
+- With tasks.org or OpenTasks as the storage provider, the order you give your
+  lists is kept instead of snapping back (#136).
 
 ## [1.3.0] - 2026-11-02
 

app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/ExternalProviderListOrderTest.kt

@@ -0,0 +1,76 @@
+package de.jeanlucmakiola.agendula.data.tasks
+
+import android.content.Context
+import androidx.test.core.app.ApplicationProvider
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import com.google.common.truth.Truth.assertThat
+import org.junit.After
+import org.junit.Assume.assumeTrue
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import java.util.concurrent.CountDownLatch
+import java.util.concurrent.TimeUnit
+
+/**
+ * List order against a real external provider (#136). Skipped unless one is
+ * installed and its permissions are granted.
+ */
+@RunWith(AndroidJUnit4::class)
+class ExternalProviderListOrderTest {
+
+    private val context = ApplicationProvider.getApplicationContext<Context>()
+    private val resolver = ProviderResolver(AndroidProviderEnvironment(context)).apply {
+        storageMode = StorageMode.EXTERNAL
+    }
+    private val source = AndroidTasksDataSource(context, resolver)
+    private val created = mutableListOf<Long>()
+
+    @Before
+    fun setUp() {
+        val provider = resolver.resolve()
+        assumeTrue("no external tasks provider, or not granted", provider != null && resolver.hasPermission(provider))
+        listOf("Order A", "Order B", "Order C").forEach { created += source.createLocalList(it, 0) }
+    }
+
+    @After
+    fun tearDown() = created.forEach { runCatching { source.deleteList(it) } }
+
+    private fun ours() = source.taskLists().map { it.id }.filter { it in created }
+
+    /** Waits until the provider has gone quiet, so a change seen afterwards is the test's own. */
+    private fun settle() {
+        var last = System.currentTimeMillis()
+        val observer = source.registerObserver { last = System.currentTimeMillis() }
+        while (System.currentTimeMillis() - last < 1_500) Thread.sleep(100)
+        observer.close()
+    }
+
+    @Test
+    fun aReorderIsKeptAndReported() {
+        val (a, b, c) = created
+        settle()
+        val changed = CountDownLatch(1)
+        val observer = source.registerObserver { changed.countDown() }
+
+        source.reorderLists(listOf(c, a, b))
+
+        assertThat(changed.await(5, TimeUnit.SECONDS)).isTrue()
+        observer.close()
+        assertThat(ours()).containsExactly(c, a, b).inOrder()
+        assertThat(AndroidTasksDataSource(context, resolver).taskLists().map { it.id }.filter { it in created })
+            .containsExactly(c, a, b).inOrder()
+    }
+
+    @Test
+    fun aDeletedListLeavesNoPositionBehind() {
+        val (a, b, c) = created
+        source.reorderLists(listOf(c, a, b))
+
+        source.deleteList(c)
+        created -= c
+
+        source.taskLists()
+        assertThat(source.storedListOrder().keys).doesNotContain(c)
+    }
+}

app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/AndroidTasksDataSource.kt

@@ -55,14 +55,47 @@ class AndroidTasksDataSource @Inject constructor(
     // --- reads ----------------------------------------------------------------
 
     override fun taskLists(): List<TaskList> {
-        val uri = TasksContract.listsUri(authority())
-        val sort = "${Lists.ACCOUNT_NAME}, ${Lists.NAME}"
-        return resolver.query(uri, TaskProjections.LISTS, null, null, sort)?.use { c ->
+        val authority = authority()
+        val lists = resolver.query(TasksContract.listsUri(authority), TaskProjections.LISTS, null, null, null)?.use { c ->
             val reader = CursorColumnReader(c)
-            buildList { while (c.moveToNext()) add(TaskMapper.taskList(reader)) }
-        } ?: emptyList()
+            buildList<TaskList> { while (c.moveToNext()) add(TaskMapper.taskList(reader)) }
+        } ?: return emptyList()
+        val order = synchronized(listOrderLock) {
+            val stored = storedListOrder(authority)
+            val known = lists.mapTo(HashSet()) { it.id }
+            // A list gone from the provider takes its position along, as its id may be
+            // reused — but not on an empty answer, which is as likely a provider mid-update.
+            if (lists.isNotEmpty() && stored.keys.any { it !in known }) {
+                stored.filterKeys { it in known }.also { storeListOrder(authority, it) }
+            } else {
+                stored
+            }
+        }
+        return ListOrder.sorted(lists, order)
     }
 
+    /** The order Agendula keeps for [authority]'s lists. */
+    internal fun storedListOrder(authority: String = authority()): Map<Long, Int> =
+        ListOrder.decode(listOrder.getString(authority, null))
+
+    private val listOrder get() = context.getSharedPreferences(LIST_ORDER, Context.MODE_PRIVATE)
+
+    private fun storeListOrder(authority: String, order: Map<Long, Int>) {
+        listOrder.edit().putString(authority, ListOrder.encode(order)).commit()
+    }
+
+    /** Kept by Agendula, as the provider's lists table has no order of its own. */
+    override fun reorderLists(listIds: List<Long>) {
+        val authority = authority()
+        synchronized(listOrderLock) {
+            storeListOrder(authority, ListOrder.reordered(taskLists().map { it.id }, listIds))
+        }
+        // Wakes the observers on the lists without asking any sync adapter to upload: nothing changed in the provider.
+        runCatching { resolver.notifyChange(TasksContract.listsUri(authority), null, 0) }
+    }
+
+    private val listOrderLock = Any()
+
     override fun tasks(query: TaskQuery): List<Task> {
         val clauses = mutableListOf<String>()
         val args = mutableListOf<String>()
@@ -787,6 +820,8 @@ class AndroidTasksDataSource @Inject constructor(
     }
 }
 
+private const val LIST_ORDER = "provider_list_order"
+
 /** An instance row of the task, or of an override standing in for one of its occurrences. */
 private const val OCCURRENCE_OF = "${Instances.TASK_ID} = ? OR ${Tasks.ORIGINAL_INSTANCE_ID} = ?"
 

app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/ListOrder.kt

@@ -0,0 +1,36 @@
+package de.jeanlucmakiola.agendula.data.tasks
+
+import de.jeanlucmakiola.agendula.domain.TaskList
+
+/**
+ * Agendula's own order for an external provider's lists, which the provider has
+ * no column for. Stored as `id=position` pairs. A list without one counts as
+ * position 0, as a new list does in Agendula's own store.
+ */
+object ListOrder {
+
+    fun decode(raw: String?): Map<Long, Int> = raw.orEmpty().split(',').mapNotNull { pair ->
+        val (id, position) = pair.split('=').takeIf { it.size == 2 } ?: return@mapNotNull null
+        id.toLongOrNull()?.let { key -> position.toIntOrNull()?.let { key to it } }
+    }.toMap()
+
+    fun encode(order: Map<Long, Int>): String = order.entries.joinToString(",") { "${it.key}=${it.value}" }
+
+    /**
+     * Positions for every list in [shown] — the lists in their current order —
+     * after one account's [listIds] were rearranged: those take each other's
+     * slots in their new order, and every other list keeps its own.
+     */
+    fun reordered(shown: List<Long>, listIds: List<Long>): Map<Long, Int> {
+        val moved = listIds.iterator()
+        val moving = listIds.toSet()
+        val result = shown.map { if (it in moving && moved.hasNext()) moved.next() else it }.toMutableList()
+        moved.forEachRemaining { if (it !in result) result += it }
+        return result.withIndex().associate { it.value to it.index }
+    }
+
+    fun sorted(lists: List<TaskList>, order: Map<Long, Int>): List<TaskList> = lists.sortedWith(
+        compareBy<TaskList>({ it.accountName.lowercase() }, { order[it.id] ?: 0 }, { it.name.lowercase() })
+            .thenBy { it.name },
+    )
+}

app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/TaskListDao.kt

@@ -18,7 +18,7 @@ interface TaskListDao {
         """
         SELECT l.*, a.display_name AS account_display_name, a.label AS account_label
         FROM task_lists l LEFT JOIN accounts a ON a.id = l.account_id
-        ORDER BY a.display_name, l.sort_order, l.name
+        ORDER BY a.display_name COLLATE NOCASE, l.sort_order, l.name COLLATE NOCASE, l.name
         """
     )
     fun lists(): List<TaskListRow>
@@ -32,6 +32,10 @@ interface TaskListDao {
     )
     fun list(listId: Long): TaskListRow?
 
+    /** The highest position among device-only lists, -1 when there are none. */
+    @Query("SELECT COALESCE(MAX(sort_order), -1) FROM task_lists WHERE account_id IS NULL")
+    fun lastLocalSortOrder(): Int
+
     @Query("SELECT * FROM task_lists WHERE id = :listId")
     fun entity(listId: Long): TaskListEntity?
 

app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/transfer/ExternalImport.kt

@@ -159,7 +159,9 @@ class ExternalImport @Inject constructor(
         val taskDao = database.tasks()
         val alarmDao = database.alarms()
 
-        val listIds = snapshot.lists.associate { list ->
+        // After the device-only lists already there, which the copies join.
+        val after = listDao.lastLocalSortOrder() + 1
+        val listIds = snapshot.lists.withIndex().associate { (position, list) ->
             list.id to listDao.insert(
                 TaskListEntity(
                     name = list.name,
@@ -168,6 +170,7 @@ class ExternalImport @Inject constructor(
                     isVisible = list.isVisible,
                     isSynced = false,
                     owner = list.owner,
+                    sortOrder = after + position,
                 ),
             )
         }

app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ListOrderTest.kt

@@ -0,0 +1,56 @@
+package de.jeanlucmakiola.agendula.data.tasks
+
+import com.google.common.truth.Truth.assertThat
+import de.jeanlucmakiola.agendula.domain.TaskList
+import org.junit.jupiter.api.Test
+
+class ListOrderTest {
+
+    private fun list(id: Long, name: String, account: String = "Radicale") = TaskList(
+        id = id, name = name, color = 0, accountName = account, accountType = "bitfire.at.davdroid",
+        isSynced = true, isVisible = true, owner = null,
+    )
+
+    @Test
+    fun `an order survives encoding`() {
+        val order = mapOf(3L to 0, 1L to 1, 2L to 2)
+        assertThat(ListOrder.decode(ListOrder.encode(order))).isEqualTo(order)
+    }
+
+    @Test
+    fun `nothing stored and garbage both decode to no order`() {
+        assertThat(ListOrder.decode(null)).isEmpty()
+        assertThat(ListOrder.decode("x=1,2,=3,4=y")).isEmpty()
+    }
+
+    @Test
+    fun `lists follow the stored order within their account (#136)`() {
+        val lists = listOf(list(1, "Groceries"), list(2, "Home"), list(3, "Work"), list(4, "Inbox", account = "Local"))
+        val order = ListOrder.reordered(listOf(4, 1, 2, 3), listOf(3, 1, 2))
+
+        assertThat(ListOrder.sorted(lists, order).map { it.id }).containsExactly(4L, 3L, 1L, 2L).inOrder()
+    }
+
+    @Test
+    fun `reordering one account leaves every other list in its slot`() {
+        // 7 and 8 are another account; 5 joined this one after the screen was drawn.
+        val order = ListOrder.reordered(listOf(7, 8, 1, 5, 2), listOf(2, 1))
+
+        assertThat(order).isEqualTo(mapOf(7L to 0, 8L to 1, 2L to 2, 5L to 3, 1L to 4))
+        assertThat(order.values.toSet()).hasSize(order.size)
+    }
+
+    @Test
+    fun `names sort without regard to case`() {
+        val lists = listOf(list(1, "Zeta"), list(2, "apple"))
+        assertThat(ListOrder.sorted(lists, emptyMap()).map { it.id }).containsExactly(2L, 1L).inOrder()
+    }
+
+    @Test
+    fun `a list without a position counts as the first, as in the own store`() {
+        val lists = listOf(list(1, "Zeta"), list(2, "Alpha"), list(3, "Mid"))
+        val order = mapOf(3L to 0, 1L to 1)
+
+        assertThat(ListOrder.sorted(lists, order).map { it.id }).containsExactly(2L, 3L, 1L).inOrder()
+    }
+}