Set order to next - 1 when moving to top

app/src/androidTest/java/com/todoroo/astrid/adapter/CaldavManualSortTaskAdapterTest.kt

@@ -23,6 +23,7 @@ import org.tasks.makers.CaldavTaskMaker.REMOTE_PARENT
 import org.tasks.makers.CaldavTaskMaker.TASK
 import org.tasks.makers.CaldavTaskMaker.newCaldavTask
 import org.tasks.makers.TaskMaker.CREATION_TIME
+import org.tasks.makers.TaskMaker.PARENT
 import org.tasks.makers.TaskMaker.newTask
 import org.tasks.preferences.Preferences
 import org.tasks.time.DateTime
@@ -73,7 +74,7 @@ class CaldavManualSortTaskAdapterTest : InjectingTestCase() {
 
         move(1, 0)
 
-        checkOrder(1, 1)
+        checkOrder(created.minusSeconds(1), 1)
         checkOrder(null, 0)
     }
 
@@ -157,6 +158,57 @@ class CaldavManualSortTaskAdapterTest : InjectingTestCase() {
         checkOrder(null, 3)
     }
 
+    @Test
+    fun moveToNewSubtask() {
+        val created = DateTime(2020, 5, 17, 9, 53, 17)
+        addTask(with(CREATION_TIME, created))
+        addTask(with(CREATION_TIME, created.plusSeconds(2)))
+
+        move(1, 1, 1)
+
+        checkOrder(null, 0)
+        checkOrder(null, 1)
+    }
+
+    @Test
+    fun moveToTopOfExistingSubtasks() {
+        val created = DateTime(2020, 5, 17, 9, 53, 17)
+        val parent = addTask(with(CREATION_TIME, created))
+        addTask(with(CREATION_TIME, created.plusSeconds(5)), with(PARENT, parent))
+        addTask(with(CREATION_TIME, created.plusSeconds(2)))
+
+        move(2, 1, 1)
+
+        checkOrder(null, 0)
+        checkOrder(created.plusSeconds(4), 2)
+        checkOrder(null, 1)
+    }
+
+    @Test
+    fun indentingChangesParent() {
+        val created = DateTime(2020, 5, 17, 9, 53, 17)
+        addTask(with(CREATION_TIME, created))
+        addTask(with(CREATION_TIME, created.plusSeconds(2)))
+
+        move(1, 1, 1)
+
+        assertEquals(tasks[0].id, tasks[1].parent)
+    }
+
+    @Test
+    fun deindentLastMultiLevelSubtask() {
+        val created = DateTime(2020, 5, 17, 9, 53, 17)
+        val grandparent = addTask(with(CREATION_TIME, created))
+        val parent = addTask(with(CREATION_TIME, created.plusSeconds(5)), with(PARENT, grandparent))
+        addTask(with(CREATION_TIME, created.plusSeconds(1)), with(PARENT, parent))
+        addTask(with(CREATION_TIME, created.plusSeconds(2)), with(PARENT, parent))
+
+        move(3, 3, 1)
+
+        assertEquals(grandparent, tasks[3].parent)
+        checkOrder(created.plusSeconds(6), 3)
+    }
+
     private fun move(from: Int, to: Int, indent: Int = 0) {
         tasks.addAll(taskDao.fetchTasks { getQuery(preferences, filter, it) })
         val adjustedTo = if (from < to) to + 1 else to // match DragAndDropRecyclerAdapter behavior
@@ -174,7 +226,7 @@ class CaldavManualSortTaskAdapterTest : InjectingTestCase() {
         }
     }
 
-    private fun addTask(vararg properties: PropertyValue<in Task?, *>) {
+    private fun addTask(vararg properties: PropertyValue<in Task?, *>): Long {
         val task = newTask(*properties)
         taskDao.createNew(task)
         val remoteParent = if (task.parent > 0) caldavDao.getRemoteIdForTask(task.parent) else null
@@ -183,6 +235,7 @@ class CaldavManualSortTaskAdapterTest : InjectingTestCase() {
                         with(TASK, task.id),
                         with(CALENDAR, "1234"),
                         with(REMOTE_PARENT, remoteParent)))
+        return task.id
     }
 
     override fun inject(component: TestComponent) = component.inject(this)

app/src/main/java/com/todoroo/astrid/adapter/CaldavManualSortTaskAdapter.kt

@@ -8,22 +8,24 @@ class CaldavManualSortTaskAdapter internal constructor(private val taskDao: Task
 
     override fun moved(from: Int, to: Int, indent: Int) {
         val task = getTask(from)
-        val previous = if (to > 0) getTask(to - 1) else null
-        val newParent = findNewParent(task, indent, to)
+        val oldParent = task.parent
+        val newParent = changeParent(task, indent, to)
 
-        if (from == to) {
+        if (oldParent == newParent && from == to) {
             return
         }
 
-        if (from != to) {
-            val newPosition = when {
-                previous == null -> 1
-                indent > previous.getIndent() -> 1
-                indent == previous.getIndent() -> previous.caldavSortOrder + 1
-                else -> getTask((to - 1 downTo 0).find { getTask(it).indent == indent }!!).caldavSortOrder + 1
-            }
-            caldavDao.move(task, newParent, newPosition)
+        val previous = if (to > 0) getTask(to - 1) else null
+        val next = if (to < count) getTask(to) else null
+
+        val newPosition = when {
+            previous == null -> next!!.caldavSortOrder - 1
+            indent > previous.getIndent() && next?.indent == indent -> next.caldavSortOrder - 1
+            indent > previous.getIndent() -> null
+            indent == previous.getIndent() -> previous.caldavSortOrder + 1
+            else -> getTask((to - 1 downTo 0).find { getTask(it).indent == indent }!!).caldavSortOrder + 1
         }
+        caldavDao.move(task, newParent, newPosition)
 
         taskDao.touch(task.id)
     }

app/src/main/java/com/todoroo/astrid/adapter/CaldavTaskAdapter.kt

@@ -21,10 +21,10 @@ open class CaldavTaskAdapter internal constructor(private val taskDao: TaskDao,
     override fun supportsParentingOrManualSort() = true
 
     override fun moved(from: Int, to: Int, indent: Int) {
-        findNewParent(getTask(from), indent, to)
+        changeParent(getTask(from), indent, to)
     }
 
-    internal fun findNewParent(task: TaskContainer, indent: Int, to: Int): Long {
+    internal fun changeParent(task: TaskContainer, indent: Int, to: Int): Long {
         val previous = if (to > 0) getTask(to - 1) else null
         val newParent = when {
             indent == 0 || previous == null -> 0

app/src/main/java/org/tasks/data/CaldavDao.kt

@@ -58,7 +58,7 @@ abstract class CaldavDao {
     }
 
     @Query("UPDATE caldav_tasks SET cd_order = :position WHERE cd_id = :id")
-    internal abstract fun update(id: Long, position: Long)
+    internal abstract fun update(id: Long, position: Long?)
 
     @Query("UPDATE caldav_tasks SET cd_remote_parent = :remoteParent WHERE cd_id = :id")
     internal abstract fun update(id: Long, remoteParent: String?)
@@ -171,14 +171,16 @@ abstract class CaldavDao {
     abstract fun updateParents(calendar: String)
 
     @Transaction
-    open fun move(task: TaskContainer, newParent: Long, newPosition: Long) {
+    open fun move(task: TaskContainer, newParent: Long, newPosition: Long?) {
         val previousParent = task.parent
         val caldavTask = task.caldavTask
         val previousPosition = task.caldavSortOrder
-        if (newParent == previousParent && newPosition < previousPosition) {
-            shiftDown(task.caldav, newParent, newPosition, previousPosition)
-        } else {
-            shiftDown(task.caldav, newParent, newPosition)
+        if (newPosition != null) {
+            if (newParent == previousParent && newPosition < previousPosition) {
+                shiftDown(task.caldav, newParent, newPosition, previousPosition)
+            } else {
+                shiftDown(task.caldav, newParent, newPosition)
+            }
         }
         caldavTask.cd_order = newPosition
         update(caldavTask.cd_id, caldavTask.cd_order)

app/src/main/java/org/tasks/data/SubsetCaldav.kt

@@ -4,7 +4,7 @@ class SubsetCaldav {
     var cd_id: Long = 0
     var cd_calendar: String? = null
     var cd_remote_parent: String? = null
-    var cd_order: Long = 0
+    var cd_order: Long? = null
 
     override fun equals(other: Any?): Boolean {
         if (this === other) return true

app/src/main/java/org/tasks/time/DateTime.java

@@ -252,6 +252,10 @@ public class DateTime {
     return add(Calendar.SECOND, seconds);
   }
 
+  public DateTime minusSeconds(int seconds) {
+    return subtract(Calendar.SECOND, seconds);
+  }
+
   public DateTime minusDays(int days) {
     return subtract(Calendar.DATE, days);
   }