Keep records mapping copies, undo only unreshaped series, settle synced moves (#137)

A copy's record now outlives edits (it stops being undoable instead), so its
occurrence still resolves to it; moves the server already has are no longer
dropped on a conflict; a series whose rule changed since is not moved back;
detached rows with their own parent or subtasks stay out; the reopened state is
kept; the editor's tidying does not count as an edit; old records are pruned.

app/schemas/de.jeanlucmakiola.agendula.data.tasks.room.TasksDatabase/5.json

@@ -2,7 +2,7 @@
   "formatVersion": 1,
   "database": {
     "version": 5,
-    "identityHash": "64dfff356aece57e8f1b81b82b84406b",
+    "identityHash": "fcb14ff68d892c72fa7459204276abb6",
     "entities": [
       {
         "tableName": "accounts",
@@ -546,7 +546,7 @@
       },
       {
         "tableName": "advanced_occurrences",
-        "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`copy_id` INTEGER NOT NULL, `master_id` INTEGER NOT NULL, `anchor` INTEGER NOT NULL, `advanced_to` INTEGER NOT NULL, `prior_dtstart` INTEGER, `prior_due` INTEGER, `prior_status` INTEGER NOT NULL, `prior_percent_complete` INTEGER, `prior_completed_at` INTEGER, `spent` INTEGER NOT NULL, `count_after` INTEGER, `dropped_exdates` TEXT NOT NULL DEFAULT '', `from_override` INTEGER NOT NULL, PRIMARY KEY(`copy_id`), FOREIGN KEY(`copy_id`) REFERENCES `tasks`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE , FOREIGN KEY(`master_id`) REFERENCES `tasks`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )",
+        "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`copy_id` INTEGER NOT NULL, `master_id` INTEGER NOT NULL, `anchor` INTEGER NOT NULL, `advanced_to` INTEGER NOT NULL, `prior_dtstart` INTEGER, `prior_due` INTEGER, `prior_status` INTEGER NOT NULL, `prior_percent_complete` INTEGER, `prior_completed_at` INTEGER, `spent` INTEGER NOT NULL, `rule_after` TEXT, `dropped_exdates` TEXT NOT NULL DEFAULT '', `from_override` INTEGER NOT NULL, `undoable` INTEGER NOT NULL DEFAULT 1, `settled` INTEGER NOT NULL DEFAULT 0, PRIMARY KEY(`copy_id`), FOREIGN KEY(`copy_id`) REFERENCES `tasks`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE , FOREIGN KEY(`master_id`) REFERENCES `tasks`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )",
         "fields": [
           {
             "fieldPath": "copyId",
@@ -605,9 +605,9 @@
             "notNull": true
           },
           {
-            "fieldPath": "countAfter",
-            "columnName": "count_after",
-            "affinity": "INTEGER"
+            "fieldPath": "ruleAfter",
+            "columnName": "rule_after",
+            "affinity": "TEXT"
           },
           {
             "fieldPath": "droppedExdates",
@@ -621,6 +621,20 @@
             "columnName": "from_override",
             "affinity": "INTEGER",
             "notNull": true
+          },
+          {
+            "fieldPath": "undoable",
+            "columnName": "undoable",
+            "affinity": "INTEGER",
+            "notNull": true,
+            "defaultValue": "1"
+          },
+          {
+            "fieldPath": "settled",
+            "columnName": "settled",
+            "affinity": "INTEGER",
+            "notNull": true,
+            "defaultValue": "0"
           }
         ],
         "primaryKey": {
@@ -748,7 +762,7 @@
     ],
     "setupQueries": [
       "CREATE TABLE IF NOT EXISTS room_master_table (id INTEGER PRIMARY KEY,identity_hash TEXT)",
-      "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, '64dfff356aece57e8f1b81b82b84406b')"
+      "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, 'fcb14ff68d892c72fa7459204276abb6')"
     ]
   }
 }

app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/room/RoomTasksDataSourceTest.kt

@@ -586,7 +586,7 @@ class RoomTasksDataSourceTest {
         assertThat(db.tasks().entity(id)!!.dtstart).isEqualTo(at + 14.days)
         val reopened = db.tasks().allIn(listId).single { it.id != id && it.dtstart == at }
         assertThat(reopened.status).isEqualTo(TaskStatus.NEEDS_ACTION)
-        assertThat(db.advancedOccurrences().forCopy(reopened.id)).isNull()
+        assertThat(db.advancedOccurrences().forCopy(reopened.id)!!.undoable).isFalse()
     }
 
     @Test
@@ -701,17 +701,18 @@ class RoomTasksDataSourceTest {
     }
 
     @Test
-    fun aCountChangedSinceIsKept() {
+    fun aSeriesReshapedSinceIsNotMovedBack() {
         val at = now
         val id = source.insertTask(form(title = "Ten"))
         makeRecurring(id, at, rule = "FREQ=WEEKLY;COUNT=10")
         source.setCompletedInstance(id, at, completed = true)
-        db.tasks().update(db.tasks().entity(id)!!.copy(rrule = "FREQ=WEEKLY;COUNT=5"))
+        db.tasks().update(db.tasks().entity(id)!!.copy(rrule = "FREQ=DAILY;COUNT=5"))
 
         source.setCompletedInstance(id, at, completed = false)
 
-        assertThat(db.tasks().entity(id)!!.dtstart).isEqualTo(at)
-        assertThat(db.tasks().entity(id)!!.rrule).isEqualTo("FREQ=WEEKLY;COUNT=5")
+        assertThat(db.tasks().entity(id)!!.dtstart).isEqualTo(at + 7.days)
+        assertThat(db.tasks().entity(id)!!.rrule).isEqualTo("FREQ=DAILY;COUNT=5")
+        assertThat(db.tasks().allIn(listId).single { it.id != id }.status).isEqualTo(TaskStatus.NEEDS_ACTION)
     }
 
     @Test
@@ -729,6 +730,72 @@ class RoomTasksDataSourceTest {
         assertThat(db.tasks().entity(id)!!.dtstart).isEqualTo(at)
     }
 
+    @Test
+    fun anEditedCopyStillStandsForItsOccurrence() {
+        val at = now
+        val id = source.insertTask(form(title = "Weekly"))
+        makeRecurring(id, at)
+        source.setCompletedInstance(id, at, completed = true)
+        val copy = db.tasks().allIn(listId).single { it.id != id }
+
+        db.tasks().update(copy.copy(title = "Watered"))
+
+        assertThat(source.task(id, at)!!.taskId).isEqualTo(copy.id)
+    }
+
+    @Test
+    fun aDetachedOccurrenceWithALifeOfItsOwnStaysOut() {
+        val at = now
+        val id = source.insertTask(form(title = "Weekly"))
+        makeRecurring(id, at)
+        val master = db.tasks().entity(id)!!
+        val earlier = db.tasks().insert(master.copy(id = 0, masterId = id, recurrenceId = at, rrule = null, status = TaskStatus.COMPLETED))
+        source.setCompletedInstance(id, at + 7.days, completed = true)
+        source.insertTask(form(title = "Follow-up").copy(parentId = earlier))
+
+        source.setCompletedInstance(id, at + 7.days, completed = false)
+
+        assertThat(db.tasks().entity(earlier)!!.masterId).isNull()
+        assertThat(source.tasks(TaskQuery(listId = listId)).map { it.occurrenceStart?.epochSeconds })
+            .doesNotContain(at.epochSeconds)
+    }
+
+    @Test
+    fun reopeningWithProgressKeepsIt() {
+        val at = now
+        val id = source.insertTask(form(title = "Weekly"))
+        makeRecurring(id, at)
+        source.setCompletedInstance(id, at, completed = true)
+        val copy = db.tasks().allIn(listId).single { it.id != id }
+
+        source.updateTask(
+            copy.id,
+            TaskForm(title = copy.title.orEmpty(), listId = listId, start = copy.dtstart, due = copy.due, percentComplete = 40),
+            reminders = null,
+        )
+
+        assertThat(db.tasks().entity(id)!!.dtstart).isEqualTo(at)
+        assertThat(db.tasks().override(id, at)!!.percentComplete).isEqualTo(40)
+    }
+
+    @Test
+    fun theEditorsTidyingDoesNotCountAsAnEdit() {
+        val at = now
+        val id = source.insertTask(form(title = "Weekly"))
+        makeRecurring(id, at)
+        db.tasks().update(db.tasks().entity(id)!!.copy(description = "From the server\n"))
+        source.setCompletedInstance(id, at, completed = true)
+        val copy = db.tasks().allIn(listId).single { it.id != id }
+
+        source.updateTask(
+            copy.id,
+            TaskForm(title = copy.title.orEmpty(), listId = listId, description = copy.description, start = copy.dtstart, due = copy.due, percentComplete = 0),
+            reminders = null,
+        )
+
+        assertThat(db.tasks().entity(id)!!.dtstart).isEqualTo(at)
+    }
+
     @Test
     fun completingANonRecurringTaskThroughTheInstancePathWritesTheRowItself() {
         val id = source.insertTask(form(title = "Buy milk", due = now + 1.days))

app/src/androidTestFull/java/de/jeanlucmakiola/agendula/data/sync/RoomSyncStoreAdvancesTest.kt

@@ -39,4 +39,19 @@ class RoomSyncStoreAdvancesTest {
         assertThat(db.advancedOccurrences().forMaster(series)).isEmpty()
         assertThat(db.tasks().entity(copy.id)!!.isDeleted).isTrue()
     }
+
+    @Test
+    fun movesTheServerAlreadyHasStay() {
+        val list = db.taskLists().insert(TaskListEntity(name = "Home", color = 0))
+        val at = Instant.fromEpochSeconds(Clock.System.now().epochSeconds)
+        val series = db.tasks().insert(TaskEntity(listId = list, uid = "s", title = "Weekly", dtstart = at, rrule = "FREQ=WEEKLY"))
+        RoomTasksDataSource(db).setCompletedInstance(series, at, true)
+        val copy = db.tasks().allIn(list).single { it.id != series }
+        val store = RoomSyncStore(db)
+        store.markSynced(listOf(series), "s.ics", "1")
+
+        store.dropAdvances(series)
+
+        assertThat(db.tasks().entity(copy.id)!!.isDeleted).isFalse()
+    }
 }

app/src/full/java/de/jeanlucmakiola/agendula/data/sync/SyncStore.kt

@@ -106,7 +106,8 @@ class RoomSyncStore @Inject constructor(
 
     override fun dropAdvances(masterId: Long) = database.runInTransaction {
         val advanced = database.advancedOccurrences()
-        advanced.forMaster(masterId).forEach { record ->
+        // Only moves the server never saw, on copies that are still just that.
+        advanced.forMaster(masterId).filter { !it.settled && it.undoable }.forEach { record ->
             val rows = advanced.detachedFor(record.copyId).mapNotNull { it.taskId } + record.copyId
             advanced.delete(record.copyId)
             rows.mapNotNull { database.tasks().entity(it) }.forEach { row ->
@@ -118,7 +119,12 @@ class RoomSyncStore @Inject constructor(
     }
 
     override fun markSynced(taskIds: List<Long>, href: String?, eTag: String?) {
-        if (taskIds.isNotEmpty()) database.tasks().markSynced(taskIds, href, eTag)
+        if (taskIds.isEmpty()) return
+        database.runInTransaction {
+            database.tasks().markSynced(taskIds, href, eTag)
+            // The server now holds those series as moved: a later conflict on them is not about the moves.
+            database.advancedOccurrences().settle(taskIds)
+        }
     }
 
     override fun setParent(taskId: Long, parentId: Long?) {

app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksRepositoryImpl.kt

@@ -68,8 +68,7 @@ class TasksRepositoryImpl @Inject constructor(
 
     override fun taskDetail(taskId: Long, occurrenceStart: Instant?): Flow<TaskDetail?> = observing {
         dataSource.task(taskId, occurrenceStart)?.let { task ->
-            // The row the read resolved: a past occurrence may have become a task of its own.
-            val owner = if (task.seriesId == null) task.taskId else taskId
+            val owner = if (task.seriesId == null) resolvedRow(task, taskId) else taskId
             TaskDetail(
                 task = task,
                 subtasks = SeriesCollapse.visible(dataSource.subtasks(owner)).filter { it.taskId != owner },
@@ -203,8 +202,7 @@ class TasksRepositoryImpl @Inject constructor(
         // Going through the occurrence forks an override instead, which takes
         // the reminders on its own row and leaves the series' alone.
         val instance = current?.takeIf { it.isRecurring }?.occurrenceStart
-        // The row the read resolved: a past occurrence may have become a task of its own.
-        if (instance == null) dataSource.updateTask(current?.taskId ?: taskId, form, reminders)
+        if (instance == null) dataSource.updateTask(resolvedRow(current, taskId), form, reminders)
         else dataSource.updateInstance(taskId, instance, form, reminders)
     }
 
@@ -238,7 +236,7 @@ class TasksRepositoryImpl @Inject constructor(
             val current = dataSource.task(taskId, occurrenceStart)
             val seriesId = current?.seriesId
             val occurrence = current?.occurrenceStart
-            if (seriesId == null || occurrence == null) return@withContext dataSource.deleteWithSubtasks(current?.taskId ?: taskId)
+            if (seriesId == null || occurrence == null) return@withContext dataSource.deleteWithSubtasks(resolvedRow(current, taskId))
             when (scope) {
                 RecurringScope.ThisOccurrence ->
                     // An override row deletes itself into an EXDATE; a generated
@@ -294,6 +292,13 @@ class TasksRepositoryImpl @Inject constructor(
             ticks.close()
         }
     }.flowOn(io)
+
+    /**
+     * The row a read for [taskId] resolved to — a past occurrence the series moved
+     * past is a task of its own — which is the one a write has to address.
+     */
+    private fun resolvedRow(resolved: Task?, taskId: Long): Long = resolved?.taskId ?: taskId
+
 }
 
 /** Subtask levels a duplicate follows; deeper than any real list, short of a cycle. */

app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/AdvancedOccurrence.kt

@@ -13,9 +13,11 @@ import de.jeanlucmakiola.agendula.domain.TaskStatus
 import kotlin.time.Instant
 
 /**
- * How a series was moved past one of its occurrences, kept so reopening the
- * completed copy can move it back (#137). Local only: never part of a `VTODO`.
- * Holds the change, not the old series, so edits made since are kept.
+ * How a series was moved past one of its occurrences (#137). It maps the
+ * occurrence to its completed copy for as long as the copy lives, and while
+ * [undoable] it also holds what reopening the copy needs to move the series
+ * back: the change, not the old series, so edits made since are kept. Local
+ * only: never part of a `VTODO`.
  */
 @Entity(
     tableName = "advanced_occurrences",
@@ -39,12 +41,16 @@ data class AdvancedOccurrenceEntity(
     @ColumnInfo(name = "prior_completed_at") val priorCompletedAt: Instant?,
     /** Occurrences `COUNT` lost to the move. */
     @ColumnInfo(name = "spent") val spent: Int,
-    /** The `COUNT` the move left; one changed since is the user's and stays. */
-    @ColumnInfo(name = "count_after") val countAfter: Int?,
+    /** The `RRULE` the move left; once it differs, the series was reshaped and is not moved back. */
+    @ColumnInfo(name = "rule_after") val ruleAfter: String?,
     /** The `EXDATE` occurrences the move dropped, as epoch millis, comma-separated. */
     @ColumnInfo(name = "dropped_exdates", defaultValue = "") val droppedExdates: String = "",
     /** The copy was an override before, and goes back to being one. */
     @ColumnInfo(name = "from_override") val fromOverride: Boolean,
+    /** Cleared once the copy is edited, moved or reopened on its own: it is then a task of its own. */
+    @ColumnInfo(name = "undoable", defaultValue = "1") val undoable: Boolean = true,
+    /** Set once the moved series reached the server, which then holds the move itself. */
+    @ColumnInfo(name = "settled", defaultValue = "0") val settled: Boolean = false,
 ) {
     val dropped: List<Instant>
         get() = droppedExdates.split(',').mapNotNull { it.toLongOrNull()?.let(Instant::fromEpochMilliseconds) }
@@ -102,6 +108,13 @@ interface AdvancedOccurrenceDao {
     @Query("DELETE FROM advanced_occurrences WHERE copy_id = :copyId")
     fun delete(copyId: Long)
 
-    @Query("DELETE FROM advanced_occurrences WHERE master_id = :masterId")
-    fun deleteForMaster(masterId: Long)
+    @Query("UPDATE advanced_occurrences SET undoable = 0 WHERE copy_id = :copyId")
+    fun keepAsOwnTask(copyId: Long)
+
+    @Query("UPDATE advanced_occurrences SET settled = 1 WHERE master_id IN (:masterIds)")
+    fun settle(masterIds: List<Long>)
+
+    /** Records too old to matter: their occurrence is long gone from every list and reminder. */
+    @Query("DELETE FROM advanced_occurrences WHERE master_id = :masterId AND anchor < :before")
+    fun pruneBefore(masterId: Long, before: Instant)
 }

app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/RoomTasksDataSource.kt

@@ -11,6 +11,7 @@ import de.jeanlucmakiola.agendula.domain.TaskForm
 import de.jeanlucmakiola.agendula.domain.TaskList
 import de.jeanlucmakiola.agendula.domain.TaskStatus
 import de.jeanlucmakiola.agendula.domain.export.ExportTask
+import de.jeanlucmakiola.agendula.domain.normalizeTags
 import de.jeanlucmakiola.agendula.domain.recurrence.ExpansionWindow
 import de.jeanlucmakiola.agendula.domain.recurrence.RecurrenceExpander
 import de.jeanlucmakiola.agendula.domain.recurrence.RecurrenceSpec
@@ -29,6 +30,9 @@ private val WINDOW_FORWARD = 730.days
 /** Residue lines that describe the old series and must not follow the new one. */
 private val SERIES_LINES = listOf("EXDATE", "RDATE", "RRULE", "RECURRENCE-ID", "UID")
 
+/** How long a record keeps mapping a ticked occurrence to its copy. */
+private val RECORD_HORIZON = 30.days
+
 /** Only bounds the search: the occurrence count is what stops expansion, so no real gap is cut off. */
 private val SERIES_LOOKAHEAD = (366 * 1000).days
 
@@ -277,6 +281,8 @@ class RoomTasksDataSource @Inject constructor(
 
     /** See [TaskMove]: the rows move, and the old collection is left a tombstone. */
     private fun move(current: TaskEntity, edited: TaskEntity) {
+        // A copy moved elsewhere is a task of its own now.
+        advanced.keepAsOwnTask(current.id)
         val plan = TaskMove.plan(
             edited = edited,
             previous = current,
@@ -414,9 +420,12 @@ class RoomTasksDataSource @Inject constructor(
 
     private inline fun tx(crossinline block: () -> Unit) = database.runInTransaction(Runnable { block() })
 
-    private fun seriesMaster(seriesId: Long): TaskEntity {
-        val row = tasks.entity(seriesId) ?: throw TaskWriteFailedException("series $seriesId")
-        return row.masterId?.let { tasks.entity(it) } ?: row
+    private fun seriesMaster(seriesId: Long): TaskEntity = rowAndMaster(seriesId).second
+
+    /** [taskId]'s row, and its series' master — the row itself unless it is an override. */
+    private fun rowAndMaster(taskId: Long): Pair<TaskEntity, TaskEntity> {
+        val row = tasks.entity(taskId) ?: throw TaskWriteFailedException("series $taskId")
+        return row to (row.masterId?.let { tasks.entity(it) } ?: row)
     }
 
     /** The occurrence's own times: its override's if it has one, else generated. */
@@ -503,8 +512,7 @@ class RoomTasksDataSource @Inject constructor(
     }
 
     private fun closeOccurrence(taskId: Long, occurrenceStart: Instant, closing: Boolean, close: (TaskEntity) -> TaskEntity) {
-        val master = seriesMaster(taskId)
-        val row = tasks.entity(taskId)!!
+        val (row, master) = rowAndMaster(taskId)
         if (master.recurrenceSpec() == null) {
             // A plain task, or the copy a ticked occurrence left: this row is the occurrence.
             saveRow(row, close(row))
@@ -540,21 +548,28 @@ class RoomTasksDataSource @Inject constructor(
      * its own, and it no longer stands for the series' occurrence.
      */
     private fun saveRow(current: TaskEntity, edited: TaskEntity) {
-        val record = advanced.forCopy(current.id)
+        val record = advanced.forCopy(current.id)?.takeIf { it.undoable }
         if (record != null) {
-            val reopenedOnly = current.isClosed && !edited.isClosed && sameApartFromState(current, edited)
-            if (reopenedOnly && reopenCopy(current, record)) return
-            if (!sameApartFromState(current, edited) || !edited.isClosed) advanced.delete(current.id)
+            val unchanged = sameApartFromState(current, edited)
+            if (unchanged && current.isClosed && !edited.isClosed && reopenCopy(current, edited, record)) return
+            if (!unchanged || !edited.isClosed) advanced.keepAsOwnTask(current.id)
         }
         tasks.update(edited)
     }
 
-    /** Nothing a user sees changed but its state — what saving a reopened copy from the editor amounts to. */
-    private fun sameApartFromState(a: TaskEntity, b: TaskEntity) =
-        a.title == b.title && a.description == b.description && a.location == b.location && a.url == b.url &&
+    /**
+     * Nothing a user sees changed but its state — what saving a reopened copy
+     * from the editor amounts to. Compared as the editor writes them back:
+     * trimmed, blank as none, tags normalised.
+     */
+    private fun sameApartFromState(a: TaskEntity, b: TaskEntity): Boolean {
+        fun text(value: String?) = value?.trim()?.ifEmpty { null }
+        return text(a.title) == text(b.title) && text(a.description) == text(b.description) &&
+            text(a.location) == text(b.location) && text(a.url) == text(b.url) &&
             a.priority == b.priority && a.dtstart == b.dtstart && a.due == b.due && a.isAllDay == b.isAllDay &&
             a.listId == b.listId && a.parentId == b.parentId && a.rrule == b.rrule && a.rdate == b.rdate &&
-            a.categories == b.categories && a.color == b.color
+            normalizeTags(a.categories) == normalizeTags(b.categories) && a.color == b.color
+    }
 
     /** [close] applied to the master — and to [override], its last occurrence — which ends the series. */
     private fun closeSeries(master: TaskEntity, override: TaskEntity?, close: (TaskEntity) -> TaskEntity) {
@@ -614,6 +629,7 @@ class RoomTasksDataSource @Inject constructor(
             id
         }
         advanced.at(master.id, floorSecond(anchor))?.let { advanced.delete(it.copyId) }
+        advanced.pruneBefore(master.id, now - RECORD_HORIZON)
         advanced.insert(
             AdvancedOccurrenceEntity(
                 copyId = copyId,
@@ -626,7 +642,7 @@ class RoomTasksDataSource @Inject constructor(
                 priorPercentComplete = master.percentComplete,
                 priorCompletedAt = master.completedAt,
                 spent = spent,
-                countAfter = countOf(ruleAfter),
+                ruleAfter = ruleAfter,
                 droppedExdates = AdvancedOccurrenceEntity.encode(TaskFormWriter.instantsBefore(master.exdate, newAnchor, zone)),
                 fromOverride = override != null,
             ),
@@ -636,10 +652,6 @@ class RoomTasksDataSource @Inject constructor(
         return true
     }
 
-    private fun countOf(rule: String?): Int? = rule?.split(';')
-        ?.firstOrNull { it.substringBefore('=').trim().equals("COUNT", ignoreCase = true) }
-        ?.substringAfter('=')?.trim()?.toIntOrNull()
-
     /** Occurrences `COUNT` has spent before [cut]: counted before `EXDATE` (RFC 5545 §3.8.5.3). */
     private fun spentBefore(spec: RecurrenceSpec, cut: Instant): Int = RecurrenceExpander.expand(
         spec.copy(exdate = null),
@@ -683,31 +695,29 @@ class RoomTasksDataSource @Inject constructor(
 
     /**
      * Undoes [advance] for the copy [row] while the series still sits where it
-     * moved it, and the copy is still just that — same list, nothing filed under
-     * it. The anchor goes back, `COUNT` gets back what it spent unless it was
-     * changed since, and the dropped `EXDATE`s return in the list's own shape;
-     * any other edit made since stays. The occurrences detached with it become
-     * overrides again, or are excluded where their copy is gone. False when it
-     * cannot, so the caller reopens the copy as a task of its own.
+     * moved it under the same rule, and the copy is still just that — same list,
+     * nothing filed under it. The anchor goes back, `COUNT` gets back what it
+     * spent, and the dropped `EXDATE`s return in the list's own shape; any other
+     * edit made since stays. The occurrence takes [reopened]'s state: as a plain
+     * occurrence again, or as an override where that state is its own. The
+     * occurrences detached with it become overrides again, or are excluded where
+     * their row is gone or has a life of its own. False when it cannot, so the
+     * caller reopens the copy as a task of its own.
      */
-    private fun reopenCopy(row: TaskEntity, record: AdvancedOccurrenceEntity): Boolean {
+    private fun reopenCopy(row: TaskEntity, reopened: TaskEntity, record: AdvancedOccurrenceEntity): Boolean {
         val master = tasks.entity(record.masterId)?.takeIf {
-            !it.isDeleted && !it.isClosed && row.listId == it.listId &&
+            !it.isDeleted && !it.isClosed && row.listId == it.listId && it.rrule == record.ruleAfter &&
                 it.recurrenceAnchor?.let { a -> sameSecond(a, record.advancedTo) } == true
         }
-        if (master == null || tasks.subtasks(row.id).isNotEmpty()) {
-            advanced.delete(row.id)
-            return false
-        }
+        if (master == null || tasks.subtasks(row.id).isNotEmpty()) return false
         val now = clock.now()
         val zone = master.timezone ?: zone()
         val synced = lists.entity(master.listId)?.accountId != null
         val (start, due) = occurrenceTimes(master, record.priorDtstart ?: record.priorDue ?: record.anchor)
-        val rule = master.rrule?.let { if (countOf(it) == record.countAfter) TaskFormWriter.remainingCount(it, -record.spent) else it }
         var back = master.copy(
             dtstart = start,
             due = due,
-            rrule = rule,
+            rrule = master.rrule?.let { TaskFormWriter.remainingCount(it, -record.spent) },
             status = if (master.status == TaskStatus.NEEDS_ACTION) record.priorStatus else master.status,
             percentComplete = master.percentComplete ?: record.priorPercentComplete,
             completedAt = master.completedAt ?: record.priorCompletedAt,
@@ -715,16 +725,16 @@ class RoomTasksDataSource @Inject constructor(
             isDirty = true,
         )
         record.dropped.forEach { back = TaskFormWriter.excepting(back, it, now, zone, dirty = synced) }
+        // A detached row given a parent or subtasks since has a life of its own, and stays out.
+        fun returns(task: TaskEntity?) = task != null && !task.isDeleted && task.listId == master.listId &&
+            task.parentId == master.parentId && tasks.subtasks(task.id).isEmpty()
         val detached = advanced.detachedFor(row.id).map { it.anchor to it.taskId?.let(tasks::entity) }
-        detached.forEach { (at, task) ->
-            if (task == null || task.isDeleted || task.listId != master.listId) back = TaskFormWriter.excepting(back, at, now, zone, dirty = synced)
-        }
+        detached.forEach { (at, task) -> if (!returns(task)) back = TaskFormWriter.excepting(back, at, now, zone, dirty = synced) }
         tasks.update(back)
-        detached.forEach { (at, task) ->
-            if (task != null && !task.isDeleted && task.listId == master.listId) toOverride(task, back, at, now)
-        }
+        detached.forEach { (at, task) -> if (returns(task)) toOverride(task!!, back, at, now) }
         advanced.delete(row.id)
-        if (record.fromOverride) toOverride(TaskFormWriter.completed(row, false, now), back, record.anchor, now)
+        val plain = reopened.status == TaskStatus.NEEDS_ACTION && reopened.percentComplete == null
+        if (record.fromOverride || !plain) toOverride(reopened, back, record.anchor, now)
         else deleteTask(row.id)
         writes.onWritten(setOf(master.listId))
         return true

app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/TaskFormWriter.kt

@@ -122,20 +122,23 @@ object TaskFormWriter {
 
     /** The instants of an `EXDATE`/`RDATE` list before [cut]; values it cannot read are left out. */
     fun instantsBefore(values: String?, cut: Instant, zone: String): List<Instant> =
-        values?.split(',')?.map(String::trim)?.mapNotNull { readValue(it, zone) }?.filter { it < cut }.orEmpty()
+        parsed(values, zone).mapNotNull { it.second }.filter { it < cut }
 
     /** The values of an `EXDATE`/`RDATE` list at or after [cut]. */
     fun valuesFrom(values: String?, cut: Instant, zone: String): String? =
         mapValues(values, zone) { it.takeIf { at -> at >= cut } }
 
     private fun mapValues(values: String?, zone: String, map: (Instant) -> Instant?): String? {
-        val list = values?.split(',')?.map(String::trim)?.filter(String::isNotEmpty) ?: return values
-        return list.mapNotNull { raw ->
-            val at = readValue(raw, zone) ?: return@mapNotNull raw
-            map(at)?.let { writeValueLike(raw, it, zone) }
+        if (values == null) return null
+        return parsed(values, zone).mapNotNull { (raw, at) ->
+            if (at == null) raw else map(at)?.let { writeValueLike(raw, it, zone) }
         }.joinToString(",").ifEmpty { null }
     }
 
+    /** Each value of an `EXDATE`/`RDATE` list with its instant, or null where it cannot be read. */
+    private fun parsed(values: String?, zone: String): List<Pair<String, Instant?>> =
+        values?.split(',')?.map(String::trim)?.filter(String::isNotEmpty)?.map { it to readValue(it, zone) }.orEmpty()
+
     private fun readValue(raw: String, zone: String): Instant? = runCatching {
         val z = java.time.ZoneId.of(zone)
         val instant = when {

app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/TasksDatabase.kt

@@ -188,7 +188,7 @@ abstract class TasksDatabase : RoomDatabase() {
         val MIGRATION_4_5 = object : Migration(4, 5) {
             override fun migrate(db: SupportSQLiteDatabase) {
                 db.execSQL(
-                    "CREATE TABLE IF NOT EXISTS `advanced_occurrences` (`copy_id` INTEGER NOT NULL, `master_id` INTEGER NOT NULL, `anchor` INTEGER NOT NULL, `advanced_to` INTEGER NOT NULL, `prior_dtstart` INTEGER, `prior_due` INTEGER, `prior_status` INTEGER NOT NULL, `prior_percent_complete` INTEGER, `prior_completed_at` INTEGER, `spent` INTEGER NOT NULL, `count_after` INTEGER, `dropped_exdates` TEXT NOT NULL DEFAULT '', `from_override` INTEGER NOT NULL, PRIMARY KEY(`copy_id`), FOREIGN KEY(`copy_id`) REFERENCES `tasks`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE , FOREIGN KEY(`master_id`) REFERENCES `tasks`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )",
+                    "CREATE TABLE IF NOT EXISTS `advanced_occurrences` (`copy_id` INTEGER NOT NULL, `master_id` INTEGER NOT NULL, `anchor` INTEGER NOT NULL, `advanced_to` INTEGER NOT NULL, `prior_dtstart` INTEGER, `prior_due` INTEGER, `prior_status` INTEGER NOT NULL, `prior_percent_complete` INTEGER, `prior_completed_at` INTEGER, `spent` INTEGER NOT NULL, `rule_after` TEXT, `dropped_exdates` TEXT NOT NULL DEFAULT '', `from_override` INTEGER NOT NULL, `undoable` INTEGER NOT NULL DEFAULT 1, `settled` INTEGER NOT NULL DEFAULT 0, PRIMARY KEY(`copy_id`), FOREIGN KEY(`copy_id`) REFERENCES `tasks`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE , FOREIGN KEY(`master_id`) REFERENCES `tasks`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )",
                 )
                 db.execSQL(
                     "CREATE INDEX IF NOT EXISTS `index_advanced_occurrences_master_id_anchor` ON `advanced_occurrences` (`master_id`, `anchor`)",