Merge pull request #5726 from element-hq/feature/bma/notificationCleanup

Notification robustness
This commit is contained in:
Benoit Marty 2025-11-14 18:02:45 +01:00 committed by GitHub
commit b40ccd94c8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
49 changed files with 1254 additions and 480 deletions

View file

@ -72,7 +72,7 @@ class RingingCallNotificationCreator(
): Notification? {
val matrixClient = matrixClientProvider.getOrRestore(sessionId).getOrNull() ?: return null
val imageLoader = imageLoaderHolder.get(matrixClient)
val largeIcon = notificationBitmapLoader.getUserIcon(
val userIcon = notificationBitmapLoader.getUserIcon(
avatarData = AvatarData(
id = roomId.value,
name = roomName,
@ -84,7 +84,7 @@ class RingingCallNotificationCreator(
val caller = Person.Builder()
.setName(senderDisplayName)
.setIcon(largeIcon)
.setIcon(userIcon)
.setImportant(true)
.build()
@ -133,12 +133,8 @@ class RingingCallNotificationCreator(
.setWhen(timestamp)
.setOngoing(true)
.setShowWhen(false)
.apply {
if (textContent != null) {
setContentText(textContent)
// Else the content text is set by the style (will be "Incoming call")
}
}
// If textContent is null, the content text is set by the style (will be "Incoming call")
.setContentText(textContent)
.setSound(Settings.System.DEFAULT_RINGTONE_URI, AudioManager.STREAM_RING)
.setTimeoutAfter(ElementCallConfig.RINGING_CALL_DURATION_SECONDS.seconds.inWholeMilliseconds)
.setContentIntent(answerIntent)

View file

@ -22,7 +22,6 @@ import androidx.core.app.NotificationManagerCompat
import androidx.core.app.PendingIntentCompat
import androidx.core.app.ServiceCompat
import androidx.core.content.ContextCompat
import androidx.core.graphics.drawable.IconCompat
import io.element.android.features.call.impl.R
import io.element.android.features.call.impl.ui.ElementCallActivity
import io.element.android.libraries.core.extensions.runCatchingExceptions
@ -69,7 +68,7 @@ class CallForegroundService : Service() {
val callActivityIntent = Intent(this, ElementCallActivity::class.java)
val pendingIntent = PendingIntentCompat.getActivity(this, 0, callActivityIntent, 0, false)
val notification = NotificationCompat.Builder(this, foregroundServiceChannel.id)
.setSmallIcon(IconCompat.createWithResource(this, CommonDrawables.ic_notification))
.setSmallIcon(CommonDrawables.ic_notification)
.setContentTitle(getString(R.string.call_foreground_service_title_android))
.setContentText(getString(R.string.call_foreground_service_message_android))
.setContentIntent(pendingIntent)

View file

@ -100,7 +100,7 @@ class DefaultActiveCallManager(
private val imageLoaderHolder: ImageLoaderHolder,
private val systemClock: SystemClock,
) : ActiveCallManager {
private val tag = "DefaultActiveCallManager"
private val tag = "ActiveCallManager"
private var timedOutCallJob: Job? = null
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)

View file

@ -25,6 +25,7 @@ import io.element.android.libraries.architecture.AsyncData
import io.element.android.libraries.architecture.Presenter
import io.element.android.libraries.architecture.runUpdatingStateNoSuccess
import io.element.android.libraries.core.extensions.runCatchingExceptions
import io.element.android.libraries.di.annotations.SessionCoroutineScope
import io.element.android.libraries.fullscreenintent.api.FullScreenIntentPermissionsState
import io.element.android.libraries.matrix.api.MatrixClient
import io.element.android.libraries.matrix.api.notificationsettings.NotificationSettingsService
@ -51,6 +52,8 @@ class NotificationSettingsPresenter(
private val pushService: PushService,
private val systemNotificationsEnabledProvider: SystemNotificationsEnabledProvider,
private val fullScreenIntentPermissionsPresenter: Presenter<FullScreenIntentPermissionsState>,
@SessionCoroutineScope
private val sessionCoroutineScope: CoroutineScope,
) : Presenter<NotificationSettingsState> {
@Composable
override fun present(): NotificationSettingsState {
@ -141,7 +144,7 @@ class NotificationSettingsPresenter(
is NotificationSettingsEvents.SetInviteForMeNotificationsEnabled -> {
localCoroutineScope.setInviteForMeNotificationsEnabled(event.enabled, changeNotificationSettingAction)
}
is NotificationSettingsEvents.SetNotificationsEnabled -> localCoroutineScope.setNotificationsEnabled(userPushStore, event.enabled)
is NotificationSettingsEvents.SetNotificationsEnabled -> sessionCoroutineScope.setNotificationsEnabled(userPushStore, event.enabled)
NotificationSettingsEvents.ClearConfigurationMismatchError -> {
matrixSettings.value = NotificationSettingsState.MatrixSettings.Invalid(fixFailed = false)
}
@ -262,5 +265,10 @@ class NotificationSettingsPresenter(
private fun CoroutineScope.setNotificationsEnabled(userPushStore: UserPushStore, enabled: Boolean) = launch {
userPushStore.setNotificationEnabledForDevice(enabled)
if (enabled) {
pushService.ensurePusherIsRegistered(matrixClient)
} else {
pushService.getCurrentPushProvider(matrixClient.sessionId)?.unregister(matrixClient)
}
}
}

View file

@ -8,9 +8,6 @@
package io.element.android.features.preferences.impl.notifications
import app.cash.molecule.RecompositionMode
import app.cash.molecule.moleculeFlow
import app.cash.turbine.test
import com.google.common.truth.Truth.assertThat
import io.element.android.libraries.architecture.AsyncData
import io.element.android.libraries.fullscreenintent.api.FullScreenIntentPermissionsState
@ -28,6 +25,9 @@ import io.element.android.libraries.pushproviders.test.FakePushProvider
import io.element.android.libraries.pushstore.test.userpushstore.FakeUserPushStoreFactory
import io.element.android.tests.testutils.awaitLastSequentialItem
import io.element.android.tests.testutils.consumeItemsUntilPredicate
import io.element.android.tests.testutils.lambda.lambdaRecorder
import io.element.android.tests.testutils.test
import kotlinx.coroutines.test.TestScope
import kotlinx.coroutines.test.runTest
import org.junit.Test
import kotlin.time.Duration.Companion.milliseconds
@ -36,9 +36,7 @@ class NotificationSettingsPresenterTest {
@Test
fun `present - ensures initial state is correct`() = runTest {
val presenter = createNotificationSettingsPresenter()
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
val initialState = awaitItem()
assertThat(initialState.appSettings.appNotificationsEnabled).isFalse()
assertThat(initialState.appSettings.systemNotificationsEnabled).isTrue()
@ -62,9 +60,7 @@ class NotificationSettingsPresenterTest {
fun `present - default group notification mode changed`() = runTest {
val notificationSettingsService = FakeNotificationSettingsService()
val presenter = createNotificationSettingsPresenter(notificationSettingsService)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
notificationSettingsService.setDefaultRoomNotificationMode(isEncrypted = true, isOneToOne = false, mode = RoomNotificationMode.ALL_MESSAGES)
notificationSettingsService.setDefaultRoomNotificationMode(isEncrypted = false, isOneToOne = false, mode = RoomNotificationMode.ALL_MESSAGES)
val updatedState = consumeItemsUntilPredicate {
@ -80,9 +76,7 @@ class NotificationSettingsPresenterTest {
fun `present - notification settings mismatched`() = runTest {
val notificationSettingsService = FakeNotificationSettingsService()
val presenter = createNotificationSettingsPresenter(notificationSettingsService)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
notificationSettingsService.setDefaultRoomNotificationMode(
isEncrypted = true,
isOneToOne = false,
@ -110,9 +104,7 @@ class NotificationSettingsPresenterTest {
initialOneToOneDefaultMode = RoomNotificationMode.MENTIONS_AND_KEYWORDS_ONLY
)
val presenter = createNotificationSettingsPresenter(notificationSettingsService)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
val initialState = awaitItem()
initialState.eventSink(NotificationSettingsEvents.FixConfigurationMismatch)
val fixedState = consumeItemsUntilPredicate(timeout = 2000.milliseconds) {
@ -125,10 +117,19 @@ class NotificationSettingsPresenterTest {
@Test
fun `present - set notifications enabled`() = runTest {
val presenter = createNotificationSettingsPresenter()
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
val unregisterWithResult = lambdaRecorder<MatrixClient, Result<Unit>> { Result.success(Unit) }
val ensurePusherIsRegisteredResult = lambdaRecorder<Result<Unit>> { Result.success(Unit) }
val presenter = createNotificationSettingsPresenter(
pushService = FakePushService(
currentPushProvider = {
FakePushProvider(
unregisterWithResult = unregisterWithResult,
)
},
ensurePusherIsRegisteredResult = ensurePusherIsRegisteredResult,
)
)
presenter.test {
val loadedState = consumeItemsUntilPredicate {
it.matrixSettings is NotificationSettingsState.MatrixSettings.Valid
}.last()
@ -138,16 +139,21 @@ class NotificationSettingsPresenterTest {
!it.appSettings.appNotificationsEnabled
}.last()
assertThat(updatedState.appSettings.appNotificationsEnabled).isFalse()
cancelAndIgnoreRemainingEvents()
unregisterWithResult.assertions().isCalledOnce()
// Enable notification again
loadedState.eventSink(NotificationSettingsEvents.SetNotificationsEnabled(true))
val updatedState2 = consumeItemsUntilPredicate {
it.appSettings.appNotificationsEnabled
}.last()
assertThat(updatedState2.appSettings.appNotificationsEnabled).isTrue()
ensurePusherIsRegisteredResult.assertions().isCalledOnce()
}
}
@Test
fun `present - set call notifications enabled`() = runTest {
val presenter = createNotificationSettingsPresenter()
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
val loadedState = consumeItemsUntilPredicate {
(it.matrixSettings as? NotificationSettingsState.MatrixSettings.Valid)?.callNotificationsEnabled == false
}.last()
@ -166,9 +172,7 @@ class NotificationSettingsPresenterTest {
@Test
fun `present - set invite for me notifications enabled`() = runTest {
val presenter = createNotificationSettingsPresenter()
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
val loadedState = consumeItemsUntilPredicate {
(it.matrixSettings as? NotificationSettingsState.MatrixSettings.Valid)?.inviteForMeNotificationsEnabled == false
}.last()
@ -187,9 +191,7 @@ class NotificationSettingsPresenterTest {
@Test
fun `present - set atRoom notifications enabled`() = runTest {
val presenter = createNotificationSettingsPresenter()
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
val loadedState = consumeItemsUntilPredicate {
(it.matrixSettings as? NotificationSettingsState.MatrixSettings.Valid)?.atRoomNotificationsEnabled == false
}.last()
@ -210,9 +212,7 @@ class NotificationSettingsPresenterTest {
val notificationSettingsService = FakeNotificationSettingsService()
val presenter = createNotificationSettingsPresenter(notificationSettingsService)
notificationSettingsService.givenSetAtRoomError(AN_EXCEPTION)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
val loadedState = consumeItemsUntilPredicate {
(it.matrixSettings as? NotificationSettingsState.MatrixSettings.Valid)?.atRoomNotificationsEnabled == false
}.last()
@ -237,9 +237,7 @@ class NotificationSettingsPresenterTest {
val presenter = createNotificationSettingsPresenter(
pushService = createFakePushService(),
)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
val initialState = awaitLastSequentialItem()
assertThat(initialState.currentPushDistributor).isEqualTo(AsyncData.Success(Distributor(value = "aDistributorValue0", name = "aDistributorName0")))
assertThat(initialState.availablePushDistributors).containsExactly(
@ -271,9 +269,7 @@ class NotificationSettingsPresenterTest {
val presenter = createNotificationSettingsPresenter(
pushService = createFakePushService(),
)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
val initialState = awaitLastSequentialItem()
assertThat(initialState.currentPushDistributor).isEqualTo(AsyncData.Success(Distributor(value = "aDistributorValue0", name = "aDistributorName0")))
assertThat(initialState.availablePushDistributors).containsExactly(
@ -298,9 +294,7 @@ class NotificationSettingsPresenterTest {
pushService = createFakePushService(),
fullScreenIntentPermissionsStateLambda = fullScreenIntentPermissionsStateLambda,
)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
val initialState = awaitLastSequentialItem()
assertThat(initialState.fullScreenIntentPermissionsState.permissionGranted).isFalse()
@ -324,9 +318,7 @@ class NotificationSettingsPresenterTest {
},
),
)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
val initialState = awaitLastSequentialItem()
initialState.eventSink.invoke(NotificationSettingsEvents.ChangePushProvider)
val withDialog = awaitItem()
@ -341,7 +333,7 @@ class NotificationSettingsPresenterTest {
}
private fun createFakePushService(
registerWithLambda: suspend (MatrixClient, PushProvider, Distributor) -> Result<Unit> = { _, _, _ ->
registerWithLambda: (MatrixClient, PushProvider, Distributor) -> Result<Unit> = { _, _, _ ->
Result.success(Unit)
}
): PushService {
@ -361,7 +353,7 @@ class NotificationSettingsPresenterTest {
)
}
private fun createNotificationSettingsPresenter(
private fun TestScope.createNotificationSettingsPresenter(
notificationSettingsService: FakeNotificationSettingsService = FakeNotificationSettingsService(),
pushService: PushService = FakePushService(),
fullScreenIntentPermissionsStateLambda: () -> FullScreenIntentPermissionsState = { aFullScreenIntentPermissionsState() },
@ -374,6 +366,7 @@ class NotificationSettingsPresenterTest {
pushService = pushService,
systemNotificationsEnabledProvider = FakeSystemNotificationsEnabledProvider(),
fullScreenIntentPermissionsPresenter = { fullScreenIntentPermissionsStateLambda() },
sessionCoroutineScope = backgroundScope,
)
}
}