diff --git a/CHANGELOG.md b/CHANGELOG.md index 03f01d0..3dd6dfc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,9 +8,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). +- Sub-tasks of a repeating task start over when ticking off its current + occurrence moves it on to its next date, as in tasks.org: completed and + half-done ones open again. Before, the next occurrence showed them already + ticked. Unticking gives them back their state, and repeating sub-tasks keep + their own schedule (#160). ## [1.3.1] - 2026-11-09 diff --git a/app/schemas/de.jeanlucmakiola.agendula.data.tasks.room.TasksDatabase/6.json b/app/schemas/de.jeanlucmakiola.agendula.data.tasks.room.TasksDatabase/6.json new file mode 100644 index 0000000..e120730 --- /dev/null +++ b/app/schemas/de.jeanlucmakiola.agendula.data.tasks.room.TasksDatabase/6.json @@ -0,0 +1,775 @@ +{ + "formatVersion": 1, + "database": { + "version": 6, + "identityHash": "6c8058a22084f0af602bc2f23c949b3c", + "entities": [ + { + "tableName": "accounts", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `display_name` TEXT NOT NULL, `principal_url` TEXT, `home_set_url` TEXT, `username` TEXT, `last_sync_at` INTEGER, `last_sync_error` TEXT, `label` TEXT)", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "displayName", + "columnName": "display_name", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "principalUrl", + "columnName": "principal_url", + "affinity": "TEXT" + }, + { + "fieldPath": "homeSetUrl", + "columnName": "home_set_url", + "affinity": "TEXT" + }, + { + "fieldPath": "username", + "columnName": "username", + "affinity": "TEXT" + }, + { + "fieldPath": "lastSyncAt", + "columnName": "last_sync_at", + "affinity": "INTEGER" + }, + { + "fieldPath": "lastSyncError", + "columnName": "last_sync_error", + "affinity": "TEXT" + }, + { + "fieldPath": "label", + "columnName": "label", + "affinity": "TEXT" + } + ], + "primaryKey": { + "autoGenerate": true, + "columnNames": [ + "id" + ] + } + }, + { + "tableName": "task_lists", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `name` TEXT NOT NULL, `color` INTEGER NOT NULL, `account_id` INTEGER, `is_visible` INTEGER NOT NULL DEFAULT 1, `is_synced` INTEGER NOT NULL DEFAULT 1, `owner` TEXT, `is_read_only` INTEGER NOT NULL DEFAULT 0, `sort_order` INTEGER NOT NULL DEFAULT 0, `href` TEXT, `ctag` TEXT, `sync_token` TEXT, `is_dirty` INTEGER NOT NULL DEFAULT 0, FOREIGN KEY(`account_id`) REFERENCES `accounts`(`id`) ON UPDATE NO ACTION ON DELETE SET NULL )", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "name", + "columnName": "name", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "color", + "columnName": "color", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "accountId", + "columnName": "account_id", + "affinity": "INTEGER" + }, + { + "fieldPath": "isVisible", + "columnName": "is_visible", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "1" + }, + { + "fieldPath": "isSynced", + "columnName": "is_synced", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "1" + }, + { + "fieldPath": "owner", + "columnName": "owner", + "affinity": "TEXT" + }, + { + "fieldPath": "isReadOnly", + "columnName": "is_read_only", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + }, + { + "fieldPath": "sortOrder", + "columnName": "sort_order", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + }, + { + "fieldPath": "href", + "columnName": "href", + "affinity": "TEXT" + }, + { + "fieldPath": "ctag", + "columnName": "ctag", + "affinity": "TEXT" + }, + { + "fieldPath": "syncToken", + "columnName": "sync_token", + "affinity": "TEXT" + }, + { + "fieldPath": "isDirty", + "columnName": "is_dirty", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + } + ], + "primaryKey": { + "autoGenerate": true, + "columnNames": [ + "id" + ] + }, + "indices": [ + { + "name": "index_task_lists_account_id", + "unique": false, + "columnNames": [ + "account_id" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_task_lists_account_id` ON `${TABLE_NAME}` (`account_id`)" + } + ], + "foreignKeys": [ + { + "table": "accounts", + "onDelete": "SET NULL", + "onUpdate": "NO ACTION", + "columns": [ + "account_id" + ], + "referencedColumns": [ + "id" + ] + } + ] + }, + { + "tableName": "tasks", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `list_id` INTEGER NOT NULL, `uid` TEXT NOT NULL, `href` TEXT, `etag` TEXT, `title` TEXT, `description` TEXT, `location` TEXT, `url` TEXT, `color` INTEGER, `categories` TEXT NOT NULL DEFAULT '', `status` INTEGER NOT NULL DEFAULT 0, `percent_complete` INTEGER, `completed_at` INTEGER, `priority` INTEGER NOT NULL DEFAULT 0, `classification` INTEGER, `dtstart` INTEGER, `due` INTEGER, `duration` TEXT, `is_all_day` INTEGER NOT NULL DEFAULT 0, `timezone` TEXT, `rrule` TEXT, `rdate` TEXT, `exdate` TEXT, `recurrence_id` INTEGER, `master_id` INTEGER, `parent_id` INTEGER, `sort_order` INTEGER NOT NULL DEFAULT 0, `created_at` INTEGER, `last_modified` INTEGER, `sequence` INTEGER NOT NULL DEFAULT 0, `is_dirty` INTEGER NOT NULL DEFAULT 0, `is_deleted` INTEGER NOT NULL DEFAULT 0, `unknown_properties` TEXT, `reminders_off` INTEGER NOT NULL DEFAULT 0, FOREIGN KEY(`list_id`) REFERENCES `task_lists`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE , FOREIGN KEY(`master_id`) REFERENCES `tasks`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE , FOREIGN KEY(`parent_id`) REFERENCES `tasks`(`id`) ON UPDATE NO ACTION ON DELETE SET NULL )", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "listId", + "columnName": "list_id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "uid", + "columnName": "uid", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "href", + "columnName": "href", + "affinity": "TEXT" + }, + { + "fieldPath": "etag", + "columnName": "etag", + "affinity": "TEXT" + }, + { + "fieldPath": "title", + "columnName": "title", + "affinity": "TEXT" + }, + { + "fieldPath": "description", + "columnName": "description", + "affinity": "TEXT" + }, + { + "fieldPath": "location", + "columnName": "location", + "affinity": "TEXT" + }, + { + "fieldPath": "url", + "columnName": "url", + "affinity": "TEXT" + }, + { + "fieldPath": "color", + "columnName": "color", + "affinity": "INTEGER" + }, + { + "fieldPath": "categories", + "columnName": "categories", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "''" + }, + { + "fieldPath": "status", + "columnName": "status", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + }, + { + "fieldPath": "percentComplete", + "columnName": "percent_complete", + "affinity": "INTEGER" + }, + { + "fieldPath": "completedAt", + "columnName": "completed_at", + "affinity": "INTEGER" + }, + { + "fieldPath": "priority", + "columnName": "priority", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + }, + { + "fieldPath": "classification", + "columnName": "classification", + "affinity": "INTEGER" + }, + { + "fieldPath": "dtstart", + "columnName": "dtstart", + "affinity": "INTEGER" + }, + { + "fieldPath": "due", + "columnName": "due", + "affinity": "INTEGER" + }, + { + "fieldPath": "duration", + "columnName": "duration", + "affinity": "TEXT" + }, + { + "fieldPath": "isAllDay", + "columnName": "is_all_day", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + }, + { + "fieldPath": "timezone", + "columnName": "timezone", + "affinity": "TEXT" + }, + { + "fieldPath": "rrule", + "columnName": "rrule", + "affinity": "TEXT" + }, + { + "fieldPath": "rdate", + "columnName": "rdate", + "affinity": "TEXT" + }, + { + "fieldPath": "exdate", + "columnName": "exdate", + "affinity": "TEXT" + }, + { + "fieldPath": "recurrenceId", + "columnName": "recurrence_id", + "affinity": "INTEGER" + }, + { + "fieldPath": "masterId", + "columnName": "master_id", + "affinity": "INTEGER" + }, + { + "fieldPath": "parentId", + "columnName": "parent_id", + "affinity": "INTEGER" + }, + { + "fieldPath": "sortOrder", + "columnName": "sort_order", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + }, + { + "fieldPath": "createdAt", + "columnName": "created_at", + "affinity": "INTEGER" + }, + { + "fieldPath": "lastModified", + "columnName": "last_modified", + "affinity": "INTEGER" + }, + { + "fieldPath": "sequence", + "columnName": "sequence", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + }, + { + "fieldPath": "isDirty", + "columnName": "is_dirty", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + }, + { + "fieldPath": "isDeleted", + "columnName": "is_deleted", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + }, + { + "fieldPath": "unknownProperties", + "columnName": "unknown_properties", + "affinity": "TEXT" + }, + { + "fieldPath": "remindersOff", + "columnName": "reminders_off", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + } + ], + "primaryKey": { + "autoGenerate": true, + "columnNames": [ + "id" + ] + }, + "indices": [ + { + "name": "index_tasks_list_id_is_deleted", + "unique": false, + "columnNames": [ + "list_id", + "is_deleted" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_tasks_list_id_is_deleted` ON `${TABLE_NAME}` (`list_id`, `is_deleted`)" + }, + { + "name": "index_tasks_parent_id", + "unique": false, + "columnNames": [ + "parent_id" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_tasks_parent_id` ON `${TABLE_NAME}` (`parent_id`)" + }, + { + "name": "index_tasks_master_id_recurrence_id", + "unique": false, + "columnNames": [ + "master_id", + "recurrence_id" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_tasks_master_id_recurrence_id` ON `${TABLE_NAME}` (`master_id`, `recurrence_id`)" + }, + { + "name": "index_tasks_is_dirty", + "unique": false, + "columnNames": [ + "is_dirty" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_tasks_is_dirty` ON `${TABLE_NAME}` (`is_dirty`)" + }, + { + "name": "index_tasks_list_id_uid_recurrence_id", + "unique": true, + "columnNames": [ + "list_id", + "uid", + "recurrence_id" + ], + "orders": [], + "createSql": "CREATE UNIQUE INDEX IF NOT EXISTS `index_tasks_list_id_uid_recurrence_id` ON `${TABLE_NAME}` (`list_id`, `uid`, `recurrence_id`)" + } + ], + "foreignKeys": [ + { + "table": "task_lists", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "list_id" + ], + "referencedColumns": [ + "id" + ] + }, + { + "table": "tasks", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "master_id" + ], + "referencedColumns": [ + "id" + ] + }, + { + "table": "tasks", + "onDelete": "SET NULL", + "onUpdate": "NO ACTION", + "columns": [ + "parent_id" + ], + "referencedColumns": [ + "id" + ] + } + ] + }, + { + "tableName": "task_alarms", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `task_id` INTEGER NOT NULL, `minutes_before` INTEGER NOT NULL, `reference` TEXT NOT NULL DEFAULT 'DUE', `message` TEXT, `uid` TEXT NOT NULL DEFAULT '', `raw` TEXT, FOREIGN KEY(`task_id`) REFERENCES `tasks`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "taskId", + "columnName": "task_id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "minutesBefore", + "columnName": "minutes_before", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "reference", + "columnName": "reference", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "'DUE'" + }, + { + "fieldPath": "message", + "columnName": "message", + "affinity": "TEXT" + }, + { + "fieldPath": "uid", + "columnName": "uid", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "''" + }, + { + "fieldPath": "raw", + "columnName": "raw", + "affinity": "TEXT" + } + ], + "primaryKey": { + "autoGenerate": true, + "columnNames": [ + "id" + ] + }, + "indices": [ + { + "name": "index_task_alarms_task_id", + "unique": false, + "columnNames": [ + "task_id" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_task_alarms_task_id` ON `${TABLE_NAME}` (`task_id`)" + } + ], + "foreignKeys": [ + { + "table": "tasks", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "task_id" + ], + "referencedColumns": [ + "id" + ] + } + ] + }, + { + "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, `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, `reset_subtasks` TEXT NOT NULL DEFAULT '', 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", + "columnName": "copy_id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "masterId", + "columnName": "master_id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "anchor", + "columnName": "anchor", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "advancedTo", + "columnName": "advanced_to", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "priorDtstart", + "columnName": "prior_dtstart", + "affinity": "INTEGER" + }, + { + "fieldPath": "priorDue", + "columnName": "prior_due", + "affinity": "INTEGER" + }, + { + "fieldPath": "priorStatus", + "columnName": "prior_status", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "priorPercentComplete", + "columnName": "prior_percent_complete", + "affinity": "INTEGER" + }, + { + "fieldPath": "priorCompletedAt", + "columnName": "prior_completed_at", + "affinity": "INTEGER" + }, + { + "fieldPath": "spent", + "columnName": "spent", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "ruleAfter", + "columnName": "rule_after", + "affinity": "TEXT" + }, + { + "fieldPath": "droppedExdates", + "columnName": "dropped_exdates", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "''" + }, + { + "fieldPath": "fromOverride", + "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" + }, + { + "fieldPath": "resetSubtasks", + "columnName": "reset_subtasks", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "''" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "copy_id" + ] + }, + "indices": [ + { + "name": "index_advanced_occurrences_master_id_anchor", + "unique": false, + "columnNames": [ + "master_id", + "anchor" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_advanced_occurrences_master_id_anchor` ON `${TABLE_NAME}` (`master_id`, `anchor`)" + } + ], + "foreignKeys": [ + { + "table": "tasks", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "copy_id" + ], + "referencedColumns": [ + "id" + ] + }, + { + "table": "tasks", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "master_id" + ], + "referencedColumns": [ + "id" + ] + } + ] + }, + { + "tableName": "advanced_detached", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `copy_id` INTEGER NOT NULL, `anchor` INTEGER NOT NULL, `task_id` INTEGER, FOREIGN KEY(`copy_id`) REFERENCES `advanced_occurrences`(`copy_id`) ON UPDATE NO ACTION ON DELETE CASCADE , FOREIGN KEY(`task_id`) REFERENCES `tasks`(`id`) ON UPDATE NO ACTION ON DELETE SET NULL )", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "copyId", + "columnName": "copy_id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "anchor", + "columnName": "anchor", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "taskId", + "columnName": "task_id", + "affinity": "INTEGER" + } + ], + "primaryKey": { + "autoGenerate": true, + "columnNames": [ + "id" + ] + }, + "indices": [ + { + "name": "index_advanced_detached_copy_id", + "unique": false, + "columnNames": [ + "copy_id" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_advanced_detached_copy_id` ON `${TABLE_NAME}` (`copy_id`)" + }, + { + "name": "index_advanced_detached_task_id", + "unique": false, + "columnNames": [ + "task_id" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_advanced_detached_task_id` ON `${TABLE_NAME}` (`task_id`)" + } + ], + "foreignKeys": [ + { + "table": "advanced_occurrences", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "copy_id" + ], + "referencedColumns": [ + "copy_id" + ] + }, + { + "table": "tasks", + "onDelete": "SET NULL", + "onUpdate": "NO ACTION", + "columns": [ + "task_id" + ], + "referencedColumns": [ + "id" + ] + } + ] + } + ], + "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, '6c8058a22084f0af602bc2f23c949b3c')" + ] + } +} \ 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 1feddab..0a61e93 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 @@ -726,6 +726,43 @@ class RoomTasksDataSourceTest { assertThat(db.tasks().overrides(daily).single().status).isEqualTo(TaskStatus.COMPLETED) } + @Test + fun aHalfDoneSubtaskStartsOverToo() { + val at = now + val id = source.insertTask(form(title = "Routine")) + makeRecurring(id, at) + val step = source.insertTask(form(title = "Step", percentComplete = 40).copy(parentId = id)) + db.tasks().update(db.tasks().entity(step)!!.copy(status = TaskStatus.IN_PROCESS, percentComplete = 40)) + + source.setCompletedInstance(id, at, completed = true) + + assertThat(db.tasks().entity(step)!!.status).isEqualTo(TaskStatus.NEEDS_ACTION) + assertThat(db.tasks().entity(step)!!.percentComplete).isNull() + } + + @Test + fun movingARoutineBackRestoresTheSubtasksItReset() { + val at = now + val id = source.insertTask(form(title = "Routine")) + makeRecurring(id, at) + val done = source.insertTask(form(title = "Step").copy(parentId = id)) + val touched = source.insertTask(form(title = "Touched since").copy(parentId = id)) + source.setCompleted(done, completed = true) + source.setCompleted(touched, completed = true) + val completedAt = db.tasks().entity(done)!!.completedAt + source.setCompletedInstance(id, at, completed = true) + db.tasks().update(db.tasks().entity(touched)!!.copy(percentComplete = 30)) + + source.setCompletedInstance(id, at, completed = false) + + assertThat(db.tasks().entity(id)!!.dtstart).isEqualTo(at) + assertThat(db.tasks().entity(done)!!.status).isEqualTo(TaskStatus.COMPLETED) + assertThat(db.tasks().entity(done)!!.completedAt).isEqualTo(completedAt) + // Changed after the reset: the user's state wins. + assertThat(db.tasks().entity(touched)!!.status).isEqualTo(TaskStatus.NEEDS_ACTION) + assertThat(db.tasks().entity(touched)!!.percentComplete).isEqualTo(30) + } + @Test fun aRoutineTickedOnItsLastOccurrenceLeavesItsSubtasksDone() { val at = now diff --git a/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/room/TasksDatabaseMigrationTest.kt b/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/room/TasksDatabaseMigrationTest.kt index ed7a030..0cd200c 100644 --- a/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/room/TasksDatabaseMigrationTest.kt +++ b/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/room/TasksDatabaseMigrationTest.kt @@ -174,6 +174,25 @@ class TasksDatabaseMigrationTest { } } + @Test + fun addsAnEmptyResetSubtasksRecord() { + helper.createDatabase(TEST_DB, 5).use { db -> + db.execSQL("INSERT INTO task_lists (id, name, color) VALUES (1, 'Personal', 0)") + db.execSQL("INSERT INTO tasks (id, list_id, uid) VALUES (1, 1, 'a'), (2, 1, 'b')") + db.execSQL( + "INSERT INTO advanced_occurrences (copy_id, master_id, anchor, advanced_to, prior_status, spent, from_override) " + + "VALUES (2, 1, 0, 1, 0, 1, 0)", + ) + } + + helper.runMigrationsAndValidate(TEST_DB, 6, true, TasksDatabase.MIGRATION_5_6).use { db -> + db.query("SELECT reset_subtasks FROM advanced_occurrences").use { c -> + assertThat(c.moveToFirst()).isTrue() + assertThat(c.getString(0)).isEmpty() + } + } + } + private companion object { const val TEST_DB = "migration-test.db" } diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/di/DataModule.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/di/DataModule.kt index 26a4a9e..ab88919 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/di/DataModule.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/di/DataModule.kt @@ -89,7 +89,7 @@ object DataProvideModule { // backed-up `.db` does not. The backup rules carry all three files and // the app checkpoints on ON_STOP. .setJournalMode(RoomDatabase.JournalMode.WRITE_AHEAD_LOGGING) - .addMigrations(TasksDatabase.MIGRATION_1_2, TasksDatabase.MIGRATION_2_3, TasksDatabase.MIGRATION_3_4, TasksDatabase.MIGRATION_4_5) + .addMigrations(TasksDatabase.MIGRATION_1_2, TasksDatabase.MIGRATION_2_3, TasksDatabase.MIGRATION_3_4, TasksDatabase.MIGRATION_4_5, TasksDatabase.MIGRATION_5_6) .build() /** 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 86bec5a..c654450 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 @@ -25,6 +25,8 @@ import de.jeanlucmakiola.agendula.domain.TaskForm import de.jeanlucmakiola.agendula.domain.TaskList import de.jeanlucmakiola.agendula.domain.export.ExportTask import de.jeanlucmakiola.agendula.domain.normalizeTags +import de.jeanlucmakiola.agendula.domain.statusFromInt +import de.jeanlucmakiola.agendula.domain.toInt import de.jeanlucmakiola.agendula.domain.recurrence.ExpansionWindow import de.jeanlucmakiola.agendula.domain.recurrence.RecurrenceExpander import de.jeanlucmakiola.agendula.domain.recurrence.RecurrenceSpec @@ -135,21 +137,8 @@ class AndroidTasksDataSource @Inject constructor( * the window still counts. All columns and a lenient `_deleted`, as in * [exportTasks], since the table's shape varies across providers. */ - override fun subtaskIds(parentTaskId: Long): List = resolver.query( - TasksContract.tasksUri(authority()), - null, - "${Tasks.PARENT_ID} = ? AND (${Tasks.DELETED} IS NULL OR ${Tasks.DELETED} = 0)", - arrayOf(parentTaskId.toString()), - null, - )?.use { c -> - val r = CursorColumnReader(c) - buildList { - while (c.moveToNext()) { - if (r.getLong(Tasks.ORIGINAL_INSTANCE_ID) != null) continue - r.getLong(Tasks.ID)?.takeIf { it != parentTaskId }?.let(::add) - } - } - } ?: emptyList() + override fun subtaskIds(parentTaskId: Long): List = + childRows(authority(), parentTaskId, projection = null) { it.getLong(Tasks.ID) } /** * All columns and no sort: the instances view differs across provider @@ -753,8 +742,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). + * When the series moved on, its sub-tasks start over for the next occurrence, + * and the tick remembers how they were (#160). */ private fun rememberingCopy(taskId: Long, occurrenceStart: Instant, closing: Boolean, write: () -> Int): Int { if (!closing) return write() @@ -763,53 +752,93 @@ 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() + val after = before?.let { + runCatching { seriesState(authority, taskId) } + .onFailure { Log.w(TAG, "Reading the series after the tick failed", it) } + .getOrNull()?.takeIf { it != before } + } + val reset = if (after != null && SubtaskReset.movedOn(before, after, runCatching { seriesSpec(taskId) }.getOrNull())) { + runCatching { resetSubtasks(authority, taskId) }.onFailure { Log.w(TAG, "Resetting the sub-tasks failed", it) }.getOrNull() + } else { + null + } runCatching { detachedSince(authority, lastId)?.let { copy -> - val tick = Tick(copy, taskId.takeIf { after != null }, before.takeIf { after != null }, after, System.currentTimeMillis()) + val tick = Tick( + copy, taskId.takeIf { after != null }, before.takeIf { after != null }, after, System.currentTimeMillis(), + reset = reset?.let(SubtaskReset::encode), + ) 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) } + } + } + } + + /** Resets what [SubtaskReset] picks under [parentId]. */ + private fun resetSubtasks(authority: String, parentId: Long): List { + val copies = ticks.copies(authority) + val reset = SubtaskReset.toReset(parentId) { id -> + childRows(authority, id, SUBTASK_STATE) { r -> + val rowId = r.getLong(Tasks.ID) ?: return@childRows null + SubtaskReset.providerNode(rowId, r.getString(Tasks.RRULE), r.getString(Tasks.RDATE), copies, subtaskState(r)) + } + } + writeSubtaskStates(authority, reset.map { it.id to SubtaskState.FRESH }) + return reset + } + + private fun subtaskState(r: ColumnReader) = + SubtaskState(statusFromInt(r.getInt(Tasks.STATUS)), r.getInt(Tasks.PERCENT_COMPLETE), r.getLong(Tasks.COMPLETED)) + + private fun subtaskOps(authority: String, states: List>): List = + states.map { (id, state) -> + val values = mapOf( + Tasks.STATUS to state.status.toInt(), + Tasks.PERCENT_COMPLETE to state.percent, + Tasks.COMPLETED to state.completedAt, + ) + ContentProviderOperation.newUpdate(taskUri(authority, id)).withValues(values.toContentValues()).build() + } + + private fun writeSubtaskStates(authority: String, states: List>) { + if (states.isNotEmpty()) resolver.applyBatch(authority, ArrayList(subtaskOps(authority, states))) + } + + /** The reset sub-tasks of [reset] that [SubtaskReset.toRestore] gives back their state. */ + private fun subtaskRestore(authority: String, reset: String?): List { + val entries = SubtaskReset.decode(reset) + if (entries.isEmpty()) return emptyList() + val current = resolver.query( + TasksContract.tasksUri(authority), + SUBTASK_STATE, + "${Tasks.ID} IN (${entries.joinToString { "?" }}) AND (${Tasks.DELETED} IS NULL OR ${Tasks.DELETED} = 0)", + entries.map { it.id.toString() }.toTypedArray(), + null, + )?.use { c -> + val r = CursorColumnReader(c) + buildMap { while (c.moveToNext()) r.getLong(Tasks.ID)?.let { put(it, subtaskState(r)) } } + }.orEmpty() + return subtaskOps(authority, SubtaskReset.toRestore(entries, current::get).map { it.id to it.prior }) + } + + /** Walks [parentId]'s direct sub-task rows, overrides left out, with [read] on each. */ + private fun childRows(authority: String, parentId: Long, projection: Array?, read: (ColumnReader) -> T?): List = + resolver.query( + TasksContract.tasksUri(authority), + projection, + "${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 || r.getLong(Tasks.ID) == parentId) continue + read(r)?.let(::add) } } - } - } - - 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() + } ?: emptyList() /** * Undoes the tick that left [taskId]'s occurrence at [occurrenceStart] — or, @@ -831,6 +860,7 @@ class AndroidTasksDataSource @Inject constructor( ContentProviderOperation.newUpdate(taskUri(authority, series)).withValues(restore.toContentValues()).build(), ContentProviderOperation.newDelete(taskUri(authority, tick.copyId)).build(), ) + runCatching { ops += subtaskRestore(authority, tick.reset) }.onFailure { Log.w(TAG, "Reading the reset sub-tasks failed", it) } resolver.applyBatch(authority, ops) ticks.remove(key) true @@ -869,7 +899,10 @@ class AndroidTasksDataSource @Inject constructor( } }?.singleOrNull() - private val ticks = TickedCopies { context.getSharedPreferences(TICKED_COPIES, Context.MODE_PRIVATE) } + private val ticks = TickedCopies( + prefs = { context.getSharedPreferences(TICKED_COPIES, Context.MODE_PRIVATE) }, + known = { context.getSharedPreferences(KNOWN_COPIES, Context.MODE_PRIVATE) }, + ) /** The copy a tick left for [taskId]'s occurrence at [occurrenceStart], if one was remembered. */ private fun tickedCopy(taskId: Long, occurrenceStart: Instant): Long? = @@ -978,7 +1011,15 @@ private const val MIGRATIONS = "provider_migrations" private const val TICKED_COPIES = "provider_ticked_copies" +/** Every copy a tick left, kept past [TickedCopies.HORIZON_MS]: nothing on the row itself marks it. */ +private const val KNOWN_COPIES = "provider_known_copies" + /** What a tick changes on a series, and an undo changes back. */ +/** What resetting a sub-task reads: whether it recurs, and its progress. */ +private val SUBTASK_STATE = arrayOf( + Tasks.ID, Tasks.ORIGINAL_INSTANCE_ID, Tasks.RRULE, Tasks.RDATE, Tasks.STATUS, Tasks.PERCENT_COMPLETE, Tasks.COMPLETED, +) + private val SERIES_STATE = arrayOf( Tasks.DTSTART, Tasks.DUE, Tasks.DURATION, Tasks.RRULE, Tasks.RDATE, Tasks.EXDATE, Tasks.STATUS, Tasks.COMPLETED, Tasks.PERCENT_COMPLETE, 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 index 16d2853..eb55d0e 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/SubtaskReset.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/SubtaskReset.kt @@ -1,39 +1,109 @@ 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, -) +import de.jeanlucmakiola.agendula.data.tasks.TasksContract.Tasks +import de.jeanlucmakiola.agendula.domain.TaskStatus +import de.jeanlucmakiola.agendula.domain.isClosed +import de.jeanlucmakiola.agendula.domain.recurrence.ExpansionWindow +import de.jeanlucmakiola.agendula.domain.recurrence.RecurrenceExpander +import de.jeanlucmakiola.agendula.domain.recurrence.RecurrenceSpec +import de.jeanlucmakiola.agendula.domain.statusFromInt +import kotlin.time.Duration.Companion.days +import kotlin.time.Instant + +/** A sub-task's progress, which a routine's next occurrence starts without. */ +internal data class SubtaskState(val status: TaskStatus, val percent: Int?, val completedAt: Long?) { + /** Open with nothing done yet: what a reset leaves. */ + val isFresh: Boolean get() = status == TaskStatus.NEEDS_ACTION && (percent ?: 0) == 0 && completedAt == null + + companion object { + val FRESH = SubtaskState(TaskStatus.NEEDS_ACTION, null, null) + } +} + +/** A sub-task as [SubtaskReset] sees it; [recurring] also covers a copy a tick of a recurring one left. */ +internal class SubtaskNode(val id: Long, val recurring: Boolean, val state: SubtaskState) + +/** A sub-task [SubtaskReset] reset, with the state it had. */ +internal data class ResetSubtask(val id: Long, val prior: SubtaskState) /** * 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. + * (#160): the ones completed or in progress start over, as in tasks.org, and + * moving the series back gives them their state again. */ internal object SubtaskReset { + /** Only bounds the search for a next occurrence. */ + private val LOOKAHEAD = (366 * 1000).days + /** - * 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. + * The sub-tasks under [parentId], all the way down, to reset. A recurring one + * has a series of its own and is left alone with everything under it; a + * cancelled one stays cancelled. */ - fun toReopen(parentId: Long, children: (Long) -> List): List { + fun toReset(parentId: Long, children: (Long) -> List): List { val seen = mutableSetOf(parentId) - val reopen = mutableListOf() + val reset = 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) - } + children(id).filter { !it.recurring && seen.add(it.id) }.forEach { + if (it.state.status != TaskStatus.CANCELLED && !it.state.isFresh) reset += ResetSubtask(it.id, it.state) + walk(it.id) + } } walk(parentId) - return reopen + return reset } + + /** + * A provider row as a node. Nothing on a copy the provider detached marks it + * as one, so only the [copies] a tick in Agendula left are known: they belong + * to a recurring sub-task and count as recurring. + */ + fun providerNode(id: Long, rrule: String?, rdate: String?, copies: Set, state: SubtaskState) = + SubtaskNode(id, !rrule.isNullOrBlank() || !rdate.isNullOrBlank() || id in copies, state) + + /** The reset sub-tasks still as the reset left them, so moving back may restore them. */ + fun toRestore(reset: List, current: (Long) -> SubtaskState?): List = + reset.filter { current(it.id)?.isFresh == true } + + fun encode(reset: List): String = reset.joinToString(";") { + listOf(it.id, it.prior.status.name, it.prior.percent ?: "", it.prior.completedAt ?: "").joinToString(",") + } + + fun decode(raw: String?): List = raw.orEmpty().split(';').mapNotNull { entry -> + val parts = entry.split(',') + if (parts.size != 4) return@mapNotNull null + val id = parts[0].toLongOrNull() ?: return@mapNotNull null + val status = TaskStatus.entries.firstOrNull { it.name == parts[1] } ?: return@mapNotNull null + ResetSubtask(id, SubtaskState(status, parts[2].toIntOrNull(), parts[3].toLongOrNull())) + } + + /** + * Whether a provider tick moved the series on, judged by its columns + * [before] and [after]: its start (or due) moved later and it is still open, + * with [next], its rule set as it now stands, generating an occurrence from + * there. An `EXDATE` or a status alone is no move; a series whose rule ran + * out is finished. + */ + fun movedOn(before: Map?, after: Map?, next: RecurrenceSpec?): Boolean { + val was = anchor(before ?: return false) ?: return false + val now = anchor(after ?: return false) ?: return false + if (now <= was) return false + if (statusFromInt((after[Tasks.STATUS] as? Number)?.toInt()).isClosed) return false + val spec = next ?: return false + val parts = spec.rrule?.split(';')?.map { it.trim() }?.filter { it.isNotEmpty() } + val count = parts?.firstOrNull { it.startsWith("COUNT=", ignoreCase = true) }?.substringAfter('=')?.toIntOrNull() + if (count != null && count <= 0) return false + // Expanded from the old start, as an expansion always yields its own start + // even past UNTIL. COUNT is left out: it was spent before the new start. + val probe = spec.copy( + rrule = parts?.filterNot { it.startsWith("COUNT=", ignoreCase = true) }?.joinToString(";"), + anchor = Instant.fromEpochMilliseconds(was), + ) + val from = Instant.fromEpochMilliseconds(now) + return RecurrenceExpander.expand(probe, ExpansionWindow(from = from, until = from + LOOKAHEAD, maxOccurrences = 1)).isNotEmpty() + } + + private fun anchor(state: Map): Long? = + (state[Tasks.DTSTART] as? Number)?.toLong() ?: (state[Tasks.DUE] as? Number)?.toLong() } diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TickedCopies.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TickedCopies.kt index 23092e4..7298904 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TickedCopies.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TickedCopies.kt @@ -5,8 +5,9 @@ import org.json.JSONObject import kotlin.time.Instant /** - * What ticking a provider occurrence left: the copy the provider detached, and - * the series' columns before and after the tick, so an undo can move it back. + * What ticking a provider occurrence left: the copy the provider detached, the + * series' columns before and after the tick, so an undo can move it back, and + * the sub-tasks it reset, as [SubtaskReset.encode] writes them. */ internal class Tick( val copyId: Long, @@ -14,10 +15,15 @@ internal class Tick( val before: Map?, val after: Map?, val at: Long, + val reset: String? = null, ) -/** The provider keeps no record of its ticks, so they are kept here for [HORIZON_MS]. */ -internal class TickedCopies(private val prefs: () -> SharedPreferences) { +/** + * The provider keeps no record of its ticks, so they are kept here for + * [HORIZON_MS]. The copies' ids are kept for good in [known]: a detached copy + * carries nothing that marks it as one. + */ +internal class TickedCopies(private val prefs: () -> SharedPreferences, private val known: () -> SharedPreferences) { fun key(authority: String, taskId: Long, occurrenceStart: Instant) = "$authority|$taskId|${wholeSecond(occurrenceStart).epochSeconds}" @@ -31,6 +37,15 @@ internal class TickedCopies(private val prefs: () -> SharedPreferences) { (value as? String)?.let(::decode)?.takeIf { it.copyId == copyId }?.let { key to it } } + /** Every copy a tick in Agendula left in [authority]. */ + fun copies(authority: String): Set { + val kept = known().getStringSet(authority, emptySet()).orEmpty().mapNotNullTo(HashSet()) { it.toLongOrNull() } + prefs().all.forEach { (key, value) -> + if (key.startsWith("$authority|")) (value as? String)?.let(::decode)?.let { kept += it.copyId } + } + return kept + } + fun put(key: String, tick: Tick) { val stale = prefs().all.filter { (_, value) -> val at = (value as? String)?.let(::decode)?.at ?: 0L @@ -40,6 +55,9 @@ internal class TickedCopies(private val prefs: () -> SharedPreferences) { stale.forEach(::remove) putString(key, encode(tick)) }.apply() + val authority = key.substringBefore('|') + val kept = known().getStringSet(authority, emptySet()).orEmpty() + known().edit().putStringSet(authority, kept + tick.copyId.toString()).apply() } fun remove(key: String) { @@ -52,6 +70,7 @@ internal class TickedCopies(private val prefs: () -> SharedPreferences) { tick.seriesId?.let { put("series", it) } tick.before?.let { put("before", columns(it)) } tick.after?.let { put("after", columns(it)) } + tick.reset?.let { put("reset", it) } }.toString() private fun columns(values: Map) = JSONObject().apply { @@ -66,6 +85,7 @@ internal class TickedCopies(private val prefs: () -> SharedPreferences) { before = json.optJSONObject("before")?.let(::values), after = json.optJSONObject("after")?.let(::values), at = json.getLong("at"), + reset = if (json.has("reset")) json.getString("reset") else null, ) }.getOrNull() 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 18f1001..d027b77 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 @@ -51,6 +51,8 @@ data class AdvancedOccurrenceEntity( @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, + /** The series' sub-tasks the move reset, with their state before, as [de.jeanlucmakiola.agendula.data.tasks.SubtaskReset.encode] writes them. */ + @ColumnInfo(name = "reset_subtasks", defaultValue = "") val resetSubtasks: String = "", ) { val dropped: List get() = droppedExdates.split(',').mapNotNull { it.toLongOrNull()?.let(Instant::fromEpochMilliseconds) } 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 800e119..4b540ff 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,8 +1,10 @@ package de.jeanlucmakiola.agendula.data.tasks.room import androidx.room.InvalidationTracker +import de.jeanlucmakiola.agendula.data.tasks.ResetSubtask import de.jeanlucmakiola.agendula.data.tasks.SubtaskNode import de.jeanlucmakiola.agendula.data.tasks.SubtaskReset +import de.jeanlucmakiola.agendula.data.tasks.SubtaskState import de.jeanlucmakiola.agendula.data.tasks.TaskQuery import de.jeanlucmakiola.agendula.domain.TaskReminder import de.jeanlucmakiola.agendula.domain.TaskReminderOrder @@ -640,7 +642,7 @@ class RoomTasksDataSource @Inject constructor( isDirty = true, ), ) - val reopenedLists = reopenSubtasks(master.id, now) + val reset = resetSubtasks(master.id, now) val copyId = if (override != null) { detach(override, now, close) } else { @@ -666,26 +668,41 @@ class RoomTasksDataSource @Inject constructor( ruleAfter = ruleAfter, droppedExdates = AdvancedOccurrenceEntity.encode(TaskFormWriter.instantsBefore(master.exdate, newAnchor, zone)), fromOverride = override != null, + resetSubtasks = SubtaskReset.encode(reset), ), ) advanced.insertDetached(detached.map { (at, id) -> AdvancedDetachedEntity(copyId = copyId, anchor = at, taskId = id) }) - writes.onWritten(setOf(master.listId) + reopenedLists) + writes.onWritten(setOf(master.listId) + reset.mapNotNull { tasks.entity(it.id)?.listId }) 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) - } + /** Resets what [SubtaskReset] picks under [parentId] for the series' next occurrence. */ + private fun resetSubtasks(parentId: Long, now: Instant): List { + val reset = SubtaskReset.toReset(parentId) { id -> + tasks.subtasks(id).map { SubtaskNode(it.task.id, it.task.isRecurring, it.task.progress) } } - return reopen.mapNotNull(tasks::entity).map { - tasks.update(TaskFormWriter.completed(it, completed = false, now = now)) - it.listId + reset.forEach { tasks.entity(it.id)?.let { sub -> tasks.update(sub.withProgress(SubtaskState.FRESH, now)) } } + return reset + } + + /** What [resetSubtasks] reset for [record], back as it was where nothing changed it since. */ + private fun restoreSubtasks(record: AdvancedOccurrenceEntity, now: Instant): Set { + val restore = SubtaskReset.toRestore(SubtaskReset.decode(record.resetSubtasks)) { tasks.entity(it)?.takeUnless { e -> e.isDeleted }?.progress } + return restore.mapNotNull { reset -> + tasks.entity(reset.id)?.also { tasks.update(it.withProgress(reset.prior, now)) }?.listId }.toSet() } + private val TaskEntity.progress get() = SubtaskState(status, percentComplete, completedAt?.toEpochMilliseconds()) + + private fun TaskEntity.withProgress(state: SubtaskState, now: Instant) = copy( + status = state.status, + percentComplete = state.percent, + completedAt = state.completedAt?.let(Instant::fromEpochMilliseconds), + lastModified = now, + isDirty = true, + ) + /** 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), @@ -770,7 +787,7 @@ class RoomTasksDataSource @Inject constructor( 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)) + writes.onWritten(setOf(master.listId) + restoreSubtasks(record, now)) return true } 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 6d81763..84e1cb8 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 @@ -24,7 +24,7 @@ import kotlin.time.Instant AdvancedOccurrenceEntity::class, AdvancedDetachedEntity::class, ], - version = 5, + version = 6, exportSchema = true, ) @TypeConverters(Converters::class) @@ -204,5 +204,12 @@ abstract class TasksDatabase : RoomDatabase() { ) } } + + /** v6 remembers which sub-tasks a moved series reset, so moving it back restores them (#160). */ + val MIGRATION_5_6 = object : Migration(5, 6) { + override fun migrate(db: SupportSQLiteDatabase) { + db.execSQL("ALTER TABLE advanced_occurrences ADD COLUMN `reset_subtasks` TEXT NOT NULL DEFAULT ''") + } + } } } 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 index 09cb351..ed3b487 100644 --- a/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/SubtaskResetTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/SubtaskResetTest.kt @@ -1,27 +1,45 @@ package de.jeanlucmakiola.agendula.data.tasks import com.google.common.truth.Truth.assertThat +import de.jeanlucmakiola.agendula.data.tasks.TasksContract.Tasks +import de.jeanlucmakiola.agendula.domain.TaskStatus +import de.jeanlucmakiola.agendula.domain.recurrence.RecurrenceSpec import org.junit.jupiter.api.Test +import kotlin.time.Instant class SubtaskResetTest { - private fun node(id: Long, title: String = "Step $id", recurring: Boolean = false, completed: Boolean = true) = - SubtaskNode(id, title, recurring, completed) + private val done = SubtaskState(TaskStatus.COMPLETED, 100, 1_000L) + private val halfway = SubtaskState(TaskStatus.IN_PROCESS, 50, null) + private val cancelled = SubtaskState(TaskStatus.CANCELLED, null, null) - private fun reopen(vararg tree: Pair>): List { + private fun node(id: Long, state: SubtaskState = done, recurring: Boolean = false) = SubtaskNode(id, recurring, state) + + private fun reset(vararg tree: Pair>): List { val children = tree.toMap() - return SubtaskReset.toReopen(1L) { children[it].orEmpty() } + return SubtaskReset.toReset(1L) { children[it].orEmpty() }.map { it.id } } @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) + fun `completed and half-done sub-tasks of a routine start over, fresh ones are left (#160)`() { + val reopened = reset(1L to listOf(node(2), node(3, SubtaskState.FRESH), node(4, halfway))) + assertThat(reopened).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)), + fun `an open sub-task with progress counts as started`() { + assertThat(reset(1L to listOf(node(2, SubtaskState(TaskStatus.NEEDS_ACTION, 20, null))))).containsExactly(2L) + } + + @Test + fun `a cancelled sub-task stays cancelled, what is under it still starts over`() { + assertThat(reset(1L to listOf(node(2, cancelled)), 2L to listOf(node(3)))).containsExactly(3L) + } + + @Test + fun `nested sub-tasks start over too, also under a fresh one`() { + val reopened = reset( + 1L to listOf(node(2, SubtaskState.FRESH)), 2L to listOf(node(3)), 3L to listOf(node(4)), ) @@ -29,27 +47,89 @@ class SubtaskResetTest { } @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)), + fun `a recurring sub-task, or a copy a tick of one left, is left alone with what is under it`() { + val reopened = reset( + 1L to listOf(node(2, recurring = true), node(3), node(5, recurring = true)), 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) + fun `a provider copy a tick in Agendula left is left alone, a plain sub-task is not`() { + val copies = setOf(3L) + val rows = listOf( + SubtaskReset.providerNode(2, rrule = "FREQ=DAILY", rdate = null, copies = copies, state = done), + SubtaskReset.providerNode(3, rrule = null, rdate = null, copies = copies, state = done), + SubtaskReset.providerNode(4, rrule = null, rdate = "20261110T080000Z", copies = copies, state = done), + SubtaskReset.providerNode(5, rrule = " ", rdate = null, copies = copies, state = done), + ) + assertThat(SubtaskReset.toReset(1L) { if (it == 1L) rows else emptyList() }.map { it.id }).containsExactly(5L) } @Test fun `a cycle or the parent listed under itself is not walked twice`() { - val reopened = reopen( + val reopened = reset( 1L to listOf(node(1), node(2)), 2L to listOf(node(3)), 3L to listOf(node(2)), ) assertThat(reopened).containsExactly(2L, 3L) } + + @Test + fun `what was reset survives encoding`() { + val reset = listOf(ResetSubtask(2, done), ResetSubtask(3, halfway), ResetSubtask(4, SubtaskState(TaskStatus.NEEDS_ACTION, 10, null))) + assertThat(SubtaskReset.decode(SubtaskReset.encode(reset))).isEqualTo(reset) + assertThat(SubtaskReset.decode("")).isEmpty() + assertThat(SubtaskReset.decode(null)).isEmpty() + assertThat(SubtaskReset.decode("x,COMPLETED,,;2,NOPE,,;3,IN_PROCESS")).isEmpty() + } + + @Test + fun `moving back restores only the sub-tasks still as the reset left them`() { + val reset = listOf(ResetSubtask(2, done), ResetSubtask(3, halfway), ResetSubtask(4, done)) + val current = mapOf(2L to SubtaskState.FRESH, 3L to done) + assertThat(SubtaskReset.toRestore(reset, current::get)).containsExactly(ResetSubtask(2, done)) + } + + private val week = 7L * 24 * 60 * 60 * 1000 + private val start = Instant.parse("2026-11-02T08:00:00Z").toEpochMilliseconds() + + private fun state(dtstart: Long?, status: Int = TasksContract.STATUS_NEEDS_ACTION, exdate: String? = null, due: Long? = null) = + mapOf(Tasks.DTSTART to dtstart, Tasks.DUE to due, Tasks.STATUS to status.toLong(), Tasks.EXDATE to exdate) + + private fun spec(at: Long, rrule: String = "FREQ=WEEKLY") = + RecurrenceSpec(rrule = rrule, rdate = null, exdate = null, anchor = Instant.fromEpochMilliseconds(at), isAllDay = false, timeZone = "UTC") + + @Test + fun `a provider series whose start moved later has moved on`() { + assertThat(SubtaskReset.movedOn(state(start), state(start + week), spec(start + week))).isTrue() + assertThat(SubtaskReset.movedOn(state(null, due = start), state(null, due = start + week), spec(start + week))).isTrue() + } + + @Test + fun `ticking a later occurrence only adds an exception and moves nothing`() { + val after = state(start, exdate = "20261109T080000Z") + assertThat(SubtaskReset.movedOn(state(start), after, spec(start))).isFalse() + } + + @Test + fun `a provider series ticked on its last occurrence is finished`() { + // OpenTasks deletes the master or drops its rule: there is no series state after. + assertThat(SubtaskReset.movedOn(state(start), null, null)).isFalse() + // Closed through its status. + assertThat(SubtaskReset.movedOn(state(start), state(start + week, TasksContract.STATUS_COMPLETED), spec(start + week))).isFalse() + // Dates moved, but the rule has nothing left from there. + val ended = spec(start + week, rrule = "FREQ=WEEKLY;UNTIL=20261103T080000Z") + assertThat(SubtaskReset.movedOn(state(start), state(start + week), ended)).isFalse() + assertThat(SubtaskReset.movedOn(state(start), state(start + week), null)).isFalse() + } + + @Test + fun `a provider series moved earlier or without dates has not moved on`() { + assertThat(SubtaskReset.movedOn(state(start), state(start - week), spec(start - week))).isFalse() + assertThat(SubtaskReset.movedOn(state(null), state(null), spec(start))).isFalse() + assertThat(SubtaskReset.movedOn(null, state(start + week), spec(start + week))).isFalse() + } }