Skip to content

Commit

Permalink
Merge pull request #2721 from element-hq/feature/bma/roomMemberDetail…
Browse files Browse the repository at this point in the history 
…sPresenterImprovement

Room member details presenter improvement
  • Loading branch information
@bmarty
bmarty committed Apr 17, 2024
2 parents e971218 + 5b3a2d8 commit e326824
Show file tree
Hide file tree
Showing 4 changed files with 133 additions and 50 deletions.
1 change: 1 addition & 0 deletions changelog.d/2721.misc
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
RoomMember screen: fallback to userProfile data, if the member is not a user of the room.
87 changes: 52 additions & 35 deletions ...o/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsPresenter.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,13 @@ 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.MatrixRoom
import io.element.android.libraries.matrix.api.user.MatrixUser
import io.element.android.libraries.matrix.ui.room.getRoomMemberAsState
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.launch

class RoomMemberDetailsPresenter @AssistedInject constructor(
Expand All @@ -56,20 +61,24 @@ class RoomMemberDetailsPresenter @AssistedInject constructor(
val coroutineScope = rememberCoroutineScope()
var confirmationDialog by remember { mutableStateOf<ConfirmationDialog?>(null) }
val roomMember by room.getRoomMemberAsState(roomMemberId)
var userProfile by remember { mutableStateOf<MatrixUser?>(null) }
val startDmActionState: MutableState<AsyncAction<RoomId>> = remember { mutableStateOf(AsyncAction.Uninitialized) }
// the room member is not really live...
val isBlocked: MutableState<AsyncData<Boolean>> = remember(roomMember) {
val isIgnored = roomMember?.isIgnored
if (isIgnored == null) {
mutableStateOf(AsyncData.Uninitialized)
} else {
mutableStateOf(AsyncData.Success(isIgnored))
}
val isBlocked: MutableState<AsyncData<Boolean>> = remember { mutableStateOf(AsyncData.Uninitialized) }
LaunchedEffect(Unit) {
client.ignoredUsersFlow
.map { ignoredUsers -> roomMemberId in ignoredUsers }
.distinctUntilChanged()
.onEach { isBlocked.value = AsyncData.Success(it) }
.launchIn(this)
}
LaunchedEffect(Unit) {
// Update room member info when opening this screen
// We don't need to assign the result as it will be automatically propagated by `room.getRoomMemberAsState`
room.getUpdatedMember(roomMemberId)
.onFailure {
// Not a member of the room, try to get the user profile
userProfile = client.getProfile(roomMemberId).getOrNull()
}
}

fun handleEvents(event: RoomMemberDetailsEvents) {
Expand Down Expand Up @@ -105,16 +114,34 @@ class RoomMemberDetailsPresenter @AssistedInject constructor(
}
}

val userName by produceState(initialValue = roomMember?.displayName) {
room.userDisplayName(roomMemberId).onSuccess { displayName ->
if (displayName != null) value = displayName
}
val userName: String? by produceState(
initialValue = roomMember?.displayName ?: userProfile?.displayName,
key1 = roomMember,
key2 = userProfile,
) {
value = room.userDisplayName(roomMemberId)
.fold(
onSuccess = { it },
onFailure = {
// Fallback to user profile
userProfile?.displayName
}
)
}

val userAvatar by produceState(initialValue = roomMember?.avatarUrl) {
room.userAvatarUrl(roomMemberId).onSuccess { avatarUrl ->
if (avatarUrl != null) value = avatarUrl
}
val userAvatar: String? by produceState(
initialValue = roomMember?.avatarUrl ?: userProfile?.avatarUrl,
key1 = roomMember,
key2 = userProfile,
) {
value = room.userAvatarUrl(roomMemberId)
.fold(
onSuccess = { it },
onFailure = {
// Fallback to user profile
userProfile?.avatarUrl
}
)
}

return RoomMemberDetailsState(
Expand All @@ -124,36 +151,26 @@ class RoomMemberDetailsPresenter @AssistedInject constructor(
isBlocked = isBlocked.value,
startDmActionState = startDmActionState.value,
displayConfirmationDialog = confirmationDialog,
isCurrentUser = client.isMe(roomMember?.userId),
isCurrentUser = client.isMe(roomMemberId),
eventSink = ::handleEvents
)
}

private fun CoroutineScope.blockUser(userId: UserId, isBlockedState: MutableState<AsyncData<Boolean>>) = launch {
isBlockedState.value = AsyncData.Loading(false)
client.ignoreUser(userId)
.fold(
onSuccess = {
isBlockedState.value = AsyncData.Success(true)
room.getUpdatedMember(userId)
},
onFailure = {
isBlockedState.value = AsyncData.Failure(it, false)
}
)
.onFailure {
isBlockedState.value = AsyncData.Failure(it, false)
}
// Note: on success, ignoredUserList will be updated.
}

private fun CoroutineScope.unblockUser(userId: UserId, isBlockedState: MutableState<AsyncData<Boolean>>) = launch {
isBlockedState.value = AsyncData.Loading(true)
client.unignoreUser(userId)
.fold(
onSuccess = {
isBlockedState.value = AsyncData.Success(false)
room.getUpdatedMember(userId)
},
onFailure = {
isBlockedState.value = AsyncData.Failure(it, true)
}
)
.onFailure {
isBlockedState.value = AsyncData.Failure(it, true)
}
// Note: on success, ignoredUserList will be updated.
}
}
90 changes: 75 additions & 15 deletions ...o/element/android/features/roomdetails/members/details/RoomMemberDetailsPresenterTests.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package io.element.android.features.roomdetails.members.details

import app.cash.molecule.RecompositionMode
import app.cash.molecule.moleculeFlow
import app.cash.turbine.ReceiveTurbine
import app.cash.turbine.test
import com.google.common.truth.Truth.assertThat
import io.element.android.features.createroom.api.StartDMAction
Expand All @@ -30,12 +31,13 @@ import io.element.android.features.roomdetails.impl.members.details.RoomMemberDe
import io.element.android.libraries.architecture.AsyncAction
import io.element.android.libraries.architecture.AsyncData
import io.element.android.libraries.matrix.api.MatrixClient
import io.element.android.libraries.matrix.api.core.UserId
import io.element.android.libraries.matrix.api.room.MatrixRoom
import io.element.android.libraries.matrix.api.room.MatrixRoomMembersState
import io.element.android.libraries.matrix.api.room.RoomMember
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.FakeMatrixClient
import io.element.android.libraries.matrix.ui.components.aMatrixUser
import io.element.android.tests.testutils.WarmUpRule
import kotlinx.collections.immutable.persistentListOf
import kotlinx.coroutines.ExperimentalCoroutinesApi
Expand All @@ -58,12 +60,12 @@ class RoomMemberDetailsPresenterTests {
}
val presenter = createRoomMemberDetailsPresenter(
room = room,
roomMember = roomMember
roomMemberId = roomMember.userId
)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
val initialState = awaitItem()
val initialState = awaitFirstItem()
assertThat(initialState.userId).isEqualTo(roomMember.userId.value)
assertThat(initialState.userName).isEqualTo(roomMember.displayName)
assertThat(initialState.avatarUrl).isEqualTo(roomMember.avatarUrl)
Expand All @@ -85,12 +87,12 @@ class RoomMemberDetailsPresenterTests {
}
val presenter = createRoomMemberDetailsPresenter(
room = room,
roomMember = roomMember
roomMemberId = roomMember.userId
)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
val initialState = awaitItem()
val initialState = awaitFirstItem()
assertThat(initialState.userName).isEqualTo(roomMember.displayName)
assertThat(initialState.avatarUrl).isEqualTo(roomMember.avatarUrl)

Expand All @@ -108,26 +110,53 @@ class RoomMemberDetailsPresenterTests {
}
val presenter = createRoomMemberDetailsPresenter(
room = room,
roomMember = roomMember
roomMemberId = roomMember.userId
)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
val initialState = awaitItem()
val initialState = awaitFirstItem()
assertThat(initialState.userName).isEqualTo(roomMember.displayName)
assertThat(initialState.avatarUrl).isEqualTo(roomMember.avatarUrl)

ensureAllEventsConsumed()
}
}

@Test
fun `present - will fallback to user profile if user is not a member of the room`() = runTest {
val bobProfile = aMatrixUser("@bob:server.org", "Bob", avatarUrl = "anAvatarUrl")
val room = aMatrixRoom().apply {
givenUserDisplayNameResult(Result.failure(Exception("Not a member!")))
givenUserAvatarUrlResult(Result.failure(Exception("Not a member!")))
}
val client = FakeMatrixClient().apply {
givenGetProfileResult(bobProfile.userId, Result.success(bobProfile))
}
val presenter = createRoomMemberDetailsPresenter(
client = client,
room = room,
roomMemberId = UserId("@bob:server.org")
)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
skipItems(2)
val initialState = awaitFirstItem()
assertThat(initialState.userName).isEqualTo("Bob")
assertThat(initialState.avatarUrl).isEqualTo("anAvatarUrl")

ensureAllEventsConsumed()
}
}

@Test
fun `present - BlockUser needing confirmation displays confirmation dialog`() = runTest {
val presenter = createRoomMemberDetailsPresenter()
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
val initialState = awaitItem()
val initialState = awaitFirstItem()
initialState.eventSink(RoomMemberDetailsEvents.BlockUser(needsConfirmation = true))

val dialogState = awaitItem()
Expand All @@ -142,17 +171,24 @@ class RoomMemberDetailsPresenterTests {

@Test
fun `present - BlockUser and UnblockUser without confirmation change the 'blocked' state`() = runTest {
val presenter = createRoomMemberDetailsPresenter()
val client = FakeMatrixClient()
val roomMember = aRoomMember()
val presenter = createRoomMemberDetailsPresenter(
client = client,
roomMemberId = roomMember.userId
)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
val initialState = awaitItem()
val initialState = awaitFirstItem()
initialState.eventSink(RoomMemberDetailsEvents.BlockUser(needsConfirmation = false))
assertThat(awaitItem().isBlocked.isLoading()).isTrue()
client.emitIgnoreUserList(listOf(roomMember.userId))
assertThat(awaitItem().isBlocked.dataOrNull()).isTrue()

initialState.eventSink(RoomMemberDetailsEvents.UnblockUser(needsConfirmation = false))
assertThat(awaitItem().isBlocked.isLoading()).isTrue()
client.emitIgnoreUserList(listOf())
assertThat(awaitItem().isBlocked.dataOrNull()).isFalse()
}
}
Expand All @@ -165,7 +201,7 @@ class RoomMemberDetailsPresenterTests {
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
val initialState = awaitItem()
val initialState = awaitFirstItem()
initialState.eventSink(RoomMemberDetailsEvents.BlockUser(needsConfirmation = false))
assertThat(awaitItem().isBlocked.isLoading()).isTrue()
val errorState = awaitItem()
Expand All @@ -176,13 +212,32 @@ class RoomMemberDetailsPresenterTests {
}
}

@Test
fun `present - UnblockUser with error`() = runTest {
val matrixClient = FakeMatrixClient()
matrixClient.givenUnignoreUserResult(Result.failure(A_THROWABLE))
val presenter = createRoomMemberDetailsPresenter(client = matrixClient)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
val initialState = awaitFirstItem()
initialState.eventSink(RoomMemberDetailsEvents.UnblockUser(needsConfirmation = false))
assertThat(awaitItem().isBlocked.isLoading()).isTrue()
val errorState = awaitItem()
assertThat(errorState.isBlocked.errorOrNull()).isEqualTo(A_THROWABLE)
// Clear error
initialState.eventSink(RoomMemberDetailsEvents.ClearBlockUserError)
assertThat(awaitItem().isBlocked).isEqualTo(AsyncData.Success(true))
}
}

@Test
fun `present - UnblockUser needing confirmation displays confirmation dialog`() = runTest {
val presenter = createRoomMemberDetailsPresenter()
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
val initialState = awaitItem()
val initialState = awaitFirstItem()
initialState.eventSink(RoomMemberDetailsEvents.UnblockUser(needsConfirmation = true))

val dialogState = awaitItem()
Expand All @@ -202,7 +257,7 @@ class RoomMemberDetailsPresenterTests {
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
val initialState = awaitItem()
val initialState = awaitFirstItem()
assertThat(initialState.startDmActionState).isInstanceOf(AsyncAction.Uninitialized::class.java)
val startDMSuccessResult = AsyncAction.Success(A_ROOM_ID)
val startDMFailureResult = AsyncAction.Failure(A_THROWABLE)
Expand All @@ -229,14 +284,19 @@ class RoomMemberDetailsPresenterTests {
}
}

private suspend fun <T> ReceiveTurbine<T>.awaitFirstItem(): T {
skipItems(1)
return awaitItem()
}

private fun createRoomMemberDetailsPresenter(
client: MatrixClient = FakeMatrixClient(),
room: MatrixRoom = aMatrixRoom(),
roomMember: RoomMember = aRoomMember(),
roomMemberId: UserId = UserId("@alice:server.org"),
startDMAction: StartDMAction = FakeStartDMAction()
): RoomMemberDetailsPresenter {
return RoomMemberDetailsPresenter(
roomMemberId = roomMember.userId,
roomMemberId = roomMemberId,
client = client,
room = room,
startDMAction = startDMAction
Expand Down
5 changes: 5 additions & 0 deletions .../matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/FakeMatrixClient.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import io.element.android.libraries.matrix.test.verification.FakeSessionVerifica
import io.element.android.tests.testutils.simulateLongTask
import kotlinx.collections.immutable.ImmutableList
import kotlinx.collections.immutable.persistentListOf
import kotlinx.collections.immutable.toImmutableList
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.MutableStateFlow
Expand Down Expand Up @@ -205,6 +206,10 @@ class FakeMatrixClient(
return RoomMembershipObserver()
}

suspend fun emitIgnoreUserList(users: List<UserId>) {
ignoredUsersFlow.emit(users.toImmutableList())
}

// Mocks

fun givenLogoutError(failure: Throwable?) {
Expand Down

0 comments on commit e326824

Please sign in to comment.