From bb6e3ad336f7d168d7cd53f4afcc120381728c6e Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Sat, 25 Jul 2026 21:34:24 +0200 Subject: [PATCH] fix(calendars): only tell upgrades about the visibility change, and reconcile on every grant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../jeanlucmakiola/calendula/CalendulaApp.kt | 6 +-- .../calendar/CalendarVisibilityReconciler.kt | 53 ++++++++++++++----- .../jeanlucmakiola/calendula/ui/RootScreen.kt | 5 ++ .../ui/calendars/CalendarVisibilityNotice.kt | 14 +++++ .../ui/permission/PermissionViewModel.kt | 13 ++--- 5 files changed, 65 insertions(+), 26 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/CalendulaApp.kt b/app/src/main/java/de/jeanlucmakiola/calendula/CalendulaApp.kt index d87f65d..ba988f6 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/CalendulaApp.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/CalendulaApp.kt @@ -47,9 +47,9 @@ class CalendulaApp : Application() { * 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 * "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 - * leaves the set pending, and granting them on the permission screen runs it - * there instead. + * install and in the steady state; a launch without the calendar permission + * leaves the set pending, and `RootScreen` runs it again once the app comes + * up holding it — whichever way it was granted. */ private fun reconcileCalendarVisibility() { val deps = EntryPointAccessors.fromApplication( diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarVisibilityReconciler.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarVisibilityReconciler.kt index 0784c7b..bca56dd 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarVisibilityReconciler.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarVisibilityReconciler.kt @@ -11,7 +11,6 @@ import dagger.hilt.android.qualifiers.ApplicationContext import dagger.hilt.components.SingletonComponent import de.jeanlucmakiola.calendula.data.di.IoDispatcher import de.jeanlucmakiola.calendula.data.prefs.CalendarPrefs -import de.jeanlucmakiola.calendula.domain.CalendarSource import de.jeanlucmakiola.calendula.domain.calendarVisibilityPlan import de.jeanlucmakiola.calendula.domain.hasSystemHiddenCalendars import kotlinx.coroutines.CoroutineDispatcher @@ -28,15 +27,19 @@ import javax.inject.Singleton * model (#75), and the standing drain for switch-offs made without * `WRITE_CALENDAR`. * - * Runs on every launch, and again the moment the permission screen grants the - * calendar permissions. It is a no-op whenever the pending set is empty, which + * Runs on every launch, and again whenever the app comes up holding the calendar + * 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 * that dies part-way resumes exactly where it stopped and never re-applies a * write the user has since undone by hand. * * The reconciliation only hides (see [calendarVisibilityPlan]). Calendars hidden - * at system level stay hidden, and the first run that sees one arms the one-time - * notice explaining why Calendula no longer lists their events. + * at system level stay hidden, and on an *upgraded* install the first run that + * 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 class CalendarVisibilityReconciler @Inject constructor( @@ -52,10 +55,19 @@ class CalendarVisibilityReconciler @Inject constructor( // so an IOException from a damaged preferences file would otherwise take // the process down on every launch. 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 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() - armNoticeOnce(calendars, pending) + settleNoticeOnce(hasSystemHiddenCalendars(calendars, pending)) if (pending.isEmpty() || !hasPermission(Manifest.permission.WRITE_CALENDAR)) { return@withContext } @@ -79,15 +91,30 @@ class CalendarVisibilityReconciler @Inject constructor( } /** - * Arm the one-time notice if the device holds a calendar switched off - * outside Calendula. Evaluated once, on the first run that can read the - * calendars at all; the answer — including "nothing to say" — is stored, so - * the notice can't resurface later, when the same state would no longer be - * news to the user. + * Settle the one-time notice: [pending] arms it, false retires it unshown. + * Answered once, by whichever run can answer it first; the answer is stored + * either way, so the notice can't resurface later, when the same state would + * no longer be news to the user. */ - private suspend fun armNoticeOnce(calendars: List, pending: Set) { + private suspend fun settleNoticeOnce(pending: Boolean) { 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 = diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/RootScreen.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/RootScreen.kt index aee5ac2..33385ff 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/RootScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/RootScreen.kt @@ -7,6 +7,7 @@ import androidx.compose.material3.ExperimentalMaterial3ExpressiveApi import androidx.compose.material3.MaterialTheme import androidx.compose.runtime.Composable import androidx.compose.runtime.DisposableEffect +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.setValue @@ -84,6 +85,10 @@ fun RootScreen( // visibility (#75); armed by the reconciler, shown over the app. val visibilityNotice: CalendarVisibilityNoticeViewModel = hiltViewModel() 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) { CalendarVisibilityNoticeDialog(onDismiss = visibilityNotice::dismiss) } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarVisibilityNotice.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarVisibilityNotice.kt index faf6313..98c3356 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarVisibilityNotice.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarVisibilityNotice.kt @@ -12,6 +12,7 @@ import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel import de.jeanlucmakiola.calendula.R +import de.jeanlucmakiola.calendula.data.calendar.CalendarVisibilityReconciler import de.jeanlucmakiola.calendula.data.prefs.CalendarPrefs import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow @@ -30,8 +31,21 @@ import javax.inject.Inject @HiltViewModel class CalendarVisibilityNoticeViewModel @Inject constructor( private val prefs: CalendarPrefs, + private val reconciler: CalendarVisibilityReconciler, ) : 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 = prefs.visibilityNoticePending .map { it == true } .stateIn( diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/permission/PermissionViewModel.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/permission/PermissionViewModel.kt index b12a257..6cc03eb 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/permission/PermissionViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/permission/PermissionViewModel.kt @@ -1,29 +1,22 @@ package de.jeanlucmakiola.calendula.ui.permission import androidx.lifecycle.ViewModel -import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel -import de.jeanlucmakiola.calendula.data.calendar.CalendarVisibilityReconciler import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow -import kotlinx.coroutines.launch import javax.inject.Inject @HiltViewModel -class PermissionViewModel @Inject constructor( - private val visibilityReconciler: CalendarVisibilityReconciler, -) : ViewModel() { +class PermissionViewModel @Inject constructor() : ViewModel() { private val _state = MutableStateFlow(PermissionUiState.Rationale) val state: StateFlow = _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() { _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() {