Improve user search using the GET profile (#754)

* Add getProfile in ClientProxyProtocol

* Add fetchProfileIfNeeded

* Refactor search

* Update StartChatSuggestedUserCell UI

* Add showProfilePopover action

* Remove popover code

* Add UTs in StartChatViewModelTests

* Add UI tests

* Cleanup code

* Refine design

* Fix design

* Localise StartChatSuggestedUserCell

* Update localisations

* Update StartChatSuggestedUserCell design

* Fix UT

* Fix PR comments

* Apply suggestions

* Fix typo
This commit is contained in:
Alfonso Grillo
2023-04-04 10:47:41 +02:00
committed by GitHub
parent b089cd6b62
commit ea979d6515
17 changed files with 232 additions and 48 deletions
@@ -37,7 +37,8 @@
"action_save" = "Save";
"action_search" = "Search";
"action_send" = "Send";
"action_share_link" = "Share";
"action_share" = "Share";
"action_share_link" = "Share link";
"action_skip" = "Skip";
"action_start" = "Start";
"action_start_chat" = "Start chat";
@@ -214,6 +215,7 @@
"screen_signout_in_progress_dialog_content" = "Signing out…";
"screen_signout_preference_item" = "Sign out";
"screen_start_chat_error_starting_chat" = "An error occurred when trying to start a chat";
"screen_start_chat_unknown_profile" = "We can’t validate this user’s Matrix ID. The invite might not be received.";
"session_verification_banner_message" = "Looks like you’re using a new device. Verify it’s you to access your encrypted messages.";
"session_verification_banner_title" = "Access your message history";
"settings_rageshake" = "Rageshake";
@@ -37,7 +37,8 @@
"action_save" = "Save";
"action_search" = "Search";
"action_send" = "Send";
"action_share_link" = "Share";
"action_share" = "Share";
"action_share_link" = "Share link";
"action_skip" = "Skip";
"action_start" = "Start";
"action_start_chat" = "Start chat";
@@ -214,6 +215,7 @@
"screen_signout_in_progress_dialog_content" = "Signing out…";
"screen_signout_preference_item" = "Sign out";
"screen_start_chat_error_starting_chat" = "An error occurred when trying to start a chat";
"screen_start_chat_unknown_profile" = "We can’t validate this user’s Matrix ID. The invite might not be received.";
"session_verification_banner_message" = "Looks like you’re using a new device. Verify it’s you to access your encrypted messages.";
"session_verification_banner_title" = "Access your message history";
"settings_rageshake" = "Rageshake";
@@ -37,7 +37,8 @@
"action_save" = "Save";
"action_search" = "Search";
"action_send" = "Send";
"action_share_link" = "Share";
"action_share" = "Share";
"action_share_link" = "Share link";
"action_skip" = "Skip";
"action_start" = "Start";
"action_start_chat" = "Start chat";
@@ -214,6 +215,7 @@
"screen_signout_in_progress_dialog_content" = "Signing out…";
"screen_signout_preference_item" = "Sign out";
"screen_start_chat_error_starting_chat" = "An error occurred when trying to start a chat";
"screen_start_chat_unknown_profile" = "We can’t validate this user’s Matrix ID. The invite might not be received.";
"session_verification_banner_message" = "Looks like you’re using a new device. Verify it’s you to access your encrypted messages.";
"session_verification_banner_title" = "Access your message history";
"settings_rageshake" = "Rageshake";
@@ -37,7 +37,8 @@
"action_save" = "Save";
"action_search" = "Search";
"action_send" = "Send";
"action_share_link" = "Share";
"action_share" = "Share";
"action_share_link" = "Share link";
"action_skip" = "Skip";
"action_start" = "Start";
"action_start_chat" = "Start chat";
@@ -214,6 +215,7 @@
"screen_signout_in_progress_dialog_content" = "Signing out…";
"screen_signout_preference_item" = "Sign out";
"screen_start_chat_error_starting_chat" = "An error occurred when trying to start a chat";
"screen_start_chat_unknown_profile" = "We can’t validate this user’s Matrix ID. The invite might not be received.";
"session_verification_banner_message" = "Looks like you’re using a new device. Verify it’s you to access your encrypted messages.";
"session_verification_banner_title" = "Access your message history";
"settings_rageshake" = "Rageshake";
@@ -37,6 +37,7 @@
"action_save" = "Salvează";
"action_search" = "Caută";
"action_send" = "Trimite";
"action_share" = "Share";
"action_share_link" = "Partajează linkul";
"action_skip" = "Omite";
"action_start" = "Începe";
@@ -214,6 +215,7 @@
"screen_signout_in_progress_dialog_content" = "Deconectare în curs...";
"screen_signout_preference_item" = "Deconectează-te";
"screen_start_chat_error_starting_chat" = "A apărut o eroare la încercarea începerii conversației";
"screen_start_chat_unknown_profile" = "We can’t validate this user’s Matrix ID. The invite might not be received.";
"session_verification_banner_message" = "Se pare că folosești un dispozitiv nou. Verifică-ți identitatea pentru acces la mesajele tale criptate.";
"session_verification_banner_title" = "Accesează istoricul mesajelor";
"settings_rageshake" = "Rageshake";
+4
View File
@@ -89,6 +89,8 @@ public enum L10n {
/// Send
public static var actionSend: String { return L10n.tr("Localizable", "action_send") }
/// Share
public static var actionShare: String { return L10n.tr("Localizable", "action_share") }
/// Share link
public static var actionShareLink: String { return L10n.tr("Localizable", "action_share_link") }
/// Skip
public static var actionSkip: String { return L10n.tr("Localizable", "action_skip") }
@@ -472,6 +474,8 @@ public enum L10n {
public static var screenSignoutPreferenceItem: String { return L10n.tr("Localizable", "screen_signout_preference_item") }
/// An error occurred when trying to start a chat
public static var screenStartChatErrorStartingChat: String { return L10n.tr("Localizable", "screen_start_chat_error_starting_chat") }
/// We can’t validate this user’s Matrix ID. The invite might not be received.
public static var screenStartChatUnknownProfile: String { return L10n.tr("Localizable", "screen_start_chat_unknown_profile") }
/// Looks like you’re using a new device. Verify it’s you to access your encrypted messages.
public static var sessionVerificationBannerMessage: String { return L10n.tr("Localizable", "session_verification_banner_message") }
/// Access your message history
@@ -24,9 +24,6 @@ class StartChatViewModel: StartChatViewModelType, StartChatViewModelProtocol {
var callback: ((StartChatViewModelAction) -> Void)?
weak var userIndicatorController: UserIndicatorControllerProtocol?
var searchTask: Task<Void, Error>? {
didSet { oldValue?.cancel() }
}
init(userSession: UserSessionProtocol, userIndicatorController: UserIndicatorControllerProtocol?) {
self.userSession = userSession
@@ -50,7 +47,7 @@ class StartChatViewModel: StartChatViewModelType, StartChatViewModelProtocol {
case .selectUser(let user):
showLoadingIndicator()
Task {
let currentDirectRoom = await userSession.clientProxy.directRoomForUserID(user.userID)
let currentDirectRoom = await clientProxy.directRoomForUserID(user.userID)
switch currentDirectRoom {
case .success(.some(let roomId)):
self.hideLoadingIndicator()
@@ -65,7 +62,9 @@ class StartChatViewModel: StartChatViewModelType, StartChatViewModelProtocol {
}
}
func displayError(_ type: ClientProxyError) {
// MARK: - Private
private func displayError(_ type: ClientProxyError) {
switch type {
case .failedRetrievingDirectRoom:
state.bindings.alertInfo = AlertInfo(id: type,
@@ -80,8 +79,6 @@ class StartChatViewModel: StartChatViewModelType, StartChatViewModelProtocol {
}
}
// MARK: - Private
private func setupBindings() {
context.$viewState
.map(\.bindings.searchQuery)
@@ -92,22 +89,56 @@ class StartChatViewModel: StartChatViewModelType, StartChatViewModelProtocol {
}
.switchToLatest()
.removeDuplicates()
.sink { [weak self] query in
self?.updateState(searchQuery: query)
.sink { [weak self] _ in
self?.fetchData()
}
.store(in: &cancellables)
}
private func updateState(searchQuery: String) {
searchTask = nil
if searchQuery.count < 3 {
private func fetchData() {
guard searchQuery.count >= 3 else {
fetchSuggestions()
} else if MatrixEntityRegex.isMatrixUserIdentifier(searchQuery) {
state.usersSection = .init(type: .searchResult, users: [UserProfile(userID: searchQuery)])
} else {
searchUsers(searchTerm: searchQuery)
return
}
Task {
await searchProfiles()
}
}
private func searchProfiles() async {
// copies the current query to check later if fetched data must be shown or not
let committedQuery = searchQuery
async let queriedProfile = getProfileIfPossible()
async let searchedUsers = clientProxy.searchUsers(searchTerm: committedQuery, limit: 5)
await updateState(committedQuery: committedQuery,
queriedProfile: queriedProfile,
searchResults: try? searchedUsers.get())
}
private func updateState(committedQuery: String, queriedProfile: UserProfile?, searchResults: SearchUsersResults?) {
guard committedQuery == searchQuery else {
return
}
let localProfile = queriedProfile ?? UserProfile(searchQuery: searchQuery)
let allResults = merge(localProfile: localProfile, searchResults: searchResults?.results)
state.usersSection = .init(type: .searchResult, users: allResults)
}
private func merge(localProfile: UserProfile?, searchResults: [UserProfile]?) -> [UserProfile] {
guard let localProfile else {
return searchResults ?? []
}
let filteredSearchResult = searchResults?.filter {
$0.userID != localProfile.userID
} ?? []
return [localProfile] + filteredSearchResult
}
private func fetchSuggestions() {
@@ -116,7 +147,7 @@ class StartChatViewModel: StartChatViewModelType, StartChatViewModelProtocol {
private func createDirectRoom(with user: UserProfile) async {
showLoadingIndicator()
let result = await userSession.clientProxy.createDirectRoom(with: user.userID)
let result = await clientProxy.createDirectRoom(with: user.userID)
hideLoadingIndicator()
switch result {
case .success(let roomId):
@@ -126,17 +157,20 @@ class StartChatViewModel: StartChatViewModelType, StartChatViewModelProtocol {
}
}
private func searchUsers(searchTerm: String) {
searchTask = Task { @MainActor in
guard
case let .success(result) = await userSession.clientProxy.searchUsers(searchTerm: searchTerm, limit: 5),
!Task.isCancelled
else {
return
}
state.usersSection = .init(type: .searchResult, users: result.results)
private func getProfileIfPossible() async -> UserProfile? {
guard searchQuery.isMatrixIdentifier else {
return nil
}
return try? await clientProxy.getProfile(for: searchQuery).get()
}
private var clientProxy: ClientProxyProtocol {
userSession.clientProxy
}
private var searchQuery: String {
context.searchQuery
}
// MARK: Loading indicator
@@ -154,3 +188,18 @@ class StartChatViewModel: StartChatViewModelType, StartChatViewModelProtocol {
userIndicatorController?.retractIndicatorWithId(Self.loadingIndicatorIdentifier)
}
}
private extension String {
var isMatrixIdentifier: Bool {
MatrixEntityRegex.isMatrixUserIdentifier(self)
}
}
private extension UserProfile {
init?(searchQuery: String) {
guard searchQuery.isMatrixIdentifier else {
return nil
}
self.init(userID: searchQuery)
}
}
@@ -20,8 +20,4 @@ import Foundation
protocol StartChatViewModelProtocol {
var callback: ((StartChatViewModelAction) -> Void)? { get set }
var context: StartChatViewModelType.Context { get }
/// Display an error to the user.
/// - Parameter type: The type of error to be displayed.
func displayError(_ type: ClientProxyError)
}
@@ -29,15 +29,29 @@ struct StartChatSuggestedUserCell: View {
imageProvider: imageProvider)
.padding(.vertical, 10)
.accessibilityHidden(true)
VStack(alignment: .leading, spacing: 4) {
Text(user.displayName ?? user.userID)
.font(.element.title3)
.foregroundColor(.element.primaryContent)
if user.displayName != nil {
Text(user.userID)
.font(.element.subheadline)
.foregroundColor(.element.tertiaryContent)
}
if !user.isVerified {
HStack(alignment: .firstTextBaseline, spacing: 4) {
Text(Image(systemName: "exclamationmark.circle.fill"))
.foregroundColor(.compound.textActionCritical)
Text(L10n.screenStartChatUnknownProfile)
.foregroundColor(.secondary)
}
.font(.compound.bodyXS)
.padding(.top, 4)
}
}
.accessibilityElement(children: .combine)
}
@@ -258,6 +258,16 @@ class ClientProxy: ClientProxyProtocol {
}
}
func getProfile(for userID: String) async -> Result<UserProfile, ClientProxyError> {
await Task.dispatch(on: clientQueue) {
do {
return try .success(.init(sdkUserProfile: self.client.getProfile(userId: userID)))
} catch {
return .failure(.failedGettingUserProfile)
}
}
}
// MARK: Private
private func loadUserAvatarURLFromCache() {
@@ -32,6 +32,7 @@ enum ClientProxyError: Error {
case failedRetrievingSessionVerificationController
case failedLoadingMedia
case failedSearchingUsers
case failedGettingUserProfile
}
enum SlidingSyncConstants {
@@ -94,4 +95,6 @@ protocol ClientProxyProtocol: AnyObject, MediaLoaderProtocol {
func setPusher(with configuration: PusherConfiguration) async throws
func searchUsers(searchTerm: String, limit: UInt) async -> Result<SearchUsersResults, ClientProxyError>
func getProfile(for userID: String) async -> Result<UserProfile, ClientProxyError>
}
@@ -115,4 +115,11 @@ class MockClientProxy: ClientProxyProtocol {
func searchUsers(searchTerm: String, limit: UInt) async -> Result<SearchUsersResults, ClientProxyError> {
searchUsersResult
}
var getProfileResult: Result<UserProfile, ClientProxyError> = .success(.init(userID: "@a:b.com", displayName: "Some user"))
private(set) var getProfileCalled = false
func getProfile(for userID: String) async -> Result<UserProfile, ClientProxyError> {
getProfileCalled = true
return getProfileResult
}
}
@@ -33,6 +33,12 @@ struct UserProfile {
displayName = sdkUserProfile.displayName
avatarURL = sdkUserProfile.avatarUrl.flatMap(URL.init(string:))
}
/// A user is meant to be "verified" when the GET profile returns back either the display name or the avatar
/// If isn't we aren't sure that the related matrix id really exists.
var isVerified: Bool {
displayName != nil || avatarURL != nil
}
}
struct SearchUsersResults {
@@ -322,6 +322,14 @@ class MockScreen: Identifiable {
let coordinator = StartChatCoordinator(parameters: .init(userSession: MockUserSession(clientProxy: clientProxy, mediaProvider: MockMediaProvider())))
navigationStackCoordinator.setRootCoordinator(coordinator)
return navigationStackCoordinator
case .startChatSearchingNonExistingID:
let navigationStackCoordinator = NavigationStackCoordinator()
let clientProxy = MockClientProxy(userID: "@mock:client.com")
clientProxy.searchUsersResult = .success(.init(results: [.mockAlice], limited: true))
clientProxy.getProfileResult = .failure(.failedGettingUserProfile)
let coordinator = StartChatCoordinator(parameters: .init(userSession: MockUserSession(clientProxy: clientProxy, mediaProvider: MockMediaProvider())))
navigationStackCoordinator.setRootCoordinator(coordinator)
return navigationStackCoordinator
case .roomMemberDetailsAccountOwner:
let navigationStackCoordinator = NavigationStackCoordinator()
let coordinator = RoomMemberDetailsCoordinator(parameters: .init(roomMemberProxy: RoomMemberProxyMock.mockMe, mediaProvider: MockMediaProvider()))
@@ -50,6 +50,7 @@ enum UITestsScreenIdentifier: String {
case reportContent
case startChat
case startChatWithSearchResults
case startChatSearchingNonExistingID
}
extension UITestsScreenIdentifier: CustomStringConvertible {