Simplify summary notifications (#2958)
This commit is contained in:
committed by
GitHub
parent
99f59c54b2
commit
0d82f19cc0
@@ -57,7 +57,6 @@ interface NotificationDataFactory {
|
||||
invitationNotifications: List<OneShotNotification>,
|
||||
simpleNotifications: List<OneShotNotification>,
|
||||
fallbackNotifications: List<OneShotNotification>,
|
||||
useCompleteNotificationFormat: Boolean
|
||||
): SummaryNotification
|
||||
}
|
||||
|
||||
@@ -149,7 +148,6 @@ class DefaultNotificationDataFactory @Inject constructor(
|
||||
invitationNotifications: List<OneShotNotification>,
|
||||
simpleNotifications: List<OneShotNotification>,
|
||||
fallbackNotifications: List<OneShotNotification>,
|
||||
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
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<OneShotNotification>,
|
||||
simpleNotifications: List<OneShotNotification>,
|
||||
fallbackNotifications: List<OneShotNotification>,
|
||||
useCompleteNotificationFormat: Boolean
|
||||
): Notification
|
||||
}
|
||||
|
||||
@@ -58,21 +55,11 @@ class DefaultSummaryGroupMessageCreator @Inject constructor(
|
||||
invitationNotifications: List<OneShotNotification>,
|
||||
simpleNotifications: List<OneShotNotification>,
|
||||
fallbackNotifications: List<OneShotNotification>,
|
||||
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
|
||||
)
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -160,6 +160,7 @@ class DefaultActiveNotificationsProviderTest {
|
||||
private fun aStatusBarNotification(id: Int, groupId: String, tag: String? = null) = mockk<StatusBarNotification> {
|
||||
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()
|
||||
}
|
||||
|
||||
|
||||
@@ -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")
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<InviteNotifiableEvent, Notification> = lambdaRecorder { _ -> A_NOTIFICATION },
|
||||
var createSimpleNotificationResult: LambdaOneParamRecorder<SimpleNotifiableEvent, Notification> = lambdaRecorder { _ -> A_NOTIFICATION },
|
||||
var createFallbackNotificationResult: LambdaOneParamRecorder<FallbackNotifiableEvent, Notification> = lambdaRecorder { _ -> A_NOTIFICATION },
|
||||
var createSummaryListNotificationResult: LambdaFiveParamsRecorder<MatrixUser, NotificationCompat.InboxStyle?, String, Boolean, Long, Notification> =
|
||||
lambdaRecorder { _, _, _, _, _ -> A_NOTIFICATION },
|
||||
var createSummaryListNotificationResult: LambdaFourParamsRecorder<MatrixUser, String, Boolean, Long, Notification> =
|
||||
lambdaRecorder { _, _, _, _ -> A_NOTIFICATION },
|
||||
var createDiagnosticNotificationResult: LambdaNoParamRecorder<Notification> = 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 {
|
||||
|
||||
@@ -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<List<NotifiableMessageEvent>, MatrixUser, ImageLoader, List<RoomNotification>> =
|
||||
lambdaRecorder { _, _, _ -> emptyList() },
|
||||
var summaryToNotificationsResult: LambdaSixParamsRecorder<
|
||||
var summaryToNotificationsResult: LambdaFiveParamsRecorder<
|
||||
MatrixUser,
|
||||
List<RoomNotification>,
|
||||
List<OneShotNotification>,
|
||||
List<OneShotNotification>,
|
||||
List<OneShotNotification>,
|
||||
Boolean,
|
||||
SummaryNotification
|
||||
> = lambdaRecorder { _, _, _, _, _, _ -> SummaryNotification.Update(A_NOTIFICATION) },
|
||||
> = lambdaRecorder { _, _, _, _, _ -> SummaryNotification.Update(A_NOTIFICATION) },
|
||||
var inviteToNotificationsResult: LambdaOneParamRecorder<List<InviteNotifiableEvent>, List<OneShotNotification>> = lambdaRecorder { _ -> emptyList() },
|
||||
var simpleEventToNotificationsResult: LambdaOneParamRecorder<List<SimpleNotifiableEvent>, List<OneShotNotification>> = lambdaRecorder { _ -> emptyList() },
|
||||
var fallbackEventToNotificationsResult: LambdaOneParamRecorder<List<FallbackNotifiableEvent>, List<OneShotNotification>> =
|
||||
@@ -75,7 +74,6 @@ class FakeNotificationDataFactory(
|
||||
invitationNotifications: List<OneShotNotification>,
|
||||
simpleNotifications: List<OneShotNotification>,
|
||||
fallbackNotifications: List<OneShotNotification>,
|
||||
useCompleteNotificationFormat: Boolean
|
||||
): SummaryNotification {
|
||||
return summaryToNotificationsResult(
|
||||
currentUser,
|
||||
@@ -83,7 +81,6 @@ class FakeNotificationDataFactory(
|
||||
invitationNotifications,
|
||||
simpleNotifications,
|
||||
fallbackNotifications,
|
||||
useCompleteNotificationFormat
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<RoomNotification>, List<OneShotNotification>, List<OneShotNotification>, List<OneShotNotification>, Boolean, Notification
|
||||
var createSummaryNotificationResult: LambdaFiveParamsRecorder<
|
||||
MatrixUser, List<RoomNotification>, List<OneShotNotification>, List<OneShotNotification>, List<OneShotNotification>, Notification
|
||||
> =
|
||||
lambdaRecorder { _, _, _, _, _, _ -> A_NOTIFICATION }
|
||||
lambdaRecorder { _, _, _, _, _ -> A_NOTIFICATION }
|
||||
) : SummaryGroupMessageCreator {
|
||||
override fun createSummaryNotification(
|
||||
currentUser: MatrixUser,
|
||||
@@ -37,7 +37,6 @@ class FakeSummaryGroupMessageCreator(
|
||||
invitationNotifications: List<OneShotNotification>,
|
||||
simpleNotifications: List<OneShotNotification>,
|
||||
fallbackNotifications: List<OneShotNotification>,
|
||||
useCompleteNotificationFormat: Boolean
|
||||
): Notification {
|
||||
return createSummaryNotificationResult(
|
||||
currentUser,
|
||||
@@ -45,7 +44,6 @@ class FakeSummaryGroupMessageCreator(
|
||||
invitationNotifications,
|
||||
simpleNotifications,
|
||||
fallbackNotifications,
|
||||
useCompleteNotificationFormat
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user