Reopen sub-tasks when a routine moves on (#160)
This commit is contained in:
1 parent
3e34c60577
commit
d3fae3e69c
6 files changed
+196
-2
No files matched your search
@@ -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
|
||||
|
||||
+40
@@ -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
|
||||
|
||||
@@ -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<SubtaskNode> = 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
|
||||
|
||||
@@ -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<SubtaskNode>): List<Long> {
|
||||
val seen = mutableSetOf(parentId)
|
||||
val reopen = mutableListOf<Long>()
|
||||
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
|
||||
}
|
||||
}
|
||||
+17
-1
@@ -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<Long> {
|
||||
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),
|
||||
|
||||
@@ -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<Long, List<SubtaskNode>>): List<Long> {
|
||||
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)
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user