Merge pull request #797 from vector-im/feature/cjs/hide-self-in-search

Author: csmith

features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsPresenter.kt

@@ -100,7 +100,7 @@ class RoomMemberDetailsPresenter @AssistedInject constructor(
             avatarUrl = userAvatar,
             isBlocked = isBlocked.value,
             displayConfirmationDialog = confirmationDialog,
-            isCurrentUser = roomMember?.userId == client.sessionId,
+            isCurrentUser = client.isMe(roomMember?.userId),
             eventSink = ::handleEvents
         )
     }

libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLastMessageFormatter.kt

@@ -62,7 +62,7 @@ class DefaultRoomLastMessageFormatter @Inject constructor(
 ) : RoomLastMessageFormatter {
 
     override fun format(event: EventTimelineItem, isDmRoom: Boolean): CharSequence? {
-        val isOutgoing = event.sender == matrixClient.sessionId
+        val isOutgoing = matrixClient.isMe(event.sender)
         val senderDisplayName = (event.senderProfile as? ProfileTimelineDetails.Ready)?.displayName ?: event.sender.value
         return when (val content = event.content) {
             is MessageContent -> processMessageContents(content, senderDisplayName, isDmRoom)

libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultTimelineEventFormatter.kt

@@ -49,7 +49,7 @@ class DefaultTimelineEventFormatter @Inject constructor(
 ) : TimelineEventFormatter {
 
     override fun format(event: EventTimelineItem): CharSequence? {
-        val isOutgoing = event.sender == matrixClient.sessionId
+        val isOutgoing = matrixClient.isMe(event.sender)
         val senderDisplayName = (event.senderProfile as? ProfileTimelineDetails.Ready)?.displayName ?: event.sender.value
         return when (val content = event.content) {
             is RoomMembershipContent -> {

libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/RoomMembershipContentFormatter.kt

@@ -33,7 +33,7 @@ class RoomMembershipContentFormatter @Inject constructor(
         senderIsYou: Boolean,
     ): CharSequence? {
         val userId = membershipContent.userId
-        val memberIsYou = userId == matrixClient.sessionId
+        val memberIsYou = matrixClient.isMe(userId)
         return when (val change = membershipContent.change) {
             MembershipChange.JOINED -> if (memberIsYou) {
                 sp.getString(R.string.state_event_room_join_by_you)

libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/MatrixClient.kt

@@ -62,4 +62,6 @@ interface MatrixClient : Closeable {
     suspend fun loadUserAvatarURLString(): Result<String?>
     suspend fun uploadMedia(mimeType: String, data: ByteArray, progressCallback: ProgressCallback?): Result<String>
     fun roomMembershipObserver(): RoomMembershipObserver
+
+    fun isMe(userId: UserId?) = userId == sessionId
 }

libraries/usersearch/impl/src/main/kotlin/io/element/android/libraries/usersearch/impl/MatrixUserRepository.kt

@@ -18,6 +18,7 @@ package io.element.android.libraries.usersearch.impl
 
 import com.squareup.anvil.annotations.ContributesBinding
 import io.element.android.libraries.di.SessionScope
+import io.element.android.libraries.matrix.api.MatrixClient
 import io.element.android.libraries.matrix.api.core.MatrixPatterns
 import io.element.android.libraries.matrix.api.core.UserId
 import io.element.android.libraries.matrix.api.user.MatrixUser
@@ -31,24 +32,29 @@ import javax.inject.Inject
 
 @ContributesBinding(SessionScope::class)
 class MatrixUserRepository @Inject constructor(
+    private val client: MatrixClient,
     private val dataSource: UserListDataSource
 ) : UserRepository {
 
     override suspend fun search(query: String): Flow<List<UserSearchResult>> = flow {
-        // Manually add a fake result with the matrixId, if any
-        val isUserId = MatrixPatterns.isUserId(query)
-        if (isUserId) {
+        // If the search term is a MXID that's not ours, we'll show a 'fake' result for that user, then update it when we get search results.
+        val shouldQueryProfile = MatrixPatterns.isUserId(query) && !client.isMe(UserId(query))
+        if (shouldQueryProfile) {
             emit(listOf(UserSearchResult(MatrixUser(UserId(query)))))
         }
 
         if (query.length >= MINIMUM_SEARCH_LENGTH) {
             // Debounce
             delay(DEBOUNCE_TIME_MILLIS)
 
-            val results = dataSource.search(query, MAXIMUM_SEARCH_RESULTS).map { UserSearchResult(it) }.toMutableList()
+            val results = dataSource
+                .search(query, MAXIMUM_SEARCH_RESULTS)
+                .filter { !client.isMe(it.userId) }
+                .map { UserSearchResult(it) }
+                .toMutableList()
 
-            // If the query is a user ID and the result doesn't contain that user ID, query the profile information explicitly
-            if (isUserId && results.none { it.matrixUser.userId.value == query }) {
+            // If the query is another user's MXID and the result doesn't contain that user ID, query the profile information explicitly
+            if (shouldQueryProfile && results.none { it.matrixUser.userId.value == query }) {
                 results.add(
                     0,
                     dataSource.getProfile(UserId(query))

libraries/usersearch/impl/src/test/kotlin/io/element/android/libraries/usersearch/impl/MatrixUserRepositoryTest.kt

@@ -18,22 +18,26 @@ package io.element.android.libraries.usersearch.impl
 
 import app.cash.turbine.test
 import com.google.common.truth.Truth.assertThat
+import io.element.android.libraries.matrix.api.core.SessionId
 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_USER_ID
 import io.element.android.libraries.matrix.test.A_USER_NAME
+import io.element.android.libraries.matrix.test.FakeMatrixClient
 import io.element.android.libraries.matrix.ui.components.aMatrixUserList
 import io.element.android.libraries.usersearch.api.UserSearchResult
 import io.element.android.libraries.usersearch.test.FakeUserListDataSource
 import kotlinx.coroutines.test.runTest
 import org.junit.Test
 
+private val SESSION_ID = SessionId("@current-user:example.com")
+
 internal class MatrixUserRepositoryTest {
 
     @Test
     fun `search - emits nothing if the search query is too short`() = runTest {
         val dataSource = FakeUserListDataSource()
-        val repository = MatrixUserRepository(dataSource)
+        val repository = MatrixUserRepository(FakeMatrixClient(SESSION_ID), dataSource)
 
         val result = repository.search("x")
 
@@ -45,7 +49,7 @@ internal class MatrixUserRepositoryTest {
     @Test
     fun `search - returns empty list if no results are found`() = runTest {
         val dataSource = FakeUserListDataSource()
-        val repository = MatrixUserRepository(dataSource)
+        val repository = MatrixUserRepository(FakeMatrixClient(SESSION_ID), dataSource)
 
         val result = repository.search("some query")
 
@@ -59,7 +63,7 @@ internal class MatrixUserRepositoryTest {
     fun `search - returns users if results are found`() = runTest {
         val dataSource = FakeUserListDataSource()
         dataSource.givenSearchResult(aMatrixUserList())
-        val repository = MatrixUserRepository(dataSource)
+        val repository = MatrixUserRepository(FakeMatrixClient(SESSION_ID), dataSource)
 
         val result = repository.search("some query")
 
@@ -72,7 +76,7 @@ internal class MatrixUserRepositoryTest {
     @Test
     fun `search - immediately returns placeholder if search is mxid`() = runTest {
         val dataSource = FakeUserListDataSource()
-        val repository = MatrixUserRepository(dataSource)
+        val repository = MatrixUserRepository(FakeMatrixClient(SESSION_ID), dataSource)
 
         val result = repository.search(A_USER_ID.value)
 
@@ -83,12 +87,40 @@ internal class MatrixUserRepositoryTest {
         }
     }
 
+    @Test
+    fun `search - doesn't return placeholder if search is the local user's mxid`() = runTest {
+        val dataSource = FakeUserListDataSource()
+        val repository = MatrixUserRepository(FakeMatrixClient(SESSION_ID), dataSource)
+
+        val result = repository.search(SESSION_ID.value)
+
+        result.test {
+            assertThat(awaitItem()).isEmpty()
+            awaitComplete()
+        }
+    }
+
+    @Test
+    fun `search - filters out results with the local user's mxid`() = runTest {
+        val searchResults = aMatrixUserList() + MatrixUser(userId = SESSION_ID, displayName = A_USER_NAME)
+        val dataSource = FakeUserListDataSource()
+        dataSource.givenSearchResult(searchResults)
+        val repository = MatrixUserRepository(FakeMatrixClient(SESSION_ID), dataSource)
+
+        val result = repository.search("some text")
+
+        result.test {
+            assertThat(awaitItem()).isEqualTo(aMatrixUserList().toUserSearchResults())
+            awaitComplete()
+        }
+    }
+
     @Test
     fun `search - does not change results if they contain searched mxid`() = runTest {
         val searchResults = aMatrixUserListWithoutUserId(A_USER_ID) + MatrixUser(userId = A_USER_ID, displayName = A_USER_NAME)
         val dataSource = FakeUserListDataSource()
         dataSource.givenSearchResult(searchResults)
-        val repository = MatrixUserRepository(dataSource)
+        val repository = MatrixUserRepository(FakeMatrixClient(SESSION_ID), dataSource)
 
         val result = repository.search(A_USER_ID.value)
 
@@ -107,7 +139,7 @@ internal class MatrixUserRepositoryTest {
         val dataSource = FakeUserListDataSource()
         dataSource.givenSearchResult(searchResults)
         dataSource.givenUserProfile(userProfile)
-        val repository = MatrixUserRepository(dataSource)
+        val repository = MatrixUserRepository(FakeMatrixClient(SESSION_ID), dataSource)
 
         val result = repository.search(A_USER_ID.value)
 
@@ -118,14 +150,32 @@ internal class MatrixUserRepositoryTest {
         }
     }
 
+    @Test
+    fun `search - doesn't add profile results if searched mxid is local user and not in results`() = runTest {
+        val userProfile = MatrixUser(userId = A_USER_ID, displayName = A_USER_NAME)
+        val searchResults = aMatrixUserListWithoutUserId(SESSION_ID)
+
+        val dataSource = FakeUserListDataSource()
+        dataSource.givenSearchResult(searchResults)
+        dataSource.givenUserProfile(userProfile)
+        val repository = MatrixUserRepository(FakeMatrixClient(SESSION_ID), dataSource)
+
+        val result = repository.search(SESSION_ID.value)
+
+        result.test {
+            assertThat(awaitItem()).isEqualTo(searchResults.toUserSearchResults())
+            awaitComplete()
+        }
+    }
+
     @Test
     fun `search - returns unresolved user if profile can't be loaded`() = runTest {
         val searchResults = aMatrixUserListWithoutUserId(A_USER_ID)
 
         val dataSource = FakeUserListDataSource()
         dataSource.givenSearchResult(searchResults)
         dataSource.givenUserProfile(null)
-        val repository = MatrixUserRepository(dataSource)
+        val repository = MatrixUserRepository(FakeMatrixClient(SESSION_ID), dataSource)
 
         val result = repository.search(A_USER_ID.value)