Compare commits
5 Commits
70bae53152
...
0f70123804
| Author | SHA1 | Date | |
|---|---|---|---|
| 0f70123804 | |||
| 009bd088e6 | |||
| b16eb770c9 | |||
| 9b2e176247 | |||
| ed99d61c41 |
@@ -47,9 +47,9 @@ class CalendulaApp : Application() {
|
|||||||
* Flush any calendar switch-off the app hasn't been allowed to write into
|
* Flush any calendar switch-off the app hasn't been allowed to write into
|
||||||
* the system's `Calendars.VISIBLE` yet — including the retired app-local
|
* the system's `Calendars.VISIBLE` yet — including the retired app-local
|
||||||
* "disabled calendars" set the upgrade inherits (#75). A no-op on a fresh
|
* "disabled calendars" set the upgrade inherits (#75). A no-op on a fresh
|
||||||
* install and in the steady state; a launch without the calendar permissions
|
* install and in the steady state; a launch without the calendar permission
|
||||||
* leaves the set pending, and granting them on the permission screen runs it
|
* leaves the set pending, and `RootScreen` runs it again once the app comes
|
||||||
* there instead.
|
* up holding it — whichever way it was granted.
|
||||||
*/
|
*/
|
||||||
private fun reconcileCalendarVisibility() {
|
private fun reconcileCalendarVisibility() {
|
||||||
val deps = EntryPointAccessors.fromApplication(
|
val deps = EntryPointAccessors.fromApplication(
|
||||||
|
|||||||
@@ -16,9 +16,12 @@ import kotlinx.coroutines.flow.Flow
|
|||||||
import kotlinx.coroutines.flow.MutableSharedFlow
|
import kotlinx.coroutines.flow.MutableSharedFlow
|
||||||
import kotlinx.coroutines.flow.combine
|
import kotlinx.coroutines.flow.combine
|
||||||
import kotlinx.coroutines.flow.distinctUntilChanged
|
import kotlinx.coroutines.flow.distinctUntilChanged
|
||||||
|
import kotlinx.coroutines.flow.drop
|
||||||
import kotlinx.coroutines.flow.first
|
import kotlinx.coroutines.flow.first
|
||||||
import kotlinx.coroutines.flow.flow
|
import kotlinx.coroutines.flow.flow
|
||||||
import kotlinx.coroutines.flow.flowOn
|
import kotlinx.coroutines.flow.flowOn
|
||||||
|
import kotlinx.coroutines.flow.map
|
||||||
|
import kotlinx.coroutines.flow.merge
|
||||||
import kotlinx.coroutines.flow.onStart
|
import kotlinx.coroutines.flow.onStart
|
||||||
import kotlinx.coroutines.sync.Mutex
|
import kotlinx.coroutines.sync.Mutex
|
||||||
import kotlinx.coroutines.sync.withLock
|
import kotlinx.coroutines.sync.withLock
|
||||||
@@ -63,21 +66,38 @@ class CalendarRepositoryImpl @Inject constructor(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Re-query signal for everything filtered by visibility: the provider's own
|
||||||
|
* notifications, plus every change to the pending switch-off set (an id
|
||||||
|
* leaves it as its `VISIBLE` write lands, which changes what is shown).
|
||||||
|
* [calendarsSnapshot] keeps the two in step.
|
||||||
|
*/
|
||||||
|
private fun visibilityTicks(): Flow<Unit> = merge(
|
||||||
|
ticks.onStart { emit(Unit) },
|
||||||
|
// The current value is already covered by the tick above; only later
|
||||||
|
// changes re-query (the set is deduped, so an unrelated DataStore write
|
||||||
|
// doesn't).
|
||||||
|
prefs.pendingDisabledCalendarIds.drop(1).map {},
|
||||||
|
)
|
||||||
|
|
||||||
// A switch-off the app hasn't been allowed to write yet is folded into the
|
// A switch-off the app hasn't been allowed to write yet is folded into the
|
||||||
// flag itself, so every consumer — the Settings switch, the filter sheet,
|
// flag itself, so every consumer — the Settings switch, the filter sheet,
|
||||||
// the form and import pickers, the widgets — reads one visibility and can't
|
// the form and import pickers, the widgets — reads one visibility and can't
|
||||||
// disagree with what the user just tapped. The reconciler reads the data
|
// disagree with what the user just tapped. The reconciler reads the data
|
||||||
// source directly, because it needs the provider's own answer.
|
// source directly, because it needs the provider's own answer.
|
||||||
override fun calendars(): Flow<List<CalendarSource>> =
|
override fun calendars(): Flow<List<CalendarSource>> =
|
||||||
combine(
|
visibilityTicks().reQuery {
|
||||||
ticks.onStart { emit(Unit) }.reQuery { calendarsSnapshot() },
|
val calendars = calendarsSnapshot()
|
||||||
prefs.pendingDisabledCalendarIds,
|
val pendingDisabled = prefs.pendingDisabledCalendarIds.first()
|
||||||
) { calendars, pendingDisabled ->
|
|
||||||
if (pendingDisabled.isEmpty()) calendars
|
if (pendingDisabled.isEmpty()) calendars
|
||||||
else calendars.map {
|
else calendars.map {
|
||||||
if (it.id in pendingDisabled) it.copy(isVisibleInSystem = false) else it
|
if (it.id in pendingDisabled) it.copy(isVisibleInSystem = false) else it
|
||||||
}
|
}
|
||||||
}.flowOn(io)
|
}
|
||||||
|
// Collapse re-emissions that carry an identical list (see
|
||||||
|
// [instances]).
|
||||||
|
.distinctUntilChanged()
|
||||||
|
.flowOn(io)
|
||||||
|
|
||||||
// Instances are filtered by the system's per-calendar VISIBLE flag ∪ the
|
// Instances are filtered by the system's per-calendar VISIBLE flag ∪ the
|
||||||
// switch-offs still waiting to be written to it ∪ the app-side hidden set:
|
// switch-offs still waiting to be written to it ∪ the app-side hidden set:
|
||||||
@@ -89,23 +109,21 @@ class CalendarRepositoryImpl @Inject constructor(
|
|||||||
// and re-enable invisible calendars.
|
// and re-enable invisible calendars.
|
||||||
override fun instances(range: ClosedRange<Instant>): Flow<List<EventInstance>> =
|
override fun instances(range: ClosedRange<Instant>): Flow<List<EventInstance>> =
|
||||||
combine(
|
combine(
|
||||||
ticks
|
visibilityTicks().reQuery {
|
||||||
.onStart { emit(Unit) }
|
// All three reads in one pass, so a list of instances is never
|
||||||
.reQuery {
|
// filtered against a visibility snapshot from another tick.
|
||||||
// Both reads in one pass, so a list of instances is never
|
QueriedInstances(
|
||||||
// filtered against a visibility snapshot from another tick.
|
instances = dataSource.instances(
|
||||||
QueriedInstances(
|
beginMillis = range.start.toEpochMillis(),
|
||||||
instances = dataSource.instances(
|
endMillis = range.endInclusive.toEpochMillis(),
|
||||||
beginMillis = range.start.toEpochMillis(),
|
),
|
||||||
endMillis = range.endInclusive.toEpochMillis(),
|
switchedOffCalendarIds = invisibleCalendarIds() +
|
||||||
),
|
prefs.pendingDisabledCalendarIds.first(),
|
||||||
invisibleCalendarIds = invisibleCalendarIds(),
|
)
|
||||||
)
|
},
|
||||||
},
|
|
||||||
prefs.hiddenCalendarIds,
|
prefs.hiddenCalendarIds,
|
||||||
prefs.pendingDisabledCalendarIds,
|
) { queried, hidden ->
|
||||||
) { queried, hidden, pendingDisabled ->
|
val excluded = hidden + queried.switchedOffCalendarIds
|
||||||
val excluded = hidden + pendingDisabled + queried.invisibleCalendarIds
|
|
||||||
if (excluded.isEmpty()) queried.instances
|
if (excluded.isEmpty()) queried.instances
|
||||||
else queried.instances.filterNot { it.calendarId in excluded }
|
else queried.instances.filterNot { it.calendarId in excluded }
|
||||||
}
|
}
|
||||||
@@ -119,7 +137,7 @@ class CalendarRepositoryImpl @Inject constructor(
|
|||||||
/** One instances query plus the visibility it must be filtered against. */
|
/** One instances query plus the visibility it must be filtered against. */
|
||||||
private data class QueriedInstances(
|
private data class QueriedInstances(
|
||||||
val instances: List<EventInstance>,
|
val instances: List<EventInstance>,
|
||||||
val invisibleCalendarIds: Set<Long>,
|
val switchedOffCalendarIds: Set<Long>,
|
||||||
)
|
)
|
||||||
|
|
||||||
/** Calendars switched off at system level — hidden, and never reminded about. */
|
/** Calendars switched off at system level — hidden, and never reminded about. */
|
||||||
@@ -129,6 +147,7 @@ class CalendarRepositoryImpl @Inject constructor(
|
|||||||
|
|
||||||
private val calendarsLock = Mutex()
|
private val calendarsLock = Mutex()
|
||||||
private var cachedGeneration = -1L
|
private var cachedGeneration = -1L
|
||||||
|
private var cachedPending: Set<Long>? = null
|
||||||
private var cachedCalendars: List<CalendarSource> = emptyList()
|
private var cachedCalendars: List<CalendarSource> = emptyList()
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -140,12 +159,21 @@ class CalendarRepositoryImpl @Inject constructor(
|
|||||||
*
|
*
|
||||||
* An empty result is never cached — it is what a read without the calendar
|
* An empty result is never cached — it is what a read without the calendar
|
||||||
* permission returns, and the grant itself doesn't notify the provider.
|
* permission returns, and the grant itself doesn't notify the provider.
|
||||||
|
*
|
||||||
|
* The pending switch-off set keys the cache alongside the tick. An id leaves
|
||||||
|
* that set the moment its `VISIBLE` write lands, while the observer that
|
||||||
|
* would invalidate the snapshot is only dispatched through the main looper
|
||||||
|
* afterwards — so a snapshot taken while the id was still pending, read
|
||||||
|
* against the set that no longer holds it, would report the calendar as *on*
|
||||||
|
* again and re-admit exactly the events being hidden.
|
||||||
*/
|
*/
|
||||||
private suspend fun calendarsSnapshot(): List<CalendarSource> = calendarsLock.withLock {
|
private suspend fun calendarsSnapshot(): List<CalendarSource> = calendarsLock.withLock {
|
||||||
val current = generation.get()
|
val current = generation.get()
|
||||||
if (current != cachedGeneration || cachedCalendars.isEmpty()) {
|
val pending = prefs.pendingDisabledCalendarIds.first()
|
||||||
|
if (current != cachedGeneration || pending != cachedPending || cachedCalendars.isEmpty()) {
|
||||||
cachedCalendars = dataSource.calendars()
|
cachedCalendars = dataSource.calendars()
|
||||||
cachedGeneration = current
|
cachedGeneration = current
|
||||||
|
cachedPending = pending
|
||||||
}
|
}
|
||||||
cachedCalendars
|
cachedCalendars
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -11,7 +11,6 @@ import dagger.hilt.android.qualifiers.ApplicationContext
|
|||||||
import dagger.hilt.components.SingletonComponent
|
import dagger.hilt.components.SingletonComponent
|
||||||
import de.jeanlucmakiola.calendula.data.di.IoDispatcher
|
import de.jeanlucmakiola.calendula.data.di.IoDispatcher
|
||||||
import de.jeanlucmakiola.calendula.data.prefs.CalendarPrefs
|
import de.jeanlucmakiola.calendula.data.prefs.CalendarPrefs
|
||||||
import de.jeanlucmakiola.calendula.domain.CalendarSource
|
|
||||||
import de.jeanlucmakiola.calendula.domain.calendarVisibilityPlan
|
import de.jeanlucmakiola.calendula.domain.calendarVisibilityPlan
|
||||||
import de.jeanlucmakiola.calendula.domain.hasSystemHiddenCalendars
|
import de.jeanlucmakiola.calendula.domain.hasSystemHiddenCalendars
|
||||||
import kotlinx.coroutines.CoroutineDispatcher
|
import kotlinx.coroutines.CoroutineDispatcher
|
||||||
@@ -28,15 +27,19 @@ import javax.inject.Singleton
|
|||||||
* model (#75), and the standing drain for switch-offs made without
|
* model (#75), and the standing drain for switch-offs made without
|
||||||
* `WRITE_CALENDAR`.
|
* `WRITE_CALENDAR`.
|
||||||
*
|
*
|
||||||
* Runs on every launch, and again the moment the permission screen grants the
|
* Runs on every launch, and again whenever the app comes up holding the calendar
|
||||||
* calendar permissions. It is a no-op whenever the pending set is empty, which
|
* permission — a grant made on Android's own app-settings screen never reaches
|
||||||
|
* the permission screen's callback. It is a no-op whenever the pending set is
|
||||||
|
* empty and the notice has been settled, which
|
||||||
* is the steady state: each entry is written and dropped individually, so a run
|
* is the steady state: each entry is written and dropped individually, so a run
|
||||||
* that dies part-way resumes exactly where it stopped and never re-applies a
|
* that dies part-way resumes exactly where it stopped and never re-applies a
|
||||||
* write the user has since undone by hand.
|
* write the user has since undone by hand.
|
||||||
*
|
*
|
||||||
* The reconciliation only hides (see [calendarVisibilityPlan]). Calendars hidden
|
* The reconciliation only hides (see [calendarVisibilityPlan]). Calendars hidden
|
||||||
* at system level stay hidden, and the first run that sees one arms the one-time
|
* at system level stay hidden, and on an *upgraded* install the first run that
|
||||||
* notice explaining why Calendula no longer lists their events.
|
* sees one arms the one-time notice explaining why Calendula no longer lists
|
||||||
|
* their events. A fresh install never had the old behaviour, so it retires that
|
||||||
|
* notice unshown — every other device ships with something hidden.
|
||||||
*/
|
*/
|
||||||
@Singleton
|
@Singleton
|
||||||
class CalendarVisibilityReconciler @Inject constructor(
|
class CalendarVisibilityReconciler @Inject constructor(
|
||||||
@@ -52,10 +55,19 @@ class CalendarVisibilityReconciler @Inject constructor(
|
|||||||
// so an IOException from a damaged preferences file would otherwise take
|
// so an IOException from a damaged preferences file would otherwise take
|
||||||
// the process down on every launch.
|
// the process down on every launch.
|
||||||
try {
|
try {
|
||||||
|
// A fresh install has no retired model behind it — nothing to
|
||||||
|
// migrate, and nothing to explain. Settled ahead of the permission
|
||||||
|
// gate so an update installed before the first grant can't make a
|
||||||
|
// first run look like an upgrade afterwards.
|
||||||
|
if (!isUpgradeInstall()) settleNoticeOnce(pending = false)
|
||||||
if (!hasPermission(Manifest.permission.READ_CALENDAR)) return@withContext
|
if (!hasPermission(Manifest.permission.READ_CALENDAR)) return@withContext
|
||||||
val pending = prefs.pendingDisabledCalendarIds.first()
|
val pending = prefs.pendingDisabledCalendarIds.first()
|
||||||
|
val noticeSettled = prefs.visibilityNoticePending.first() != null
|
||||||
|
// The steady state, and every run after the first: nothing left to
|
||||||
|
// drain and nothing left to decide, so don't pay for the query.
|
||||||
|
if (pending.isEmpty() && noticeSettled) return@withContext
|
||||||
val calendars = dataSource.calendars()
|
val calendars = dataSource.calendars()
|
||||||
armNoticeOnce(calendars, pending)
|
settleNoticeOnce(hasSystemHiddenCalendars(calendars, pending))
|
||||||
if (pending.isEmpty() || !hasPermission(Manifest.permission.WRITE_CALENDAR)) {
|
if (pending.isEmpty() || !hasPermission(Manifest.permission.WRITE_CALENDAR)) {
|
||||||
return@withContext
|
return@withContext
|
||||||
}
|
}
|
||||||
@@ -79,15 +91,30 @@ class CalendarVisibilityReconciler @Inject constructor(
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Arm the one-time notice if the device holds a calendar switched off
|
* Settle the one-time notice: [pending] arms it, false retires it unshown.
|
||||||
* outside Calendula. Evaluated once, on the first run that can read the
|
* Answered once, by whichever run can answer it first; the answer is stored
|
||||||
* calendars at all; the answer — including "nothing to say" — is stored, so
|
* either way, so the notice can't resurface later, when the same state would
|
||||||
* the notice can't resurface later, when the same state would no longer be
|
* no longer be news to the user.
|
||||||
* news to the user.
|
|
||||||
*/
|
*/
|
||||||
private suspend fun armNoticeOnce(calendars: List<CalendarSource>, pending: Set<Long>) {
|
private suspend fun settleNoticeOnce(pending: Boolean) {
|
||||||
if (prefs.visibilityNoticePending.first() != null) return
|
if (prefs.visibilityNoticePending.first() != null) return
|
||||||
prefs.setVisibilityNoticePending(hasSystemHiddenCalendars(calendars, pending))
|
prefs.setVisibilityNoticePending(pending)
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Whether this install has ever run an earlier version. The notice explains
|
||||||
|
* a change to behaviour the user has already seen, so a first install has
|
||||||
|
* nothing to announce — and hidden calendars are the *norm* on a fresh
|
||||||
|
* device (a second account's, "Holidays in …", a subscribed calendar), which
|
||||||
|
* would otherwise put a changelog dialog in front of a first-run user.
|
||||||
|
*/
|
||||||
|
private fun isUpgradeInstall(): Boolean = try {
|
||||||
|
@Suppress("DEPRECATION")
|
||||||
|
val info = context.packageManager.getPackageInfo(context.packageName, 0)
|
||||||
|
info.lastUpdateTime > info.firstInstallTime
|
||||||
|
} catch (e: PackageManager.NameNotFoundException) {
|
||||||
|
Log.w(TAG, "Own package info unavailable; treating as a fresh install", e)
|
||||||
|
false
|
||||||
}
|
}
|
||||||
|
|
||||||
private fun hasPermission(permission: String): Boolean =
|
private fun hasPermission(permission: String): Boolean =
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ import androidx.datastore.preferences.core.edit
|
|||||||
import androidx.datastore.preferences.core.longPreferencesKey
|
import androidx.datastore.preferences.core.longPreferencesKey
|
||||||
import androidx.datastore.preferences.core.stringPreferencesKey
|
import androidx.datastore.preferences.core.stringPreferencesKey
|
||||||
import kotlinx.coroutines.flow.Flow
|
import kotlinx.coroutines.flow.Flow
|
||||||
|
import kotlinx.coroutines.flow.distinctUntilChanged
|
||||||
import kotlinx.coroutines.flow.map
|
import kotlinx.coroutines.flow.map
|
||||||
import javax.inject.Inject
|
import javax.inject.Inject
|
||||||
import javax.inject.Singleton
|
import javax.inject.Singleton
|
||||||
@@ -28,9 +29,13 @@ class CalendarPrefs @Inject constructor(
|
|||||||
private val store: DataStore<Preferences>,
|
private val store: DataStore<Preferences>,
|
||||||
) {
|
) {
|
||||||
|
|
||||||
val hiddenCalendarIds: Flow<Set<Long>> = store.data.map { prefs ->
|
// Both id sets are deduped: the store is shared with SettingsPrefs, so every
|
||||||
prefs[HIDDEN_IDS_KEY].parseIds()
|
// unrelated write (a settings toggle, the last-used calendar) re-emits an
|
||||||
}
|
// identical set otherwise — and a change to the pending set now costs a
|
||||||
|
// fresh provider read in CalendarRepositoryImpl.
|
||||||
|
val hiddenCalendarIds: Flow<Set<Long>> = store.data
|
||||||
|
.map { prefs -> prefs[HIDDEN_IDS_KEY].parseIds() }
|
||||||
|
.distinctUntilChanged()
|
||||||
|
|
||||||
suspend fun setHiddenCalendarIds(ids: Set<Long>) {
|
suspend fun setHiddenCalendarIds(ids: Set<Long>) {
|
||||||
store.edit { prefs -> prefs.writeIds(HIDDEN_IDS_KEY, ids) }
|
store.edit { prefs -> prefs.writeIds(HIDDEN_IDS_KEY, ids) }
|
||||||
@@ -49,9 +54,9 @@ class CalendarPrefs @Inject constructor(
|
|||||||
* provider entry by entry the moment the app may write, and nothing ever
|
* provider entry by entry the moment the app may write, and nothing ever
|
||||||
* adds to it while it may.
|
* adds to it while it may.
|
||||||
*/
|
*/
|
||||||
val pendingDisabledCalendarIds: Flow<Set<Long>> = store.data.map { prefs ->
|
val pendingDisabledCalendarIds: Flow<Set<Long>> = store.data
|
||||||
prefs[DISABLED_IDS_KEY].parseIds()
|
.map { prefs -> prefs[DISABLED_IDS_KEY].parseIds() }
|
||||||
}
|
.distinctUntilChanged()
|
||||||
|
|
||||||
suspend fun addPendingDisabledCalendarIds(ids: Collection<Long>) =
|
suspend fun addPendingDisabledCalendarIds(ids: Collection<Long>) =
|
||||||
editPendingDisabled { it + ids }
|
editPendingDisabled { it + ids }
|
||||||
|
|||||||
@@ -0,0 +1,30 @@
|
|||||||
|
package de.jeanlucmakiola.calendula.data.reminders
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Still relevant while the event has not ended: a reminder for an event that is
|
||||||
|
* already over is pointless to re-surface. Falls back to the begin time when the
|
||||||
|
* end is unknown (0L).
|
||||||
|
*/
|
||||||
|
internal fun ReminderAlert.isRelevantAt(nowMillis: Long): Boolean =
|
||||||
|
(endMillis.takeIf { it > 0L } ?: beginMillis) >= nowMillis
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The alerts [EventReminderReceiver] may mark handled (`STATE_FIRED`): the ones
|
||||||
|
* it posted, plus the ones it silenced whose event is already over.
|
||||||
|
*
|
||||||
|
* A silenced alert for an event still ahead is deliberately left
|
||||||
|
* `STATE_SCHEDULED`. Silencing is not handling — the calendar is switched off in
|
||||||
|
* Calendula while the provider still holds `VISIBLE = 1` (a read-only install,
|
||||||
|
* or an upgrade whose flush hasn't landed), so switching it back on before the
|
||||||
|
* event must still be able to surface the reminder. [ReminderAlertStore.dueAlerts]
|
||||||
|
* only ever returns scheduled rows, so marking them here would lose them for
|
||||||
|
* good; leaving them makes the provider's own table the stash
|
||||||
|
* ([ReminderRecovery]).
|
||||||
|
*/
|
||||||
|
internal fun handledAlertIds(
|
||||||
|
due: List<ReminderAlert>,
|
||||||
|
postedIds: Set<Long>,
|
||||||
|
nowMillis: Long,
|
||||||
|
): List<Long> = due
|
||||||
|
.filter { it.alertId in postedIds || !it.isRelevantAt(nowMillis) }
|
||||||
|
.map { it.alertId }
|
||||||
@@ -30,7 +30,9 @@ import javax.inject.Inject
|
|||||||
* Settings → Calendars has `Calendars.VISIBLE = 0`, and the provider creates no
|
* Settings → Calendars has `Calendars.VISIBLE = 0`, and the provider creates no
|
||||||
* alert rows for it in the first place (#75). The one case that flag can't
|
* alert rows for it in the first place (#75). The one case that flag can't
|
||||||
* cover — a read-only install, which keeps its switches app-side — is gated in
|
* cover — a read-only install, which keeps its switches app-side — is gated in
|
||||||
* [ReminderNotifier.post], where the snoozed re-show passes too.
|
* [ReminderNotifier.post], where the snoozed re-show passes too. What that gate
|
||||||
|
* silences is *not* marked fired while the event is still ahead, so switching
|
||||||
|
* the calendar back on can still surface it (see [handledAlertIds]).
|
||||||
*/
|
*/
|
||||||
@AndroidEntryPoint
|
@AndroidEntryPoint
|
||||||
class EventReminderReceiver : BroadcastReceiver() {
|
class EventReminderReceiver : BroadcastReceiver() {
|
||||||
@@ -52,8 +54,10 @@ class EventReminderReceiver : BroadcastReceiver() {
|
|||||||
if (settingsPrefs.remindersEnabled.first()) {
|
if (settingsPrefs.remindersEnabled.first()) {
|
||||||
val now = System.currentTimeMillis()
|
val now = System.currentTimeMillis()
|
||||||
val due = alertStore.dueAlerts(now)
|
val due = alertStore.dueAlerts(now)
|
||||||
due.forEach { notifier.post(it) }
|
val postedIds = due
|
||||||
alertStore.markFired(due.map { it.alertId }, now)
|
.filter { notifier.post(it) }
|
||||||
|
.mapTo(mutableSetOf()) { it.alertId }
|
||||||
|
alertStore.markFired(handledAlertIds(due, postedIds, now), now)
|
||||||
}
|
}
|
||||||
} finally {
|
} finally {
|
||||||
pendingResult.finish()
|
pendingResult.finish()
|
||||||
|
|||||||
@@ -64,8 +64,13 @@ class ReminderNotifier @Inject constructor(
|
|||||||
calendarId in calendarPrefs.pendingDisabledCalendarIds.first() ||
|
calendarId in calendarPrefs.pendingDisabledCalendarIds.first() ||
|
||||||
calendarDataSource.isCalendarVisible(calendarId) == false
|
calendarDataSource.isCalendarVisible(calendarId) == false
|
||||||
|
|
||||||
suspend fun post(alert: ReminderAlert) {
|
/**
|
||||||
if (isSilenced(alert.calendarId)) return
|
* Post [alert], unless its calendar is switched off. Returns whether the
|
||||||
|
* notification was put up: a silenced alert must stay unhandled so that
|
||||||
|
* switching the calendar back on can still surface it (see [handledAlertIds]).
|
||||||
|
*/
|
||||||
|
suspend fun post(alert: ReminderAlert): Boolean {
|
||||||
|
if (isSilenced(alert.calendarId)) return false
|
||||||
ensureChannel()
|
ensureChannel()
|
||||||
val title = alert.title.ifBlank { context.getString(R.string.event_untitled) }
|
val title = alert.title.ifBlank { context.getString(R.string.event_untitled) }
|
||||||
val is24Hour = settingsPrefs.timeFormat.first()
|
val is24Hour = settingsPrefs.timeFormat.first()
|
||||||
@@ -117,6 +122,8 @@ class ReminderNotifier @Inject constructor(
|
|||||||
// POST_NOTIFICATIONS was revoked between canPost() and here.
|
// POST_NOTIFICATIONS was revoked between canPost() and here.
|
||||||
Log.w(TAG, "Could not post reminder for event ${alert.eventId}", e)
|
Log.w(TAG, "Could not post reminder for event ${alert.eventId}", e)
|
||||||
}
|
}
|
||||||
|
// Handled either way: re-running it would hit the same revoked permission.
|
||||||
|
return true
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Remove a posted reminder (snooze re-shows it later; dismiss is final). */
|
/** Remove a posted reminder (snooze re-shows it later; dismiss is final). */
|
||||||
|
|||||||
@@ -0,0 +1,46 @@
|
|||||||
|
package de.jeanlucmakiola.calendula.data.reminders
|
||||||
|
|
||||||
|
import de.jeanlucmakiola.calendula.data.di.IoDispatcher
|
||||||
|
import de.jeanlucmakiola.calendula.data.prefs.SettingsPrefs
|
||||||
|
import kotlinx.coroutines.CoroutineDispatcher
|
||||||
|
import kotlinx.coroutines.flow.first
|
||||||
|
import kotlinx.coroutines.withContext
|
||||||
|
import javax.inject.Inject
|
||||||
|
import javax.inject.Singleton
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Re-posts the reminders a switched-off calendar silenced, when it is switched
|
||||||
|
* back on while they still matter.
|
||||||
|
*
|
||||||
|
* Only the app-side switch needs this — a read-only install, or an upgrade the
|
||||||
|
* reconciler hasn't flushed yet. With `Calendars.VISIBLE = 0` the provider
|
||||||
|
* deletes the calendar's alert rows itself and re-creates them on the way back;
|
||||||
|
* app-side the rows stay, still `STATE_SCHEDULED`, because
|
||||||
|
* [EventReminderReceiver] deliberately leaves the ones it silenced unhandled
|
||||||
|
* (see [handledAlertIds]). So the provider's own table is the stash, and nothing
|
||||||
|
* is mirrored locally.
|
||||||
|
*
|
||||||
|
* Best effort at switch-on time: it mirrors the receiver's gates (reminders on,
|
||||||
|
* notifications postable) and there is no later re-scan, so an alert left
|
||||||
|
* unposted because those are closed is simply released.
|
||||||
|
*/
|
||||||
|
@Singleton
|
||||||
|
class ReminderRecovery @Inject constructor(
|
||||||
|
private val alertStore: ReminderAlertStore,
|
||||||
|
private val notifier: ReminderNotifier,
|
||||||
|
private val settingsPrefs: SettingsPrefs,
|
||||||
|
@IoDispatcher private val io: CoroutineDispatcher,
|
||||||
|
) {
|
||||||
|
|
||||||
|
suspend fun rePostFor(calendarIds: Collection<Long>) = withContext(io) {
|
||||||
|
if (calendarIds.isEmpty()) return@withContext
|
||||||
|
if (!settingsPrefs.remindersEnabled.first() || !notifier.canPost()) return@withContext
|
||||||
|
val now = System.currentTimeMillis()
|
||||||
|
val ids = calendarIds.toSet()
|
||||||
|
val recovered = alertStore.dueAlerts(now)
|
||||||
|
.filter { it.calendarId in ids && it.isRelevantAt(now) }
|
||||||
|
if (recovered.isEmpty()) return@withContext
|
||||||
|
val postedIds = recovered.filter { notifier.post(it) }.map { it.alertId }
|
||||||
|
alertStore.markFired(postedIds, now)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -7,6 +7,7 @@ import androidx.compose.material3.ExperimentalMaterial3ExpressiveApi
|
|||||||
import androidx.compose.material3.MaterialTheme
|
import androidx.compose.material3.MaterialTheme
|
||||||
import androidx.compose.runtime.Composable
|
import androidx.compose.runtime.Composable
|
||||||
import androidx.compose.runtime.DisposableEffect
|
import androidx.compose.runtime.DisposableEffect
|
||||||
|
import androidx.compose.runtime.LaunchedEffect
|
||||||
import androidx.compose.runtime.mutableStateOf
|
import androidx.compose.runtime.mutableStateOf
|
||||||
import androidx.compose.runtime.remember
|
import androidx.compose.runtime.remember
|
||||||
import androidx.compose.runtime.setValue
|
import androidx.compose.runtime.setValue
|
||||||
@@ -84,6 +85,10 @@ fun RootScreen(
|
|||||||
// visibility (#75); armed by the reconciler, shown over the app.
|
// visibility (#75); armed by the reconciler, shown over the app.
|
||||||
val visibilityNotice: CalendarVisibilityNoticeViewModel = hiltViewModel()
|
val visibilityNotice: CalendarVisibilityNoticeViewModel = hiltViewModel()
|
||||||
val noticePending by visibilityNotice.pending.collectAsStateWithLifecycle()
|
val noticePending by visibilityNotice.pending.collectAsStateWithLifecycle()
|
||||||
|
// Runs on entry however the permission was granted — including from
|
||||||
|
// Android's app-settings screen, which only comes back through the
|
||||||
|
// ON_RESUME check above. Cheap once there is nothing left to do.
|
||||||
|
LaunchedEffect(Unit) { visibilityNotice.reconcile() }
|
||||||
if (onboardingDone == true && noticePending) {
|
if (onboardingDone == true && noticePending) {
|
||||||
CalendarVisibilityNoticeDialog(onDismiss = visibilityNotice::dismiss)
|
CalendarVisibilityNoticeDialog(onDismiss = visibilityNotice::dismiss)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -12,6 +12,7 @@ import androidx.lifecycle.ViewModel
|
|||||||
import androidx.lifecycle.viewModelScope
|
import androidx.lifecycle.viewModelScope
|
||||||
import dagger.hilt.android.lifecycle.HiltViewModel
|
import dagger.hilt.android.lifecycle.HiltViewModel
|
||||||
import de.jeanlucmakiola.calendula.R
|
import de.jeanlucmakiola.calendula.R
|
||||||
|
import de.jeanlucmakiola.calendula.data.calendar.CalendarVisibilityReconciler
|
||||||
import de.jeanlucmakiola.calendula.data.prefs.CalendarPrefs
|
import de.jeanlucmakiola.calendula.data.prefs.CalendarPrefs
|
||||||
import kotlinx.coroutines.flow.SharingStarted
|
import kotlinx.coroutines.flow.SharingStarted
|
||||||
import kotlinx.coroutines.flow.StateFlow
|
import kotlinx.coroutines.flow.StateFlow
|
||||||
@@ -30,8 +31,21 @@ import javax.inject.Inject
|
|||||||
@HiltViewModel
|
@HiltViewModel
|
||||||
class CalendarVisibilityNoticeViewModel @Inject constructor(
|
class CalendarVisibilityNoticeViewModel @Inject constructor(
|
||||||
private val prefs: CalendarPrefs,
|
private val prefs: CalendarPrefs,
|
||||||
|
private val reconciler: CalendarVisibilityReconciler,
|
||||||
) : ViewModel() {
|
) : ViewModel() {
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Reconcile whenever the app comes up with the calendar permission held.
|
||||||
|
* The launch itself is covered by `CalendulaApp`, but a permission granted
|
||||||
|
* on Android's app-settings screen comes back through `RootScreen`'s
|
||||||
|
* ON_RESUME and never touches the permission screen's callback — so the
|
||||||
|
* trigger hangs off "we are showing the app", not off one grant route.
|
||||||
|
* Settled runs cost two DataStore reads and stop there.
|
||||||
|
*/
|
||||||
|
fun reconcile() {
|
||||||
|
viewModelScope.launch { reconciler.run() }
|
||||||
|
}
|
||||||
|
|
||||||
val pending: StateFlow<Boolean> = prefs.visibilityNoticePending
|
val pending: StateFlow<Boolean> = prefs.visibilityNoticePending
|
||||||
.map { it == true }
|
.map { it == true }
|
||||||
.stateIn(
|
.stateIn(
|
||||||
|
|||||||
@@ -13,6 +13,7 @@ import de.jeanlucmakiola.calendula.data.di.IoDispatcher
|
|||||||
import de.jeanlucmakiola.calendula.data.ics.IcsExporter
|
import de.jeanlucmakiola.calendula.data.ics.IcsExporter
|
||||||
import de.jeanlucmakiola.calendula.data.prefs.BackupStatus
|
import de.jeanlucmakiola.calendula.data.prefs.BackupStatus
|
||||||
import de.jeanlucmakiola.calendula.data.prefs.SettingsPrefs
|
import de.jeanlucmakiola.calendula.data.prefs.SettingsPrefs
|
||||||
|
import de.jeanlucmakiola.calendula.data.reminders.ReminderRecovery
|
||||||
import de.jeanlucmakiola.calendula.domain.CalendarSource
|
import de.jeanlucmakiola.calendula.domain.CalendarSource
|
||||||
import de.jeanlucmakiola.calendula.domain.ics.IcsWriter
|
import de.jeanlucmakiola.calendula.domain.ics.IcsWriter
|
||||||
import kotlinx.coroutines.CoroutineDispatcher
|
import kotlinx.coroutines.CoroutineDispatcher
|
||||||
@@ -43,6 +44,7 @@ class CalendarsViewModel @Inject constructor(
|
|||||||
private val repository: CalendarRepository,
|
private val repository: CalendarRepository,
|
||||||
private val icsExporter: IcsExporter,
|
private val icsExporter: IcsExporter,
|
||||||
private val settingsPrefs: SettingsPrefs,
|
private val settingsPrefs: SettingsPrefs,
|
||||||
|
private val reminderRecovery: ReminderRecovery,
|
||||||
@IoDispatcher private val io: CoroutineDispatcher,
|
@IoDispatcher private val io: CoroutineDispatcher,
|
||||||
) : ViewModel() {
|
) : ViewModel() {
|
||||||
|
|
||||||
@@ -125,9 +127,14 @@ class CalendarsViewModel @Inject constructor(
|
|||||||
* every surface *and* the provider stops (or resumes) scheduling its
|
* every surface *and* the provider stops (or resumes) scheduling its
|
||||||
* reminders. Nothing is patched by hand — the provider notifies and the
|
* reminders. Nothing is patched by hand — the provider notifies and the
|
||||||
* observer re-queries.
|
* observer re-queries.
|
||||||
|
*
|
||||||
|
* Switching one back on also re-posts the reminders it silenced while it was
|
||||||
|
* off and that are still relevant — those the app kept app-side because it
|
||||||
|
* may not write the flag ([ReminderRecovery]).
|
||||||
*/
|
*/
|
||||||
fun setCalendarVisible(id: Long, visible: Boolean) = write {
|
fun setCalendarVisible(id: Long, visible: Boolean) = write {
|
||||||
repository.setCalendarsVisible(listOf(id), visible)
|
repository.setCalendarsVisible(listOf(id), visible)
|
||||||
|
if (visible) reminderRecovery.rePostFor(listOf(id))
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -138,6 +145,7 @@ class CalendarsViewModel @Inject constructor(
|
|||||||
*/
|
*/
|
||||||
fun setAccountVisible(ids: Collection<Long>, visible: Boolean) = write {
|
fun setAccountVisible(ids: Collection<Long>, visible: Boolean) = write {
|
||||||
repository.setCalendarsVisible(ids, visible)
|
repository.setCalendarsVisible(ids, visible)
|
||||||
|
if (visible) reminderRecovery.rePostFor(ids)
|
||||||
}
|
}
|
||||||
|
|
||||||
// --- Automatic backup (issue #8) ------------------------------------
|
// --- Automatic backup (issue #8) ------------------------------------
|
||||||
|
|||||||
@@ -184,6 +184,9 @@ class EventEditViewModel @Inject constructor(
|
|||||||
* [state]). Managed special-dates calendars are excluded too: their events
|
* [state]). Managed special-dates calendars are excluded too: their events
|
||||||
* are owned by the contact sync, which would delete any user event created
|
* are owned by the contact sync, which would delete any user event created
|
||||||
* there.
|
* there.
|
||||||
|
*
|
||||||
|
* This is the list of *targets*. An event already living in an excluded
|
||||||
|
* calendar keeps it — [state] adds it back to the picker.
|
||||||
*/
|
*/
|
||||||
private val writableCalendars: Flow<List<CalendarSource>> = allCalendars.map { calendars ->
|
private val writableCalendars: Flow<List<CalendarSource>> = allCalendars.map { calendars ->
|
||||||
calendars.filter { it.canModifyContents && it.isVisibleInSystem && !it.isManaged }
|
calendars.filter { it.canModifyContents && it.isVisibleInSystem && !it.isManaged }
|
||||||
@@ -232,11 +235,16 @@ class EventEditViewModel @Inject constructor(
|
|||||||
// off the calendar's durable marker, not a stored id, so it holds after a
|
// off the calendar's durable marker, not a stored id, so it holds after a
|
||||||
// backup restore too.
|
// backup restore too.
|
||||||
val isManaged = local.editTarget != null && resolvedCalendar?.isManaged == true
|
val isManaged = local.editTarget != null && resolvedCalendar?.isManaged == true
|
||||||
// The picker offers writable calendars only; when editing a managed event
|
// The picker offers writable calendars only; the event's own calendar is
|
||||||
// its own (excluded) calendar is added back so the row still names it.
|
// added back whenever it isn't among them — a managed special-dates one,
|
||||||
|
// or one switched off on this device — so the row keeps naming it instead
|
||||||
|
// of reading as the "no calendar" error, and saving can leave the event
|
||||||
|
// where it is. A calendar the app may not write to is still no target.
|
||||||
|
val ownCalendar = resolvedCalendar?.takeIf { own ->
|
||||||
|
own.canModifyContents && external.writable.none { it.id == own.id }
|
||||||
|
}
|
||||||
val pickerCalendars =
|
val pickerCalendars =
|
||||||
if (isManaged && resolvedCalendar != null) external.writable + resolvedCalendar
|
if (ownCalendar != null) external.writable + ownCalendar else external.writable
|
||||||
else external.writable
|
|
||||||
// An all-day event is date-anchored, so a zone is meaningless on it —
|
// An all-day event is date-anchored, so a zone is meaningless on it —
|
||||||
// the field is withheld from both lists rather than shown as a no-op.
|
// the field is withheld from both lists rather than shown as a no-op.
|
||||||
val offerableFields = EventFormField.entries.toSet() -
|
val offerableFields = EventFormField.entries.toSet() -
|
||||||
|
|||||||
@@ -1,29 +1,22 @@
|
|||||||
package de.jeanlucmakiola.calendula.ui.permission
|
package de.jeanlucmakiola.calendula.ui.permission
|
||||||
|
|
||||||
import androidx.lifecycle.ViewModel
|
import androidx.lifecycle.ViewModel
|
||||||
import androidx.lifecycle.viewModelScope
|
|
||||||
import dagger.hilt.android.lifecycle.HiltViewModel
|
import dagger.hilt.android.lifecycle.HiltViewModel
|
||||||
import de.jeanlucmakiola.calendula.data.calendar.CalendarVisibilityReconciler
|
|
||||||
import kotlinx.coroutines.flow.MutableStateFlow
|
import kotlinx.coroutines.flow.MutableStateFlow
|
||||||
import kotlinx.coroutines.flow.StateFlow
|
import kotlinx.coroutines.flow.StateFlow
|
||||||
import kotlinx.coroutines.flow.asStateFlow
|
import kotlinx.coroutines.flow.asStateFlow
|
||||||
import kotlinx.coroutines.launch
|
|
||||||
import javax.inject.Inject
|
import javax.inject.Inject
|
||||||
|
|
||||||
@HiltViewModel
|
@HiltViewModel
|
||||||
class PermissionViewModel @Inject constructor(
|
class PermissionViewModel @Inject constructor() : ViewModel() {
|
||||||
private val visibilityReconciler: CalendarVisibilityReconciler,
|
|
||||||
) : ViewModel() {
|
|
||||||
|
|
||||||
private val _state = MutableStateFlow<PermissionUiState>(PermissionUiState.Rationale)
|
private val _state = MutableStateFlow<PermissionUiState>(PermissionUiState.Rationale)
|
||||||
val state: StateFlow<PermissionUiState> = _state.asStateFlow()
|
val state: StateFlow<PermissionUiState> = _state.asStateFlow()
|
||||||
|
|
||||||
|
// The visibility reconcile a grant owes (#75) hangs off RootScreen showing
|
||||||
|
// the app instead: it has to cover the grants made outside it too.
|
||||||
fun onGranted() {
|
fun onGranted() {
|
||||||
_state.value = PermissionUiState.Granted
|
_state.value = PermissionUiState.Granted
|
||||||
// The visibility reconcile needs the calendar permissions, so the launch
|
|
||||||
// that started without them skipped it (#75) — this is the moment it can
|
|
||||||
// finally run. A no-op on a fresh install, where nothing is pending.
|
|
||||||
viewModelScope.launch { visibilityReconciler.run() }
|
|
||||||
}
|
}
|
||||||
|
|
||||||
fun onDenied() {
|
fun onDenied() {
|
||||||
|
|||||||
@@ -74,9 +74,14 @@ class SettingsViewModel @Inject constructor(
|
|||||||
|
|
||||||
private val dynamicColorAvailable = Build.VERSION.SDK_INT >= Build.VERSION_CODES.S
|
private val dynamicColorAvailable = Build.VERSION.SDK_INT >= Build.VERSION_CODES.S
|
||||||
|
|
||||||
/** Writable calendars — the only ones that take a per-calendar reminder override. */
|
/**
|
||||||
|
* Writable calendars that are switched on — the only ones that take a
|
||||||
|
* per-calendar reminder override. A calendar switched off in Settings →
|
||||||
|
* Calendars is `VISIBLE = 0`, so the provider schedules no alarms for it and
|
||||||
|
* a default reminder configured there could never fire (#75).
|
||||||
|
*/
|
||||||
private val writableCalendars: Flow<List<CalendarSource>> = repository.calendars()
|
private val writableCalendars: Flow<List<CalendarSource>> = repository.calendars()
|
||||||
.map { calendars -> calendars.filter { it.canModifyContents } }
|
.map { calendars -> calendars.filter { it.canModifyContents && it.isVisibleInSystem } }
|
||||||
.catch { emit(emptyList()) }
|
.catch { emit(emptyList()) }
|
||||||
|
|
||||||
val state: StateFlow<SettingsUiState> =
|
val state: StateFlow<SettingsUiState> =
|
||||||
|
|||||||
@@ -376,6 +376,8 @@ class CalendarRepositoryImplTest {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// The next tick invalidates it — one fresh read, not one per flow.
|
// The next tick invalidates it — one fresh read, not one per flow.
|
||||||
|
// (The list has to change: an identical one is collapsed.)
|
||||||
|
fake.calendarsResult = listOf(makeCal(1L), makeCal(2L), makeCal(3L))
|
||||||
fake.tick()
|
fake.tick()
|
||||||
awaitItem()
|
awaitItem()
|
||||||
assertThat(fake.calendarQueries).isEqualTo(2)
|
assertThat(fake.calendarQueries).isEqualTo(2)
|
||||||
@@ -383,6 +385,64 @@ class CalendarRepositoryImplTest {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `calendars does not re-emit an unchanged list`(@TempDir tempDir: Path) = runTest {
|
||||||
|
// The store is shared with SettingsPrefs, so an unrelated write would
|
||||||
|
// otherwise re-run every view's combine for an identical list.
|
||||||
|
val prefs = newPrefs(tempDir)
|
||||||
|
val fake = FakeCalendarDataSource().apply { calendarsResult = listOf(makeCal(1L)) }
|
||||||
|
val repo = CalendarRepositoryImpl(fake, prefs, newSettings(tempDir), UnconfinedTestDispatcher(testScheduler))
|
||||||
|
|
||||||
|
repo.calendars().test {
|
||||||
|
assertThat(awaitItem().map { it.id }).containsExactly(1L)
|
||||||
|
|
||||||
|
prefs.setLastUsedCalendarId(1L)
|
||||||
|
fake.tick()
|
||||||
|
|
||||||
|
expectNoEvents()
|
||||||
|
cancelAndIgnoreRemainingEvents()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `a flushed switch-off never reads as on again before the provider ticks`(
|
||||||
|
@TempDir tempDir: Path,
|
||||||
|
) = runTest {
|
||||||
|
// The reconciler's shape: write VISIBLE = 0 straight to the provider,
|
||||||
|
// then release the id app-side. The provider's notification only arrives
|
||||||
|
// afterwards (it is dispatched through the main looper), so the release
|
||||||
|
// must not be read against the snapshot from before the write — that
|
||||||
|
// would flash exactly the events being hidden back into every view.
|
||||||
|
val prefs = newPrefs(tempDir)
|
||||||
|
prefs.addPendingDisabledCalendarIds(setOf(2L))
|
||||||
|
val fake = FakeCalendarDataSource().apply {
|
||||||
|
calendarsResult = listOf(makeCal(1L), makeCal(2L))
|
||||||
|
instancesResult = { _, _ ->
|
||||||
|
listOf(makeEvent(10L, "A", calendarId = 1L), makeEvent(11L, "B", calendarId = 2L))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
val repo = CalendarRepositoryImpl(fake, prefs, newSettings(tempDir), UnconfinedTestDispatcher(testScheduler))
|
||||||
|
val range = Instant.fromEpochMilliseconds(0)..Instant.fromEpochMilliseconds(10_000L)
|
||||||
|
|
||||||
|
// Warm the snapshot the way an open view would.
|
||||||
|
repo.instances(range).test {
|
||||||
|
assertThat(awaitItem().map { it.title }).containsExactly("A")
|
||||||
|
cancelAndIgnoreRemainingEvents()
|
||||||
|
}
|
||||||
|
|
||||||
|
fake.setCalendarVisible(2L, false) // no tick(): the observer hasn't fired yet
|
||||||
|
prefs.removePendingDisabledCalendarIds(setOf(2L))
|
||||||
|
|
||||||
|
repo.instances(range).test {
|
||||||
|
assertThat(awaitItem().map { it.title }).containsExactly("A")
|
||||||
|
cancelAndIgnoreRemainingEvents()
|
||||||
|
}
|
||||||
|
repo.calendars().test {
|
||||||
|
assertThat(awaitItem().single { it.id == 2L }.isVisibleInSystem).isFalse()
|
||||||
|
cancelAndIgnoreRemainingEvents()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun `searchEvents drops results from calendars that are off or hidden`(
|
fun `searchEvents drops results from calendars that are off or hidden`(
|
||||||
@TempDir tempDir: Path,
|
@TempDir tempDir: Path,
|
||||||
|
|||||||
@@ -0,0 +1,66 @@
|
|||||||
|
package de.jeanlucmakiola.calendula.data.reminders
|
||||||
|
|
||||||
|
import com.google.common.truth.Truth.assertThat
|
||||||
|
import org.junit.jupiter.api.Test
|
||||||
|
|
||||||
|
/**
|
||||||
|
* What the reminder receiver may mark handled. The silenced-but-still-ahead case
|
||||||
|
* is the one that matters: those rows are the only copy of the reminder (#75).
|
||||||
|
*/
|
||||||
|
class AlertHandlingTest {
|
||||||
|
|
||||||
|
private val now = 1_700_000_000_000L
|
||||||
|
|
||||||
|
private fun alert(
|
||||||
|
id: Long,
|
||||||
|
calendarId: Long = 1L,
|
||||||
|
beginMillis: Long = now + 60_000L,
|
||||||
|
endMillis: Long = now + 3_600_000L,
|
||||||
|
) = ReminderAlert(
|
||||||
|
alertId = id,
|
||||||
|
eventId = id * 10,
|
||||||
|
calendarId = calendarId,
|
||||||
|
beginMillis = beginMillis,
|
||||||
|
endMillis = endMillis,
|
||||||
|
title = "E $id",
|
||||||
|
location = null,
|
||||||
|
isAllDay = false,
|
||||||
|
)
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `posted alerts are handled`() {
|
||||||
|
val due = listOf(alert(1L), alert(2L))
|
||||||
|
|
||||||
|
assertThat(handledAlertIds(due, postedIds = setOf(1L, 2L), nowMillis = now))
|
||||||
|
.containsExactly(1L, 2L)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `a silenced alert whose event is still ahead stays unhandled`() {
|
||||||
|
// Switching its calendar back on before the event has to bring it back,
|
||||||
|
// and dueAlerts only ever returns STATE_SCHEDULED rows.
|
||||||
|
val due = listOf(alert(1L), alert(2L))
|
||||||
|
|
||||||
|
assertThat(handledAlertIds(due, postedIds = setOf(1L), nowMillis = now))
|
||||||
|
.containsExactly(1L)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `a silenced alert whose event is over is handled`() {
|
||||||
|
// Nothing left to re-surface, so it must not linger as scheduled.
|
||||||
|
val over = alert(2L, beginMillis = now - 7_200_000L, endMillis = now - 3_600_000L)
|
||||||
|
val due = listOf(alert(1L), over)
|
||||||
|
|
||||||
|
assertThat(handledAlertIds(due, postedIds = setOf(1L), nowMillis = now))
|
||||||
|
.containsExactly(1L, 2L)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `an unknown end time falls back to the begin time`() {
|
||||||
|
val started = alert(1L, beginMillis = now - 1L, endMillis = 0L)
|
||||||
|
val notYet = alert(2L, beginMillis = now + 1L, endMillis = 0L)
|
||||||
|
|
||||||
|
assertThat(handledAlertIds(listOf(started, notYet), postedIds = emptySet(), nowMillis = now))
|
||||||
|
.containsExactly(1L)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -108,6 +108,27 @@ class EventEditViewModelTest {
|
|||||||
job.cancel()
|
job.cancel()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `editing an event in a switched-off calendar keeps it in the picker`(
|
||||||
|
@TempDir tempDir: Path,
|
||||||
|
) = runTest(dispatcher) {
|
||||||
|
// Otherwise the calendar row renders as the "no calendar" error and any
|
||||||
|
// pick routes the save through a move the user never asked for.
|
||||||
|
val fake = FakeCalendarDataSource().apply {
|
||||||
|
calendarsResult = listOf(cal(1L), cal(2L, visible = false))
|
||||||
|
eventDetailResult = { detail(calendarId = 2L) }
|
||||||
|
}
|
||||||
|
val vm = viewModel(tempDir, fake)
|
||||||
|
val job = activate(vm)
|
||||||
|
|
||||||
|
vm.openForEdit(eventId = 42L, beginMillis = beginMillis, endMillis = endMillis)
|
||||||
|
advanceUntilIdle()
|
||||||
|
|
||||||
|
assertThat(vm.state.value?.calendars?.map { it.id }).containsExactly(1L, 2L)
|
||||||
|
assertThat(vm.state.value?.form?.calendarId).isEqualTo(2L)
|
||||||
|
job.cancel()
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun `changing the calendar routes the save through a move, not an update`(
|
fun `changing the calendar routes the save through a move, not an update`(
|
||||||
@TempDir tempDir: Path,
|
@TempDir tempDir: Path,
|
||||||
|
|||||||
@@ -171,7 +171,10 @@ the app has not been allowed to write yet (read-only permission grant, or a
|
|||||||
pre-permission launch); `CalendarVisibilityReconciler` drains it entry by entry,
|
pre-permission launch); `CalendarVisibilityReconciler` drains it entry by entry,
|
||||||
and until it does, the repository and `ReminderNotifier.post` honour it. That
|
and until it does, the repository and `ReminderNotifier.post` honour it. That
|
||||||
gate also covers a snooze re-shown from our own alarm after its calendar was
|
gate also covers a snooze re-shown from our own alarm after its calendar was
|
||||||
switched off. The drawer's filter sheet (`CalendarPrefs.hiddenCalendarIds`) is a
|
switched off. Silencing is not handling: an alert the gate drops keeps its
|
||||||
|
`SCHEDULED` state while its event is still ahead (`handledAlertIds`), so
|
||||||
|
switching the calendar back on re-posts it (`ReminderRecovery`) instead of
|
||||||
|
losing it — the provider's own table is the stash. The drawer's filter sheet (`CalendarPrefs.hiddenCalendarIds`) is a
|
||||||
separate in-app declutter that never touches reminders. See
|
separate in-app declutter that never touches reminders. See
|
||||||
`docs/design/calendar-visibility-model.md`.
|
`docs/design/calendar-visibility-model.md`.
|
||||||
Deliberately absent until real devices prove it necessary: own alarm
|
Deliberately absent until real devices prove it necessary: own alarm
|
||||||
|
|||||||
@@ -3,15 +3,17 @@
|
|||||||
Status: **implemented** on `fix/calendar-visibility-model` (on-device review owed)
|
Status: **implemented** on `fix/calendar-visibility-model` (on-device review owed)
|
||||||
Date: 2026-07-25, revised 2026-07-25 after code review
|
Date: 2026-07-25, revised 2026-07-25 after code review
|
||||||
|
|
||||||
Built as specified, then revised in two places the review found (see §4, §6):
|
Built as specified, then revised in two places the first review found (see §4,
|
||||||
the reconciliation no longer switches calendars *on*, and a calendar switched
|
§6): the reconciliation no longer switches calendars *on*, and a calendar
|
||||||
off without `WRITE_CALENDAR` is kept app-side instead of being lost. Other
|
switched off without `WRITE_CALENDAR` is kept app-side instead of being lost.
|
||||||
notes: `CalendarSource` gained a `syncsEvents` flag (read for #76's "not synced"
|
A second review pass found four more, all folded in below: the notice was armed
|
||||||
label); the reconcile runs from `CalendulaApp.onCreate` *and* from
|
on fresh installs too (§4), a silenced reminder could be marked handled and lost
|
||||||
`PermissionViewModel.onGranted`, so a launch that starts without the calendar
|
(§3), an event living in a switched-off calendar lost its calendar in the edit
|
||||||
permissions still catches up the moment they are granted; and the Settings hint
|
form (§2), and the reconcile only ran for one of the two ways a permission can
|
||||||
string was reworded rather than dropped — it described the retired app-only
|
be granted (§4). Other notes: `CalendarSource` gained a `syncsEvents` flag (read
|
||||||
behaviour, so leaving it would have been the one piece of false text on screen.
|
for #76's "not synced" label); and the Settings hint string was reworded rather
|
||||||
|
than dropped — it described the retired app-only behaviour, so leaving it would
|
||||||
|
have been the one piece of false text on screen.
|
||||||
Tracking: Codeberg #75. Split out of this work: #76 (read-only / not-synced
|
Tracking: Codeberg #75. Split out of this work: #76 (read-only / not-synced
|
||||||
communication), #77 (accounts merged by name).
|
communication), #77 (accounts merged by name).
|
||||||
|
|
||||||
@@ -112,8 +114,21 @@ which is what a user assumes, and the change does not leave the device.
|
|||||||
- Settings → Calendars row state derives from `isVisibleInSystem`.
|
- Settings → Calendars row state derives from `isVisibleInSystem`.
|
||||||
- Swap the display predicate from `id in disabledIds` to `!isVisibleInSystem` in
|
- Swap the display predicate from `id in disabledIds` to `!isVisibleInSystem` in
|
||||||
`CalendarRepositoryImpl.instances`/`searchEvents`, `EventEditViewModel` (form
|
`CalendarRepositoryImpl.instances`/`searchEvents`, `EventEditViewModel` (form
|
||||||
picker), `ImportViewModel` (import picker).
|
picker), `ImportViewModel` (import picker), `SettingsViewModel` (the
|
||||||
|
per-calendar reminder overrides — a default reminder on a switched-off calendar
|
||||||
|
could never fire).
|
||||||
- `hiddenCalendarIds` keeps working exactly as it does today, unioned as before.
|
- `hiddenCalendarIds` keeps working exactly as it does today, unioned as before.
|
||||||
|
- Those lists are the *targets* a user may pick. An event that already lives in an
|
||||||
|
excluded calendar keeps it: `EventEditViewModel` adds the event's own calendar
|
||||||
|
back to the picker when it isn't among them (review — otherwise editing an event
|
||||||
|
in a calendar switched off on this device renders the row as the "no calendar"
|
||||||
|
error, with an enabled picker that turns any pick into a calendar *move*).
|
||||||
|
- The repository caches one `Calendars` read per provider tick, and that cache is
|
||||||
|
keyed on the pending set as well as the tick (review). An id leaves the pending
|
||||||
|
set the instant its `VISIBLE` write lands, but the `ContentObserver` that
|
||||||
|
invalidates the snapshot is dispatched through the main looper — reading the
|
||||||
|
new set against the old snapshot re-admits exactly the events being hidden
|
||||||
|
until it arrives.
|
||||||
|
|
||||||
### 3. Deletions
|
### 3. Deletions
|
||||||
|
|
||||||
@@ -128,14 +143,30 @@ provider alert row behind them — a snooze re-shown from our own exact alarm,
|
|||||||
scheduled before the calendar was switched off, and a read-only install whose
|
scheduled before the calendar was switched off, and a read-only install whose
|
||||||
switch lives in the pending set. One choke point covers both receivers.
|
switch lives in the pending set. One choke point covers both receivers.
|
||||||
|
|
||||||
|
**Silencing is not handling** (second review). "`VISIBLE = 0` ⇒ no alert rows"
|
||||||
|
holds only where the flag was actually written; where the switch lives in the
|
||||||
|
pending set the provider keeps creating and broadcasting rows. Marking those
|
||||||
|
`STATE_FIRED` with the rest loses them for good — `dueAlerts` only ever returns
|
||||||
|
`STATE_SCHEDULED`. So the receiver marks what it *posted*, plus what it silenced
|
||||||
|
for an event already over (`handledAlertIds`); a silenced alert for an event
|
||||||
|
still ahead stays scheduled, which makes the provider's own table the stash the
|
||||||
|
deleted `SuppressedReminderStore` used to be. `ReminderRecovery` re-posts them
|
||||||
|
when the calendar is switched back on, so recovery doesn't wait for the next
|
||||||
|
unrelated broadcast.
|
||||||
|
|
||||||
Net code reduction. Update the now-false KDoc on `CalendarPrefs` claiming the toggle
|
Net code reduction. Update the now-false KDoc on `CalendarPrefs` claiming the toggle
|
||||||
"never touches the system's VISIBLE/SYNC_EVENTS flags, so other calendar apps are
|
"never touches the system's VISIBLE/SYNC_EVENTS flags, so other calendar apps are
|
||||||
unaffected".
|
unaffected".
|
||||||
|
|
||||||
### 4. Reconciliation (hide-only, standing)
|
### 4. Reconciliation (hide-only, standing)
|
||||||
|
|
||||||
`CalendarVisibilityReconciler` runs on every launch and again when the permission
|
`CalendarVisibilityReconciler` runs on every launch, and again whenever
|
||||||
screen grants the permissions. It drains
|
`RootScreen` comes up holding the calendar permission. That second trigger sat on
|
||||||
|
`PermissionViewModel.onGranted` at first, which only fires for the in-app request
|
||||||
|
— a permission granted on Android's own app-settings screen comes back through
|
||||||
|
`RootScreen`'s `ON_RESUME` and would have left the drain unrun for the session
|
||||||
|
(review). Settled runs cost two DataStore reads and return before the query. It
|
||||||
|
drains
|
||||||
`CalendarPrefs.pendingDisabledCalendarIds` — the retired `disabledCalendarIds`
|
`CalendarPrefs.pendingDisabledCalendarIds` — the retired `disabledCalendarIds`
|
||||||
key, re-read under a new name — into the provider:
|
key, re-read under a new name — into the provider:
|
||||||
|
|
||||||
@@ -167,6 +198,15 @@ reconciler arms a one-time dialog (`CalendarVisibilityNoticeDialog`) explaining
|
|||||||
that visibility now follows the device and where to change it. The answer —
|
that visibility now follows the device and where to change it. The answer —
|
||||||
including "nothing to say" — is stored, so it can never resurface later.
|
including "nothing to say" — is stored, so it can never resurface later.
|
||||||
|
|
||||||
|
**Upgrades only** (second review). The notice explains a change to behaviour the
|
||||||
|
user has seen before, and a device holding something hidden at system level is
|
||||||
|
the *norm* on a fresh install — a second account's calendars, "Holidays in …", a
|
||||||
|
subscribed calendar. Arming on that state alone put a changelog dialog in front
|
||||||
|
of first-run users. `firstInstallTime != lastUpdateTime` is the gate; a fresh
|
||||||
|
install retires the notice unshown, ahead of the permission check, so an app
|
||||||
|
update installed before the first grant can't make it look like an upgrade
|
||||||
|
afterwards.
|
||||||
|
|
||||||
### 4b. No `WRITE_CALENDAR`
|
### 4b. No `WRITE_CALENDAR`
|
||||||
|
|
||||||
Only `READ_CALENDAR` gates the app (`RootScreen`), and `PermissionScreen` says as
|
Only `READ_CALENDAR` gates the app (`RootScreen`), and `PermissionScreen` says as
|
||||||
@@ -177,7 +217,8 @@ that set filters `instances`/`searchEvents` and gates `ReminderNotifier.post`
|
|||||||
exactly as `VISIBLE` would. If `WRITE_CALENDAR` ever arrives, the reconciler
|
exactly as `VISIBLE` would. If `WRITE_CALENDAR` ever arrives, the reconciler
|
||||||
flushes it and the app-side copy disappears. This is the one place a second
|
flushes it and the app-side copy disappears. This is the one place a second
|
||||||
visibility model still exists, and it exists only where the first one is
|
visibility model still exists, and it exists only where the first one is
|
||||||
unwritable.
|
unwritable — and the one place the provider keeps creating alert rows for a
|
||||||
|
calendar the user switched off, which is why §3's silencing must stay reversible.
|
||||||
|
|
||||||
### 5. `sync_events = 0` rows
|
### 5. `sync_events = 0` rows
|
||||||
|
|
||||||
@@ -209,6 +250,10 @@ All JVM-testable:
|
|||||||
- the display-predicate swap, and the pending set filtering alongside it
|
- the display-predicate swap, and the pending set filtering alongside it
|
||||||
- the read-only path: no provider write, the choice parked in the pending set,
|
- the read-only path: no provider write, the choice parked in the pending set,
|
||||||
events filtered from it
|
events filtered from it
|
||||||
|
- what a due alert may be marked as handled (`handledAlertIds`), including the
|
||||||
|
silenced-but-still-ahead row that must stay scheduled
|
||||||
|
- the flush race: writing `VISIBLE` and releasing the pending id without a
|
||||||
|
provider tick in between must not re-admit the calendar's events
|
||||||
- `FakeCalendarDataSource` assertions that `setCalendarVisible` addresses a single
|
- `FakeCalendarDataSource` assertions that `setCalendarVisible` addresses a single
|
||||||
calendar by id
|
calendar by id
|
||||||
- one `Calendars` query per provider tick, however many flows are collecting
|
- one `Calendars` query per provider tick, however many flows are collecting
|
||||||
|
|||||||
Reference in New Issue
Block a user