From 0d82f19cc05a09520b0b520b02ed4e436f90f05a Mon Sep 17 00:00:00 2001 From: Jorge Martin Espinosa Date: Fri, 31 May 2024 12:31:09 +0200 Subject: [PATCH] Simplify summary notifications (#2958) --- .../notifications/NotificationDataFactory.kt | 3 - .../notifications/NotificationRenderer.kt | 1 - .../SummaryGroupMessageCreator.kt | 117 +----------------- .../factories/NotificationCreator.kt | 5 - .../DefaultActiveNotificationsProviderTest.kt | 1 + .../DefaultSummaryGroupMessageCreatorTest.kt | 41 +----- .../DefaultNotificationCreatorTest.kt | 2 - .../fake/FakeNotificationCreator.kt | 10 +- .../fake/FakeNotificationDataFactory.kt | 9 +- .../fake/FakeSummaryGroupMessageCreator.kt | 10 +- 10 files changed, 16 insertions(+), 183 deletions(-) diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationDataFactory.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationDataFactory.kt index 45464bed8e..aff2cc7fca 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationDataFactory.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationDataFactory.kt @@ -57,7 +57,6 @@ interface NotificationDataFactory { invitationNotifications: List, simpleNotifications: List, fallbackNotifications: List, - useCompleteNotificationFormat: Boolean ): SummaryNotification } @@ -149,7 +148,6 @@ class DefaultNotificationDataFactory @Inject constructor( invitationNotifications: List, simpleNotifications: List, fallbackNotifications: List, - useCompleteNotificationFormat: Boolean ): SummaryNotification { return when { roomNotifications.isEmpty() && invitationNotifications.isEmpty() && simpleNotifications.isEmpty() -> SummaryNotification.Removed @@ -160,7 +158,6 @@ class DefaultNotificationDataFactory @Inject constructor( invitationNotifications = invitationNotifications, simpleNotifications = simpleNotifications, fallbackNotifications = fallbackNotifications, - useCompleteNotificationFormat = useCompleteNotificationFormat ) ) } diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationRenderer.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationRenderer.kt index b9f8edea05..a1509aaa96 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationRenderer.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationRenderer.kt @@ -52,7 +52,6 @@ class NotificationRenderer @Inject constructor( invitationNotifications = invitationNotifications, simpleNotifications = simpleNotifications, fallbackNotifications = fallbackNotifications, - useCompleteNotificationFormat = useCompleteNotificationFormat ) // Remove summary first to avoid briefly displaying it after dismissing the last notification diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/SummaryGroupMessageCreator.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/SummaryGroupMessageCreator.kt index 19cc4593e6..def1c58d3e 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/SummaryGroupMessageCreator.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/SummaryGroupMessageCreator.kt @@ -17,12 +17,10 @@ package io.element.android.libraries.push.impl.notifications import android.app.Notification -import androidx.core.app.NotificationCompat import com.squareup.anvil.annotations.ContributesBinding import io.element.android.libraries.di.AppScope import io.element.android.libraries.matrix.api.user.MatrixUser import io.element.android.libraries.push.impl.R -import io.element.android.libraries.push.impl.notifications.debug.annotateForDebug import io.element.android.libraries.push.impl.notifications.factories.NotificationCreator import io.element.android.services.toolbox.api.strings.StringProvider import javax.inject.Inject @@ -34,7 +32,6 @@ interface SummaryGroupMessageCreator { invitationNotifications: List, simpleNotifications: List, fallbackNotifications: List, - useCompleteNotificationFormat: Boolean ): Notification } @@ -58,21 +55,11 @@ class DefaultSummaryGroupMessageCreator @Inject constructor( invitationNotifications: List, simpleNotifications: List, fallbackNotifications: List, - useCompleteNotificationFormat: Boolean ): Notification { - val summaryInboxStyle = NotificationCompat.InboxStyle().also { style -> - roomNotifications.forEach { style.addLine(it.summaryLine.annotateForDebug(40)) } - invitationNotifications.forEach { style.addLine(it.summaryLine.annotateForDebug(41)) } - simpleNotifications.forEach { style.addLine(it.summaryLine.annotateForDebug(42)) } - fallbackNotifications.forEach { style.addLine(it.summaryLine) } - } - val summaryIsNoisy = roomNotifications.any { it.shouldBing } || invitationNotifications.any { it.isNoisy } || simpleNotifications.any { it.isNoisy } - val messageCount = roomNotifications.fold(initial = 0) { acc, current -> acc + current.messageCount } - val lastMessageTimestamp = roomNotifications.lastOrNull()?.latestTimestamp ?: invitationNotifications.lastOrNull()?.timestamp ?: simpleNotifications.last().timestamp @@ -80,109 +67,9 @@ class DefaultSummaryGroupMessageCreator @Inject constructor( // FIXME roomIdToEventMap.size is not correct, this is the number of rooms val nbEvents = roomNotifications.size + simpleNotifications.size val sumTitle = stringProvider.getQuantityString(R.plurals.notification_compat_summary_title, nbEvents, nbEvents) - summaryInboxStyle.setBigContentTitle(sumTitle.annotateForDebug(43)) - // .setSummaryText(stringProvider.getQuantityString(R.plurals.notification_unread_notified_messages, nbEvents, nbEvents).annotateForDebug(44)) - // Use account name now, for multi-session - .setSummaryText(currentUser.userId.value.annotateForDebug(44)) - return if (useCompleteNotificationFormat) { - notificationCreator.createSummaryListNotification( - currentUser, - summaryInboxStyle, - sumTitle, - noisy = summaryIsNoisy, - lastMessageTimestamp = lastMessageTimestamp - ) - } else { - processSimpleGroupSummary( - currentUser, - summaryIsNoisy, - messageCount, - simpleNotifications.size, - invitationNotifications.size, - roomNotifications.size, - lastMessageTimestamp - ) - } - } - - private fun processSimpleGroupSummary( - currentUser: MatrixUser, - summaryIsNoisy: Boolean, - messageEventsCount: Int, - simpleEventsCount: Int, - invitationEventsCount: Int, - roomCount: Int, - lastMessageTimestamp: Long - ): Notification { - // Add the simple events as message (?) - val messageNotificationCount = messageEventsCount + simpleEventsCount - - val privacyTitle = if (invitationEventsCount > 0) { - val invitationsStr = stringProvider.getQuantityString( - R.plurals.notification_invitations, - invitationEventsCount, - invitationEventsCount - ) - if (messageNotificationCount > 0) { - // Invitation and message - val messageStr = stringProvider.getQuantityString( - R.plurals.notification_new_messages_for_room, - messageNotificationCount, - messageNotificationCount - ) - if (roomCount > 1) { - // In several rooms - val roomStr = stringProvider.getQuantityString( - R.plurals.notification_unread_notified_messages_in_room_rooms, - roomCount, - roomCount - ) - stringProvider.getString( - R.string.notification_unread_notified_messages_in_room_and_invitation, - messageStr, - roomStr, - invitationsStr - ) - } else { - // In one room - stringProvider.getString( - R.string.notification_unread_notified_messages_and_invitation, - messageStr, - invitationsStr - ) - } - } else { - // Only invitation - invitationsStr - } - } else { - // No invitation, only messages - val messageStr = stringProvider.getQuantityString( - R.plurals.notification_new_messages_for_room, - messageNotificationCount, - messageNotificationCount - ) - if (roomCount > 1) { - // In several rooms - val roomStr = stringProvider.getQuantityString( - R.plurals.notification_unread_notified_messages_in_room_rooms, - roomCount, - roomCount - ) - stringProvider.getString( - R.string.notification_unread_notified_messages_in_room, - messageStr, - roomStr - ) - } else { - // In one room - messageStr - } - } return notificationCreator.createSummaryListNotification( - currentUser = currentUser, - style = null, - compatSummary = privacyTitle, + currentUser, + sumTitle, noisy = summaryIsNoisy, lastMessageTimestamp = lastMessageTimestamp ) diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/factories/NotificationCreator.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/factories/NotificationCreator.kt index d1851d44a2..cd6b32086b 100755 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/factories/NotificationCreator.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/factories/NotificationCreator.kt @@ -85,7 +85,6 @@ interface NotificationCreator { */ fun createSummaryListNotification( currentUser: MatrixUser, - style: NotificationCompat.InboxStyle?, compatSummary: String, noisy: Boolean, lastMessageTimestamp: Long @@ -331,7 +330,6 @@ class DefaultNotificationCreator @Inject constructor( */ override fun createSummaryListNotification( currentUser: MatrixUser, - style: NotificationCompat.InboxStyle?, compatSummary: String, noisy: Boolean, lastMessageTimestamp: Long @@ -343,12 +341,9 @@ class DefaultNotificationCreator @Inject constructor( .setOnlyAlertOnce(true) // used in compat < N, after summary is built based on child notifications .setWhen(lastMessageTimestamp) - .setStyle(style) - .setContentTitle(currentUser.userId.value.annotateForDebug(9)) .setCategory(NotificationCompat.CATEGORY_MESSAGE) .setSmallIcon(smallIcon) // set content text to support devices running API level < 24 - .setContentText(compatSummary.annotateForDebug(10)) .setGroup(currentUser.userId.value) // set this notification as the summary for the group .setGroupSummary(true) diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultActiveNotificationsProviderTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultActiveNotificationsProviderTest.kt index 6f40534335..4f715ed283 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultActiveNotificationsProviderTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultActiveNotificationsProviderTest.kt @@ -160,6 +160,7 @@ class DefaultActiveNotificationsProviderTest { private fun aStatusBarNotification(id: Int, groupId: String, tag: String? = null) = mockk { every { this@mockk.id } returns id every { this@mockk.tag } returns tag + @Suppress("DEPRECATION") every { this@mockk.notification } returns Notification.Builder(InstrumentationRegistry.getInstrumentation().targetContext).setGroup(groupId).build() } diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultSummaryGroupMessageCreatorTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultSummaryGroupMessageCreatorTest.kt index 190ae9330a..6f53f2e140 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultSummaryGroupMessageCreatorTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultSummaryGroupMessageCreatorTest.kt @@ -26,7 +26,6 @@ import io.element.android.services.toolbox.test.strings.FakeStringProvider import io.element.android.services.toolbox.test.systemclock.A_FAKE_TIMESTAMP import io.element.android.tests.testutils.lambda.any import io.element.android.tests.testutils.lambda.nonNull -import io.element.android.tests.testutils.lambda.value import kotlinx.coroutines.test.runTest import org.junit.Test import org.junit.runner.RunWith @@ -35,7 +34,7 @@ import org.robolectric.RobolectricTestRunner @RunWith(RobolectricTestRunner::class) class DefaultSummaryGroupMessageCreatorTest { @Test - fun `process notifications with complete format`() = runTest { + fun `process notifications`() = runTest { val notificationCreator = FakeNotificationCreator() val summaryCreator = DefaultSummaryGroupMessageCreator( stringProvider = FakeStringProvider(), @@ -57,47 +56,11 @@ class DefaultSummaryGroupMessageCreatorTest { invitationNotifications = emptyList(), simpleNotifications = emptyList(), fallbackNotifications = emptyList(), - useCompleteNotificationFormat = true, ) notificationCreator.createSummaryListNotificationResult.assertions() .isCalledOnce() - .with(any(), nonNull(), any(), any(), any()) - - // Set from the events included - @Suppress("DEPRECATION") - assertThat(result.priority).isEqualTo(NotificationCompat.PRIORITY_DEFAULT) - } - - @Test - fun `process notifications without complete format`() = runTest { - val notificationCreator = FakeNotificationCreator() - val summaryCreator = DefaultSummaryGroupMessageCreator( - stringProvider = FakeStringProvider(), - notificationCreator = notificationCreator, - ) - - val result = summaryCreator.createSummaryNotification( - currentUser = aMatrixUser(), - roomNotifications = listOf( - RoomNotification( - notification = Notification(), - roomId = A_ROOM_ID, - summaryLine = "", - messageCount = 1, - latestTimestamp = A_FAKE_TIMESTAMP + 10, - shouldBing = true, - ) - ), - invitationNotifications = emptyList(), - simpleNotifications = emptyList(), - fallbackNotifications = emptyList(), - useCompleteNotificationFormat = false, - ) - - notificationCreator.createSummaryListNotificationResult.assertions() - .isCalledOnce() - .with(any(), value(null), any(), any(), any()) + .with(any(), nonNull(), any(), any()) // Set from the events included @Suppress("DEPRECATION") diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/factories/DefaultNotificationCreatorTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/factories/DefaultNotificationCreatorTest.kt index 7af19d66a7..de0c22d1a6 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/factories/DefaultNotificationCreatorTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/factories/DefaultNotificationCreatorTest.kt @@ -192,7 +192,6 @@ class DefaultNotificationCreatorTest { val matrixUser = aMatrixUser() val result = sut.createSummaryListNotification( currentUser = matrixUser, - style = null, compatSummary = "compatSummary", noisy = false, lastMessageTimestamp = 123_456L, @@ -208,7 +207,6 @@ class DefaultNotificationCreatorTest { val matrixUser = aMatrixUser() val result = sut.createSummaryListNotification( currentUser = matrixUser, - style = null, compatSummary = "compatSummary", noisy = true, lastMessageTimestamp = 123_456L, diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/fake/FakeNotificationCreator.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/fake/FakeNotificationCreator.kt index bf20fef14e..988ec3083f 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/fake/FakeNotificationCreator.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/fake/FakeNotificationCreator.kt @@ -18,7 +18,6 @@ package io.element.android.libraries.push.impl.notifications.fake import android.app.Notification import android.graphics.Bitmap -import androidx.core.app.NotificationCompat import coil.ImageLoader import io.element.android.libraries.matrix.api.core.ThreadId import io.element.android.libraries.matrix.api.user.MatrixUser @@ -29,7 +28,7 @@ import io.element.android.libraries.push.impl.notifications.model.FallbackNotifi import io.element.android.libraries.push.impl.notifications.model.InviteNotifiableEvent import io.element.android.libraries.push.impl.notifications.model.NotifiableMessageEvent import io.element.android.libraries.push.impl.notifications.model.SimpleNotifiableEvent -import io.element.android.tests.testutils.lambda.LambdaFiveParamsRecorder +import io.element.android.tests.testutils.lambda.LambdaFourParamsRecorder import io.element.android.tests.testutils.lambda.LambdaListAnyParamsRecorder import io.element.android.tests.testutils.lambda.LambdaNoParamRecorder import io.element.android.tests.testutils.lambda.LambdaOneParamRecorder @@ -41,8 +40,8 @@ class FakeNotificationCreator( var createRoomInvitationNotificationResult: LambdaOneParamRecorder = lambdaRecorder { _ -> A_NOTIFICATION }, var createSimpleNotificationResult: LambdaOneParamRecorder = lambdaRecorder { _ -> A_NOTIFICATION }, var createFallbackNotificationResult: LambdaOneParamRecorder = lambdaRecorder { _ -> A_NOTIFICATION }, - var createSummaryListNotificationResult: LambdaFiveParamsRecorder = - lambdaRecorder { _, _, _, _, _ -> A_NOTIFICATION }, + var createSummaryListNotificationResult: LambdaFourParamsRecorder = + lambdaRecorder { _, _, _, _ -> A_NOTIFICATION }, var createDiagnosticNotificationResult: LambdaNoParamRecorder = lambdaRecorder { -> A_NOTIFICATION }, ) : NotificationCreator { override suspend fun createMessagesListNotification( @@ -75,12 +74,11 @@ class FakeNotificationCreator( override fun createSummaryListNotification( currentUser: MatrixUser, - style: NotificationCompat.InboxStyle?, compatSummary: String, noisy: Boolean, lastMessageTimestamp: Long ): Notification { - return createSummaryListNotificationResult(currentUser, style, compatSummary, noisy, lastMessageTimestamp) + return createSummaryListNotificationResult(currentUser, compatSummary, noisy, lastMessageTimestamp) } override fun createDiagnosticNotification(): Notification { diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/fake/FakeNotificationDataFactory.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/fake/FakeNotificationDataFactory.kt index 6cf4196bb5..221d3d0878 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/fake/FakeNotificationDataFactory.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/fake/FakeNotificationDataFactory.kt @@ -27,23 +27,22 @@ import io.element.android.libraries.push.impl.notifications.model.FallbackNotifi import io.element.android.libraries.push.impl.notifications.model.InviteNotifiableEvent import io.element.android.libraries.push.impl.notifications.model.NotifiableMessageEvent import io.element.android.libraries.push.impl.notifications.model.SimpleNotifiableEvent +import io.element.android.tests.testutils.lambda.LambdaFiveParamsRecorder import io.element.android.tests.testutils.lambda.LambdaOneParamRecorder -import io.element.android.tests.testutils.lambda.LambdaSixParamsRecorder import io.element.android.tests.testutils.lambda.LambdaThreeParamsRecorder import io.element.android.tests.testutils.lambda.lambdaRecorder class FakeNotificationDataFactory( var messageEventToNotificationsResult: LambdaThreeParamsRecorder, MatrixUser, ImageLoader, List> = lambdaRecorder { _, _, _ -> emptyList() }, - var summaryToNotificationsResult: LambdaSixParamsRecorder< + var summaryToNotificationsResult: LambdaFiveParamsRecorder< MatrixUser, List, List, List, List, - Boolean, SummaryNotification - > = lambdaRecorder { _, _, _, _, _, _ -> SummaryNotification.Update(A_NOTIFICATION) }, + > = lambdaRecorder { _, _, _, _, _ -> SummaryNotification.Update(A_NOTIFICATION) }, var inviteToNotificationsResult: LambdaOneParamRecorder, List> = lambdaRecorder { _ -> emptyList() }, var simpleEventToNotificationsResult: LambdaOneParamRecorder, List> = lambdaRecorder { _ -> emptyList() }, var fallbackEventToNotificationsResult: LambdaOneParamRecorder, List> = @@ -75,7 +74,6 @@ class FakeNotificationDataFactory( invitationNotifications: List, simpleNotifications: List, fallbackNotifications: List, - useCompleteNotificationFormat: Boolean ): SummaryNotification { return summaryToNotificationsResult( currentUser, @@ -83,7 +81,6 @@ class FakeNotificationDataFactory( invitationNotifications, simpleNotifications, fallbackNotifications, - useCompleteNotificationFormat ) } } diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/fake/FakeSummaryGroupMessageCreator.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/fake/FakeSummaryGroupMessageCreator.kt index 08083ceb18..07f142b43f 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/fake/FakeSummaryGroupMessageCreator.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/fake/FakeSummaryGroupMessageCreator.kt @@ -22,14 +22,14 @@ import io.element.android.libraries.push.impl.notifications.OneShotNotification import io.element.android.libraries.push.impl.notifications.RoomNotification import io.element.android.libraries.push.impl.notifications.SummaryGroupMessageCreator import io.element.android.libraries.push.impl.notifications.fixtures.A_NOTIFICATION -import io.element.android.tests.testutils.lambda.LambdaSixParamsRecorder +import io.element.android.tests.testutils.lambda.LambdaFiveParamsRecorder import io.element.android.tests.testutils.lambda.lambdaRecorder class FakeSummaryGroupMessageCreator( - var createSummaryNotificationResult: LambdaSixParamsRecorder< - MatrixUser, List, List, List, List, Boolean, Notification + var createSummaryNotificationResult: LambdaFiveParamsRecorder< + MatrixUser, List, List, List, List, Notification > = - lambdaRecorder { _, _, _, _, _, _ -> A_NOTIFICATION } + lambdaRecorder { _, _, _, _, _ -> A_NOTIFICATION } ) : SummaryGroupMessageCreator { override fun createSummaryNotification( currentUser: MatrixUser, @@ -37,7 +37,6 @@ class FakeSummaryGroupMessageCreator( invitationNotifications: List, simpleNotifications: List, fallbackNotifications: List, - useCompleteNotificationFormat: Boolean ): Notification { return createSummaryNotificationResult( currentUser, @@ -45,7 +44,6 @@ class FakeSummaryGroupMessageCreator( invitationNotifications, simpleNotifications, fallbackNotifications, - useCompleteNotificationFormat ) } }