Merge pull request #3022 from element-hq/feature/bma/notificationSetting

Improve UX on notification setting changes

changelog.d/1647.bugfix

@@ -0,0 +1 @@
+Improve UX on notification setting changes.

features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsPresenter.kt

@@ -29,7 +29,7 @@ import androidx.compose.runtime.setValue
 import io.element.android.libraries.architecture.AsyncAction
 import io.element.android.libraries.architecture.AsyncData
 import io.element.android.libraries.architecture.Presenter
-import io.element.android.libraries.architecture.runCatchingUpdatingState
+import io.element.android.libraries.architecture.runUpdatingStateNoSuccess
 import io.element.android.libraries.matrix.api.MatrixClient
 import io.element.android.libraries.matrix.api.notificationsettings.NotificationSettingsService
 import io.element.android.libraries.matrix.api.room.RoomNotificationMode
@@ -74,7 +74,7 @@ class NotificationSettingsPresenter @Inject constructor(
 
         LaunchedEffect(Unit) {
             fetchSettings(matrixSettings)
-            observeNotificationSettings(matrixSettings)
+            observeNotificationSettings(matrixSettings, changeNotificationSettingAction)
         }
 
         // List of PushProvider -> Distributor
@@ -172,11 +172,15 @@ class NotificationSettingsPresenter @Inject constructor(
     }
 
     @OptIn(FlowPreview::class)
-    private fun CoroutineScope.observeNotificationSettings(target: MutableState<NotificationSettingsState.MatrixSettings>) {
+    private fun CoroutineScope.observeNotificationSettings(
+        target: MutableState<NotificationSettingsState.MatrixSettings>,
+        changeNotificationSettingAction: MutableState<AsyncAction<Unit>>,
+    ) {
         notificationSettingsService.notificationSettingsChangeFlow
             .debounce(0.5.seconds)
             .onEach {
                 fetchSettings(target)
+                changeNotificationSettingAction.value = AsyncAction.Uninitialized
             }
             .launchIn(this)
     }
@@ -238,21 +242,21 @@ class NotificationSettingsPresenter @Inject constructor(
     }
 
     private fun CoroutineScope.setAtRoomNotificationsEnabled(enabled: Boolean, action: MutableState<AsyncAction<Unit>>) = launch {
-        suspend {
+        action.runUpdatingStateNoSuccess {
-            notificationSettingsService.setRoomMentionEnabled(enabled).getOrThrow()
+            notificationSettingsService.setRoomMentionEnabled(enabled)
-        }.runCatchingUpdatingState(action)
+        }
     }
 
     private fun CoroutineScope.setCallNotificationsEnabled(enabled: Boolean, action: MutableState<AsyncAction<Unit>>) = launch {
-        suspend {
+        action.runUpdatingStateNoSuccess {
-            notificationSettingsService.setCallEnabled(enabled).getOrThrow()
+            notificationSettingsService.setCallEnabled(enabled)
-        }.runCatchingUpdatingState(action)
+        }
     }
 
     private fun CoroutineScope.setInviteForMeNotificationsEnabled(enabled: Boolean, action: MutableState<AsyncAction<Unit>>) = launch {
-        suspend {
+        action.runUpdatingStateNoSuccess {
-            notificationSettingsService.setInviteForMeEnabled(enabled).getOrThrow()
+            notificationSettingsService.setInviteForMeEnabled(enabled)
-        }.runCatchingUpdatingState(action)
+        }
     }
 
     private fun CoroutineScope.setNotificationsEnabled(userPushStore: UserPushStore, enabled: Boolean) = launch {

features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/edit/EditDefaultNotificationSettingPresenter.kt

@@ -29,7 +29,7 @@ import dagger.assisted.AssistedFactory
 import dagger.assisted.AssistedInject
 import io.element.android.libraries.architecture.AsyncAction
 import io.element.android.libraries.architecture.Presenter
-import io.element.android.libraries.architecture.runCatchingUpdatingState
+import io.element.android.libraries.architecture.runUpdatingStateNoSuccess
 import io.element.android.libraries.matrix.api.MatrixClient
 import io.element.android.libraries.matrix.api.notificationsettings.NotificationSettingsService
 import io.element.android.libraries.matrix.api.room.RoomNotificationMode
@@ -73,7 +73,7 @@ class EditDefaultNotificationSettingPresenter @AssistedInject constructor(
         val localCoroutineScope = rememberCoroutineScope()
         LaunchedEffect(Unit) {
             fetchSettings(mode)
-            observeNotificationSettings(mode)
+            observeNotificationSettings(mode, changeNotificationSettingAction)
             observeRoomSummaries(roomsWithUserDefinedMode)
             displayMentionsOnlyDisclaimer = !notificationSettingsService.canHomeServerPushEncryptedEventsToDevice().getOrDefault(true)
         }
@@ -102,11 +102,15 @@ class EditDefaultNotificationSettingPresenter @AssistedInject constructor(
     }
 
     @OptIn(FlowPreview::class)
-    private fun CoroutineScope.observeNotificationSettings(mode: MutableState<RoomNotificationMode?>) {
+    private fun CoroutineScope.observeNotificationSettings(
+        mode: MutableState<RoomNotificationMode?>,
+        changeNotificationSettingAction: MutableState<AsyncAction<Unit>>,
+    ) {
         notificationSettingsService.notificationSettingsChangeFlow
             .debounce(0.5.seconds)
             .onEach {
                 fetchSettings(mode)
+                changeNotificationSettingAction.value = AsyncAction.Uninitialized
             }
             .launchIn(this)
     }
@@ -139,10 +143,12 @@ class EditDefaultNotificationSettingPresenter @AssistedInject constructor(
     }
 
     private fun CoroutineScope.setDefaultNotificationMode(mode: RoomNotificationMode, action: MutableState<AsyncAction<Unit>>) = launch {
-        suspend {
+        action.runUpdatingStateNoSuccess {
             // On modern clients, we don't have different settings for encrypted and non-encrypted rooms (Legacy clients did).
-            notificationSettingsService.setDefaultRoomNotificationMode(isEncrypted = true, mode = mode, isOneToOne = isOneToOne).getOrThrow()
+            notificationSettingsService.setDefaultRoomNotificationMode(isEncrypted = true, mode = mode, isOneToOne = isOneToOne)
-            notificationSettingsService.setDefaultRoomNotificationMode(isEncrypted = false, mode = mode, isOneToOne = isOneToOne).getOrThrow()
+                .map {
-        }.runCatchingUpdatingState(action)
+                    notificationSettingsService.setDefaultRoomNotificationMode(isEncrypted = false, mode = mode, isOneToOne = isOneToOne)
+                }
+        }
     }
 }

features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/notificationsettings/RoomNotificationSettingsEvents.kt

@@ -19,7 +19,7 @@ package io.element.android.features.roomdetails.impl.notificationsettings
 import io.element.android.libraries.matrix.api.room.RoomNotificationMode
 
 sealed interface RoomNotificationSettingsEvents {
-    data class RoomNotificationModeChanged(val mode: RoomNotificationMode) : RoomNotificationSettingsEvents
+    data class ChangeRoomNotificationMode(val mode: RoomNotificationMode) : RoomNotificationSettingsEvents
     data class SetNotificationMode(val isDefault: Boolean) : RoomNotificationSettingsEvents
     data object DeleteCustomNotification : RoomNotificationSettingsEvents
     data object ClearSetNotificationError : RoomNotificationSettingsEvents

features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/notificationsettings/RoomNotificationSettingsPresenter.kt

@@ -32,6 +32,7 @@ import io.element.android.libraries.architecture.AsyncAction
 import io.element.android.libraries.architecture.AsyncData
 import io.element.android.libraries.architecture.Presenter
 import io.element.android.libraries.architecture.runCatchingUpdatingState
+import io.element.android.libraries.core.coroutine.suspendWithMinimumDuration
 import io.element.android.libraries.matrix.api.notificationsettings.NotificationSettingsService
 import io.element.android.libraries.matrix.api.room.MatrixRoom
 import io.element.android.libraries.matrix.api.room.RoomNotificationMode
@@ -95,7 +96,7 @@ class RoomNotificationSettingsPresenter @AssistedInject constructor(
 
         fun handleEvents(event: RoomNotificationSettingsEvents) {
             when (event) {
-                is RoomNotificationSettingsEvents.RoomNotificationModeChanged -> {
+                is RoomNotificationSettingsEvents.ChangeRoomNotificationMode -> {
                     localCoroutineScope.setRoomNotificationMode(event.mode, pendingRoomNotificationMode, pendingSetDefault, setNotificationSettingAction)
                 }
                 is RoomNotificationSettingsEvents.SetNotificationMode -> {
@@ -171,7 +172,7 @@ class RoomNotificationSettingsPresenter @AssistedInject constructor(
         pendingDefaultState: MutableState<Boolean?>,
         action: MutableState<AsyncAction<Unit>>
     ) = launch {
-        suspend {
+        suspendWithMinimumDuration {
             pendingModeState.value = mode
             pendingDefaultState.value = false
             val result = notificationSettingsService.setRoomNotificationMode(room.roomId, mode)
@@ -187,7 +188,7 @@ class RoomNotificationSettingsPresenter @AssistedInject constructor(
         action: MutableState<AsyncAction<Unit>>,
         pendingDefaultState: MutableState<Boolean?>
     ) = launch {
-        suspend {
+        suspendWithMinimumDuration {
             pendingDefaultState.value = true
             val result = notificationSettingsService.restoreDefaultRoomNotificationMode(room.roomId)
             if (result.isFailure) {

features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/notificationsettings/RoomNotificationSettingsView.kt

@@ -149,7 +149,7 @@ private fun RoomSpecificNotificationSettingsView(
                         enabled = !state.displayIsDefault.orTrue(),
                         displayMentionsOnlyDisclaimer = state.displayMentionsOnlyDisclaimer,
                         onSelectOption = {
-                            state.eventSink(RoomNotificationSettingsEvents.RoomNotificationModeChanged(it.mode))
+                            state.eventSink(RoomNotificationSettingsEvents.ChangeRoomNotificationMode(it.mode))
                         },
                     )
                 }

features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/notificationsettings/UserDefinedRoomNotificationSettingsView.kt

@@ -68,7 +68,7 @@ fun UserDefinedRoomNotificationSettingsView(
                     enabled = !state.displayIsDefault.orTrue(),
                     displayMentionsOnlyDisclaimer = state.displayMentionsOnlyDisclaimer,
                     onSelectOption = {
-                        state.eventSink(RoomNotificationSettingsEvents.RoomNotificationModeChanged(it.mode))
+                        state.eventSink(RoomNotificationSettingsEvents.ChangeRoomNotificationMode(it.mode))
                     },
                 )
             }

features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/notificationsettings/RoomNotificationSettingsPresenterTest.kt

@@ -55,7 +55,7 @@ class RoomNotificationSettingsPresenterTest {
         moleculeFlow(RecompositionMode.Immediate) {
             presenter.present()
         }.test {
-            awaitItem().eventSink(RoomNotificationSettingsEvents.RoomNotificationModeChanged(RoomNotificationMode.MENTIONS_AND_KEYWORDS_ONLY))
+            awaitItem().eventSink(RoomNotificationSettingsEvents.ChangeRoomNotificationMode(RoomNotificationMode.MENTIONS_AND_KEYWORDS_ONLY))
             val updatedState = consumeItemsUntilPredicate {
                 it.roomNotificationSettings.dataOrNull()?.mode == RoomNotificationMode.MENTIONS_AND_KEYWORDS_ONLY
             }.last()
@@ -129,7 +129,7 @@ class RoomNotificationSettingsPresenterTest {
             presenter.present()
         }.test {
             val initialState = awaitItem()
-            initialState.eventSink(RoomNotificationSettingsEvents.RoomNotificationModeChanged(RoomNotificationMode.MENTIONS_AND_KEYWORDS_ONLY))
+            initialState.eventSink(RoomNotificationSettingsEvents.ChangeRoomNotificationMode(RoomNotificationMode.MENTIONS_AND_KEYWORDS_ONLY))
             initialState.eventSink(RoomNotificationSettingsEvents.SetNotificationMode(true))
             val defaultState = consumeItemsUntilPredicate {
                 it.roomNotificationSettings.dataOrNull()?.mode == RoomNotificationMode.MENTIONS_AND_KEYWORDS_ONLY
@@ -148,7 +148,7 @@ class RoomNotificationSettingsPresenterTest {
             presenter.present()
         }.test {
             val initialState = awaitItem()
-            initialState.eventSink(RoomNotificationSettingsEvents.RoomNotificationModeChanged(RoomNotificationMode.MENTIONS_AND_KEYWORDS_ONLY))
+            initialState.eventSink(RoomNotificationSettingsEvents.ChangeRoomNotificationMode(RoomNotificationMode.MENTIONS_AND_KEYWORDS_ONLY))
             initialState.eventSink(RoomNotificationSettingsEvents.SetNotificationMode(true))
             val failedState = consumeItemsUntilPredicate {
                 it.restoreDefaultAction.isFailure()

libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/AsyncAction.kt

@@ -125,6 +125,24 @@ suspend inline fun <T> MutableState<AsyncAction<T>>.runUpdatingState(
     resultBlock = resultBlock,
 )
 
+/**
+ * Run the given block and update the state accordingly, using only Loading and Failure states.
+ * It's up to the caller to manage the Success state.
+ */
+@OptIn(ExperimentalContracts::class)
+inline fun <T> MutableState<AsyncAction<T>>.runUpdatingStateNoSuccess(
+    resultBlock: () -> Result<Unit>,
+): Result<Unit> {
+    contract {
+        callsInPlace(resultBlock, InvocationKind.EXACTLY_ONCE)
+    }
+    value = AsyncAction.Loading
+    return resultBlock()
+        .onFailure { failure ->
+            value = AsyncAction.Failure(failure)
+        }
+}
+
 /**
  * Calls the specified [Result]-returning function [resultBlock]
  * encapsulating its progress and return value into an [AsyncAction] while

libraries/core/src/main/kotlin/io/element/android/libraries/core/coroutine/Suspend.kt

@@ -0,0 +1,30 @@
+/*
+ * Copyright (c) 2024 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.element.android.libraries.core.coroutine
+
+import kotlinx.coroutines.delay
+import kotlin.system.measureTimeMillis
+
+fun suspendWithMinimumDuration(
+    minimumDurationMillis: Long = 500,
+    block: suspend () -> Unit
+) = suspend {
+    val duration = measureTimeMillis {
+        block()
+    }
+    delay(minimumDurationMillis - duration)
+}