From 95e7172362e33f1d3fef09e37277cbe7b45c350b Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Fri, 6 Nov 2026 19:31:25 +0100 Subject: [PATCH] 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. --- .../5.json | 26 ++++-- .../tasks/room/RoomTasksDataSourceTest.kt | 77 ++++++++++++++-- .../data/sync/RoomSyncStoreAdvancesTest.kt | 15 ++++ .../agendula/data/sync/SyncStore.kt | 10 ++- .../data/tasks/TasksRepositoryImpl.kt | 15 ++-- .../data/tasks/room/AdvancedOccurrence.kt | 27 ++++-- .../data/tasks/room/RoomTasksDataSource.kt | 88 +++++++++++-------- .../data/tasks/room/TaskFormWriter.kt | 13 +-- .../agendula/data/tasks/room/TasksDatabase.kt | 2 +- 9 files changed, 203 insertions(+), 70 deletions(-) diff --git a/app/schemas/de.jeanlucmakiola.agendula.data.tasks.room.TasksDatabase/5.json b/app/schemas/de.jeanlucmakiola.agendula.data.tasks.room.TasksDatabase/5.json index dd13a3a..30c5361 100644 --- a/app/schemas/de.jeanlucmakiola.agendula.data.tasks.room.TasksDatabase/5.json +++ b/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')" ] } } \ No newline at end of file diff --git a/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/room/RoomTasksDataSourceTest.kt b/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/room/RoomTasksDataSourceTest.kt index 533ce71..78d4223 100644 --- a/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/room/RoomTasksDataSourceTest.kt +++ b/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)) diff --git a/app/src/androidTestFull/java/de/jeanlucmakiola/agendula/data/sync/RoomSyncStoreAdvancesTest.kt b/app/src/androidTestFull/java/de/jeanlucmakiola/agendula/data/sync/RoomSyncStoreAdvancesTest.kt index ea85e87..2551e25 100644 --- a/app/src/androidTestFull/java/de/jeanlucmakiola/agendula/data/sync/RoomSyncStoreAdvancesTest.kt +++ b/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() + } } diff --git a/app/src/full/java/de/jeanlucmakiola/agendula/data/sync/SyncStore.kt b/app/src/full/java/de/jeanlucmakiola/agendula/data/sync/SyncStore.kt index c95b8ab..d29b918 100644 --- a/app/src/full/java/de/jeanlucmakiola/agendula/data/sync/SyncStore.kt +++ b/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, 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?) { diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksRepositoryImpl.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksRepositoryImpl.kt index b578d0a..84b039c 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksRepositoryImpl.kt +++ b/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 = 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. */ diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/AdvancedOccurrence.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/AdvancedOccurrence.kt index 81f950f..836092a 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/AdvancedOccurrence.kt +++ b/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 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) + + /** 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) } diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/RoomTasksDataSource.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/RoomTasksDataSource.kt index 2e33011..ac21b02 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/RoomTasksDataSource.kt +++ b/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 { + 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 diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/TaskFormWriter.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/TaskFormWriter.kt index e849b60..c7743f0 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/TaskFormWriter.kt +++ b/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 = - 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> = + 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 { diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/TasksDatabase.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/TasksDatabase.kt index d3dc1e8..6d81763 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/TasksDatabase.kt +++ b/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`)",