fix(calendars): only tell upgrades about the visibility change, and reconcile on every grant
Two holes in the reconciler, both found by review. The one-time notice armed on any device holding a calendar at VISIBLE=0, which is the norm on a fresh install: a second account's calendars, "Holidays in X", a subscribed calendar. A brand-new user got a changelog dialog about a migration they never experienced, right after onboarding. It is gated on firstInstallTime != lastUpdateTime now, and a fresh install retires the notice unshown ahead of the permission check — so an update installed before the first grant can't make it look like an upgrade afterwards. The catch-up run hung off PermissionViewModel.onGranted, which only fires for the in-app request. Granting from Android's app-settings screen comes back through RootScreen's ON_RESUME, so an upgrading user who took that route kept their inherited switch-offs unflushed — their events filtered app-side while the provider went on scheduling the reminders they asked to stop. The trigger sits on RootScreen showing the app instead, which covers both routes. To keep that cheap, a settled run now returns after two DataStore reads instead of querying every calendar first. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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(
|
||||||
|
|||||||
@@ -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 =
|
||||||
|
|||||||
@@ -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(
|
||||||
|
|||||||
@@ -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() {
|
||||||
|
|||||||
Reference in New Issue
Block a user