diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsPresenter.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsPresenter.kt index a78b94c866..317c6e796e 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsPresenter.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsPresenter.kt @@ -83,19 +83,19 @@ class NotificationSettingsPresenter @Inject constructor( } } } - // List of Distributor names - val distributorNames = remember { - distributors.map { it.second.name }.toImmutableList() + // List of Distributors + val availableDistributors = remember { + distributors.map { it.second }.toImmutableList() } - var currentDistributorName by remember { mutableStateOf>(AsyncData.Uninitialized) } + var currentDistributor by remember { mutableStateOf>(AsyncData.Uninitialized) } var refreshPushProvider by remember { mutableIntStateOf(0) } LaunchedEffect(refreshPushProvider) { val p = pushService.getCurrentPushProvider() - val name = p?.getCurrentDistributor(matrixClient.sessionId)?.name - currentDistributorName = if (name != null) { - AsyncData.Success(name) + val distributor = p?.getCurrentDistributor(matrixClient.sessionId) + currentDistributor = if (distributor != null) { + AsyncData.Success(distributor) } else { AsyncData.Failure(Exception("Failed to get current push provider")) } @@ -108,24 +108,23 @@ class NotificationSettingsPresenter @Inject constructor( ) = 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 + val (pushProvider, distributor) = data + // No op if the distributor is the same. + if (distributor == currentDistributor.dataOrNull()) return@launch + currentDistributor = AsyncData.Loading(currentDistributor.dataOrNull()) + pushService.registerWith( + matrixClient = matrixClient, + pushProvider = pushProvider, + distributor = distributor + ) + .fold( + { + refreshPushProvider++ + }, + { + currentDistributor = AsyncData.Failure(it) + } ) - .fold( - { - refreshPushProvider++ - }, - { - currentDistributorName = AsyncData.Failure(it) - } - ) - } } fun handleEvents(event: NotificationSettingsEvents) { @@ -162,8 +161,8 @@ class NotificationSettingsPresenter @Inject constructor( appNotificationsEnabled = appNotificationsEnabled.value ), changeNotificationSettingAction = changeNotificationSettingAction.value, - currentPushDistributor = currentDistributorName, - availablePushDistributors = distributorNames, + currentPushDistributor = currentDistributor, + availablePushDistributors = availableDistributors, showChangePushProviderDialog = showChangePushProviderDialog, fullScreenIntentPermissionsState = key(refreshFullScreenIntentSettings) { fullScreenIntentPermissionsPresenter.present() }, eventSink = ::handleEvents diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsState.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsState.kt index 5f3b00a174..a7a1bf9192 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsState.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsState.kt @@ -12,6 +12,7 @@ import io.element.android.libraries.architecture.AsyncAction import io.element.android.libraries.architecture.AsyncData import io.element.android.libraries.fullscreenintent.api.FullScreenIntentPermissionsState import io.element.android.libraries.matrix.api.room.RoomNotificationMode +import io.element.android.libraries.pushproviders.api.Distributor import kotlinx.collections.immutable.ImmutableList @Immutable @@ -19,8 +20,8 @@ data class NotificationSettingsState( val matrixSettings: MatrixSettings, val appSettings: AppSettings, val changeNotificationSettingAction: AsyncAction, - val currentPushDistributor: AsyncData, - val availablePushDistributors: ImmutableList, + val currentPushDistributor: AsyncData, + val availablePushDistributors: ImmutableList, val showChangePushProviderDialog: Boolean, val fullScreenIntentPermissionsState: FullScreenIntentPermissionsState, val eventSink: (NotificationSettingsEvents) -> Unit, diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsStateProvider.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsStateProvider.kt index 01765d2399..c0193bd6a2 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsStateProvider.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsStateProvider.kt @@ -13,6 +13,7 @@ import io.element.android.libraries.architecture.AsyncData import io.element.android.libraries.fullscreenintent.api.FullScreenIntentPermissionsState import io.element.android.libraries.fullscreenintent.api.aFullScreenIntentPermissionsState import io.element.android.libraries.matrix.api.room.RoomNotificationMode +import io.element.android.libraries.pushproviders.api.Distributor import kotlinx.collections.immutable.persistentListOf import kotlinx.collections.immutable.toImmutableList @@ -24,11 +25,19 @@ open class NotificationSettingsStateProvider : PreviewParameterProvider = AsyncData.Success("Firebase"), - availablePushDistributors: List = listOf("Firebase", "ntfy"), + currentPushDistributor: AsyncData = AsyncData.Success(aDistributor("Firebase")), + availablePushDistributors: List = listOf( + aDistributor("Firebase"), + aDistributor("ntfy"), + ), showChangePushProviderDialog: Boolean = false, fullScreenIntentPermissionsState: FullScreenIntentPermissionsState = aFullScreenIntentPermissionsState(), eventSink: (NotificationSettingsEvents) -> Unit = {}, @@ -88,3 +100,11 @@ fun aInvalidNotificationSettingsState( fullScreenIntentPermissionsState = aFullScreenIntentPermissionsState(), eventSink = eventSink, ) + +fun aDistributor( + name: String = "Name", + value: String = "$name Value", +) = Distributor( + value = value, + name = name, +) diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsView.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsView.kt index d74ed8581c..a1bdf7921c 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsView.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsView.kt @@ -206,7 +206,7 @@ private fun NotificationSettingsContentView( stringResource(id = CommonStrings.common_error) ) is AsyncData.Success -> ListItemContent.Text( - state.currentPushDistributor.dataOrNull() ?: "" + state.currentPushDistributor.dataOrNull()?.name ?: "" ) }, onClick = { @@ -219,8 +219,14 @@ private fun NotificationSettingsContentView( if (state.showChangePushProviderDialog) { SingleSelectionDialog( title = stringResource(id = R.string.screen_advanced_settings_choose_distributor_dialog_title_android), - options = state.availablePushDistributors.map { - ListOption(title = it) + options = state.availablePushDistributors.map { distributor -> + // If there are several distributors with the same name, use the full name + val title = if (state.availablePushDistributors.count { it.name == distributor.name } > 1) { + distributor.fullName + } else { + distributor.name + } + ListOption(title = title) }.toImmutableList(), initialSelection = state.availablePushDistributors.indexOf(state.currentPushDistributor.dataOrNull()), onSelectOption = { index -> diff --git a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsPresenterTest.kt b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsPresenterTest.kt index 57179955a8..fcefd5c43d 100644 --- a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsPresenterTest.kt +++ b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsPresenterTest.kt @@ -240,8 +240,11 @@ class NotificationSettingsPresenterTest { presenter.present() }.test { val initialState = awaitLastSequentialItem() - assertThat(initialState.currentPushDistributor).isEqualTo(AsyncData.Success("aDistributorName0")) - assertThat(initialState.availablePushDistributors).containsExactly("aDistributorName0", "aDistributorName1") + assertThat(initialState.currentPushDistributor).isEqualTo(AsyncData.Success(Distributor(value = "aDistributorValue0", name = "aDistributorName0"))) + assertThat(initialState.availablePushDistributors).containsExactly( + Distributor(value = "aDistributorValue0", name = "aDistributorName0"), + Distributor(value = "aDistributorValue1", name = "aDistributorName1"), + ) initialState.eventSink.invoke(NotificationSettingsEvents.ChangePushProvider) val withDialog = awaitItem() assertThat(withDialog.showChangePushProviderDialog).isTrue() @@ -257,11 +260,35 @@ class NotificationSettingsPresenterTest { assertThat(withNewProvider.currentPushDistributor).isInstanceOf(AsyncData.Loading::class.java) skipItems(1) val lastItem = awaitItem() - assertThat(lastItem.currentPushDistributor).isEqualTo(AsyncData.Success("aDistributorName1")) + assertThat(lastItem.currentPushDistributor).isEqualTo(AsyncData.Success(Distributor(value = "aDistributorValue1", name = "aDistributorName1"))) cancelAndIgnoreRemainingEvents() } } + @Test + fun `present - change push provider to the same value is no op`() = runTest { + val presenter = createNotificationSettingsPresenter( + pushService = createFakePushService(), + ) + moleculeFlow(RecompositionMode.Immediate) { + presenter.present() + }.test { + val initialState = awaitLastSequentialItem() + assertThat(initialState.currentPushDistributor).isEqualTo(AsyncData.Success(Distributor(value = "aDistributorValue0", name = "aDistributorName0"))) + assertThat(initialState.availablePushDistributors).containsExactly( + Distributor(value = "aDistributorValue0", name = "aDistributorName0"), + Distributor(value = "aDistributorValue1", name = "aDistributorName1"), + ) + initialState.eventSink.invoke(NotificationSettingsEvents.ChangePushProvider) + assertThat(awaitItem().showChangePushProviderDialog).isTrue() + // Choose the same value (index 0) + initialState.eventSink(NotificationSettingsEvents.SetPushProvider(0)) + val withNewProvider = awaitItem() + assertThat(withNewProvider.showChangePushProviderDialog).isFalse() + expectNoEvents() + } + } + @Test fun `present - RefreshSystemNotificationsEnabled also refreshes fullScreenIntentState`() = runTest { var lambdaResult = aFullScreenIntentPermissionsState(permissionGranted = false) diff --git a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsViewTest.kt b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsViewTest.kt index 01cc817941..2aa485a192 100644 --- a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsViewTest.kt +++ b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsViewTest.kt @@ -267,7 +267,7 @@ class NotificationSettingsViewTest { state = aValidNotificationSettingsState( eventSink = eventsRecorder, showChangePushProviderDialog = true, - availablePushDistributors = listOf("P1", "P2") + availablePushDistributors = listOf(aDistributor("P1"), aDistributor("P2")) ), ) rule.onNodeWithText("P2").performClick() diff --git a/libraries/pushproviders/api/src/main/kotlin/io/element/android/libraries/pushproviders/api/Distributor.kt b/libraries/pushproviders/api/src/main/kotlin/io/element/android/libraries/pushproviders/api/Distributor.kt index c99e5072b7..d623740444 100644 --- a/libraries/pushproviders/api/src/main/kotlin/io/element/android/libraries/pushproviders/api/Distributor.kt +++ b/libraries/pushproviders/api/src/main/kotlin/io/element/android/libraries/pushproviders/api/Distributor.kt @@ -18,4 +18,6 @@ package io.element.android.libraries.pushproviders.api data class Distributor( val value: String, val name: String, -) +) { + val fullName = "$name ($value)" +}