Merge pull request #2928 from element-hq/feature/bma/movePushSetting

Move push provider setting
element-hq · May 28, 2024 · 17678ad · 17678ad
2 parents f74032d + 35a02a2
commit 17678ad
Show file tree

Hide file tree

Showing 52 changed files with 376 additions and 317 deletions.
diff --git a/changelog.d/2912.misc b/changelog.d/2912.misc
@@ -0,0 +1 @@
+Move push provider setting to the "Notifications" screen and display it only when several push provider are available.
diff --git a/...in/kotlin/io/element/android/features/preferences/impl/advanced/AdvancedSettingsEvents.kt b/...in/kotlin/io/element/android/features/preferences/impl/advanced/AdvancedSettingsEvents.kt
@@ -24,7 +24,4 @@ sealed interface AdvancedSettingsEvents {
     data object ChangeTheme : AdvancedSettingsEvents
     data object CancelChangeTheme : AdvancedSettingsEvents
     data class SetTheme(val theme: Theme) : AdvancedSettingsEvents
-    data object ChangePushProvider : AdvancedSettingsEvents
-    data object CancelChangePushProvider : AdvancedSettingsEvents
-    data class SetPushProvider(val index: Int) : AdvancedSettingsEvents
 }
diff --git a/...kotlin/io/element/android/features/preferences/impl/advanced/AdvancedSettingsPresenter.kt b/...kotlin/io/element/android/features/preferences/impl/advanced/AdvancedSettingsPresenter.kt
@@ -17,10 +17,8 @@
 package io.element.android.features.preferences.impl.advanced
 
 import androidx.compose.runtime.Composable
-import androidx.compose.runtime.LaunchedEffect
 import androidx.compose.runtime.collectAsState
 import androidx.compose.runtime.getValue
-import androidx.compose.runtime.mutableIntStateOf
 import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.remember
 import androidx.compose.runtime.rememberCoroutineScope
@@ -29,22 +27,13 @@ import io.element.android.compound.theme.Theme
 import io.element.android.compound.theme.mapToTheme
 import io.element.android.features.preferences.api.store.AppPreferencesStore
 import io.element.android.features.preferences.api.store.SessionPreferencesStore
-import io.element.android.libraries.architecture.AsyncAction
 import io.element.android.libraries.architecture.Presenter
-import io.element.android.libraries.matrix.api.MatrixClient
-import io.element.android.libraries.push.api.PushService
-import io.element.android.libraries.pushproviders.api.Distributor
-import io.element.android.libraries.pushproviders.api.PushProvider
-import kotlinx.collections.immutable.toImmutableList
-import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.launch
 import javax.inject.Inject
 
 class AdvancedSettingsPresenter @Inject constructor(
     private val appPreferencesStore: AppPreferencesStore,
     private val sessionPreferencesStore: SessionPreferencesStore,
-    private val matrixClient: MatrixClient,
-    private val pushService: PushService,
 ) : Presenter<AdvancedSettingsState> {
     @Composable
     override fun present(): AdvancedSettingsState {
@@ -61,61 +50,6 @@ class AdvancedSettingsPresenter @Inject constructor(
             .collectAsState(initial = Theme.System)
         var showChangeThemeDialog by remember { mutableStateOf(false) }
 
-        // List of PushProvider -> Distributor
-        val distributors = remember {
-            pushService.getAvailablePushProviders()
-                .flatMap { pushProvider ->
-                    pushProvider.getDistributors().map { distributor ->
-                        pushProvider to distributor
-                    }
-                }
-        }
-        // List of Distributor names
-        val distributorNames = remember {
-            distributors.map { it.second.name }
-        }
-
-        var currentDistributorName by remember { mutableStateOf<AsyncAction<String>>(AsyncAction.Uninitialized) }
-        var refreshPushProvider by remember { mutableIntStateOf(0) }
-
-        LaunchedEffect(refreshPushProvider) {
-            val p = pushService.getCurrentPushProvider()
-            val name = p?.getCurrentDistributor(matrixClient)?.name
-            currentDistributorName = if (name != null) {
-                AsyncAction.Success(name)
-            } else {
-                AsyncAction.Failure(Exception("Failed to get current push provider"))
-            }
-        }
-
-        var showChangePushProviderDialog by remember { mutableStateOf(false) }
-
-        fun CoroutineScope.changePushProvider(
-            data: Pair<PushProvider, Distributor>?
-        ) = launch {
-            showChangePushProviderDialog = false
-            data ?: return@launch
-            // No op if the value is the same.
-            if (data.second.name == currentDistributorName.dataOrNull()) return@launch
-            currentDistributorName = AsyncAction.Loading
-            data.let { (pushProvider, distributor) ->
-                pushService.registerWith(
-                    matrixClient = matrixClient,
-                    pushProvider = pushProvider,
-                    distributor = distributor
-                )
-                    .fold(
-                        {
-                            currentDistributorName = AsyncAction.Success(distributor.name)
-                            refreshPushProvider++
-                        },
-                        {
-                            currentDistributorName = AsyncAction.Failure(it)
-                        }
-                    )
-            }
-        }
-
         fun handleEvents(event: AdvancedSettingsEvents) {
             when (event) {
                 is AdvancedSettingsEvents.SetDeveloperModeEnabled -> localCoroutineScope.launch {
@@ -130,9 +64,6 @@ class AdvancedSettingsPresenter @Inject constructor(
                     appPreferencesStore.setTheme(event.theme.name)
                     showChangeThemeDialog = false
                 }
-                AdvancedSettingsEvents.ChangePushProvider -> showChangePushProviderDialog = true
-                AdvancedSettingsEvents.CancelChangePushProvider -> showChangePushProviderDialog = false
-                is AdvancedSettingsEvents.SetPushProvider -> localCoroutineScope.changePushProvider(distributors.getOrNull(event.index))
             }
         }
 
@@ -141,9 +72,6 @@ class AdvancedSettingsPresenter @Inject constructor(
             isSharePresenceEnabled = isSharePresenceEnabled,
             theme = theme,
             showChangeThemeDialog = showChangeThemeDialog,
-            currentPushDistributor = currentDistributorName,
-            availablePushDistributors = distributorNames.toImmutableList(),
-            showChangePushProviderDialog = showChangePushProviderDialog,
             eventSink = { handleEvents(it) }
         )
     }

diff --git a/...ain/kotlin/io/element/android/features/preferences/impl/advanced/AdvancedSettingsState.kt b/...ain/kotlin/io/element/android/features/preferences/impl/advanced/AdvancedSettingsState.kt
@@ -17,16 +17,11 @@
 package io.element.android.features.preferences.impl.advanced
 
 import io.element.android.compound.theme.Theme
-import io.element.android.libraries.architecture.AsyncAction
-import kotlinx.collections.immutable.ImmutableList
 
 data class AdvancedSettingsState(
     val isDeveloperModeEnabled: Boolean,
     val isSharePresenceEnabled: Boolean,
     val theme: Theme,
     val showChangeThemeDialog: Boolean,
-    val currentPushDistributor: AsyncAction<String>,
-    val availablePushDistributors: ImmutableList<String>,
-    val showChangePushProviderDialog: Boolean,
     val eventSink: (AdvancedSettingsEvents) -> Unit
 )
diff --git a/...in/io/element/android/features/preferences/impl/advanced/AdvancedSettingsStateProvider.kt b/...in/io/element/android/features/preferences/impl/advanced/AdvancedSettingsStateProvider.kt
@@ -18,8 +18,6 @@ package io.element.android.features.preferences.impl.advanced
 
 import androidx.compose.ui.tooling.preview.PreviewParameterProvider
 import io.element.android.compound.theme.Theme
-import io.element.android.libraries.architecture.AsyncAction
-import kotlinx.collections.immutable.toImmutableList
 
 open class AdvancedSettingsStateProvider : PreviewParameterProvider<AdvancedSettingsState> {
     override val values: Sequence<AdvancedSettingsState>
@@ -28,27 +26,18 @@ open class AdvancedSettingsStateProvider : PreviewParameterProvider<AdvancedSett
             aAdvancedSettingsState(isDeveloperModeEnabled = true),
             aAdvancedSettingsState(showChangeThemeDialog = true),
             aAdvancedSettingsState(isSendPublicReadReceiptsEnabled = true),
-            aAdvancedSettingsState(showChangePushProviderDialog = true),
-            aAdvancedSettingsState(currentPushDistributor = AsyncAction.Loading),
-            aAdvancedSettingsState(currentPushDistributor = AsyncAction.Failure(Exception("Failed to change distributor"))),
         )
 }
 
 fun aAdvancedSettingsState(
     isDeveloperModeEnabled: Boolean = false,
     isSendPublicReadReceiptsEnabled: Boolean = false,
     showChangeThemeDialog: Boolean = false,
-    currentPushDistributor: AsyncAction<String> = AsyncAction.Success("Firebase"),
-    availablePushDistributors: List<String> = listOf("Firebase", "ntfy"),
-    showChangePushProviderDialog: Boolean = false,
     eventSink: (AdvancedSettingsEvents) -> Unit = {},
 ) = AdvancedSettingsState(
     isDeveloperModeEnabled = isDeveloperModeEnabled,
     isSharePresenceEnabled = isSendPublicReadReceiptsEnabled,
     theme = Theme.System,
     showChangeThemeDialog = showChangeThemeDialog,
-    currentPushDistributor = currentPushDistributor,
-    availablePushDistributors = availablePushDistributors.toImmutableList(),
-    showChangePushProviderDialog = showChangePushProviderDialog,
     eventSink = eventSink
 )
diff --git a/...main/kotlin/io/element/android/features/preferences/impl/advanced/AdvancedSettingsView.kt b/...main/kotlin/io/element/android/features/preferences/impl/advanced/AdvancedSettingsView.kt
@@ -16,24 +16,19 @@
 
 package io.element.android.features.preferences.impl.advanced
 
-import androidx.compose.foundation.layout.size
-import androidx.compose.foundation.progressSemantics
 import androidx.compose.runtime.Composable
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.res.stringResource
 import androidx.compose.ui.tooling.preview.PreviewParameter
-import androidx.compose.ui.unit.dp
 import io.element.android.compound.theme.Theme
 import io.element.android.compound.theme.themes
 import io.element.android.features.preferences.impl.R
-import io.element.android.libraries.architecture.AsyncAction
 import io.element.android.libraries.designsystem.components.dialogs.ListOption
 import io.element.android.libraries.designsystem.components.dialogs.SingleSelectionDialog
 import io.element.android.libraries.designsystem.components.list.ListItemContent
 import io.element.android.libraries.designsystem.components.preferences.PreferencePage
 import io.element.android.libraries.designsystem.preview.ElementPreview
 import io.element.android.libraries.designsystem.preview.PreviewsDayNight
-import io.element.android.libraries.designsystem.theme.components.CircularProgressIndicator
 import io.element.android.libraries.designsystem.theme.components.ListItem
 import io.element.android.libraries.designsystem.theme.components.Text
 import io.element.android.libraries.ui.strings.CommonStrings
@@ -86,34 +81,6 @@ fun AdvancedSettingsView(
             ),
             onClick = { state.eventSink(AdvancedSettingsEvents.SetSharePresenceEnabled(!state.isSharePresenceEnabled)) }
         )
-        ListItem(
-            headlineContent = {
-                Text(text = stringResource(id = R.string.screen_advanced_settings_push_provider_android))
-            },
-            trailingContent = when (state.currentPushDistributor) {
-                AsyncAction.Uninitialized,
-                AsyncAction.Confirming,
-                AsyncAction.Loading -> ListItemContent.Custom {
-                    CircularProgressIndicator(
-                        modifier = Modifier
-                            .progressSemantics()
-                            .size(20.dp),
-                        strokeWidth = 2.dp
-                    )
-                }
-                is AsyncAction.Failure -> ListItemContent.Text(
-                    stringResource(id = CommonStrings.common_error)
-                )
-                is AsyncAction.Success -> ListItemContent.Text(
-                    state.currentPushDistributor.dataOrNull() ?: ""
-                )
-            },
-            onClick = {
-                if (state.currentPushDistributor.isReady()) {
-                    state.eventSink(AdvancedSettingsEvents.ChangePushProvider)
-                }
-            }
-        )
     }
 
     if (state.showChangeThemeDialog) {
@@ -130,22 +97,6 @@ fun AdvancedSettingsView(
             onDismissRequest = { state.eventSink(AdvancedSettingsEvents.CancelChangeTheme) },
         )
     }
-
-    if (state.showChangePushProviderDialog) {
-        SingleSelectionDialog(
-            title = stringResource(id = R.string.screen_advanced_settings_choose_distributor_dialog_title_android),
-            options = state.availablePushDistributors.map {
-                ListOption(title = it)
-            }.toImmutableList(),
-            initialSelection = state.availablePushDistributors.indexOf(state.currentPushDistributor.dataOrNull()),
-            onOptionSelected = { index ->
-                state.eventSink(
-                    AdvancedSettingsEvents.SetPushProvider(index)
-                )
-            },
-            onDismissRequest = { state.eventSink(AdvancedSettingsEvents.CancelChangePushProvider) },
-        )
-    }
 }
 
 @Composable

diff --git a/.../io/element/android/features/preferences/impl/notifications/NotificationSettingsEvents.kt b/.../io/element/android/features/preferences/impl/notifications/NotificationSettingsEvents.kt
@@ -25,4 +25,7 @@ sealed interface NotificationSettingsEvents {
     data object FixConfigurationMismatch : NotificationSettingsEvents
     data object ClearConfigurationMismatchError : NotificationSettingsEvents
     data object ClearNotificationChangeError : NotificationSettingsEvents
+    data object ChangePushProvider : NotificationSettingsEvents
+    data object CancelChangePushProvider : NotificationSettingsEvents
+    data class SetPushProvider(val index: Int) : NotificationSettingsEvents
 }
diff --git a/.../element/android/features/preferences/impl/notifications/NotificationSettingsPresenter.kt b/.../element/android/features/preferences/impl/notifications/NotificationSettingsPresenter.kt
@@ -20,17 +20,25 @@ import androidx.compose.runtime.Composable
 import androidx.compose.runtime.LaunchedEffect
 import androidx.compose.runtime.MutableState
 import androidx.compose.runtime.collectAsState
+import androidx.compose.runtime.getValue
+import androidx.compose.runtime.mutableIntStateOf
 import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.remember
 import androidx.compose.runtime.rememberCoroutineScope
+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.matrix.api.MatrixClient
 import io.element.android.libraries.matrix.api.notificationsettings.NotificationSettingsService
 import io.element.android.libraries.matrix.api.room.RoomNotificationMode
+import io.element.android.libraries.push.api.PushService
+import io.element.android.libraries.pushproviders.api.Distributor
+import io.element.android.libraries.pushproviders.api.PushProvider
 import io.element.android.libraries.pushstore.api.UserPushStore
 import io.element.android.libraries.pushstore.api.UserPushStoreFactory
+import kotlinx.collections.immutable.toImmutableList
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.FlowPreview
 import kotlinx.coroutines.flow.debounce
@@ -44,7 +52,8 @@ class NotificationSettingsPresenter @Inject constructor(
     private val notificationSettingsService: NotificationSettingsService,
     private val userPushStoreFactory: UserPushStoreFactory,
     private val matrixClient: MatrixClient,
-    private val systemNotificationsEnabledProvider: SystemNotificationsEnabledProvider
+    private val pushService: PushService,
+    private val systemNotificationsEnabledProvider: SystemNotificationsEnabledProvider,
 ) : Presenter<NotificationSettingsState> {
     @Composable
     override fun present(): NotificationSettingsState {
@@ -68,6 +77,60 @@ class NotificationSettingsPresenter @Inject constructor(
             observeNotificationSettings(matrixSettings)
         }
 
+        // List of PushProvider -> Distributor
+        val distributors = remember {
+            pushService.getAvailablePushProviders()
+                .flatMap { pushProvider ->
+                    pushProvider.getDistributors().map { distributor ->
+                        pushProvider to distributor
+                    }
+                }
+        }
+        // List of Distributor names
+        val distributorNames = remember {
+            distributors.map { it.second.name }.toImmutableList()
+        }
+
+        var currentDistributorName by remember { mutableStateOf<AsyncData<String>>(AsyncData.Uninitialized) }
+        var refreshPushProvider by remember { mutableIntStateOf(0) }
+
+        LaunchedEffect(refreshPushProvider) {
+            val p = pushService.getCurrentPushProvider()
+            val name = p?.getCurrentDistributor(matrixClient)?.name
+            currentDistributorName = if (name != null) {
+                AsyncData.Success(name)
+            } else {
+                AsyncData.Failure(Exception("Failed to get current push provider"))
+            }
+        }
+
+        var showChangePushProviderDialog by remember { mutableStateOf(false) }
+
+        fun CoroutineScope.changePushProvider(
+            data: Pair<PushProvider, Distributor>?
+        ) = launch {
+            showChangePushProviderDialog = false
+            data ?: return@launch
+            // No op if the value is the same.
+            if (data.second.name == currentDistributorName.dataOrNull()) return@launch
+            currentDistributorName = AsyncData.Loading(currentDistributorName.dataOrNull())
+            data.let { (pushProvider, distributor) ->
+                pushService.registerWith(
+                    matrixClient = matrixClient,
+                    pushProvider = pushProvider,
+                    distributor = distributor
+                )
+                    .fold(
+                        {
+                            refreshPushProvider++
+                        },
+                        {
+                            currentDistributorName = AsyncData.Failure(it)
+                        }
+                    )
+            }
+        }
+
         fun handleEvents(event: NotificationSettingsEvents) {
             when (event) {
                 is NotificationSettingsEvents.SetAtRoomNotificationsEnabled -> {
@@ -88,6 +151,9 @@ class NotificationSettingsPresenter @Inject constructor(
                     systemNotificationsEnabled.value = systemNotificationsEnabledProvider.notificationsEnabled()
                 }
                 NotificationSettingsEvents.ClearNotificationChangeError -> changeNotificationSettingAction.value = AsyncAction.Uninitialized
+                NotificationSettingsEvents.ChangePushProvider -> showChangePushProviderDialog = true
+                NotificationSettingsEvents.CancelChangePushProvider -> showChangePushProviderDialog = false
+                is NotificationSettingsEvents.SetPushProvider -> localCoroutineScope.changePushProvider(distributors.getOrNull(event.index))
             }
         }
 
@@ -98,6 +164,9 @@ class NotificationSettingsPresenter @Inject constructor(
                 appNotificationsEnabled = appNotificationsEnabled.value
             ),
             changeNotificationSettingAction = changeNotificationSettingAction.value,
+            currentPushDistributor = currentDistributorName,
+            availablePushDistributors = distributorNames,
+            showChangePushProviderDialog = showChangePushProviderDialog,
             eventSink = ::handleEvents
         )
     }
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Move push provider setting to the "Notifications" screen and display it only when several push provider are available.