Merge pull request #847 from vector-im/feature/bma/blockUserUx

bmarty · web-flow · commit 281d0dde5650 · 2023-07-12T16:33:12.000+02:00
Improve block/unblock user ux
diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/blockuser/BlockUserSection.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/blockuser/BlockUserSection.kt
@@ -25,28 +25,65 @@ import androidx.compose.ui.res.stringResource
 import io.element.android.features.roomdetails.impl.R
 import io.element.android.features.roomdetails.impl.members.details.RoomMemberDetailsEvents
 import io.element.android.features.roomdetails.impl.members.details.RoomMemberDetailsState
+import io.element.android.libraries.architecture.Async
+import io.element.android.libraries.core.bool.orFalse
 import io.element.android.libraries.designsystem.components.dialogs.ConfirmationDialog
+import io.element.android.libraries.designsystem.components.dialogs.RetryDialog
 import io.element.android.libraries.designsystem.components.preferences.PreferenceCategory
 import io.element.android.libraries.designsystem.components.preferences.PreferenceText
+import io.element.android.libraries.ui.strings.CommonStrings
 
 @Composable
 internal fun BlockUserSection(state: RoomMemberDetailsState, modifier: Modifier = Modifier) {
     PreferenceCategory(showDivider = false, modifier = modifier) {
-        if (state.isBlocked) {
-            PreferenceText(
-                title = stringResource(R.string.screen_dm_details_unblock_user),
-                icon = Icons.Outlined.Block,
-                onClick = { state.eventSink(RoomMemberDetailsEvents.UnblockUser(needsConfirmation = true)) },
-            )
-        } else {
-            PreferenceText(
-                title = stringResource(R.string.screen_dm_details_block_user),
-                icon = Icons.Outlined.Block,
-                tintColor = MaterialTheme.colorScheme.error,
-                onClick = { state.eventSink(RoomMemberDetailsEvents.BlockUser(needsConfirmation = true)) },
-            )
+        when (state.isBlocked) {
+            is Async.Failure -> PreferenceBlockUser(isBlocked = state.isBlocked.prevData, isLoading = false, eventSink = state.eventSink)
+            is Async.Loading -> PreferenceBlockUser(isBlocked = state.isBlocked.prevData, isLoading = true, eventSink = state.eventSink)
+            is Async.Success -> PreferenceBlockUser(isBlocked = state.isBlocked.data, isLoading = false, eventSink = state.eventSink)
+            Async.Uninitialized -> PreferenceBlockUser(isBlocked = null, isLoading = true, eventSink = state.eventSink)
         }
     }
+    if (state.isBlocked is Async.Failure) {
+        RetryDialog(
+            content = stringResource(CommonStrings.error_unknown),
+            onDismiss = { state.eventSink(RoomMemberDetailsEvents.ClearBlockUserError) },
+            onRetry = {
+                val event = when (state.isBlocked.prevData) {
+                    true -> RoomMemberDetailsEvents.UnblockUser(needsConfirmation = false)
+                    false -> RoomMemberDetailsEvents.BlockUser(needsConfirmation = false)
+                    null -> /*Should not happen */ RoomMemberDetailsEvents.ClearBlockUserError
+                }
+                state.eventSink(event)
+            },
+        )
+    }
+}
+
+@Composable
+private fun PreferenceBlockUser(
+    isBlocked: Boolean?,
+    isLoading: Boolean,
+    eventSink: (RoomMemberDetailsEvents) -> Unit,
+    modifier: Modifier = Modifier,
+) {
+    if (isBlocked.orFalse()) {
+        PreferenceText(
+            title = stringResource(R.string.screen_dm_details_unblock_user),
+            icon = Icons.Outlined.Block,
+            onClick = { if (!isLoading) eventSink(RoomMemberDetailsEvents.UnblockUser(needsConfirmation = true)) },
+            loadingCurrentValue = isLoading,
+            modifier = modifier,
+        )
+    } else {
+        PreferenceText(
+            title = stringResource(R.string.screen_dm_details_block_user),
+            icon = Icons.Outlined.Block,
+            tintColor = MaterialTheme.colorScheme.error,
+            onClick = { if (!isLoading) eventSink(RoomMemberDetailsEvents.BlockUser(needsConfirmation = true)) },
+            loadingCurrentValue = isLoading,
+            modifier = modifier,
+        )
+    }
 }
 
 @Composable
diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsEvents.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsEvents.kt
@@ -19,5 +19,6 @@ package io.element.android.features.roomdetails.impl.members.details
 sealed interface RoomMemberDetailsEvents {
     data class BlockUser(val needsConfirmation: Boolean = false) : RoomMemberDetailsEvents
     data class UnblockUser(val needsConfirmation: Boolean = false) : RoomMemberDetailsEvents
+    object ClearBlockUserError : RoomMemberDetailsEvents
     object ClearConfirmationDialog : RoomMemberDetailsEvents
 }
diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsPresenter.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsPresenter.kt
@@ -28,6 +28,7 @@ import androidx.compose.runtime.setValue
 import dagger.assisted.Assisted
 import dagger.assisted.AssistedInject
 import io.element.android.features.roomdetails.impl.members.details.RoomMemberDetailsState.ConfirmationDialog
+import io.element.android.libraries.architecture.Async
 import io.element.android.libraries.architecture.Presenter
 import io.element.android.libraries.core.bool.orFalse
 import io.element.android.libraries.matrix.api.MatrixClient
@@ -53,8 +54,13 @@ class RoomMemberDetailsPresenter @AssistedInject constructor(
         var confirmationDialog by remember { mutableStateOf<ConfirmationDialog?>(null) }
         val roomMember by room.getRoomMemberAsState(roomMemberId)
         // the room member is not really live...
-        val isBlocked = remember {
-            mutableStateOf(roomMember?.isIgnored.orFalse())
+        val isBlocked: MutableState<Async<Boolean>> = remember(roomMember) {
+            val isIgnored = roomMember?.isIgnored
+            if (isIgnored == null) {
+                mutableStateOf(Async.Uninitialized)
+            } else {
+                mutableStateOf(Async.Success(isIgnored))
+            }
         }
         LaunchedEffect(Unit) {
             room.updateMembers()
@@ -79,6 +85,9 @@ class RoomMemberDetailsPresenter @AssistedInject constructor(
                     }
                 }
                 RoomMemberDetailsEvents.ClearConfirmationDialog -> confirmationDialog = null
+                RoomMemberDetailsEvents.ClearBlockUserError -> {
+                    isBlocked.value = Async.Success(isBlocked.value.dataOrNull().orFalse())
+                }
             }
         }
 
@@ -105,20 +114,31 @@ class RoomMemberDetailsPresenter @AssistedInject constructor(
         )
     }
 
-    private fun CoroutineScope.blockUser(userId: UserId, isBlockedState: MutableState<Boolean>) = launch {
+    private fun CoroutineScope.blockUser(userId: UserId, isBlockedState: MutableState<Async<Boolean>>) = launch {
+        isBlockedState.value = Async.Loading(false)
         client.ignoreUser(userId)
-            .map {
-                isBlockedState.value = true
-                room.updateMembers()
-            }
-
+            .fold(
+                onSuccess = {
+                    isBlockedState.value = Async.Success(true)
+                    room.updateMembers()
+                },
+                onFailure = {
+                    isBlockedState.value = Async.Failure(it, false)
+                }
+            )
     }
 
-    private fun CoroutineScope.unblockUser(userId: UserId, isBlockedState: MutableState<Boolean>) = launch {
+    private fun CoroutineScope.unblockUser(userId: UserId, isBlockedState: MutableState<Async<Boolean>>) = launch {
+        isBlockedState.value = Async.Loading(true)
         client.unignoreUser(userId)
-            .map {
-                isBlockedState.value = false
-                room.updateMembers()
-            }
+            .fold(
+                onSuccess = {
+                    isBlockedState.value = Async.Success(false)
+                    room.updateMembers()
+                },
+                onFailure = {
+                    isBlockedState.value = Async.Failure(it, true)
+                }
+            )
     }
 }
diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsState.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsState.kt
@@ -16,11 +16,13 @@
 
 package io.element.android.features.roomdetails.impl.members.details
 
+import io.element.android.libraries.architecture.Async
+
 data class RoomMemberDetailsState(
     val userId: String,
     val userName: String?,
     val avatarUrl: String?,
-    val isBlocked: Boolean,
+    val isBlocked: Async<Boolean>,
     val displayConfirmationDialog: ConfirmationDialog? = null,
     val isCurrentUser: Boolean,
     val eventSink: (RoomMemberDetailsEvents) -> Unit
diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsStateProvider.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsStateProvider.kt
@@ -17,15 +17,17 @@
 package io.element.android.features.roomdetails.impl.members.details
 
 import androidx.compose.ui.tooling.preview.PreviewParameterProvider
+import io.element.android.libraries.architecture.Async
 
 open class RoomMemberDetailsStateProvider : PreviewParameterProvider<RoomMemberDetailsState> {
     override val values: Sequence<RoomMemberDetailsState>
         get() = sequenceOf(
             aRoomMemberDetailsState(),
             aRoomMemberDetailsState().copy(userName = null),
-            aRoomMemberDetailsState().copy(isBlocked = true),
+            aRoomMemberDetailsState().copy(isBlocked = Async.Success(true)),
             aRoomMemberDetailsState().copy(displayConfirmationDialog = RoomMemberDetailsState.ConfirmationDialog.Block),
             aRoomMemberDetailsState().copy(displayConfirmationDialog = RoomMemberDetailsState.ConfirmationDialog.Unblock),
+            aRoomMemberDetailsState().copy(isBlocked = Async.Loading(true)),
             // Add other states here
         )
 }
@@ -34,7 +36,7 @@ fun aRoomMemberDetailsState() = RoomMemberDetailsState(
     userId = "@daniel:domain.com",
     userName = "Daniel",
     avatarUrl = null,
-    isBlocked = false,
+    isBlocked = Async.Success(false),
     isCurrentUser = false,
     eventSink = {},
 )
diff --git a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/members/details/RoomMemberDetailsPresenterTests.kt b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/members/details/RoomMemberDetailsPresenterTests.kt
@@ -25,7 +25,9 @@ import io.element.android.features.roomdetails.impl.members.aRoomMember
 import io.element.android.features.roomdetails.impl.members.details.RoomMemberDetailsEvents
 import io.element.android.features.roomdetails.impl.members.details.RoomMemberDetailsPresenter
 import io.element.android.features.roomdetails.impl.members.details.RoomMemberDetailsState
+import io.element.android.libraries.architecture.Async
 import io.element.android.libraries.matrix.api.room.MatrixRoomMembersState
+import io.element.android.libraries.matrix.test.A_THROWABLE
 import io.element.android.libraries.matrix.test.FakeMatrixClient
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.test.runTest
@@ -50,7 +52,7 @@ class RoomMemberDetailsPresenterTests {
             Truth.assertThat(initialState.userId).isEqualTo(roomMember.userId.value)
             Truth.assertThat(initialState.userName).isEqualTo(roomMember.displayName)
             Truth.assertThat(initialState.avatarUrl).isEqualTo(roomMember.avatarUrl)
-            Truth.assertThat(initialState.isBlocked).isEqualTo(roomMember.isIgnored)
+            Truth.assertThat(initialState.isBlocked).isEqualTo(Async.Success(roomMember.isIgnored))
             skipItems(1)
             val loadedState = awaitItem()
             Truth.assertThat(loadedState.userName).isEqualTo("A custom name")
@@ -129,10 +131,33 @@ class RoomMemberDetailsPresenterTests {
         }.test {
             val initialState = awaitItem()
             initialState.eventSink(RoomMemberDetailsEvents.BlockUser(needsConfirmation = false))
-            Truth.assertThat(awaitItem().isBlocked).isTrue()
+            Truth.assertThat(awaitItem().isBlocked.isLoading()).isTrue()
+            Truth.assertThat(awaitItem().isBlocked.dataOrNull()).isTrue()
 
             initialState.eventSink(RoomMemberDetailsEvents.UnblockUser(needsConfirmation = false))
-            Truth.assertThat(awaitItem().isBlocked).isFalse()
+            Truth.assertThat(awaitItem().isBlocked.isLoading()).isTrue()
+            Truth.assertThat(awaitItem().isBlocked.dataOrNull()).isFalse()
+        }
+    }
+
+    @Test
+    fun `present - BlockUser with error`() = runTest {
+        val room = aMatrixRoom()
+        val roomMember = aRoomMember()
+        val matrixClient = FakeMatrixClient()
+        matrixClient.givenIgnoreUserResult(Result.failure(A_THROWABLE))
+        val presenter = RoomMemberDetailsPresenter(matrixClient, room, roomMember.userId)
+        moleculeFlow(RecompositionClock.Immediate) {
+            presenter.present()
+        }.test {
+            val initialState = awaitItem()
+            initialState.eventSink(RoomMemberDetailsEvents.BlockUser(needsConfirmation = false))
+            Truth.assertThat(awaitItem().isBlocked.isLoading()).isTrue()
+            val errorState = awaitItem()
+            Truth.assertThat(errorState.isBlocked.errorOrNull()).isEqualTo(A_THROWABLE)
+            // Clear error
+            initialState.eventSink(RoomMemberDetailsEvents.ClearBlockUserError)
+            Truth.assertThat(awaitItem().isBlocked).isEqualTo(Async.Success(false))
         }
     }
 
diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/FakeMatrixClient.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/FakeMatrixClient.kt
@@ -38,6 +38,7 @@ import io.element.android.libraries.matrix.test.room.FakeMatrixRoom
 import io.element.android.libraries.matrix.test.room.FakeRoomSummaryDataSource
 import io.element.android.libraries.matrix.test.sync.FakeSyncService
 import io.element.android.libraries.matrix.test.verification.FakeSessionVerificationService
+import io.element.android.tests.testutils.simulateLongTask
 import kotlinx.coroutines.delay
 
 class FakeMatrixClient(
@@ -72,11 +73,11 @@ class FakeMatrixClient(
         return findDmResult
     }
 
-    override suspend fun ignoreUser(userId: UserId): Result<Unit> {
+    override suspend fun ignoreUser(userId: UserId): Result<Unit> = simulateLongTask {
         return ignoreUserResult
     }
 
-    override suspend fun unignoreUser(userId: UserId): Result<Unit> {
+    override suspend fun unignoreUser(userId: UserId): Result<Unit> = simulateLongTask {
         return unignoreUserResult
     }
 
diff --git a/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.features.roomdetails.impl.members.details_null_DefaultGroup_RoomMemberDetailsViewDarkPreview--3_3_null_5,NEXUS_5,1.0,en].png b/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.features.roomdetails.impl.members.details_null_DefaultGroup_RoomMemberDetailsViewDarkPreview--3_3_null_5,NEXUS_5,1.0,en].png
@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:c7ca068387cff8faf728a989488e7c4b5b07983c4b24162ff82a14fd90b82d05
+size 20609
diff --git a/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.features.roomdetails.impl.members.details_null_DefaultGroup_RoomMemberDetailsViewLightPreview--2_2_null_5,NEXUS_5,1.0,en].png b/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.features.roomdetails.impl.members.details_null_DefaultGroup_RoomMemberDetailsViewLightPreview--2_2_null_5,NEXUS_5,1.0,en].png
@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:e1599be0c5a37083c015378ee47a78a90dcf670f4df5d85dd25d39f4b2edbdf6
+size 21147

Original file line number	Diff line number	Diff line change
`@@ -19,5 +19,6 @@ package io.element.android.features.roomdetails.impl.members.details`
`19`	`19`	`sealed interface RoomMemberDetailsEvents {`
`20`	`20`	`data class BlockUser(val needsConfirmation: Boolean = false) : RoomMemberDetailsEvents`
`21`	`21`	`data class UnblockUser(val needsConfirmation: Boolean = false) : RoomMemberDetailsEvents`
	`22`	`+ object ClearBlockUserError : RoomMemberDetailsEvents`
`22`	`23`	`object ClearConfirmationDialog : RoomMemberDetailsEvents`
`23`	`24`	`}`
Original file line number	Diff line number	Diff line change
`@@ -38,6 +38,7 @@ import io.element.android.libraries.matrix.test.room.FakeMatrixRoom`
`38`	`38`	`import io.element.android.libraries.matrix.test.room.FakeRoomSummaryDataSource`
`39`	`39`	`import io.element.android.libraries.matrix.test.sync.FakeSyncService`
`40`	`40`	`import io.element.android.libraries.matrix.test.verification.FakeSessionVerificationService`
	`41`	`+import io.element.android.tests.testutils.simulateLongTask`
`41`	`42`	`import kotlinx.coroutines.delay`
`42`	`43`
`43`	`44`	`class FakeMatrixClient(`
`@@ -72,11 +73,11 @@ class FakeMatrixClient(`
`72`	`73`	`return findDmResult`
`73`	`74`	`}`
`74`	`75`
`75`		`- override suspend fun ignoreUser(userId: UserId): Result<Unit> {`
	`76`	`+ override suspend fun ignoreUser(userId: UserId): Result<Unit> = simulateLongTask {`
`76`	`77`	`return ignoreUserResult`
`77`	`78`	`}`
`78`	`79`
`79`		`- override suspend fun unignoreUser(userId: UserId): Result<Unit> {`
	`80`	`+ override suspend fun unignoreUser(userId: UserId): Result<Unit> = simulateLongTask {`
`80`	`81`	`return unignoreUserResult`
`81`	`82`	`}`
`82`	`83`