From 6f18d4890503a5a278c01199c30de49986ff65c7 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 13 Jul 2022 18:24:17 +0100 Subject: [PATCH 1/6] adding missing messaging style config --- .../dapk/st/notifications/AndroidNotificationStyleBuilder.kt | 2 ++ .../kotlin/app/dapk/st/notifications/NotificationChannels.kt | 2 +- .../kotlin/app/dapk/st/notifications/NotificationFactory.kt | 1 - 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/features/notifications/src/main/kotlin/app/dapk/st/notifications/AndroidNotificationStyleBuilder.kt b/features/notifications/src/main/kotlin/app/dapk/st/notifications/AndroidNotificationStyleBuilder.kt index 5da92f3..ec91fba 100644 --- a/features/notifications/src/main/kotlin/app/dapk/st/notifications/AndroidNotificationStyleBuilder.kt +++ b/features/notifications/src/main/kotlin/app/dapk/st/notifications/AndroidNotificationStyleBuilder.kt @@ -34,6 +34,8 @@ class AndroidNotificationStyleBuilder( .setKey(person.key) .build() ).also { style -> + style.conversationTitle = title + style.isGroupConversation = isGroup content.forEach { val sender = personBuilderFactory() .setName(it.sender.name) 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 676d968..6533b10 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 @@ -4,7 +4,7 @@ import android.app.NotificationChannel import android.app.NotificationManager import android.os.Build -private const val channelId = "message" +const val channelId = "message" class NotificationChannels( private val notificationManager: NotificationManager 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 84d54d4..7585d5b 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 @@ -10,7 +10,6 @@ import app.dapk.st.matrix.sync.RoomOverview import app.dapk.st.navigator.IntentFactory private const val GROUP_ID = "st" -private const val channelId = "message" class NotificationFactory( private val context: Context, From 63618276b7a8e0d31b5f5237ad62acfac20d6b7b Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 13 Jul 2022 18:32:10 +0100 Subject: [PATCH 2/6] initialisating channels as part of the first notification emission rather than blocking application init --- .../dapk/st/notifications/RenderNotificationsUseCase.kt | 8 +++----- .../st/notifications/RenderNotificationsUseCaseTest.kt | 7 ++++++- 2 files changed, 9 insertions(+), 6 deletions(-) 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 efb0d21..35a58c6 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 @@ -6,19 +6,17 @@ import app.dapk.st.matrix.sync.RoomEvent import app.dapk.st.matrix.sync.RoomOverview import kotlinx.coroutines.flow.collect import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.flow.onStart class RenderNotificationsUseCase( private val notificationRenderer: NotificationRenderer, private val observeRenderableUnreadEventsUseCase: ObserveUnreadNotificationsUseCase, - notificationChannels: NotificationChannels, + private val notificationChannels: NotificationChannels, ) { - init { - notificationChannels.initChannels() - } - suspend fun listenForNotificationChanges() { observeRenderableUnreadEventsUseCase() + .onStart { notificationChannels.initChannels() } .onEach { (each, diff) -> renderUnreadChange(each, diff) } .collect() } diff --git a/features/notifications/src/test/kotlin/app/dapk/st/notifications/RenderNotificationsUseCaseTest.kt b/features/notifications/src/test/kotlin/app/dapk/st/notifications/RenderNotificationsUseCaseTest.kt index be968b5..6cc7836 100644 --- a/features/notifications/src/test/kotlin/app/dapk/st/notifications/RenderNotificationsUseCaseTest.kt +++ b/features/notifications/src/test/kotlin/app/dapk/st/notifications/RenderNotificationsUseCaseTest.kt @@ -25,7 +25,12 @@ class RenderNotificationsUseCaseTest { ) @Test - fun `when creating use case instance, then initiates channels`() { + fun `given events, when listening for changes then initiates channels once`() = runTest { + fakeNotificationRenderer.instance.expect { it.render(any()) } + fakeObserveUnreadNotificationsUseCase.given().emits(AN_UNREAD_NOTIFICATIONS) + + renderNotificationsUseCase.listenForNotificationChanges() + fakeNotificationChannels.verifyInitiated() } From b0a438ee98d7b31d84141db025b0cf5a05ca0671 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 13 Jul 2022 23:03:58 +0100 Subject: [PATCH 3/6] diffing historical event timestamps to avoid acting on them as new events - fixes an issue where the database window would pull in legacy events after marking new events as read --- .../ObserveUnreadNotificationsUseCaseImpl.kt | 39 +++++++++++++++---- ...rveUnreadRenderNotificationsUseCaseTest.kt | 33 +++++++++++++--- 2 files changed, 59 insertions(+), 13 deletions(-) diff --git a/features/notifications/src/main/kotlin/app/dapk/st/notifications/ObserveUnreadNotificationsUseCaseImpl.kt b/features/notifications/src/main/kotlin/app/dapk/st/notifications/ObserveUnreadNotificationsUseCaseImpl.kt index dd7f7dc..8a3860d 100644 --- a/features/notifications/src/main/kotlin/app/dapk/st/notifications/ObserveUnreadNotificationsUseCaseImpl.kt +++ b/features/notifications/src/main/kotlin/app/dapk/st/notifications/ObserveUnreadNotificationsUseCaseImpl.kt @@ -34,10 +34,12 @@ private fun Flow.onlyRenderableChanges(): Flow { log(AppLogTag.NOTIFICATION, "Ignoring unread change due to no currently showing messages and changes are all messages marked as read") false } + else -> true } } @@ -45,29 +47,48 @@ private fun Flow.onlyRenderableChanges(): Flow>>.mapWithDiff(): Flow>, NotificationDiff>> { - val previousUnreadEvents = mutableMapOf>() + val previousUnreadEvents = mutableMapOf>() return this.map { each -> - val allUnreadIds = each.toIds() + val allUnreadIds = each.toTimestampedIds() val notificationDiff = calculateDiff(allUnreadIds, previousUnreadEvents) previousUnreadEvents.clearAndPutAll(allUnreadIds) each to notificationDiff } } -private fun calculateDiff(allUnread: Map>, previousUnread: Map>?): NotificationDiff { +private fun calculateDiff(allUnread: Map>, previousUnread: Map>?): NotificationDiff { + val previousLatestEventTimestamps = previousUnread.toLatestTimestamps() val newRooms = allUnread.filter { !previousUnread.containsKey(it.key) }.keys - val unchanged = previousUnread?.filter { allUnread.containsKey(it.key) && it.value == allUnread[it.key] } ?: emptyMap() - val changedOrNew = allUnread.filterNot { unchanged.containsKey(it.key) } + + val unchanged = previousUnread?.filter { + allUnread.containsKey(it.key) && (it.value == allUnread[it.key]) + } ?: emptyMap() + val changedOrNew = allUnread.filterNot { unchanged.containsKey(it.key) }.mapValues { (key, value) -> + val isChangedRoom = !newRooms.contains(key) + if (isChangedRoom) { + val latest = previousLatestEventTimestamps[key] ?: 0L + value.filter { + val isExistingEvent = (previousUnread?.get(key)?.contains(it) ?: false) + !isExistingEvent && it.second > latest + } + } else { + value + } + }.filter { it.value.isNotEmpty() } val removed = previousUnread?.filter { !allUnread.containsKey(it.key) } ?: emptyMap() - return NotificationDiff(unchanged, changedOrNew, removed, newRooms) + return NotificationDiff(unchanged.toEventIds(), changedOrNew.toEventIds(), removed.toEventIds(), newRooms) } -private fun List.toEventIds() = this.map { it.eventId } +private fun Map>?.toLatestTimestamps() = this?.mapValues { it.value.maxOf { it.second } } ?: emptyMap() -private fun Map>.toIds() = this +private fun Map>.toEventIds() = this.mapValues { it.value.map { it.first } } + +private fun Map>.toTimestampedIds() = this .mapValues { it.value.toEventIds() } .mapKeys { it.key.roomId } +private fun List.toEventIds() = this.map { it.eventId to it.utcTimestamp } + private fun Flow.avoidShowingPreviousNotificationsOnLaunch() = drop(1) data class NotificationDiff( @@ -76,3 +97,5 @@ data class NotificationDiff( val removed: Map>, val newRooms: Set ) + +typealias TimestampedEventId = Pair \ No newline at end of file diff --git a/features/notifications/src/test/kotlin/app/dapk/st/notifications/ObserveUnreadRenderNotificationsUseCaseTest.kt b/features/notifications/src/test/kotlin/app/dapk/st/notifications/ObserveUnreadRenderNotificationsUseCaseTest.kt index fdce470..1a932fe 100644 --- a/features/notifications/src/test/kotlin/app/dapk/st/notifications/ObserveUnreadRenderNotificationsUseCaseTest.kt +++ b/features/notifications/src/test/kotlin/app/dapk/st/notifications/ObserveUnreadRenderNotificationsUseCaseTest.kt @@ -3,8 +3,11 @@ package app.dapk.st.notifications import app.dapk.st.matrix.sync.RoomEvent import app.dapk.st.matrix.sync.RoomOverview import fake.FakeRoomStore -import fixture.* import fixture.NotificationDiffFixtures.aNotificationDiff +import fixture.aRoomId +import fixture.aRoomMessageEvent +import fixture.aRoomOverview +import fixture.anEventId import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.toList import kotlinx.coroutines.test.runTest @@ -12,8 +15,8 @@ import org.amshove.kluent.shouldBeEqualTo import org.junit.Test private val NO_UNREADS = emptyMap>() -private val A_MESSAGE = aRoomMessageEvent(eventId = anEventId("1"), content = "hello") -private val A_MESSAGE_2 = aRoomMessageEvent(eventId = anEventId("2"), content = "world") +private val A_MESSAGE = aRoomMessageEvent(eventId = anEventId("1"), content = "hello", utcTimestamp = 1000) +private val A_MESSAGE_2 = aRoomMessageEvent(eventId = anEventId("2"), content = "world", utcTimestamp = 2000) private val A_ROOM_OVERVIEW = aRoomOverview(roomId = aRoomId("1")) private val A_ROOM_OVERVIEW_2 = aRoomOverview(roomId = aRoomId("2")) @@ -48,7 +51,7 @@ class ObserveUnreadRenderNotificationsUseCaseTest { changedOrNew = A_ROOM_OVERVIEW.toDiff(A_MESSAGE), newRooms = setOf(A_ROOM_OVERVIEW.roomId) ), - A_ROOM_OVERVIEW.withUnreads(A_MESSAGE, A_MESSAGE_2) to aNotificationDiff(changedOrNew = A_ROOM_OVERVIEW.toDiff(A_MESSAGE, A_MESSAGE_2)) + A_ROOM_OVERVIEW.withUnreads(A_MESSAGE, A_MESSAGE_2) to aNotificationDiff(changedOrNew = A_ROOM_OVERVIEW.toDiff(A_MESSAGE_2)) ) } @@ -61,7 +64,7 @@ class ObserveUnreadRenderNotificationsUseCaseTest { val result = useCase.invoke().toList() result shouldBeEqualTo listOf( - A_ROOM_OVERVIEW.withUnreads(A_MESSAGE, A_MESSAGE_2) to aNotificationDiff(changedOrNew = A_ROOM_OVERVIEW.toDiff(A_MESSAGE, A_MESSAGE_2)) + A_ROOM_OVERVIEW.withUnreads(A_MESSAGE, A_MESSAGE_2) to aNotificationDiff(changedOrNew = A_ROOM_OVERVIEW.toDiff(A_MESSAGE_2)) ) } @@ -76,6 +79,26 @@ class ObserveUnreadRenderNotificationsUseCaseTest { result shouldBeEqualTo emptyList() } + @Test + fun `given new and then historical message, when reading a message, then only emits the latest`() = runTest { + fakeRoomStore.givenUnreadEvents( + flowOf( + NO_UNREADS, + A_ROOM_OVERVIEW.withUnreads(A_MESSAGE), + A_ROOM_OVERVIEW.withUnreads(A_MESSAGE, A_MESSAGE.copy(eventId = anEventId("old"), utcTimestamp = -1)) + ) + ) + + val result = useCase.invoke().toList() + + result shouldBeEqualTo listOf( + A_ROOM_OVERVIEW.withUnreads(A_MESSAGE) to aNotificationDiff( + changedOrNew = A_ROOM_OVERVIEW.toDiff(A_MESSAGE), + newRooms = setOf(A_ROOM_OVERVIEW.roomId) + ), + ) + } + @Test fun `given initial unreads, when reading a duplicate unread, then emits nothing`() = runTest { fakeRoomStore.givenUnreadEvents( From 2a0f28d3b395031470cee92b0ec7163729b70d9e Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Tue, 26 Jul 2022 22:48:24 +0100 Subject: [PATCH 4/6] 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, From 3ac9df765df98138243e51af1e6feb1baa41d8f3 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 27 Jul 2022 05:34:28 +0000 Subject: [PATCH 5/6] Bump junit-jupiter-engine from 5.8.2 to 5.9.0 Bumps [junit-jupiter-engine](https://github.com/junit-team/junit5) from 5.8.2 to 5.9.0. - [Release notes](https://github.com/junit-team/junit5/releases) - [Commits](https://github.com/junit-team/junit5/compare/r5.8.2...r5.9.0) --- updated-dependencies: - dependency-name: org.junit.jupiter:junit-jupiter-engine dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 2bdc19a..d3b79c0 100644 --- a/build.gradle +++ b/build.gradle @@ -136,7 +136,7 @@ ext.kotlinTest = { dependencies -> dependencies.testImplementation 'org.jetbrains.kotlinx:kotlinx-coroutines-test:1.6.4' dependencies.testImplementation 'org.junit.jupiter:junit-jupiter-api:5.8.2' - dependencies.testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine:5.8.2' + dependencies.testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine:5.9.0' } ext.kotlinFixtures = { dependencies -> From 8c1265c957c57aa6d6dd8a536e15d1212a85f3ae Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 28 Jul 2022 19:43:27 +0100 Subject: [PATCH 6/6] avoiding notification overlap by only using alerting summary group instead of all notifications --- .../st/notifications/NotificationChannels.kt | 13 +++++++++++++ .../st/notifications/NotificationFactory.kt | 15 +++++++++------ .../st/notifications/NotificationRenderer.kt | 3 ++- .../notifications/NotificationFactoryTest.kt | 18 ++++++++++++++---- .../kotlin/fixture/NotificationFixtures.kt | 4 +++- 5 files changed, 41 insertions(+), 12 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 0ca041f..12dbd8d 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 @@ -7,6 +7,7 @@ import android.os.Build const val DIRECT_CHANNEL_ID = "direct_channel_id" const val GROUP_CHANNEL_ID = "group_channel_id" +const val SUMMARY_CHANNEL_ID = "summary_channel_id" private const val CHATS_NOTIFICATION_GROUP_ID = "chats_notification_group" @@ -43,6 +44,18 @@ class NotificationChannels( } ) } + + if (notificationManager.getNotificationChannel(SUMMARY_CHANNEL_ID) == null) { + notificationManager.createNotificationChannel( + NotificationChannel( + SUMMARY_CHANNEL_ID, + "Other notifications", + NotificationManager.IMPORTANCE_DEFAULT, + ).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 87068cb..50e8193 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 @@ -37,10 +37,7 @@ class NotificationFactory( val last = sortedEvents.last() return NotificationTypes.Room( AndroidNotification( - channelId = when { - roomOverview.isDm() -> DIRECT_CHANNEL_ID - else -> GROUP_CHANNEL_ID - }, + channelId = SUMMARY_CHANNEL_ID, whenTimestamp = last.utcTimestamp, groupId = GROUP_ID, groupAlertBehavior = deviceMeta.whenPOrHigher( @@ -59,7 +56,11 @@ class NotificationFactory( roomId = roomOverview.roomId, summary = last.content, messageCount = sortedEvents.size, - isAlerting = shouldAlertMoreThanOnce + isAlerting = shouldAlertMoreThanOnce, + summaryChannelId = when { + roomOverview.isDm() -> DIRECT_CHANNEL_ID + else -> GROUP_CHANNEL_ID + } ) } @@ -67,7 +68,7 @@ class NotificationFactory( val summaryInboxStyle = notificationStyleFactory.summary(notifications) val openAppIntent = intentFactory.notificationOpenApp(context) return AndroidNotification( - channelId = notifications.firstOrNull { it.isAlerting }?.notification?.channelId ?: notifications.first().notification.channelId, + channelId = notifications.mostRecent().summaryChannelId, messageStyle = summaryInboxStyle, alertMoreThanOnce = notifications.any { it.isAlerting }, smallIcon = R.drawable.ic_notification_small_icon, @@ -83,4 +84,6 @@ class NotificationFactory( } } +private fun List.mostRecent() = this.sortedBy { it.notification.whenTimestamp }.first() + private fun RoomOverview.isDm() = !this.isGroup diff --git a/features/notifications/src/main/kotlin/app/dapk/st/notifications/NotificationRenderer.kt b/features/notifications/src/main/kotlin/app/dapk/st/notifications/NotificationRenderer.kt index 5d4e177..5a2cc7c 100644 --- a/features/notifications/src/main/kotlin/app/dapk/st/notifications/NotificationRenderer.kt +++ b/features/notifications/src/main/kotlin/app/dapk/st/notifications/NotificationRenderer.kt @@ -68,7 +68,8 @@ sealed interface NotificationTypes { val roomId: RoomId, val summary: String, val messageCount: Int, - val isAlerting: Boolean + val isAlerting: Boolean, + val summaryChannelId: String, ) : NotificationTypes data class DismissRoom(val roomId: RoomId) : NotificationTypes 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 af98498..0c466b6 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 @@ -32,7 +32,6 @@ private val EVENTS = listOf( aNotifiable("message two", utcTimestamp = 2), ) - class NotificationFactoryTest { private val fakeContext = FakeContext() @@ -50,7 +49,12 @@ class NotificationFactoryTest { @Test fun `given alerting room notification, when creating summary, then is alerting`() { - val notifications = listOf(aRoomNotification(notification = anAndroidNotification(channelId = A_CHANNEL_ID), isAlerting = true)) + val notifications = listOf( + aRoomNotification( + summaryChannelId = A_CHANNEL_ID, + notification = anAndroidNotification(channelId = A_CHANNEL_ID), isAlerting = true + ) + ) fakeIntentFactory.givenNotificationOpenApp(fakeContext.instance).returns(AN_OPEN_APP_INTENT) fakeNotificationStyleFactory.givenSummary(notifications).returns(anInboxStyle()) @@ -61,7 +65,12 @@ class NotificationFactoryTest { @Test fun `given non alerting room notification, when creating summary, then is alerting`() { - val notifications = listOf(aRoomNotification(notification = anAndroidNotification(channelId = A_CHANNEL_ID), isAlerting = false)) + val notifications = listOf( + aRoomNotification( + summaryChannelId = A_CHANNEL_ID, + notification = anAndroidNotification(channelId = A_CHANNEL_ID), isAlerting = false + ) + ) fakeIntentFactory.givenNotificationOpenApp(fakeContext.instance).returns(AN_OPEN_APP_INTENT) fakeNotificationStyleFactory.givenSummary(notifications).returns(anInboxStyle()) @@ -129,7 +138,7 @@ class NotificationFactoryTest { shouldAlertMoreThanOnce: Boolean, ) = NotificationTypes.Room( AndroidNotification( - channelId = channel, + channelId = SUMMARY_CHANNEL_ID, whenTimestamp = LATEST_EVENT.utcTimestamp, groupId = "st", groupAlertBehavior = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) Notification.GROUP_ALERT_SUMMARY else null, @@ -146,6 +155,7 @@ class NotificationFactoryTest { summary = LATEST_EVENT.content, messageCount = EVENTS.size, isAlerting = shouldAlertMoreThanOnce, + summaryChannelId = channel, ) private fun expectedSummary(channelId: String, shouldAlertMoreThanOnce: Boolean) = AndroidNotification( diff --git a/features/notifications/src/test/kotlin/fixture/NotificationFixtures.kt b/features/notifications/src/test/kotlin/fixture/NotificationFixtures.kt index caa8a7a..46d824a 100644 --- a/features/notifications/src/test/kotlin/fixture/NotificationFixtures.kt +++ b/features/notifications/src/test/kotlin/fixture/NotificationFixtures.kt @@ -18,12 +18,14 @@ object NotificationFixtures { summary: String = "a summary line", messageCount: Int = 1, isAlerting: Boolean = false, + summaryChannelId: String = "a-summary-channel-id", ) = NotificationTypes.Room( notification, aRoomId(), summary = summary, messageCount = messageCount, - isAlerting = isAlerting + isAlerting = isAlerting, + summaryChannelId = summaryChannelId ) fun aDismissRoomNotification(