Update modification timestamp logic (#2585)

app/src/main/java/com/todoroo/astrid/dao/TaskDao.kt

@@ -106,9 +106,8 @@ class TaskDao @Inject constructor(
         }
 
     suspend fun save(task: Task, original: Task?) {
-        if (taskDao.update(task, original)) {
-            afterUpdate(task, original)
-        }
+        val updated = taskDao.update(task, original)
+        afterUpdate(updated, original)
     }
 
     private suspend fun afterUpdate(task: Task, original: Task?) {

app/src/main/java/com/todoroo/astrid/data/Task.kt

@@ -187,31 +187,25 @@ data class Task(
     val isNew: Boolean
         get() = id == NO_ID
 
-    fun insignificantChange(task: Task?): Boolean {
-        if (this === task) {
-            return true
-        }
-        return if (task == null) {
-            false
-        } else id == task.id
-                && title == task.title
-                && priority == task.priority
-                && dueDate == task.dueDate
-                && hideUntil == task.hideUntil
-                && creationDate == task.creationDate
-                && modificationDate == task.modificationDate
-                && completionDate == task.completionDate
-                && deletionDate == task.deletionDate
-                && notes == task.notes
-                && estimatedSeconds == task.estimatedSeconds
-                && elapsedSeconds == task.elapsedSeconds
-                && ringFlags == task.ringFlags
-                && recurrence == task.recurrence
-                && calendarURI == task.calendarURI
-                && parent == task.parent
-                && remoteId == task.remoteId
-                && order == task.order
-    }
+    fun significantChange(task: Task): Boolean =
+        id != task.id
+                || title != task.title
+                || priority != task.priority
+                || dueDate != task.dueDate
+                || hideUntil != task.hideUntil
+                || creationDate != task.creationDate
+                || modificationDate != task.modificationDate
+                || completionDate != task.completionDate
+                || deletionDate != task.deletionDate
+                || notes != task.notes
+                || estimatedSeconds != task.estimatedSeconds
+                || elapsedSeconds != task.elapsedSeconds
+                || ringFlags != task.ringFlags
+                || recurrence != task.recurrence
+                || calendarURI != task.calendarURI
+                || parent != task.parent
+                || remoteId != task.remoteId
+                || order != task.order
 
     fun googleTaskUpToDate(original: Task?): Boolean {
         if (this === original) {

app/src/main/java/org/tasks/caldav/iCalendar.kt

@@ -215,7 +215,7 @@ class iCalendar @Inject constructor(
                     `object` = obj
                 )
         val isNew = caldavTask.id == com.todoroo.astrid.data.Task.NO_ID
-        val dirty = task.modificationDate > caldavTask.lastSync || caldavTask.lastSync == 0L
+        val dirty = !isNew && task.modificationDate > caldavTask.lastSync
         val local = vtodoCache.getVtodo(calendar, caldavTask)?.let { fromVtodo(it) }
         task.applyRemote(remote, local)
         caldavTask.applyRemote(remote, local)
@@ -263,7 +263,11 @@ class iCalendar @Inject constructor(
 
         task.suppressSync()
         task.suppressRefresh()
-        taskDao.save(task)
+        if (isNew) {
+            taskDao.save(task, null)
+        } else {
+            taskDao.save(task)
+        }
         vtodoCache.putVtodo(calendar, caldavTask, vtodo)
         caldavTask.etag = eTag
         if (!dirty) {

app/src/main/java/org/tasks/caldav/iCalendarMerge.kt

@@ -2,6 +2,7 @@ package org.tasks.caldav
 
 import at.bitfire.ical4android.Task
 import com.todoroo.andlib.utility.DateUtilities
+import com.todoroo.andlib.utility.DateUtilities.now
 import com.todoroo.astrid.data.Task.Priority.Companion.HIGH
 import com.todoroo.astrid.data.Task.Priority.Companion.LOW
 import com.todoroo.astrid.data.Task.Priority.Companion.MEDIUM
@@ -23,6 +24,7 @@ fun com.todoroo.astrid.data.Task.applyRemote(
 ): com.todoroo.astrid.data.Task {
     applyCompletedAt(remote, local)
     applyCreatedAt(remote, local)
+    applyModified(remote, local)
     applyTitle(remote, local)
     applyDescription(remote, local)
     applyPriority(remote, local)
@@ -61,7 +63,16 @@ private fun com.todoroo.astrid.data.Task.applyCreatedAt(remote: Task, local: Tas
     val localCreated = local?.createdAt?.let { newDateTime(it, UTC) }?.toLocal()?.millis
     if (localCreated == null || localCreated == creationDate) {
         remote.createdAt?.let {
-            creationDate = newDateTime(it, UTC).toLocal().millis
+            creationDate = newDateTime(it, UTC).toLocal().millis.coerceAtMost(now())
+        }
+    }
+}
+
+private fun com.todoroo.astrid.data.Task.applyModified(remote: Task, local: Task?) {
+    val localModified = local?.lastModified?.let { newDateTime(it, UTC) }?.toLocal()?.millis
+    if (localModified == null || localModified == modificationDate) {
+        remote.lastModified?.let {
+            modificationDate = newDateTime(it, UTC).toLocal().millis.coerceAtMost(now())
         }
     }
 }

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

@@ -188,15 +188,19 @@ FROM recursive_tasks
     @Insert
     abstract suspend fun insert(task: Task): Long
 
-    suspend fun update(task: Task, original: Task? = null): Boolean {
-        if (!task.insignificantChange(original)) {
-            task.modificationDate = now()
-        }
-        return updateInternal(task) == 1
-    }
+    suspend fun update(task: Task, original: Task? = null): Task =
+        task
+            .copy(
+                modificationDate = when {
+                    original?.let { task.significantChange(it) } == true -> now()
+                    task.modificationDate == 0L -> task.creationDate
+                    else -> task.modificationDate
+                }
+            )
+            .also { updateInternal(it) }
 
     @Update
-    internal abstract suspend fun updateInternal(task: Task): Int
+    internal abstract suspend fun updateInternal(task: Task)
 
     suspend fun createNew(task: Task): Long {
         task.id = NO_ID