Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Room List Filters implementation #2423

Merged
merged 17 commits into from
Feb 8, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
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
4 changes: 4 additions & 0 deletions ElementX.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@
4BB51476A29E7E27BC14EA22 /* UserDetailsEditScreenViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 022E6BD64CB4610B9C95FC02 /* UserDetailsEditScreenViewModel.swift */; };
4C356F5CCB4CDC99BFA45185 /* AppLockSetupPINScreenViewModelProtocol.swift in Sources */ = {isa = PBXBuildFile; fileRef = B7884BD256C091EB511B2EDF /* AppLockSetupPINScreenViewModelProtocol.swift */; };
4C5A638DAA8AF64565BA4866 /* EncryptedRoomTimelineItem.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5351EBD7A0B9610548E4B7B2 /* EncryptedRoomTimelineItem.swift */; };
4C8C0C9FC10BA73AB7780534 /* RoomListFiltersStateTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8AE0C9653870803E4F91F474 /* RoomListFiltersStateTests.swift */; };
4E0D9E09B52CEC4C0E6211A8 /* MediaPickerScreenCoordinator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 64F49FB9EE2913234F06CE68 /* MediaPickerScreenCoordinator.swift */; };
4E8A2A2CFEB212F14E49E1A1 /* AppLockSetupSettingsScreen.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5484457C81B325660901B161 /* AppLockSetupSettingsScreen.swift */; };
4E8F17EBA24FBBA6ABB62ECB /* MockBackgroundTaskService.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3948D16F021DFDB2CD26EAA8 /* MockBackgroundTaskService.swift */; };
Expand Down Expand Up @@ -1565,6 +1566,7 @@
8977176AB534AA41630395BC /* LegalInformationScreenViewModelProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LegalInformationScreenViewModelProtocol.swift; sourceTree = "<group>"; };
897DF5E9A70CE05A632FC8AF /* UTType.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UTType.swift; sourceTree = "<group>"; };
8AB10FA6570DD08B3966C159 /* WelcomeScreenScreenUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WelcomeScreenScreenUITests.swift; sourceTree = "<group>"; };
8AE0C9653870803E4F91F474 /* RoomListFiltersStateTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RoomListFiltersStateTests.swift; sourceTree = "<group>"; };
8AE78FA0011E07920AE83135 /* PlainMentionBuilder.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PlainMentionBuilder.swift; sourceTree = "<group>"; };
8AFCE895ECFFA53FEE64D62B /* MediaLoader.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MediaLoader.swift; sourceTree = "<group>"; };
8BEBF0E59F25E842EDB6FD11 /* LocationSharingScreenModels.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LocationSharingScreenModels.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -3343,6 +3345,7 @@
00E5B2CBEF8F96424F095508 /* RoomDetailsEditScreenViewModelTests.swift */,
2EFE1922F39398ABFB36DF3F /* RoomDetailsViewModelTests.swift */,
4FCB2126C091EEF2454B4D56 /* RoomFlowCoordinatorTests.swift */,
8AE0C9653870803E4F91F474 /* RoomListFiltersStateTests.swift */,
EC589E641AE46EFB2962534D /* RoomMemberDetailsViewModelTests.swift */,
69B63F817FE305548DB4B512 /* RoomMembersListViewModelTests.swift */,
58D295F0081084F38DB20893 /* RoomNotificationSettingsScreenViewModelTests.swift */,
Expand Down Expand Up @@ -5328,6 +5331,7 @@
9DD84E014ADFB2DD813022D5 /* RoomDetailsEditScreenViewModelTests.swift in Sources */,
EA974337FA7D040E7C74FE6E /* RoomDetailsViewModelTests.swift in Sources */,
095D3906CF2F940C2D2D17CC /* RoomFlowCoordinatorTests.swift in Sources */,
4C8C0C9FC10BA73AB7780534 /* RoomListFiltersStateTests.swift in Sources */,
6B31508C6334C617360C2EAB /* RoomMemberDetailsViewModelTests.swift in Sources */,
CAF8755E152204F55F8D6B5B /* RoomMembersListViewModelTests.swift in Sources */,
E49F74BD93230BDEFFE5EA51 /* RoomNotificationSettingsScreenViewModelTests.swift in Sources */,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ class GlobalSearchScreenViewModel: GlobalSearchScreenViewModelType, GlobalSearch
.map(\.bindings.searchQuery)
.removeDuplicates()
.sink { [weak self] searchQuery in
self?.roomSummaryProvider.setFilter(.normalizedMatchRoomName(searchQuery))
// Not sure about this I imagine the global search should not care about filters?
self?.roomSummaryProvider.setFilter(.include(.init(query: searchQuery)))
Velin92 marked this conversation as resolved.
Show resolved Hide resolved
}
.store(in: &cancellables)

Expand All @@ -60,7 +61,8 @@ class GlobalSearchScreenViewModel: GlobalSearchScreenViewModelType, GlobalSearch
switch viewAction {
case .dismiss:
actionsSubject.send(.dismiss)
roomSummaryProvider.setFilter(.all) // This is a shared provider
// Also not sure about this one
roomSummaryProvider.setFilter(.include(.all)) // This is a shared provider
Velin92 marked this conversation as resolved.
Show resolved Hide resolved
case .select(let roomID):
actionsSubject.send(.select(roomID: roomID))
case .reachedTop:
Expand Down
15 changes: 8 additions & 7 deletions ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift
Velin92 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,6 @@ enum RoomListFilter: Int, CaseIterable, Identifiable {
case rooms
case unreads
case favourites
case lowPriority
Velin92 marked this conversation as resolved.
Show resolved Hide resolved

var localizedName: String {
switch self {
Expand All @@ -216,8 +215,6 @@ enum RoomListFilter: Int, CaseIterable, Identifiable {
return L10n.screenRoomlistFilterUnreads
case .favourites:
return L10n.screenRoomlistFilterFavourites
case .lowPriority:
return L10n.screenRoomlistFilterLowPriority
}
}

Expand All @@ -230,15 +227,14 @@ enum RoomListFilter: Int, CaseIterable, Identifiable {
case .unreads:
return nil
case .favourites:
return .lowPriority
case .lowPriority:
return .favourites
// When we will have Low Priority we may need to return it here
Velin92 marked this conversation as resolved.
Show resolved Hide resolved
return nil
}
}
}

final class RoomListFiltersState: ObservableObject {
Velin92 marked this conversation as resolved.
Show resolved Hide resolved
@Published private var enabledFilters: Set<RoomListFilter>
@Published private(set) var enabledFilters: Set<RoomListFilter>
Velin92 marked this conversation as resolved.
Show resolved Hide resolved

init(enabledFilters: Set<RoomListFilter> = []) {
Velin92 marked this conversation as resolved.
Show resolved Hide resolved
self.enabledFilters = enabledFilters
Expand All @@ -265,6 +261,11 @@ final class RoomListFiltersState: ObservableObject {

func set(_ filter: RoomListFilter, isEnabled: Bool) {
Velin92 marked this conversation as resolved.
Show resolved Hide resolved
if isEnabled {
if let complementaryFilter = filter.complementaryFilter,
enabledFilters.contains(complementaryFilter) {
MXLog.error("[RoomListFiltersState] adding mutually exclusive filters is not allowed")
Velin92 marked this conversation as resolved.
Show resolved Hide resolved
return
}
enabledFilters.insert(filter)
} else {
enabledFilters.remove(filter)
Expand Down
22 changes: 17 additions & 5 deletions ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,17 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol
.store(in: &cancellables)

appSettings.$roomListFiltersEnabled
.weakAssign(to: \.state.shouldShowFilters, on: self)
.sink { [weak self] value in
guard let self else {
return
}
if !value {
state.shouldShowFilters = false
state.filtersState.clearFilters()
Velin92 marked this conversation as resolved.
Show resolved Hide resolved
} else {
state.shouldShowFilters = true
}
}
.store(in: &cancellables)

appSettings.$markAsUnreadEnabled
Expand All @@ -92,8 +102,9 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol

let isSearchFieldFocused = context.$viewState.map(\.bindings.isSearchFieldFocused)
let searchQuery = context.$viewState.map(\.bindings.searchQuery)
let enabledFilters = context.viewState.filtersState.$enabledFilters
isSearchFieldFocused
.combineLatest(searchQuery)
.combineLatest(searchQuery, enabledFilters)
.removeDuplicates { $0 == $1 }
.map { _ in () }
.sink { [weak self] in
Expand Down Expand Up @@ -192,12 +203,13 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol

private func updateFilter() {
if state.shouldHideRoomList {
roomSummaryProvider?.setFilter(.none)
roomSummaryProvider?.setFilter(.excludeAll)
} else {
if state.bindings.isSearchFieldFocused {
roomSummaryProvider?.setFilter(.normalizedMatchRoomName(state.bindings.searchQuery))
roomSummaryProvider?.setFilter(.include(.init(query: state.bindings.searchQuery,
filters: state.filtersState.enabledFilters)))
} else {
roomSummaryProvider?.setFilter(.all)
roomSummaryProvider?.setFilter(.include(.init(filters: state.filtersState.enabledFilters)))
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class MessageForwardingScreenViewModel: MessageForwardingScreenViewModelType, Me
.removeDuplicates()
.sink { [weak self] searchQuery in
guard let self else { return }
self.roomSummaryProvider?.setFilter(.normalizedMatchRoomName(searchQuery))
self.roomSummaryProvider?.setFilter(.include(.init(query: searchQuery)))
}
.store(in: &cancellables)

Expand All @@ -60,7 +60,7 @@ class MessageForwardingScreenViewModel: MessageForwardingScreenViewModelType, Me
switch viewAction {
case .cancel:
actionsSubject.send(.dismiss)
roomSummaryProvider?.setFilter(.all)
roomSummaryProvider?.setFilter(.include(.all))
case .send:
guard let roomID = state.selectedRoomID else {
fatalError()
Expand Down
12 changes: 6 additions & 6 deletions ElementX/Sources/Services/Client/ClientProxy.swift
Original file line number Diff line number Diff line change
Expand Up @@ -313,9 +313,9 @@ class ClientProxy: ClientProxyProtocol {
var (roomListItem, room) = await roomTupleForIdentifier(identifier)

if let roomListItem, let room {
return await RoomProxy(roomListItem: roomListItem,
room: room,
backgroundTaskService: backgroundTaskService)
return try? await RoomProxy(roomListItem: roomListItem,
room: room,
backgroundTaskService: backgroundTaskService)
}

// Else wait for the visible rooms list to go into fully loaded
Expand All @@ -341,9 +341,9 @@ class ClientProxy: ClientProxyProtocol {
return nil
}

return await RoomProxy(roomListItem: roomListItem,
room: room,
backgroundTaskService: backgroundTaskService)
return try? await RoomProxy(roomListItem: roomListItem,
room: room,
backgroundTaskService: backgroundTaskService)
}

func loadUserDisplayName() async -> Result<Void, ClientProxyError> {
Expand Down
18 changes: 6 additions & 12 deletions ElementX/Sources/Services/Room/RoomProxy.swift
Original file line number Diff line number Diff line change
Expand Up @@ -49,20 +49,14 @@ class RoomProxy: RoomProxyProtocol {
var ownUserID: String {
room.ownUserId()
}
init?(roomListItem: RoomListItemProtocol,
room: RoomProtocol,
backgroundTaskService: BackgroundTaskServiceProtocol) async {

init(roomListItem: RoomListItemProtocol,
room: RoomProtocol,
backgroundTaskService: BackgroundTaskServiceProtocol) async throws {
self.roomListItem = roomListItem
self.room = room
self.backgroundTaskService = backgroundTaskService

do {
timeline = try await TimelineProxy(timeline: room.timeline(), backgroundTaskService: backgroundTaskService)
} catch {
MXLog.error("Failed creating timeline with error: \(error)")
return nil
}
timeline = try await TimelineProxy(timeline: room.timeline(), backgroundTaskService: backgroundTaskService)

Task {
await updateMembers()
Expand Down Expand Up @@ -130,7 +124,7 @@ class RoomProxy: RoomProxyProtocol {
var canonicalAlias: String? {
room.canonicalAlias()
}

var avatarURL: URL? {
roomListItem.avatarUrl().flatMap(URL.init(string:))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ enum MockRoomSummaryProviderState {

class MockRoomSummaryProvider: RoomSummaryProviderProtocol {
private let initialRooms: [RoomSummary]
private(set) var currentFilter: RoomSummaryProviderFilter?

private let roomListSubject: CurrentValueSubject<[RoomSummary], Never>
var roomListPublisher: CurrentValuePublisher<[RoomSummary], Never> {
Expand Down Expand Up @@ -60,17 +61,16 @@ class MockRoomSummaryProvider: RoomSummaryProviderProtocol {
func updateVisibleRange(_ range: Range<Int>) { }

func setFilter(_ filter: RoomSummaryProviderFilter) {
currentFilter = filter
switch filter {
case .all:
roomListSubject.send(initialRooms)
case .none:
roomListSubject.send([])
case .normalizedMatchRoomName(let filter):
if filter.isEmpty {
roomListSubject.send(initialRooms)
case let .include(filterLogic):
if let query = filterLogic.query, !query.isEmpty {
roomListSubject.send(initialRooms.filter { $0.name?.localizedCaseInsensitiveContains(query) ?? false })
} else {
roomListSubject.send(initialRooms.filter { $0.name?.localizedCaseInsensitiveContains(filter) ?? false })
roomListSubject.send(initialRooms)
}
case .excludeAll:
roomListSubject.send([])
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ class RoomSummaryProvider: RoomSummaryProviderProtocol {
})

// Forces the listener above to be called with the current state
setFilter(.all)
setFilter(.include(.all))

listUpdatesTaskHandle = listUpdatesSubscriptionResult?.entriesStream

Expand Down Expand Up @@ -151,12 +151,16 @@ class RoomSummaryProvider: RoomSummaryProviderProtocol {

func setFilter(_ filter: RoomSummaryProviderFilter) {
switch filter {
case .none:
case .excludeAll:
_ = listUpdatesSubscriptionResult?.controller.setFilter(kind: .none)
case .all:
_ = listUpdatesSubscriptionResult?.controller.setFilter(kind: .allNonLeft)
case .normalizedMatchRoomName(let query):
_ = listUpdatesSubscriptionResult?.controller.setFilter(kind: .normalizedMatchRoomName(pattern: query.lowercased()))
case let .include(filterLogic):
Velin92 marked this conversation as resolved.
Show resolved Hide resolved
var filters = filterLogic.filters.compactMap(\.rustFilter)
if let query = filterLogic.query {
filters.append(.normalizedMatchRoomName(pattern: query.lowercased()))
}
// We never want to show left rooms.
filters.append(.nonLeft)
_ = listUpdatesSubscriptionResult?.controller.setFilter(kind: .all(filters: filters))
}
}

Expand Down Expand Up @@ -438,3 +442,19 @@ private class RoomListStateObserver: RoomListLoadingStateListener {
onUpdateClosure(state)
}
}

private extension RoomListFilter {
Velin92 marked this conversation as resolved.
Show resolved Hide resolved
var rustFilter: RoomListEntriesDynamicFilterKind? {
switch self {
case .people:
Velin92 marked this conversation as resolved.
Show resolved Hide resolved
return .kind(expectedKind: .directMessages)
Velin92 marked this conversation as resolved.
Show resolved Hide resolved
case .rooms:
return .kind(expectedKind: .group)
Velin92 marked this conversation as resolved.
Show resolved Hide resolved
Velin92 marked this conversation as resolved.
Show resolved Hide resolved
case .unreads:
return .unread
case .favourites:
// Not implemented yet
return nil
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,28 @@ enum RoomSummary: CustomStringConvertible, Equatable {
}
}

enum RoomSummaryProviderFilter {
case none
case all
case normalizedMatchRoomName(String)
enum RoomSummaryProviderFilter: Equatable {
Velin92 marked this conversation as resolved.
Show resolved Hide resolved
struct Predicate: Equatable {
let query: String?
let filters: Set<RoomListFilter>

static var all: Predicate {
Predicate()
}

/// - Parameters:
/// - query: If provided the filter will do a normalized search, default is nil
/// - filters: Additional filters that can be provided for further filtering the room list, default is empty which means no additional filtering is done
init(query: String? = nil, filters: Set<RoomListFilter> = []) {
self.query = query
self.filters = filters
}
}

/// Filters out everything
case excludeAll
/// Includes only the items that satisfy the predicate logic
case include(Predicate)
}

protocol RoomSummaryProviderProtocol {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,9 @@ class RoomTimelineController: RoomTimelineControllerProtocol {
}

func sendReadReceipt(for itemID: TimelineItemIdentifier) async -> Result<Void, RoomTimelineControllerError> {
guard let eventID = itemID.eventID
else { return .success(()) }
guard let eventID = itemID.eventID else {
return .failure(.generic)
Velin92 marked this conversation as resolved.
Show resolved Hide resolved
}

switch await roomProxy.timeline.sendReadReceipt(for: eventID,
type: appSettings.sendReadReceiptsEnabled ? .read : .readPrivate) {
Expand Down
19 changes: 18 additions & 1 deletion UnitTests/Sources/HomeScreenViewModelTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,13 @@ class HomeScreenViewModelTests: XCTestCase {
var clientProxy: MockClientProxy!
var context: HomeScreenViewModelType.Context! { viewModel.context }
var cancellables = Set<AnyCancellable>()
var roomSummaryProvider: MockRoomSummaryProvider!

override func setUpWithError() throws {
ServiceLocator.shared.settings.roomListFiltersEnabled = true
cancellables.removeAll()
clientProxy = MockClientProxy(userID: "@mock:client.com")
roomSummaryProvider = MockRoomSummaryProvider(state: .loaded(.mockRooms))
clientProxy = MockClientProxy(userID: "@mock:client.com", roomSummaryProvider: roomSummaryProvider)
viewModel = HomeScreenViewModel(userSession: MockUserSession(clientProxy: clientProxy,
mediaProvider: MockMediaProvider(),
voiceMessageMediaManager: VoiceMessageMediaManagerMock()),
Expand All @@ -37,6 +40,10 @@ class HomeScreenViewModelTests: XCTestCase {
userIndicatorController: ServiceLocator.shared.userIndicatorController)
}

override func tearDown() {
AppSettings.reset()
}

func testSelectRoom() async throws {
let mockRoomId = "mock_room_id"
var correctResult = false
Expand Down Expand Up @@ -153,4 +160,14 @@ class HomeScreenViewModelTests: XCTestCase {
XCTAssertNil(context.alertInfo)
XCTAssertTrue(correctResult)
}

func testFilters() async throws {
context.viewState.filtersState.set(.people, isEnabled: true)
try await Task.sleep(for: .milliseconds(100))
XCTAssertEqual(roomSummaryProvider.currentFilter, RoomSummaryProviderFilter.include(.init(filters: [.people])))
context.isSearchFieldFocused = true
context.searchQuery = "Test"
try await Task.sleep(for: .milliseconds(100))
XCTAssertEqual(roomSummaryProvider.currentFilter, RoomSummaryProviderFilter.include(.init(query: "Test", filters: [.people])))
}
}
Loading
Loading