Merge pull request #2930 from element-hq/feature/bma/blockedUserData

Render blocked user data (behind a disabled feature flag)
element-hq · May 28, 2024 · f74032d · f74032d
2 parents c511d59 + 21802be
commit f74032d
Show file tree

Hide file tree

Showing 23 changed files with 226 additions and 45 deletions.
diff --git a/changelog.d/2930.misc b/changelog.d/2930.misc
@@ -0,0 +1 @@
+Add a feature flag ShowBlockedUsersDetails, disabled by default to render display name and avatar of blocked users in the blocked users list.
diff --git a/...kotlin/io/element/android/features/preferences/impl/blockedusers/BlockedUsersPresenter.kt b/...kotlin/io/element/android/features/preferences/impl/blockedusers/BlockedUsersPresenter.kt
@@ -21,20 +21,26 @@ import androidx.compose.runtime.MutableState
 import androidx.compose.runtime.collectAsState
 import androidx.compose.runtime.getValue
 import androidx.compose.runtime.mutableStateOf
+import androidx.compose.runtime.produceState
 import androidx.compose.runtime.remember
 import androidx.compose.runtime.rememberCoroutineScope
 import androidx.compose.runtime.setValue
 import io.element.android.libraries.architecture.AsyncAction
 import io.element.android.libraries.architecture.Presenter
 import io.element.android.libraries.architecture.runUpdatingState
+import io.element.android.libraries.featureflag.api.FeatureFlagService
+import io.element.android.libraries.featureflag.api.FeatureFlags
 import io.element.android.libraries.matrix.api.MatrixClient
 import io.element.android.libraries.matrix.api.core.UserId
+import io.element.android.libraries.matrix.api.user.MatrixUser
+import kotlinx.collections.immutable.toPersistentList
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.launch
 import javax.inject.Inject
 
 class BlockedUsersPresenter @Inject constructor(
     private val matrixClient: MatrixClient,
+    private val featureFlagService: FeatureFlagService,
 ) : Presenter<BlockedUsersState> {
     @Composable
     override fun present(): BlockedUsersState {
@@ -47,7 +53,24 @@ class BlockedUsersPresenter @Inject constructor(
             mutableStateOf(AsyncAction.Uninitialized)
         }
 
+        val renderBlockedUsersDetail = featureFlagService
+            .isFeatureEnabledFlow(FeatureFlags.ShowBlockedUsersDetails)
+            .collectAsState(initial = false)
         val ignoredUserIds by matrixClient.ignoredUsersFlow.collectAsState()
+        val ignoredMatrixUser by produceState(
+            initialValue = ignoredUserIds.map { MatrixUser(userId = it) },
+            key1 = renderBlockedUsersDetail.value,
+            key2 = ignoredUserIds
+        ) {
+            value = ignoredUserIds.map {
+                if (renderBlockedUsersDetail.value) {
+                    matrixClient.getProfile(it).getOrNull()
+                } else {
+                    null
+                }
+                    ?: MatrixUser(userId = it)
+            }
+        }
 
         fun handleEvents(event: BlockedUsersEvents) {
             when (event) {
@@ -68,7 +91,7 @@ class BlockedUsersPresenter @Inject constructor(
             }
         }
         return BlockedUsersState(
-            blockedUsers = ignoredUserIds,
+            blockedUsers = ignoredMatrixUser.toPersistentList(),
             unblockUserAction = unblockUserAction.value,
             eventSink = ::handleEvents
         )

diff --git a/...ain/kotlin/io/element/android/features/preferences/impl/blockedusers/BlockedUsersState.kt b/...ain/kotlin/io/element/android/features/preferences/impl/blockedusers/BlockedUsersState.kt
@@ -17,11 +17,11 @@
 package io.element.android.features.preferences.impl.blockedusers
 
 import io.element.android.libraries.architecture.AsyncAction
-import io.element.android.libraries.matrix.api.core.UserId
+import io.element.android.libraries.matrix.api.user.MatrixUser
 import kotlinx.collections.immutable.ImmutableList
 
 data class BlockedUsersState(
-    val blockedUsers: ImmutableList<UserId>,
+    val blockedUsers: ImmutableList<MatrixUser>,
     val unblockUserAction: AsyncAction<Unit>,
     val eventSink: (BlockedUsersEvents) -> Unit,
 )
diff --git a/...lement/android/features/preferences/impl/blockedusers/BlockedUsersStatePreviewProvider.kt b/...lement/android/features/preferences/impl/blockedusers/BlockedUsersStatePreviewProvider.kt
@@ -18,31 +18,31 @@ package io.element.android.features.preferences.impl.blockedusers
 
 import androidx.compose.ui.tooling.preview.PreviewParameterProvider
 import io.element.android.libraries.architecture.AsyncAction
-import io.element.android.libraries.matrix.api.core.UserId
+import io.element.android.libraries.matrix.api.user.MatrixUser
 import io.element.android.libraries.matrix.ui.components.aMatrixUserList
 import kotlinx.collections.immutable.toPersistentList
 
 class BlockedUsersStatePreviewProvider : PreviewParameterProvider<BlockedUsersState> {
     override val values: Sequence<BlockedUsersState>
         get() = sequenceOf(
             aBlockedUsersState(),
+            aBlockedUsersState(blockedUsers = aMatrixUserList().map { it.copy(displayName = null, avatarUrl = null) }),
             aBlockedUsersState(blockedUsers = emptyList()),
             aBlockedUsersState(unblockUserAction = AsyncAction.Confirming),
-            // Sadly there's no good way to preview Loading or Failure states since they're presented with an animation
-            // All these 3 screen states will be displayed as the Uninitialized one
             aBlockedUsersState(unblockUserAction = AsyncAction.Loading),
             aBlockedUsersState(unblockUserAction = AsyncAction.Failure(Throwable("Failed to unblock user"))),
             aBlockedUsersState(unblockUserAction = AsyncAction.Success(Unit)),
         )
 }
 
 internal fun aBlockedUsersState(
-    blockedUsers: List<UserId> = aMatrixUserList().map { it.userId },
+    blockedUsers: List<MatrixUser> = aMatrixUserList(),
     unblockUserAction: AsyncAction<Unit> = AsyncAction.Uninitialized,
+    eventSink: (BlockedUsersEvents) -> Unit = {},
 ): BlockedUsersState {
     return BlockedUsersState(
         blockedUsers = blockedUsers.toPersistentList(),
         unblockUserAction = unblockUserAction,
-        eventSink = {},
+        eventSink = eventSink,
     )
 }
diff --git a/...main/kotlin/io/element/android/features/preferences/impl/blockedusers/BlockedUsersView.kt b/...main/kotlin/io/element/android/features/preferences/impl/blockedusers/BlockedUsersView.kt
@@ -73,9 +73,9 @@ fun BlockedUsersView(
             LazyColumn(
                 modifier = Modifier.padding(padding)
             ) {
-                items(state.blockedUsers) { userId ->
+                items(state.blockedUsers) { matrixUser ->
                     BlockedUserItem(
-                        userId = userId,
+                        matrixUser = matrixUser,
                         onClick = { state.eventSink(BlockedUsersEvents.Unblock(it)) }
                     )
                 }
@@ -121,12 +121,12 @@ fun BlockedUsersView(
 
 @Composable
 private fun BlockedUserItem(
-    userId: UserId,
+    matrixUser: MatrixUser,
     onClick: (UserId) -> Unit,
 ) {
     MatrixUserRow(
-        modifier = Modifier.clickable { onClick(userId) },
-        matrixUser = MatrixUser(userId),
+        modifier = Modifier.clickable { onClick(matrixUser.userId) },
+        matrixUser = matrixUser,
     )
 }
 

diff --git a/...t/kotlin/io/element/android/features/preferences/impl/blockedusers/BlockedUserViewTest.kt b/...t/kotlin/io/element/android/features/preferences/impl/blockedusers/BlockedUserViewTest.kt
@@ -0,0 +1,109 @@
+/*
+ * Copyright (c) 2024 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.element.android.features.preferences.impl.blockedusers
+
+import androidx.activity.ComponentActivity
+import androidx.compose.ui.test.junit4.AndroidComposeTestRule
+import androidx.compose.ui.test.junit4.createAndroidComposeRule
+import androidx.compose.ui.test.onNodeWithText
+import androidx.compose.ui.test.performClick
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import io.element.android.features.preferences.impl.R
+import io.element.android.libraries.architecture.AsyncAction
+import io.element.android.libraries.matrix.ui.components.aMatrixUserList
+import io.element.android.libraries.ui.strings.CommonStrings
+import io.element.android.tests.testutils.EnsureNeverCalled
+import io.element.android.tests.testutils.EventsRecorder
+import io.element.android.tests.testutils.clickOn
+import io.element.android.tests.testutils.ensureCalledOnce
+import io.element.android.tests.testutils.pressBack
+import org.junit.Rule
+import org.junit.Test
+import org.junit.rules.TestRule
+import org.junit.runner.RunWith
+
+@RunWith(AndroidJUnit4::class)
+class BlockedUserViewTest {
+    @get:Rule
+    val rule = createAndroidComposeRule<ComponentActivity>()
+
+    @Test
+    fun `clicking on back invokes back callback`() {
+        val eventsRecorder = EventsRecorder<BlockedUsersEvents>(expectEvents = false)
+        ensureCalledOnce { callback ->
+            rule.setLogoutView(
+                aBlockedUsersState(
+                    eventSink = eventsRecorder
+                ),
+                onBackClicked = callback,
+            )
+            rule.pressBack()
+        }
+    }
+
+    @Test
+    fun `clicking on a user emits the expected Event`() {
+        val eventsRecorder = EventsRecorder<BlockedUsersEvents>()
+        val userList = aMatrixUserList()
+        rule.setLogoutView(
+            aBlockedUsersState(
+                blockedUsers = userList,
+                eventSink = eventsRecorder
+            ),
+        )
+        rule.onNodeWithText(userList.first().displayName.orEmpty()).performClick()
+        eventsRecorder.assertSingle(BlockedUsersEvents.Unblock(userList.first().userId))
+    }
+
+    @Test
+    fun `clicking on cancel sends a BlockedUsersEvents`() {
+        val eventsRecorder = EventsRecorder<BlockedUsersEvents>()
+        rule.setLogoutView(
+            aBlockedUsersState(
+                unblockUserAction = AsyncAction.Confirming,
+                eventSink = eventsRecorder
+            ),
+        )
+        rule.clickOn(CommonStrings.action_cancel)
+        eventsRecorder.assertSingle(BlockedUsersEvents.Cancel)
+    }
+
+    @Test
+    fun `clicking on confirm sends a BlockedUsersEvents`() {
+        val eventsRecorder = EventsRecorder<BlockedUsersEvents>()
+        rule.setLogoutView(
+            aBlockedUsersState(
+                unblockUserAction = AsyncAction.Confirming,
+                eventSink = eventsRecorder
+            ),
+        )
+        rule.clickOn(R.string.screen_blocked_users_unblock_alert_action)
+        eventsRecorder.assertSingle(BlockedUsersEvents.ConfirmUnblock)
+    }
+}
+
+private fun <R : TestRule> AndroidComposeTestRule<R, ComponentActivity>.setLogoutView(
+    state: BlockedUsersState,
+    onBackClicked: () -> Unit = EnsureNeverCalled(),
+) {
+    setContent {
+        BlockedUsersView(
+            state = state,
+            onBackPressed = onBackClicked,
+        )
+    }
+}
diff --git a/...n/io/element/android/features/preferences/impl/blockedusers/BlockedUsersPresenterTests.kt b/...n/io/element/android/features/preferences/impl/blockedusers/BlockedUsersPresenterTests.kt
@@ -21,6 +21,11 @@ import app.cash.molecule.moleculeFlow
 import app.cash.turbine.test
 import com.google.common.truth.Truth.assertThat
 import io.element.android.libraries.architecture.AsyncAction
+import io.element.android.libraries.featureflag.api.FeatureFlagService
+import io.element.android.libraries.featureflag.api.FeatureFlags
+import io.element.android.libraries.featureflag.test.FakeFeatureFlagService
+import io.element.android.libraries.matrix.api.user.MatrixUser
+import io.element.android.libraries.matrix.test.AN_EXCEPTION
 import io.element.android.libraries.matrix.test.A_USER_ID
 import io.element.android.libraries.matrix.test.A_USER_ID_2
 import io.element.android.libraries.matrix.test.FakeMatrixClient
@@ -52,7 +57,7 @@ class BlockedUsersPresenterTests {
             presenter.present()
         }.test {
             with(awaitItem()) {
-                assertThat(blockedUsers).isEqualTo(persistentListOf(A_USER_ID))
+                assertThat(blockedUsers).isEqualTo(persistentListOf(MatrixUser(A_USER_ID)))
                 assertThat(unblockUserAction).isEqualTo(AsyncAction.Uninitialized)
             }
         }
@@ -68,14 +73,39 @@ class BlockedUsersPresenterTests {
             presenter.present()
         }.test {
             with(awaitItem()) {
-                assertThat(blockedUsers).containsAtLeastElementsIn(persistentListOf(A_USER_ID))
-                assertThat(unblockUserAction).isEqualTo(AsyncAction.Uninitialized)
+                assertThat(blockedUsers).isEqualTo(listOf(MatrixUser(A_USER_ID)))
             }
-
             matrixClient.ignoredUsersFlow.value = persistentListOf(A_USER_ID, A_USER_ID_2)
+            skipItems(1)
             with(awaitItem()) {
-                assertThat(blockedUsers).isEqualTo(persistentListOf(A_USER_ID, A_USER_ID_2))
-                assertThat(unblockUserAction).isEqualTo(AsyncAction.Uninitialized)
+                assertThat(blockedUsers).isEqualTo(listOf(MatrixUser(A_USER_ID), MatrixUser(A_USER_ID_2)))
+            }
+        }
+    }
+
+    @Test
+    fun `present - blocked users list with data`() = runTest {
+        val alice = MatrixUser(A_USER_ID, displayName = "Alice", avatarUrl = "aliceAvatar")
+        val matrixClient = FakeMatrixClient().apply {
+            ignoredUsersFlow.value = persistentListOf(A_USER_ID, A_USER_ID_2)
+            givenGetProfileResult(A_USER_ID, Result.success(alice))
+            givenGetProfileResult(A_USER_ID_2, Result.failure(AN_EXCEPTION))
+        }
+        val presenter = aBlockedUsersPresenter(
+            matrixClient = matrixClient,
+            featureFlagService = FakeFeatureFlagService().apply {
+                setFeatureEnabled(FeatureFlags.ShowBlockedUsersDetails, true)
+            }
+        )
+        moleculeFlow(RecompositionMode.Immediate) {
+            presenter.present()
+        }.test {
+            with(awaitItem()) {
+                assertThat(blockedUsers).isEqualTo(listOf(MatrixUser(A_USER_ID), MatrixUser(A_USER_ID_2)))
+            }
+            // Alice is resolved
+            with(awaitItem()) {
+                assertThat(blockedUsers).isEqualTo(listOf(alice, MatrixUser(A_USER_ID_2)))
             }
         }
     }
@@ -157,5 +187,9 @@ class BlockedUsersPresenterTests {
 
     private fun aBlockedUsersPresenter(
         matrixClient: FakeMatrixClient = FakeMatrixClient(),
-    ) = BlockedUsersPresenter(matrixClient)
+        featureFlagService: FeatureFlagService = FakeFeatureFlagService(),
+    ) = BlockedUsersPresenter(
+        matrixClient = matrixClient,
+        featureFlagService = featureFlagService,
+    )
 }
diff --git a/...tureflag/api/src/main/kotlin/io/element/android/libraries/featureflag/api/FeatureFlags.kt b/...tureflag/api/src/main/kotlin/io/element/android/libraries/featureflag/api/FeatureFlags.kt
@@ -81,5 +81,12 @@ enum class FeatureFlags(
         description = "Allow user to search for public rooms in their homeserver",
         defaultValue = false,
         isFinished = false,
-    )
+    ),
+    ShowBlockedUsersDetails(
+        key = "feature.showBlockedUsersDetails",
+        title = "Show blocked users details",
+        description = "Show the name and avatar of blocked users in the blocked users list",
+        defaultValue = false,
+        isFinished = false,
+    ),
 }
diff --git a/...rc/main/kotlin/io/element/android/libraries/featureflag/impl/StaticFeatureFlagProvider.kt b/...rc/main/kotlin/io/element/android/libraries/featureflag/impl/StaticFeatureFlagProvider.kt
@@ -41,6 +41,7 @@ class StaticFeatureFlagProvider @Inject constructor() :
                 FeatureFlags.Mentions -> true
                 FeatureFlags.MarkAsUnread -> true
                 FeatureFlags.RoomDirectorySearch -> false
+                FeatureFlags.ShowBlockedUsersDetails -> false
             }
         } else {
             false

diff --git a/...users_BlockedUsersView_null_BlockedUsersView-Day-3_4_null_0,NEXUS_5,1.0,en].png b/...users_BlockedUsersView_null_BlockedUsersView-Day-3_4_null_0,NEXUS_5,1.0,en].png
diff --git a/...users_BlockedUsersView_null_BlockedUsersView-Day-3_4_null_1,NEXUS_5,1.0,en].png b/...users_BlockedUsersView_null_BlockedUsersView-Day-3_4_null_1,NEXUS_5,1.0,en].png
diff --git a/...users_BlockedUsersView_null_BlockedUsersView-Day-3_4_null_2,NEXUS_5,1.0,en].png b/...users_BlockedUsersView_null_BlockedUsersView-Day-3_4_null_2,NEXUS_5,1.0,en].png
diff --git a/...users_BlockedUsersView_null_BlockedUsersView-Day-3_4_null_3,NEXUS_5,1.0,en].png b/...users_BlockedUsersView_null_BlockedUsersView-Day-3_4_null_3,NEXUS_5,1.0,en].png
diff --git a/...users_BlockedUsersView_null_BlockedUsersView-Day-3_4_null_4,NEXUS_5,1.0,en].png b/...users_BlockedUsersView_null_BlockedUsersView-Day-3_4_null_4,NEXUS_5,1.0,en].png
diff --git a/...users_BlockedUsersView_null_BlockedUsersView-Day-3_4_null_5,NEXUS_5,1.0,en].png b/...users_BlockedUsersView_null_BlockedUsersView-Day-3_4_null_5,NEXUS_5,1.0,en].png
diff --git a/...users_BlockedUsersView_null_BlockedUsersView-Day-3_4_null_6,NEXUS_5,1.0,en].png b/...users_BlockedUsersView_null_BlockedUsersView-Day-3_4_null_6,NEXUS_5,1.0,en].png
diff --git a/...ers_BlockedUsersView_null_BlockedUsersView-Night-3_5_null_0,NEXUS_5,1.0,en].png b/...ers_BlockedUsersView_null_BlockedUsersView-Night-3_5_null_0,NEXUS_5,1.0,en].png
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Add a feature flag ShowBlockedUsersDetails, disabled by default to render display name and avatar of blocked users in the blocked users list.