Improve user profile search (#1266)

* Filter account owner in UserDiscoveryService

* Add tests and refine logics
This commit is contained in:
Alfonso Grillo
2023-07-06 15:53:31 +02:00
committed by GitHub
parent 1de2a0b3e3
commit 3afb8fa867
2 changed files with 30 additions and 9 deletions
@@ -22,18 +22,18 @@ final class UserDiscoveryService: UserDiscoveryServiceProtocol {
init(clientProxy: ClientProxyProtocol) { init(clientProxy: ClientProxyProtocol) {
self.clientProxy = clientProxy self.clientProxy = clientProxy
} }
func fetchSuggestions() async -> Result<[UserProfileProxy], UserDiscoveryErrorType> { func fetchSuggestions() async -> Result<[UserProfileProxy], UserDiscoveryErrorType> {
.success([.mockAlice, .mockBob, .mockCharlie]) .success(filterAccountOwner([.mockAlice, .mockBob, .mockCharlie]))
} }
func searchProfiles(with searchQuery: String) async -> Result<[UserProfileProxy], UserDiscoveryErrorType> { func searchProfiles(with searchQuery: String) async -> Result<[UserProfileProxy], UserDiscoveryErrorType> {
async let queriedProfile = profileIfPossible(with: searchQuery) async let queriedProfile = profileIfPossible(with: searchQuery)
do { do {
async let searchedUsers = clientProxy.searchUsers(searchTerm: searchQuery, limit: 10).get() async let searchedUsers = clientProxy.searchUsers(searchTerm: searchQuery, limit: 10).get()
let users = try await merge(queriedProfile: queriedProfile, searchResults: searchedUsers) let users = try await merge(queriedProfile: queriedProfile, searchResults: searchedUsers)
return .success(users) return .success(filterAccountOwner(users))
} catch { } catch {
// we want to show the profile (if any) even if the search fails // we want to show the profile (if any) even if the search fails
if let queriedProfile = await queriedProfile { if let queriedProfile = await queriedProfile {
@@ -43,14 +43,14 @@ final class UserDiscoveryService: UserDiscoveryServiceProtocol {
} }
} }
} }
private func merge(queriedProfile: UserProfileProxy?, searchResults: SearchUsersResultsProxy) -> [UserProfileProxy] { private func merge(queriedProfile: UserProfileProxy?, searchResults: SearchUsersResultsProxy) -> [UserProfileProxy] {
let searchResults = searchResults.results let searchResults = searchResults.results
guard let queriedProfile else { guard let queriedProfile else {
return searchResults return searchResults
} }
let filteredSearchResult = searchResults.filter { let filteredSearchResult = searchResults.filter {
$0.userID != queriedProfile.userID $0.userID != queriedProfile.userID
} }
@@ -59,7 +59,7 @@ final class UserDiscoveryService: UserDiscoveryServiceProtocol {
} }
private func profileIfPossible(with searchQuery: String) async -> UserProfileProxy? { private func profileIfPossible(with searchQuery: String) async -> UserProfileProxy? {
guard searchQuery.isMatrixIdentifier else { guard searchQuery.isMatrixIdentifier, searchQuery != clientProxy.userID else {
return nil return nil
} }
@@ -68,6 +68,11 @@ final class UserDiscoveryService: UserDiscoveryServiceProtocol {
// fallback to a "local profile" if the profile api fails // fallback to a "local profile" if the profile api fails
return getProfileResult ?? .init(userID: searchQuery) return getProfileResult ?? .init(userID: searchQuery)
} }
private func filterAccountOwner(_ profiles: [UserProfileProxy]) -> [UserProfileProxy] {
let accountOwnerID = clientProxy.userID
return profiles.filter { $0.userID != accountOwnerID }
}
} }
private extension String { private extension String {
@@ -24,7 +24,7 @@ class UserDiscoveryServiceTest: XCTestCase {
var clientProxy: MockClientProxy! var clientProxy: MockClientProxy!
override func setUpWithError() throws { override func setUpWithError() throws {
clientProxy = .init(userID: "") clientProxy = .init(userID: "@foo:matrix.org")
service = UserDiscoveryService(clientProxy: clientProxy) service = UserDiscoveryService(clientProxy: clientProxy)
} }
@@ -34,6 +34,13 @@ class UserDiscoveryServiceTest: XCTestCase {
let results = await (try? search(query: "AAA").get()) ?? [] let results = await (try? search(query: "AAA").get()) ?? []
assertSearchResults(results, toBe: 1) assertSearchResults(results, toBe: 1)
} }
func testOwnerIsFiltered() async throws {
clientProxy.searchUsersResult = .success(.init(results: [UserProfileProxy(userID: "@foo:matrix.org")], limited: true))
let results = await (try? search(query: "AAA").get()) ?? []
assertSearchResults(results, toBe: 0)
}
func testGetProfileIsNotCalled() async { func testGetProfileIsNotCalled() async {
clientProxy.searchUsersResult = .success(.init(results: searchResults, limited: true)) clientProxy.searchUsersResult = .success(.init(results: searchResults, limited: true))
@@ -43,6 +50,15 @@ class UserDiscoveryServiceTest: XCTestCase {
assertSearchResults(results, toBe: 3) assertSearchResults(results, toBe: 3)
XCTAssertFalse(clientProxy.getProfileCalled) XCTAssertFalse(clientProxy.getProfileCalled)
} }
func testGetProfileIsNotCalledForAccountOwnerID() async {
clientProxy.searchUsersResult = .success(.init(results: searchResults, limited: true))
clientProxy.getProfileResult = .success(.init(userID: "@alice:matrix.org"))
let results = await (try? search(query: "foo:matrix.org").get()) ?? []
assertSearchResults(results, toBe: 3)
XCTAssertFalse(clientProxy.getProfileCalled)
}
func testLocalResultShows() async { func testLocalResultShows() async {
clientProxy.searchUsersResult = .success(.init(results: searchResults, limited: true)) clientProxy.searchUsersResult = .success(.init(results: searchResults, limited: true))