From 82ea12f0dbeef3cb0d4f89cf4b88aa3fb181207b Mon Sep 17 00:00:00 2001 From: Alex Baker Date: Fri, 22 Jan 2021 15:47:32 -0600 Subject: [PATCH] Uncomplete subtasks after scheduling repeat --- .../astrid/repeats/RepeatWithSubtasksTests.kt | 52 +++++++++++++++++++ .../astrid/repeats/RepeatTaskHelper.kt | 6 ++- .../todoroo/astrid/service/TaskCompleter.kt | 16 ++++-- .../astrid/repeats/RepeatTaskHelperTest.kt | 15 +++--- 4 files changed, 74 insertions(+), 15 deletions(-) create mode 100644 app/src/androidTest/java/com/todoroo/astrid/repeats/RepeatWithSubtasksTests.kt diff --git a/app/src/androidTest/java/com/todoroo/astrid/repeats/RepeatWithSubtasksTests.kt b/app/src/androidTest/java/com/todoroo/astrid/repeats/RepeatWithSubtasksTests.kt new file mode 100644 index 000000000..2c5b269f7 --- /dev/null +++ b/app/src/androidTest/java/com/todoroo/astrid/repeats/RepeatWithSubtasksTests.kt @@ -0,0 +1,52 @@ +package com.todoroo.astrid.repeats + +import com.google.ical.values.RRule +import com.natpryce.makeiteasy.MakeItEasy.with +import dagger.hilt.android.testing.HiltAndroidTest +import dagger.hilt.android.testing.UninstallModules +import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertFalse +import org.junit.Test +import org.tasks.data.TaskDao +import org.tasks.injection.InjectingTestCase +import org.tasks.injection.ProductionModule +import org.tasks.makers.TaskMaker.COMPLETION_TIME +import org.tasks.makers.TaskMaker.PARENT +import org.tasks.makers.TaskMaker.RRULE +import org.tasks.makers.TaskMaker.newTask +import org.tasks.time.DateTime +import javax.inject.Inject + +@UninstallModules(ProductionModule::class) +@HiltAndroidTest +class RepeatWithSubtasksTests : InjectingTestCase() { + @Inject lateinit var taskDao: TaskDao + @Inject lateinit var repeat: RepeatTaskHelper + + @Test + fun uncompleteGrandchildren() = runBlocking { + val grandparent = taskDao.createNew(newTask(with(RRULE, RRule("RRULE:FREQ=DAILY")))) + val parent = taskDao.createNew(newTask(with(PARENT, grandparent))) + val child = taskDao.createNew(newTask( + with(PARENT, parent), + with(COMPLETION_TIME, DateTime()) + )) + + repeat.handleRepeat(taskDao.fetch(grandparent)!!) + + assertFalse(taskDao.fetch(child)!!.isCompleted) + } + + @Test + fun uncompleteGoogleTaskChildren() = runBlocking { + val parent = taskDao.createNew(newTask(with(RRULE, RRule("RRULE:FREQ=DAILY")))) + val child = taskDao.createNew(newTask( + with(PARENT, parent), + with(COMPLETION_TIME, DateTime()) + )) + + repeat.handleRepeat(taskDao.fetch(parent)!!) + + assertFalse(taskDao.fetch(child)!!.isCompleted) + } +} \ No newline at end of file diff --git a/app/src/main/java/com/todoroo/astrid/repeats/RepeatTaskHelper.kt b/app/src/main/java/com/todoroo/astrid/repeats/RepeatTaskHelper.kt index af187ae19..f42ebc73d 100644 --- a/app/src/main/java/com/todoroo/astrid/repeats/RepeatTaskHelper.kt +++ b/app/src/main/java/com/todoroo/astrid/repeats/RepeatTaskHelper.kt @@ -13,6 +13,7 @@ import com.todoroo.astrid.dao.TaskDao import com.todoroo.astrid.data.Task import com.todoroo.astrid.data.Task.Companion.createDueDate import com.todoroo.astrid.gcal.GCalHelper +import com.todoroo.astrid.service.TaskCompleter import org.tasks.LocalBroadcastManager import org.tasks.date.DateTimeUtils.newDate import org.tasks.date.DateTimeUtils.newDateTime @@ -27,7 +28,9 @@ class RepeatTaskHelper @Inject constructor( private val gcalHelper: GCalHelper, private val alarmService: AlarmService, private val taskDao: TaskDao, - private val localBroadcastManager: LocalBroadcastManager) { + private val localBroadcastManager: LocalBroadcastManager, + private val taskCompleter: TaskCompleter, +) { suspend fun handleRepeat(task: Task) { val recurrence = task.sanitizedRecurrence() val repeatAfterCompletion = task.repeatAfterCompletion() @@ -68,6 +71,7 @@ class RepeatTaskHelper @Inject constructor( .takeIf { it > 0 } ?: newDueDate - (computeNextDueDate(task, recurrence, repeatAfterCompletion) - newDueDate) alarmService.rescheduleAlarms(task.id, previousDueDate, newDueDate) + taskCompleter.completeChildren(task.id, 0L) localBroadcastManager.broadcastRepeat(task.id, previousDueDate, newDueDate) } } diff --git a/app/src/main/java/com/todoroo/astrid/service/TaskCompleter.kt b/app/src/main/java/com/todoroo/astrid/service/TaskCompleter.kt index ae32b42f8..282cb4c03 100644 --- a/app/src/main/java/com/todoroo/astrid/service/TaskCompleter.kt +++ b/app/src/main/java/com/todoroo/astrid/service/TaskCompleter.kt @@ -20,13 +20,19 @@ class TaskCompleter @Inject internal constructor( suspend fun setComplete(item: Task, completed: Boolean) { val completionDate = if (completed) DateUtilities.now() else 0L setComplete(listOf(item), completionDate) - val tasks = googleTaskDao.getChildTasks(item.id) - .plus(taskDao.getChildren(item.id) + completeChildren(item.id, completionDate) + } + + suspend fun completeChildren(id: Long, completionDate: Long) { + googleTaskDao + .getChildTasks(id) + .plus(taskDao.getChildren(id) .takeIf { it.isNotEmpty() } ?.let { taskDao.fetch(it) } - ?: emptyList()) - .filter { it.isCompleted != completed } - setComplete(tasks, completionDate) + ?: emptyList() + ) + .filter { it.isCompleted != completionDate > 0 } + .let { setComplete(it, completionDate)} } private suspend fun setComplete(tasks: List, completionDate: Long) { diff --git a/app/src/test/java/com/todoroo/astrid/repeats/RepeatTaskHelperTest.kt b/app/src/test/java/com/todoroo/astrid/repeats/RepeatTaskHelperTest.kt index ff5931ae4..9489d9c37 100644 --- a/app/src/test/java/com/todoroo/astrid/repeats/RepeatTaskHelperTest.kt +++ b/app/src/test/java/com/todoroo/astrid/repeats/RepeatTaskHelperTest.kt @@ -7,6 +7,7 @@ import com.todoroo.astrid.alarms.AlarmService import com.todoroo.astrid.dao.TaskDao import com.todoroo.astrid.data.Task import com.todoroo.astrid.gcal.GCalHelper +import com.todoroo.astrid.service.TaskCompleter import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.runBlockingTest import org.junit.After @@ -35,6 +36,7 @@ class RepeatTaskHelperTest { private lateinit var gCalHelper: GCalHelper private lateinit var helper: RepeatTaskHelper private lateinit var mocks: InOrder + private lateinit var taskCompleter: TaskCompleter @Before fun setUp() { @@ -42,8 +44,9 @@ class RepeatTaskHelperTest { alarmService = Mockito.mock(AlarmService::class.java) gCalHelper = Mockito.mock(GCalHelper::class.java) localBroadcastManager = Mockito.mock(LocalBroadcastManager::class.java) + taskCompleter = Mockito.mock(TaskCompleter::class.java) mocks = Mockito.inOrder(alarmService, gCalHelper, localBroadcastManager) - helper = RepeatTaskHelper(gCalHelper, alarmService, taskDao, localBroadcastManager) + helper = RepeatTaskHelper(gCalHelper, alarmService, taskDao, localBroadcastManager, taskCompleter) } @After @@ -190,14 +193,8 @@ class RepeatTaskHelperTest { freezeClock { repeatAndVerify( task, - Task.createDueDate( - Task.URGENCY_SPECIFIC_DAY, - DateTime().millis - ), - Task.createDueDate( - Task.URGENCY_SPECIFIC_DAY, - DateTime().plusDays(1).millis - ) + Task.createDueDate(Task.URGENCY_SPECIFIC_DAY, DateTime().millis), + Task.createDueDate(Task.URGENCY_SPECIFIC_DAY, DateTime().plusDays(1).millis) ) } }