diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/reminders/DueReminderReceiver.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/reminders/DueReminderReceiver.kt index dba31d7..01aac13 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/reminders/DueReminderReceiver.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/reminders/DueReminderReceiver.kt @@ -3,6 +3,7 @@ package de.jeanlucmakiola.agendula.data.reminders import android.content.BroadcastReceiver import android.content.Context import android.content.Intent +import androidx.core.net.toUri import dagger.hilt.android.AndroidEntryPoint import de.jeanlucmakiola.agendula.data.prefs.SettingsPrefs import de.jeanlucmakiola.agendula.data.tasks.TasksDataSource @@ -45,7 +46,14 @@ class DueReminderReceiver : BroadcastReceiver() { companion object { private const val EXTRA_TASK_ID = "de.jeanlucmakiola.agendula.extra.TASK_ID" - fun intent(context: Context, taskId: Long): Intent = - Intent(context, DueReminderReceiver::class.java).putExtra(EXTRA_TASK_ID, taskId) + /** + * [triggerAt] rides in the intent *data*, not just an extra: PendingIntent + * identity ignores extras, so two occurrences of the same recurring task + * would otherwise collapse into one alarm under FLAG_UPDATE_CURRENT. + */ + fun intent(context: Context, taskId: Long, triggerAt: Long): Intent = + Intent(context, DueReminderReceiver::class.java) + .setData("agendula://reminder/$taskId/$triggerAt".toUri()) + .putExtra(EXTRA_TASK_ID, taskId) } } diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/reminders/ProviderChangeReceiver.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/reminders/ProviderChangeReceiver.kt index 82c491b..c0c74e9 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/reminders/ProviderChangeReceiver.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/reminders/ProviderChangeReceiver.kt @@ -3,7 +3,9 @@ package de.jeanlucmakiola.agendula.data.reminders import android.content.BroadcastReceiver import android.content.Context import android.content.Intent +import android.os.SystemClock import dagger.hilt.android.AndroidEntryPoint +import de.jeanlucmakiola.agendula.data.tasks.ProviderResolver import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.SupervisorJob @@ -20,10 +22,25 @@ import javax.inject.Inject class ProviderChangeReceiver : BroadcastReceiver() { @Inject lateinit var scheduler: ReminderScheduler + @Inject lateinit var providerResolver: ProviderResolver private val scope = CoroutineScope(SupervisorJob() + Dispatchers.Default) override fun onReceive(context: Context, intent: Intent) { + // The receiver has to stay exported to hear the provider's broadcast, and + // the sender holds no permission we could require — so validate the + // broadcast itself. Without this, any installed app can spam a full + // re-sync (an unbounded provider read) by firing a matching intent. + if (intent.action != Intent.ACTION_PROVIDER_CHANGED) return + val authority = providerResolver.resolve()?.authority ?: return + if (intent.data?.host != authority) return + // External sync can fire these in bursts; one re-sync per burst is plenty. + val now = SystemClock.elapsedRealtime() + synchronized(Companion) { + if (now - lastSyncAt < MIN_SYNC_INTERVAL_MS) return + lastSyncAt = now + } + val pending = goAsync() scope.launch { try { @@ -33,4 +50,11 @@ class ProviderChangeReceiver : BroadcastReceiver() { } } } + + private companion object { + const val MIN_SYNC_INTERVAL_MS = 10_000L + + @Volatile + var lastSyncAt = -MIN_SYNC_INTERVAL_MS + } } diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/reminders/ReminderScheduler.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/reminders/ReminderScheduler.kt index 4cea316..8f1cb58 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/reminders/ReminderScheduler.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/reminders/ReminderScheduler.kt @@ -43,35 +43,57 @@ class ReminderScheduler @Inject constructor( val tasks = runCatching { dataSource.tasks(TaskQuery(includeCompleted = false)) } .getOrElse { return@withContext } + // One reminder per *occurrence*: the instances view yields a row per + // occurrence, all sharing a taskId, so this is a Set rather than a + // taskId-keyed Map — keying by task would collapse a daily recurring task + // down to one arbitrary reminder (the query is unsorted, so which one + // survived was provider-defined). + // Per-task leads, stored as Alarm property rows. One query for all of them. + val perTask = runCatching { dataSource.alarms() }.getOrElse { emptyMap() } + val desired = tasks .filter { !it.isClosed && it.due != null } .mapNotNull { task -> - // The task's list may override the global lead, or opt out entirely - // (override = null), in which case it gets no reminder at all. - val lead = settings.reminderLeadFor(task.listId) ?: return@mapNotNull null - task.taskId to (task.due!!.toEpochMilliseconds() - lead.coerceAtLeast(0) * 60_000L) + // A reminder set on the task itself wins; otherwise the task's list + // may override the global lead, or opt out entirely (override = + // null), in which case it gets no reminder at all. + val lead = perTask[task.taskId] + ?: settings.reminderLeadFor(task.listId) + ?: return@mapNotNull null + ScheduledReminder( + taskId = task.taskId, + triggerAt = task.due!!.toEpochMilliseconds() - lead.coerceAtLeast(0) * 60_000L, + ) } - .toMap() - .filterValues { it in now..horizon } + // The lower bound trails `now` so a reminder missed while the device was + // off still fires once on boot instead of being silently dropped — + // setExactAndAllowWhileIdle delivers a past trigger immediately. Anything + // already armed stays armed (the diff below), so it can't re-fire. + .filter { it.triggerAt in (now - MISSED_GRACE_MS)..horizon } + .toSet() val previous = store.all() - (previous.keys - desired.keys).forEach { cancel(it) } - desired.forEach { (taskId, triggerAt) -> - if (previous[taskId] != triggerAt) schedule(taskId, triggerAt) - } + (previous - desired).forEach { cancel(it) } + (desired - previous).forEach { schedule(it) } store.replace(desired) } private fun alarmManager(): AlarmManager = context.getSystemService(AlarmManager::class.java) - private fun pendingIntent(taskId: Long, create: Boolean): PendingIntent? { + private fun pendingIntent(reminder: ScheduledReminder, create: Boolean): PendingIntent? { val flags = (if (create) PendingIntent.FLAG_UPDATE_CURRENT else PendingIntent.FLAG_NO_CREATE) or PendingIntent.FLAG_IMMUTABLE - return PendingIntent.getBroadcast(context, taskId.toInt(), DueReminderReceiver.intent(context, taskId), flags) + return PendingIntent.getBroadcast( + context, + reminder.requestCode, + DueReminderReceiver.intent(context, reminder.taskId, reminder.triggerAt), + flags, + ) } - private fun schedule(taskId: Long, triggerAt: Long) { - val pi = pendingIntent(taskId, create = true) ?: return + private fun schedule(reminder: ScheduledReminder) { + val triggerAt = reminder.triggerAt + val pi = pendingIntent(reminder, create = true) ?: return val am = alarmManager() val canExact = Build.VERSION.SDK_INT < Build.VERSION_CODES.S || am.canScheduleExactAlarms() if (canExact) { @@ -81,19 +103,21 @@ class ReminderScheduler @Inject constructor( } } - private fun cancel(taskId: Long) { - pendingIntent(taskId, create = false)?.let { + private fun cancel(reminder: ScheduledReminder) { + pendingIntent(reminder, create = false)?.let { alarmManager().cancel(it) it.cancel() } } private suspend fun clearAll() { - store.all().keys.forEach { cancel(it) } - store.replace(emptyMap()) + store.all().forEach { cancel(it) } + store.replace(emptySet()) } private companion object { const val WINDOW_MS = 30L * 24 * 60 * 60 * 1000 // 30 days + /** How long after its trigger a missed reminder is still worth firing. */ + const val MISSED_GRACE_MS = 6L * 60 * 60 * 1000 // 6 hours } } diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/reminders/ScheduledReminderStore.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/reminders/ScheduledReminderStore.kt index 1331b0a..a802258 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/reminders/ScheduledReminderStore.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/reminders/ScheduledReminderStore.kt @@ -9,25 +9,38 @@ import javax.inject.Inject import javax.inject.Singleton /** - * Remembers which task reminders are currently scheduled (taskId → trigger time), - * so [ReminderScheduler] can diff against a fresh computation and cancel only the - * alarms that changed. Persisted in DataStore as a set of `taskId|trigger` strings. + * One armed alarm. A recurring task has many occurrences sharing a [taskId], so + * the trigger time is part of the identity — keying by task alone would collapse + * a daily task down to a single reminder. + */ +data class ScheduledReminder(val taskId: Long, val triggerAt: Long) { + /** + * Request code for this alarm's PendingIntent. Derived from both fields so + * sibling occurrences don't share (and overwrite) one alarm slot. + */ + val requestCode: Int get() = (taskId * 31 + triggerAt).hashCode() +} + +/** + * Remembers which task reminders are currently armed, so [ReminderScheduler] can + * diff against a fresh computation and touch only the alarms that changed. + * Persisted in DataStore as a set of `taskId|trigger` strings. */ @Singleton class ScheduledReminderStore @Inject constructor( private val dataStore: DataStore, ) { - suspend fun all(): Map = + suspend fun all(): Set = dataStore.data.first()[KEY].orEmpty().mapNotNull { entry -> val parts = entry.split('|') val id = parts.getOrNull(0)?.toLongOrNull() val at = parts.getOrNull(1)?.toLongOrNull() - if (id != null && at != null) id to at else null - }.toMap() + if (id != null && at != null) ScheduledReminder(id, at) else null + }.toSet() - suspend fun replace(scheduled: Map) { + suspend fun replace(scheduled: Set) { dataStore.edit { prefs -> - prefs[KEY] = scheduled.entries.map { "${it.key}|${it.value}" }.toSet() + prefs[KEY] = scheduled.map { "${it.taskId}|${it.triggerAt}" }.toSet() } } 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 1ddbfa6..7487a95 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 @@ -11,6 +11,7 @@ import android.os.Looper import dagger.hilt.android.qualifiers.ApplicationContext import de.jeanlucmakiola.agendula.data.tasks.TasksContract.Instances import de.jeanlucmakiola.agendula.data.tasks.TasksContract.Lists +import de.jeanlucmakiola.agendula.data.tasks.TasksContract.Properties import de.jeanlucmakiola.agendula.data.tasks.TasksContract.Tasks import de.jeanlucmakiola.agendula.domain.Task import de.jeanlucmakiola.agendula.domain.TaskForm @@ -98,6 +99,50 @@ class AndroidTasksDataSource @Inject constructor( if (rows == 0) throw TaskWriteFailedException("update task $taskId") } + override fun updateInstance(instanceId: Long, form: TaskForm) { + val values = TaskWriteMapper.instanceValues(form, ZoneId.systemDefault().id) + val uri = TasksContract.instanceUri(authority(), instanceId) + val rows = resolver.update(uri, values.toContentValues(), null, null) + if (rows == 0) throw TaskWriteFailedException("update instance $instanceId") + } + + override fun setAlarm(taskId: Long, minutesBeforeDue: Int?) { + val uri = TasksContract.propertiesUri(authority()) + // Replace rather than update: the provider's AlarmHandler re-validates the + // whole row on every update, so a partial edit throws — and delete+insert + // means we never have to track property_id. + resolver.delete( + uri, + "${Properties.TASK_ID} = ? AND ${Properties.MIMETYPE} = ?", + arrayOf(taskId.toString(), TasksContract.Alarm.MIMETYPE), + ) + if (minutesBeforeDue != null) { + resolver.insert(uri, TaskWriteMapper.alarmValues(taskId, minutesBeforeDue).toContentValues()) + ?: throw TaskWriteFailedException("set alarm for task $taskId") + } + } + + override fun alarms(): Map { + val uri = TasksContract.propertiesUri(authority()) + val projection = arrayOf(Properties.TASK_ID, TasksContract.Alarm.MINUTES_BEFORE) + return resolver.query( + uri, + projection, + "${Properties.MIMETYPE} = ?", + arrayOf(TasksContract.Alarm.MIMETYPE), + null, + )?.use { c -> + val reader = CursorColumnReader(c) + buildMap { + while (c.moveToNext()) { + val id = reader.getLong(Properties.TASK_ID) + val minutes = reader.getInt(TasksContract.Alarm.MINUTES_BEFORE) + if (id != null && minutes != null) put(id, minutes) + } + } + } ?: emptyMap() + } + override fun setCompleted(taskId: Long, completed: Boolean) { val values = TaskWriteMapper.completionValues(completed, System.currentTimeMillis()) val rows = resolver.update(taskUri(authority(), taskId), values.toContentValues(), null, null) @@ -127,9 +172,16 @@ class AndroidTasksDataSource @Inject constructor( val observer = object : ContentObserver(Handler(Looper.getMainLooper())) { override fun onChange(selfChange: Boolean) = onChange() } - resolver.registerContentObserver(TasksContract.instancesUri(provider.authority), true, observer) - resolver.registerContentObserver(TasksContract.listsUri(provider.authority), true, observer) - return AutoCloseable { resolver.unregisterContentObserver(observer) } + // Register both or neither: if the second call throws, the first + // registration would otherwise leak (no AutoCloseable was handed back yet). + try { + resolver.registerContentObserver(TasksContract.instancesUri(provider.authority), true, observer) + resolver.registerContentObserver(TasksContract.listsUri(provider.authority), true, observer) + } catch (e: RuntimeException) { + runCatching { resolver.unregisterContentObserver(observer) } + throw e + } + return AutoCloseable { runCatching { resolver.unregisterContentObserver(observer) } } } private fun Map.toContentValues(): ContentValues { diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/ProviderFlow.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/ProviderFlow.kt new file mode 100644 index 0000000..472df64 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/ProviderFlow.kt @@ -0,0 +1,31 @@ +package de.jeanlucmakiola.agendula.data.tasks + +import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.retryWhen + +private const val BASE_RETRY_MS = 1_000L +private const val MAX_RETRY_MS = 30_000L + +/** 1s, 2s, 4s … capped at 30s, so a permanently-absent provider costs little. */ +private fun retryDelayMs(attempt: Long): Long = + (BASE_RETRY_MS shl attempt.coerceAtMost(5).toInt()).coerceAtMost(MAX_RETRY_MS) + +/** + * Recover a provider-backed flow without killing it. + * + * Provider reads fail for reasons that resolve on their own: the read permission + * isn't granted yet (first launch collects before the permission gate), or the + * provider app is mid-update. A terminal `catch` swallows the failure *and* + * cancels the upstream, so the flow never produces again — the screen stays empty + * until the process restarts, even after the user grants the permission. + * + * This emits [fallback] instead and keeps retrying with a capped backoff, so the + * collector recovers on its own once the provider becomes readable. + */ +fun Flow.recoveringFromProviderFailure(fallback: () -> T): Flow = + retryWhen { _, attempt -> + emit(fallback()) + delay(retryDelayMs(attempt)) + true + } 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 c8b36ab..11273e8 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 @@ -38,7 +38,14 @@ object TaskMapper { listName = r.getString(Tasks.LIST_NAME), accountName = r.getString(Tasks.ACCOUNT_NAME), parentId = r.getLong(Tasks.PARENT_ID), - isRecurring = r.getBoolean(Instances.IS_RECURRING), + // 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. + isRecurring = r.getString(Tasks.RRULE) != null || + r.getString(Tasks.RDATE) != null || + r.getBoolean(Instances.IS_RECURRING), distanceFromCurrent = r.getInt(Instances.DISTANCE_FROM_CURRENT), created = instant(Tasks.CREATED), lastModified = instant(Tasks.LAST_MODIFIED), diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TaskProjections.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TaskProjections.kt index 6b6fa45..b7464bb 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TaskProjections.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TaskProjections.kt @@ -1,8 +1,6 @@ package de.jeanlucmakiola.agendula.data.tasks -import de.jeanlucmakiola.agendula.data.tasks.TasksContract.Instances import de.jeanlucmakiola.agendula.data.tasks.TasksContract.Lists -import de.jeanlucmakiola.agendula.data.tasks.TasksContract.Tasks /** Column lists requested from the provider. Order is irrelevant; we read by name. */ object TaskProjections { @@ -18,31 +16,9 @@ object TaskProjections { Lists.ACCOUNT_TYPE, ) - /** Read from the `instances` view (inherits all task columns). */ - val INSTANCES: Array = arrayOf( - Tasks.ID, - Instances.TASK_ID, - Tasks.LIST_ID, - Tasks.TITLE, - Tasks.DESCRIPTION, - Tasks.LOCATION, - Tasks.URL, - Tasks.PRIORITY, - Tasks.STATUS, - Tasks.PERCENT_COMPLETE, - Tasks.COMPLETED, - Tasks.IS_ALLDAY, - Tasks.TZ, - Instances.INSTANCE_START, - Instances.INSTANCE_DUE, - Tasks.TASK_COLOR, - Tasks.LIST_COLOR, - Tasks.LIST_NAME, - Tasks.ACCOUNT_NAME, - Tasks.PARENT_ID, - Instances.IS_RECURRING, - Instances.DISTANCE_FROM_CURRENT, - Tasks.CREATED, - Tasks.LAST_MODIFIED, - ) + // No `instances` projection on purpose: that read passes `projection = null` + // (all columns), because the view's shape differs across provider versions — + // tasks.org's bundled OpenTasks has no `is_recurring`, for one. A fixed list + // here would drift out of sync with the by-name mapper and quietly drop + // columns it depends on. See AndroidTasksDataSource.queryInstances. } diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TaskWriteMapper.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TaskWriteMapper.kt index 601ac24..494e78d 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TaskWriteMapper.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TaskWriteMapper.kt @@ -1,9 +1,21 @@ package de.jeanlucmakiola.agendula.data.tasks +import de.jeanlucmakiola.agendula.data.tasks.TasksContract.Alarm import de.jeanlucmakiola.agendula.data.tasks.TasksContract.Lists +import de.jeanlucmakiola.agendula.data.tasks.TasksContract.Properties import de.jeanlucmakiola.agendula.data.tasks.TasksContract.Tasks import de.jeanlucmakiola.agendula.domain.TaskForm import de.jeanlucmakiola.agendula.domain.toICal +import kotlin.time.Instant + +private const val MILLIS_PER_DAY = 24L * 60 * 60 * 1000 + +/** Floor to UTC midnight when [allDay], else pass through unchanged. */ +private fun Instant.forAllDay(allDay: Boolean): Instant = + if (!allDay) this + else Instant.fromEpochMilliseconds( + Math.floorDiv(toEpochMilliseconds(), MILLIS_PER_DAY) * MILLIS_PER_DAY, + ) /** * Turns a [TaskForm] / mutation into a name→value map. Pure (no ContentValues), @@ -39,8 +51,17 @@ object TaskWriteMapper { } } put(Tasks.IS_ALLDAY, if (form.isAllDay) 1 else 0) - put(Tasks.DTSTART, form.start?.toEpochMilliseconds()) - put(Tasks.DUE, form.due?.toEpochMilliseconds()) + // All-day tasks are date-only in iCalendar. The provider reads them back + // through DateTime.toAllDay(), which drops the time-of-day and resolves the + // remaining date against UTC — so a local-midnight instant lands on the + // previous day for anyone west of UTC. Pin all-day values to UTC midnight. + put(Tasks.DTSTART, form.start?.forAllDay(form.isAllDay)?.toEpochMilliseconds()) + put(Tasks.DUE, form.due?.forAllDay(form.isAllDay)?.toEpochMilliseconds()) + // DUE and DURATION are mutually exclusive. The provider's Validating + // processor evaluates the *merged* row (supplied values over the stored + // ones), so writing DUE onto a task that already carries a DURATION throws + // "Only one of DUE or DURATION must be supplied." Clear it alongside. + put(Tasks.DURATION, null) put(Tasks.PARENT_ID, form.parentId) // The provider treats a null tz as local time; set it explicitly for // timed tasks so the stored instant is unambiguous across zones. @@ -48,6 +69,16 @@ object TaskWriteMapper { put(Tasks.TZ, if (timed) tzId else null) } + /** + * Values for an update through the *instances* URI (a recurring occurrence). + * The provider clones the row into an override and strips list/recurrence + * fields as it goes, so LIST_ID and PARENT_ID are dropped here rather than + * written and silently ignored — moving one occurrence between lists or + * parents isn't a thing the override model expresses. + */ + fun instanceValues(form: TaskForm, tzId: String): Map = + taskValues(form, tzId) - Tasks.LIST_ID - Tasks.PARENT_ID + fun completionValues(completed: Boolean, nowMillis: Long): Map = if (completed) { mapOf( @@ -63,6 +94,19 @@ object TaskWriteMapper { ) } + /** + * A reminder for [taskId], as an Alarm property row. The provider's validator + * requires MINUTES_BEFORE, REFERENCE (non-negative) and ALARM_TYPE on every + * write, so all three are always present. + */ + fun alarmValues(taskId: Long, minutesBeforeDue: Int): Map = mapOf( + Properties.TASK_ID to taskId, + Properties.MIMETYPE to Alarm.MIMETYPE, + Alarm.MINUTES_BEFORE to minutesBeforeDue, + Alarm.REFERENCE to Alarm.REFERENCE_DUE, + Alarm.ALARM_TYPE to Alarm.TYPE_MESSAGE, + ) + fun localListValues(name: String, color: Int): Map = mapOf( Lists.NAME to name.trim(), Lists.COLOR to color, diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksContract.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksContract.kt index a400d59..9a25f46 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksContract.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksContract.kt @@ -66,6 +66,9 @@ object TasksContract { const val IS_ALLDAY = "is_allday" const val TZ = "tz" const val RRULE = "rrule" + const val RDATE = "rdate" + /** Set on an override row — the master occurrence this one replaces. */ + const val ORIGINAL_INSTANCE_ID = "original_instance_id" const val PARENT_ID = "parent_id" const val SORTING = "sorting" const val CREATED = "created" @@ -98,6 +101,50 @@ object TasksContract { const val IS_RECURRING = "is_recurring" } + /** The `properties` table — per-task side rows, discriminated by [Properties.MIMETYPE]. */ + object Properties { + const val PATH = "properties" + const val PROPERTY_ID = "property_id" + const val TASK_ID = "task_id" + const val MIMETYPE = "mimetype" + } + + /** + * An alarm property row — a per-task reminder lead. + * + * Storage and sync format *only*: the provider fires nothing (its alarm + * scheduling is commented out and the internal `alarms` table is never + * populated), so [de.jeanlucmakiola.agendula.data.reminders.ReminderScheduler] + * still arms the real AlarmManager alarm. Writing it here is what makes the + * lead survive a sync and show up in other OpenTasks clients. + * + * The columns are the generic `dataN` slots; the meanings below are the + * Alarm property's contract for them. + */ + object Alarm { + const val MIMETYPE = "vnd.android.cursor.item/alarm" + + /** `data0` — minutes from the reference date; positive means *before* it. */ + const val MINUTES_BEFORE = "data0" + + /** `data1` — which date to count from. */ + const val REFERENCE = "data1" + + /** `data2` — optional message shown with the alarm. */ + const val MESSAGE = "data2" + + /** `data3` — alarm kind. Must be present, and non-zero to count as an alarm. */ + const val ALARM_TYPE = "data3" + + const val REFERENCE_DUE = 1 + const val REFERENCE_START = 2 + + /** 0 (NOTHING) is excluded from the provider's `has_alarms` count — use MESSAGE. */ + const val TYPE_MESSAGE = 1 + } + + fun propertiesUri(authority: String): Uri = Uri.parse("content://$authority/${Properties.PATH}") + // --- status values (TaskColumns.STATUS_*) -------------------------------- const val STATUS_NEEDS_ACTION = 0 const val STATUS_IN_PROCESS = 1 @@ -112,6 +159,15 @@ object TasksContract { fun tasksUri(authority: String): Uri = Uri.parse("content://$authority/${Tasks.PATH}") fun instancesUri(authority: String): Uri = Uri.parse("content://$authority/${Instances.PATH}") + /** + * A single occurrence. Updating through this URI is how a *recurring* task is + * edited: the provider clones the row into an override task + * (`original_instance_id` set, recurrence fields stripped) instead of moving + * the series anchor, which is what writing to `tasks/` would do. + */ + fun instanceUri(authority: String, instanceId: Long): Uri = + Uri.parse("content://$authority/${Instances.PATH}/$instanceId") + /** Append the sync-adapter params required to write local-account rows. */ fun asSyncAdapter(uri: Uri, accountName: String, accountType: String): Uri = uri.buildUpon() diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksDataSource.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksDataSource.kt index e73d7ee..6054f09 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksDataSource.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksDataSource.kt @@ -23,6 +23,25 @@ interface TasksDataSource { fun insertTask(form: TaskForm): Long fun updateTask(taskId: Long, form: TaskForm) + + /** + * Update a single occurrence of a recurring task, addressed by its *instance* + * row id. The provider forks an override task rather than moving the series + * anchor — which is what [updateTask] would do, since a recurring task's + * start/due are read from the instances view. + */ + fun updateInstance(instanceId: Long, form: TaskForm) + /** + * Set (or clear, with `null`) the per-task reminder lead, stored as an Alarm + * property row. The provider never fires it — [de.jeanlucmakiola.agendula + * .data.reminders.ReminderScheduler] does — but persisting it here is what + * syncs the lead and shares it with other OpenTasks clients. + */ + fun setAlarm(taskId: Long, minutesBeforeDue: Int?) + + /** Every task's reminder lead, by task id. One query, for the scheduler. */ + fun alarms(): Map + fun setCompleted(taskId: Long, completed: Boolean) fun deleteTask(taskId: Long) fun createLocalList(name: String, color: Int): Long diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksRepository.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksRepository.kt index 10efd6f..3333e6c 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksRepository.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksRepository.kt @@ -39,6 +39,13 @@ interface TasksRepository { suspend fun updateTask(taskId: Long, form: TaskForm, expectedLastModified: Instant? = null) suspend fun setCompleted(taskId: Long, completed: Boolean) suspend fun deleteTask(taskId: Long) + + /** + * The per-task reminder lead in minutes before due, or `null` if the task has + * none (in which case the list's / global setting applies). Read when the edit + * form loads so saving can't silently drop it. + */ + suspend fun reminderFor(taskId: Long): Int? suspend fun createLocalList(name: String, color: Int): Long /** Synchronous snapshot for the permission/onboarding gate. */ diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksRepositoryImpl.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksRepositoryImpl.kt index d137d1e..8ac224e 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksRepositoryImpl.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksRepositoryImpl.kt @@ -80,18 +80,39 @@ class TasksRepositoryImpl @Inject constructor( } override suspend fun createTask(form: TaskForm): Long = - withContext(io) { dataSource.insertTask(form) } + withContext(io) { + val id = dataSource.insertTask(form) + form.reminderMinutesBeforeDue?.let { dataSource.setAlarm(id, it) } + id + } + + override suspend fun reminderFor(taskId: Long): Int? = + withContext(io) { runCatching { dataSource.alarms()[taskId] }.getOrNull() } override suspend fun updateTask(taskId: Long, form: TaskForm, expectedLastModified: Instant?) = withContext(io) { - // Conflict-safe overwrite: re-read just before writing and bail if the - // provider's last_modified moved since the form captured it (external - // sync / another app). A null baseline means "force / overwrite anyway". + // Re-read just before writing: it settles the conflict check *and* tells + // us which URI to write through. + val current = dataSource.task(taskId) + // Conflict-safe overwrite: bail if the provider's last_modified moved + // since the form captured it (external sync / another app). A null + // baseline means "force / overwrite anyway". if (expectedLastModified != null) { - val current = dataSource.task(taskId)?.lastModified - if (current != null && current != expectedLastModified) throw TaskConflictException(taskId) + val seen = current?.lastModified + if (seen != null && seen != expectedLastModified) throw TaskConflictException(taskId) + } + // Write the reminder first: forking a recurring occurrence copies the + // task's properties onto the new override row, so setting the alarm + // beforehand is what carries it across. + dataSource.setAlarm(taskId, form.reminderMinutesBeforeDue) + // A recurring task's start/due come from the instances view, so writing + // them back to tasks/ would re-anchor the whole series. Going through + // the occurrence lets the provider fork an override instead. + if (current != null && current.isRecurring) { + dataSource.updateInstance(current.id, form) + } else { + dataSource.updateTask(taskId, form) } - dataSource.updateTask(taskId, form) } override suspend fun setCompleted(taskId: Long, completed: Boolean) = diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/domain/AllDayTime.kt b/app/src/main/java/de/jeanlucmakiola/agendula/domain/AllDayTime.kt new file mode 100644 index 0000000..e86fca0 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/agendula/domain/AllDayTime.kt @@ -0,0 +1,42 @@ +package de.jeanlucmakiola.agendula.domain + +import java.time.ZoneId +import java.time.ZoneOffset +import kotlin.time.Instant + +/** + * All-day tasks are date-only in iCalendar. OpenTasks reads them back through + * `DateTime.toAllDay()`, which discards the time-of-day and resolves the + * remaining date against UTC — so the storage convention is **UTC midnight of + * the intended calendar date, with a null timezone**. Timed tasks, by contrast, + * are ordinary instants rendered in the device's zone. + * + * These two conventions disagree about which day a given instant is, which is + * why every all-day value needs an explicit conversion rather than a raw + * `Instant` passed straight through. + */ + +/** UTC midnight of [date] — the storage form for an all-day value. */ +fun allDayInstantOf(date: java.time.LocalDate): Instant = + Instant.fromEpochMilliseconds(date.atStartOfDay(ZoneOffset.UTC).toInstant().toEpochMilli()) + +/** + * The calendar date this instant denotes: read in UTC for [allDay] values, + * in [zone] for timed ones. + */ +fun Instant.calendarDate(allDay: Boolean, zone: ZoneId = ZoneId.systemDefault()): java.time.LocalDate = + java.time.Instant.ofEpochMilli(toEpochMilliseconds()) + .atZone(if (allDay) ZoneOffset.UTC else zone) + .toLocalDate() + +/** + * Move an instant across the two conventions when the all-day switch flips, so + * the day the user is looking at stays put. Without this, toggling all-day off + * turns a UTC-midnight value into "02:00" in Berlin (or the previous day, 19:00, + * in New York) — reading to the user as "the time reset itself". + */ +fun Instant.rebasedForAllDay(allDay: Boolean, zone: ZoneId = ZoneId.systemDefault()): Instant = + if (allDay) allDayInstantOf(calendarDate(allDay = false, zone = zone)) + else Instant.fromEpochMilliseconds( + calendarDate(allDay = true).atStartOfDay(zone).toInstant().toEpochMilli(), + ) diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/domain/Models.kt b/app/src/main/java/de/jeanlucmakiola/agendula/domain/Models.kt index 47e8e68..e19e9cc 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/domain/Models.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/domain/Models.kt @@ -48,6 +48,12 @@ data class Task( val listName: String?, val accountName: String?, val parentId: Long?, + /** + * This row carries a recurrence rule, so [id] is one occurrence of a series + * and [start]/[due] are that occurrence's resolved times — *not* the master's + * anchor. Edits must go through the instances URI (see + * [de.jeanlucmakiola.agendula.data.tasks.TasksContract.instanceUri]). + */ val isRecurring: Boolean, val distanceFromCurrent: Int?, val created: Instant?, diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/RootScreen.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/RootScreen.kt index e481bd7..fc41b08 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/ui/RootScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/RootScreen.kt @@ -11,12 +11,16 @@ import androidx.compose.material3.Button import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable +import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.getValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.res.stringResource import androidx.compose.ui.unit.dp import androidx.hilt.navigation.compose.hiltViewModel +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleEventObserver +import androidx.lifecycle.compose.LocalLifecycleOwner import androidx.lifecycle.compose.collectAsStateWithLifecycle import de.jeanlucmakiola.agendula.R import de.jeanlucmakiola.agendula.data.tasks.ProviderStatus @@ -39,6 +43,18 @@ fun RootScreen( ActivityResultContracts.RequestMultiplePermissions(), ) { permissionViewModel.refresh() } + // Re-check on every resume, not just after the in-app request: the user may + // have granted the permission (or installed a provider) in system Settings and + // come back, and otherwise the gate would hold until the process restarts. + val lifecycleOwner = LocalLifecycleOwner.current + DisposableEffect(lifecycleOwner) { + val observer = LifecycleEventObserver { _, event -> + if (event == Lifecycle.Event.ON_RESUME) permissionViewModel.refresh() + } + lifecycleOwner.lifecycle.addObserver(observer) + onDispose { lifecycleOwner.lifecycle.removeObserver(observer) } + } + when (permission.status) { ProviderStatus.NO_PROVIDER -> Gate( modifier = modifier, diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/common/DateTimeField.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/common/DateTimeField.kt deleted file mode 100644 index 493f41f..0000000 --- a/app/src/main/java/de/jeanlucmakiola/agendula/ui/common/DateTimeField.kt +++ /dev/null @@ -1,167 +0,0 @@ -package de.jeanlucmakiola.agendula.ui.common - -import de.jeanlucmakiola.floret.time.formatDateTime -import androidx.compose.foundation.layout.Arrangement -import androidx.compose.foundation.layout.Column -import androidx.compose.foundation.layout.Row -import androidx.compose.foundation.layout.fillMaxWidth -import androidx.compose.foundation.layout.padding -import androidx.compose.foundation.shape.RoundedCornerShape -import androidx.compose.material.icons.Icons -import androidx.compose.material.icons.rounded.Clear -import androidx.compose.material.icons.rounded.Event -import androidx.compose.material3.DatePicker -import androidx.compose.material3.DatePickerDialog -import androidx.compose.material3.ExperimentalMaterial3Api -import androidx.compose.material3.Icon -import androidx.compose.material3.IconButton -import androidx.compose.material3.MaterialTheme -import androidx.compose.material3.Surface -import androidx.compose.material3.Text -import androidx.compose.material3.TextButton -import androidx.compose.material3.TimePicker -import androidx.compose.material3.rememberDatePickerState -import androidx.compose.material3.rememberTimePickerState -import androidx.compose.runtime.Composable -import androidx.compose.runtime.getValue -import androidx.compose.runtime.mutableStateOf -import androidx.compose.runtime.remember -import androidx.compose.runtime.setValue -import androidx.compose.ui.Alignment -import androidx.compose.ui.Modifier -import androidx.compose.ui.res.stringResource -import androidx.compose.ui.unit.dp -import androidx.compose.ui.window.Dialog -import de.jeanlucmakiola.agendula.R -import java.time.LocalDate -import java.time.LocalTime -import java.time.ZoneId -import java.time.ZoneOffset -import kotlin.time.Instant - -private val zone: ZoneId get() = ZoneId.systemDefault() - -internal fun Instant.toLocalDate(): LocalDate = - java.time.Instant.ofEpochMilli(toEpochMilliseconds()).atZone(zone).toLocalDate() - -internal fun Instant.toLocalTime(): LocalTime = - java.time.Instant.ofEpochMilli(toEpochMilliseconds()).atZone(zone).toLocalTime() - -internal fun localToInstant(date: LocalDate, time: LocalTime): Instant = - Instant.fromEpochMilliseconds(date.atTime(time).atZone(zone).toInstant().toEpochMilli()) - -/** - * A labelled date(-time) field for the edit form: a tonal row showing the - * current value (or nothing), tappable to pick a date and — unless [allDay] — - * a time. A clear affordance appears once a value is set. Emits `null` when - * cleared. Styled to match the app's rounded tonal family. - */ -@OptIn(ExperimentalMaterial3Api::class) -@Composable -fun DateTimeField( - label: String, - value: Instant?, - allDay: Boolean, - onChange: (Instant?) -> Unit, - modifier: Modifier = Modifier, -) { - var showDatePicker by remember { mutableStateOf(false) } - var showTimePicker by remember { mutableStateOf(false) } - var pendingDate by remember { mutableStateOf(null) } - - Surface( - onClick = { showDatePicker = true }, - shape = RoundedCornerShape(22.dp), - color = MaterialTheme.colorScheme.surfaceContainerHigh, - modifier = modifier.fillMaxWidth(), - ) { - Row( - modifier = Modifier.padding(horizontal = 20.dp, vertical = 14.dp), - verticalAlignment = Alignment.CenterVertically, - horizontalArrangement = Arrangement.spacedBy(12.dp), - ) { - Icon(Icons.Rounded.Event, contentDescription = null, tint = MaterialTheme.colorScheme.onSurfaceVariant) - Column(modifier = Modifier.weight(1f)) { - Text( - text = label, - style = MaterialTheme.typography.labelMedium, - color = MaterialTheme.colorScheme.onSurfaceVariant, - ) - Text( - text = value?.formatDateTime(allDay) ?: stringResource(R.string.edit_set), - style = MaterialTheme.typography.bodyLarge, - ) - } - if (value != null) { - IconButton(onClick = { onChange(null) }) { - Icon(Icons.Rounded.Clear, contentDescription = stringResource(R.string.edit_clear)) - } - } - } - } - - if (showDatePicker) { - val initialMillis = (value ?: Instant.fromEpochMilliseconds(System.currentTimeMillis())) - .toLocalDate().atStartOfDay(ZoneOffset.UTC).toInstant().toEpochMilli() - val dateState = rememberDatePickerState(initialSelectedDateMillis = initialMillis) - DatePickerDialog( - onDismissRequest = { showDatePicker = false }, - confirmButton = { - TextButton( - onClick = { - showDatePicker = false - val millis = dateState.selectedDateMillis ?: return@TextButton - val date = java.time.Instant.ofEpochMilli(millis) - .atZone(ZoneOffset.UTC).toLocalDate() - if (allDay) { - onChange(localToInstant(date, LocalTime.MIDNIGHT)) - } else { - pendingDate = date - showTimePicker = true - } - }, - ) { Text(stringResource(android.R.string.ok)) } - }, - dismissButton = { - TextButton(onClick = { showDatePicker = false }) { - Text(stringResource(android.R.string.cancel)) - } - }, - ) { DatePicker(state = dateState) } - } - - if (showTimePicker) { - val base = value ?: Instant.fromEpochMilliseconds(System.currentTimeMillis()) - val timeState = rememberTimePickerState( - initialHour = base.toLocalTime().hour, - initialMinute = base.toLocalTime().minute, - ) - Dialog(onDismissRequest = { showTimePicker = false }) { - Surface( - shape = RoundedCornerShape(28.dp), - color = MaterialTheme.colorScheme.surfaceContainerHigh, - ) { - Column( - modifier = Modifier.padding(24.dp), - horizontalAlignment = Alignment.CenterHorizontally, - verticalArrangement = Arrangement.spacedBy(16.dp), - ) { - TimePicker(state = timeState) - Row( - modifier = Modifier.fillMaxWidth(), - horizontalArrangement = Arrangement.End, - ) { - TextButton(onClick = { showTimePicker = false }) { - Text(stringResource(android.R.string.cancel)) - } - TextButton(onClick = { - showTimePicker = false - val date = pendingDate ?: return@TextButton - onChange(localToInstant(date, LocalTime.of(timeState.hour, timeState.minute))) - }) { Text(stringResource(android.R.string.ok)) } - } - } - } - } - } -} diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/common/PickerTime.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/common/PickerTime.kt new file mode 100644 index 0000000..be77de3 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/common/PickerTime.kt @@ -0,0 +1,20 @@ +package de.jeanlucmakiola.agendula.ui.common + +import java.time.LocalDate +import java.time.LocalTime +import java.time.ZoneId +import kotlin.time.Instant + +/** + * Zone helpers shared by the date/time pickers. All-day conversions live in + * [de.jeanlucmakiola.agendula.domain.AllDayTime] — these cover the timed case, + * where the device zone is the right frame of reference. + */ + +private val zone: ZoneId get() = ZoneId.systemDefault() + +internal fun Instant.toLocalTime(): LocalTime = + java.time.Instant.ofEpochMilli(toEpochMilliseconds()).atZone(zone).toLocalTime() + +internal fun localToInstant(date: LocalDate, time: LocalTime): Instant = + Instant.fromEpochMilliseconds(date.atTime(time).atZone(zone).toInstant().toEpochMilli()) diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/detail/TaskDetailScreen.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/detail/TaskDetailScreen.kt index f1ae23a..15b37a1 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/ui/detail/TaskDetailScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/detail/TaskDetailScreen.kt @@ -501,13 +501,16 @@ private fun taskWhenLines(task: Task): Pair? { val due = task.due return when { start != null && due != null -> { - val sameDay = start.formatDate() == due.formatDate() - val primary = if (sameDay) due.formatDate() else "${start.formatDate()} – ${due.formatDate()}" - val secondary = if (task.isAllDay) null else "${start.formatTime()} – ${due.formatTime()}" + val allDay = task.isAllDay + val sameDay = start.formatDate(allDay) == due.formatDate(allDay) + val primary = + if (sameDay) due.formatDate(allDay) + else "${start.formatDate(allDay)} – ${due.formatDate(allDay)}" + val secondary = if (allDay) null else "${start.formatTime()} – ${due.formatTime()}" primary to secondary } - due != null -> due.formatDate() to if (task.isAllDay) null else due.formatTime() - start != null -> start.formatDate() to if (task.isAllDay) null else start.formatTime() + due != null -> due.formatDate(task.isAllDay) to if (task.isAllDay) null else due.formatTime() + start != null -> start.formatDate(task.isAllDay) to if (task.isAllDay) null else start.formatTime() else -> null } } diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/detail/TaskDetailViewModel.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/detail/TaskDetailViewModel.kt index 2e48a3a..98db1a4 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/ui/detail/TaskDetailViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/detail/TaskDetailViewModel.kt @@ -4,6 +4,7 @@ import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel import de.jeanlucmakiola.agendula.data.tasks.TasksRepository +import de.jeanlucmakiola.agendula.data.tasks.recoveringFromProviderFailure import de.jeanlucmakiola.agendula.domain.Task import de.jeanlucmakiola.agendula.domain.TaskDetail import de.jeanlucmakiola.agendula.domain.TaskForm @@ -11,7 +12,6 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow -import kotlinx.coroutines.flow.catch import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.map @@ -42,7 +42,7 @@ class TaskDetailViewModel @Inject constructor( if (detail == null) TaskDetailUiState.NotFound else TaskDetailUiState.Content(detail) } .onStart { emit(TaskDetailUiState.Loading) } - .catch { emit(TaskDetailUiState.NotFound) } + .recoveringFromProviderFailure { TaskDetailUiState.NotFound } } .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), TaskDetailUiState.Loading) diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/edit/TaskEditScreen.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/edit/TaskEditScreen.kt index 5d09499..570ca19 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/ui/edit/TaskEditScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/edit/TaskEditScreen.kt @@ -99,7 +99,8 @@ import de.jeanlucmakiola.floret.time.formatTime import de.jeanlucmakiola.agendula.ui.common.localToInstant import de.jeanlucmakiola.floret.components.pastelize import de.jeanlucmakiola.floret.components.positionOf -import de.jeanlucmakiola.agendula.ui.common.toLocalDate +import de.jeanlucmakiola.agendula.domain.allDayInstantOf +import de.jeanlucmakiola.agendula.domain.calendarDate import de.jeanlucmakiola.agendula.ui.common.toLocalTime import de.jeanlucmakiola.agendula.ui.tasklist.priorityLabel import java.time.LocalTime @@ -178,7 +179,7 @@ private fun EditContent( val accent = selectedList?.let { pastelize(it.color, dark) } ?: MaterialTheme.colorScheme.primary val gap = 12.dp - var pickerTarget by remember { mutableStateOf(null) } + var pickerTarget by rememberSaveable { mutableStateOf(null) } var showListPicker by rememberSaveable { mutableStateOf(false) } var showParentPicker by rememberSaveable { mutableStateOf(false) } var showReminderPicker by rememberSaveable { mutableStateOf(false) } @@ -653,7 +654,7 @@ private fun ScheduleRow( ) } else { Text( - text = value.formatDate(), + text = value.formatDate(allDay), style = MaterialTheme.typography.titleMedium, color = valueColor, modifier = Modifier.clickable(onClick = onPick).padding(vertical = 8.dp, horizontal = 6.dp), @@ -689,12 +690,15 @@ private fun DateTimePickerFlow( onResult: (Instant) -> Unit, onDismiss: () -> Unit, ) { - var pendingDate by remember { mutableStateOf(null) } - var showTime by remember { mutableStateOf(false) } + var pendingDate by rememberSaveable { mutableStateOf(null) } + var showTime by rememberSaveable { mutableStateOf(false) } if (!showTime) { + // M3's DatePicker speaks UTC millis. An all-day value is already UTC-based, + // a timed one is read in the device zone — calendarDate picks the right frame + // so the dialog opens on the day the rest of the UI shows. val initialMillis = (initial ?: nowInstant()) - .toLocalDate().atStartOfDay(ZoneOffset.UTC).toInstant().toEpochMilli() + .calendarDate(allDay).atStartOfDay(ZoneOffset.UTC).toInstant().toEpochMilli() val dateState = rememberDatePickerState(initialSelectedDateMillis = initialMillis) DatePickerDialog( onDismissRequest = onDismiss, @@ -703,7 +707,7 @@ private fun DateTimePickerFlow( val millis = dateState.selectedDateMillis ?: run { onDismiss(); return@TextButton } val date = java.time.Instant.ofEpochMilli(millis).atZone(ZoneOffset.UTC).toLocalDate() if (allDay) { - onResult(localToInstant(date, LocalTime.MIDNIGHT)) + onResult(allDayInstantOf(date)) } else { pendingDate = date showTime = true @@ -865,7 +869,7 @@ private fun ParentPickerSheet( GroupedRow( title = task.title.ifBlank { stringResource(R.string.task_untitled) }, position = positionOf(index, section.tasks.size), - summary = task.due?.formatDate(), + summary = task.due?.formatDate(task.isAllDay), selected = task.taskId == selectedId, minHeight = 56.dp, onClick = { choose(task.taskId) }, diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/edit/TaskEditViewModel.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/edit/TaskEditViewModel.kt index d25d0e5..03d5e63 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/ui/edit/TaskEditViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/edit/TaskEditViewModel.kt @@ -15,6 +15,7 @@ import de.jeanlucmakiola.agendula.domain.TaskFormError import de.jeanlucmakiola.agendula.domain.TaskFormField import de.jeanlucmakiola.agendula.domain.TaskList import de.jeanlucmakiola.agendula.domain.populatedFields +import de.jeanlucmakiola.agendula.domain.rebasedForAllDay import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow @@ -70,6 +71,15 @@ class TaskEditViewModel @Inject constructor( private var editingTaskId: Long? = null + /** + * Whether the form has already been populated. The host `LaunchedEffect` + * re-fires whenever the composition restarts — an Activity recreation + * (rotation, theme/font/display-size change, split-screen, unfolding) — while + * this ViewModel survives on the nav back stack. Without this guard the + * rebind would overwrite in-progress edits with the untouched provider row. + */ + private var bound = false + /** `last_modified` captured when the form loaded — the conflict-check baseline. */ private var baselineLastModified: Instant? = null @@ -78,6 +88,8 @@ class TaskEditViewModel @Inject constructor( /** Start a fresh task, optionally pre-selecting a list / parent. */ fun bindNew(presetListId: Long? = null, parentId: Long? = null) { + if (bound) return + bound = true editingTaskId = null baselineLastModified = null viewModelScope.launch { @@ -103,6 +115,8 @@ class TaskEditViewModel @Inject constructor( /** Load an existing task for editing. */ fun bindEdit(taskId: Long) { + if (bound && editingTaskId == taskId) return + bound = true editingTaskId = taskId viewModelScope.launch { defaultFields = settingsPrefs.settings.first().defaultEditFields @@ -126,6 +140,7 @@ class TaskEditViewModel @Inject constructor( priority = task.priority, parentId = task.parentId, percentComplete = task.percentComplete, + reminderMinutesBeforeDue = repository.reminderFor(taskId), lists = lists, parentCandidates = loadParents(task.listId, selfId = taskId), ), @@ -178,7 +193,19 @@ class TaskEditViewModel @Inject constructor( fun onStartChange(value: Instant?) = update { it.copy(start = value) } fun onDueChange(value: Instant?) = update { it.copy(due = value) } - fun onAllDayChange(value: Boolean) = update { it.copy(isAllDay = value) } + /** + * All-day and timed values use different conventions (UTC midnight vs. a real + * instant in the device zone), so the switch has to move the timestamps too — + * flipping the flag alone makes an all-day task read back as "02:00", which + * looks to the user like the time reset itself. + */ + fun onAllDayChange(value: Boolean) = update { + it.copy( + isAllDay = value, + start = it.start?.rebasedForAllDay(value), + due = it.due?.rebasedForAllDay(value), + ) + } fun onPriorityChange(value: Priority) = update { it.copy(priority = value) } fun onPercentChange(value: Int?) = update { it.copy(percentComplete = value?.coerceIn(0, 100)) } fun onParentChange(parentId: Long?) = update { it.copy(parentId = parentId) } diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/lists/ListsViewModel.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/lists/ListsViewModel.kt index a6d5071..d6b8aeb 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/ui/lists/ListsViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/lists/ListsViewModel.kt @@ -4,6 +4,7 @@ import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel import de.jeanlucmakiola.agendula.data.tasks.TasksRepository +import de.jeanlucmakiola.agendula.data.tasks.recoveringFromProviderFailure import de.jeanlucmakiola.floret.time.DayWindow import de.jeanlucmakiola.agendula.domain.SmartList import de.jeanlucmakiola.agendula.domain.Task @@ -12,7 +13,6 @@ import de.jeanlucmakiola.agendula.domain.TaskFiltering import de.jeanlucmakiola.agendula.domain.TaskList import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow -import kotlinx.coroutines.flow.catch import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.stateIn import java.time.ZoneId @@ -56,7 +56,7 @@ class ListsViewModel @Inject constructor( repository.tasks(TaskFilter.Smart(SmartList.COMPLETED)), ) { lists, openTasks, completedTasks -> buildContent(lists, openTasks, completedTasks) as ListsUiState - }.catch { emit(ListsUiState.Failure) } + }.recoveringFromProviderFailure { ListsUiState.Failure } .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), ListsUiState.Loading) private fun buildContent( diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/settings/SettingsViewModel.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/settings/SettingsViewModel.kt index aa2fb24..e8d0614 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/ui/settings/SettingsViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/settings/SettingsViewModel.kt @@ -7,12 +7,12 @@ import de.jeanlucmakiola.agendula.data.prefs.Settings import de.jeanlucmakiola.agendula.data.prefs.SettingsPrefs import de.jeanlucmakiola.agendula.data.prefs.ThemeMode import de.jeanlucmakiola.agendula.data.tasks.TasksRepository +import de.jeanlucmakiola.agendula.data.tasks.recoveringFromProviderFailure import de.jeanlucmakiola.agendula.domain.TaskFormField import de.jeanlucmakiola.agendula.domain.TaskList import de.jeanlucmakiola.floret.reminders.ReminderOverride import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow -import kotlinx.coroutines.flow.catch import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch @@ -33,8 +33,14 @@ class SettingsViewModel @Inject constructor( repository: TasksRepository, ) : ViewModel() { + // MainActivity collects this for the theme, above the permission gate and for + // the whole Activity lifetime — so the list flow must survive the pre-grant + // SecurityException and recover once permission is given, not die for good. val state: StateFlow = - combine(prefs.settings, repository.taskLists().catch { emit(emptyList()) }) { settings, lists -> + combine( + prefs.settings, + repository.taskLists().recoveringFromProviderFailure { emptyList() }, + ) { settings, lists -> SettingsUiState(settings, lists) }.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), SettingsUiState()) diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/tasklist/TaskListViewModel.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/tasklist/TaskListViewModel.kt index 3120e14..4287b7e 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/ui/tasklist/TaskListViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/tasklist/TaskListViewModel.kt @@ -5,6 +5,7 @@ import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel import de.jeanlucmakiola.agendula.data.prefs.SettingsPrefs import de.jeanlucmakiola.agendula.data.tasks.TasksRepository +import de.jeanlucmakiola.agendula.data.tasks.recoveringFromProviderFailure import de.jeanlucmakiola.agendula.domain.Task import de.jeanlucmakiola.agendula.domain.TaskFilter import de.jeanlucmakiola.agendula.domain.TaskForm @@ -12,7 +13,6 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow -import kotlinx.coroutines.flow.catch import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.flow.flatMapLatest @@ -87,7 +87,10 @@ class TaskListViewModel @Inject constructor( } } .onStart { emit(TaskListUiState.Loading) } - .catch { emit(TaskListUiState.Failure) } + // Recover rather than terminate: a provider hiccup (mid-update, + // permission not yet granted) shows Failure but keeps retrying, + // so the screen heals itself instead of staying stuck. + .recoveringFromProviderFailure { TaskListUiState.Failure } } .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), TaskListUiState.Loading) @@ -112,6 +115,9 @@ class TaskListViewModel @Inject constructor( combine(ids.map { id -> repository.subtasks(id).map { id to it } }) { it.toMap() } } } + // Without this an exception here escapes stateIn's coroutine, past + // viewModelScope's SupervisorJob, and crashes the process. + .recoveringFromProviderFailure { emptyMap() } .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), emptyMap()) /** The screen reports which expanded parents need their children fetched. */ 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 7d70aa9..1876328 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 @@ -50,6 +50,31 @@ class TaskMapperTest { assertThat(task.isSubtask).isTrue() } + @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 `recurrence is detected from rdate alone`() { + val task = TaskMapper.task( + MapColumnReader(mapOf(Tasks.ID to 1L, Tasks.RDATE to "20260720T090000Z")), + ) + assertThat(task.isRecurring).isTrue() + } + + @Test + fun `a plain task is not recurring`() { + val task = TaskMapper.task(MapColumnReader(mapOf(Tasks.ID to 1L, Tasks.TITLE to "One-off"))) + assertThat(task.isRecurring).isFalse() + } + @Test fun `falls back to instance id when task_id missing, and list color when no task color`() { val task = TaskMapper.task( diff --git a/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/TaskWriteMapperTest.kt b/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/TaskWriteMapperTest.kt index faa5034..4ec590e 100644 --- a/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/TaskWriteMapperTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/TaskWriteMapperTest.kt @@ -85,6 +85,57 @@ class TaskWriteMapperTest { assertThat(values[Tasks.TZ]).isNull() } + @Test + fun `all-day timestamps are pinned to UTC midnight`() { + // 2026-07-20T22:00Z — i.e. local midnight on the 21st in Berlin (UTC+2). + // The provider resolves all-day dates against UTC, so storing this as-is + // would land the task on the 20th for anyone reading it back. + val berlinMidnight = Instant.fromEpochMilliseconds(1_784_412_000_000L) + val values = TaskWriteMapper.taskValues( + TaskForm(title = "Holiday", listId = 1L, start = berlinMidnight, due = berlinMidnight, isAllDay = true), + tzId = "Europe/Berlin", + ) + + val dayMs = 24L * 60 * 60 * 1000 + assertThat(values[Tasks.DUE] as Long % dayMs).isEqualTo(0L) + assertThat(values[Tasks.DTSTART] as Long % dayMs).isEqualTo(0L) + } + + @Test + fun `timed timestamps are written untouched`() { + val at = Instant.fromEpochMilliseconds(1_784_412_345_678L) + val values = TaskWriteMapper.taskValues( + TaskForm(title = "Standup", listId = 1L, start = at, due = at), + tzId = "Europe/Berlin", + ) + assertThat(values[Tasks.DTSTART]).isEqualTo(1_784_412_345_678L) + assertThat(values[Tasks.DUE]).isEqualTo(1_784_412_345_678L) + } + + @Test + fun `duration is always cleared so it cannot collide with due`() { + // The provider validates the *merged* row and throws "Only one of DUE or + // DURATION must be supplied" if the stored row still carries a duration. + val values = TaskWriteMapper.taskValues( + TaskForm(title = "x", listId = 1L, due = Instant.fromEpochMilliseconds(5_000L)), + tzId = "UTC", + ) + assertThat(values.containsKey(Tasks.DURATION)).isTrue() + assertThat(values[Tasks.DURATION]).isNull() + } + + @Test + fun `instance values drop list and parent, which an override cannot express`() { + val form = TaskForm(title = "x", listId = 4L, parentId = 7L, due = Instant.fromEpochMilliseconds(1_000L)) + val values = TaskWriteMapper.instanceValues(form, tzId = "UTC") + + assertThat(values.containsKey(Tasks.LIST_ID)).isFalse() + assertThat(values.containsKey(Tasks.PARENT_ID)).isFalse() + // …but still carries the edit itself. + assertThat(values[Tasks.TITLE]).isEqualTo("x") + assertThat(values[Tasks.DUE]).isEqualTo(1_000L) + } + @Test fun `completion sets status, percent and timestamp, un-completion clears them`() { val done = TaskWriteMapper.completionValues(completed = true, nowMillis = 999L) @@ -97,6 +148,22 @@ class TaskWriteMapperTest { assertThat(undone[Tasks.COMPLETED]).isNull() } + @Test + fun `alarm carries every column the provider's validator demands`() { + val values = TaskWriteMapper.alarmValues(taskId = 12L, minutesBeforeDue = 30) + + assertThat(values[TasksContract.Properties.TASK_ID]).isEqualTo(12L) + assertThat(values[TasksContract.Properties.MIMETYPE]) + .isEqualTo("vnd.android.cursor.item/alarm") + assertThat(values[TasksContract.Alarm.MINUTES_BEFORE]).isEqualTo(30) + // REFERENCE must be present and non-negative, ALARM_TYPE present and + // non-zero (0 is excluded from the provider's has_alarms count). + assertThat(values[TasksContract.Alarm.REFERENCE]).isEqualTo(TasksContract.Alarm.REFERENCE_DUE) + assertThat(values[TasksContract.Alarm.ALARM_TYPE]).isEqualTo(TasksContract.Alarm.TYPE_MESSAGE) + // property_id must be absent or the insert is rejected. + assertThat(values.containsKey(TasksContract.Properties.PROPERTY_ID)).isFalse() + } + @Test fun `local list uses the LOCAL account`() { val values = TaskWriteMapper.localListValues("Inbox", 0x123) diff --git a/app/src/test/java/de/jeanlucmakiola/agendula/domain/AllDayTimeTest.kt b/app/src/test/java/de/jeanlucmakiola/agendula/domain/AllDayTimeTest.kt new file mode 100644 index 0000000..957192f --- /dev/null +++ b/app/src/test/java/de/jeanlucmakiola/agendula/domain/AllDayTimeTest.kt @@ -0,0 +1,60 @@ +package de.jeanlucmakiola.agendula.domain + +import com.google.common.truth.Truth.assertThat +import org.junit.jupiter.api.Test +import java.time.LocalDate +import java.time.ZoneId +import kotlin.time.Instant + +class AllDayTimeTest { + + private val berlin = ZoneId.of("Europe/Berlin") // UTC+2 in July + private val newYork = ZoneId.of("America/New_York") // UTC-4 in July + private val julyTwentieth = LocalDate.of(2026, 7, 20) + + @Test + fun `an all-day instant is UTC midnight of its date`() { + val instant = allDayInstantOf(julyTwentieth) + assertThat(instant.toEpochMilliseconds() % (24L * 60 * 60 * 1000)).isEqualTo(0L) + assertThat(instant.calendarDate(allDay = true)).isEqualTo(julyTwentieth) + } + + @Test + fun `an all-day date reads the same everywhere, unlike a timed one`() { + val allDay = allDayInstantOf(julyTwentieth) + // The whole point: zone must not change which day an all-day value denotes. + assertThat(allDay.calendarDate(allDay = true, zone = berlin)).isEqualTo(julyTwentieth) + assertThat(allDay.calendarDate(allDay = true, zone = newYork)).isEqualTo(julyTwentieth) + // Read as a timed value in New York it would slip to the 19th — the bug. + assertThat(allDay.calendarDate(allDay = false, zone = newYork)).isEqualTo(julyTwentieth.minusDays(1)) + } + + @Test + fun `toggling all-day off keeps the day and lands on local midnight`() { + val allDay = allDayInstantOf(julyTwentieth) + val timed = allDay.rebasedForAllDay(allDay = false, zone = berlin) + + assertThat(timed.calendarDate(allDay = false, zone = berlin)).isEqualTo(julyTwentieth) + val local = java.time.Instant.ofEpochMilli(timed.toEpochMilliseconds()).atZone(berlin) + assertThat(local.toLocalTime()).isEqualTo(java.time.LocalTime.MIDNIGHT) + } + + @Test + fun `toggling all-day on keeps the day the user was looking at`() { + // 2026-07-20T23:30 in Berlin — late enough that a naive UTC read slips a day. + val lateEvening = Instant.fromEpochMilliseconds( + julyTwentieth.atTime(23, 30).atZone(berlin).toInstant().toEpochMilli(), + ) + val allDay = lateEvening.rebasedForAllDay(allDay = true, zone = berlin) + + assertThat(allDay.calendarDate(allDay = true)).isEqualTo(julyTwentieth) + } + + @Test + fun `round-tripping the toggle is stable`() { + val original = allDayInstantOf(julyTwentieth) + val there = original.rebasedForAllDay(allDay = false, zone = newYork) + val back = there.rebasedForAllDay(allDay = true, zone = newYork) + assertThat(back).isEqualTo(original) + } +} diff --git a/floret-kit b/floret-kit index 5a576c4..396e538 160000 --- a/floret-kit +++ b/floret-kit @@ -1 +1 @@ -Subproject commit 5a576c4d288e469a75c396b365e6ace8290aa3d0 +Subproject commit 396e5389032bbd5f16d95fb997c1e497df780621