Merge pull request #4312 from element-hq/feature/bma/fixMultipleNtfy

Fix issues due to multiple ntfy applications with the same name.
This commit is contained in:
Benoit Marty 2025-02-26 17:21:51 +01:00 committed by GitHub
commit 1bbcedfa38
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
19 changed files with 121 additions and 60 deletions

View file

@ -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<String>>(AsyncData.Uninitialized) }
var currentDistributor by remember { mutableStateOf<AsyncData<Distributor>>(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

View file

@ -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<Unit>,
val currentPushDistributor: AsyncData<String>,
val availablePushDistributors: ImmutableList<String>,
val currentPushDistributor: AsyncData<Distributor>,
val availablePushDistributors: ImmutableList<Distributor>,
val showChangePushProviderDialog: Boolean,
val fullScreenIntentPermissionsState: FullScreenIntentPermissionsState,
val eventSink: (NotificationSettingsEvents) -> Unit,

View file

@ -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<Notifica
aValidNotificationSettingsState(changeNotificationSettingAction = AsyncAction.Loading),
aValidNotificationSettingsState(changeNotificationSettingAction = AsyncAction.Failure(Throwable("error"))),
aValidNotificationSettingsState(
availablePushDistributors = listOf("Firebase"),
availablePushDistributors = listOf(aDistributor("Firebase")),
changeNotificationSettingAction = AsyncAction.Failure(Throwable("error")),
),
aValidNotificationSettingsState(availablePushDistributors = listOf("Firebase")),
aValidNotificationSettingsState(availablePushDistributors = listOf(aDistributor("Firebase"))),
aValidNotificationSettingsState(showChangePushProviderDialog = true),
aValidNotificationSettingsState(
availablePushDistributors = listOf(
aDistributor("Firebase"),
aDistributor("ntfy", "app.id1"),
aDistributor("ntfy", "app.id2"),
),
showChangePushProviderDialog = true,
),
aValidNotificationSettingsState(currentPushDistributor = AsyncData.Loading()),
aValidNotificationSettingsState(currentPushDistributor = AsyncData.Failure(Exception("Failed to change distributor"))),
aInvalidNotificationSettingsState(),
@ -45,8 +54,11 @@ fun aValidNotificationSettingsState(
inviteForMeNotificationsEnabled: Boolean = true,
systemNotificationsEnabled: Boolean = true,
appNotificationEnabled: Boolean = true,
currentPushDistributor: AsyncData<String> = AsyncData.Success("Firebase"),
availablePushDistributors: List<String> = listOf("Firebase", "ntfy"),
currentPushDistributor: AsyncData<Distributor> = AsyncData.Success(aDistributor("Firebase")),
availablePushDistributors: List<Distributor> = 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,
)

View file

@ -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 ->

View file

@ -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)

View file

@ -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()

View file

@ -18,4 +18,6 @@ package io.element.android.libraries.pushproviders.api
data class Distributor(
val value: String,
val name: String,
)
) {
val fullName = "$name ($value)"
}

View file

@ -1,3 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:16d1a40f1236f2b0070c05851cfb5f1e73e420361d62aafb8ac5776e20b94db4
size 42127
oid sha256:f96978e1ea1f0ccf62d977e5d41e7036523304b6b42b27b82281756fb5546b39
size 45057

View file

@ -1,3 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:a56d884dacc6f73de25255dc4c0960e15968a6b9b89cfab858571d5f6c5a16e6
size 59133
oid sha256:16d1a40f1236f2b0070c05851cfb5f1e73e420361d62aafb8ac5776e20b94db4
size 42127

View file

@ -1,3 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:c7317fdf96ff21fe516286aca2720c89390eab57c15336a4fa82afd5473d29ec
size 15296
oid sha256:a56d884dacc6f73de25255dc4c0960e15968a6b9b89cfab858571d5f6c5a16e6
size 59133

View file

@ -0,0 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:c7317fdf96ff21fe516286aca2720c89390eab57c15336a4fa82afd5473d29ec
size 15296

View file

@ -1,3 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:afafd3155ea2199c3e0d566797337a1bb5c88ffdf8beac3274d91829ef91d44d
size 43807
oid sha256:796869e88b376df6e40b6ee51d471cbb9ba19f308a5353a818eb182b6d7fbe47
size 44214

View file

@ -1,3 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:f96978e1ea1f0ccf62d977e5d41e7036523304b6b42b27b82281756fb5546b39
size 45057
oid sha256:afafd3155ea2199c3e0d566797337a1bb5c88ffdf8beac3274d91829ef91d44d
size 43807

View file

@ -1,3 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:8dd44e2127936e2e01d99933a2f18e50ca760f614f6a3e1c3162e3387d4931ee
size 39759
oid sha256:4f22b7825bfa438f55514a3d09696de34450c16f9d4481c4892be9c8dcb45191
size 42486

View file

@ -1,3 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:399efdebf67b1c042dc105d6538ea4fb622314c883fd6d3899eebcedd3ff357e
size 57540
oid sha256:8dd44e2127936e2e01d99933a2f18e50ca760f614f6a3e1c3162e3387d4931ee
size 39759

View file

@ -1,3 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:0c484008012b2d569c6d313823b4ea8c96521fb6a172d37ee1db11f6794dce62
size 14806
oid sha256:399efdebf67b1c042dc105d6538ea4fb622314c883fd6d3899eebcedd3ff357e
size 57540

View file

@ -0,0 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:0c484008012b2d569c6d313823b4ea8c96521fb6a172d37ee1db11f6794dce62
size 14806

View file

@ -1,3 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:8ddfa90c2eaf6b493ebc9c2c5164888716918c7b453c49d7653a0cd4dd69af81
size 42092
oid sha256:bc77d365ea38cd50d5fac90efb15a610b52a069cbb799b8bf0d9924ea650ab79
size 41408

View file

@ -1,3 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:4f22b7825bfa438f55514a3d09696de34450c16f9d4481c4892be9c8dcb45191
size 42486
oid sha256:8ddfa90c2eaf6b493ebc9c2c5164888716918c7b453c49d7653a0cd4dd69af81
size 42092