Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

AsyncAction confirming with param #3667

Merged
merged 4 commits into from
Oct 15, 2024
Merged

AsyncAction confirming with param #3667

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fb52657
Let `AsyncAction.Confirming` be an interface, with a `AsyncAction.Con…
bmarty Oct 14, 2024
ecd7bb0
AsyncActionView: provide the value to the confirmationDialog lambda.
bmarty Oct 14, 2024
03fd7af
AcceptDeclineInviteState: create ConfirmingDeclineInvite to host Invi…
bmarty Oct 14, 2024
ca4f906
Split long lines
bmarty Oct 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions ...c/main/kotlin/io/element/android/features/invite/api/response/AcceptDeclineInviteState.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,8 @@ package io.element.android.features.invite.api.response

import io.element.android.libraries.architecture.AsyncAction
import io.element.android.libraries.matrix.api.core.RoomId
import java.util.Optional

data class AcceptDeclineInviteState(
val invite: Optional<InviteData>,
val acceptAction: AsyncAction<RoomId>,
val declineAction: AsyncAction<RoomId>,
val eventSink: (AcceptDeclineInviteEvents) -> Unit,
Expand Down
13 changes: 4 additions & 9 deletions ...otlin/io/element/android/features/invite/api/response/AcceptDeclineInviteStateProvider.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,20 @@ package io.element.android.features.invite.api.response
import androidx.compose.ui.tooling.preview.PreviewParameterProvider
import io.element.android.libraries.architecture.AsyncAction
import io.element.android.libraries.matrix.api.core.RoomId
import java.util.Optional

open class AcceptDeclineInviteStateProvider : PreviewParameterProvider<AcceptDeclineInviteState> {
override val values: Sequence<AcceptDeclineInviteState>
get() = sequenceOf(
anAcceptDeclineInviteState(),
anAcceptDeclineInviteState(
invite = Optional.of(
InviteData(RoomId("!room:matrix.org"), isDm = true, roomName = "Alice"),
declineAction = ConfirmingDeclineInvite(
InviteData(RoomId("!room:matrix.org"), isDm = true, roomName = "Alice")
),
declineAction = AsyncAction.ConfirmingNoParams,
),
anAcceptDeclineInviteState(
invite = Optional.of(
InviteData(RoomId("!room:matrix.org"), isDm = false, roomName = "Some room"),
declineAction = ConfirmingDeclineInvite(
InviteData(RoomId("!room:matrix.org"), isDm = false, roomName = "Some room")
),
declineAction = AsyncAction.ConfirmingNoParams,
),
anAcceptDeclineInviteState(
acceptAction = AsyncAction.Failure(Throwable("Whoops")),
Expand All @@ -38,12 +35,10 @@ open class AcceptDeclineInviteStateProvider : PreviewParameterProvider<AcceptDec
}

fun anAcceptDeclineInviteState(
invite: Optional<InviteData> = Optional.empty(),
acceptAction: AsyncAction<RoomId> = AsyncAction.Uninitialized,
declineAction: AsyncAction<RoomId> = AsyncAction.Uninitialized,
eventSink: (AcceptDeclineInviteEvents) -> Unit = {}
) = AcceptDeclineInviteState(
invite = invite,
acceptAction = acceptAction,
declineAction = declineAction,
eventSink = eventSink,
Expand Down
14 changes: 14 additions & 0 deletions ...rc/main/kotlin/io/element/android/features/invite/api/response/ConfirmingDeclineInvite.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/*
* Copyright 2024 New Vector Ltd.
*
* SPDX-License-Identifier: AGPL-3.0-only
* Please see LICENSE in the repository root for full details.
*/

package io.element.android.features.invite.api.response

import io.element.android.libraries.architecture.AsyncAction

data class ConfirmingDeclineInvite(
val inviteData: InviteData,
) : AsyncAction.Confirming
24 changes: 3 additions & 21 deletions ...n/kotlin/io/element/android/features/invite/impl/response/AcceptDeclineInvitePresenter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,13 @@ package io.element.android.features.invite.impl.response

import androidx.compose.runtime.Composable
import androidx.compose.runtime.MutableState
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberCoroutineScope
import androidx.compose.runtime.setValue
import im.vector.app.features.analytics.plan.JoinedRoom
import io.element.android.features.invite.api.response.AcceptDeclineInviteEvents
import io.element.android.features.invite.api.response.AcceptDeclineInviteState
import io.element.android.features.invite.api.response.InviteData
import io.element.android.features.invite.api.response.ConfirmingDeclineInvite
import io.element.android.libraries.architecture.AsyncAction
import io.element.android.libraries.architecture.Presenter
import io.element.android.libraries.architecture.runCatchingUpdatingState
Expand All @@ -29,9 +27,7 @@ import io.element.android.libraries.matrix.api.room.join.JoinRoom
import io.element.android.libraries.push.api.notifications.NotificationCleaner
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.launch
import java.util.Optional
import javax.inject.Inject
import kotlin.jvm.optionals.getOrNull

class AcceptDeclineInvitePresenter @Inject constructor(
private val client: MatrixClient,
Expand All @@ -43,35 +39,22 @@ class AcceptDeclineInvitePresenter @Inject constructor(
val localCoroutineScope = rememberCoroutineScope()
val acceptedAction: MutableState<AsyncAction<RoomId>> = remember { mutableStateOf(AsyncAction.Uninitialized) }
val declinedAction: MutableState<AsyncAction<RoomId>> = remember { mutableStateOf(AsyncAction.Uninitialized) }
var currentInvite by remember {
mutableStateOf<Optional<InviteData>>(Optional.empty())
}

fun handleEvents(event: AcceptDeclineInviteEvents) {
when (event) {
is AcceptDeclineInviteEvents.AcceptInvite -> {
// currentInvite is used to render the decline confirmation dialog
// and to reuse the roomId when the user confirm the rejection of the invitation.
// Just set it to empty here.
currentInvite = Optional.empty()
localCoroutineScope.acceptInvite(event.invite.roomId, acceptedAction)
}

is AcceptDeclineInviteEvents.DeclineInvite -> {
currentInvite = Optional.of(event.invite)
declinedAction.value = AsyncAction.ConfirmingNoParams
declinedAction.value = ConfirmingDeclineInvite(event.invite)
}

is InternalAcceptDeclineInviteEvents.ConfirmDeclineInvite -> {
declinedAction.value = AsyncAction.Uninitialized
currentInvite.getOrNull()?.let {
localCoroutineScope.declineInvite(it.roomId, declinedAction)
}
currentInvite = Optional.empty()
localCoroutineScope.declineInvite(event.roomId, declinedAction)
}

is InternalAcceptDeclineInviteEvents.CancelDeclineInvite -> {
currentInvite = Optional.empty()
declinedAction.value = AsyncAction.Uninitialized
}

Expand All @@ -86,7 +69,6 @@ class AcceptDeclineInvitePresenter @Inject constructor(
}

return AcceptDeclineInviteState(
invite = currentInvite,
acceptAction = acceptedAction.value,
declineAction = declinedAction.value,
eventSink = ::handleEvents
Expand Down
12 changes: 6 additions & 6 deletions ...c/main/kotlin/io/element/android/features/invite/impl/response/AcceptDeclineInviteView.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import androidx.compose.ui.res.stringResource
import androidx.compose.ui.tooling.preview.PreviewParameter
import io.element.android.features.invite.api.response.AcceptDeclineInviteState
import io.element.android.features.invite.api.response.AcceptDeclineInviteStateProvider
import io.element.android.features.invite.api.response.ConfirmingDeclineInvite
import io.element.android.features.invite.api.response.InviteData
import io.element.android.features.invite.impl.R
import io.element.android.libraries.designsystem.components.async.AsyncActionView
Expand All @@ -22,7 +23,6 @@ import io.element.android.libraries.designsystem.preview.ElementPreview
import io.element.android.libraries.designsystem.preview.PreviewsDayNight
import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.ui.strings.CommonStrings
import kotlin.jvm.optionals.getOrNull

@Composable
fun AcceptDeclineInviteView(
Expand All @@ -45,13 +45,13 @@ fun AcceptDeclineInviteView(
onErrorDismiss = {
state.eventSink(InternalAcceptDeclineInviteEvents.DismissDeclineError)
},
confirmationDialog = {
val invite = state.invite.getOrNull()
if (invite != null) {
confirmationDialog = { confirming ->
// Note: confirming will always be of type ConfirmingDeclineInvite.
if (confirming is ConfirmingDeclineInvite) {
Comment on lines +49 to +50
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we just force cast it then?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could but why take the risk of a crash?
Previous code was not val invite = state.invite!!, so I prefer to keep the code safe.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could but why take the risk of a crash?

Can we then log an error or upload some error report to sentry? I'm worried this might just silently fail, although the tests should cover it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My previous attempt to fix this would have avoid this drawback. But it was too invasive, so we go with this approach. This is not a new issue, so I guess this is fine.

DeclineConfirmationDialog(
invite = invite,
invite = confirming.inviteData,
onConfirmClick = {
state.eventSink(InternalAcceptDeclineInviteEvents.ConfirmDeclineInvite)
state.eventSink(InternalAcceptDeclineInviteEvents.ConfirmDeclineInvite(confirming.inviteData.roomId))
},
onDismissClick = {
state.eventSink(InternalAcceptDeclineInviteEvents.CancelDeclineInvite)
Expand Down
3 changes: 2 additions & 1 deletion ...lin/io/element/android/features/invite/impl/response/InternalAcceptDeclineInviteEvents.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@
package io.element.android.features.invite.impl.response

import io.element.android.features.invite.api.response.AcceptDeclineInviteEvents
import io.element.android.libraries.matrix.api.core.RoomId

sealed interface InternalAcceptDeclineInviteEvents : AcceptDeclineInviteEvents {
data object ConfirmDeclineInvite : InternalAcceptDeclineInviteEvents
data class ConfirmDeclineInvite(val roomId: RoomId) : InternalAcceptDeclineInviteEvents
data object CancelDeclineInvite : InternalAcceptDeclineInviteEvents
data object DismissAcceptError : InternalAcceptDeclineInviteEvents
data object DismissDeclineError : InternalAcceptDeclineInviteEvents
Expand Down
26 changes: 8 additions & 18 deletions ...tlin/io/element/android/features/invite/impl/response/AcceptDeclineInvitePresenterTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ package io.element.android.features.invite.impl.response
import com.google.common.truth.Truth.assertThat
import im.vector.app.features.analytics.plan.JoinedRoom
import io.element.android.features.invite.api.response.AcceptDeclineInviteEvents
import io.element.android.features.invite.api.response.ConfirmingDeclineInvite
import io.element.android.features.invite.api.response.InviteData
import io.element.android.libraries.architecture.AsyncAction
import io.element.android.libraries.matrix.api.MatrixClient
Expand All @@ -33,7 +34,6 @@ import io.element.android.tests.testutils.test
import kotlinx.coroutines.test.runTest
import org.junit.Rule
import org.junit.Test
import java.util.Optional

class AcceptDeclineInvitePresenterTest {
@get:Rule
Expand All @@ -46,7 +46,6 @@ class AcceptDeclineInvitePresenterTest {
awaitItem().also { state ->
assertThat(state.acceptAction).isInstanceOf(AsyncAction.Uninitialized::class.java)
assertThat(state.declineAction).isInstanceOf(AsyncAction.Uninitialized::class.java)
assertThat(state.invite).isEqualTo(Optional.empty<InviteData>())
}
}
}
Expand All @@ -61,17 +60,13 @@ class AcceptDeclineInvitePresenterTest {
AcceptDeclineInviteEvents.DeclineInvite(inviteData)
)
}
skipItems(1)
awaitItem().also { state ->
assertThat(state.invite).isEqualTo(Optional.of(inviteData))
assertThat(state.declineAction).isInstanceOf(AsyncAction.Confirming::class.java)
assertThat(state.declineAction).isEqualTo(ConfirmingDeclineInvite(inviteData))
state.eventSink(
InternalAcceptDeclineInviteEvents.CancelDeclineInvite
)
}
skipItems(1)
awaitItem().also { state ->
assertThat(state.invite).isEqualTo(Optional.empty<InviteData>())
assertThat(state.declineAction).isInstanceOf(AsyncAction.Uninitialized::class.java)
}
}
Expand All @@ -93,22 +88,20 @@ class AcceptDeclineInvitePresenterTest {
AcceptDeclineInviteEvents.DeclineInvite(inviteData)
)
}
skipItems(1)
awaitItem().also { state ->
assertThat(state.declineAction).isEqualTo(ConfirmingDeclineInvite(inviteData))
state.eventSink(
InternalAcceptDeclineInviteEvents.ConfirmDeclineInvite
InternalAcceptDeclineInviteEvents.ConfirmDeclineInvite(inviteData.roomId)
)
}
skipItems(2)
assertThat(awaitItem().declineAction.isLoading()).isTrue()
awaitItem().also { state ->
assertThat(state.declineAction).isInstanceOf(AsyncAction.Failure::class.java)
state.eventSink(
InternalAcceptDeclineInviteEvents.DismissDeclineError
)
}
skipItems(1)
awaitItem().also { state ->
assertThat(state.invite).isEqualTo(Optional.empty<InviteData>())
assertThat(state.declineAction).isInstanceOf(AsyncAction.Uninitialized::class.java)
}
cancelAndConsumeRemainingEvents()
Expand Down Expand Up @@ -141,13 +134,13 @@ class AcceptDeclineInvitePresenterTest {
AcceptDeclineInviteEvents.DeclineInvite(inviteData)
)
}
skipItems(1)
awaitItem().also { state ->
assertThat(state.declineAction).isEqualTo(ConfirmingDeclineInvite(inviteData))
state.eventSink(
InternalAcceptDeclineInviteEvents.ConfirmDeclineInvite
InternalAcceptDeclineInviteEvents.ConfirmDeclineInvite(inviteData.roomId)
)
}
skipItems(2)
assertThat(awaitItem().declineAction.isLoading()).isTrue()
awaitItem().also { state ->
assertThat(state.declineAction).isInstanceOf(AsyncAction.Success::class.java)
}
Expand All @@ -173,7 +166,6 @@ class AcceptDeclineInvitePresenterTest {
)
}
awaitItem().also { state ->
assertThat(state.invite).isEqualTo(Optional.empty<InviteData>())
assertThat(state.acceptAction).isEqualTo(AsyncAction.Loading)
}
awaitItem().also { state ->
Expand All @@ -183,7 +175,6 @@ class AcceptDeclineInvitePresenterTest {
)
}
awaitItem().also { state ->
assertThat(state.invite).isEqualTo(Optional.empty<InviteData>())
assertThat(state.acceptAction).isInstanceOf(AsyncAction.Uninitialized::class.java)
}
cancelAndConsumeRemainingEvents()
Expand Down Expand Up @@ -220,7 +211,6 @@ class AcceptDeclineInvitePresenterTest {
)
}
awaitItem().also { state ->
assertThat(state.invite).isEqualTo(Optional.empty<InviteData>())
assertThat(state.acceptAction).isEqualTo(AsyncAction.Loading)
}
awaitItem().also { state ->
Expand Down
5 changes: 3 additions & 2 deletions ...rix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeInvitedRoom.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,15 @@ import io.element.android.libraries.matrix.api.room.InvitedRoom
import io.element.android.libraries.matrix.test.A_ROOM_ID
import io.element.android.libraries.matrix.test.A_SESSION_ID
import io.element.android.tests.testutils.lambda.lambdaError
import io.element.android.tests.testutils.simulateLongTask

class FakeInvitedRoom(
override val sessionId: SessionId = A_SESSION_ID,
override val roomId: RoomId = A_ROOM_ID,
private val declineInviteResult: () -> Result<Unit> = { lambdaError() }
) : InvitedRoom {
override suspend fun declineInvite(): Result<Unit> {
return declineInviteResult()
override suspend fun declineInvite(): Result<Unit> = simulateLongTask {
declineInviteResult()
}

override fun close() = Unit
Expand Down