From e64051f7bd243feec351257e888e7fe1d165721b Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 7 May 2024 09:47:46 +0200 Subject: [PATCH] Improve code and display error. --- .../impl/advanced/AdvancedSettingsEvents.kt | 2 +- .../advanced/AdvancedSettingsPresenter.kt | 81 +++++++++++++------ .../impl/advanced/AdvancedSettingsState.kt | 3 +- .../advanced/AdvancedSettingsStateProvider.kt | 5 +- .../impl/advanced/AdvancedSettingsView.kt | 39 ++++++--- .../libraries/architecture/AsyncAction.kt | 2 + 6 files changed, 94 insertions(+), 38 deletions(-) diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/advanced/AdvancedSettingsEvents.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/advanced/AdvancedSettingsEvents.kt index 31c80acb7d..4ed4277599 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/advanced/AdvancedSettingsEvents.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/advanced/AdvancedSettingsEvents.kt @@ -26,5 +26,5 @@ sealed interface AdvancedSettingsEvents { data class SetTheme(val theme: Theme) : AdvancedSettingsEvents data object ChangePushProvider : AdvancedSettingsEvents data object CancelChangePushProvider : AdvancedSettingsEvents - data class SetPushProvider(val distributorName: String) : AdvancedSettingsEvents + data class SetPushProvider(val index: Int) : AdvancedSettingsEvents } diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/advanced/AdvancedSettingsPresenter.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/advanced/AdvancedSettingsPresenter.kt index 3dd9f54e6f..77654d6c4c 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/advanced/AdvancedSettingsPresenter.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/advanced/AdvancedSettingsPresenter.kt @@ -29,10 +29,14 @@ 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 @@ -57,20 +61,60 @@ class AdvancedSettingsPresenter @Inject constructor( .collectAsState(initial = Theme.System) var showChangeThemeDialog by remember { mutableStateOf(false) } - var currentPushProvider by remember { mutableStateOf(null) } - var distributors by remember { mutableStateOf>(emptyList()) } + // List of PushProvider -> Distributor + var distributors by remember { mutableStateOf>>(emptyList()) } + var distributorNames by remember { mutableStateOf>(emptyList()) } + LaunchedEffect(Unit) { + distributors = pushService.getAvailablePushProviders() + .flatMap { pushProvider -> + pushProvider.getDistributors().map { distributor -> + pushProvider to distributor + } + } + distributorNames = distributors.map { it.second.name } + } + + var currentDistributorName by remember { mutableStateOf>(AsyncAction.Uninitialized) } var refreshPushProvider by remember { mutableIntStateOf(0) } LaunchedEffect(refreshPushProvider) { val p = pushService.getCurrentPushProvider() - currentPushProvider = p?.getCurrentDistributor(matrixClient)?.name - distributors = pushService.getAvailablePushProviders() - .flatMap { pushProvider -> - pushProvider.getDistributors().map { it.name } - } + 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? + ) = 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 { @@ -87,23 +131,7 @@ class AdvancedSettingsPresenter @Inject constructor( } AdvancedSettingsEvents.ChangePushProvider -> showChangePushProviderDialog = true AdvancedSettingsEvents.CancelChangePushProvider -> showChangePushProviderDialog = false - is AdvancedSettingsEvents.SetPushProvider -> { - localCoroutineScope.launch { - // Retrieve the push provider - // TODO rework this - val pushProvider = pushService.getAvailablePushProviders().firstOrNull { pushProvider -> - pushProvider.getDistributors().any { it.name == event.distributorName } - } ?: return@launch - val distributor = pushProvider.getDistributors().firstOrNull { it.name == event.distributorName } ?: return@launch - pushService.registerWith( - matrixClient, - pushProvider = pushProvider, - distributor = distributor - ) - showChangePushProviderDialog = false - refreshPushProvider++ - } - } + is AdvancedSettingsEvents.SetPushProvider -> localCoroutineScope.changePushProvider(distributors.getOrNull(event.index)) } } @@ -112,10 +140,11 @@ class AdvancedSettingsPresenter @Inject constructor( isSharePresenceEnabled = isSharePresenceEnabled, theme = theme, showChangeThemeDialog = showChangeThemeDialog, - pushDistributor = currentPushProvider ?: "", - pushDistributors = distributors.toImmutableList(), + pushDistributor = currentDistributorName, + pushDistributors = distributorNames.toImmutableList(), showChangePushProviderDialog = showChangePushProviderDialog, eventSink = { handleEvents(it) } ) } } + diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/advanced/AdvancedSettingsState.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/advanced/AdvancedSettingsState.kt index 8a0184bf8b..36e5c5744d 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/advanced/AdvancedSettingsState.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/advanced/AdvancedSettingsState.kt @@ -17,6 +17,7 @@ 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( @@ -24,7 +25,7 @@ data class AdvancedSettingsState( val isSharePresenceEnabled: Boolean, val theme: Theme, val showChangeThemeDialog: Boolean, - val pushDistributor: String, + val pushDistributor: AsyncAction, val pushDistributors: ImmutableList, val showChangePushProviderDialog: Boolean, val eventSink: (AdvancedSettingsEvents) -> Unit diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/advanced/AdvancedSettingsStateProvider.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/advanced/AdvancedSettingsStateProvider.kt index ff15917c91..092b940bd4 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/advanced/AdvancedSettingsStateProvider.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/advanced/AdvancedSettingsStateProvider.kt @@ -18,6 +18,7 @@ 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 { @@ -28,6 +29,8 @@ open class AdvancedSettingsStateProvider : PreviewParameterProvider = AsyncAction.Success("Firebase"), pushDistributors: List = listOf("Firebase", "ntfy"), showChangePushProviderDialog: Boolean = false, ) = AdvancedSettingsState( diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/advanced/AdvancedSettingsView.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/advanced/AdvancedSettingsView.kt index fc0c2d8f69..12f2ef57d6 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/advanced/AdvancedSettingsView.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/advanced/AdvancedSettingsView.kt @@ -16,19 +16,24 @@ 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,11 +91,28 @@ fun AdvancedSettingsView( // TODO i18n Text(text = "Push provider") }, - trailingContent = ListItemContent.Text( - state.pushDistributor - ), + trailingContent = when (state.pushDistributor) { + 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.pushDistributor.dataOrNull() ?: "" + ) + }, onClick = { - state.eventSink(AdvancedSettingsEvents.ChangePushProvider) + if (state.pushDistributor.isReady()) { + state.eventSink(AdvancedSettingsEvents.ChangePushProvider) + } } ) } @@ -112,15 +134,14 @@ fun AdvancedSettingsView( if (state.showChangePushProviderDialog) { SingleSelectionDialog( + title = "Select Push provider", options = state.pushDistributors.map { ListOption(title = it) }.toImmutableList(), - initialSelection = state.pushDistributors.indexOf(state.pushDistributor), - onOptionSelected = { + initialSelection = state.pushDistributors.indexOf(state.pushDistributor.dataOrNull()), + onOptionSelected = { index -> state.eventSink( - AdvancedSettingsEvents.SetPushProvider( - state.pushDistributors[it] - ) + AdvancedSettingsEvents.SetPushProvider(index) ) }, onDismissRequest = { state.eventSink(AdvancedSettingsEvents.CancelChangePushProvider) }, diff --git a/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/AsyncAction.kt b/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/AsyncAction.kt index e69b1b9eaf..9545f1ab68 100644 --- a/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/AsyncAction.kt +++ b/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/AsyncAction.kt @@ -86,6 +86,8 @@ sealed interface AsyncAction { fun isFailure(): Boolean = this is Failure fun isSuccess(): Boolean = this is Success + + fun isReady() = isSuccess() || isFailure() } suspend inline fun MutableState>.runCatchingUpdatingState(