From 49fb32f52cd7a245ee5b44dd6fa12353c3812bf6 Mon Sep 17 00:00:00 2001 From: Alex Baker Date: Wed, 20 May 2020 16:48:20 -0500 Subject: [PATCH] Set order to next - 1 when moving to top --- .../CaldavManualSortTaskAdapterTest.kt | 57 ++++++++++++++++++- .../adapter/CaldavManualSortTaskAdapter.kt | 24 ++++---- .../astrid/adapter/CaldavTaskAdapter.kt | 4 +- app/src/main/java/org/tasks/data/CaldavDao.kt | 14 +++-- .../main/java/org/tasks/data/SubsetCaldav.kt | 2 +- .../main/java/org/tasks/time/DateTime.java | 4 ++ 6 files changed, 83 insertions(+), 22 deletions(-) diff --git a/app/src/androidTest/java/com/todoroo/astrid/adapter/CaldavManualSortTaskAdapterTest.kt b/app/src/androidTest/java/com/todoroo/astrid/adapter/CaldavManualSortTaskAdapterTest.kt index d63ff4d3c..8cf52e8f8 100644 --- a/app/src/androidTest/java/com/todoroo/astrid/adapter/CaldavManualSortTaskAdapterTest.kt +++ b/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) { + private fun addTask(vararg properties: PropertyValue): 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) diff --git a/app/src/main/java/com/todoroo/astrid/adapter/CaldavManualSortTaskAdapter.kt b/app/src/main/java/com/todoroo/astrid/adapter/CaldavManualSortTaskAdapter.kt index 721bdaa3e..9d315ebd4 100644 --- a/app/src/main/java/com/todoroo/astrid/adapter/CaldavManualSortTaskAdapter.kt +++ b/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) } diff --git a/app/src/main/java/com/todoroo/astrid/adapter/CaldavTaskAdapter.kt b/app/src/main/java/com/todoroo/astrid/adapter/CaldavTaskAdapter.kt index 04d5e1d89..e76d75b08 100644 --- a/app/src/main/java/com/todoroo/astrid/adapter/CaldavTaskAdapter.kt +++ b/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 diff --git a/app/src/main/java/org/tasks/data/CaldavDao.kt b/app/src/main/java/org/tasks/data/CaldavDao.kt index 3e0091141..7d39bb6c3 100644 --- a/app/src/main/java/org/tasks/data/CaldavDao.kt +++ b/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) diff --git a/app/src/main/java/org/tasks/data/SubsetCaldav.kt b/app/src/main/java/org/tasks/data/SubsetCaldav.kt index 11cfb1764..ced87d80a 100644 --- a/app/src/main/java/org/tasks/data/SubsetCaldav.kt +++ b/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 diff --git a/app/src/main/java/org/tasks/time/DateTime.java b/app/src/main/java/org/tasks/time/DateTime.java index ebb3e500e..b0d6aaf8c 100644 --- a/app/src/main/java/org/tasks/time/DateTime.java +++ b/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); }