Refactor search related functionality (#436)

Refactor search related functionality

This is a prelude to adding the feature of inviting users to
a room, getting everything in the right place and reusable.

What this does:

## User search refactor

Moves the (global) user search logic (dealing with MXIDs,
minimum lengths, debounces) into a `UserRepository`.

This now sits in a `usersearch` library, which will be
used by the create room flow and the new invite flow.

## SearchBar logic pull-up

Every place we use SearchBar, we're doing the same things
to style placeholders, show back/cancel buttons, etc.

We also have a results type that is duplicated for
basically every feature that uses the search bar.

I've pushed all this common functionality into the
SearchBar itself. This makes the component a bit less
general purpose, but saves a lot of repetition.

## Remove the userlist feature

Almost all the functionality of the userlist feature
is now exclusively used by the create room feature.
Room details uses its own version because the
requirements are different.

Components useful elsewhere (SelectedUsers and
SelectedUser) have gone to matrixui, everything else
has gone to createroom.

## Other bits and pieces

I've fixed everywhere that uses Scaffold to correctly
consume the WindowInsets if the contentPadding is
applied to the contents (which it universally is).
This was a change in the last version of Material3
(I guess previously Scaffold handled the consumption
for us). This fixes weird gaps above search bars.

Added overloads for the MatrixUserRow and
CheckedMatrixUserRow that take the name/subtitle/avatar
separately, so the invites list can pass arbitrary
text like "User has already been invited".

The `blockuser` package was for some reason not
under `impl` but alongside it, I've bumped it into
the right place.
This commit is contained in:
Chris Smith 2023-05-22 10:06:54 +01:00 committed by GitHub
parent e2a2374c4a
commit 1eac67bf25
137 changed files with 1002 additions and 675 deletions

View file

@ -0,0 +1,43 @@
/*
* Copyright (c) 2023 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.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.UserId
import io.element.android.libraries.matrix.api.user.MatrixUser
import io.element.android.libraries.usersearch.api.UserListDataSource
import javax.inject.Inject
@ContributesBinding(SessionScope::class)
class MatrixUserListDataSource @Inject constructor(
private val client: MatrixClient
) : UserListDataSource {
override suspend fun search(query: String): List<MatrixUser> {
val res = client.searchUsers(query, MAX_SEARCH_RESULTS)
return res.getOrNull()?.results.orEmpty()
}
override suspend fun getProfile(userId: UserId): MatrixUser? {
return client.getProfile(userId).getOrNull()
}
companion object {
private const val MAX_SEARCH_RESULTS = 5L
}
}

View file

@ -0,0 +1,64 @@
/*
* Copyright (c) 2023 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.libraries.usersearch.impl
import com.squareup.anvil.annotations.ContributesBinding
import io.element.android.libraries.di.SessionScope
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
import io.element.android.libraries.usersearch.api.UserListDataSource
import io.element.android.libraries.usersearch.api.UserRepository
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.flow
import javax.inject.Inject
@ContributesBinding(SessionScope::class)
class MatrixUserRepository @Inject constructor(
private val dataSource: UserListDataSource
) : UserRepository {
override suspend fun search(query: String): Flow<List<MatrixUser>> = flow {
// Manually add a fake result with the matrixId, if any
val isUserId = MatrixPatterns.isUserId(query)
if (isUserId) {
emit(listOf(MatrixUser(UserId(query))))
}
if (query.length >= MINIMUM_SEARCH_LENGTH) {
// Debounce
delay(DEBOUNCE_TIME_MILLIS)
val results = dataSource.search(query).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.userId.value == query }) {
val getProfileResult: MatrixUser? = dataSource.getProfile(UserId(query))
val profile = getProfileResult ?: MatrixUser(UserId(query))
results.add(0, profile)
}
emit(results)
}
}
companion object {
private const val DEBOUNCE_TIME_MILLIS = 500L
private const val MINIMUM_SEARCH_LENGTH = 3
}
}

View file

@ -0,0 +1,101 @@
/*
* Copyright (c) 2023 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.libraries.usersearch.impl
import com.google.common.truth.Truth
import io.element.android.libraries.matrix.api.core.UserId
import io.element.android.libraries.matrix.api.user.MatrixSearchUserResults
import io.element.android.libraries.matrix.api.user.MatrixUser
import io.element.android.libraries.matrix.test.AN_AVATAR_URL
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.A_USER_NAME
import io.element.android.libraries.matrix.test.FakeMatrixClient
import kotlinx.coroutines.test.runTest
import org.junit.Test
internal class MatrixUserListDataSourceTest {
@Test
fun `search - returns users on success`() = runTest {
val matrixClient = FakeMatrixClient()
matrixClient.givenSearchUsersResult(
searchTerm = "test",
result = Result.success(
MatrixSearchUserResults(
results = listOf(
aMatrixUserProfile(),
aMatrixUserProfile(userId = A_USER_ID_2)
),
limited = false
)
)
)
val dataSource = MatrixUserListDataSource(matrixClient)
val results = dataSource.search("test")
Truth.assertThat(results).containsExactly(
aMatrixUserProfile(),
aMatrixUserProfile(userId = A_USER_ID_2)
)
}
@Test
fun `search - returns empty list on error`() = runTest {
val matrixClient = FakeMatrixClient()
matrixClient.givenSearchUsersResult(
searchTerm = "test",
result = Result.failure(Throwable("Ruhroh"))
)
val dataSource = MatrixUserListDataSource(matrixClient)
val results = dataSource.search("test")
Truth.assertThat(results).isEmpty()
}
@Test
fun `get profile - returns user on success`() = runTest {
val matrixClient = FakeMatrixClient()
matrixClient.givenGetProfileResult(
userId = A_USER_ID,
result = Result.success(aMatrixUserProfile())
)
val dataSource = MatrixUserListDataSource(matrixClient)
val result = dataSource.getProfile(A_USER_ID)
Truth.assertThat(result).isEqualTo(aMatrixUserProfile())
}
@Test
fun `get profile - returns null on error`() = runTest {
val matrixClient = FakeMatrixClient()
matrixClient.givenGetProfileResult(
userId = A_USER_ID,
result = Result.failure(Throwable("Ruhroh"))
)
val dataSource = MatrixUserListDataSource(matrixClient)
val result = dataSource.getProfile(A_USER_ID)
Truth.assertThat(result).isNull()
}
private fun aMatrixUserProfile(
userId: UserId = A_USER_ID,
displayName: String = A_USER_NAME,
avatarUrl: String = AN_AVATAR_URL
) = MatrixUser(userId, displayName, avatarUrl)
}

View file

@ -0,0 +1,140 @@
/*
* Copyright (c) 2023 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.libraries.usersearch.impl
import app.cash.turbine.test
import com.google.common.truth.Truth.assertThat
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.ui.components.aMatrixUserList
import io.element.android.libraries.usersearch.test.FakeUserListDataSource
import kotlinx.coroutines.test.runTest
import org.junit.Test
internal class MatrixUserRepositoryTest {
@Test
fun `search - emits nothing if the search query is too short`() = runTest {
val dataSource = FakeUserListDataSource()
val repository = MatrixUserRepository(dataSource)
val result = repository.search("x")
result.test {
awaitComplete()
}
}
@Test
fun `search - returns empty list if no results are found`() = runTest {
val dataSource = FakeUserListDataSource()
val repository = MatrixUserRepository(dataSource)
val result = repository.search("some query")
result.test {
assertThat(awaitItem()).isEmpty()
awaitComplete()
}
}
@Test
fun `search - returns users if results are found`() = runTest {
val dataSource = FakeUserListDataSource()
dataSource.givenSearchResult(aMatrixUserList())
val repository = MatrixUserRepository(dataSource)
val result = repository.search("some query")
result.test {
assertThat(awaitItem()).isEqualTo(aMatrixUserList())
awaitComplete()
}
}
@Test
fun `search - immediately returns placeholder if search is mxid`() = runTest {
val dataSource = FakeUserListDataSource()
val repository = MatrixUserRepository(dataSource)
val result = repository.search(A_USER_ID.value)
result.test {
assertThat(awaitItem()).isEqualTo(listOf(MatrixUser(userId = A_USER_ID)))
skipItems(1)
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 result = repository.search(A_USER_ID.value)
result.test {
skipItems(1)
assertThat(awaitItem()).isEqualTo(searchResults)
awaitComplete()
}
}
@Test
fun `search - gets profile results if searched mxid not in results`() = runTest {
val userProfile = MatrixUser(userId = A_USER_ID, displayName = A_USER_NAME)
val searchResults = aMatrixUserListWithoutUserId(A_USER_ID)
val dataSource = FakeUserListDataSource()
dataSource.givenSearchResult(searchResults)
dataSource.givenUserProfile(userProfile)
val repository = MatrixUserRepository(dataSource)
val result = repository.search(A_USER_ID.value)
result.test {
skipItems(1)
assertThat(awaitItem()).isEqualTo(listOf(userProfile) + searchResults)
awaitComplete()
}
}
@Test
fun `search - just shows id 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 result = repository.search(A_USER_ID.value)
result.test {
skipItems(1)
assertThat(awaitItem()).isEqualTo(listOf(MatrixUser(userId = A_USER_ID)) + searchResults)
awaitComplete()
}
}
private fun aMatrixUserListWithoutUserId(userId: UserId) = aMatrixUserList().filterNot { it.userId == userId }
}