Don't clear completed with recurring ancestors

app/src/androidTest/java/com/todoroo/astrid/service/TaskDeleterTest.kt

@@ -0,0 +1,138 @@
+package com.todoroo.astrid.service
+
+import com.google.ical.values.RRule
+import com.natpryce.makeiteasy.MakeItEasy.with
+import com.todoroo.astrid.core.BuiltInFilterExposer.Companion.getMyTasksFilter
+import dagger.hilt.android.testing.HiltAndroidTest
+import dagger.hilt.android.testing.UninstallModules
+import kotlinx.coroutines.runBlocking
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Test
+import org.tasks.data.GoogleTaskDao
+import org.tasks.data.TaskDao
+import org.tasks.injection.InjectingTestCase
+import org.tasks.injection.ProductionModule
+import org.tasks.makers.GoogleTaskMaker
+import org.tasks.makers.GoogleTaskMaker.TASK
+import org.tasks.makers.GoogleTaskMaker.newGoogleTask
+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 TaskDeleterTest : InjectingTestCase() {
+    @Inject lateinit var taskDao: TaskDao
+    @Inject lateinit var taskDeleter: TaskDeleter
+    @Inject lateinit var googleTaskDao: GoogleTaskDao
+
+    @Test
+    fun clearCompletedTask() = runBlocking {
+        val task = taskDao.createNew(newTask(with(COMPLETION_TIME, DateTime())))
+
+        clearCompleted()
+
+        assertTrue(taskDao.fetch(task)!!.isDeleted)
+    }
+
+    @Test
+    fun dontDeleteTaskWithRecurringParent() = runBlocking {
+        val parent = taskDao.createNew(newTask(with(RRULE, RRule("RRULE:FREQ=DAILY;INTERVAL=1"))))
+        val child = taskDao.createNew(newTask(
+                with(PARENT, parent),
+                with(COMPLETION_TIME, DateTime())
+        ))
+
+        clearCompleted()
+
+        assertFalse(taskDao.fetch(child)!!.isDeleted)
+    }
+
+    @Test
+    fun dontDeleteTaskWithRecurringGrandparent() = runBlocking {
+        val grandparent = taskDao.createNew(newTask(with(RRULE, RRule("RRULE:FREQ=DAILY;INTERVAL=1"))))
+        val parent = taskDao.createNew(newTask(with(PARENT, grandparent)))
+        val child = taskDao.createNew(newTask(
+                with(PARENT, parent),
+                with(COMPLETION_TIME, DateTime())
+        ))
+
+        clearCompleted()
+
+        assertFalse(taskDao.fetch(child)!!.isDeleted)
+    }
+
+    @Test
+    fun clearGrandchildWithNoRecurringAncestors() = runBlocking {
+        val grandparent = taskDao.createNew(newTask())
+        val parent = taskDao.createNew(newTask(with(PARENT, grandparent)))
+        val child = taskDao.createNew(newTask(
+                with(PARENT, parent),
+                with(COMPLETION_TIME, DateTime())
+        ))
+
+        clearCompleted()
+
+        assertTrue(taskDao.fetch(child)!!.isDeleted)
+    }
+
+    @Test
+    fun clearGrandchildWithCompletedRecurringAncestor() = runBlocking {
+        val grandparent = taskDao.createNew(newTask(
+                with(RRULE, RRule("RRULE:FREQ=DAILY;INTERVAL=1")),
+                with(COMPLETION_TIME, DateTime())
+        ))
+        val parent = taskDao.createNew(newTask(with(PARENT, grandparent)))
+        val child = taskDao.createNew(newTask(
+                with(PARENT, parent),
+                with(COMPLETION_TIME, DateTime())
+        ))
+
+        clearCompleted()
+
+        assertTrue(taskDao.fetch(child)!!.isDeleted)
+    }
+
+    @Test
+    fun dontClearCompletedGoogleTaskWithRecurringParent() = runBlocking {
+        val parent = taskDao.createNew(newTask(with(RRULE, RRule("RRULE:FREQ=DAILY;INTERVAL=1"))))
+        val child = taskDao.createNew(newTask(with(COMPLETION_TIME, DateTime())))
+        googleTaskDao.insert(newGoogleTask(with(TASK, child), with(GoogleTaskMaker.PARENT, parent)))
+
+        clearCompleted()
+
+        assertFalse(taskDao.fetch(child)!!.isDeleted)
+    }
+
+    @Test
+    fun clearCompletedGoogleTaskWithNonRecurringParent() = runBlocking {
+        val parent = taskDao.createNew(newTask())
+        val child = taskDao.createNew(newTask(with(COMPLETION_TIME, DateTime())))
+        googleTaskDao.insert(newGoogleTask(with(TASK, child), with(GoogleTaskMaker.PARENT, parent)))
+
+        clearCompleted()
+
+        assertTrue(taskDao.fetch(child)!!.isDeleted)
+    }
+
+    @Test
+    fun clearCompletedGoogleTaskWithCompletedRecurringParent() = runBlocking {
+        val parent = taskDao.createNew(newTask(
+                with(RRULE, RRule("RRULE:FREQ=DAILY;INTERVAL=1")),
+                with(COMPLETION_TIME, DateTime())
+        ))
+        val child = taskDao.createNew(newTask(with(COMPLETION_TIME, DateTime())))
+        googleTaskDao.insert(newGoogleTask(with(TASK, child), with(GoogleTaskMaker.PARENT, parent)))
+
+        clearCompleted()
+
+        assertTrue(taskDao.fetch(child)!!.isDeleted)
+    }
+
+    private suspend fun clearCompleted() =
+            taskDeleter.clearCompleted(getMyTasksFilter(context.resources))
+}

app/src/androidTest/java/org/tasks/opentasks/OpenTasksSynchronizerTest.kt

@@ -1,7 +1,6 @@
 package org.tasks.opentasks
 
-import com.natpryce.makeiteasy.MakeItEasy
-import com.todoroo.astrid.helper.UUIDHelper
+import com.natpryce.makeiteasy.MakeItEasy.with
 import dagger.hilt.android.testing.HiltAndroidTest
 import dagger.hilt.android.testing.UninstallModules
 import kotlinx.coroutines.runBlocking
@@ -18,6 +17,7 @@ import org.tasks.data.TaskDao
 import org.tasks.injection.InjectingTestCase
 import org.tasks.injection.ProductionModule
 import org.tasks.makers.CaldavTaskMaker
+import org.tasks.makers.CaldavTaskMaker.newCaldavTask
 import org.tasks.makers.TaskMaker.newTask
 import org.tasks.preferences.Preferences
 import javax.inject.Inject
@@ -97,7 +97,6 @@ class OpenTasksSynchronizerTest : InjectingTestCase() {
         val (_, list) = openTaskDao.insertList(url = "url1")
         caldavDao.insert(CaldavCalendar().apply {
             account = list.account
-            uuid = UUIDHelper.newUUID()
             url = "url2"
         })
 
@@ -110,9 +109,9 @@ class OpenTasksSynchronizerTest : InjectingTestCase() {
     fun simplePushNewTask() = runBlocking {
         val (_, list) = openTaskDao.insertList()
         val taskId = taskDao.insert(newTask())
-        caldavDao.insert(CaldavTaskMaker.newCaldavTask(
-                MakeItEasy.with(CaldavTaskMaker.CALENDAR, list.uuid),
-                MakeItEasy.with(CaldavTaskMaker.TASK, taskId)
+        caldavDao.insert(newCaldavTask(
+                with(CaldavTaskMaker.CALENDAR, list.uuid),
+                with(CaldavTaskMaker.TASK, taskId)
         ))
 
         synchronizer.sync()

app/src/main/java/com/todoroo/astrid/service/TaskDeleter.kt

@@ -43,6 +43,9 @@ class TaskDeleter @Inject constructor(
         val completed = taskDao.fetchTasks(preferences, deleteFilter)
                 .filter(TaskContainer::isCompleted)
                 .map(TaskContainer::getId)
+                .toMutableList()
+        completed.removeAll(deletionDao.hasRecurringAncestors(completed))
+        completed.removeAll(googleTaskDao.hasRecurringParent(completed))
         markDeleted(completed)
         return completed.size
     }

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

@@ -5,6 +5,7 @@ import androidx.room.Delete
 import androidx.room.Query
 import androidx.room.Transaction
 import org.tasks.data.CaldavDao.Companion.LOCAL
+import org.tasks.db.SuspendDbUtils.chunkedMap
 import org.tasks.db.SuspendDbUtils.eachChunk
 import java.util.*
 
@@ -28,6 +29,32 @@ abstract class DeletionDao {
     @Query("DELETE FROM tasks WHERE _id IN(:ids)")
     internal abstract suspend fun deleteTasks(ids: List<Long>)
 
+    suspend fun hasRecurringAncestors(ids: List<Long>): List<Long> =
+            ids.chunkedMap { internalHasRecurringAncestors(it) }
+
+    @Query("""
+WITH RECURSIVE recursive_tasks (descendent, parent, recurring) AS (
+    SELECT _id, parent, 0
+    FROM tasks
+    WHERE _id IN (:ids)
+      AND parent > 0
+    UNION ALL
+    SELECT recursive_tasks.descendent,
+           tasks.parent,
+           CASE
+               WHEN recursive_tasks.recurring THEN 1
+               WHEN recurrence IS NOT NULL AND recurrence != '' AND completed = 0 THEN 1
+               ELSE 0
+               END
+    FROM tasks
+             INNER JOIN recursive_tasks ON recursive_tasks.parent = _id
+)
+SELECT DISTINCT(descendent)
+FROM recursive_tasks
+WHERE recurring = 1
+    """)
+    abstract suspend fun internalHasRecurringAncestors(ids: List<Long>): List<Long>
+
     @Transaction
     open suspend fun delete(ids: List<Long>) {
         ids.eachChunk {

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

@@ -2,6 +2,7 @@ package org.tasks.data
 
 import androidx.room.*
 import com.todoroo.astrid.data.Task
+import org.tasks.db.SuspendDbUtils.chunkedMap
 import org.tasks.time.DateTimeUtils.currentTimeMillis
 
 @Dao
@@ -91,6 +92,21 @@ abstract class GoogleTaskDao {
     @Query("SELECT gt_task FROM google_tasks WHERE gt_parent IN (:ids) AND gt_deleted = 0")
     abstract suspend fun getChildren(ids: List<Long>): List<Long>
 
+    suspend fun hasRecurringParent(ids: List<Long>): List<Long> =
+            ids.chunkedMap { internalHasRecurringParent(ids) }
+
+    @Query("""
+SELECT gt_task
+FROM google_tasks
+         INNER JOIN tasks ON gt_parent = _id
+WHERE gt_task IN (:ids)
+  AND gt_deleted = 0
+  AND tasks.recurrence IS NOT NULL
+  AND tasks.recurrence != ''
+  AND tasks.completed = 0
+        """)
+    abstract suspend fun internalHasRecurringParent(ids: List<Long>): List<Long>
+
     @Query("SELECT tasks.* FROM tasks JOIN google_tasks ON tasks._id = gt_task WHERE gt_parent = :taskId")
     abstract suspend fun getChildTasks(taskId: Long): List<Task>
 

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

@@ -154,18 +154,19 @@ SELECT EXISTS(SELECT 1 FROM tasks WHERE parent > 0 AND deleted = 0) AS hasSubtas
 
     suspend fun getChildren(id: Long): List<Long> = getChildren(listOf(id))
 
-    @Query("WITH RECURSIVE "
-            + " recursive_tasks (task) AS ( "
-            + " SELECT _id "
-            + " FROM tasks "
-            + "WHERE parent IN (:ids)"
-            + "UNION ALL "
-            + " SELECT _id "
-            + " FROM tasks "
-            + " INNER JOIN recursive_tasks "
-            + "  ON recursive_tasks.task = tasks.parent"
-            + " WHERE tasks.deleted = 0)"
-            + "SELECT task FROM recursive_tasks")
+    @Query("""
+WITH RECURSIVE recursive_tasks (task) AS (
+    SELECT _id
+    FROM tasks
+    WHERE parent IN (:ids)
+    UNION ALL
+    SELECT _id
+    FROM tasks
+             INNER JOIN recursive_tasks ON recursive_tasks.task = tasks.parent
+    WHERE tasks.deleted = 0)
+SELECT task
+FROM recursive_tasks
+    """)
     abstract suspend fun getChildren(ids: List<Long>): List<Long>
 
     @Query("UPDATE tasks SET collapsed = :collapsed WHERE _id = :id")