diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderChannelPlan.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderChannelPlan.kt index 4eb99f89..cb59e17b 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderChannelPlan.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderChannelPlan.kt @@ -1,6 +1,7 @@ package de.jeanlucmakiola.calendula.data.reminders import de.jeanlucmakiola.calendula.domain.CalendarSource +import de.jeanlucmakiola.calendula.ui.common.groupByAccount /** The single channel every reminder posted on before they moved per calendar (#145). */ const val LEGACY_REMINDER_CHANNEL_ID = "reminders" @@ -19,13 +20,13 @@ fun isReminderChannelId(channelId: String): Boolean = channelId.startsWith(CHANN fun isReminderGroupId(groupId: String): Boolean = groupId.startsWith(GROUP_PREFIX) /** - * One account's channel group. The app's own device calendars share a single - * group; [ambiguous] marks a name another group shows under a different type, - * so the label can name the source app as well. + * One account's channel group, labelled as the calendar manager labels it. The + * app's own device calendars share a single group ([isLocal]); [ambiguous] + * marks a [label] another group shows too, so it should name the source app. */ data class ReminderChannelGroupSpec( val id: String, - val accountName: String, + val label: String, val accountType: String, val isLocal: Boolean, val ambiguous: Boolean, @@ -39,67 +40,69 @@ data class ReminderChannelSpec( val groupId: String, ) +/** What a channel already looks like in the system, as far as the plan compares it. */ +data class ExistingReminderChannel(val id: String, val name: String) + data class ReminderChannelPlan( val groups: List, - /** Every channel that should exist; creating an existing one only refreshes its name. */ val channels: List, + /** The [channels] that are missing or whose name changed. */ + val channelsToWrite: List, val staleChannelIds: List, val staleGroupIds: List, ) /** * What the reminder channels should look like for [calendars]: one channel per - * calendar inside a group per account, and the reminder channels and groups in - * [existingChannelIds] / [existingGroupIds] that no calendar claims any more. - * Ids outside the reminder namespace (the backup channel, the legacy channel) - * are never touched. + * calendar inside a group per account, which of them need writing, and the + * reminder channels and groups in [existing] / [existingGroupIds] no calendar + * claims any more. Ids outside the reminder namespace (the backup channel, the + * legacy channel) are never touched. */ fun planReminderChannels( calendars: List, - existingChannelIds: Collection, + existing: Collection, existingGroupIds: Collection, + unnamed: String, ): ReminderChannelPlan { - val groupsByKey = calendars.groupBy(::accountKey) - val nameCounts = groupsByKey.keys.filterNotNull().groupingBy { it.first }.eachCount() - val groups = groupsByKey.map { (key, members) -> - if (key == null) { - ReminderChannelGroupSpec( - id = LOCAL_GROUP_ID, - accountName = members.first().accountName, - accountType = members.first().accountType, - isLocal = true, - ambiguous = false, - ) - } else { - val (name, type) = key - ReminderChannelGroupSpec( - id = "$GROUP_PREFIX$type/$name", - accountName = name, - accountType = type, - isLocal = false, - ambiguous = nameCounts.getValue(name) > 1, + val (local, synced) = calendars.partition { it.isLocal } + val groups = buildList { + if (local.isNotEmpty()) { + add(ReminderChannelGroupSpec(LOCAL_GROUP_ID, "", local.first().accountType, isLocal = true, ambiguous = false)) + } + synced.groupByAccount().forEach { group -> + add( + ReminderChannelGroupSpec( + id = accountGroupId(group.key.type, group.key.name), + label = group.label, + accountType = group.accountType, + isLocal = false, + ambiguous = group.ambiguous, + ), ) } } - val groupIdByKey = groupsByKey.keys.zip(groups.map { it.id }).toMap() val channels = calendars.map { calendar -> ReminderChannelSpec( id = reminderChannelId(calendar.id), calendarId = calendar.id, - name = calendar.displayName, - groupId = groupIdByKey.getValue(accountKey(calendar)), + name = calendar.displayName.ifBlank { unnamed }, + groupId = if (calendar.isLocal) LOCAL_GROUP_ID else accountGroupId(calendar.accountType, calendar.accountName), ) } + val existingById = existing.associateBy { it.id } val wantedChannels = channels.mapTo(HashSet()) { it.id } val wantedGroups = groups.mapTo(HashSet()) { it.id } return ReminderChannelPlan( groups = groups, channels = channels, - staleChannelIds = existingChannelIds.filter { isReminderChannelId(it) && it !in wantedChannels }, + channelsToWrite = channels.filter { spec -> + val current = existingById[spec.id] + current == null || current.name != spec.name + }, + staleChannelIds = existingById.keys.filter { isReminderChannelId(it) && it !in wantedChannels }, staleGroupIds = existingGroupIds.filter { isReminderGroupId(it) && it !in wantedGroups }, ) } -/** The app's own device calendars share one group (null); every other account has its own. */ -private fun accountKey(calendar: CalendarSource): Pair? = - if (calendar.isLocal) null else calendar.accountName to calendar.accountType +private fun accountGroupId(type: String, name: String): String = "$GROUP_PREFIX$type/$name" diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderChannels.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderChannels.kt index a4422687..a7f8f004 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderChannels.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderChannels.kt @@ -8,44 +8,39 @@ import android.util.Log import dagger.hilt.android.qualifiers.ApplicationContext import de.jeanlucmakiola.calendula.R import de.jeanlucmakiola.calendula.data.calendar.CalendarDataSource -import de.jeanlucmakiola.calendula.data.prefs.CalendarPrefs -import de.jeanlucmakiola.calendula.ui.common.sourceAppPackages -import kotlinx.coroutines.flow.first +import de.jeanlucmakiola.calendula.ui.common.sourceAppLabel import javax.inject.Inject import javax.inject.Singleton /** - * The reminder notification channels: one per switched-on calendar, grouped - * by account (#145), so each calendar can be muted or given its own sound in - * the system settings before it ever fires. + * The reminder notification channels: one per calendar, grouped by account + * (#145), created up front so each calendar can be muted or given its own sound + * in the system settings before it ever fires. * * A channel the user has touched keeps their settings; [sync] only creates the - * missing ones, refreshes names, and deletes channels whose calendar is gone or - * switched off — the system restores a deleted channel's settings if its id - * comes back. + * missing ones, renames changed ones, and deletes those whose calendar is gone. */ @Singleton class ReminderChannels @Inject constructor( @ApplicationContext private val context: Context, private val calendarDataSource: CalendarDataSource, - private val calendarPrefs: CalendarPrefs, ) { private val manager: NotificationManager get() = context.getSystemService(NotificationManager::class.java) - /** Bring the channels in line with the provider's switched-on calendars. */ - suspend fun sync() { - val pending = calendarPrefs.pendingDisabledCalendarIds.first() - val calendars = calendarDataSource.calendars().filter { it.isVisibleInSystem && it.id !in pending } + /** Bring the channels in line with the provider's calendars. */ + fun sync() { val manager = manager - val plan = planReminderChannels( - calendars = calendars, - existingChannelIds = manager.notificationChannels.map { it.id }, - existingGroupIds = manager.notificationChannelGroups.map { it.id }, - ) + val existing = manager.notificationChannels + val plan = plan(existing) plan.staleChannelIds.forEach(manager::deleteNotificationChannel) plan.staleGroupIds.forEach(manager::deleteNotificationChannelGroup) - create(plan) + val groupNames = manager.notificationChannelGroups.associate { it.id to it.name?.toString() } + write( + groups = plan.groups.filter { groupNames[it.id] != groupLabel(it) }, + channels = plan.channelsToWrite, + existing = existing, + ) if (plan.channels.isNotEmpty()) retireLegacyChannel() } @@ -55,32 +50,52 @@ class ReminderChannels @Inject constructor( */ fun channelFor(calendarId: Long): String? { val id = reminderChannelId(calendarId) + val manager = manager if (manager.getNotificationChannel(id) != null) return id - val calendar = calendarDataSource.calendars().firstOrNull { it.id == calendarId } ?: return null - create(planReminderChannels(listOf(calendar), emptyList(), emptyList())) + val existing = manager.notificationChannels + val plan = plan(existing) + val spec = plan.channels.firstOrNull { it.calendarId == calendarId } ?: return null + val knownGroups = manager.notificationChannelGroups.mapTo(HashSet()) { it.id } + write( + groups = plan.groups.filter { it.id == spec.groupId && it.id !in knownGroups }, + channels = listOf(spec), + existing = existing, + ) return id } - private fun create(plan: ReminderChannelPlan) { + private fun plan(existing: List): ReminderChannelPlan = planReminderChannels( + calendars = calendarDataSource.calendars(), + existing = existing.map { ExistingReminderChannel(it.id, it.name.toString()) }, + existingGroupIds = manager.notificationChannelGroups.map { it.id }, + unnamed = context.getString(R.string.calendar_unnamed), + ) + + private fun write( + groups: List, + channels: List, + existing: List, + ) { val manager = manager - manager.createNotificationChannelGroups( - plan.groups.map { NotificationChannelGroup(it.id, groupLabel(it)) }, - ) + if (groups.isNotEmpty()) { + manager.createNotificationChannelGroups(groups.map { NotificationChannelGroup(it.id, groupLabel(it)) }) + } + if (channels.isEmpty()) return + val byId = existing.associateBy { it.id } + val template = byId[LEGACY_REMINDER_CHANNEL_ID] // Only a channel's first creation sets its behaviour; later calls just // rename it, so an existing one passes its own importance back. - val template = manager.getNotificationChannel(LEGACY_REMINDER_CHANNEL_ID) - val unnamed = context.getString(R.string.calendar_unnamed) manager.createNotificationChannels( - plan.channels.map { spec -> - val existing = manager.getNotificationChannel(spec.id) + channels.map { spec -> + val current = byId[spec.id] NotificationChannel( spec.id, - spec.name.ifBlank { unnamed }, - existing?.importance ?: template?.importance ?: NotificationManager.IMPORTANCE_HIGH, + spec.name, + current?.importance ?: template?.importance ?: NotificationManager.IMPORTANCE_HIGH, ).apply { description = context.getString(R.string.reminder_channel_description) group = spec.groupId - if (existing == null && template != null) inheritFrom(template) + if (current == null && template != null) inheritFrom(template) } }, ) @@ -92,8 +107,9 @@ class ReminderChannels @Inject constructor( */ private fun NotificationChannel.inheritFrom(template: NotificationChannel) { setSound(template.sound, template.audioAttributes) - enableVibration(template.shouldVibrate()) + // The pattern setter also flips vibration on or off, so it goes first. vibrationPattern = template.vibrationPattern + enableVibration(template.shouldVibrate()) enableLights(template.shouldShowLights()) lightColor = template.lightColor lockscreenVisibility = template.lockscreenVisibility @@ -119,17 +135,9 @@ class ReminderChannels @Inject constructor( private fun groupLabel(group: ReminderChannelGroupSpec): String { if (group.isLocal) return context.getString(R.string.calendars_local_header) - val name = group.accountName.ifBlank { group.accountType } - .ifBlank { context.getString(R.string.calendar_unnamed) } + val name = group.label.ifBlank { context.getString(R.string.calendar_unnamed) } if (!group.ambiguous) return name - return context.getString(R.string.calendars_account_from_source, name, sourceAppLabel(group.accountType)) - } - - private fun sourceAppLabel(accountType: String): String { - val pm = context.packageManager - return sourceAppPackages(context, accountType).firstNotNullOfOrNull { pkg -> - runCatching { pm.getApplicationLabel(pm.getApplicationInfo(pkg, 0)).toString() }.getOrNull() - } ?: accountType + return context.getString(R.string.calendars_account_from_source, name, sourceAppLabel(context, group.accountType)) } private companion object { diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderScanner.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderScanner.kt index 32716da1..92845523 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderScanner.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderScanner.kt @@ -14,7 +14,6 @@ import de.jeanlucmakiola.calendula.domain.reminders.reminderWatermark import de.jeanlucmakiola.calendula.domain.reminders.scheduleReminders import de.jeanlucmakiola.floret.android.hasPermission import de.jeanlucmakiola.floret.di.IoDispatcher -import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.CoroutineStart import kotlinx.coroutines.SupervisorJob @@ -78,7 +77,6 @@ class ReminderScanner @Inject constructor( private suspend fun runScan() { val now = System.currentTimeMillis() if (!hasReadCalendar()) return - syncChannels() if (!settingsPrefs.remindersEnabled.first()) { // Reminders off: drop the wake-up, but keep the watermark moving @@ -88,6 +86,7 @@ class ReminderScanner @Inject constructor( return } + syncChannels() val lookahead = reminderQueryHorizon(LOOKAHEAD_MILLIS, source.longestReminderMinutes()) // Reach into the past too: an all-day "at time of event" encodes to a // negative offset, and a catch-up pass needs the occurrences it missed. @@ -121,14 +120,13 @@ class ReminderScanner @Inject constructor( } /** - * Every trigger that can add, rename or drop a calendar reaches a scan, so - * the channels are kept in step here. A failure never costs the reminders. + * A provider change re-scans, so a calendar added, renamed or removed keeps + * its channel in step here. Skipped while reminders are off, when the + * channels have nothing to do. A failure never costs the reminders. */ - private suspend fun syncChannels() { + private fun syncChannels() { try { channels.sync() - } catch (e: CancellationException) { - throw e } catch (e: SecurityException) { throw e } catch (e: Exception) { diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/AccountGroups.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/AccountGroups.kt index c92c1ce3..d2a56674 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/AccountGroups.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/AccountGroups.kt @@ -1,5 +1,6 @@ package de.jeanlucmakiola.calendula.ui.common +import android.content.Context import androidx.compose.runtime.Composable import androidx.compose.runtime.remember import androidx.compose.ui.platform.LocalContext @@ -72,11 +73,13 @@ fun accountGroupTitle(group: CalendarAccountGroup): String { @Composable fun sourceAppName(accountType: String): String { val context = LocalContext.current - return remember(accountType) { - val pm = context.packageManager - val packages = sourceAppPackages(context, accountType) - packages.firstNotNullOfOrNull { pkg -> - runCatching { pm.getApplicationLabel(pm.getApplicationInfo(pkg, 0)).toString() }.getOrNull() - } ?: accountType - } + return remember(accountType) { sourceAppLabel(context, accountType) } +} + +/** [sourceAppName] outside composition, e.g. for notification channel groups. */ +fun sourceAppLabel(context: Context, accountType: String): String { + val pm = context.packageManager + return sourceAppPackages(context, accountType).firstNotNullOfOrNull { pkg -> + runCatching { pm.getApplicationLabel(pm.getApplicationInfo(pkg, 0)).toString() }.getOrNull() + } ?: accountType } diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/data/reminders/ReminderChannelPlanTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/data/reminders/ReminderChannelPlanTest.kt index 83ea80e9..4227de86 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/data/reminders/ReminderChannelPlanTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/data/reminders/ReminderChannelPlanTest.kt @@ -24,8 +24,9 @@ class ReminderChannelPlanTest { calendar(2, "me@example.org", "com.google"), calendar(3, "dav", "bitfire.at.davdroid"), ), - existingChannelIds = emptyList(), + existing = emptyList(), existingGroupIds = emptyList(), + unnamed = "Unnamed", ) assertThat(plan.groups).hasSize(2) assertThat(plan.channels.map { it.id }) @@ -42,8 +43,9 @@ class ReminderChannelPlanTest { calendar(1, "Calendula", "LOCAL", local = true), calendar(2, "Other", "LOCAL", local = true), ), - existingChannelIds = emptyList(), + existing = emptyList(), existingGroupIds = emptyList(), + unnamed = "Unnamed", ) assertThat(plan.groups).hasSize(1) assertThat(plan.groups.single().isLocal).isTrue() @@ -57,27 +59,47 @@ class ReminderChannelPlanTest { calendar(2, "me@example.org", "bitfire.at.davdroid"), calendar(3, "work", "bitfire.at.davdroid"), ), - existingChannelIds = emptyList(), + existing = emptyList(), existingGroupIds = emptyList(), + unnamed = "Unnamed", ) assertThat(plan.groups.filter { it.ambiguous }.map { it.accountType }) .containsExactly("com.google", "bitfire.at.davdroid") - assertThat(plan.groups.single { it.accountName == "work" }.ambiguous).isFalse() + assertThat(plan.groups.single { it.label == "work" }.ambiguous).isFalse() } @Test fun `stale reminder channels and groups go, foreign ones stay`() { val current = calendar(1, "me@example.org", "com.google") - val keptGroup = planReminderChannels(listOf(current), emptyList(), emptyList()).groups.single().id - val gone = planReminderChannels(listOf(calendar(9, "old", "com.google")), emptyList(), emptyList()) - val plan = planReminderChannels( + fun plan(calendars: List, existing: List = emptyList(), groups: List = emptyList()) = + planReminderChannels(calendars, existing.map { ExistingReminderChannel(it, "Cal ${it.last()}") }, groups, "Unnamed") + val keptGroup = plan(listOf(current)).groups.single().id + val goneGroup = plan(listOf(calendar(9, "old", "com.google"))).groups.single().id + val result = plan( calendars = listOf(current), - existingChannelIds = listOf( - reminderChannelId(1), reminderChannelId(9), LEGACY_REMINDER_CHANNEL_ID, "backup", - ), - existingGroupIds = listOf(keptGroup, gone.groups.single().id, "other"), + existing = listOf(reminderChannelId(1), reminderChannelId(9), LEGACY_REMINDER_CHANNEL_ID, "backup"), + groups = listOf(keptGroup, goneGroup, "other"), ) - assertThat(plan.staleChannelIds).containsExactly(reminderChannelId(9)) - assertThat(plan.staleGroupIds).containsExactly(gone.groups.single().id) + assertThat(result.staleChannelIds).containsExactly(reminderChannelId(9)) + assertThat(result.staleGroupIds).containsExactly(goneGroup) + } + + @Test + fun `only missing or renamed channels are written`() { + val plan = planReminderChannels( + calendars = listOf( + calendar(1, "a", "com.google"), + calendar(2, "a", "com.google"), + calendar(3, "a", "com.google").copy(displayName = ""), + ), + existing = listOf( + ExistingReminderChannel(reminderChannelId(1), "Cal 1"), + ExistingReminderChannel(reminderChannelId(2), "Old name"), + ), + existingGroupIds = emptyList(), + unnamed = "Unnamed", + ) + assertThat(plan.channelsToWrite.map { it.calendarId }).containsExactly(2L, 3L) + assertThat(plan.channelsToWrite.last().name).isEqualTo("Unnamed") } } diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index a3963343..503c49f6 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -392,11 +392,11 @@ switched off. The drawer's filter sheet (`CalendarPrefs.hiddenCalendarIds`) is a separate in-app declutter that never touches reminders. **A channel per calendar (#145).** `ReminderChannels` keeps one notification -channel per switched-on calendar, grouped by account, so a calendar can be muted -or given its own sound in the system settings before it ever fires. Every scan -syncs them first: missing channels are created, names refreshed, and channels of -gone or switched-off calendars deleted (the system restores a deleted channel's -settings if the id returns). A new channel copies the sound, vibration and +channel per calendar, grouped by account, so a calendar can be muted or given +its own sound in the system settings before it ever fires. Every scan with +reminders on syncs them: missing channels are created, renamed ones updated, and +channels of deleted calendars removed. Switched-off calendars keep theirs, so +toggling one never cancels a showing reminder or loses its settings. A new channel copies the sound, vibration and importance of the pre-#145 single `reminders` channel, which is deleted once nothing showing still sits on it. Settings → Notifications only deep-links into the system's channel screens (#340); the app never sets a sound itself. Separate