From 2a0f28d3b395031470cee92b0ec7163729b70d9e Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Tue, 26 Jul 2022 22:48:24 +0100 Subject: [PATCH] splitting dm and group notifications into separate channels - adding notification system group - only notifying the summary to avoid overlapping notifications --- .../st/notifications/NotificationChannels.kt | 32 ++++++++++++++++--- .../st/notifications/NotificationFactory.kt | 15 ++++++--- .../notifications/NotificationStateMapper.kt | 9 +++++- .../RenderNotificationsUseCase.kt | 3 -- .../notifications/NotificationFactoryTest.kt | 24 +++++++++----- .../kotlin/fixture/NotificationFixtures.kt | 3 +- 6 files changed, 63 insertions(+), 23 deletions(-) diff --git a/features/notifications/src/main/kotlin/app/dapk/st/notifications/NotificationChannels.kt b/features/notifications/src/main/kotlin/app/dapk/st/notifications/NotificationChannels.kt index 6533b10..0ca041f 100644 --- a/features/notifications/src/main/kotlin/app/dapk/st/notifications/NotificationChannels.kt +++ b/features/notifications/src/main/kotlin/app/dapk/st/notifications/NotificationChannels.kt @@ -1,10 +1,14 @@ package app.dapk.st.notifications import android.app.NotificationChannel +import android.app.NotificationChannelGroup import android.app.NotificationManager import android.os.Build -const val channelId = "message" +const val DIRECT_CHANNEL_ID = "direct_channel_id" +const val GROUP_CHANNEL_ID = "group_channel_id" + +private const val CHATS_NOTIFICATION_GROUP_ID = "chats_notification_group" class NotificationChannels( private val notificationManager: NotificationManager @@ -12,13 +16,31 @@ class NotificationChannels( fun initChannels() { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { - if (notificationManager.getNotificationChannel(channelId) == null) { + notificationManager.createNotificationChannelGroup(NotificationChannelGroup(CHATS_NOTIFICATION_GROUP_ID, "Chats")) + + if (notificationManager.getNotificationChannel(DIRECT_CHANNEL_ID) == null) { notificationManager.createNotificationChannel( NotificationChannel( - channelId, - "messages", + DIRECT_CHANNEL_ID, + "Direct notifications", NotificationManager.IMPORTANCE_HIGH, - ) + ).also { + it.enableVibration(true) + it.enableLights(true) + it.group = CHATS_NOTIFICATION_GROUP_ID + } + ) + } + + if (notificationManager.getNotificationChannel(GROUP_CHANNEL_ID) == null) { + notificationManager.createNotificationChannel( + NotificationChannel( + GROUP_CHANNEL_ID, + "Group notifications", + NotificationManager.IMPORTANCE_HIGH, + ).also { + it.group = CHATS_NOTIFICATION_GROUP_ID + } ) } } diff --git a/features/notifications/src/main/kotlin/app/dapk/st/notifications/NotificationFactory.kt b/features/notifications/src/main/kotlin/app/dapk/st/notifications/NotificationFactory.kt index 7585d5b..87068cb 100644 --- a/features/notifications/src/main/kotlin/app/dapk/st/notifications/NotificationFactory.kt +++ b/features/notifications/src/main/kotlin/app/dapk/st/notifications/NotificationFactory.kt @@ -34,17 +34,21 @@ class NotificationFactory( else -> newRooms.contains(roomOverview.roomId) } + val last = sortedEvents.last() return NotificationTypes.Room( AndroidNotification( - channelId = channelId, - whenTimestamp = sortedEvents.last().utcTimestamp, + channelId = when { + roomOverview.isDm() -> DIRECT_CHANNEL_ID + else -> GROUP_CHANNEL_ID + }, + whenTimestamp = last.utcTimestamp, groupId = GROUP_ID, groupAlertBehavior = deviceMeta.whenPOrHigher( block = { Notification.GROUP_ALERT_SUMMARY }, fallback = { null } ), shortcutId = roomOverview.roomId.value, - alertMoreThanOnce = shouldAlertMoreThanOnce, + alertMoreThanOnce = false, contentIntent = openRoomIntent, messageStyle = messageStyle, category = Notification.CATEGORY_MESSAGE, @@ -53,7 +57,7 @@ class NotificationFactory( autoCancel = true ), roomId = roomOverview.roomId, - summary = sortedEvents.last().content, + summary = last.content, messageCount = sortedEvents.size, isAlerting = shouldAlertMoreThanOnce ) @@ -63,7 +67,7 @@ class NotificationFactory( val summaryInboxStyle = notificationStyleFactory.summary(notifications) val openAppIntent = intentFactory.notificationOpenApp(context) return AndroidNotification( - channelId = channelId, + channelId = notifications.firstOrNull { it.isAlerting }?.notification?.channelId ?: notifications.first().notification.channelId, messageStyle = summaryInboxStyle, alertMoreThanOnce = notifications.any { it.isAlerting }, smallIcon = R.drawable.ic_notification_small_icon, @@ -74,6 +78,7 @@ class NotificationFactory( fallback = { null } ), isGroupSummary = true, + category = Notification.CATEGORY_MESSAGE, ) } } diff --git a/features/notifications/src/main/kotlin/app/dapk/st/notifications/NotificationStateMapper.kt b/features/notifications/src/main/kotlin/app/dapk/st/notifications/NotificationStateMapper.kt index c9d4c17..4fe1496 100644 --- a/features/notifications/src/main/kotlin/app/dapk/st/notifications/NotificationStateMapper.kt +++ b/features/notifications/src/main/kotlin/app/dapk/st/notifications/NotificationStateMapper.kt @@ -16,7 +16,14 @@ class NotificationStateMapper( val messageEvents = roomEventsToNotifiableMapper.map(events) when (messageEvents.isEmpty()) { true -> NotificationTypes.DismissRoom(roomOverview.roomId) - false -> notificationFactory.createMessageNotification(messageEvents, roomOverview, state.roomsWithNewEvents, state.newRooms) + false -> { + notificationFactory.createMessageNotification( + events = messageEvents, + roomOverview = roomOverview, + roomsWithNewEvents = state.roomsWithNewEvents, + newRooms = state.newRooms + ) + } } } diff --git a/features/notifications/src/main/kotlin/app/dapk/st/notifications/RenderNotificationsUseCase.kt b/features/notifications/src/main/kotlin/app/dapk/st/notifications/RenderNotificationsUseCase.kt index 35a58c6..59128eb 100644 --- a/features/notifications/src/main/kotlin/app/dapk/st/notifications/RenderNotificationsUseCase.kt +++ b/features/notifications/src/main/kotlin/app/dapk/st/notifications/RenderNotificationsUseCase.kt @@ -1,7 +1,5 @@ package app.dapk.st.notifications -import app.dapk.st.core.AppLogTag.NOTIFICATION -import app.dapk.st.core.log import app.dapk.st.matrix.sync.RoomEvent import app.dapk.st.matrix.sync.RoomOverview import kotlinx.coroutines.flow.collect @@ -22,7 +20,6 @@ class RenderNotificationsUseCase( } private suspend fun renderUnreadChange(allUnread: Map>, diff: NotificationDiff) { - log(NOTIFICATION, "unread changed - render notifications") notificationRenderer.render( NotificationState( allUnread = allUnread, diff --git a/features/notifications/src/test/kotlin/app/dapk/st/notifications/NotificationFactoryTest.kt b/features/notifications/src/test/kotlin/app/dapk/st/notifications/NotificationFactoryTest.kt index 350f60b..af98498 100644 --- a/features/notifications/src/test/kotlin/app/dapk/st/notifications/NotificationFactoryTest.kt +++ b/features/notifications/src/test/kotlin/app/dapk/st/notifications/NotificationFactoryTest.kt @@ -7,6 +7,7 @@ import app.dapk.st.core.DeviceMeta import app.dapk.st.matrix.common.AvatarUrl import app.dapk.st.matrix.sync.RoomOverview import fake.FakeContext +import fixture.NotificationDelegateFixtures.anAndroidNotification import fixture.NotificationDelegateFixtures.anInboxStyle import fixture.NotificationFixtures.aRoomNotification import fixture.aRoomId @@ -16,6 +17,7 @@ import kotlinx.coroutines.test.runTest import org.amshove.kluent.shouldBeEqualTo import org.junit.Test +private const val A_CHANNEL_ID = "a channel id" private val AN_OPEN_APP_INTENT = aPendingIntent() private val AN_OPEN_ROOM_INTENT = aPendingIntent() private val A_NOTIFICATION_STYLE = anInboxStyle() @@ -48,24 +50,24 @@ class NotificationFactoryTest { @Test fun `given alerting room notification, when creating summary, then is alerting`() { - val notifications = listOf(aRoomNotification(isAlerting = true)) + val notifications = listOf(aRoomNotification(notification = anAndroidNotification(channelId = A_CHANNEL_ID), isAlerting = true)) fakeIntentFactory.givenNotificationOpenApp(fakeContext.instance).returns(AN_OPEN_APP_INTENT) fakeNotificationStyleFactory.givenSummary(notifications).returns(anInboxStyle()) val result = notificationFactory.createSummary(notifications) - result shouldBeEqualTo expectedSummary(shouldAlertMoreThanOnce = true) + result shouldBeEqualTo expectedSummary(channelId = A_CHANNEL_ID, shouldAlertMoreThanOnce = true) } @Test fun `given non alerting room notification, when creating summary, then is alerting`() { - val notifications = listOf(aRoomNotification(isAlerting = false)) + val notifications = listOf(aRoomNotification(notification = anAndroidNotification(channelId = A_CHANNEL_ID), isAlerting = false)) fakeIntentFactory.givenNotificationOpenApp(fakeContext.instance).returns(AN_OPEN_APP_INTENT) fakeNotificationStyleFactory.givenSummary(notifications).returns(anInboxStyle()) val result = notificationFactory.createSummary(notifications) - result shouldBeEqualTo expectedSummary(shouldAlertMoreThanOnce = false) + result shouldBeEqualTo expectedSummary(channelId = A_CHANNEL_ID, shouldAlertMoreThanOnce = false) } @Test @@ -75,6 +77,7 @@ class NotificationFactoryTest { val result = notificationFactory.createMessageNotification(EVENTS, A_GROUP_ROOM_OVERVIEW, setOf(A_ROOM_ID), newRooms = setOf(A_ROOM_ID)) result shouldBeEqualTo expectedMessage( + channel = GROUP_CHANNEL_ID, shouldAlertMoreThanOnce = true, ) } @@ -86,6 +89,7 @@ class NotificationFactoryTest { val result = notificationFactory.createMessageNotification(EVENTS, A_GROUP_ROOM_OVERVIEW, setOf(A_ROOM_ID), newRooms = emptySet()) result shouldBeEqualTo expectedMessage( + channel = GROUP_CHANNEL_ID, shouldAlertMoreThanOnce = false, ) } @@ -97,6 +101,7 @@ class NotificationFactoryTest { val result = notificationFactory.createMessageNotification(EVENTS, A_DM_ROOM_OVERVIEW, setOf(A_ROOM_ID), newRooms = setOf(A_ROOM_ID)) result shouldBeEqualTo expectedMessage( + channel = DIRECT_CHANNEL_ID, shouldAlertMoreThanOnce = true, ) } @@ -108,6 +113,7 @@ class NotificationFactoryTest { val result = notificationFactory.createMessageNotification(EVENTS, A_DM_ROOM_OVERVIEW, setOf(A_ROOM_ID), newRooms = emptySet()) result shouldBeEqualTo expectedMessage( + channel = DIRECT_CHANNEL_ID, shouldAlertMoreThanOnce = true, ) } @@ -119,15 +125,16 @@ class NotificationFactoryTest { } private fun expectedMessage( + channel: String, shouldAlertMoreThanOnce: Boolean, ) = NotificationTypes.Room( AndroidNotification( - channelId = "message", + channelId = channel, whenTimestamp = LATEST_EVENT.utcTimestamp, groupId = "st", groupAlertBehavior = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) Notification.GROUP_ALERT_SUMMARY else null, shortcutId = A_ROOM_ID.value, - alertMoreThanOnce = shouldAlertMoreThanOnce, + alertMoreThanOnce = false, contentIntent = AN_OPEN_ROOM_INTENT, messageStyle = A_NOTIFICATION_STYLE, category = Notification.CATEGORY_MESSAGE, @@ -141,13 +148,14 @@ class NotificationFactoryTest { isAlerting = shouldAlertMoreThanOnce, ) - private fun expectedSummary(shouldAlertMoreThanOnce: Boolean) = AndroidNotification( - channelId = "message", + private fun expectedSummary(channelId: String, shouldAlertMoreThanOnce: Boolean) = AndroidNotification( + channelId = channelId, messageStyle = A_NOTIFICATION_STYLE, alertMoreThanOnce = shouldAlertMoreThanOnce, smallIcon = R.drawable.ic_notification_small_icon, contentIntent = AN_OPEN_APP_INTENT, groupId = "st", + category = Notification.CATEGORY_MESSAGE, isGroupSummary = true, autoCancel = true ) diff --git a/features/notifications/src/test/kotlin/fixture/NotificationFixtures.kt b/features/notifications/src/test/kotlin/fixture/NotificationFixtures.kt index d476f2f..caa8a7a 100644 --- a/features/notifications/src/test/kotlin/fixture/NotificationFixtures.kt +++ b/features/notifications/src/test/kotlin/fixture/NotificationFixtures.kt @@ -14,11 +14,12 @@ object NotificationFixtures { ) = Notifications(summaryNotification, delegates) fun aRoomNotification( + notification: AndroidNotification = anAndroidNotification(), summary: String = "a summary line", messageCount: Int = 1, isAlerting: Boolean = false, ) = NotificationTypes.Room( - anAndroidNotification(), + notification, aRoomId(), summary = summary, messageCount = messageCount,