From 41593a0e9d153d7fd049d9915b3e9080fec47b7d Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Sat, 25 Jul 2026 12:44:56 +0200 Subject: [PATCH] fix(calendars): make the Settings toggle the one visibility model (#75) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reminders never fired for a calendar hidden at system level and nothing hinted at it: the provider only schedules reminder alarms for Calendars.VISIBLE=1, while Calendula filtered with its own disabledCalendarIds pref and parsed isVisibleInSystem without ever using it — two models that could disagree indefinitely. Settings → Calendars now writes Calendars.VISIBLE, one calendar per update (CalendarProvider2 skips its own checkNextAlarm() reschedule for any selection that isn't _id=), and every display predicate reads isVisibleInSystem. The drawer's filter sheet stays a purely in-app declutter and still leaves reminders alone. With VISIBLE=0 the provider creates no alert rows, so there is nothing left to suppress: the disabled-calendar gates, SuppressedReminderStore and the re-enable recovery are gone. A one-shot migration reconciles the retired set with the app's state winning — enabled in-app and syncing gets shown, disabled gets hidden, everything else untouched — so the upgrade changes nothing the user sees. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 16 +++ .../jeanlucmakiola/calendula/CalendulaApp.kt | 17 +++ .../data/calendar/CalendarDataSource.kt | 31 +++++ .../calendula/data/calendar/CalendarMapper.kt | 5 + .../data/calendar/CalendarRepository.kt | 9 ++ .../data/calendar/CalendarRepositoryImpl.kt | 56 +++++--- .../calendar/CalendarVisibilityMigration.kt | 85 ++++++++++++ .../calendula/data/calendar/Projections.kt | 2 + .../calendula/data/prefs/CalendarPrefs.kt | 46 ++++--- .../data/reminders/EventReminderReceiver.kt | 41 +----- .../data/reminders/ReminderNotifier.kt | 7 - .../data/reminders/SuppressedReminderStore.kt | 130 ------------------ .../domain/CalendarVisibilityPlan.kt | 45 ++++++ .../jeanlucmakiola/calendula/domain/Models.kt | 15 ++ .../calendula/ui/calendars/CalendarsScreen.kt | 41 +++--- .../ui/calendars/CalendarsViewModel.kt | 76 ++-------- .../calendula/ui/edit/EventEditViewModel.kt | 20 ++- .../calendula/ui/filter/FilterViewModel.kt | 11 +- .../calendula/ui/imports/ImportViewModel.kt | 16 +-- .../ui/permission/PermissionViewModel.kt | 12 +- app/src/main/res/values/strings.xml | 4 +- .../data/calendar/CalendarMapperTest.kt | 14 ++ .../calendar/CalendarRepositoryImplTest.kt | 81 ++++++++--- .../data/calendar/FakeCalendarDataSource.kt | 13 ++ .../calendula/data/prefs/CalendarPrefsTest.kt | 55 +++++--- .../data/reminders/PostableAlertsTest.kt | 71 ---------- .../reminders/SuppressedReminderStoreTest.kt | 129 ----------------- .../domain/CalendarVisibilityPlanTest.kt | 96 +++++++++++++ .../ui/edit/EventEditViewModelTest.kt | 22 ++- docs/ARCHITECTURE.md | 9 ++ 30 files changed, 619 insertions(+), 556 deletions(-) create mode 100644 app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarVisibilityMigration.kt delete mode 100644 app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/SuppressedReminderStore.kt create mode 100644 app/src/main/java/de/jeanlucmakiola/calendula/domain/CalendarVisibilityPlan.kt delete mode 100644 app/src/test/java/de/jeanlucmakiola/calendula/data/reminders/PostableAlertsTest.kt delete mode 100644 app/src/test/java/de/jeanlucmakiola/calendula/data/reminders/SuppressedReminderStoreTest.kt create mode 100644 app/src/test/java/de/jeanlucmakiola/calendula/domain/CalendarVisibilityPlanTest.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index c16f6ae..a0f8157 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- Reminders now arrive for every calendar you have switched on. A calendar that + was hidden at system level — switched off in another calendar app, or never + switched on after being added — still showed its events and listed their + reminders in Calendula, but never notified: Android only schedules reminder + alarms for calendars marked visible, and Calendula kept its own separate + on/off list that had no say in it. There is now one switch: **Settings → + Calendars** turns a calendar on or off for the whole device, so what you see + and what reminds you can no longer disagree. Your current selection is carried + over on first launch, and calendars whose events aren't stored on this device + are left as they are ([#75]). + + The drawer's filter is unchanged and still app-only: hiding a calendar there + tidies your view without silencing its reminders. + ## [2.16.0] — 2026-07-24 ### Added @@ -1112,3 +1127,4 @@ automatically, with zero telemetry and no internet permission. [#42]: https://codeberg.org/jlmakiola/calendula/issues/42 [#44]: https://codeberg.org/jlmakiola/calendula/issues/44 [#70]: https://codeberg.org/jlmakiola/calendula/issues/70 +[#75]: https://codeberg.org/jlmakiola/calendula/issues/75 diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/CalendulaApp.kt b/app/src/main/java/de/jeanlucmakiola/calendula/CalendulaApp.kt index 87fd0a2..d9044c5 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/CalendulaApp.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/CalendulaApp.kt @@ -4,6 +4,7 @@ import android.app.Application import dagger.hilt.android.EntryPointAccessors import dagger.hilt.android.HiltAndroidApp import de.jeanlucmakiola.calendula.data.backup.BackupScheduler +import de.jeanlucmakiola.calendula.data.calendar.CalendarVisibilityMigration import de.jeanlucmakiola.calendula.data.backup.BackupWorker import de.jeanlucmakiola.calendula.data.contacts.SpecialDatesScheduler import de.jeanlucmakiola.calendula.data.contacts.SpecialDatesSyncWorker @@ -39,6 +40,22 @@ class CalendulaApp : Application() { ) reconcileAutoBackup() reconcileSpecialDates() + migrateCalendarVisibility() + } + + /** + * Fold the retired app-local "disabled calendars" set into the system's + * `Calendars.VISIBLE` once (#75). A no-op after it has run, and on a fresh + * install; a launch without the calendar permissions leaves it pending, and + * granting them on the permission screen runs it there instead. + */ + private fun migrateCalendarVisibility() { + val deps = EntryPointAccessors.fromApplication( + this, CalendarVisibilityMigration.Deps::class.java, + ) + CoroutineScope(SupervisorJob() + Dispatchers.Default).launch { + deps.calendarVisibilityMigration().runIfNeeded() + } } /** diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarDataSource.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarDataSource.kt index ac0b56e..77fe9b8 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarDataSource.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarDataSource.kt @@ -110,6 +110,16 @@ interface CalendarDataSource { /** Permanently delete a local calendar the app owns, with all its events. */ fun deleteCalendar(id: Long) + /** + * Show or hide the calendar device-wide by writing `Calendars.VISIBLE` — the + * app's one visibility model (#75). `VISIBLE` also gates the provider's own + * reminder scheduling, so switching a calendar off here is what actually + * stops its notifications; switching it on is what brings them back. + * Writable by a plain app (one of the three columns the platform documents + * as such) and device-local — no sync adapter pushes it anywhere. + */ + fun setCalendarVisible(id: Long, visible: Boolean) + /** * Create a local calendar tagged as the special-dates mirror for [type] * (a marker in `CAL_SYNC2`); returns its `Calendars._ID`. Otherwise a normal @@ -373,6 +383,27 @@ class AndroidCalendarDataSource @Inject constructor( if (deleted == 0) throw WriteFailedException("delete calendar id=$id") } + /** + * Addressed by appended id on the plain (non-sync-adapter) Calendars URI, + * one calendar per call. Both parts are load-bearing: + * `CalendarProvider2.updateInTransaction` short-circuits to a raw database + * update unless the selection is `_id=…`, skipping the dirty marking *and* + * the `checkNextAlarm()` reschedule — i.e. an `_id IN (…)` batch would write + * the flag but never re-arm the reminder alarms this write exists to + * trigger. The sync-adapter URI is avoided so the write also applies to + * synced calendars, which is where the bug bites. + */ + override fun setCalendarVisible(id: Long, visible: Boolean) { + val values = ContentValues().apply { + put(CalendarContract.Calendars.VISIBLE, if (visible) 1 else 0) + } + val rows = resolver.update( + ContentUris.withAppendedId(CalendarContract.Calendars.CONTENT_URI, id), + values, null, null, + ) + if (rows == 0) throw WriteFailedException("set calendar visibility id=$id") + } + override fun createManagedCalendar(displayName: String, color: Int, type: SpecialDateType): Long { val name = displayName.trim().ifEmpty { Fallbacks.UNNAMED_CALENDAR } val values = ContentValues().apply { diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarMapper.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarMapper.kt index c4d214a..af2ac96 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarMapper.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarMapper.kt @@ -31,5 +31,10 @@ internal fun ColumnReader.toCalendarSource(): CalendarSource { isManaged = isLocal && getString(CalendarProjection.IDX_MANAGED_MARKER) ?.startsWith(CalendarProjection.MANAGED_MARKER_PREFIX) == true, + // A provider that leaves the column NULL is treated as syncing — the + // harmless default, since this flag only ever holds the one-shot + // visibility migration back from switching a calendar on. + syncsEvents = isNull(CalendarProjection.IDX_SYNC_EVENTS) || + getInt(CalendarProjection.IDX_SYNC_EVENTS) != 0, ) } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepository.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepository.kt index c006159..1dcd32a 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepository.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepository.kt @@ -38,6 +38,15 @@ interface CalendarRepository { /** Permanently delete a local calendar the app owns, with all its events. */ suspend fun deleteCalendar(id: Long) + /** + * Show or hide [ids] device-wide (`Calendars.VISIBLE`), which is also what + * turns the provider's reminder scheduling for them on or off — see + * [CalendarDataSource.setCalendarVisible]. Each calendar is written on its + * own, in order; a failure part-way leaves the earlier writes standing (the + * observer reports whatever actually landed). + */ + suspend fun setCalendarsVisible(ids: Collection, visible: Boolean) + /** * Every event of the writable local calendars, ready to serialise into a * whole-calendar `.ics` backup (see [CalendarDataSource.exportableEvents]). diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepositoryImpl.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepositoryImpl.kt index f9528d2..059d473 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepositoryImpl.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepositoryImpl.kt @@ -57,42 +57,59 @@ class CalendarRepositoryImpl @Inject constructor( .reQuery { dataSource.calendars() } .flowOn(io) - // Instances are filtered by the app-side hidden ∪ disabled calendar sets - // (M3): an event is dropped whenever the user has hidden *or* disabled its - // calendar. Re-runs when the provider ticks *or* either set changes — - // toggling a calendar in the filter sheet or the calendar manager updates - // every view immediately. [calendars] stays unfiltered so those screens can - // list and re-enable hidden/disabled calendars. + // Instances are filtered by the system's per-calendar VISIBLE flag ∪ the + // app-side hidden set: an event is dropped when the user switched its + // calendar off in Settings → Calendars (which also stops the provider + // scheduling its reminders) *or* hid it in the filter sheet. Re-runs when + // the provider ticks — writing VISIBLE notifies, so switching a calendar + // updates every view — or when the hidden set changes. [calendars] stays + // unfiltered so those screens can list and re-enable invisible calendars. override fun instances(range: ClosedRange): Flow> = combine( ticks .onStart { emit(Unit) } .reQuery { - dataSource.instances( - beginMillis = range.start.toEpochMillis(), - endMillis = range.endInclusive.toEpochMillis(), + // Both reads in one pass, so a list of instances is never + // filtered against a visibility snapshot from another tick. + QueriedInstances( + instances = dataSource.instances( + beginMillis = range.start.toEpochMillis(), + endMillis = range.endInclusive.toEpochMillis(), + ), + invisibleCalendarIds = invisibleCalendarIds(), ) }, prefs.hiddenCalendarIds, - prefs.disabledCalendarIds, - ) { instances, hidden, disabled -> - val excluded = hidden + disabled - if (excluded.isEmpty()) instances - else instances.filterNot { it.calendarId in excluded } + ) { queried, hidden -> + val excluded = hidden + queried.invisibleCalendarIds + if (excluded.isEmpty()) queried.instances + else queried.instances.filterNot { it.calendarId in excluded } } - // hidden and disabled both derive from one DataStore, so toggling - // either makes both re-emit and combine briefly surfaces the same - // list twice — collapse the duplicate so views don't re-render for it. + // Any DataStore edit re-emits the hidden set even when it is + // unchanged (e.g. writing the last-used calendar), which would + // re-surface an identical list — collapse those so views don't + // re-render for them. .distinctUntilChanged() .flowOn(io) + /** One instances query plus the visibility it must be filtered against. */ + private data class QueriedInstances( + val instances: List, + val invisibleCalendarIds: Set, + ) + + /** Calendars switched off at system level — hidden, and never reminded about. */ + private fun invisibleCalendarIds(): Set = dataSource.calendars() + .filterNot { it.isVisibleInSystem } + .mapTo(mutableSetOf()) { it.id } + override suspend fun eventDetail(eventId: Long): EventDetail = withContext(io) { dataSource.eventDetail(eventId) ?: throw NoSuchEventException(eventId) } override suspend fun searchEvents(query: String): List = withContext(io) { if (query.isBlank()) return@withContext emptyList() - val excluded = prefs.hiddenCalendarIds.first() + prefs.disabledCalendarIds.first() + val excluded = prefs.hiddenCalendarIds.first() + invisibleCalendarIds() dataSource.searchEvents(query) .let { if (excluded.isEmpty()) it else it.filterNot { e -> e.calendarId in excluded } } } @@ -118,6 +135,9 @@ class CalendarRepositoryImpl @Inject constructor( override suspend fun deleteCalendar(id: Long) = withContext(io) { dataSource.deleteCalendar(id) } + override suspend fun setCalendarsVisible(ids: Collection, visible: Boolean) = + withContext(io) { ids.forEach { dataSource.setCalendarVisible(it, visible) } } + override suspend fun exportEvents(calendarIds: Set?) = withContext(io) { dataSource.exportableEvents(calendarIds) } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarVisibilityMigration.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarVisibilityMigration.kt new file mode 100644 index 0000000..9d7a656 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarVisibilityMigration.kt @@ -0,0 +1,85 @@ +package de.jeanlucmakiola.calendula.data.calendar + +import android.Manifest +import android.content.Context +import android.content.pm.PackageManager +import android.util.Log +import androidx.core.content.ContextCompat +import dagger.hilt.EntryPoint +import dagger.hilt.InstallIn +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.calendarVisibilityPlan +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.withContext +import kotlin.coroutines.cancellation.CancellationException +import javax.inject.Inject +import javax.inject.Singleton + +/** + * One-shot reconciliation of the two visibility models Calendula used to run in + * parallel (#75): the app-local "disabled calendars" set is folded into the + * system's `Calendars.VISIBLE`, which from now on is the only one. See + * [calendarVisibilityPlan] for the rules — the app's state wins, so what the + * user sees today is exactly what they keep seeing after the upgrade. + * + * Runs at most once. Both permissions are required (the flag is a provider + * write), so a run that finds them missing changes nothing and leaves the guard + * unset — the next launch, or the moment the permission screen grants them, + * tries again. Only once the writes land is the guard set and the retired key + * dropped. + */ +@Singleton +class CalendarVisibilityMigration @Inject constructor( + @ApplicationContext private val context: Context, + private val dataSource: CalendarDataSource, + private val prefs: CalendarPrefs, + @IoDispatcher private val io: CoroutineDispatcher, +) { + + suspend fun runIfNeeded() = withContext(io) { + if (prefs.visibilityMigrationDone.first()) return@withContext + if (!hasCalendarPermissions()) return@withContext + try { + val plan = calendarVisibilityPlan( + calendars = dataSource.calendars(), + disabledCalendarIds = prefs.legacyDisabledCalendarIds(), + ) + // One calendar per write: the provider skips its reminder-alarm + // reschedule for anything but a single-id update (see + // [CalendarDataSource.setCalendarVisible]). + plan.hide.forEach { dataSource.setCalendarVisible(it, false) } + plan.show.forEach { dataSource.setCalendarVisible(it, true) } + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + // A part-applied plan is safe to redo: every write is idempotent and + // the source set is still on disk, so leave the guard unset and try + // again next launch rather than stranding the user half-migrated. + Log.w(TAG, "Calendar visibility migration failed; will retry", e) + return@withContext + } + prefs.setVisibilityMigrationDone() + prefs.clearLegacyDisabledCalendarIds() + } + + private fun hasCalendarPermissions(): Boolean = + listOf(Manifest.permission.READ_CALENDAR, Manifest.permission.WRITE_CALENDAR) + .all { + ContextCompat.checkSelfPermission(context, it) == PackageManager.PERMISSION_GRANTED + } + + /** Lets non-injectable entry points (the Application) reach the migration. */ + @EntryPoint + @InstallIn(SingletonComponent::class) + interface Deps { + fun calendarVisibilityMigration(): CalendarVisibilityMigration + } + + private companion object { + const val TAG = "CalendarVisibilityMigration" + } +} diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/Projections.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/Projections.kt index 774e19b..21eb4e7 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/Projections.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/Projections.kt @@ -19,6 +19,7 @@ internal object CalendarProjection { // uses to recognise its own managed calendars, independent of any // stored preference id (which a backup restore / data wipe can lose). MANAGED_MARKER_COLUMN, + CalendarContract.Calendars.SYNC_EVENTS, ) const val DESCRIPTION_COLUMN: String = CalendarContract.Calendars.CAL_SYNC1 @@ -36,6 +37,7 @@ internal object CalendarProjection { const val IDX_ACCESS_LEVEL = 6 const val IDX_DESCRIPTION = 7 const val IDX_MANAGED_MARKER = 8 + const val IDX_SYNC_EVENTS = 9 } internal object InstanceProjection { diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/prefs/CalendarPrefs.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/prefs/CalendarPrefs.kt index bc01927..8c85d8a 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/prefs/CalendarPrefs.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/prefs/CalendarPrefs.kt @@ -2,17 +2,21 @@ package de.jeanlucmakiola.calendula.data.prefs import androidx.datastore.core.DataStore import androidx.datastore.preferences.core.Preferences +import androidx.datastore.preferences.core.booleanPreferencesKey import androidx.datastore.preferences.core.edit import androidx.datastore.preferences.core.longPreferencesKey import androidx.datastore.preferences.core.stringPreferencesKey import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.map import javax.inject.Inject import javax.inject.Singleton /** - * App-side preference for "calendars the user has hidden in this app", - * separate from the system's per-calendar VISIBLE flag. + * App-side preference for "calendars the user has hidden in this app" — the + * drawer's filter sheet, a purely in-app declutter. It deliberately does *not* + * suppress reminders; switching a calendar off entirely is the system's + * `Calendars.VISIBLE` flag, written straight to the provider (#75). * * Persisted as a comma-separated string of Long ids; non-numeric tokens are * silently dropped (defensive — see CalendarPrefsTest). @@ -40,28 +44,35 @@ class CalendarPrefs @Inject constructor( } /** - * App-side preference for "calendars the user has disabled in this app" — a - * heavier level than [hiddenCalendarIds]. A disabled calendar is removed from - * every surface (drawer filter, event-form picker, import picker) and its - * events never appear; it stays listed only in Settings → Calendars so it can - * be re-enabled. Stored exactly like the hidden set; never touches the - * system's VISIBLE/SYNC_EVENTS flags, so other calendar apps are unaffected. + * The retired app-local "disabled calendars" set, readable only so the + * one-shot visibility migration can reconcile it into `Calendars.VISIBLE` + * (see CalendarVisibilityMigration). Nothing else may consult it — it is a + * second visibility model, which is exactly what #75 removed. Dropped by + * [clearLegacyDisabledCalendarIds] once the migration has run. */ - val disabledCalendarIds: Flow> = store.data.map { prefs -> + suspend fun legacyDisabledCalendarIds(): Set = store.data.first().let { prefs -> prefs[DISABLED_IDS_KEY].orEmpty() .split(',') .mapNotNull { it.trim().toLongOrNull() } .toSet() } - suspend fun setDisabledCalendarIds(ids: Set) { - store.edit { prefs -> - if (ids.isEmpty()) { - prefs.remove(DISABLED_IDS_KEY) - } else { - prefs[DISABLED_IDS_KEY] = ids.sorted().joinToString(",") - } - } + suspend fun clearLegacyDisabledCalendarIds() { + store.edit { prefs -> prefs.remove(DISABLED_IDS_KEY) } + } + + /** + * Whether the one-shot reconciliation of the retired disabled set into + * `Calendars.VISIBLE` has run. Set only once it actually wrote (it needs + * `WRITE_CALENDAR`), so a run skipped for a missing permission is retried on + * the next launch. + */ + val visibilityMigrationDone: Flow = store.data.map { prefs -> + prefs[VISIBILITY_MIGRATION_KEY] == true + } + + suspend fun setVisibilityMigrationDone() { + store.edit { prefs -> prefs[VISIBILITY_MIGRATION_KEY] = true } } /** @@ -79,6 +90,7 @@ class CalendarPrefs @Inject constructor( companion object { internal val HIDDEN_IDS_KEY = stringPreferencesKey("hidden_calendar_ids") internal val DISABLED_IDS_KEY = stringPreferencesKey("disabled_calendar_ids") + internal val VISIBILITY_MIGRATION_KEY = booleanPreferencesKey("visibility_migration_done") internal val LAST_USED_CALENDAR_KEY = longPreferencesKey("last_used_calendar_id") } } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/EventReminderReceiver.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/EventReminderReceiver.kt index 22fc5cf..a83cc52 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/EventReminderReceiver.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/EventReminderReceiver.kt @@ -8,7 +8,6 @@ import android.content.pm.PackageManager import android.provider.CalendarContract import androidx.core.content.ContextCompat import dagger.hilt.android.AndroidEntryPoint -import de.jeanlucmakiola.calendula.data.prefs.CalendarPrefs import de.jeanlucmakiola.calendula.data.prefs.SettingsPrefs import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers @@ -17,29 +16,6 @@ import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch import javax.inject.Inject -/** - * True when [this] alert belongs to a calendar the user disabled in-app, so its - * reminder must be suppressed (mirroring the event filtering in - * CalendarRepositoryImpl). Alerts whose calendar is unknown (id 0L — e.g. a - * pre-upgrade snooze PendingIntent minted before EXTRA_CALENDAR_ID existed) are - * never treated as disabled. This is the one predicate the disabled-calendar - * gate is built from: [postableAlerts] here and the choke point in - * [ReminderNotifier.post] both use it. - */ -internal fun ReminderAlert.isForDisabledCalendar(disabledCalendarIds: Set): Boolean = - calendarId != 0L && calendarId in disabledCalendarIds - -/** - * The due alerts that should actually surface as notifications: everything - * except alerts whose calendar the user has disabled in-app. The caller still - * marks the full due set fired, so suppressed alerts are not re-broadcast by the - * provider. - */ -internal fun postableAlerts( - due: List, - disabledCalendarIds: Set, -): List = due.filterNot { it.isForDisabledCalendar(disabledCalendarIds) } - /** * Becomes the app that turns the calendar provider's reminder alarms into * visible notifications (the Etar model — the provider broadcasts @@ -49,6 +25,10 @@ internal fun postableAlerts( * we query every still-scheduled, due `CalendarAlerts` row ourselves, post * them, and mark them fired. Posting happens before marking — a crash in * between re-posts silently (same tag) rather than losing the reminder. + * + * There is no per-calendar filtering here: a calendar switched off in + * Settings → Calendars has `Calendars.VISIBLE = 0`, and the provider creates no + * alert rows for it in the first place (#75). */ @AndroidEntryPoint class EventReminderReceiver : BroadcastReceiver() { @@ -56,8 +36,6 @@ class EventReminderReceiver : BroadcastReceiver() { @Inject lateinit var alertStore: ReminderAlertStore @Inject lateinit var notifier: ReminderNotifier @Inject lateinit var settingsPrefs: SettingsPrefs - @Inject lateinit var calendarPrefs: CalendarPrefs - @Inject lateinit var suppressedStore: SuppressedReminderStore override fun onReceive(context: Context, intent: Intent) { if (intent.action != CalendarContract.ACTION_EVENT_REMINDER) return @@ -72,17 +50,8 @@ class EventReminderReceiver : BroadcastReceiver() { if (settingsPrefs.remindersEnabled.first()) { val now = System.currentTimeMillis() val due = alertStore.dueAlerts(now) - val disabled = calendarPrefs.disabledCalendarIds.first() - val postable = postableAlerts(due, disabled) - // Suppress reminders for disabled calendars, but still mark - // every due alert fired so the provider stops re-broadcasting - // the suppressed ones. Stash those suppressed alerts so - // re-enabling their calendar can recover them (they would - // otherwise stay STATE_FIRED forever with no re-scan). - postable.forEach { notifier.post(it) } + due.forEach { notifier.post(it) } alertStore.markFired(due.map { it.alertId }, now) - suppressedStore.stash(due - postable.toSet(), now) - suppressedStore.purgeExpired(now) } } finally { pendingResult.finish() diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderNotifier.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderNotifier.kt index 73a4656..6c62972 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderNotifier.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderNotifier.kt @@ -14,7 +14,6 @@ import androidx.core.content.ContextCompat import dagger.hilt.android.qualifiers.ApplicationContext import de.jeanlucmakiola.calendula.MainActivity import de.jeanlucmakiola.calendula.R -import de.jeanlucmakiola.calendula.data.prefs.CalendarPrefs import de.jeanlucmakiola.calendula.data.prefs.SettingsPrefs import de.jeanlucmakiola.calendula.data.prefs.is24Hour import de.jeanlucmakiola.calendula.data.prefs.resolveFirstDay @@ -38,7 +37,6 @@ import javax.inject.Singleton class ReminderNotifier @Inject constructor( @ApplicationContext private val context: Context, private val settingsPrefs: SettingsPrefs, - private val calendarPrefs: CalendarPrefs, ) { /** False when the user declined `POST_NOTIFICATIONS` or muted the app. */ @@ -50,11 +48,6 @@ class ReminderNotifier @Inject constructor( } suspend fun post(alert: ReminderAlert) { - // The single choke point for the disabled-calendar gate: it covers both - // the provider broadcast (EventReminderReceiver) and a snoozed re-show - // (ReminderActionReceiver), so a calendar disabled after a snooze no - // longer notifies — without either receiver duplicating the check. - if (alert.isForDisabledCalendar(calendarPrefs.disabledCalendarIds.first())) return ensureChannel() val title = alert.title.ifBlank { context.getString(R.string.event_untitled) } val is24Hour = settingsPrefs.timeFormat.first() diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/SuppressedReminderStore.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/SuppressedReminderStore.kt deleted file mode 100644 index eb5b6e0..0000000 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/SuppressedReminderStore.kt +++ /dev/null @@ -1,130 +0,0 @@ -package de.jeanlucmakiola.calendula.data.reminders - -import androidx.datastore.core.DataStore -import androidx.datastore.preferences.core.MutablePreferences -import androidx.datastore.preferences.core.Preferences -import androidx.datastore.preferences.core.edit -import androidx.datastore.preferences.core.stringSetPreferencesKey -import java.util.Base64 -import javax.inject.Inject -import javax.inject.Singleton - -/** - * 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). Used both to decide what to re-post and to purge the stash. - */ -internal fun ReminderAlert.isRelevantAt(nowMillis: Long): Boolean = - (endMillis.takeIf { it > 0L } ?: beginMillis) >= nowMillis - -/** - * Local stash of reminder alerts that fired while their calendar was disabled - * in-app. [EventReminderReceiver] marks every due alert `STATE_FIRED` regardless - * (so the provider stops re-broadcasting the suppressed ones), which would - * otherwise lose those reminders forever — there is no re-scan. Stashing lets - * [de.jeanlucmakiola.calendula.ui.calendars.CalendarsViewModel] re-post them if - * the user re-enables the calendar before the event is over. - * - * Persisted in the shared preferences DataStore as a set of self-describing - * strings (one per alert); the stash never reaches a screen, so there is no - * domain model. Entries whose event has already ended are dropped on the next - * stash/recover/purge, so the stash only ever holds a handful of pending alerts. - */ -@Singleton -class SuppressedReminderStore @Inject constructor( - private val store: DataStore, -) { - - /** Add [alerts] to the stash, replacing any existing entry with the same id. */ - suspend fun stash(alerts: List, nowMillis: Long) { - if (alerts.isEmpty()) return - store.edit { prefs -> - val byId = decodeAll(prefs).associateByTo(mutableMapOf()) { it.alertId } - alerts.forEach { byId[it.alertId] = it } - prefs.putStash(byId.values.filter { it.isRelevantAt(nowMillis) }) - } - } - - /** - * Remove and return the still-relevant stashed alerts belonging to any of - * [calendarIds]; drops expired entries for every calendar in passing. - */ - suspend fun recoverFor(calendarIds: Set, nowMillis: Long): List { - val recovered = mutableListOf() - store.edit { prefs -> - val kept = decodeAll(prefs).filter { alert -> - when { - !alert.isRelevantAt(nowMillis) -> false // expired: drop - alert.calendarId in calendarIds -> { recovered += alert; false } - else -> true - } - } - prefs.putStash(kept) - } - return recovered - } - - /** Drop entries whose event has already ended — cheap opportunistic cleanup. */ - suspend fun purgeExpired(nowMillis: Long) { - store.edit { prefs -> - prefs.putStash(decodeAll(prefs).filter { it.isRelevantAt(nowMillis) }) - } - } - - private fun decodeAll(prefs: Preferences): List = - prefs[KEY].orEmpty().mapNotNull { decodeStashEntry(it) } - - private fun MutablePreferences.putStash(alerts: List) { - val encoded = alerts.map { encodeStashEntry(it) }.toSet() - if (encoded.isEmpty()) remove(KEY) else set(KEY, encoded) - } - - private companion object { - val KEY = stringSetPreferencesKey("suppressed_reminders") - } -} - -// One stash entry as a delimited string. The '|' separator is safe because every -// free-text field is Base64-encoded first (that alphabet never contains '|'), and -// a null location is stored as a distinct sentinel that Base64 also never yields. -private const val FIELD_SEP = "|" -private const val NULL_LOCATION = "-" - -internal fun encodeStashEntry(alert: ReminderAlert): String = listOf( - alert.alertId.toString(), - alert.eventId.toString(), - alert.calendarId.toString(), - alert.beginMillis.toString(), - alert.endMillis.toString(), - if (alert.isAllDay) "1" else "0", - alert.title.toBase64(), - alert.location?.toBase64() ?: NULL_LOCATION, -).joinToString(FIELD_SEP) - -/** Reverse of [encodeStashEntry]; returns null for a malformed entry (dropped). */ -internal fun decodeStashEntry(raw: String): ReminderAlert? { - val parts = raw.split(FIELD_SEP) - if (parts.size != 8) return null - return try { - ReminderAlert( - alertId = parts[0].toLong(), - eventId = parts[1].toLong(), - calendarId = parts[2].toLong(), - beginMillis = parts[3].toLong(), - endMillis = parts[4].toLong(), - title = parts[6].fromBase64(), - location = parts[7].takeIf { it != NULL_LOCATION }?.fromBase64(), - isAllDay = parts[5] == "1", - ) - } catch (e: NumberFormatException) { - null - } catch (e: IllegalArgumentException) { // bad Base64 - null - } -} - -private fun String.toBase64(): String = - Base64.getEncoder().encodeToString(toByteArray(Charsets.UTF_8)) - -private fun String.fromBase64(): String = - String(Base64.getDecoder().decode(this), Charsets.UTF_8) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/domain/CalendarVisibilityPlan.kt b/app/src/main/java/de/jeanlucmakiola/calendula/domain/CalendarVisibilityPlan.kt new file mode 100644 index 0000000..e3e0ca6 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/calendula/domain/CalendarVisibilityPlan.kt @@ -0,0 +1,45 @@ +package de.jeanlucmakiola.calendula.domain + +/** + * The `Calendars.VISIBLE` writes that bring the system flag in line with the + * app's retired disabled set. Empty when the two already agree. + */ +data class CalendarVisibilityPlan( + val show: Set = emptySet(), + val hide: Set = emptySet(), +) { + val isEmpty: Boolean get() = show.isEmpty() && hide.isEmpty() +} + +/** + * Reconcile the two visibility models Calendula used to keep in parallel (#75), + * with the app's state winning: + * + * - enabled in-app but hidden at system level → show it, *unless* the calendar + * doesn't sync its events to the device ([CalendarSource.syncsEvents]). Those + * hold no local events, so switching them on would produce nothing but an + * "enabled yet permanently empty" row; left off, they read honestly as off. + * - disabled in-app → hide it at system level, which is what stops the provider + * scheduling its reminders. + * - anything already in agreement is left untouched, so the migration writes as + * little as possible. + * + * App-state-wins is also what keeps the upgrade invisible: reconciling the other + * way would make events the user sees today vanish. + */ +fun calendarVisibilityPlan( + calendars: List, + disabledCalendarIds: Set, +): CalendarVisibilityPlan { + val show = mutableSetOf() + val hide = mutableSetOf() + for (calendar in calendars) { + val disabledInApp = calendar.id in disabledCalendarIds + when { + disabledInApp && calendar.isVisibleInSystem -> hide += calendar.id + !disabledInApp && !calendar.isVisibleInSystem && calendar.syncsEvents -> + show += calendar.id + } + } + return CalendarVisibilityPlan(show = show, hide = hide) +} diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/domain/Models.kt b/app/src/main/java/de/jeanlucmakiola/calendula/domain/Models.kt index aaf4fb5..3d6fd02 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/domain/Models.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/domain/Models.kt @@ -8,6 +8,13 @@ data class CalendarSource( val accountName: String, val accountType: String, val color: Int, + /** + * The system's per-calendar `Calendars.VISIBLE` flag — the single visibility + * model: it decides both what Calendula shows and whether the provider + * schedules this calendar's reminder alarms at all (#75). Settings → + * Calendars writes it; the drawer's filter sheet is a separate, purely + * in-app declutter that leaves reminders alone. + */ val isVisibleInSystem: Boolean, /** * Whether events in this calendar can be created/edited/deleted @@ -34,6 +41,14 @@ data class CalendarSource( * even after a backup restore clears the app's stored ids. */ val isManaged: Boolean = false, + /** + * Whether the provider keeps this calendar's events on the device + * (`Calendars.SYNC_EVENTS`). Independent of [isVisibleInSystem]: a calendar + * with `syncsEvents = false` holds no local events at all, so making it + * visible cannot produce a single one — which is why the one-shot visibility + * migration leaves those calendars switched off. + */ + val syncsEvents: Boolean = true, ) data class EventInstance( diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarsScreen.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarsScreen.kt index 1c99d16..ade9210 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarsScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarsScreen.kt @@ -139,7 +139,6 @@ fun CalendarsScreen( viewModel: CalendarsViewModel = hiltViewModel(), ) { val calendars by viewModel.calendars.collectAsStateWithLifecycle() - val disabledIds by viewModel.disabledCalendarIds.collectAsStateWithLifecycle() val error by viewModel.error.collectAsStateWithLifecycle() val backupResult by viewModel.backupResult.collectAsStateWithLifecycle() val autoBackup by viewModel.autoBackup.collectAsStateWithLifecycle() @@ -177,7 +176,6 @@ fun CalendarsScreen( CalendarsList( local = calendars.filter { it.isLocal }, synced = calendars.filterNot { it.isLocal }, - disabledIds = disabledIds, error = error, onConsumeError = viewModel::consumeError, backupResult = backupResult, @@ -191,8 +189,8 @@ fun CalendarsScreen( onBack = onBack, onAdd = { editorSession++; editorId = NEW_CALENDAR_ID }, onEdit = { calendar -> editorSession++; editorId = calendar.id }, - onSetDisabled = viewModel::setDisabled, - onSetAccountDisabled = viewModel::setAccountDisabled, + onSetVisible = viewModel::setCalendarVisible, + onSetAccountVisible = viewModel::setAccountVisible, ) } } @@ -201,7 +199,6 @@ fun CalendarsScreen( private fun CalendarsList( local: List, synced: List, - disabledIds: Set, error: Boolean, onConsumeError: () -> Unit, backupResult: BackupResult?, @@ -215,8 +212,8 @@ private fun CalendarsList( onBack: () -> Unit, onAdd: () -> Unit, onEdit: (CalendarSource) -> Unit, - onSetDisabled: (Long, Boolean) -> Unit, - onSetAccountDisabled: (Collection, Boolean) -> Unit, + onSetVisible: (Long, Boolean) -> Unit, + onSetAccountVisible: (Collection, Boolean) -> Unit, ) { val context = LocalContext.current val snackbarHostState = remember { SnackbarHostState() } @@ -286,7 +283,7 @@ private fun CalendarsList( // Local (device-only) calendars — one collapsible group. The header's // "+" adds a calendar; the switch enables/disables them all at once; // tapping a calendar row opens its editor. - val localDisabled = local.isNotEmpty() && local.all { it.id in disabledIds } + val localDisabled = local.isNotEmpty() && local.none { it.isVisibleInSystem } CalendarGroup( title = stringResource(R.string.calendars_local_header), expanded = localExpanded, @@ -298,14 +295,14 @@ private fun CalendarsList( onManage = onAdd, onToggleExpand = { localExpanded = !localExpanded }, showToggleAll = local.isNotEmpty(), - allEnabled = local.none { it.id in disabledIds }, - onToggleAll = { enabled -> onSetAccountDisabled(local.map { it.id }, !enabled) }, + allEnabled = local.all { it.isVisibleInSystem }, + onToggleAll = { enabled -> onSetAccountVisible(local.map { it.id }, enabled) }, ) { if (local.isEmpty()) { HintText(stringResource(R.string.calendars_local_empty)) } else { local.forEachIndexed { index, calendar -> - val disabled = calendar.id in disabledIds + val disabled = !calendar.isVisibleInSystem GroupedRow( title = calendar.displayName, summary = calendar.description, @@ -317,7 +314,7 @@ private fun CalendarsList( EnableSwitch( calendarName = calendar.displayName, enabled = !disabled, - onToggle = { enabled -> onSetDisabled(calendar.id, !enabled) }, + onToggle = { enabled -> onSetVisible(calendar.id, enabled) }, ) }, onClick = { onEdit(calendar) }, @@ -415,7 +412,7 @@ private fun CalendarsList( .forEach { (account, cals) -> val expanded = account !in collapsedAccounts val accountType = cals.first().accountType - val accountDisabled = cals.all { it.id in disabledIds } + val accountDisabled = cals.none { it.isVisibleInSystem } Spacer(Modifier.height(16.dp)) CalendarGroup( title = account, @@ -436,11 +433,11 @@ private fun CalendarsList( } }, showToggleAll = true, - allEnabled = cals.none { it.id in disabledIds }, - onToggleAll = { enabled -> onSetAccountDisabled(cals.map { it.id }, !enabled) }, + allEnabled = cals.all { it.isVisibleInSystem }, + onToggleAll = { enabled -> onSetAccountVisible(cals.map { it.id }, enabled) }, ) { cals.forEachIndexed { index, calendar -> - val disabled = calendar.id in disabledIds + val disabled = !calendar.isVisibleInSystem GroupedRow( title = calendar.displayName, position = if (index == cals.lastIndex) Position.Bottom else Position.Middle, @@ -451,7 +448,7 @@ private fun CalendarsList( EnableSwitch( calendarName = calendar.displayName, enabled = !disabled, - onToggle = { enabled -> onSetDisabled(calendar.id, !enabled) }, + onToggle = { enabled -> onSetVisible(calendar.id, enabled) }, ) }, ) @@ -698,10 +695,12 @@ private fun CalendarEditor( } /** - * The per-row enable/disable control. Checked = the calendar is shown in the - * app; unchecking disables it (events, filters and pickers all drop it) without - * touching any provider data. Carries its own content description so the toggle - * is self-describing to screen readers even on a dimmed row. + * The per-row on/off control, writing the system's `Calendars.VISIBLE`: checked + * = the calendar is shown, unchecked = it drops out of every surface (events, + * filters, pickers) and the provider stops scheduling its reminders. The flag is + * device-local — nothing is deleted and nothing is synced anywhere. Carries its + * own content description so the toggle is self-describing to screen readers + * even on a dimmed row. */ @Composable private fun EnableSwitch( diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarsViewModel.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarsViewModel.kt index 8439616..78423c0 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarsViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarsViewModel.kt @@ -12,10 +12,7 @@ import de.jeanlucmakiola.calendula.data.calendar.CalendarRepository import de.jeanlucmakiola.calendula.data.di.IoDispatcher import de.jeanlucmakiola.calendula.data.ics.IcsExporter import de.jeanlucmakiola.calendula.data.prefs.BackupStatus -import de.jeanlucmakiola.calendula.data.prefs.CalendarPrefs import de.jeanlucmakiola.calendula.data.prefs.SettingsPrefs -import de.jeanlucmakiola.calendula.data.reminders.ReminderNotifier -import de.jeanlucmakiola.calendula.data.reminders.SuppressedReminderStore import de.jeanlucmakiola.calendula.domain.CalendarSource import de.jeanlucmakiola.calendula.domain.ics.IcsWriter import kotlinx.coroutines.CoroutineDispatcher @@ -45,10 +42,7 @@ class CalendarsViewModel @Inject constructor( @ApplicationContext private val context: Context, private val repository: CalendarRepository, private val icsExporter: IcsExporter, - private val prefs: CalendarPrefs, private val settingsPrefs: SettingsPrefs, - private val suppressedStore: SuppressedReminderStore, - private val notifier: ReminderNotifier, @IoDispatcher private val io: CoroutineDispatcher, ) : ViewModel() { @@ -62,20 +56,6 @@ class CalendarsViewModel @Inject constructor( initialValue = emptyList(), ) - /** - * Calendars the user has disabled in the app. This screen is the only - * surface that lists them, so it both reads the set (to dim the rows) and - * toggles it. Every other surface simply excludes these ids. - */ - val disabledCalendarIds: StateFlow> = - prefs.disabledCalendarIds - .flowOn(io) - .stateIn( - scope = viewModelScope, - started = SharingStarted.WhileSubscribed(5_000L), - initialValue = emptySet(), - ) - /** Automatic-backup settings + last-run status, for the Backup section UI. */ val autoBackup: StateFlow = combine( settingsPrefs.autoBackupEnabled, @@ -140,54 +120,24 @@ class CalendarsViewModel @Inject constructor( } /** - * Enable or disable a calendar app-side. Disabling removes it from every - * surface but Settings → Calendars (and hides its events) without touching - * provider data — purely a reversible Calendula-local view choice. + * Switch a calendar on or off. This is the app's one visibility model: it + * writes the system's `Calendars.VISIBLE`, so the calendar disappears from + * every surface *and* the provider stops (or resumes) scheduling its + * reminders. Nothing is patched by hand — the provider notifies and the + * observer re-queries. */ - fun setDisabled(id: Long, disabled: Boolean) { - viewModelScope.launch { - val current = prefs.disabledCalendarIds.first() - val next = if (disabled) current + id else current - id - if (next != current) { - prefs.setDisabledCalendarIds(next) - if (!disabled) recoverReminders(setOf(id)) - } - } + fun setCalendarVisible(id: Long, visible: Boolean) = write { + repository.setCalendarsVisible(listOf(id), visible) } /** - * Enable or disable every calendar of one account in a single write — the - * "toggle all" affordance on an account header. Done as one set update so the - * per-calendar [setDisabled] calls can't race each other. + * Switch every calendar of one account on or off — the "toggle all" + * affordance on an account header. Each row is written on its own (the + * provider only re-arms reminder alarms for a single-id update), in one + * coroutine so the writes can't race each other. */ - fun setAccountDisabled(ids: Collection, disabled: Boolean) { - viewModelScope.launch { - val current = prefs.disabledCalendarIds.first() - val next = if (disabled) current + ids else current - ids.toSet() - if (next != current) { - prefs.setDisabledCalendarIds(next) - if (!disabled) recoverReminders(current intersect ids.toSet()) - } - } - } - - /** - * Re-post the reminders that fired while [reEnabledIds] were disabled and are - * still relevant (event not yet over), then drop them from the stash. Runs - * after the disabled set is written, so the notifier's own disabled gate lets - * them through. Best-effort at re-enable time: it mirrors the receiver gates - * (reminders on + postable), and there is no later re-scan, so alerts left - * unposted because those gates are closed are simply released. - */ - private suspend fun recoverReminders(reEnabledIds: Set) { - if (reEnabledIds.isEmpty()) return - val recovered = suppressedStore.recoverFor(reEnabledIds, System.currentTimeMillis()) - if (recovered.isNotEmpty() && - settingsPrefs.remindersEnabled.first() && - notifier.canPost() - ) { - recovered.forEach { notifier.post(it) } - } + fun setAccountVisible(ids: Collection, visible: Boolean) = write { + repository.setCalendarsVisible(ids, visible) } // --- Automatic backup (issue #8) ------------------------------------ diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditViewModel.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditViewModel.kt index 45ae36f..457b998 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditViewModel.kt @@ -177,18 +177,16 @@ class EventEditViewModel @Inject constructor( repository.calendars().catch { emit(emptyList()) } /** - * Writable calendars — the only valid event targets. Disabled calendars are - * excluded, so you can't create into a calendar you've removed from the app; - * a last-used preselect landing on a now-disabled calendar falls back to the - * first remaining writable one (handled by [resolvedCalendarId] and [state]). - * Managed special-dates calendars are excluded too: their events are owned by - * the contact sync, which would delete any user event created there. + * Writable calendars — the only valid event targets. Calendars switched off + * in Settings → Calendars are excluded, so you can't create into one you've + * turned off; a last-used preselect landing on a now-off calendar falls back + * to the first remaining writable one (handled by [resolvedCalendarId] and + * [state]). Managed special-dates calendars are excluded too: their events + * are owned by the contact sync, which would delete any user event created + * there. */ - private val writableCalendars: Flow> = combine( - allCalendars, - prefs.disabledCalendarIds, - ) { calendars, disabled -> - calendars.filter { it.canModifyContents && it.id !in disabled && !it.isManaged } + private val writableCalendars: Flow> = allCalendars.map { calendars -> + calendars.filter { it.canModifyContents && it.isVisibleInSystem && !it.isManaged } } /** The target calendar id, resolved exactly as the form shows it. */ diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/filter/FilterViewModel.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/filter/FilterViewModel.kt index 28dd615..66062b0 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/filter/FilterViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/filter/FilterViewModel.kt @@ -30,12 +30,11 @@ class FilterViewModel @Inject constructor( combine( repository.calendars(), prefs.hiddenCalendarIds, - prefs.disabledCalendarIds, - ) { calendars, hidden, disabled -> - // Disabled calendars are gone from the app entirely — they don't - // belong in the drawer's hide/show list (you can't hide what's - // already disabled). They live only in Settings → Calendars. - val enabled = calendars.filterNot { it.id in disabled } + ) { calendars, hidden -> + // Calendars switched off in Settings → Calendars are off device-wide + // and don't belong in the drawer's hide/show list (you can't hide + // what is already off). They live only in Settings → Calendars. + val enabled = calendars.filter { it.isVisibleInSystem } if (enabled.isEmpty()) { FilterUiState.Failure(FailureReason.NoCalendarsConfigured) } else { diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/imports/ImportViewModel.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/imports/ImportViewModel.kt index 8ba9084..d7c536a 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/imports/ImportViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/imports/ImportViewModel.kt @@ -7,7 +7,6 @@ import dagger.hilt.android.lifecycle.HiltViewModel import de.jeanlucmakiola.calendula.data.calendar.CalendarRepository import de.jeanlucmakiola.calendula.data.di.IoDispatcher import de.jeanlucmakiola.calendula.data.ics.IcsImporter -import de.jeanlucmakiola.calendula.data.prefs.CalendarPrefs import de.jeanlucmakiola.calendula.domain.CalendarSource import de.jeanlucmakiola.calendula.domain.EventForm import de.jeanlucmakiola.calendula.domain.ics.IcsImportSummary @@ -57,7 +56,6 @@ sealed interface ImportUiState { class ImportViewModel @Inject constructor( private val repository: CalendarRepository, private val importer: IcsImporter, - private val prefs: CalendarPrefs, @IoDispatcher private val io: CoroutineDispatcher, ) : ViewModel() { @@ -87,16 +85,18 @@ class ImportViewModel @Inject constructor( warnings = parsed.warnings, ) else -> { - // A disabled calendar is removed from the app, so it can't be - // an import target — exclude it alongside the read-only ones. - // Managed special-dates calendars are contact-derived and - // editor-locked, so they're not a valid destination either. - val disabled = prefs.disabledCalendarIds.first() + // A calendar switched off in Settings → Calendars is off + // everywhere, so it can't be an import target — exclude it + // alongside the read-only ones. Managed special-dates + // calendars are contact-derived and editor-locked, so + // they're not a valid destination either. ImportUiState.Many( events = parsed.events, warnings = parsed.warnings, calendars = repository.calendars().first() - .filter { it.canModifyContents && !it.isManaged && it.id !in disabled }, + .filter { + it.canModifyContents && !it.isManaged && it.isVisibleInSystem + }, ) } } 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 05df891..6754834 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,20 +1,30 @@ 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.CalendarVisibilityMigration 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() : ViewModel() { +class PermissionViewModel @Inject constructor( + private val visibilityMigration: CalendarVisibilityMigration, +) : ViewModel() { private val _state = MutableStateFlow(PermissionUiState.Rationale) val state: StateFlow = _state.asStateFlow() fun onGranted() { _state.value = PermissionUiState.Granted + // The one-shot visibility migration needs the calendar permissions, so a + // launch that started without them left it pending (#75) — this is the + // moment it can finally run. A no-op once it has, and on a fresh install + // (nothing was ever disabled) it just stamps its guard. + viewModelScope.launch { visibilityMigration.runIfNeeded() } } fun onDenied() { diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index ef9bfce..96b3f4a 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -473,8 +473,8 @@ Your calendars No local calendars yet. Create one to keep events on this device only. Add calendar - Turn a calendar off to remove it from the app — its events, filters and pickers. Nothing is deleted, and you can turn it back on here anytime. - Show \"%1$s\" in the app + Turn a calendar off to hide it on this device — its events disappear from the app and it stops reminding you. Nothing is deleted, no other device is affected, and you can turn it back on here anytime. + Show \"%1$s\" Synced calendars These come from accounts on your device. Create and edit them in their own app. Manage in app diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/CalendarMapperTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/CalendarMapperTest.kt index ea02483..1645692 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/CalendarMapperTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/CalendarMapperTest.kt @@ -15,6 +15,7 @@ class CalendarMapperTest { visible: Int = 1, accessLevel: Int = CalendarContract.Calendars.CAL_ACCESS_OWNER, description: String? = null, + syncEvents: Int? = 1, ): MapColumnReader = MapColumnReader( CalendarProjection.IDX_ID to id, CalendarProjection.IDX_DISPLAY_NAME to displayName, @@ -24,6 +25,7 @@ class CalendarMapperTest { CalendarProjection.IDX_VISIBLE to visible, CalendarProjection.IDX_ACCESS_LEVEL to accessLevel, CalendarProjection.IDX_DESCRIPTION to description, + CalendarProjection.IDX_SYNC_EVENTS to syncEvents, ) @Test @@ -49,6 +51,18 @@ class CalendarMapperTest { ) } + @Test + fun `sync_events 0 marks the calendar as not syncing its events`() { + assertThat(reader(syncEvents = 0).toCalendarSource().syncsEvents).isFalse() + } + + @Test + fun `a NULL sync_events column is treated as syncing`() { + // The harmless default: it only ever holds the visibility migration back + // from switching a calendar on. + assertThat(reader(syncEvents = null).toCalendarSource().syncsEvents).isTrue() + } + @Test fun `null displayName falls back to placeholder`() { val src = reader(displayName = null).toCalendarSource() diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepositoryImplTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepositoryImplTest.kt index 14fa836..6ad5df5 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepositoryImplTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepositoryImplTest.kt @@ -41,8 +41,12 @@ class CalendarRepositoryImplTest { produceFile = { tempDir.resolve("repo_test_prefs.preferences_pb").toFile() }, ) - private fun makeCal(id: Long, name: String = "Cal $id"): CalendarSource = - CalendarSource(id, name, "x@y", "LOCAL", 0xFF112233.toInt(), true) + private fun makeCal( + id: Long, + name: String = "Cal $id", + visible: Boolean = true, + ): CalendarSource = + CalendarSource(id, name, "x@y", "LOCAL", 0xFF112233.toInt(), visible) private fun makeEvent( id: Long, @@ -171,37 +175,40 @@ class CalendarRepositoryImplTest { } @Test - fun `instances drops events whose calendar the user disabled`(@TempDir tempDir: Path) = runTest { - val prefs = newPrefs(tempDir) - prefs.setDisabledCalendarIds(setOf(2L)) + fun `instances drops events whose calendar is hidden at system level`( + @TempDir tempDir: Path, + ) = runTest { val fake = FakeCalendarDataSource().apply { + calendarsResult = listOf(makeCal(1L), makeCal(2L, visible = false)) instancesResult = { _, _ -> listOf( - makeEvent(10L, "Enabled", calendarId = 1L), - makeEvent(11L, "Disabled", calendarId = 2L), + makeEvent(10L, "Shown", calendarId = 1L), + makeEvent(11L, "Switched off", calendarId = 2L), ) } } - val repo = CalendarRepositoryImpl(fake, prefs, newSettings(tempDir), UnconfinedTestDispatcher(testScheduler)) + val repo = CalendarRepositoryImpl(fake, newPrefs(tempDir), newSettings(tempDir), UnconfinedTestDispatcher(testScheduler)) val range = Instant.fromEpochMilliseconds(0)..Instant.fromEpochMilliseconds(10_000L) repo.instances(range).test { - assertThat(awaitItem().map { it.title }).containsExactly("Enabled") + assertThat(awaitItem().map { it.title }).containsExactly("Shown") cancelAndIgnoreRemainingEvents() } } @Test - fun `instances applies the union of hidden and disabled sets`(@TempDir tempDir: Path) = runTest { + fun `instances applies the union of hidden and system-invisible calendars`( + @TempDir tempDir: Path, + ) = runTest { val prefs = newPrefs(tempDir) prefs.setHiddenCalendarIds(setOf(2L)) - prefs.setDisabledCalendarIds(setOf(3L)) val fake = FakeCalendarDataSource().apply { + calendarsResult = listOf(makeCal(1L), makeCal(2L), makeCal(3L, visible = false)) instancesResult = { _, _ -> listOf( makeEvent(10L, "Shown", calendarId = 1L), makeEvent(11L, "Hidden", calendarId = 2L), - makeEvent(12L, "Disabled", calendarId = 3L), + makeEvent(12L, "Switched off", calendarId = 3L), ) } } @@ -215,9 +222,11 @@ class CalendarRepositoryImplTest { } @Test - fun `instances re-emits when the disabled set changes`(@TempDir tempDir: Path) = runTest { - val prefs = newPrefs(tempDir) + fun `instances re-emit after a calendar is switched off in the provider`( + @TempDir tempDir: Path, + ) = runTest { val fake = FakeCalendarDataSource().apply { + calendarsResult = listOf(makeCal(1L), makeCal(2L)) instancesResult = { _, _ -> listOf( makeEvent(10L, "A", calendarId = 1L), @@ -225,19 +234,59 @@ class CalendarRepositoryImplTest { ) } } - val repo = CalendarRepositoryImpl(fake, prefs, newSettings(tempDir), UnconfinedTestDispatcher(testScheduler)) + val repo = CalendarRepositoryImpl(fake, newPrefs(tempDir), newSettings(tempDir), UnconfinedTestDispatcher(testScheduler)) val range = Instant.fromEpochMilliseconds(0)..Instant.fromEpochMilliseconds(10_000L) repo.instances(range).test { assertThat(awaitItem().map { it.title }).containsExactly("A", "B").inOrder() - prefs.setDisabledCalendarIds(setOf(2L)) + // The write itself is what the provider notifies about; the observer + // tick is what makes the views re-query. + repo.setCalendarsVisible(listOf(2L), false) + fake.tick() assertThat(awaitItem().map { it.title }).containsExactly("A") cancelAndIgnoreRemainingEvents() } } + @Test + fun `setCalendarsVisible addresses each calendar on its own`( + @TempDir tempDir: Path, + ) = runTest { + // An _id IN (…) batch would skip the provider's own reminder-alarm + // reschedule, so every calendar must be written by appended id. + val fake = FakeCalendarDataSource().apply { + calendarsResult = listOf(makeCal(1L), makeCal(2L), makeCal(3L)) + } + val repo = CalendarRepositoryImpl(fake, newPrefs(tempDir), newSettings(tempDir), Dispatchers.Unconfined) + + repo.setCalendarsVisible(listOf(1L, 3L), false) + + assertThat(fake.visibilityWrites).containsExactly(1L to false, 3L to false).inOrder() + } + + @Test + fun `searchEvents drops results from calendars that are off or hidden`( + @TempDir tempDir: Path, + ) = runTest { + val prefs = newPrefs(tempDir) + prefs.setHiddenCalendarIds(setOf(3L)) + val fake = FakeCalendarDataSource().apply { + calendarsResult = listOf(makeCal(1L), makeCal(2L, visible = false), makeCal(3L)) + searchResult = { + listOf( + makeEvent(10L, "Shown", calendarId = 1L), + makeEvent(11L, "Switched off", calendarId = 2L), + makeEvent(12L, "Hidden", calendarId = 3L), + ) + } + } + val repo = CalendarRepositoryImpl(fake, prefs, newSettings(tempDir), Dispatchers.Unconfined) + + assertThat(repo.searchEvents("e").map { it.title }).containsExactly("Shown") + } + @Test fun `createEvent delegates and returns the new id`(@TempDir tempDir: Path) = runTest { val fake = FakeCalendarDataSource().apply { nextInsertId = 77L } diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/FakeCalendarDataSource.kt b/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/FakeCalendarDataSource.kt index dce1a32..ca2aa79 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/FakeCalendarDataSource.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/FakeCalendarDataSource.kt @@ -95,6 +95,19 @@ internal class FakeCalendarDataSource : CalendarDataSource { updatedCalendars += UpdatedCalendar(id, displayName, color, description) } + /** (id, visible) pairs passed to [setCalendarVisible], in call order. */ + val visibilityWrites = mutableListOf>() + + override fun setCalendarVisible(id: Long, visible: Boolean) { + writeError?.let { throw it } + visibilityWrites += id to visible + // Reflect the write so a follow-up [calendars] read sees it, the way the + // provider would once its notification has re-triggered the query. + calendarsResult = calendarsResult.map { + if (it.id == id) it.copy(isVisibleInSystem = visible) else it + } + } + override fun deleteCalendar(id: Long) { writeError?.let { throw it } deletedCalendarIds += id diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/data/prefs/CalendarPrefsTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/data/prefs/CalendarPrefsTest.kt index a077bae..930af94 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/data/prefs/CalendarPrefsTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/data/prefs/CalendarPrefsTest.kt @@ -52,32 +52,51 @@ class CalendarPrefsTest { } @Test - fun `disabledCalendarIds defaults to empty when unset`(@TempDir tempDir: Path) = runTest { - val prefs = CalendarPrefs(newDataStore(tempDir)) - assertThat(prefs.disabledCalendarIds.first()).isEmpty() + fun `legacy disabled set reads back what an older version stored`( + @TempDir tempDir: Path, + ) = runTest { + val store = newDataStore(tempDir) + val prefs = CalendarPrefs(store) + store.updateData { p -> + p.toMutablePreferences().apply { this[CalendarPrefs.DISABLED_IDS_KEY] = "2,9" } + } + assertThat(prefs.legacyDisabledCalendarIds()).isEqualTo(setOf(2L, 9L)) } @Test - fun `setDisabledCalendarIds round-trips through DataStore`(@TempDir tempDir: Path) = runTest { + fun `legacy disabled set is empty when nothing was ever stored`( + @TempDir tempDir: Path, + ) = runTest { val prefs = CalendarPrefs(newDataStore(tempDir)) - prefs.setDisabledCalendarIds(setOf(1L, 42L, 7L)) - assertThat(prefs.disabledCalendarIds.first()).isEqualTo(setOf(1L, 42L, 7L)) + assertThat(prefs.legacyDisabledCalendarIds()).isEmpty() } @Test - fun `setting empty disabled set clears storage`(@TempDir tempDir: Path) = runTest { - val prefs = CalendarPrefs(newDataStore(tempDir)) - prefs.setDisabledCalendarIds(setOf(1L)) - prefs.setDisabledCalendarIds(emptySet()) - assertThat(prefs.disabledCalendarIds.first()).isEmpty() - } - - @Test - fun `hidden and disabled sets are stored independently`(@TempDir tempDir: Path) = runTest { - val prefs = CalendarPrefs(newDataStore(tempDir)) + fun `clearing the legacy disabled set leaves the hidden set alone`( + @TempDir tempDir: Path, + ) = runTest { + val store = newDataStore(tempDir) + val prefs = CalendarPrefs(store) prefs.setHiddenCalendarIds(setOf(1L)) - prefs.setDisabledCalendarIds(setOf(2L)) + store.updateData { p -> + p.toMutablePreferences().apply { this[CalendarPrefs.DISABLED_IDS_KEY] = "2" } + } + + prefs.clearLegacyDisabledCalendarIds() + + assertThat(prefs.legacyDisabledCalendarIds()).isEmpty() assertThat(prefs.hiddenCalendarIds.first()).isEqualTo(setOf(1L)) - assertThat(prefs.disabledCalendarIds.first()).isEqualTo(setOf(2L)) + } + + @Test + fun `visibility migration guard defaults to not-done and latches`( + @TempDir tempDir: Path, + ) = runTest { + val prefs = CalendarPrefs(newDataStore(tempDir)) + assertThat(prefs.visibilityMigrationDone.first()).isFalse() + + prefs.setVisibilityMigrationDone() + + assertThat(prefs.visibilityMigrationDone.first()).isTrue() } } diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/data/reminders/PostableAlertsTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/data/reminders/PostableAlertsTest.kt deleted file mode 100644 index 5fedc7d..0000000 --- a/app/src/test/java/de/jeanlucmakiola/calendula/data/reminders/PostableAlertsTest.kt +++ /dev/null @@ -1,71 +0,0 @@ -package de.jeanlucmakiola.calendula.data.reminders - -import com.google.common.truth.Truth.assertThat -import org.junit.jupiter.api.Test - -class PostableAlertsTest { - - private fun alert(alertId: Long, calendarId: Long) = ReminderAlert( - alertId = alertId, - eventId = alertId * 10, - calendarId = calendarId, - beginMillis = 0L, - endMillis = 0L, - title = "Event $alertId", - location = null, - isAllDay = false, - ) - - @Test - fun `keeps alerts when no calendar is disabled`() { - val due = listOf(alert(1, calendarId = 100), alert(2, calendarId = 200)) - - val postable = postableAlerts(due, disabledCalendarIds = emptySet()) - - assertThat(postable).isEqualTo(due) - } - - @Test - fun `drops alerts for a disabled calendar`() { - val keep = alert(1, calendarId = 100) - val drop = alert(2, calendarId = 200) - - val postable = postableAlerts(listOf(keep, drop), disabledCalendarIds = setOf(200)) - - assertThat(postable).containsExactly(keep) - } - - @Test - fun `drops every alert when all their calendars are disabled`() { - val due = listOf(alert(1, calendarId = 100), alert(2, calendarId = 100)) - - val postable = postableAlerts(due, disabledCalendarIds = setOf(100)) - - assertThat(postable).isEmpty() - } - - @Test - fun `keeps multiple alerts from the same enabled calendar`() { - val due = listOf(alert(1, calendarId = 100), alert(2, calendarId = 100)) - - val postable = postableAlerts(due, disabledCalendarIds = setOf(999)) - - assertThat(postable).isEqualTo(due) - } - - @Test - fun `alert with unknown calendar id 0 is never treated as disabled`() { - // Pre-upgrade snooze PendingIntents carry no calendar id (defaults to 0L). - val preUpgrade = alert(1, calendarId = 0L) - - assertThat(preUpgrade.isForDisabledCalendar(disabledCalendarIds = setOf(0L))).isFalse() - assertThat(postableAlerts(listOf(preUpgrade), disabledCalendarIds = setOf(0L))) - .containsExactly(preUpgrade) - } - - @Test - fun `isForDisabledCalendar matches only the disabled ids`() { - assertThat(alert(1, calendarId = 200).isForDisabledCalendar(setOf(200))).isTrue() - assertThat(alert(1, calendarId = 100).isForDisabledCalendar(setOf(200))).isFalse() - } -} diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/data/reminders/SuppressedReminderStoreTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/data/reminders/SuppressedReminderStoreTest.kt deleted file mode 100644 index 74c5eda..0000000 --- a/app/src/test/java/de/jeanlucmakiola/calendula/data/reminders/SuppressedReminderStoreTest.kt +++ /dev/null @@ -1,129 +0,0 @@ -package de.jeanlucmakiola.calendula.data.reminders - -import androidx.datastore.core.DataStore -import androidx.datastore.preferences.core.PreferenceDataStoreFactory -import androidx.datastore.preferences.core.Preferences -import com.google.common.truth.Truth.assertThat -import kotlinx.coroutines.test.runTest -import org.junit.jupiter.api.Test -import org.junit.jupiter.api.io.TempDir -import java.nio.file.Path - -class SuppressedReminderStoreTest { - - private fun newDataStore(tempDir: Path): DataStore = - PreferenceDataStoreFactory.create( - produceFile = { tempDir.resolve("test_prefs.preferences_pb").toFile() }, - ) - - private fun alert( - alertId: Long, - calendarId: Long, - endMillis: Long = Long.MAX_VALUE, - title: String = "Event $alertId", - location: String? = null, - ) = ReminderAlert( - alertId = alertId, - eventId = alertId * 10, - calendarId = calendarId, - beginMillis = 0L, - endMillis = endMillis, - title = title, - location = location, - isAllDay = false, - ) - - @Test - fun `encode then decode round-trips every field including delimiters`() { - val original = alert( - alertId = 7, - calendarId = 42, - endMillis = 123_456_789L, - // Free-text with the field separator and other awkward characters. - title = "Lunch | with | Alice", - location = "Café, 3rd floor | room B", - ).copy(beginMillis = 100L, isAllDay = true) - - val decoded = decodeStashEntry(encodeStashEntry(original)) - - assertThat(decoded).isEqualTo(original) - } - - @Test - fun `decode returns null for a malformed entry`() { - assertThat(decodeStashEntry("not-a-valid-entry")).isNull() - } - - @Test - fun `null location round-trips`() { - val original = alert(1, calendarId = 1, location = null) - assertThat(decodeStashEntry(encodeStashEntry(original))).isEqualTo(original) - } - - @Test - fun `recoverFor returns and removes only the re-enabled calendars`() = runTest { - val store = SuppressedReminderStore(newDataStore(tempDir)) - val keep = alert(1, calendarId = 100) - val recoverA = alert(2, calendarId = 200) - val recoverB = alert(3, calendarId = 200) - store.stash(listOf(keep, recoverA, recoverB), nowMillis = 0L) - - val recovered = store.recoverFor(setOf(200L), nowMillis = 0L) - - assertThat(recovered).containsExactly(recoverA, recoverB) - // The still-disabled calendar's alert stays stashed; the recovered ones are gone. - assertThat(store.recoverFor(setOf(100L, 200L), nowMillis = 0L)).containsExactly(keep) - } - - @Test - fun `stash drops alerts whose event already ended`() = runTest { - val store = SuppressedReminderStore(newDataStore(tempDir)) - val past = alert(1, calendarId = 100, endMillis = 500L) - val future = alert(2, calendarId = 100, endMillis = 2_000L) - - store.stash(listOf(past, future), nowMillis = 1_000L) - - assertThat(store.recoverFor(setOf(100L), nowMillis = 1_000L)).containsExactly(future) - } - - @Test - fun `purgeExpired removes only entries past their event end`() = runTest { - val store = SuppressedReminderStore(newDataStore(tempDir)) - // Stash both before "now" so neither is dropped on write, then advance time. - store.stash( - listOf( - alert(1, calendarId = 100, endMillis = 500L), - alert(2, calendarId = 100, endMillis = 2_000L), - ), - nowMillis = 0L, - ) - - store.purgeExpired(nowMillis = 1_000L) - - assertThat(store.recoverFor(setOf(100L), nowMillis = 0L).map { it.alertId }) - .containsExactly(2L) - } - - @Test - fun `stash replaces an existing entry with the same alert id`() = runTest { - val store = SuppressedReminderStore(newDataStore(tempDir)) - store.stash(listOf(alert(1, calendarId = 100, title = "old")), nowMillis = 0L) - store.stash(listOf(alert(1, calendarId = 100, title = "new")), nowMillis = 0L) - - val recovered = store.recoverFor(setOf(100L), nowMillis = 0L) - - assertThat(recovered).hasSize(1) - assertThat(recovered.single().title).isEqualTo("new") - } - - @Test - fun `isRelevantAt is true up to the event end and false after`() { - val a = alert(1, calendarId = 1, endMillis = 1_000L) - assertThat(a.isRelevantAt(999L)).isTrue() - assertThat(a.isRelevantAt(1_000L)).isTrue() - assertThat(a.isRelevantAt(1_001L)).isFalse() - } - - @TempDir - lateinit var tempDir: Path -} diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/domain/CalendarVisibilityPlanTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/domain/CalendarVisibilityPlanTest.kt new file mode 100644 index 0000000..c6f9a54 --- /dev/null +++ b/app/src/test/java/de/jeanlucmakiola/calendula/domain/CalendarVisibilityPlanTest.kt @@ -0,0 +1,96 @@ +package de.jeanlucmakiola.calendula.domain + +import com.google.common.truth.Truth.assertThat +import org.junit.jupiter.api.Test + +/** + * The one-shot reconciliation of the retired app-local "disabled calendars" set + * into the system's `Calendars.VISIBLE` (#75), across every + * (sync_events, visible) combination. + */ +class CalendarVisibilityPlanTest { + + private fun cal( + id: Long, + visible: Boolean = true, + syncsEvents: Boolean = true, + ): CalendarSource = CalendarSource( + id = id, + displayName = "Cal $id", + accountName = "acc@local", + accountType = "LOCAL", + color = 0, + isVisibleInSystem = visible, + syncsEvents = syncsEvents, + ) + + @Test + fun `a calendar disabled in-app is switched off at system level`() { + val plan = calendarVisibilityPlan(listOf(cal(1L, visible = true)), setOf(1L)) + assertThat(plan.hide).containsExactly(1L) + assertThat(plan.show).isEmpty() + } + + @Test + fun `a calendar enabled in-app but hidden at system level is switched on`() { + val plan = calendarVisibilityPlan(listOf(cal(1L, visible = false)), emptySet()) + assertThat(plan.show).containsExactly(1L) + assertThat(plan.hide).isEmpty() + } + + @Test + fun `a not-synced calendar is never switched on`() { + // sync_events = 0 means the provider holds none of its events locally, + // so making it visible could only produce an enabled-yet-empty row. + val plan = calendarVisibilityPlan( + listOf(cal(1L, visible = false, syncsEvents = false)), + emptySet(), + ) + assertThat(plan.isEmpty).isTrue() + } + + @Test + fun `a not-synced calendar disabled in-app is still switched off`() { + val plan = calendarVisibilityPlan( + listOf(cal(1L, visible = true, syncsEvents = false)), + setOf(1L), + ) + assertThat(plan.hide).containsExactly(1L) + } + + @Test + fun `calendars that already agree are left untouched`() { + val plan = calendarVisibilityPlan( + listOf(cal(1L, visible = true), cal(2L, visible = false)), + disabledCalendarIds = setOf(2L), + ) + assertThat(plan.isEmpty).isTrue() + } + + @Test + fun `a disabled id for a calendar that no longer exists writes nothing`() { + val plan = calendarVisibilityPlan(listOf(cal(1L)), setOf(99L)) + assertThat(plan.isEmpty).isTrue() + } + + @Test + fun `a mixed device is reconciled in both directions at once`() { + val plan = calendarVisibilityPlan( + listOf( + cal(1L, visible = true), // enabled, already visible → untouched + cal(2L, visible = false), // enabled but hidden → show + cal(3L, visible = true), // disabled in-app → hide + cal(4L, visible = false, syncsEvents = false), // not synced → untouched + ), + disabledCalendarIds = setOf(3L), + ) + assertThat(plan.show).containsExactly(2L) + assertThat(plan.hide).containsExactly(3L) + } + + @Test + fun `nothing disabled and everything visible is a no-op`() { + val plan = calendarVisibilityPlan(listOf(cal(1L), cal(2L)), emptySet()) + assertThat(plan.isEmpty).isTrue() + } +} diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/ui/edit/EventEditViewModelTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/ui/edit/EventEditViewModelTest.kt index 51a9819..898399f 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/ui/edit/EventEditViewModelTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/ui/edit/EventEditViewModelTest.kt @@ -45,9 +45,9 @@ class EventEditViewModelTest { private val beginMillis = 1_781_164_800_000L private val endMillis = beginMillis + 3_600_000L - private fun cal(id: Long): CalendarSource = CalendarSource( + private fun cal(id: Long, visible: Boolean = true): CalendarSource = CalendarSource( id = id, displayName = "Cal $id", accountName = "acc@local", accountType = "LOCAL", - color = 0xFF112233.toInt(), isVisibleInSystem = true, canModifyContents = true, + color = 0xFF112233.toInt(), isVisibleInSystem = visible, canModifyContents = true, ) private fun detail(calendarId: Long, rrule: String? = null): EventDetail = EventDetail( @@ -90,6 +90,24 @@ class EventEditViewModelTest { /** Keep [EventEditViewModel.state] hot so it computes while the test drives it. */ private fun CoroutineScope.activate(vm: EventEditViewModel): Job = launch { vm.state.collect {} } + @Test + fun `a calendar switched off in settings is not offered as a target`( + @TempDir tempDir: Path, + ) = runTest(dispatcher) { + val fake = FakeCalendarDataSource().apply { + calendarsResult = listOf(cal(1L), cal(2L, visible = false)) + eventDetailResult = { detail(calendarId = 1L) } + } + 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) + job.cancel() + } + @Test fun `changing the calendar routes the save through a move, not an update`( @TempDir tempDir: Path, diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index a8c3600..302dac8 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -157,6 +157,15 @@ sequenceDiagram Posting happens before marking: a crash in between re-posts silently (same tag + `setOnlyAlertOnce`) rather than losing a reminder. Swiped notifications never return because `FIRED` rows are never re-queried. + +**One visibility model.** The provider only schedules alarms for calendars with +`Calendars.VISIBLE = 1`, so that flag *is* the app's on/off switch: Settings → +Calendars writes it (one calendar per update — `CalendarProvider2` skips its own +`checkNextAlarm()` reschedule for any selection that isn't `_id=`), and every +display predicate reads `CalendarSource.isVisibleInSystem`. There is deliberately +no second, app-local model and therefore nothing to suppress here. The drawer's +filter sheet (`CalendarPrefs.hiddenCalendarIds`) is a separate in-app declutter +that never touches reminders. See `docs/design/calendar-visibility-model.md`. Deliberately absent until real devices prove it necessary: own alarm scheduling, `BOOT_COMPLETED`, snooze/dismiss actions, battery-exemption prompts.