From 219f8cffb9150341c91591a65576d2068337d106 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Mon, 5 Feb 2024 18:23:51 +0100 Subject: [PATCH 01/11] WIP --- .../GlobalSearchScreenViewModel.swift | 6 ++-- .../Screens/HomeScreen/HomeScreenModels.swift | 2 +- .../HomeScreen/HomeScreenViewModel.swift | 20 ++++++++--- .../MessageForwardingScreenViewModel.swift | 4 +-- .../Sources/Services/Client/ClientProxy.swift | 12 +++---- .../Sources/Services/Room/RoomProxy.swift | 18 ++++------ .../RoomSummary/MockRoomSummaryProvider.swift | 5 +-- .../RoomSummary/RoomSummaryProvider.swift | 34 ++++++++++++++++--- .../RoomSummaryProviderProtocol.swift | 4 +-- .../RoomTimelineController.swift | 5 +-- 10 files changed, 72 insertions(+), 38 deletions(-) diff --git a/ElementX/Sources/Screens/GlobalSearchScreen/GlobalSearchScreenViewModel.swift b/ElementX/Sources/Screens/GlobalSearchScreen/GlobalSearchScreenViewModel.swift index 643eb198be..06aa1528e8 100644 --- a/ElementX/Sources/Screens/GlobalSearchScreen/GlobalSearchScreenViewModel.swift +++ b/ElementX/Sources/Screens/GlobalSearchScreen/GlobalSearchScreenViewModel.swift @@ -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(.normalizedMatchRoomName(query: searchQuery, filters: [])) } .store(in: &cancellables) @@ -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(.all(filters: [])) // This is a shared provider case .select(let roomID): actionsSubject.send(.select(roomID: roomID)) case .reachedTop: diff --git a/ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift b/ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift index 17724e6dc0..9b10d5e2dc 100644 --- a/ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift +++ b/ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift @@ -238,7 +238,7 @@ enum RoomListFilter: Int, CaseIterable, Identifiable { } final class RoomListFiltersState: ObservableObject { - @Published private var enabledFilters: Set + @Published private(set) var enabledFilters: Set init(enabledFilters: Set = []) { self.enabledFilters = enabledFilters diff --git a/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift b/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift index 6deff19bcd..982493ceb9 100644 --- a/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift +++ b/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift @@ -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() + } else { + state.shouldShowFilters = true + } + } .store(in: &cancellables) appSettings.$markAsUnreadEnabled @@ -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 @@ -195,9 +206,10 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol roomSummaryProvider?.setFilter(.none) } else { if state.bindings.isSearchFieldFocused { - roomSummaryProvider?.setFilter(.normalizedMatchRoomName(state.bindings.searchQuery)) + roomSummaryProvider?.setFilter(.normalizedMatchRoomName(query: state.bindings.searchQuery, + filters: state.filtersState.enabledFilters)) } else { - roomSummaryProvider?.setFilter(.all) + roomSummaryProvider?.setFilter(.all(filters: state.filtersState.enabledFilters)) } } } diff --git a/ElementX/Sources/Screens/MessageForwardingScreen/MessageForwardingScreenViewModel.swift b/ElementX/Sources/Screens/MessageForwardingScreen/MessageForwardingScreenViewModel.swift index ae2ee78f7c..56ec5a0a12 100644 --- a/ElementX/Sources/Screens/MessageForwardingScreen/MessageForwardingScreenViewModel.swift +++ b/ElementX/Sources/Screens/MessageForwardingScreen/MessageForwardingScreenViewModel.swift @@ -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(.normalizedMatchRoomName(query: searchQuery, filters: [])) } .store(in: &cancellables) @@ -60,7 +60,7 @@ class MessageForwardingScreenViewModel: MessageForwardingScreenViewModelType, Me switch viewAction { case .cancel: actionsSubject.send(.dismiss) - roomSummaryProvider?.setFilter(.all) + roomSummaryProvider?.setFilter(.all(filters: [])) case .send: guard let roomID = state.selectedRoomID else { fatalError() diff --git a/ElementX/Sources/Services/Client/ClientProxy.swift b/ElementX/Sources/Services/Client/ClientProxy.swift index 5954499dd4..6bd93c02a8 100644 --- a/ElementX/Sources/Services/Client/ClientProxy.swift +++ b/ElementX/Sources/Services/Client/ClientProxy.swift @@ -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 @@ -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 { diff --git a/ElementX/Sources/Services/Room/RoomProxy.swift b/ElementX/Sources/Services/Room/RoomProxy.swift index 7f20be67dc..6d0642af22 100644 --- a/ElementX/Sources/Services/Room/RoomProxy.swift +++ b/ElementX/Sources/Services/Room/RoomProxy.swift @@ -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() @@ -130,7 +124,7 @@ class RoomProxy: RoomProxyProtocol { var canonicalAlias: String? { room.canonicalAlias() } - + var avatarURL: URL? { roomListItem.avatarUrl().flatMap(URL.init(string:)) } diff --git a/ElementX/Sources/Services/Room/RoomSummary/MockRoomSummaryProvider.swift b/ElementX/Sources/Services/Room/RoomSummary/MockRoomSummaryProvider.swift index 9c531efd69..45f94771dc 100644 --- a/ElementX/Sources/Services/Room/RoomSummary/MockRoomSummaryProvider.swift +++ b/ElementX/Sources/Services/Room/RoomSummary/MockRoomSummaryProvider.swift @@ -65,11 +65,12 @@ class MockRoomSummaryProvider: RoomSummaryProviderProtocol { roomListSubject.send(initialRooms) case .none: roomListSubject.send([]) + // TODO: improve this mock case .normalizedMatchRoomName(let filter): - if filter.isEmpty { + if filter.query.isEmpty { roomListSubject.send(initialRooms) } else { - roomListSubject.send(initialRooms.filter { $0.name?.localizedCaseInsensitiveContains(filter) ?? false }) + roomListSubject.send(initialRooms.filter { $0.name?.localizedCaseInsensitiveContains(filter.query) ?? false }) } } } diff --git a/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProvider.swift b/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProvider.swift index 3f296137bd..cd99a56cf6 100644 --- a/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProvider.swift +++ b/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProvider.swift @@ -102,7 +102,7 @@ class RoomSummaryProvider: RoomSummaryProviderProtocol { }) // Forces the listener above to be called with the current state - setFilter(.all) + setFilter(.all(filters: [])) listUpdatesTaskHandle = listUpdatesSubscriptionResult?.entriesStream @@ -153,10 +153,15 @@ class RoomSummaryProvider: RoomSummaryProviderProtocol { switch filter { case .none: _ = 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 .all(filters): + var filters = filters.compactMap(\.rustFilter) + filters.append(.nonLeft) + _ = listUpdatesSubscriptionResult?.controller.setFilter(kind: .all(filters: filters)) + case let .normalizedMatchRoomName(query, filters): + var filters = filters.compactMap(\.rustFilter) + filters.append(.normalizedMatchRoomName(pattern: query.lowercased())) + filters.append(.nonLeft) + _ = listUpdatesSubscriptionResult?.controller.setFilter(kind: .all(filters: filters)) } } @@ -438,3 +443,22 @@ private class RoomListStateObserver: RoomListLoadingStateListener { onUpdateClosure(state) } } + +private extension RoomListFilter { + var rustFilter: RoomListEntriesDynamicFilterKind? { + switch self { + case .people: + return .kind(expectedKind: .directMessages) + case .rooms: + return .kind(expectedKind: .group) + case .unreads: + return .unread + case .favourites: + // Not implemented yet + return nil + case .lowPriority: + // Not implemented yet + return nil + } + } +} diff --git a/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProviderProtocol.swift b/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProviderProtocol.swift index f33db5767e..57b2e2e54b 100644 --- a/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProviderProtocol.swift +++ b/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProviderProtocol.swift @@ -89,8 +89,8 @@ enum RoomSummary: CustomStringConvertible, Equatable { enum RoomSummaryProviderFilter { case none - case all - case normalizedMatchRoomName(String) + case all(filters: Set) + case normalizedMatchRoomName(query: String, filters: Set) } protocol RoomSummaryProviderProtocol { diff --git a/ElementX/Sources/Services/Timeline/TimelineController/RoomTimelineController.swift b/ElementX/Sources/Services/Timeline/TimelineController/RoomTimelineController.swift index a60f19d7be..fad74117f9 100644 --- a/ElementX/Sources/Services/Timeline/TimelineController/RoomTimelineController.swift +++ b/ElementX/Sources/Services/Timeline/TimelineController/RoomTimelineController.swift @@ -92,8 +92,9 @@ class RoomTimelineController: RoomTimelineControllerProtocol { } func sendReadReceipt(for itemID: TimelineItemIdentifier) async -> Result { - guard let eventID = itemID.eventID - else { return .success(()) } + guard let eventID = itemID.eventID else { + return .failure(.generic) + } switch await roomProxy.timeline.sendReadReceipt(for: eventID, type: appSettings.sendReadReceiptsEnabled ? .read : .readPrivate) { From 9a98bb92fabf068e3fb8777c1fc33393e16e2716 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Tue, 6 Feb 2024 10:46:00 +0100 Subject: [PATCH 02/11] removed unnecessary filter --- .../Sources/Screens/HomeScreen/HomeScreenModels.swift | 8 ++------ .../Services/Room/RoomSummary/RoomSummaryProvider.swift | 3 --- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift b/ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift index 9b10d5e2dc..b22bccb8e2 100644 --- a/ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift +++ b/ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift @@ -204,7 +204,6 @@ enum RoomListFilter: Int, CaseIterable, Identifiable { case rooms case unreads case favourites - case lowPriority var localizedName: String { switch self { @@ -216,8 +215,6 @@ enum RoomListFilter: Int, CaseIterable, Identifiable { return L10n.screenRoomlistFilterUnreads case .favourites: return L10n.screenRoomlistFilterFavourites - case .lowPriority: - return L10n.screenRoomlistFilterLowPriority } } @@ -230,9 +227,8 @@ 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 + return nil } } } diff --git a/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProvider.swift b/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProvider.swift index cd99a56cf6..a23a0da8d4 100644 --- a/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProvider.swift +++ b/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProvider.swift @@ -456,9 +456,6 @@ private extension RoomListFilter { case .favourites: // Not implemented yet return nil - case .lowPriority: - // Not implemented yet - return nil } } } From 95cd2da4b06bd8b81f5c424fa9a72af92c33bc7b Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Tue, 6 Feb 2024 12:01:08 +0100 Subject: [PATCH 03/11] Improved the filtering API - more readable filtering API - wrote some docs on its usage --- .../GlobalSearchScreenViewModel.swift | 4 ++-- .../HomeScreen/HomeScreenViewModel.swift | 8 +++---- .../MessageForwardingScreenViewModel.swift | 4 ++-- .../RoomSummary/MockRoomSummaryProvider.swift | 15 +++++------- .../RoomSummary/RoomSummaryProvider.swift | 17 +++++++------ .../RoomSummaryProviderProtocol.swift | 24 ++++++++++++++++--- 6 files changed, 43 insertions(+), 29 deletions(-) diff --git a/ElementX/Sources/Screens/GlobalSearchScreen/GlobalSearchScreenViewModel.swift b/ElementX/Sources/Screens/GlobalSearchScreen/GlobalSearchScreenViewModel.swift index 06aa1528e8..62420fddca 100644 --- a/ElementX/Sources/Screens/GlobalSearchScreen/GlobalSearchScreenViewModel.swift +++ b/ElementX/Sources/Screens/GlobalSearchScreen/GlobalSearchScreenViewModel.swift @@ -46,7 +46,7 @@ class GlobalSearchScreenViewModel: GlobalSearchScreenViewModelType, GlobalSearch .removeDuplicates() .sink { [weak self] searchQuery in // Not sure about this I imagine the global search should not care about filters? - self?.roomSummaryProvider.setFilter(.normalizedMatchRoomName(query: searchQuery, filters: [])) + self?.roomSummaryProvider.setFilter(.include(.init(query: searchQuery))) } .store(in: &cancellables) @@ -62,7 +62,7 @@ class GlobalSearchScreenViewModel: GlobalSearchScreenViewModelType, GlobalSearch case .dismiss: actionsSubject.send(.dismiss) // Also not sure about this one - roomSummaryProvider.setFilter(.all(filters: [])) // This is a shared provider + roomSummaryProvider.setFilter(.include(.all)) // This is a shared provider case .select(let roomID): actionsSubject.send(.select(roomID: roomID)) case .reachedTop: diff --git a/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift b/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift index 982493ceb9..91609461da 100644 --- a/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift +++ b/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift @@ -203,13 +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(query: state.bindings.searchQuery, - filters: state.filtersState.enabledFilters)) + roomSummaryProvider?.setFilter(.include(.init(query: state.bindings.searchQuery, + filters: state.filtersState.enabledFilters))) } else { - roomSummaryProvider?.setFilter(.all(filters: state.filtersState.enabledFilters)) + roomSummaryProvider?.setFilter(.include(.init(filters: state.filtersState.enabledFilters))) } } } diff --git a/ElementX/Sources/Screens/MessageForwardingScreen/MessageForwardingScreenViewModel.swift b/ElementX/Sources/Screens/MessageForwardingScreen/MessageForwardingScreenViewModel.swift index 56ec5a0a12..32ec813b48 100644 --- a/ElementX/Sources/Screens/MessageForwardingScreen/MessageForwardingScreenViewModel.swift +++ b/ElementX/Sources/Screens/MessageForwardingScreen/MessageForwardingScreenViewModel.swift @@ -49,7 +49,7 @@ class MessageForwardingScreenViewModel: MessageForwardingScreenViewModelType, Me .removeDuplicates() .sink { [weak self] searchQuery in guard let self else { return } - self.roomSummaryProvider?.setFilter(.normalizedMatchRoomName(query: searchQuery, filters: [])) + self.roomSummaryProvider?.setFilter(.include(.init(query: searchQuery))) } .store(in: &cancellables) @@ -60,7 +60,7 @@ class MessageForwardingScreenViewModel: MessageForwardingScreenViewModelType, Me switch viewAction { case .cancel: actionsSubject.send(.dismiss) - roomSummaryProvider?.setFilter(.all(filters: [])) + roomSummaryProvider?.setFilter(.include(.all)) case .send: guard let roomID = state.selectedRoomID else { fatalError() diff --git a/ElementX/Sources/Services/Room/RoomSummary/MockRoomSummaryProvider.swift b/ElementX/Sources/Services/Room/RoomSummary/MockRoomSummaryProvider.swift index 45f94771dc..e351985dc9 100644 --- a/ElementX/Sources/Services/Room/RoomSummary/MockRoomSummaryProvider.swift +++ b/ElementX/Sources/Services/Room/RoomSummary/MockRoomSummaryProvider.swift @@ -61,17 +61,14 @@ class MockRoomSummaryProvider: RoomSummaryProviderProtocol { func setFilter(_ filter: RoomSummaryProviderFilter) { switch filter { - case .all: - roomListSubject.send(initialRooms) - case .none: - roomListSubject.send([]) - // TODO: improve this mock - case .normalizedMatchRoomName(let filter): - if filter.query.isEmpty { - roomListSubject.send(initialRooms) + case let .include(filterLogic): + if let query = filterLogic.query { + roomListSubject.send(initialRooms.filter { $0.name?.localizedCaseInsensitiveContains(query) ?? false }) } else { - roomListSubject.send(initialRooms.filter { $0.name?.localizedCaseInsensitiveContains(filter.query) ?? false }) + roomListSubject.send(initialRooms) } + case .excludeAll: + roomListSubject.send([]) } } } diff --git a/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProvider.swift b/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProvider.swift index a23a0da8d4..8c749c43c1 100644 --- a/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProvider.swift +++ b/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProvider.swift @@ -102,7 +102,7 @@ class RoomSummaryProvider: RoomSummaryProviderProtocol { }) // Forces the listener above to be called with the current state - setFilter(.all(filters: [])) + setFilter(.include(.all)) listUpdatesTaskHandle = listUpdatesSubscriptionResult?.entriesStream @@ -151,15 +151,14 @@ class RoomSummaryProvider: RoomSummaryProviderProtocol { func setFilter(_ filter: RoomSummaryProviderFilter) { switch filter { - case .none: + case .excludeAll: _ = listUpdatesSubscriptionResult?.controller.setFilter(kind: .none) - case let .all(filters): - var filters = filters.compactMap(\.rustFilter) - filters.append(.nonLeft) - _ = listUpdatesSubscriptionResult?.controller.setFilter(kind: .all(filters: filters)) - case let .normalizedMatchRoomName(query, filters): - var filters = filters.compactMap(\.rustFilter) - filters.append(.normalizedMatchRoomName(pattern: query.lowercased())) + case let .include(filterLogic): + 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)) } diff --git a/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProviderProtocol.swift b/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProviderProtocol.swift index 57b2e2e54b..04228ff409 100644 --- a/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProviderProtocol.swift +++ b/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProviderProtocol.swift @@ -88,9 +88,27 @@ enum RoomSummary: CustomStringConvertible, Equatable { } enum RoomSummaryProviderFilter { - case none - case all(filters: Set) - case normalizedMatchRoomName(query: String, filters: Set) + struct Predicate { + let query: String? + let filters: Set + + 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 = []) { + 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 { From a2842a3aa113c3d24753caf48d517fdcd9496c26 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Tue, 6 Feb 2024 13:24:21 +0100 Subject: [PATCH 04/11] added tests --- .../Screens/HomeScreen/HomeScreenModels.swift | 5 ++ .../RoomSummary/MockRoomSummaryProvider.swift | 2 + .../RoomSummaryProviderProtocol.swift | 4 +- .../Sources/HomeScreenViewModelTests.swift | 19 ++++- .../Sources/RoomListFiltersStateTests.swift | 74 +++++++++++++++++++ 5 files changed, 101 insertions(+), 3 deletions(-) create mode 100644 UnitTests/Sources/RoomListFiltersStateTests.swift diff --git a/ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift b/ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift index b22bccb8e2..7b3427144f 100644 --- a/ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift +++ b/ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift @@ -261,6 +261,11 @@ final class RoomListFiltersState: ObservableObject { func set(_ filter: RoomListFilter, isEnabled: Bool) { if isEnabled { + if let complementaryFilter = filter.complementaryFilter, + enabledFilters.contains(complementaryFilter) { + MXLog.error("[RoomListFiltersState] adding mutually exclusive filters is not allowed") + return + } enabledFilters.insert(filter) } else { enabledFilters.remove(filter) diff --git a/ElementX/Sources/Services/Room/RoomSummary/MockRoomSummaryProvider.swift b/ElementX/Sources/Services/Room/RoomSummary/MockRoomSummaryProvider.swift index e351985dc9..620098e0b8 100644 --- a/ElementX/Sources/Services/Room/RoomSummary/MockRoomSummaryProvider.swift +++ b/ElementX/Sources/Services/Room/RoomSummary/MockRoomSummaryProvider.swift @@ -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> { @@ -60,6 +61,7 @@ class MockRoomSummaryProvider: RoomSummaryProviderProtocol { func updateVisibleRange(_ range: Range) { } func setFilter(_ filter: RoomSummaryProviderFilter) { + currentFilter = filter switch filter { case let .include(filterLogic): if let query = filterLogic.query { diff --git a/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProviderProtocol.swift b/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProviderProtocol.swift index 04228ff409..f8a3350507 100644 --- a/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProviderProtocol.swift +++ b/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProviderProtocol.swift @@ -87,8 +87,8 @@ enum RoomSummary: CustomStringConvertible, Equatable { } } -enum RoomSummaryProviderFilter { - struct Predicate { +enum RoomSummaryProviderFilter: Equatable { + struct Predicate: Equatable { let query: String? let filters: Set diff --git a/UnitTests/Sources/HomeScreenViewModelTests.swift b/UnitTests/Sources/HomeScreenViewModelTests.swift index 1d8196c03b..2b38ac5a48 100644 --- a/UnitTests/Sources/HomeScreenViewModelTests.swift +++ b/UnitTests/Sources/HomeScreenViewModelTests.swift @@ -25,10 +25,13 @@ class HomeScreenViewModelTests: XCTestCase { var clientProxy: MockClientProxy! var context: HomeScreenViewModelType.Context! { viewModel.context } var cancellables = Set() + 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()), @@ -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 @@ -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]))) + } } diff --git a/UnitTests/Sources/RoomListFiltersStateTests.swift b/UnitTests/Sources/RoomListFiltersStateTests.swift new file mode 100644 index 0000000000..1bf971e08b --- /dev/null +++ b/UnitTests/Sources/RoomListFiltersStateTests.swift @@ -0,0 +1,74 @@ +// +// Copyright 2024 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. +// + +import XCTest + +@testable import ElementX + +final class RoomListFiltersStateTests: XCTestCase { + var state: RoomListFiltersState! + + override func setUp() { + state = RoomListFiltersState() + } + + func testInitialState() { + XCTAssertFalse(state.isFiltering) + XCTAssertEqual(state.enabledFilters, []) + XCTAssertEqual(state.sortedAvailableFilters, RoomListFilter.allCases) + } + + func testSetAndUnsetFilters() { + state.set(.unreads, isEnabled: true) + XCTAssertTrue(state.isFiltering) + XCTAssertEqual(state.enabledFilters, [.unreads]) + XCTAssertEqual(state.sortedAvailableFilters, [.people, .rooms, .favourites]) + state.set(.unreads, isEnabled: false) + XCTAssertFalse(state.isFiltering) + XCTAssertEqual(state.enabledFilters, []) + XCTAssertEqual(state.sortedAvailableFilters, RoomListFilter.allCases) + } + + func testMutuallyExclusiveFilters() { + state.set(.people, isEnabled: true) + // This is not allowed and should do nothing + state.set(.rooms, isEnabled: true) + XCTAssertTrue(state.isFiltering) + XCTAssertEqual(state.enabledFilters, [.people]) + XCTAssertEqual(state.sortedAvailableFilters, [.unreads, .favourites]) + state.set(.people, isEnabled: false) + state.set(.rooms, isEnabled: true) + // This is not allowed and should do nothing + state.set(.people, isEnabled: true) + state.set(.unreads, isEnabled: true) + XCTAssertTrue(state.isFiltering) + XCTAssertEqual(state.enabledFilters, [.rooms, .unreads]) + XCTAssertEqual(state.sortedAvailableFilters, [.favourites]) + } + + func testClearFilters() { + state.set(.people, isEnabled: true) + state.set(.unreads, isEnabled: true) + state.set(.favourites, isEnabled: true) + XCTAssertTrue(state.isFiltering) + XCTAssertEqual(state.enabledFilters, [.people, .unreads, .favourites]) + XCTAssertEqual(state.sortedAvailableFilters, []) + state.clearFilters() + XCTAssertFalse(state.isFiltering) + XCTAssertEqual(state.enabledFilters, []) + XCTAssertEqual(state.sortedAvailableFilters, RoomListFilter.allCases) + } +} From a660505c6aeec9069fb486e9b7e4f0f208329ab9 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Tue, 6 Feb 2024 16:16:26 +0100 Subject: [PATCH 05/11] fixing tests --- .../Services/Room/RoomSummary/MockRoomSummaryProvider.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ElementX/Sources/Services/Room/RoomSummary/MockRoomSummaryProvider.swift b/ElementX/Sources/Services/Room/RoomSummary/MockRoomSummaryProvider.swift index 620098e0b8..4f8d27738f 100644 --- a/ElementX/Sources/Services/Room/RoomSummary/MockRoomSummaryProvider.swift +++ b/ElementX/Sources/Services/Room/RoomSummary/MockRoomSummaryProvider.swift @@ -64,7 +64,7 @@ class MockRoomSummaryProvider: RoomSummaryProviderProtocol { currentFilter = filter switch filter { case let .include(filterLogic): - if let query = filterLogic.query { + if let query = filterLogic.query, !query.isEmpty { roomListSubject.send(initialRooms.filter { $0.name?.localizedCaseInsensitiveContains(query) ?? false }) } else { roomListSubject.send(initialRooms) From ee5aa0c4e2401601f4f6d30a3a602e39736828d3 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Tue, 6 Feb 2024 16:20:22 +0100 Subject: [PATCH 06/11] restored proj --- ElementX.xcodeproj/project.pbxproj | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ElementX.xcodeproj/project.pbxproj b/ElementX.xcodeproj/project.pbxproj index 53a447301c..65ce39fd9f 100644 --- a/ElementX.xcodeproj/project.pbxproj +++ b/ElementX.xcodeproj/project.pbxproj @@ -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 */; }; @@ -1565,6 +1566,7 @@ 8977176AB534AA41630395BC /* LegalInformationScreenViewModelProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LegalInformationScreenViewModelProtocol.swift; sourceTree = ""; }; 897DF5E9A70CE05A632FC8AF /* UTType.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UTType.swift; sourceTree = ""; }; 8AB10FA6570DD08B3966C159 /* WelcomeScreenScreenUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WelcomeScreenScreenUITests.swift; sourceTree = ""; }; + 8AE0C9653870803E4F91F474 /* RoomListFiltersStateTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RoomListFiltersStateTests.swift; sourceTree = ""; }; 8AE78FA0011E07920AE83135 /* PlainMentionBuilder.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PlainMentionBuilder.swift; sourceTree = ""; }; 8AFCE895ECFFA53FEE64D62B /* MediaLoader.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MediaLoader.swift; sourceTree = ""; }; 8BEBF0E59F25E842EDB6FD11 /* LocationSharingScreenModels.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LocationSharingScreenModels.swift; sourceTree = ""; }; @@ -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 */, @@ -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 */, From 6dfbb73904a7ed229a67343003a8ba900d21e679 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Tue, 6 Feb 2024 16:38:30 +0100 Subject: [PATCH 07/11] solved some conflict issues. --- ElementX/Sources/Services/Client/ClientProxy.swift | 12 ++++++------ ElementX/Sources/Services/Room/RoomProxy.swift | 13 +++++++++---- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/ElementX/Sources/Services/Client/ClientProxy.swift b/ElementX/Sources/Services/Client/ClientProxy.swift index 6bd93c02a8..efc85c2aee 100644 --- a/ElementX/Sources/Services/Client/ClientProxy.swift +++ b/ElementX/Sources/Services/Client/ClientProxy.swift @@ -313,9 +313,9 @@ class ClientProxy: ClientProxyProtocol { var (roomListItem, room) = await roomTupleForIdentifier(identifier) if let roomListItem, let room { - return try? 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 @@ -341,9 +341,9 @@ class ClientProxy: ClientProxyProtocol { return nil } - return try? await RoomProxy(roomListItem: roomListItem, - room: room, - backgroundTaskService: backgroundTaskService) + return try await RoomProxy(roomListItem: roomListItem, + room: room, + backgroundTaskService: backgroundTaskService) } func loadUserDisplayName() async -> Result { diff --git a/ElementX/Sources/Services/Room/RoomProxy.swift b/ElementX/Sources/Services/Room/RoomProxy.swift index 6d0642af22..bccf35983d 100644 --- a/ElementX/Sources/Services/Room/RoomProxy.swift +++ b/ElementX/Sources/Services/Room/RoomProxy.swift @@ -50,13 +50,18 @@ class RoomProxy: RoomProxyProtocol { room.ownUserId() } - init(roomListItem: RoomListItemProtocol, - room: RoomProtocol, - backgroundTaskService: BackgroundTaskServiceProtocol) async throws { + init?(roomListItem: RoomListItemProtocol, + room: RoomProtocol, + backgroundTaskService: BackgroundTaskServiceProtocol) async { self.roomListItem = roomListItem self.room = room self.backgroundTaskService = backgroundTaskService - timeline = try await TimelineProxy(timeline: room.timeline(), backgroundTaskService: backgroundTaskService) + do { + timeline = try await TimelineProxy(timeline: room.timeline(), backgroundTaskService: backgroundTaskService) + } catch { + MXLog.error("Failed creating timeline with error: \(error)") + return nil + } Task { await updateMembers() From cdb6bcf2d12dcf5dda53abfbac47a82b6be93609 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Wed, 7 Feb 2024 12:17:38 +0100 Subject: [PATCH 08/11] adressing pr comments renaming --- .../GlobalSearchScreenViewModel.swift | 2 - .../Screens/HomeScreen/HomeScreenModels.swift | 86 ------------- .../HomeScreen/HomeScreenViewModel.swift | 6 +- .../View/Filters/RoomListFilterModels.swift | 119 ++++++++++++++++++ .../View/Filters/RoomListFilterView.swift | 6 +- .../View/Filters/RoomListFiltersView.swift | 6 +- .../Sources/Services/Client/ClientProxy.swift | 12 +- .../RoomSummary/MockRoomSummaryProvider.swift | 4 +- .../RoomSummary/RoomSummaryProvider.swift | 16 --- .../Sources/HomeScreenViewModelTests.swift | 2 +- .../Sources/RoomListFiltersStateTests.swift | 50 ++++---- 11 files changed, 160 insertions(+), 149 deletions(-) create mode 100644 ElementX/Sources/Screens/HomeScreen/View/Filters/RoomListFilterModels.swift diff --git a/ElementX/Sources/Screens/GlobalSearchScreen/GlobalSearchScreenViewModel.swift b/ElementX/Sources/Screens/GlobalSearchScreen/GlobalSearchScreenViewModel.swift index 62420fddca..d151a443df 100644 --- a/ElementX/Sources/Screens/GlobalSearchScreen/GlobalSearchScreenViewModel.swift +++ b/ElementX/Sources/Screens/GlobalSearchScreen/GlobalSearchScreenViewModel.swift @@ -45,7 +45,6 @@ class GlobalSearchScreenViewModel: GlobalSearchScreenViewModelType, GlobalSearch .map(\.bindings.searchQuery) .removeDuplicates() .sink { [weak self] searchQuery in - // Not sure about this I imagine the global search should not care about filters? self?.roomSummaryProvider.setFilter(.include(.init(query: searchQuery))) } .store(in: &cancellables) @@ -61,7 +60,6 @@ class GlobalSearchScreenViewModel: GlobalSearchScreenViewModelType, GlobalSearch switch viewAction { case .dismiss: actionsSubject.send(.dismiss) - // Also not sure about this one roomSummaryProvider.setFilter(.include(.all)) // This is a shared provider case .select(let roomID): actionsSubject.send(.select(roomID: roomID)) diff --git a/ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift b/ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift index 7b3427144f..d34438eac7 100644 --- a/ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift +++ b/ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift @@ -194,89 +194,3 @@ struct HomeScreenRoom: Identifiable, Equatable { isPlaceholder: true) } } - -enum RoomListFilter: Int, CaseIterable, Identifiable { - var id: Int { - rawValue - } - - case people - case rooms - case unreads - case favourites - - var localizedName: String { - switch self { - case .people: - return L10n.screenRoomlistFilterPeople - case .rooms: - return L10n.screenRoomlistFilterRooms - case .unreads: - return L10n.screenRoomlistFilterUnreads - case .favourites: - return L10n.screenRoomlistFilterFavourites - } - } - - var complementaryFilter: RoomListFilter? { - switch self { - case .people: - return .rooms - case .rooms: - return .people - case .unreads: - return nil - case .favourites: - // When we will have Low Priority we may need to return it here - return nil - } - } -} - -final class RoomListFiltersState: ObservableObject { - @Published private(set) var enabledFilters: Set - - init(enabledFilters: Set = []) { - self.enabledFilters = enabledFilters - } - - var sortedEnabledFilters: [RoomListFilter] { - enabledFilters.sorted(by: { $0.rawValue < $1.rawValue }) - } - - var sortedAvailableFilters: [RoomListFilter] { - var availableFilters = Set(RoomListFilter.allCases) - for filter in enabledFilters { - availableFilters.remove(filter) - if let complementaryFilter = filter.complementaryFilter { - availableFilters.remove(complementaryFilter) - } - } - return availableFilters.sorted(by: { $0.rawValue < $1.rawValue }) - } - - var isFiltering: Bool { - !enabledFilters.isEmpty - } - - func set(_ filter: RoomListFilter, isEnabled: Bool) { - if isEnabled { - if let complementaryFilter = filter.complementaryFilter, - enabledFilters.contains(complementaryFilter) { - MXLog.error("[RoomListFiltersState] adding mutually exclusive filters is not allowed") - return - } - enabledFilters.insert(filter) - } else { - enabledFilters.remove(filter) - } - } - - func clearFilters() { - enabledFilters.removeAll() - } - - func isEnabled(_ filter: RoomListFilter) -> Bool { - enabledFilters.contains(filter) - } -} diff --git a/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift b/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift index 91609461da..a0d1db02c8 100644 --- a/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift +++ b/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift @@ -102,7 +102,7 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol let isSearchFieldFocused = context.$viewState.map(\.bindings.isSearchFieldFocused) let searchQuery = context.$viewState.map(\.bindings.searchQuery) - let enabledFilters = context.viewState.filtersState.$enabledFilters + let enabledFilters = context.viewState.filtersState.$activeFilters isSearchFieldFocused .combineLatest(searchQuery, enabledFilters) .removeDuplicates { $0 == $1 } @@ -207,9 +207,9 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol } else { if state.bindings.isSearchFieldFocused { roomSummaryProvider?.setFilter(.include(.init(query: state.bindings.searchQuery, - filters: state.filtersState.enabledFilters))) + filters: state.filtersState.activeFilters))) } else { - roomSummaryProvider?.setFilter(.include(.init(filters: state.filtersState.enabledFilters))) + roomSummaryProvider?.setFilter(.include(.init(filters: state.filtersState.activeFilters))) } } } diff --git a/ElementX/Sources/Screens/HomeScreen/View/Filters/RoomListFilterModels.swift b/ElementX/Sources/Screens/HomeScreen/View/Filters/RoomListFilterModels.swift new file mode 100644 index 0000000000..cd52a7e227 --- /dev/null +++ b/ElementX/Sources/Screens/HomeScreen/View/Filters/RoomListFilterModels.swift @@ -0,0 +1,119 @@ +// +// Copyright 2024 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. +// + +import Combine +import Foundation + +import MatrixRustSDK + +enum RoomListFilter: Int, CaseIterable, Identifiable { + var id: Int { + rawValue + } + + case people + case rooms + case unreads + case favourites + + var localizedName: String { + switch self { + case .people: + return L10n.screenRoomlistFilterPeople + case .rooms: + return L10n.screenRoomlistFilterRooms + case .unreads: + return L10n.screenRoomlistFilterUnreads + case .favourites: + return L10n.screenRoomlistFilterFavourites + } + } + + var incompatibleFilter: RoomListFilter? { + switch self { + case .people: + return .rooms + case .rooms: + return .people + case .unreads: + return nil + case .favourites: + // When we will have Low Priority we may need to return it here + return nil + } + } + + var rustFilter: RoomListEntriesDynamicFilterKind? { + switch self { + case .people: + return .category(expect: .people) + case .rooms: + return .category(expect: .group) + case .unreads: + return .unread + case .favourites: + // Not implemented yet + return nil + } + } +} + +final class RoomListFiltersState: ObservableObject { + @Published private(set) var activeFilters: Set + + init(activeFilters: Set = []) { + self.activeFilters = activeFilters + } + + var sortedActiveFilters: [RoomListFilter] { + activeFilters.sorted(by: { $0.rawValue < $1.rawValue }) + } + + var availableFilters: [RoomListFilter] { + var availableFilters = Set(RoomListFilter.allCases) + for filter in activeFilters { + availableFilters.remove(filter) + if let complementaryFilter = filter.incompatibleFilter { + availableFilters.remove(complementaryFilter) + } + } + return availableFilters.sorted(by: { $0.rawValue < $1.rawValue }) + } + + var isFiltering: Bool { + !activeFilters.isEmpty + } + + func activateFilter(_ filter: RoomListFilter) { + if let incompatibleFilter = filter.incompatibleFilter, + activeFilters.contains(incompatibleFilter) { + fatalError("[RoomListFiltersState] adding mutually exclusive filters is not allowed") + } + activeFilters.insert(filter) + } + + func deactivateFilter(_ filter: RoomListFilter) { + activeFilters.remove(filter) + } + + func clearFilters() { + activeFilters.removeAll() + } + + func isFilterActive(_ filter: RoomListFilter) -> Bool { + activeFilters.contains(filter) + } +} diff --git a/ElementX/Sources/Screens/HomeScreen/View/Filters/RoomListFilterView.swift b/ElementX/Sources/Screens/HomeScreen/View/Filters/RoomListFilterView.swift index e12d2cc563..41de9681aa 100644 --- a/ElementX/Sources/Screens/HomeScreen/View/Filters/RoomListFilterView.swift +++ b/ElementX/Sources/Screens/HomeScreen/View/Filters/RoomListFilterView.swift @@ -22,9 +22,9 @@ struct RoomListFilterView: View { var body: some View { let binding = Binding(get: { - state.isEnabled(filter) + state.isFilterActive(filter) }, set: { isEnabled, _ in - state.set(filter, isEnabled: isEnabled) + isEnabled ? state.activateFilter(filter) : state.deactivateFilter(filter) }) Toggle(isOn: binding) { Text(filter.localizedName) @@ -36,7 +36,7 @@ struct RoomListFilterView: View { struct RoomListFilterView_Previews: PreviewProvider, TestablePreview { static var previews: some View { RoomListFilterView(filter: .people, state: .init()) - RoomListFilterView(filter: .people, state: .init(enabledFilters: [.people])) + RoomListFilterView(filter: .people, state: .init(activeFilters: [.people])) } } diff --git a/ElementX/Sources/Screens/HomeScreen/View/Filters/RoomListFiltersView.swift b/ElementX/Sources/Screens/HomeScreen/View/Filters/RoomListFiltersView.swift index 48d6c2cfa8..f46f130307 100644 --- a/ElementX/Sources/Screens/HomeScreen/View/Filters/RoomListFiltersView.swift +++ b/ElementX/Sources/Screens/HomeScreen/View/Filters/RoomListFiltersView.swift @@ -31,10 +31,10 @@ struct RoomListFiltersView: View { .hidden() .frame(width: 0) } - ForEach(state.sortedEnabledFilters) { filter in + ForEach(state.sortedActiveFilters) { filter in RoomListFilterView(filter: filter, state: state) } - ForEach(state.sortedAvailableFilters) { filter in + ForEach(state.availableFilters) { filter in RoomListFilterView(filter: filter, state: state) } } @@ -62,6 +62,6 @@ struct RoomListFiltersView: View { struct RoomListFiltersView_Previews: PreviewProvider, TestablePreview { static var previews: some View { RoomListFiltersView(state: .init()) - RoomListFiltersView(state: .init(enabledFilters: [.rooms, .favourites])) + RoomListFiltersView(state: .init(activeFilters: [.rooms, .favourites])) } } diff --git a/ElementX/Sources/Services/Client/ClientProxy.swift b/ElementX/Sources/Services/Client/ClientProxy.swift index efc85c2aee..5954499dd4 100644 --- a/ElementX/Sources/Services/Client/ClientProxy.swift +++ b/ElementX/Sources/Services/Client/ClientProxy.swift @@ -313,9 +313,9 @@ class ClientProxy: ClientProxyProtocol { var (roomListItem, room) = await roomTupleForIdentifier(identifier) if let roomListItem, let room { - return try await RoomProxy(roomListItem: roomListItem, - room: room, - backgroundTaskService: backgroundTaskService) + return await RoomProxy(roomListItem: roomListItem, + room: room, + backgroundTaskService: backgroundTaskService) } // Else wait for the visible rooms list to go into fully loaded @@ -341,9 +341,9 @@ class ClientProxy: ClientProxyProtocol { return nil } - return try await RoomProxy(roomListItem: roomListItem, - room: room, - backgroundTaskService: backgroundTaskService) + return await RoomProxy(roomListItem: roomListItem, + room: room, + backgroundTaskService: backgroundTaskService) } func loadUserDisplayName() async -> Result { diff --git a/ElementX/Sources/Services/Room/RoomSummary/MockRoomSummaryProvider.swift b/ElementX/Sources/Services/Room/RoomSummary/MockRoomSummaryProvider.swift index 4f8d27738f..338462de05 100644 --- a/ElementX/Sources/Services/Room/RoomSummary/MockRoomSummaryProvider.swift +++ b/ElementX/Sources/Services/Room/RoomSummary/MockRoomSummaryProvider.swift @@ -63,8 +63,8 @@ class MockRoomSummaryProvider: RoomSummaryProviderProtocol { func setFilter(_ filter: RoomSummaryProviderFilter) { currentFilter = filter switch filter { - case let .include(filterLogic): - if let query = filterLogic.query, !query.isEmpty { + case let .include(predicate): + if let query = predicate.query, !query.isEmpty { roomListSubject.send(initialRooms.filter { $0.name?.localizedCaseInsensitiveContains(query) ?? false }) } else { roomListSubject.send(initialRooms) diff --git a/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProvider.swift b/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProvider.swift index 8c749c43c1..c7f4d28bd1 100644 --- a/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProvider.swift +++ b/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProvider.swift @@ -442,19 +442,3 @@ private class RoomListStateObserver: RoomListLoadingStateListener { onUpdateClosure(state) } } - -private extension RoomListFilter { - var rustFilter: RoomListEntriesDynamicFilterKind? { - switch self { - case .people: - return .kind(expectedKind: .directMessages) - case .rooms: - return .kind(expectedKind: .group) - case .unreads: - return .unread - case .favourites: - // Not implemented yet - return nil - } - } -} diff --git a/UnitTests/Sources/HomeScreenViewModelTests.swift b/UnitTests/Sources/HomeScreenViewModelTests.swift index 2b38ac5a48..fa180c3ff9 100644 --- a/UnitTests/Sources/HomeScreenViewModelTests.swift +++ b/UnitTests/Sources/HomeScreenViewModelTests.swift @@ -162,7 +162,7 @@ class HomeScreenViewModelTests: XCTestCase { } func testFilters() async throws { - context.viewState.filtersState.set(.people, isEnabled: true) + context.viewState.filtersState.activateFilter(.people) try await Task.sleep(for: .milliseconds(100)) XCTAssertEqual(roomSummaryProvider.currentFilter, RoomSummaryProviderFilter.include(.init(filters: [.people]))) context.isSearchFieldFocused = true diff --git a/UnitTests/Sources/RoomListFiltersStateTests.swift b/UnitTests/Sources/RoomListFiltersStateTests.swift index 1bf971e08b..ca0629c5d4 100644 --- a/UnitTests/Sources/RoomListFiltersStateTests.swift +++ b/UnitTests/Sources/RoomListFiltersStateTests.swift @@ -27,48 +27,44 @@ final class RoomListFiltersStateTests: XCTestCase { func testInitialState() { XCTAssertFalse(state.isFiltering) - XCTAssertEqual(state.enabledFilters, []) - XCTAssertEqual(state.sortedAvailableFilters, RoomListFilter.allCases) + XCTAssertEqual(state.activeFilters, []) + XCTAssertEqual(state.availableFilters, RoomListFilter.allCases) } func testSetAndUnsetFilters() { - state.set(.unreads, isEnabled: true) + state.activateFilter(.unreads) XCTAssertTrue(state.isFiltering) - XCTAssertEqual(state.enabledFilters, [.unreads]) - XCTAssertEqual(state.sortedAvailableFilters, [.people, .rooms, .favourites]) - state.set(.unreads, isEnabled: false) + XCTAssertEqual(state.activeFilters, [.unreads]) + XCTAssertEqual(state.availableFilters, [.people, .rooms, .favourites]) + state.deactivateFilter(.unreads) XCTAssertFalse(state.isFiltering) - XCTAssertEqual(state.enabledFilters, []) - XCTAssertEqual(state.sortedAvailableFilters, RoomListFilter.allCases) + XCTAssertEqual(state.activeFilters, []) + XCTAssertEqual(state.availableFilters, RoomListFilter.allCases) } func testMutuallyExclusiveFilters() { - state.set(.people, isEnabled: true) - // This is not allowed and should do nothing - state.set(.rooms, isEnabled: true) + state.activateFilter(.people) XCTAssertTrue(state.isFiltering) - XCTAssertEqual(state.enabledFilters, [.people]) - XCTAssertEqual(state.sortedAvailableFilters, [.unreads, .favourites]) - state.set(.people, isEnabled: false) - state.set(.rooms, isEnabled: true) - // This is not allowed and should do nothing - state.set(.people, isEnabled: true) - state.set(.unreads, isEnabled: true) + XCTAssertEqual(state.activeFilters, [.people]) + XCTAssertEqual(state.availableFilters, [.unreads, .favourites]) + state.deactivateFilter(.people) + state.activateFilter(.rooms) + state.activateFilter(.unreads) XCTAssertTrue(state.isFiltering) - XCTAssertEqual(state.enabledFilters, [.rooms, .unreads]) - XCTAssertEqual(state.sortedAvailableFilters, [.favourites]) + XCTAssertEqual(state.activeFilters, [.rooms, .unreads]) + XCTAssertEqual(state.availableFilters, [.favourites]) } func testClearFilters() { - state.set(.people, isEnabled: true) - state.set(.unreads, isEnabled: true) - state.set(.favourites, isEnabled: true) + state.activateFilter(.people) + state.activateFilter(.unreads) + state.activateFilter(.favourites) XCTAssertTrue(state.isFiltering) - XCTAssertEqual(state.enabledFilters, [.people, .unreads, .favourites]) - XCTAssertEqual(state.sortedAvailableFilters, []) + XCTAssertEqual(state.activeFilters, [.people, .unreads, .favourites]) + XCTAssertEqual(state.availableFilters, []) state.clearFilters() XCTAssertFalse(state.isFiltering) - XCTAssertEqual(state.enabledFilters, []) - XCTAssertEqual(state.sortedAvailableFilters, RoomListFilter.allCases) + XCTAssertEqual(state.activeFilters, []) + XCTAssertEqual(state.availableFilters, RoomListFilter.allCases) } } From 7e633967ac226246b9fea981928400808ec4946b Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Wed, 7 Feb 2024 13:22:56 +0100 Subject: [PATCH 09/11] renamed a variable --- .../Services/Room/RoomSummary/RoomSummaryProvider.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProvider.swift b/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProvider.swift index c7f4d28bd1..c998ce2759 100644 --- a/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProvider.swift +++ b/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProvider.swift @@ -153,9 +153,9 @@ class RoomSummaryProvider: RoomSummaryProviderProtocol { switch filter { case .excludeAll: _ = listUpdatesSubscriptionResult?.controller.setFilter(kind: .none) - case let .include(filterLogic): - var filters = filterLogic.filters.compactMap(\.rustFilter) - if let query = filterLogic.query { + case let .include(predicate): + var filters = predicate.filters.compactMap(\.rustFilter) + if let query = predicate.query { filters.append(.normalizedMatchRoomName(pattern: query.lowercased())) } // We never want to show left rooms. From 0c3290c4a12a07aacf291bd04998aa246745c7c4 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Thu, 8 Feb 2024 16:11:20 +0100 Subject: [PATCH 10/11] bump sdk --- ElementX.xcodeproj/project.pbxproj | 6 +++++- .../xcshareddata/swiftpm/Package.resolved | 4 ++-- project.yml | 2 +- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/ElementX.xcodeproj/project.pbxproj b/ElementX.xcodeproj/project.pbxproj index aa46e5fc9b..c0b1a3947b 100644 --- a/ElementX.xcodeproj/project.pbxproj +++ b/ElementX.xcodeproj/project.pbxproj @@ -969,6 +969,7 @@ F421FD5979EF53C8204BDC77 /* SecureBackupLogoutConfirmationScreenUITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1CC09F30B0E1010951952BDC /* SecureBackupLogoutConfirmationScreenUITests.swift */; }; F4433EF57B4BB3C077F8B00E /* SessionVerificationScreenViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = ADD9E0FFA29EAACFF3AB9732 /* SessionVerificationScreenViewModel.swift */; }; F4971845B5C4F270F6BC5745 /* ScaledFrameModifier.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5D82F234B3576BD6268C7950 /* ScaledFrameModifier.swift */; }; + F4996C82A4B3A5FF0C8EDD03 /* RoomListFilterModels.swift in Sources */ = {isa = PBXBuildFile; fileRef = E06AAD6D9D3F5833E7A5A2F9 /* RoomListFilterModels.swift */; }; F508683B76EF7B23BB2CBD6D /* TimelineItemPlainStylerView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 94BCC8A9C73C1F838122C645 /* TimelineItemPlainStylerView.swift */; }; F50A6FCE26714E27FE5495DD /* PollOptionView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 50F23B21CF15F9F4BAA0788B /* PollOptionView.swift */; }; F519DE17A3A0F760307B2E6D /* InviteUsersScreenViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02D155E09BF961BBA8F85263 /* InviteUsersScreenViewModel.swift */; }; @@ -1885,6 +1886,7 @@ DF05DA24F71B455E8EFEBC3B /* SessionVerificationViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SessionVerificationViewModelTests.swift; sourceTree = ""; }; DF3D25B3EDB283B5807EADCF /* ReadMarkerRoomTimelineItem.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ReadMarkerRoomTimelineItem.swift; sourceTree = ""; }; E062C1750EFC8627DE4CAB8E /* MapTilerAuthorization.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MapTilerAuthorization.swift; sourceTree = ""; }; + E06AAD6D9D3F5833E7A5A2F9 /* RoomListFilterModels.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RoomListFilterModels.swift; sourceTree = ""; }; E0FCA0957FAA0E15A9F5579D /* en */ = {isa = PBXFileReference; lastKnownFileType = text.plist.stringsdict; name = en; path = en.lproj/Untranslated.stringsdict; sourceTree = ""; }; E10DA51DBC8C7E1460DBCCBD /* UserProfileListRow.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UserProfileListRow.swift; sourceTree = ""; }; E1573D28C8A9FB6399D0EEFB /* SecureBackupLogoutConfirmationScreenCoordinator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SecureBackupLogoutConfirmationScreenCoordinator.swift; sourceTree = ""; }; @@ -2132,6 +2134,7 @@ 037A5661B26EC6BE068188D7 /* Filters */ = { isa = PBXGroup; children = ( + E06AAD6D9D3F5833E7A5A2F9 /* RoomListFilterModels.swift */, E6372DD10DED30E7AD7BCE21 /* RoomListFiltersView.swift */, 24EC819497BB5F8C4998D760 /* RoomListFilterView.swift */, ); @@ -5800,6 +5803,7 @@ 42F1C8731166633E35A6D7E6 /* RoomEventStringBuilder.swift in Sources */, D55AF9B5B55FEED04771A461 /* RoomFlowCoordinator.swift in Sources */, 04A16B45228F7678A027C079 /* RoomHeaderView.swift in Sources */, + F4996C82A4B3A5FF0C8EDD03 /* RoomListFilterModels.swift in Sources */, 4A9CEEE612D6D8B3DDBD28BA /* RoomListFilterView.swift in Sources */, 33F1FB19F222BA9930AB1A00 /* RoomListFiltersView.swift in Sources */, FA4296218444C48BC890F46B /* RoomMemberDetails.swift in Sources */, @@ -6767,7 +6771,7 @@ repositoryURL = "https://github.com/matrix-org/matrix-rust-components-swift"; requirement = { kind = exactVersion; - version = 1.1.37; + version = 1.1.38; }; }; 821C67C9A7F8CC3FD41B28B4 /* XCRemoteSwiftPackageReference "emojibase-bindings" */ = { diff --git a/ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 0ec0a7b921..a724ef7b27 100644 --- a/ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -129,8 +129,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/matrix-org/matrix-rust-components-swift", "state" : { - "revision" : "4d0d75004f8361530d7424ab198e363027823718", - "version" : "1.1.37" + "revision" : "691d8b0f0994d9669fadbd2452bef7270f3713ad", + "version" : "1.1.38" } }, { diff --git a/project.yml b/project.yml index 1c0460c5a8..8fbfdad88a 100644 --- a/project.yml +++ b/project.yml @@ -47,7 +47,7 @@ packages: # Element/Matrix dependencies MatrixRustSDK: url: https://github.com/matrix-org/matrix-rust-components-swift - exactVersion: 1.1.37 + exactVersion: 1.1.38 # path: ../matrix-rust-sdk Compound: url: https://github.com/element-hq/compound-ios From e55ba8ee78daa52773dc9e9534da2fee8a59f3ec Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Thu, 8 Feb 2024 16:14:53 +0100 Subject: [PATCH 11/11] changelog --- changelog.d/pr-2423.wip | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/pr-2423.wip diff --git a/changelog.d/pr-2423.wip b/changelog.d/pr-2423.wip new file mode 100644 index 0000000000..c4ffd5a823 --- /dev/null +++ b/changelog.d/pr-2423.wip @@ -0,0 +1 @@ +All Filters have been implemented, except for the Favourites one. \ No newline at end of file