From d3fae3e69c5160c5ea247bb561ef2647a870a272 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Thu, 12 Nov 2026 15:57:55 +0100 Subject: [PATCH] Reopen sub-tasks when a routine moves on (#160) --- CHANGELOG.md | 5 ++ .../tasks/room/RoomTasksDataSourceTest.kt | 40 ++++++++++++++ .../data/tasks/AndroidTasksDataSource.kt | 41 +++++++++++++- .../agendula/data/tasks/SubtaskReset.kt | 39 +++++++++++++ .../data/tasks/room/RoomTasksDataSource.kt | 18 +++++- .../agendula/data/tasks/SubtaskResetTest.kt | 55 +++++++++++++++++++ 6 files changed, 196 insertions(+), 2 deletions(-) create mode 100644 app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/SubtaskReset.kt create mode 100644 app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/SubtaskResetTest.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index 26e197b..03f01d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ All notable changes to this project are documented here. The format follows ## [Unreleased] +### Fixed +- Completed sub-tasks of a repeating task open again when the task moves on to + its next date, as in tasks.org. Before, the next occurrence showed them + already ticked. Repeating sub-tasks keep their own schedule (#160). + ## [1.3.1] - 2026-11-09 ### Changed 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 4dcfedb..1feddab 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 @@ -700,6 +700,46 @@ class RoomTasksDataSourceTest { assertThat(db.tasks().overrides(id)).hasSize(1) } + @Test + fun movingARoutineOnReopensItsCompletedSubtasks() { + val at = now + val id = source.insertTask(form(title = "Routine")) + makeRecurring(id, at) + val done = source.insertTask(form(title = "Step").copy(parentId = id)) + val nested = source.insertTask(form(title = "Nested").copy(parentId = done)) + val cancelled = source.insertTask(form(title = "Skipped").copy(parentId = id)) + val daily = source.insertTask(form(title = "Daily part").copy(parentId = id)) + makeRecurring(daily, at, rule = "FREQ=DAILY") + source.setCompleted(done, completed = true) + source.setCompleted(nested, completed = true) + source.setCancelled(cancelled, cancelled = true) + source.setCompletedInstance(daily, at, completed = true) + + source.setCompletedInstance(id, at, completed = true) + + assertThat(db.tasks().entity(id)!!.dtstart).isEqualTo(at + 7.days) + assertThat(db.tasks().entity(done)!!.status).isEqualTo(TaskStatus.NEEDS_ACTION) + assertThat(db.tasks().entity(done)!!.completedAt).isNull() + assertThat(db.tasks().entity(nested)!!.status).isEqualTo(TaskStatus.NEEDS_ACTION) + assertThat(db.tasks().entity(cancelled)!!.status).isEqualTo(TaskStatus.CANCELLED) + // A recurring subtask has its own series, and its closed occurrence stays closed. + assertThat(db.tasks().overrides(daily).single().status).isEqualTo(TaskStatus.COMPLETED) + } + + @Test + fun aRoutineTickedOnItsLastOccurrenceLeavesItsSubtasksDone() { + val at = now + val id = source.insertTask(form(title = "Once more")) + makeRecurring(id, at, rule = "FREQ=WEEKLY;COUNT=1") + val step = source.insertTask(form(title = "Step").copy(parentId = id)) + source.setCompleted(step, completed = true) + + source.setCompletedInstance(id, at, completed = true) + + assertThat(db.tasks().entity(id)!!.status).isEqualTo(TaskStatus.COMPLETED) + assertThat(db.tasks().entity(step)!!.status).isEqualTo(TaskStatus.COMPLETED) + } + @Test fun aSeriesReshapedSinceIsNotMovedBack() { val at = now diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/AndroidTasksDataSource.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/AndroidTasksDataSource.kt index 67fa2b4..86bec5a 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/AndroidTasksDataSource.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/AndroidTasksDataSource.kt @@ -753,6 +753,8 @@ class AndroidTasksDataSource @Inject constructor( * Runs [write], and when it closed an occurrence the provider then detached * into a task of its own, remembers that copy, so the occurrence can still be * read and reopened, and how the series moved, so reopening can move it back. + * When the series moved on, its completed sub-tasks open again for the next + * occurrence (#160). */ private fun rememberingCopy(taskId: Long, occurrenceStart: Instant, closing: Boolean, write: () -> Int): Int { if (!closing) return write() @@ -761,17 +763,54 @@ class AndroidTasksDataSource @Inject constructor( val before = runCatching { seriesState(authority, taskId) }.getOrNull() return write().also { rows -> if (rows > 0) { + val after = runCatching { before?.let { seriesState(authority, taskId) }?.takeIf { it != before } }.getOrNull() runCatching { detachedSince(authority, lastId)?.let { copy -> - val after = before?.let { seriesState(authority, taskId) }?.takeIf { it != before } val tick = Tick(copy, taskId.takeIf { after != null }, before.takeIf { after != null }, after, System.currentTimeMillis()) ticks.put(ticks.key(authority, taskId, occurrenceStart), tick) } }.onFailure { Log.w(TAG, "Looking up the detached copy failed", it) } + if (after != null && after[Tasks.STATUS].isOpenStatus()) { + runCatching { reopenSubtasks(authority, taskId) }.onFailure { Log.w(TAG, "Reopening the sub-tasks failed", it) } + } } } } + private fun Any?.isOpenStatus(): Boolean { + val status = (this as? Number)?.toInt() ?: (this as? String)?.toIntOrNull() ?: return true + return status != TasksContract.STATUS_COMPLETED && status != TasksContract.STATUS_CANCELLED + } + + /** Reopens what [SubtaskReset] picks under [parentId]. */ + private fun reopenSubtasks(authority: String, parentId: Long) { + val reopen = SubtaskReset.toReopen(parentId) { id -> subtaskNodes(authority, id) } + if (reopen.isEmpty()) return + val values = TaskWriteMapper.completionValues(false, System.currentTimeMillis()).toContentValues() + val ops = reopen.mapTo(ArrayList()) { ContentProviderOperation.newUpdate(taskUri(authority, it)).withValues(values).build() } + resolver.applyBatch(authority, ops) + } + + /** [parentId]'s direct sub-tasks; a copy a tick left counts as part of its recurring sibling. */ + private fun subtaskNodes(authority: String, parentId: Long): List = resolver.query( + TasksContract.tasksUri(authority), + null, + "${Tasks.PARENT_ID} = ? AND (${Tasks.DELETED} IS NULL OR ${Tasks.DELETED} = 0)", + arrayOf(parentId.toString()), + null, + )?.use { c -> + val r = CursorColumnReader(c) + buildList { + while (c.moveToNext()) { + if (r.getLong(Tasks.ORIGINAL_INSTANCE_ID) != null) continue + val id = r.getLong(Tasks.ID)?.takeIf { it != parentId } ?: continue + val recurring = !r.getString(Tasks.RRULE).isNullOrBlank() || !r.getString(Tasks.RDATE).isNullOrBlank() || + ticks.forCopy(authority, id) != null + add(SubtaskNode(id, r.getString(Tasks.TITLE), recurring, r.getInt(Tasks.STATUS) == TasksContract.STATUS_COMPLETED)) + } + } + } ?: emptyList() + /** * Undoes the tick that left [taskId]'s occurrence at [occurrenceStart] — or, * with no occurrence, the tick that left the copy [taskId]: the series goes diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/SubtaskReset.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/SubtaskReset.kt new file mode 100644 index 0000000..16d2853 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/SubtaskReset.kt @@ -0,0 +1,39 @@ +package de.jeanlucmakiola.agendula.data.tasks + +/** A sub-task as [SubtaskReset] sees it. */ +internal class SubtaskNode( + val id: Long, + val title: String?, + val recurring: Boolean, + val completed: Boolean, +) + +/** + * What a routine's sub-tasks do when its series moves on to the next occurrence + * (#160): the completed ones open again, as in tasks.org, all the way down. + */ +internal object SubtaskReset { + + /** + * The completed sub-tasks under [parentId] to reopen. A recurring one has a + * series of its own and is left alone with everything under it, and so is a + * plain one titled like a recurring sibling: the completed copy a tick of that + * sibling left behind. + */ + fun toReopen(parentId: Long, children: (Long) -> List): List { + val seen = mutableSetOf(parentId) + val reopen = mutableListOf() + fun walk(id: Long) { + val subtasks = children(id) + val recurringTitles = subtasks.filter { it.recurring }.mapNotNull { it.title?.trim() }.toSet() + subtasks + .filter { !it.recurring && it.title?.trim() !in recurringTitles && seen.add(it.id) } + .forEach { + if (it.completed) reopen += it.id + walk(it.id) + } + } + walk(parentId) + return reopen + } +} 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 8990084..800e119 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 @@ -1,6 +1,8 @@ package de.jeanlucmakiola.agendula.data.tasks.room import androidx.room.InvalidationTracker +import de.jeanlucmakiola.agendula.data.tasks.SubtaskNode +import de.jeanlucmakiola.agendula.data.tasks.SubtaskReset import de.jeanlucmakiola.agendula.data.tasks.TaskQuery import de.jeanlucmakiola.agendula.domain.TaskReminder import de.jeanlucmakiola.agendula.domain.TaskReminderOrder @@ -638,6 +640,7 @@ class RoomTasksDataSource @Inject constructor( isDirty = true, ), ) + val reopenedLists = reopenSubtasks(master.id, now) val copyId = if (override != null) { detach(override, now, close) } else { @@ -666,10 +669,23 @@ class RoomTasksDataSource @Inject constructor( ), ) advanced.insertDetached(detached.map { (at, id) -> AdvancedDetachedEntity(copyId = copyId, anchor = at, taskId = id) }) - writes.onWritten(setOf(master.listId)) + writes.onWritten(setOf(master.listId) + reopenedLists) return true } + /** Reopens what [SubtaskReset] picks under [parentId] for the series' next occurrence; the lists it wrote to. */ + private fun reopenSubtasks(parentId: Long, now: Instant): Set { + val reopen = SubtaskReset.toReopen(parentId) { id -> + tasks.subtasks(id).map { + SubtaskNode(it.task.id, it.task.title, it.task.isRecurring, it.task.status == TaskStatus.COMPLETED) + } + } + return reopen.mapNotNull(tasks::entity).map { + tasks.update(TaskFormWriter.completed(it, completed = false, now = now)) + it.listId + }.toSet() + } + /** 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), diff --git a/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/SubtaskResetTest.kt b/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/SubtaskResetTest.kt new file mode 100644 index 0000000..09cb351 --- /dev/null +++ b/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/SubtaskResetTest.kt @@ -0,0 +1,55 @@ +package de.jeanlucmakiola.agendula.data.tasks + +import com.google.common.truth.Truth.assertThat +import org.junit.jupiter.api.Test + +class SubtaskResetTest { + + private fun node(id: Long, title: String = "Step $id", recurring: Boolean = false, completed: Boolean = true) = + SubtaskNode(id, title, recurring, completed) + + private fun reopen(vararg tree: Pair>): List { + val children = tree.toMap() + return SubtaskReset.toReopen(1L) { children[it].orEmpty() } + } + + @Test + fun `completed sub-tasks of a routine reopen, open ones are left as they are (#160)`() { + assertThat(reopen(1L to listOf(node(2), node(3, completed = false), node(4)))).containsExactly(2L, 4L) + } + + @Test + fun `nested sub-tasks reopen too, also under an open one`() { + val reopened = reopen( + 1L to listOf(node(2, completed = false)), + 2L to listOf(node(3)), + 3L to listOf(node(4)), + ) + assertThat(reopened).containsExactly(3L, 4L) + } + + @Test + fun `a recurring sub-task and everything under it are left alone`() { + val reopened = reopen( + 1L to listOf(node(2, recurring = true), node(3)), + 2L to listOf(node(4)), + ) + assertThat(reopened).containsExactly(3L) + } + + @Test + fun `the completed copy a recurring sibling left is left alone`() { + val reopened = reopen(1L to listOf(node(2, "Water plants", recurring = true), node(3, " Water plants"), node(4, "Sweep"))) + assertThat(reopened).containsExactly(4L) + } + + @Test + fun `a cycle or the parent listed under itself is not walked twice`() { + val reopened = reopen( + 1L to listOf(node(1), node(2)), + 2L to listOf(node(3)), + 3L to listOf(node(2)), + ) + assertThat(reopened).containsExactly(2L, 3L) + } +}