Move a series on when its first occurrence is ticked off (#137)
Agendula's own store kept the series open and wrote a completed RECURRENCE-ID override, which Infomaniak and Apple Reminders show as still open. The series now advances to its next occurrence and a completed copy is kept; reopening that occurrence moves it back.
This commit is contained in:
1 parent
7bcac70cec
commit
9e5e98ccee
4 files changed
+244
-19
No files matched your search
@@ -7,6 +7,13 @@ All notable changes to this project are documented here. The format follows
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Changed
|
||||
- Ticking off the current occurrence of a repeating task moves the series on
|
||||
to its next date and keeps a completed copy, as tasks.org and Apple
|
||||
Reminders do. Other apps and the web views of servers such as Infomaniak now
|
||||
see it as done instead of still open. Unticking it moves the series back
|
||||
(#137).
|
||||
|
||||
## [1.3.0] - 2026-11-02
|
||||
|
||||
### Added
|
||||
|
||||
+8
-4
@@ -685,9 +685,11 @@ class ProviderWriterTest {
|
||||
store().setCompletedInstance(series, today, false)
|
||||
val override = db.tasks().overrides(series).single()
|
||||
db.tasks().update(override.copy(title = "Standup (moved room)"))
|
||||
val row = readRow(ProviderTable.INSTANCES, instanceId(today))
|
||||
// The second occurrence: completing the first moves the series on instead.
|
||||
val second = today + 1.days
|
||||
val row = readRow(ProviderTable.INSTANCES, instanceId(second))
|
||||
|
||||
writer.update(ProviderTable.INSTANCES, row + (Tasks.STATUS to 2), byId(ProviderTable.INSTANCES, instanceId(today)), null)
|
||||
writer.update(ProviderTable.INSTANCES, row + (Tasks.STATUS to 2), byId(ProviderTable.INSTANCES, instanceId(second)), null)
|
||||
|
||||
assertThat(task(series)!!.title).isEqualTo("Standup")
|
||||
assertThat(task(override.id)!!.title).isEqualTo("Standup (moved room)")
|
||||
@@ -734,9 +736,11 @@ class ProviderWriterTest {
|
||||
val series = db.tasks().insert(
|
||||
TaskEntity(listId = home, uid = "f", title = "Floating", due = today, rrule = "FREQ=DAILY;COUNT=3", timezone = null),
|
||||
)
|
||||
val row = readRow(ProviderTable.INSTANCES, instanceId(today))
|
||||
// The second occurrence: completing the first moves the series on instead.
|
||||
val second = today + 1.days
|
||||
val row = readRow(ProviderTable.INSTANCES, instanceId(second))
|
||||
|
||||
writer.update(ProviderTable.INSTANCES, row + (Tasks.STATUS to 2), byId(ProviderTable.INSTANCES, instanceId(today)), null)
|
||||
writer.update(ProviderTable.INSTANCES, row + (Tasks.STATUS to 2), byId(ProviderTable.INSTANCES, instanceId(second)), null)
|
||||
writer.update(ProviderTable.TASKS, mapOf(Tasks.TITLE to "Still floating"), byId(ProviderTable.TASKS, series), null)
|
||||
|
||||
assertThat(db.tasks().overrides(series).single().status).isEqualTo(TaskStatus.COMPLETED)
|
||||
|
||||
+94
-5
@@ -211,7 +211,8 @@ class RoomTasksDataSourceTest {
|
||||
@Test
|
||||
fun completingOneOccurrenceLeavesTheRestOfTheSeriesOpen() {
|
||||
val id = source.insertTask(form(title = "Water the plants"))
|
||||
makeRecurring(id, now)
|
||||
// Overdue occurrences before it, so it is not the first and forks an override.
|
||||
makeRecurring(id, now - 14.days)
|
||||
val open = { source.tasks(TaskQuery(listId = listId)).filter { it.taskId == id } }
|
||||
val before = open()
|
||||
val target = before.first { it.distanceFromCurrent == 0 }
|
||||
@@ -237,7 +238,8 @@ class RoomTasksDataSourceTest {
|
||||
@Test
|
||||
fun reopeningACompletedOccurrenceReusesItsOverride() {
|
||||
val id = source.insertTask(form(title = "Water the plants"))
|
||||
makeRecurring(id, now)
|
||||
// Overdue occurrences before it, so it is not the first and forks an override.
|
||||
makeRecurring(id, now - 14.days)
|
||||
val target = source.tasks(TaskQuery(listId = listId))
|
||||
.first { it.taskId == id && it.distanceFromCurrent == 0 }
|
||||
|
||||
@@ -254,7 +256,8 @@ class RoomTasksDataSourceTest {
|
||||
@Test
|
||||
fun cancellingOneOccurrenceLeavesTheRestOfTheSeriesOpen() {
|
||||
val id = source.insertTask(form(title = "Water the plants"))
|
||||
makeRecurring(id, now)
|
||||
// Overdue occurrences before it, so it is not the first and forks an override.
|
||||
makeRecurring(id, now - 14.days)
|
||||
val target = source.tasks(TaskQuery(listId = listId))
|
||||
.first { it.taskId == id && it.distanceFromCurrent == 0 }
|
||||
|
||||
@@ -269,6 +272,90 @@ class RoomTasksDataSourceTest {
|
||||
assertThat(db.tasks().override(id, target.occurrenceStart)!!.status).isEqualTo(TaskStatus.NEEDS_ACTION)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun completingTheFirstOccurrenceMovesTheSeriesOnAndKeepsACopy() {
|
||||
val at = now
|
||||
val id = source.insertTask(form(title = "Water the plants"))
|
||||
makeRecurring(id, at, rule = "FREQ=WEEKLY;COUNT=4")
|
||||
|
||||
source.setCompletedInstance(id, at, completed = true)
|
||||
|
||||
val master = db.tasks().entity(id)!!
|
||||
assertThat(master.status).isEqualTo(TaskStatus.NEEDS_ACTION)
|
||||
assertThat(master.dtstart).isEqualTo(at + 7.days)
|
||||
assertThat(master.due).isEqualTo(at + 8.days)
|
||||
assertThat(master.rrule).isEqualTo("FREQ=WEEKLY;COUNT=3")
|
||||
assertThat(master.isDirty).isTrue()
|
||||
assertThat(db.tasks().overrides(id)).isEmpty()
|
||||
val copy = db.tasks().allIn(listId).single { it.id != id }
|
||||
assertThat(copy.status).isEqualTo(TaskStatus.COMPLETED)
|
||||
assertThat(copy.uid).isNotEqualTo(master.uid)
|
||||
assertThat(copy.rrule).isNull()
|
||||
assertThat(copy.masterId).isNull()
|
||||
assertThat(copy.dtstart).isEqualTo(at)
|
||||
assertThat(copy.title).isEqualTo("Water the plants")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun reopeningTheOccurrenceMovesTheSeriesBack() {
|
||||
val at = now
|
||||
val id = source.insertTask(form(title = "Water the plants"))
|
||||
makeRecurring(id, at, rule = "FREQ=WEEKLY;COUNT=4")
|
||||
source.setCompletedInstance(id, at, completed = true)
|
||||
|
||||
source.setCompletedInstance(id, at, completed = false)
|
||||
|
||||
val master = db.tasks().entity(id)!!
|
||||
assertThat(master.dtstart).isEqualTo(at)
|
||||
assertThat(master.rrule).isEqualTo("FREQ=WEEKLY;COUNT=4")
|
||||
assertThat(db.tasks().allIn(listId).filter { !it.isDeleted }.map { it.id }).containsExactly(id)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun completingAnEditedFirstOccurrenceKeepsItsEdits() {
|
||||
val at = now
|
||||
val id = source.insertTask(form(title = "Water the plants"))
|
||||
makeRecurring(id, at)
|
||||
source.updateInstance(id, at, TaskForm(title = "Water the cactus", listId = listId, start = at, due = at + 1.days), reminders = null)
|
||||
val override = db.tasks().override(id, at)!!
|
||||
|
||||
source.setCompletedInstance(override.id, at, completed = true)
|
||||
|
||||
assertThat(db.tasks().overrides(id)).isEmpty()
|
||||
assertThat(db.tasks().entity(id)!!.dtstart).isEqualTo(at + 7.days)
|
||||
val copy = db.tasks().allIn(listId).single { it.id != id }
|
||||
assertThat(copy.title).isEqualTo("Water the cactus")
|
||||
assertThat(copy.status).isEqualTo(TaskStatus.COMPLETED)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun completingTheLastOccurrenceClosesTheSeries() {
|
||||
val at = now
|
||||
val id = source.insertTask(form(title = "Water the plants"))
|
||||
makeRecurring(id, at, rule = "FREQ=WEEKLY;COUNT=1")
|
||||
|
||||
source.setCompletedInstance(id, at, completed = true)
|
||||
|
||||
assertThat(db.tasks().entity(id)!!.status).isEqualTo(TaskStatus.COMPLETED)
|
||||
assertThat(db.tasks().allIn(listId).map { it.id }).containsExactly(id)
|
||||
|
||||
source.setCompletedInstance(id, at, completed = false)
|
||||
|
||||
assertThat(db.tasks().entity(id)!!.status).isEqualTo(TaskStatus.NEEDS_ACTION)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun cancellingTheFirstOccurrenceMovesTheSeriesOn() {
|
||||
val at = now
|
||||
val id = source.insertTask(form(title = "Water the plants"))
|
||||
makeRecurring(id, at)
|
||||
|
||||
source.setCancelledInstance(id, at, cancelled = true)
|
||||
|
||||
assertThat(db.tasks().entity(id)!!.dtstart).isEqualTo(at + 7.days)
|
||||
assertThat(db.tasks().allIn(listId).single { it.id != id }.status).isEqualTo(TaskStatus.CANCELLED)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun completingANonRecurringTaskThroughTheInstancePathWritesTheRowItself() {
|
||||
val id = source.insertTask(form(title = "Buy milk", due = now + 1.days))
|
||||
@@ -657,10 +744,12 @@ class RoomTasksDataSourceTest {
|
||||
val outOfWindow = db.tasks().insert(
|
||||
TaskEntity(listId = listId, uid = "old", title = "old", dtstart = longAgo, rrule = "FREQ=DAILY;COUNT=2", parentId = parent),
|
||||
)
|
||||
val at = now
|
||||
val series = db.tasks().insert(
|
||||
TaskEntity(listId = listId, uid = "s", title = "s", dtstart = now, rrule = "FREQ=DAILY;COUNT=3", parentId = parent),
|
||||
TaskEntity(listId = listId, uid = "s", title = "s", dtstart = at, rrule = "FREQ=DAILY;COUNT=3", parentId = parent),
|
||||
)
|
||||
source.setCompletedInstance(series, now, true)
|
||||
// Not the first occurrence, so it forks an override rather than a copy.
|
||||
source.setCompletedInstance(series, at + 1.days, true)
|
||||
val plain = source.insertTask(form("plain").copy(parentId = parent))
|
||||
|
||||
assertThat(source.subtaskIds(parent)).containsExactly(outOfWindow, series, plain)
|
||||
|
||||
+135
-10
@@ -28,6 +28,11 @@ 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")
|
||||
|
||||
private const val OCCURRENCE_MARK = "X-AGENDULA-OCCURRENCE-OF:"
|
||||
|
||||
/** How far ahead to look for a series' next occurrence: past any yearly gap, with room to spare. */
|
||||
private val SERIES_LOOKAHEAD = (366 * 10).days
|
||||
|
||||
/** Enough to count every occurrence before a split point without bounding a real series. */
|
||||
internal const val SPLIT_COUNT_CEILING = 100_000
|
||||
|
||||
@@ -479,18 +484,138 @@ class RoomTasksDataSource @Inject constructor(
|
||||
}
|
||||
|
||||
/**
|
||||
* Ticking one occurrence forks a `RECURRENCE-ID` override carrying the
|
||||
* completion — the same model [updateInstance] writes. Writing the status onto
|
||||
* the master instead would close the series: the master is what
|
||||
* [TaskDao.tasks] filters on, so every occurrence, past and future, would
|
||||
* leave every list at once.
|
||||
* Ticking a series' first open occurrence moves the series on to its next
|
||||
* one and keeps a completed copy, as tasks.org and Apple Reminders do — a
|
||||
* completed `RECURRENCE-ID` override alone leaves the series showing as open
|
||||
* in most other clients (#137). Unticking that copy's occurrence moves the
|
||||
* series back. Any other occurrence forks an override carrying the completion,
|
||||
* the same model [updateInstance] writes. Writing the status onto the master
|
||||
* instead would close the series.
|
||||
*/
|
||||
override fun setCompletedInstance(taskId: Long, occurrenceStart: Instant, completed: Boolean) =
|
||||
writeInstance(taskId, occurrenceStart, "complete") { TaskFormWriter.completed(it, completed, clock.now()) }
|
||||
override fun setCompletedInstance(taskId: Long, occurrenceStart: Instant, completed: Boolean) = tx {
|
||||
val close = { it: TaskEntity -> TaskFormWriter.completed(it, completed, clock.now()) }
|
||||
if (!moveSeries(taskId, occurrenceStart, closing = completed, close)) {
|
||||
writeInstance(taskId, occurrenceStart, "complete", close)
|
||||
}
|
||||
}
|
||||
|
||||
/** Same override model as [setCompletedInstance]. */
|
||||
override fun setCancelledInstance(taskId: Long, occurrenceStart: Instant, cancelled: Boolean) =
|
||||
writeInstance(taskId, occurrenceStart, "cancel") { TaskFormWriter.cancelled(it, cancelled, clock.now()) }
|
||||
/** Same model as [setCompletedInstance]. */
|
||||
override fun setCancelledInstance(taskId: Long, occurrenceStart: Instant, cancelled: Boolean) = tx {
|
||||
val close = { it: TaskEntity -> TaskFormWriter.cancelled(it, cancelled, clock.now()) }
|
||||
if (!moveSeries(taskId, occurrenceStart, closing = cancelled, close)) {
|
||||
writeInstance(taskId, occurrenceStart, "cancel", close)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Moves the series behind [taskId] past [occurrenceStart] when [closing] its
|
||||
* first occurrence, or back onto it when reopening the one it last moved past.
|
||||
* False when neither applies, so the caller writes an override instead.
|
||||
*/
|
||||
private fun moveSeries(taskId: Long, occurrenceStart: Instant, closing: Boolean, close: (TaskEntity) -> TaskEntity): Boolean {
|
||||
val row = tasks.entity(taskId) ?: return false
|
||||
val master = row.masterId?.let { tasks.entity(it) } ?: row
|
||||
if (master.recurrenceSpec() == null) return false
|
||||
val override = if (row.masterId != null) row else tasks.override(master.id, occurrenceStart)
|
||||
val anchor = override?.recurrenceId ?: occurrenceStart
|
||||
val closed = master.status == TaskStatus.COMPLETED || master.status == TaskStatus.CANCELLED
|
||||
if (closed) {
|
||||
// Only its last occurrence closes a series; reopening that one reopens it.
|
||||
if (closing || master.recurrenceAnchor?.let { sameSecond(it, anchor) } != true) return false
|
||||
tasks.update(close(master))
|
||||
override?.let { tasks.update(close(it)) }
|
||||
writes.onWritten(setOf(master.listId))
|
||||
return true
|
||||
}
|
||||
return if (closing) advance(master, anchor, override, close) else rewind(master, anchor)
|
||||
}
|
||||
|
||||
private fun advance(master: TaskEntity, anchor: Instant, override: TaskEntity?, close: (TaskEntity) -> TaskEntity): Boolean {
|
||||
val (first, next) = firstTwo(master) ?: return false
|
||||
if (!sameSecond(first, anchor)) return false
|
||||
if (next == null) {
|
||||
tasks.update(close(master))
|
||||
override?.let { tasks.update(close(it)) }
|
||||
writes.onWritten(setOf(master.listId))
|
||||
return true
|
||||
}
|
||||
val now = clock.now()
|
||||
val (start, due) = override?.let { it.dtstart to it.due } ?: occurrenceTimes(master, anchor)
|
||||
val copy = close(
|
||||
(override ?: master).copy(
|
||||
id = 0,
|
||||
uid = UUID.randomUUID().toString(),
|
||||
masterId = null,
|
||||
recurrenceId = null,
|
||||
href = null,
|
||||
etag = null,
|
||||
rrule = null,
|
||||
rdate = null,
|
||||
exdate = null,
|
||||
dtstart = start,
|
||||
due = due,
|
||||
createdAt = now,
|
||||
sequence = 0,
|
||||
isDeleted = false,
|
||||
unknownProperties = (seriesFree(master.unknownProperties) + occurrenceMark(master, anchor)).joinToString("\r\n"),
|
||||
),
|
||||
)
|
||||
val copyId = tasks.insert(copy)
|
||||
alarms.copyToTask(override?.id ?: master.id, copyId, sameResource = false)
|
||||
override?.let { tasks.delete(it.id) }
|
||||
// Keeps the anchor's own sub-second part, which expansion drops.
|
||||
tasks.update(shifted(master, master.recurrenceAnchor!! + (next - first), spent = 1, now))
|
||||
writes.onWritten(setOf(master.listId))
|
||||
return true
|
||||
}
|
||||
|
||||
private fun rewind(master: TaskEntity, anchor: Instant): Boolean {
|
||||
val anchorNow = master.recurrenceAnchor ?: return false
|
||||
if (anchor >= anchorNow || sameSecond(anchor, anchorNow)) return false
|
||||
val back = shifted(master, anchor, spent = -1, clock.now())
|
||||
val (first, next) = firstTwo(back) ?: return false
|
||||
if (!sameSecond(first, anchor) || next == null || !sameSecond(next, anchorNow)) return false
|
||||
tasks.update(back)
|
||||
val mark = occurrenceMark(master, anchor)
|
||||
tasks.allIn(master.listId)
|
||||
.filter { !it.isDeleted && it.unknownProperties?.lines()?.contains(mark) == true }
|
||||
.forEach { deleteTask(it.id) }
|
||||
writes.onWritten(setOf(master.listId))
|
||||
return true
|
||||
}
|
||||
|
||||
/** The series' first occurrence and the one after it (null when it has no other). */
|
||||
private fun firstTwo(master: TaskEntity): Pair<Instant, Instant?>? {
|
||||
val spec = master.recurrenceSpec() ?: return null
|
||||
val from = Instant.fromEpochSeconds(spec.anchor.epochSeconds)
|
||||
val occurrences = RecurrenceExpander.expand(spec, ExpansionWindow(from = from, until = from + SERIES_LOOKAHEAD, maxOccurrences = 2))
|
||||
val first = occurrences.firstOrNull() ?: return null
|
||||
return first to occurrences.getOrNull(1)
|
||||
}
|
||||
|
||||
/** [master] anchored on [at], its `COUNT` reduced by [spent] and the exceptions before [at] dropped. */
|
||||
private fun shifted(master: TaskEntity, at: Instant, spent: Int, now: Instant): TaskEntity {
|
||||
val (start, due) = occurrenceTimes(master, at)
|
||||
val zone = master.timezone ?: zone()
|
||||
return master.copy(
|
||||
dtstart = start,
|
||||
due = due,
|
||||
rrule = master.rrule?.let { TaskFormWriter.remainingCount(it, spent) },
|
||||
exdate = if (spent > 0) TaskFormWriter.valuesFrom(master.exdate, at, zone) else master.exdate,
|
||||
lastModified = now,
|
||||
isDirty = true,
|
||||
)
|
||||
}
|
||||
|
||||
private fun seriesFree(unknown: String?): List<String> =
|
||||
unknown?.lines()?.filterNot { line -> SERIES_LINES.any { line.startsWith(it, ignoreCase = true) } || line.startsWith(OCCURRENCE_MARK) }
|
||||
?.filter { it.isNotBlank() }.orEmpty()
|
||||
|
||||
/** Expansion runs in whole seconds; stored anchors may carry milliseconds. */
|
||||
private fun sameSecond(a: Instant, b: Instant) = a.epochSeconds == b.epochSeconds
|
||||
|
||||
/** Links a completed copy to the occurrence it was split off, so reopening it can move the series back. */
|
||||
private fun occurrenceMark(master: TaskEntity, anchor: Instant) = "$OCCURRENCE_MARK${master.uid};${anchor.toEpochMilliseconds()}"
|
||||
|
||||
/** Applies [write] to the override for [occurrenceStart], forking one if there is none yet. */
|
||||
private fun writeInstance(
|
||||
|
||||
Reference in new issue
Block a user