Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,17 @@ import Foundation
/// Suggests users only, preserving the historical mention behaviour. By default
/// it searches the channel's members and watchers; pass `mentionAllAppUsers: true`
/// at initialization to search across all app users instead.
///
/// When the channel has more members than are available in
/// `ChatChannel.lastActiveMembers`, members are searched remotely via
/// ``MemberList``.
public final class DefaultMentionSuggestionsProvider: MentionSuggestionsProvider, Sendable {
/// When `true`, user suggestions are searched across all app users instead
/// of only the channel's members and watchers.
public let mentionAllAppUsers: Bool

private let userSearch: UserSearch
private let makeMemberList: @Sendable (ChannelMemberListQuery) -> MemberList

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why's it a closure and not a member list?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is because we need to create a MemberList object per query. At the moment we do not have a MemberSearch object, like now we do with ChannelSearch. It could make sense to do it now πŸ€” WDYT? Or do it in a seperate PR

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about just adding state later version? We should start moving towards async-await more.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then we have nice set of these. Channel, messages, member. Also the add members view in contact info vould benefit from debounce supported MemberSearch.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the state layer; the provider only uses the state layer, actually. But yeah, at the moment we don't have MemberSearch. But if we want to add it in this PR, I can do it

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@laevandus @martinmitrevski I've updated the PR to include MemberSearch. Also, I added debouncing support for the other providers (Role Search and Group Search). While I was at it, I also added the missing GroupSearchCOntroller that was missing. (Why not)

private let currentUserId: @Sendable () -> UserId?

/// Creates a new default mention suggestions provider.
Expand All @@ -25,22 +30,18 @@ public final class DefaultMentionSuggestionsProvider: MentionSuggestionsProvider
public init(client: ChatClient, mentionAllAppUsers: Bool = false) {
self.mentionAllAppUsers = mentionAllAppUsers
userSearch = client.makeUserSearch()
makeMemberList = { client.makeMemberList(with: $0) }
currentUserId = { [weak client] in client?.currentUserId }
}

public func mentionSuggestions(for request: MentionSuggestionsRequest) async throws -> [MentionSuggestion] {
let users: [ChatUser]
if mentionAllAppUsers {
let query = MentionSuggestionsSearch.allAppUsersQuery(for: request.text)
users = try await userSearch.search(query: query)
} else {
let channel = request.channel
users = MentionSuggestionsSearch.searchUsers(
channel.lastActiveWatchers.map(\.self) + channel.lastActiveMembers.map(\.self),
by: request.text,
excludingId: currentUserId()
)
}
let users = try await MentionSuggestionsSearch.fetchUsers(
for: request,
mentionAllAppUsers: mentionAllAppUsers,
currentUserId: currentUserId(),
userSearch: userSearch,
makeMemberList: makeMemberList
)
return users.map { MentionSuggestion.user($0) }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ import Foundation
/// In addition to user mentions, it suggests `@here`, `@channel`, roles and user groups.
/// Which of these types is suggested is determined by the channel's own
/// capabilities (`notify-here`, `notify-channel`, `notify-role`, `notify-group`).
///
/// When the channel has more members than are available in
/// `ChatChannel.lastActiveMembers`, members are searched remotely via
/// ``MemberList``.
public final class EnhancedMentionSuggestionsProvider: MentionSuggestionsProvider, Sendable {
/// When `true`, user suggestions are searched across all app users instead
/// of only the channel's members and watchers.
Expand All @@ -17,6 +21,7 @@ public final class EnhancedMentionSuggestionsProvider: MentionSuggestionsProvide
private let userSearch: UserSearch
private let roleSearch: RoleSearch
private let userGroupSearch: UserGroupSearch
private let makeMemberList: @Sendable (ChannelMemberListQuery) -> MemberList
private let currentUserId: @Sendable () -> UserId?

/// Creates a new enhanced mention suggestions provider.
Expand All @@ -29,6 +34,7 @@ public final class EnhancedMentionSuggestionsProvider: MentionSuggestionsProvide
userSearch = client.makeUserSearch()
roleSearch = client.makeRoleSearch()
userGroupSearch = client.makeUserGroupSearch()
makeMemberList = { client.makeMemberList(with: $0) }
currentUserId = { [weak client] in client?.currentUserId }
}

Expand Down Expand Up @@ -98,23 +104,18 @@ public final class EnhancedMentionSuggestionsProvider: MentionSuggestionsProvide
}

private func fetchUsers(for request: MentionSuggestionsRequest) async -> [MentionSuggestion] {
let users: [ChatUser]
if mentionAllAppUsers {
do {
let query = MentionSuggestionsSearch.allAppUsersQuery(for: request.text)
users = try await userSearch.search(query: query)
} catch {
log.error("Failed to fetch user suggestions: \(error)")
users = []
}
} else {
let channel = request.channel
users = MentionSuggestionsSearch.searchUsers(
channel.lastActiveWatchers.map(\.self) + channel.lastActiveMembers.map(\.self),
by: request.text,
excludingId: currentUserId()
do {
let users = try await MentionSuggestionsSearch.fetchUsers(
for: request,
mentionAllAppUsers: mentionAllAppUsers,
currentUserId: currentUserId(),
userSearch: userSearch,
makeMemberList: makeMemberList
)
return users.map { MentionSuggestion.user($0) }
} catch {
log.error("Failed to fetch user suggestions: \(error)")
return []
}
return users.map { MentionSuggestion.user($0) }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,60 @@ enum MentionSuggestionsSearch {
sort: [.init(key: .name, isAscending: true)]
)
}

/// Builds the query used to search channel members for mention suggestions.
static func channelMembersQuery(cid: ChannelId, for searchInput: String) -> ChannelMemberListQuery {
ChannelMemberListQuery(
cid: cid,
filter: .autocomplete(.name, text: searchInput),
sort: [.init(key: .name, isAscending: true)]
)
}

/// Whether mention suggestions should query members from the backend.
///
/// `ChatChannel.lastActiveMembers` is capped (default 100), so larger channels
/// need a remote member search.
static func requiresRemoteMemberSearch(for channel: ChatChannel) -> Bool {
channel.memberCount > channel.lastActiveMembers.count
}

/// Resolves user mention suggestions for the given request.
///
/// Search strategy, in order:
/// 1. All app users, when `mentionAllAppUsers` is `true`.
/// 2. Remote channel member search via ``MemberList``, when the channel has
/// more members than are available in `lastActiveMembers`.
/// 3. Local search over `lastActiveMembers` and `lastActiveWatchers`.
static func fetchUsers(
for request: MentionSuggestionsRequest,
mentionAllAppUsers: Bool,
currentUserId: UserId?,
userSearch: UserSearch,
makeMemberList: (ChannelMemberListQuery) -> MemberList
) async throws -> [ChatUser] {
if mentionAllAppUsers {
let query = allAppUsersQuery(for: request.text)
return try await userSearch.search(query: query)
Comment thread
nuno-vieira marked this conversation as resolved.
}

let channel = request.channel
if requiresRemoteMemberSearch(for: channel) {
// Empty `@` should not load every member of a large channel.
guard !request.text.isEmpty else { return [] }

let query = channelMembersQuery(cid: channel.cid, for: request.text)
let memberList = makeMemberList(query)
let members = try await memberList.loadMembers(
with: Pagination(pageSize: query.pagination.pageSize)
)
return members.filter { $0.id != currentUserId }
}

return searchUsers(
channel.lastActiveWatchers.map(\.self) + channel.lastActiveMembers.map(\.self),
by: request.text,
excludingId: currentUserId
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,47 @@ final class DefaultMentionSuggestionsProvider_Tests: XCTestCase {
XCTAssertTrue(suggestions.allSatisfy { $0.kind is MentionSuggestion.User })
}

func test_mentionSuggestions_whenLargeChannel_searchesMembersRemotely() async throws {
let cid = ChannelId.unique
try client.mockDatabaseContainer.createChannel(cid: cid, withMessages: false)

let remoteMember = ChannelMemberResponse.dummy(
user: .dummy(userId: "remote-user", name: "Remote User")
)
let response: Result<MembersResponse, Error> = .success(.dummy(members: [remoteMember]))
client.mockAPIClient.test_mockResponseResult(response)

let provider = DefaultMentionSuggestionsProvider(client: client)
let channel = ChatChannel.mock(
cid: cid,
lastActiveMembers: [.mock(id: "local-only", name: "Local")],
memberCount: 1000
)

let suggestions = try await provider.mentionSuggestions(
for: MentionSuggestionsRequest(text: "Rem", channel: channel)
)

XCTAssertEqual(client.mockAPIClient.request_endpoint?.path.value, "/api/v2/chat/members")
XCTAssertEqual(userIds(from: suggestions), ["remote-user"])
}

func test_mentionSuggestions_whenLargeChannelAndEmptyText_returnsNoUsers() async throws {
let provider = DefaultMentionSuggestionsProvider(client: client)
let channel = ChatChannel.mock(
cid: .unique,
lastActiveMembers: [.mock(id: "local-only", name: "Local")],
memberCount: 1000
)

let suggestions = try await provider.mentionSuggestions(
for: MentionSuggestionsRequest(text: "", channel: channel)
)

XCTAssertNil(client.mockAPIClient.request_endpoint)
XCTAssertTrue(suggestions.isEmpty)
}

// MARK: - private

private func userIds(from suggestions: [MentionSuggestion]) -> [String] {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,25 +106,79 @@ final class EnhancedMentionSuggestionsProvider_Tests: XCTestCase {
XCTAssertTrue(suggestions[0].kind is MentionSuggestion.Channel)
}

func test_mentionSuggestions_whenLargeChannel_searchesMembersRemotely() async throws {
client.mockRolesRepository.searchRoles_completion_result = .success([])
client.mockUserGroupsRepository.searchUserGroups_completion_result = .success([])

let cid = ChannelId.unique
try client.mockDatabaseContainer.createChannel(cid: cid, withMessages: false)

let remoteMember = ChannelMemberResponse.dummy(
user: .dummy(userId: "remote-user", name: "Remote User")
)
let response: Result<MembersResponse, Error> = .success(.dummy(members: [remoteMember]))
client.mockAPIClient.test_mockResponseResult(response)

let provider = makeProvider()
let channel = makeChannel(
cid: cid,
capabilities: [],
lastActiveMembers: [.mock(id: "local-only", name: "Local")],
memberCount: 1000
)

let suggestions = try await provider.mentionSuggestions(
for: MentionSuggestionsRequest(text: "Rem", channel: channel)
)

XCTAssertEqual(client.mockAPIClient.request_endpoint?.path.value, "/api/v2/chat/members")
XCTAssertEqual(userIds(from: suggestions), ["remote-user"])
}

func test_mentionSuggestions_whenLargeChannelAndEmptyText_returnsBroadcastsWithoutFetchingMembers() async throws {
let provider = makeProvider()
let channel = makeChannel(
lastActiveMembers: [.mock(id: "local-only", name: "Local")],
memberCount: 1000
)

let suggestions = try await provider.mentionSuggestions(
for: MentionSuggestionsRequest(text: "", channel: channel)
)

XCTAssertNil(client.mockAPIClient.request_endpoint)
XCTAssertTrue(suggestions.contains { $0.kind is MentionSuggestion.Channel })
XCTAssertTrue(suggestions.contains { $0.kind is MentionSuggestion.Here })
XCTAssertFalse(suggestions.contains { $0.kind is MentionSuggestion.User })
}

// MARK: - private

private func makeProvider(mentionAllAppUsers: Bool = false) -> EnhancedMentionSuggestionsProvider {
EnhancedMentionSuggestionsProvider(client: client, mentionAllAppUsers: mentionAllAppUsers)
}

private func makeChannel(
capabilities: Set<ChannelCapability> = [.notifyHere, .notifyChannel, .notifyRole, .notifyGroup]
cid: ChannelId = .unique,
capabilities: Set<ChannelCapability> = [.notifyHere, .notifyChannel, .notifyRole, .notifyGroup],
lastActiveMembers: [ChatChannelMember] = [
.mock(id: "martin", name: "Martin"),
.mock(id: "john", name: "John")
],
memberCount: Int = 0
) -> ChatChannel {
ChatChannel.mock(
cid: .unique,
cid: cid,
ownCapabilities: capabilities,
lastActiveMembers: [
.mock(id: "martin", name: "Martin"),
.mock(id: "john", name: "John")
]
lastActiveMembers: lastActiveMembers,
memberCount: memberCount
)
}

private func userIds(from suggestions: [MentionSuggestion]) -> [String] {
suggestions.compactMap { ($0.kind as? MentionSuggestion.User)?.user.id }
}

private func makeGroup(id: String) -> UserGroup {
UserGroup.dummy(id: id, name: "Group \(id)")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,4 +99,38 @@ final class MentionSuggestionsSearch_Tests: XCTestCase {
)
XCTAssertEqual(query.sort, [.init(key: .name, isAscending: true)])
}

// MARK: - channelMembersQuery

func test_channelMembersQuery_buildsAutocompleteNameFilterSortedByName() {
let cid = ChannelId.unique
let query = MentionSuggestionsSearch.channelMembersQuery(cid: cid, for: "abc")

XCTAssertEqual(query.cid, cid)
XCTAssertEqual(query.filter, .autocomplete(.name, text: "abc"))
XCTAssertEqual(query.sort, [.init(key: .name, isAscending: true)])
}

// MARK: - requiresRemoteMemberSearch

func test_requiresRemoteMemberSearch_whenMemberCountExceedsLastActiveMembers() {
let channel = ChatChannel.mock(
cid: .unique,
lastActiveMembers: [.mock(id: "a"), .mock(id: "b")],
memberCount: 1000
)

XCTAssertTrue(MentionSuggestionsSearch.requiresRemoteMemberSearch(for: channel))
}

func test_requiresRemoteMemberSearch_whenAllMembersAreLocal_returnsFalse() {
let members: [ChatChannelMember] = [.mock(id: "a"), .mock(id: "b")]
let channel = ChatChannel.mock(
cid: .unique,
lastActiveMembers: members,
memberCount: members.count
)

XCTAssertFalse(MentionSuggestionsSearch.requiresRemoteMemberSearch(for: channel))
}
}
Loading