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.

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

Display a bottom sheet to let user confirm the DM creation #4233

Merged
merged 8 commits into from
Feb 5, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/*
* Copyright 2025 New Vector Ltd.
*
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
* Please see LICENSE files in the repository root for full details.
*/

package io.element.android.features.createroom.api

import io.element.android.libraries.architecture.AsyncAction
import io.element.android.libraries.matrix.api.user.MatrixUser

data class ConfirmingStartDmWithMatrixUser(
val matrixUser: MatrixUser,
) : AsyncAction.Confirming
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,19 @@ package io.element.android.features.createroom.api
import androidx.compose.runtime.MutableState
import io.element.android.libraries.architecture.AsyncAction
import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.api.core.UserId
import io.element.android.libraries.matrix.api.user.MatrixUser

interface StartDMAction {
/**
* Try to find an existing DM with the given user, or create one if none exists.
* @param userId The user to start a DM with.
* @param matrixUser The user to start a DM with.
* @param createIfDmDoesNotExist If true, create a DM if one does not exist. If false and the DM
* does not exist, the action will fail with the value [ConfirmingStartDmWithMatrixUser].
* @param actionState The state to update with the result of the action.
*/
suspend fun execute(userId: UserId, actionState: MutableState<AsyncAction<RoomId>>)
suspend fun execute(
matrixUser: MatrixUser,
createIfDmDoesNotExist: Boolean,
actionState: MutableState<AsyncAction<RoomId>>,
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,15 @@ package io.element.android.features.createroom.impl
import androidx.compose.runtime.MutableState
import com.squareup.anvil.annotations.ContributesBinding
import im.vector.app.features.analytics.plan.CreatedRoom
import io.element.android.features.createroom.api.ConfirmingStartDmWithMatrixUser
import io.element.android.features.createroom.api.StartDMAction
import io.element.android.libraries.architecture.AsyncAction
import io.element.android.libraries.di.SessionScope
import io.element.android.libraries.matrix.api.MatrixClient
import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.api.core.UserId
import io.element.android.libraries.matrix.api.room.StartDMResult
import io.element.android.libraries.matrix.api.room.startDM
import io.element.android.libraries.matrix.api.user.MatrixUser
import io.element.android.services.analytics.api.AnalyticsService
import javax.inject.Inject

Expand All @@ -26,9 +27,13 @@ class DefaultStartDMAction @Inject constructor(
private val matrixClient: MatrixClient,
private val analyticsService: AnalyticsService,
) : StartDMAction {
override suspend fun execute(userId: UserId, actionState: MutableState<AsyncAction<RoomId>>) {
override suspend fun execute(
matrixUser: MatrixUser,
createIfDmDoesNotExist: Boolean,
actionState: MutableState<AsyncAction<RoomId>>,
) {
actionState.value = AsyncAction.Loading
when (val result = matrixClient.startDM(userId)) {
when (val result = matrixClient.startDM(matrixUser.userId, createIfDmDoesNotExist)) {
is StartDMResult.Success -> {
if (result.isNew) {
analyticsService.capture(CreatedRoom(isDM = true))
Expand All @@ -38,6 +43,9 @@ class DefaultStartDMAction @Inject constructor(
is StartDMResult.Failure -> {
actionState.value = AsyncAction.Failure(result.throwable)
}
StartDMResult.DmDoesNotExist -> {
actionState.value = ConfirmingStartDmWithMatrixUser(matrixUser = matrixUser)
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,11 @@ class CreateRoomRootPresenter @Inject constructor(
fun handleEvents(event: CreateRoomRootEvents) {
when (event) {
is CreateRoomRootEvents.StartDM -> localCoroutineScope.launch {
startDMAction.execute(event.matrixUser.userId, startDmActionState)
startDMAction.execute(
matrixUser = event.matrixUser,
createIfDmDoesNotExist = startDmActionState.value is AsyncAction.Confirming,
actionState = startDmActionState,
)
}
CreateRoomRootEvents.CancelStartDM -> startDmActionState.value = AsyncAction.Uninitialized
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
package io.element.android.features.createroom.impl.root

import androidx.compose.ui.tooling.preview.PreviewParameterProvider
import io.element.android.features.createroom.api.ConfirmingStartDmWithMatrixUser
import io.element.android.features.createroom.impl.userlist.UserListState
import io.element.android.features.createroom.impl.userlist.aRecentDirectRoomList
import io.element.android.features.createroom.impl.userlist.aUserListState
Expand Down Expand Up @@ -49,6 +50,9 @@ open class CreateRoomRootStateProvider : PreviewParameterProvider<CreateRoomRoot
recentDirectRooms = aRecentDirectRoomList()
)
),
aCreateRoomRootState(
startDmAction = ConfirmingStartDmWithMatrixUser(aMatrixUser()),
),
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import androidx.compose.ui.tooling.preview.PreviewParameter
import androidx.compose.ui.unit.dp
import io.element.android.compound.theme.ElementTheme
import io.element.android.compound.tokens.generated.CompoundIcons
import io.element.android.features.createroom.api.ConfirmingStartDmWithMatrixUser
import io.element.android.features.createroom.impl.R
import io.element.android.features.createroom.impl.components.UserListView
import io.element.android.libraries.designsystem.components.async.AsyncActionView
Expand All @@ -43,6 +44,7 @@ import io.element.android.libraries.designsystem.theme.components.Scaffold
import io.element.android.libraries.designsystem.theme.components.Text
import io.element.android.libraries.designsystem.theme.components.TopAppBar
import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.ui.components.CreateDmConfirmationBottomSheet
import io.element.android.libraries.matrix.ui.components.MatrixUserRow
import io.element.android.libraries.ui.strings.CommonStrings
import kotlinx.collections.immutable.persistentListOf
Expand Down Expand Up @@ -110,6 +112,19 @@ fun CreateRoomRootView(
?: state.eventSink(CreateRoomRootEvents.CancelStartDM)
},
onErrorDismiss = { state.eventSink(CreateRoomRootEvents.CancelStartDM) },
confirmationDialog = { data ->
if (data is ConfirmingStartDmWithMatrixUser) {
CreateDmConfirmationBottomSheet(
matrixUser = data.matrixUser,
onSendInvite = {
state.eventSink(CreateRoomRootEvents.StartDM(data.matrixUser))
},
onDismiss = {
state.eventSink(CreateRoomRootEvents.CancelStartDM)
},
)
}
},
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,14 @@ package io.element.android.features.createroom.impl
import androidx.compose.runtime.mutableStateOf
import com.google.common.truth.Truth.assertThat
import im.vector.app.features.analytics.plan.CreatedRoom
import io.element.android.features.createroom.api.ConfirmingStartDmWithMatrixUser
import io.element.android.libraries.architecture.AsyncAction
import io.element.android.libraries.matrix.api.MatrixClient
import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.test.A_ROOM_ID
import io.element.android.libraries.matrix.test.A_THROWABLE
import io.element.android.libraries.matrix.test.A_USER_ID
import io.element.android.libraries.matrix.test.FakeMatrixClient
import io.element.android.libraries.matrix.ui.components.aMatrixUser
import io.element.android.services.analytics.api.AnalyticsService
import io.element.android.services.analytics.test.FakeAnalyticsService
import kotlinx.coroutines.test.runTest
Expand All @@ -28,10 +29,12 @@ class DefaultStartDMActionTest {
val matrixClient = FakeMatrixClient().apply {
givenFindDmResult(A_ROOM_ID)
}
val action = createStartDMAction(matrixClient)
val analyticsService = FakeAnalyticsService()
val action = createStartDMAction(matrixClient, analyticsService)
val state = mutableStateOf<AsyncAction<RoomId>>(AsyncAction.Uninitialized)
action.execute(A_USER_ID, state)
action.execute(aMatrixUser(), true, state)
assertThat(state.value).isEqualTo(AsyncAction.Success(A_ROOM_ID))
assertThat(analyticsService.capturedEvents).isEmpty()
}

@Test
Expand All @@ -43,21 +46,38 @@ class DefaultStartDMActionTest {
val analyticsService = FakeAnalyticsService()
val action = createStartDMAction(matrixClient, analyticsService)
val state = mutableStateOf<AsyncAction<RoomId>>(AsyncAction.Uninitialized)
action.execute(A_USER_ID, state)
action.execute(aMatrixUser(), true, state)
assertThat(state.value).isEqualTo(AsyncAction.Success(A_ROOM_ID))
assertThat(analyticsService.capturedEvents).containsExactly(CreatedRoom(isDM = true))
}

@Test
fun `when dm is not found, and createIfDmDoesNotExist is false, assert dm is not created and state is updated to confirmation state`() = runTest {
val matrixClient = FakeMatrixClient().apply {
givenFindDmResult(null)
givenCreateDmResult(Result.success(A_ROOM_ID))
}
val analyticsService = FakeAnalyticsService()
val action = createStartDMAction(matrixClient, analyticsService)
val state = mutableStateOf<AsyncAction<RoomId>>(AsyncAction.Uninitialized)
val matrixUser = aMatrixUser()
action.execute(matrixUser, false, state)
assertThat(state.value).isEqualTo(ConfirmingStartDmWithMatrixUser(matrixUser))
assertThat(analyticsService.capturedEvents).isEmpty()
}

@Test
fun `when dm creation fails, assert state is updated with given error`() = runTest {
val matrixClient = FakeMatrixClient().apply {
givenFindDmResult(null)
givenCreateDmResult(Result.failure(A_THROWABLE))
}
val action = createStartDMAction(matrixClient)
val analyticsService = FakeAnalyticsService()
val action = createStartDMAction(matrixClient, analyticsService)
val state = mutableStateOf<AsyncAction<RoomId>>(AsyncAction.Uninitialized)
action.execute(A_USER_ID, state)
action.execute(aMatrixUser(), true, state)
assertThat(state.value).isEqualTo(AsyncAction.Failure(A_THROWABLE))
assertThat(analyticsService.capturedEvents).isEmpty()
}

private fun createStartDMAction(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,29 @@

package io.element.android.features.createroom.impl.root

import androidx.compose.runtime.MutableState
import app.cash.molecule.RecompositionMode
import app.cash.molecule.moleculeFlow
import app.cash.turbine.test
import com.google.common.truth.Truth.assertThat
import io.element.android.features.createroom.api.ConfirmingStartDmWithMatrixUser
import io.element.android.features.createroom.api.StartDMAction
import io.element.android.features.createroom.impl.userlist.FakeUserListPresenter
import io.element.android.features.createroom.impl.userlist.FakeUserListPresenterFactory
import io.element.android.features.createroom.impl.userlist.UserListDataStore
import io.element.android.features.createroom.test.FakeStartDMAction
import io.element.android.libraries.architecture.AsyncAction
import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.api.core.UserId
import io.element.android.libraries.matrix.api.user.MatrixUser
import io.element.android.libraries.matrix.test.A_ROOM_ID
import io.element.android.libraries.matrix.test.A_THROWABLE
import io.element.android.libraries.matrix.test.core.aBuildMeta
import io.element.android.libraries.usersearch.test.FakeUserRepository
import io.element.android.tests.testutils.WarmUpRule
import io.element.android.tests.testutils.lambda.any
import io.element.android.tests.testutils.lambda.lambdaRecorder
import io.element.android.tests.testutils.lambda.value
import kotlinx.coroutines.test.runTest
import org.junit.Rule
import org.junit.Test
Expand All @@ -33,46 +39,130 @@ class CreateRoomRootPresenterTest {
val warmUpRule = WarmUpRule()

@Test
fun `present - start DM action complete scenario`() = runTest {
val startDMAction = FakeStartDMAction()
fun `present - start DM action failure scenario`() = runTest {
val startDMFailureResult = AsyncAction.Failure(A_THROWABLE)
val executeResult = lambdaRecorder<MatrixUser, Boolean, MutableState<AsyncAction<RoomId>>, Unit> { _, _, actionState ->
actionState.value = startDMFailureResult
}
val startDMAction = FakeStartDMAction(executeResult = executeResult)
val presenter = createCreateRoomRootPresenter(startDMAction)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
val initialState = awaitItem()

assertThat(initialState.startDmAction).isInstanceOf(AsyncAction.Uninitialized::class.java)
assertThat(initialState.applicationName).isEqualTo(aBuildMeta().applicationName)
assertThat(initialState.userListState.selectedUsers).isEmpty()
assertThat(initialState.userListState.isSearchActive).isFalse()
assertThat(initialState.userListState.isMultiSelectionEnabled).isFalse()

val matrixUser = MatrixUser(UserId("@name:domain"))
val startDMSuccessResult = AsyncAction.Success(A_ROOM_ID)
val startDMFailureResult = AsyncAction.Failure(A_THROWABLE)

// Failure
startDMAction.givenExecuteResult(startDMFailureResult)
initialState.eventSink(CreateRoomRootEvents.StartDM(matrixUser))
assertThat(awaitItem().startDmAction).isInstanceOf(AsyncAction.Loading::class.java)
awaitItem().also { state ->
assertThat(state.startDmAction).isEqualTo(startDMFailureResult)
executeResult.assertions().isCalledOnce().with(
value(matrixUser),
value(false),
any(),
)
state.eventSink(CreateRoomRootEvents.CancelStartDM)
}

// Success
startDMAction.givenExecuteResult(startDMSuccessResult)
awaitItem().also { state ->
assertThat(state.startDmAction).isEqualTo(AsyncAction.Uninitialized)
state.eventSink(CreateRoomRootEvents.StartDM(matrixUser))
assertThat(state.startDmAction.isUninitialized()).isTrue()
}
assertThat(awaitItem().startDmAction).isInstanceOf(AsyncAction.Loading::class.java)
}
}

@Test
fun `present - start DM action success scenario`() = runTest {
val startDMSuccessResult = AsyncAction.Success(A_ROOM_ID)
val executeResult = lambdaRecorder<MatrixUser, Boolean, MutableState<AsyncAction<RoomId>>, Unit> { _, _, actionState ->
actionState.value = startDMSuccessResult
}
val startDMAction = FakeStartDMAction(executeResult = executeResult)
val presenter = createCreateRoomRootPresenter(startDMAction)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
val initialState = awaitItem()
assertThat(initialState.startDmAction).isInstanceOf(AsyncAction.Uninitialized::class.java)
assertThat(initialState.applicationName).isEqualTo(aBuildMeta().applicationName)
assertThat(initialState.userListState.selectedUsers).isEmpty()
assertThat(initialState.userListState.isSearchActive).isFalse()
assertThat(initialState.userListState.isMultiSelectionEnabled).isFalse()
val matrixUser = MatrixUser(UserId("@name:domain"))
initialState.eventSink(CreateRoomRootEvents.StartDM(matrixUser))
awaitItem().also { state ->
assertThat(state.startDmAction).isEqualTo(startDMSuccessResult)
executeResult.assertions().isCalledOnce().with(
value(matrixUser),
value(false),
any(),
)
}
}
}

@Test
fun `present - start DM action confirmation scenario - cancel`() = runTest {
val matrixUser = MatrixUser(UserId("@name:domain"))
val startDMConfirmationResult = ConfirmingStartDmWithMatrixUser(matrixUser)
val executeResult = lambdaRecorder<MatrixUser, Boolean, MutableState<AsyncAction<RoomId>>, Unit> { _, _, actionState ->
actionState.value = startDMConfirmationResult
}
val startDMAction = FakeStartDMAction(executeResult = executeResult)
val presenter = createCreateRoomRootPresenter(startDMAction)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
val initialState = awaitItem()
assertThat(initialState.startDmAction).isInstanceOf(AsyncAction.Uninitialized::class.java)
initialState.eventSink(CreateRoomRootEvents.StartDM(matrixUser))
val confirmingState = awaitItem()
assertThat(confirmingState.startDmAction).isEqualTo(startDMConfirmationResult)
executeResult.assertions().isCalledOnce().with(
value(matrixUser),
value(false),
any(),
)
// Cancelling should not create the DM
confirmingState.eventSink(CreateRoomRootEvents.CancelStartDM)
val finalState = awaitItem()
assertThat(finalState.startDmAction.isUninitialized()).isTrue()
executeResult.assertions().isCalledExactly(1)
}
}

@Test
fun `present - start DM action confirmation scenario - confirm`() = runTest {
val matrixUser = MatrixUser(UserId("@name:domain"))
val startDMConfirmationResult = ConfirmingStartDmWithMatrixUser(matrixUser)
val executeResult = lambdaRecorder<MatrixUser, Boolean, MutableState<AsyncAction<RoomId>>, Unit> { _, _, actionState ->
actionState.value = startDMConfirmationResult
}
val startDMAction = FakeStartDMAction(executeResult = executeResult)
val presenter = createCreateRoomRootPresenter(startDMAction)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
val initialState = awaitItem()
assertThat(initialState.startDmAction).isInstanceOf(AsyncAction.Uninitialized::class.java)
initialState.eventSink(CreateRoomRootEvents.StartDM(matrixUser))
val confirmingState = awaitItem()
assertThat(confirmingState.startDmAction).isEqualTo(startDMConfirmationResult)
executeResult.assertions().isCalledOnce().with(
value(matrixUser),
value(false),
any(),
)
// Start DM again should invoke the action with createIfDmDoesNotExist = true
confirmingState.eventSink(CreateRoomRootEvents.StartDM(matrixUser))
executeResult.assertions().isCalledExactly(2).withSequence(
listOf(value(matrixUser), value(false), any()),
listOf(value(matrixUser), value(true), any()),
)
}
}

private fun createCreateRoomRootPresenter(
startDMAction: StartDMAction = FakeStartDMAction(),
): CreateRoomRootPresenter {
Expand Down
Loading
Loading