Simplify ForwardMessagesState by using AsyncAction.

This commit is contained in:
Benoit Marty 2024-05-30 12:22:20 +02:00 committed by Benoit Marty
parent e6fab7eee2
commit 130bce178a
6 changed files with 42 additions and 83 deletions

View file

@ -36,7 +36,6 @@ import io.element.android.libraries.matrix.api.core.EventId
import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.roomselect.api.RoomSelectEntryPoint import io.element.android.libraries.roomselect.api.RoomSelectEntryPoint
import io.element.android.libraries.roomselect.api.RoomSelectMode import io.element.android.libraries.roomselect.api.RoomSelectMode
import kotlinx.collections.immutable.ImmutableList
import kotlinx.parcelize.Parcelize import kotlinx.parcelize.Parcelize
@ContributesNode(RoomScope::class) @ContributesNode(RoomScope::class)
@ -99,7 +98,7 @@ class ForwardMessagesNode @AssistedInject constructor(
} }
} }
private fun onForwardSuccess(roomIds: ImmutableList<RoomId>) { private fun onForwardSuccess(roomIds: List<RoomId>) {
navigateUp() navigateUp()
if (roomIds.size == 1) { if (roomIds.size == 1) {
val targetRoomId = roomIds.first() val targetRoomId = roomIds.first()

View file

@ -18,15 +18,13 @@ package io.element.android.features.messages.impl.forward
import androidx.compose.runtime.Composable import androidx.compose.runtime.Composable
import androidx.compose.runtime.MutableState import androidx.compose.runtime.MutableState
import androidx.compose.runtime.derivedStateOf
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import dagger.assisted.Assisted import dagger.assisted.Assisted
import dagger.assisted.AssistedFactory import dagger.assisted.AssistedFactory
import dagger.assisted.AssistedInject import dagger.assisted.AssistedInject
import io.element.android.libraries.architecture.AsyncData import io.element.android.libraries.architecture.AsyncAction
import io.element.android.libraries.architecture.Presenter import io.element.android.libraries.architecture.Presenter
import io.element.android.libraries.architecture.runCatchingUpdatingState
import io.element.android.libraries.matrix.api.core.EventId import io.element.android.libraries.matrix.api.core.EventId
import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.api.timeline.TimelineProvider import io.element.android.libraries.matrix.api.timeline.TimelineProvider
@ -38,7 +36,7 @@ import kotlinx.coroutines.launch
class ForwardMessagesPresenter @AssistedInject constructor( class ForwardMessagesPresenter @AssistedInject constructor(
@Assisted eventId: String, @Assisted eventId: String,
private val matrixCoroutineScope: CoroutineScope, private val appCoroutineScope: CoroutineScope,
private val timelineProvider: TimelineProvider, private val timelineProvider: TimelineProvider,
) : Presenter<ForwardMessagesState> { ) : Presenter<ForwardMessagesState> {
private val eventId: EventId = EventId(eventId) private val eventId: EventId = EventId(eventId)
@ -48,28 +46,22 @@ class ForwardMessagesPresenter @AssistedInject constructor(
fun create(eventId: String): ForwardMessagesPresenter fun create(eventId: String): ForwardMessagesPresenter
} }
private val forwardingActionState: MutableState<AsyncData<ImmutableList<RoomId>>> = mutableStateOf(AsyncData.Uninitialized) private val forwardingActionState: MutableState<AsyncAction<List<RoomId>>> = mutableStateOf(AsyncAction.Uninitialized)
fun onRoomSelected(roomIds: List<RoomId>) { fun onRoomSelected(roomIds: List<RoomId>) {
matrixCoroutineScope.forwardEvent(eventId, roomIds.toPersistentList(), forwardingActionState) appCoroutineScope.forwardEvent(eventId, roomIds.toPersistentList(), forwardingActionState)
} }
@Composable @Composable
override fun present(): ForwardMessagesState { override fun present(): ForwardMessagesState {
val forwardingSucceeded by remember {
derivedStateOf { forwardingActionState.value.dataOrNull() }
}
fun handleEvents(event: ForwardMessagesEvents) { fun handleEvents(event: ForwardMessagesEvents) {
when (event) { when (event) {
ForwardMessagesEvents.ClearError -> forwardingActionState.value = AsyncData.Uninitialized ForwardMessagesEvents.ClearError -> forwardingActionState.value = AsyncAction.Uninitialized
} }
} }
return ForwardMessagesState( return ForwardMessagesState(
isForwarding = forwardingActionState.value.isLoading(), forwardAction = forwardingActionState.value,
error = (forwardingActionState.value as? AsyncData.Failure)?.error,
forwardingSucceeded = forwardingSucceeded,
eventSink = { handleEvents(it) } eventSink = { handleEvents(it) }
) )
} }
@ -77,12 +69,11 @@ class ForwardMessagesPresenter @AssistedInject constructor(
private fun CoroutineScope.forwardEvent( private fun CoroutineScope.forwardEvent(
eventId: EventId, eventId: EventId,
roomIds: ImmutableList<RoomId>, roomIds: ImmutableList<RoomId>,
isForwardMessagesState: MutableState<AsyncData<ImmutableList<RoomId>>>, isForwardMessagesState: MutableState<AsyncAction<List<RoomId>>>,
) = launch { ) = launch {
isForwardMessagesState.value = AsyncData.Loading() suspend {
timelineProvider.getActiveTimeline().forwardEvent(eventId, roomIds).fold( timelineProvider.getActiveTimeline().forwardEvent(eventId, roomIds).getOrThrow()
{ isForwardMessagesState.value = AsyncData.Success(roomIds) }, roomIds
{ isForwardMessagesState.value = AsyncData.Failure(it) } }.runCatchingUpdatingState(isForwardMessagesState)
)
} }
} }

View file

@ -16,13 +16,10 @@
package io.element.android.features.messages.impl.forward package io.element.android.features.messages.impl.forward
import io.element.android.libraries.architecture.AsyncAction
import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.core.RoomId
import kotlinx.collections.immutable.ImmutableList
data class ForwardMessagesState( data class ForwardMessagesState(
// TODO Migrate to an Async val forwardAction: AsyncAction<List<RoomId>>,
val isForwarding: Boolean,
val error: Throwable?,
val forwardingSucceeded: ImmutableList<RoomId>?,
val eventSink: (ForwardMessagesEvents) -> Unit val eventSink: (ForwardMessagesEvents) -> Unit
) )

View file

@ -17,34 +17,30 @@
package io.element.android.features.messages.impl.forward package io.element.android.features.messages.impl.forward
import androidx.compose.ui.tooling.preview.PreviewParameterProvider import androidx.compose.ui.tooling.preview.PreviewParameterProvider
import io.element.android.libraries.architecture.AsyncAction
import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.core.RoomId
import kotlinx.collections.immutable.ImmutableList
import kotlinx.collections.immutable.persistentListOf
open class ForwardMessagesStateProvider : PreviewParameterProvider<ForwardMessagesState> { open class ForwardMessagesStateProvider : PreviewParameterProvider<ForwardMessagesState> {
override val values: Sequence<ForwardMessagesState> override val values: Sequence<ForwardMessagesState>
get() = sequenceOf( get() = sequenceOf(
aForwardMessagesState(), aForwardMessagesState(),
aForwardMessagesState( aForwardMessagesState(
isForwarding = true, forwardAction = AsyncAction.Loading,
), ),
aForwardMessagesState( aForwardMessagesState(
forwardingSucceeded = persistentListOf(RoomId("!room2:domain")), forwardAction = AsyncAction.Success(
listOf(RoomId("!room2:domain")),
)
), ),
aForwardMessagesState( aForwardMessagesState(
error = Throwable("error"), forwardAction = AsyncAction.Failure(Throwable("error")),
), ),
// Add other states here
) )
} }
fun aForwardMessagesState( fun aForwardMessagesState(
isForwarding: Boolean = false, forwardAction: AsyncAction<List<RoomId>> = AsyncAction.Uninitialized,
error: Throwable? = null,
forwardingSucceeded: ImmutableList<RoomId>? = null,
) = ForwardMessagesState( ) = ForwardMessagesState(
isForwarding = isForwarding, forwardAction = forwardAction,
error = error,
forwardingSucceeded = forwardingSucceeded,
eventSink = {} eventSink = {}
) )

View file

@ -17,45 +17,25 @@
package io.element.android.features.messages.impl.forward package io.element.android.features.messages.impl.forward
import androidx.compose.runtime.Composable import androidx.compose.runtime.Composable
import androidx.compose.ui.Modifier
import androidx.compose.ui.tooling.preview.PreviewParameter import androidx.compose.ui.tooling.preview.PreviewParameter
import io.element.android.libraries.designsystem.components.ProgressDialog import io.element.android.libraries.designsystem.components.async.AsyncActionView
import io.element.android.libraries.designsystem.components.dialogs.ErrorDialog
import io.element.android.libraries.designsystem.components.dialogs.ErrorDialogDefaults
import io.element.android.libraries.designsystem.preview.ElementPreview import io.element.android.libraries.designsystem.preview.ElementPreview
import io.element.android.libraries.designsystem.preview.PreviewsDayNight import io.element.android.libraries.designsystem.preview.PreviewsDayNight
import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.core.RoomId
import kotlinx.collections.immutable.ImmutableList
@Composable @Composable
fun ForwardMessagesView( fun ForwardMessagesView(
state: ForwardMessagesState, state: ForwardMessagesState,
onForwardSuccess: (ImmutableList<RoomId>) -> Unit, onForwardSuccess: (List<RoomId>) -> Unit,
modifier: Modifier = Modifier,
) { ) {
if (state.forwardingSucceeded != null) { AsyncActionView(
onForwardSuccess(state.forwardingSucceeded) async = state.forwardAction,
return onSuccess = {
} onForwardSuccess(it)
},
if (state.isForwarding) { onErrorDismiss = {
ProgressDialog(modifier) state.eventSink(ForwardMessagesEvents.ClearError)
} },
if (state.error != null) {
ForwardingErrorDialog(
modifier = modifier,
onDismiss = { state.eventSink(ForwardMessagesEvents.ClearError) },
)
}
}
@Composable
private fun ForwardingErrorDialog(onDismiss: () -> Unit, modifier: Modifier = Modifier) {
ErrorDialog(
content = ErrorDialogDefaults.title,
onDismiss = onDismiss,
modifier = modifier,
) )
} }

View file

@ -20,6 +20,7 @@ import app.cash.molecule.RecompositionMode
import app.cash.molecule.moleculeFlow import app.cash.molecule.moleculeFlow
import app.cash.turbine.test import app.cash.turbine.test
import com.google.common.truth.Truth.assertThat import com.google.common.truth.Truth.assertThat
import io.element.android.libraries.architecture.AsyncAction
import io.element.android.libraries.matrix.api.core.EventId import io.element.android.libraries.matrix.api.core.EventId
import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.test.AN_EVENT_ID import io.element.android.libraries.matrix.test.AN_EVENT_ID
@ -28,13 +29,11 @@ import io.element.android.libraries.matrix.test.room.aRoomSummaryDetails
import io.element.android.libraries.matrix.test.timeline.FakeTimeline import io.element.android.libraries.matrix.test.timeline.FakeTimeline
import io.element.android.libraries.matrix.test.timeline.LiveTimelineProvider import io.element.android.libraries.matrix.test.timeline.LiveTimelineProvider
import io.element.android.tests.testutils.WarmUpRule import io.element.android.tests.testutils.WarmUpRule
import io.element.android.tests.testutils.lambda.assert
import io.element.android.tests.testutils.lambda.lambdaRecorder import io.element.android.tests.testutils.lambda.lambdaRecorder
import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.test.runTest import kotlinx.coroutines.test.runTest
import org.junit.Rule import org.junit.Rule
import org.junit.Test import org.junit.Test
import java.lang.IllegalStateException
class ForwardMessagesPresenterTest { class ForwardMessagesPresenterTest {
@get:Rule @get:Rule
@ -47,9 +46,7 @@ class ForwardMessagesPresenterTest {
presenter.present() presenter.present()
}.test { }.test {
val initialState = awaitItem() val initialState = awaitItem()
assertThat(initialState.isForwarding).isFalse() assertThat(initialState.forwardAction.isUninitialized()).isTrue()
assertThat(initialState.error).isNull()
assertThat(initialState.forwardingSucceeded).isNull()
} }
} }
@ -70,11 +67,10 @@ class ForwardMessagesPresenterTest {
val summary = aRoomSummaryDetails() val summary = aRoomSummaryDetails()
presenter.onRoomSelected(listOf(summary.roomId)) presenter.onRoomSelected(listOf(summary.roomId))
val forwardingState = awaitItem() val forwardingState = awaitItem()
assertThat(forwardingState.isForwarding).isTrue() assertThat(forwardingState.forwardAction.isLoading()).isTrue()
val successfulForwardState = awaitItem() val successfulForwardState = awaitItem()
assertThat(successfulForwardState.isForwarding).isFalse() assertThat(successfulForwardState.forwardAction).isEqualTo(AsyncAction.Success(listOf(summary.roomId)))
assertThat(successfulForwardState.forwardingSucceeded).isNotNull() forwardEventLambda.assertions().isCalledOnce()
assert(forwardEventLambda).isCalledOnce()
} }
} }
@ -96,11 +92,11 @@ class ForwardMessagesPresenterTest {
presenter.onRoomSelected(listOf(summary.roomId)) presenter.onRoomSelected(listOf(summary.roomId))
skipItems(1) skipItems(1)
val failedForwardState = awaitItem() val failedForwardState = awaitItem()
assertThat(failedForwardState.error).isNotNull() assertThat(failedForwardState.forwardAction.isFailure()).isTrue()
// Then clear error // Then clear error
failedForwardState.eventSink(ForwardMessagesEvents.ClearError) failedForwardState.eventSink(ForwardMessagesEvents.ClearError)
assertThat(awaitItem().error).isNull() assertThat(awaitItem().forwardAction.isUninitialized()).isTrue()
assert(forwardEventLambda).isCalledOnce() forwardEventLambda.assertions().isCalledOnce()
} }
} }
@ -111,6 +107,6 @@ class ForwardMessagesPresenterTest {
) = ForwardMessagesPresenter( ) = ForwardMessagesPresenter(
eventId = eventId.value, eventId = eventId.value,
timelineProvider = LiveTimelineProvider(fakeMatrixRoom), timelineProvider = LiveTimelineProvider(fakeMatrixRoom),
matrixCoroutineScope = coroutineScope, appCoroutineScope = coroutineScope,
) )
} }