diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a65170..be3d2c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ All notable changes to this project are documented here. The format follows ## [Unreleased] +### Fixed +- With tasks.org as the storage provider, recurring tasks are recognised as + recurring. Before, editing one removed its recurrence and ticking it off + closed the whole series. With tasks.org and OpenTasks alike, the editor now + shows the task's repeat rule instead of none (#135). + ## [1.3.0] - 2026-11-02 ### Added 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 958a0d5..711161f 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 @@ -111,19 +111,47 @@ class AndroidTasksDataSource @Inject constructor( val uri = TasksContract.instancesUri(authority()) // projection = null (all columns): the instances view differs across // provider versions (e.g. tasks.org's bundled OpenTasks has no - // `is_recurring` / `distance_from_current`). The by-name mapper reads - // whatever is present and nulls the rest; the repository re-sorts, so we - // don't depend on a provider sort column either. + // `is_recurring`). The by-name mapper reads whatever is present and + // nulls the rest; the repository re-sorts, so we don't depend on a + // provider sort column either. val tasks = resolver.query(uri, null, selection, args, null) ?.use { c -> val reader = CursorColumnReader(c) - buildList { while (c.moveToNext()) add(TaskMapper.task(reader)) } + val taskIds = buildSet { + while (c.moveToNext()) reader.getLong(Instances.TASK_ID)?.let(::add) + } + // The view hides the rule (`null AS rrule`), so it is read from + // the tasks table and handed to the mapper. + val rules = seriesRules(taskIds) + c.moveToPosition(-1) + buildList { + while (c.moveToNext()) add(TaskMapper.task(reader, reader.getLong(Instances.TASK_ID)?.let(rules::get))) + } } ?: emptyList() if (tasks.isEmpty()) return tasks val tags = categories(tasks.map { it.taskId }) return tasks.map { task -> tags[task.taskId]?.let { task.copy(tags = it) } ?: task } } + /** The recurrence of each of [taskIds] that is a series, keyed by task id. */ + private fun seriesRules(taskIds: Collection): Map = taskIds.chunked(MAX_SQL_ARGS).flatMap { chunk -> + resolver.query( + TasksContract.tasksUri(authority()), + arrayOf(Tasks.ID, Tasks.RRULE, Tasks.RDATE), + "${Tasks.ID} IN (${chunk.joinToString(",") { "?" }})", + chunk.map(Long::toString).toTypedArray(), + null, + )?.use { c -> + val reader = CursorColumnReader(c) + buildList { + while (c.moveToNext()) { + val id = reader.getLong(Tasks.ID) ?: continue + TaskMapper.seriesRule(reader)?.let { add(id to it) } + } + } + } ?: emptyList() + }.toMap() + override fun exportTasks(listId: Long): List { // projection = null for the same reason queryInstances uses it: the tasks // table's shape varies across provider versions, and the by-name mapper diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TaskMapper.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TaskMapper.kt index e5d810f..e6b3063 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TaskMapper.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TaskMapper.kt @@ -13,17 +13,19 @@ import kotlin.time.Instant /** Maps a provider row (via [ColumnReader]) to a domain model. Pure + testable. */ object TaskMapper { - fun task(r: ColumnReader): Task { + /** + * Maps an `instances` row. [rule] is the series' rule set as stored on its + * `tasks` row: the instances view masks `rrule`/`rdate` with `NULL` on every + * provider version, so the row alone cannot tell a series from a one-off on + * tasks.org's bundled provider (DB 22), which also lacks `is_recurring`. + */ + fun task(r: ColumnReader, rule: SeriesRule? = null): Task { fun instant(name: String): Instant? = r.getLong(name)?.let { Instant.fromEpochMilliseconds(it) } val rowId = r.getLong(Tasks.ID) ?: 0L - // Derived from the rule columns rather than the `is_recurring` column - // alone: that column only exists from OpenTasks 1.4.0 (DB 23) and is - // absent on tasks.org's bundled provider (DB 22), where reading it - // would silently report every recurring task as one-off — and route - // its edits onto the series anchor. - val recurring = r.getString(Tasks.RRULE) != null || + val recurring = rule != null || + r.getString(Tasks.RRULE) != null || r.getString(Tasks.RDATE) != null || r.getBoolean(Instances.IS_RECURRING) return Task( @@ -52,10 +54,17 @@ object TaskMapper { created = instant(Tasks.CREATED), lastModified = instant(Tasks.LAST_MODIFIED), seriesId = if (recurring) r.getLong(Instances.TASK_ID) ?: rowId else null, - recurrenceRule = r.getString(Tasks.RRULE), + recurrenceRule = rule?.rrule ?: r.getString(Tasks.RRULE), ) } + /** The rule columns of a `tasks` row, or null when it does not recur. */ + fun seriesRule(r: ColumnReader): SeriesRule? { + val rrule = r.getString(Tasks.RRULE)?.ifBlank { null } + val rdate = r.getString(Tasks.RDATE)?.ifBlank { null } + return if (rrule == null && rdate == null) null else SeriesRule(rrule, rdate) + } + /** * The occurrence's `RECURRENCE-ID` anchor. * @@ -119,3 +128,6 @@ object TaskMapper { owner = r.getString(Lists.OWNER), ) } + +/** A series' recurrence as stored on its master row. */ +data class SeriesRule(val rrule: String?, val rdate: String?) diff --git a/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/TaskMapperTest.kt b/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/TaskMapperTest.kt index 781b02a..e73f5ce 100644 --- a/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/TaskMapperTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/TaskMapperTest.kt @@ -7,6 +7,7 @@ import de.jeanlucmakiola.agendula.data.tasks.TasksContract.Tasks import de.jeanlucmakiola.agendula.domain.Priority import de.jeanlucmakiola.agendula.domain.TaskStatus import org.junit.jupiter.api.Test +import kotlin.time.Instant class TaskMapperTest { @@ -85,15 +86,52 @@ class TaskMapperTest { @Test fun `recurrence is detected from rrule when is_recurring is absent`() { - // tasks.org's bundled provider is DB 22 and has no `is_recurring` column; - // reading it alone would report the series as one-off and send its edits - // to the master row, re-anchoring the whole thing. val task = TaskMapper.task( MapColumnReader(mapOf(Tasks.ID to 1L, Tasks.RRULE to "FREQ=WEEKLY;BYDAY=MO")), ) assertThat(task.isRecurring).isTrue() } + @Test + fun `a series is detected from its master rule when the view masks it (#135)`() { + // tasks.org's bundled provider (DB 22): `null AS rrule` in the instances + // view and no `is_recurring`, so only the tasks row knows it recurs. + val task = TaskMapper.task( + MapColumnReader( + mapOf( + Tasks.ID to 90L, + Instances.TASK_ID to 9L, + Instances.INSTANCE_START to 1_000L, + Instances.INSTANCE_ORIGINAL_TIME to 1_000L, + ), + ), + SeriesRule(rrule = "FREQ=DAILY", rdate = null), + ) + assertThat(task.isRecurring).isTrue() + assertThat(task.seriesId).isEqualTo(9L) + assertThat(task.occurrenceStart).isEqualTo(Instant.fromEpochMilliseconds(1_000L)) + assertThat(task.isOccurrence).isTrue() + assertThat(task.recurrenceRule).isEqualTo("FREQ=DAILY") + } + + @Test + fun `the master rule fills the editor even where is_recurring exists`() { + // OpenTasks DB 23 flags the row but masks the rule all the same; without + // it the editor opens empty and saving the series drops the RRULE. + val task = TaskMapper.task( + MapColumnReader(mapOf(Tasks.ID to 90L, Instances.TASK_ID to 9L, Instances.IS_RECURRING to 1)), + SeriesRule(rrule = "FREQ=WEEKLY;BYDAY=MO", rdate = null), + ) + assertThat(task.recurrenceRule).isEqualTo("FREQ=WEEKLY;BYDAY=MO") + } + + @Test + fun `a tasks row without a rule is no series`() { + assertThat(TaskMapper.seriesRule(MapColumnReader(mapOf(Tasks.ID to 1L, Tasks.RRULE to "")))).isNull() + assertThat(TaskMapper.seriesRule(MapColumnReader(mapOf(Tasks.ID to 1L, Tasks.RDATE to "20260720T090000Z")))) + .isEqualTo(SeriesRule(rrule = null, rdate = "20260720T090000Z")) + } + @Test fun `recurrence is detected from rdate alone`() { val task = TaskMapper.task(