From f91b7441e7f7e539a8c10908824b5b45d242da77 Mon Sep 17 00:00:00 2001 From: Mauro <34335419+Velin92@users.noreply.github.com> Date: Fri, 2 Oct 2026 12:04:07 +0200 Subject: [PATCH] Suggest recently visited rooms when forwarding messages (#6251) * Suggest recently visited rooms when forwarding messages The Forward to screen now shows up to 5 recently visited rooms in a Suggestions section, above a Chats section that lists every room. The suggestions are never searched: while searching, only the Chats section is shown and filtered. The source room, invites, spaces and upgraded rooms are never suggested, and the section is hidden when there is nothing to suggest. The suggestions are resolved from the room summaries rather than room proxies, so they show up without holding back the list, and they are re-resolved as the room list loads after a cold start. ClientProxy now exposes the recently visited room IDs for this. The recently visited rooms filter is now async, so callers choose its isolation instead of asserting it at runtime. * Apply review suggestions to the forwarding suggestions Rooms: - Each room is listed once. Suggested rooms are left out of Chats unless searching, where Chats lists every result. This also removes the duplicated row whose selection changed without the tapped row's animation. - The Chats header is hidden when the section has no rooms. Suggestions: - Resolve them once when the screen opens instead of on every room list update. --- .../en-US.lproj/Localizable.strings | 1 + .../en.lproj/Localizable.strings | 1 + ElementX/Sources/Generated/Strings.swift | 2 + ElementX/Sources/Mocks/ClientProxyMock.swift | 1 + .../Mocks/Generated/GeneratedMocks.swift | 32 ++++- .../Mocks/RoomSummaryProviderMock.swift | 3 +- .../MessageForwardingScreenModels.swift | 23 ++++ .../MessageForwardingScreenViewModel.swift | 31 ++--- .../View/MessageForwardingScreen.swift | 75 +++++++++--- .../SpaceAddRoomsScreenViewModel.swift | 13 +-- .../Sources/Services/Client/ClientProxy.swift | 24 ++-- .../Services/Client/ClientProxyProtocol.swift | 4 +- ...eForwardingScreen.Searching-iPad-en-GB.png | 3 + ...ForwardingScreen.Searching-iPad-pseudo.png | 3 + ...orwardingScreen.Searching-iPhone-en-GB.png | 3 + ...rwardingScreen.Searching-iPhone-pseudo.png | 3 + ...orwardingScreen.Suggestions-iPad-en-GB.png | 3 + ...rwardingScreen.Suggestions-iPad-pseudo.png | 3 + ...wardingScreen.Suggestions-iPhone-en-GB.png | 3 + ...ardingScreen.Suggestions-iPhone-pseudo.png | 3 + .../messageForwardingScreen.iPad-en-GB-0.png | 3 - .../messageForwardingScreen.iPad-pseudo-0.png | 3 - ...messageForwardingScreen.iPhone-en-GB-0.png | 3 - ...essageForwardingScreen.iPhone-pseudo-0.png | 3 - ...essageForwardingScreenViewModelTests.swift | 109 +++++++++++++++++- 25 files changed, 287 insertions(+), 68 deletions(-) create mode 100644 PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.Searching-iPad-en-GB.png create mode 100644 PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.Searching-iPad-pseudo.png create mode 100644 PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.Searching-iPhone-en-GB.png create mode 100644 PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.Searching-iPhone-pseudo.png create mode 100644 PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.Suggestions-iPad-en-GB.png create mode 100644 PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.Suggestions-iPad-pseudo.png create mode 100644 PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.Suggestions-iPhone-en-GB.png create mode 100644 PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.Suggestions-iPhone-pseudo.png delete mode 100644 PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.iPad-en-GB-0.png delete mode 100644 PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.iPad-pseudo-0.png delete mode 100644 PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.iPhone-en-GB-0.png delete mode 100644 PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.iPhone-pseudo-0.png diff --git a/ElementX/Resources/Localizations/en-US.lproj/Localizable.strings b/ElementX/Resources/Localizations/en-US.lproj/Localizable.strings index 4568ff530..01a86c86b 100644 --- a/ElementX/Resources/Localizations/en-US.lproj/Localizable.strings +++ b/ElementX/Resources/Localizations/en-US.lproj/Localizable.strings @@ -240,6 +240,7 @@ "common_call_started" = "Call started"; "common_call_you_declined" = "You declined a call"; "common_chat_backup" = "Chat backup"; +"common_chats" = "Chats"; "common_code_block" = "Code block"; "common_copied_to_clipboard" = "Copied to clipboard"; "common_copyright" = "Copyright"; diff --git a/ElementX/Resources/Localizations/en.lproj/Localizable.strings b/ElementX/Resources/Localizations/en.lproj/Localizable.strings index 108d6415e..6b3dd3806 100644 --- a/ElementX/Resources/Localizations/en.lproj/Localizable.strings +++ b/ElementX/Resources/Localizations/en.lproj/Localizable.strings @@ -240,6 +240,7 @@ "common_call_started" = "Call started"; "common_call_you_declined" = "You declined a call"; "common_chat_backup" = "Chat backup"; +"common_chats" = "Chats"; "common_code_block" = "Code block"; "common_copied_to_clipboard" = "Copied to clipboard"; "common_copyright" = "Copyright"; diff --git a/ElementX/Sources/Generated/Strings.swift b/ElementX/Sources/Generated/Strings.swift index f8f1e4fa1..2b1bc9cf7 100644 --- a/ElementX/Sources/Generated/Strings.swift +++ b/ElementX/Sources/Generated/Strings.swift @@ -536,6 +536,8 @@ internal nonisolated enum L10n { internal static var commonCallYouDeclined: String { return L10n.tr("Localizable", "common_call_you_declined") } /// Chat backup internal static var commonChatBackup: String { return L10n.tr("Localizable", "common_chat_backup") } + /// Chats + internal static var commonChats: String { return L10n.tr("Localizable", "common_chats") } /// Code block internal static var commonCodeBlock: String { return L10n.tr("Localizable", "common_code_block") } /// Copied to clipboard diff --git a/ElementX/Sources/Mocks/ClientProxyMock.swift b/ElementX/Sources/Mocks/ClientProxyMock.swift index bfde32478..b0962a392 100644 --- a/ElementX/Sources/Mocks/ClientProxyMock.swift +++ b/ElementX/Sources/Mocks/ClientProxyMock.swift @@ -106,6 +106,7 @@ extension ClientProxyMock { unignoreUserReturnValue = .success(()) trackRecentlyVisitedRoomReturnValue = .success(()) + recentlyVisitedRoomIDsReturnValue = .success([]) recentlyVisitedRoomsFilterReturnValue = [] recentConversationCounterpartsReturnValue = [] diff --git a/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift b/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift index 58f4fcba3..fa6da126c 100644 --- a/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift +++ b/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift @@ -4225,6 +4225,34 @@ nonisolated class ClientProxyMock: ClientProxyProtocol, @unchecked Sendable { return trackRecentlyVisitedRoomReturnValue } } + //MARK: - recentlyVisitedRoomIDs + + private let recentlyVisitedRoomIDsCallsCountLock = NSLock() + private nonisolated(unsafe) var recentlyVisitedRoomIDsUnderlyingCallsCount = 0 + var recentlyVisitedRoomIDsCallsCount: Int { + get { recentlyVisitedRoomIDsCallsCountLock.withLock { recentlyVisitedRoomIDsUnderlyingCallsCount } } + set { recentlyVisitedRoomIDsCallsCountLock.withLock { recentlyVisitedRoomIDsUnderlyingCallsCount = newValue } } + } + var recentlyVisitedRoomIDsCalled: Bool { + return recentlyVisitedRoomIDsCallsCount > 0 + } + + private let recentlyVisitedRoomIDsReturnValueLock = NSLock() + private nonisolated(unsafe) var recentlyVisitedRoomIDsUnderlyingReturnValue: Result<[String], ClientProxyError>! + var recentlyVisitedRoomIDsReturnValue: Result<[String], ClientProxyError>! { + get { recentlyVisitedRoomIDsReturnValueLock.withLock { recentlyVisitedRoomIDsUnderlyingReturnValue } } + set { recentlyVisitedRoomIDsReturnValueLock.withLock { recentlyVisitedRoomIDsUnderlyingReturnValue = newValue } } + } + nonisolated(unsafe) var recentlyVisitedRoomIDsClosure: (() async -> Result<[String], ClientProxyError>)? + + @concurrent func recentlyVisitedRoomIDs() async -> Result<[String], ClientProxyError> { + recentlyVisitedRoomIDsCallsCountLock.withLock { recentlyVisitedRoomIDsUnderlyingCallsCount += 1 } + if let recentlyVisitedRoomIDsClosure = recentlyVisitedRoomIDsClosure { + return await recentlyVisitedRoomIDsClosure() + } else { + return recentlyVisitedRoomIDsReturnValue + } + } //MARK: - recentlyVisitedRooms private let recentlyVisitedRoomsFilterCallsCountLock = NSLock() @@ -4243,9 +4271,9 @@ nonisolated class ClientProxyMock: ClientProxyProtocol, @unchecked Sendable { get { recentlyVisitedRoomsFilterReturnValueLock.withLock { recentlyVisitedRoomsFilterUnderlyingReturnValue } } set { recentlyVisitedRoomsFilterReturnValueLock.withLock { recentlyVisitedRoomsFilterUnderlyingReturnValue = newValue } } } - nonisolated(unsafe) var recentlyVisitedRoomsFilterClosure: ((@Sendable (JoinedRoomProxyProtocol) -> Bool) async -> [JoinedRoomProxyProtocol])? + nonisolated(unsafe) var recentlyVisitedRoomsFilterClosure: ((@Sendable (JoinedRoomProxyProtocol) async -> Bool) async -> [JoinedRoomProxyProtocol])? - @concurrent func recentlyVisitedRooms(filter: @Sendable (JoinedRoomProxyProtocol) -> Bool) async -> [JoinedRoomProxyProtocol] { + @concurrent func recentlyVisitedRooms(filter: @Sendable (JoinedRoomProxyProtocol) async -> Bool) async -> [JoinedRoomProxyProtocol] { recentlyVisitedRoomsFilterCallsCountLock.withLock { recentlyVisitedRoomsFilterUnderlyingCallsCount += 1 } if let recentlyVisitedRoomsFilterClosure = recentlyVisitedRoomsFilterClosure { return await recentlyVisitedRoomsFilterClosure(filter) diff --git a/ElementX/Sources/Mocks/RoomSummaryProviderMock.swift b/ElementX/Sources/Mocks/RoomSummaryProviderMock.swift index fd41cae10..ff1f376b1 100644 --- a/ElementX/Sources/Mocks/RoomSummaryProviderMock.swift +++ b/ElementX/Sources/Mocks/RoomSummaryProviderMock.swift @@ -72,13 +72,14 @@ extension RoomSummaryProviderMock { extension RoomSummary { static func mock(id: String, name: String, + isSpace: Bool = false, canonicalAlias: String? = nil) -> RoomSummary { RoomSummary(room: RoomSDKMock(.init()), id: id, joinRequestType: nil, name: name, isDirect: false, - isSpace: false, + isSpace: isSpace, avatarURL: nil, heroes: [], activeMembersCount: 0, diff --git a/ElementX/Sources/Screens/MessageForwardingScreen/MessageForwardingScreenModels.swift b/ElementX/Sources/Screens/MessageForwardingScreen/MessageForwardingScreenModels.swift index 664bf68e7..b32423c61 100644 --- a/ElementX/Sources/Screens/MessageForwardingScreen/MessageForwardingScreenModels.swift +++ b/ElementX/Sources/Screens/MessageForwardingScreen/MessageForwardingScreenModels.swift @@ -15,11 +15,25 @@ enum MessageForwardingScreenViewModelAction { } struct MessageForwardingScreenViewState: BindableState { + /// The recently visited rooms, never filtered by the search query. + var suggestedRooms: [MessageForwardingRoom] = [] var rooms: [MessageForwardingRoom] = [] var selectedRoomIDs: Set = [] let maxRoomSelectionCount = 5 var bindings = MessageForwardingScreenViewStateBindings() + var showsSuggestions: Bool { + bindings.searchQuery.isEmpty && !suggestedRooms.isEmpty + } + + /// Every room appears once: the suggested ones are left out unless searching. + var chats: [MessageForwardingRoom] { + guard showsSuggestions else { return rooms } + + let suggestedRoomIDs = Set(suggestedRooms.map(\.id)) + return rooms.filter { !suggestedRoomIDs.contains($0.id) } + } + var isAtRoomSelectionLimit: Bool { selectedRoomIDs.count >= maxRoomSelectionCount } @@ -44,6 +58,15 @@ struct MessageForwardingRoom: Identifiable, Equatable { let avatar: RoomAvatar } +extension MessageForwardingRoom { + init(summary: RoomSummary) { + self.init(id: summary.id, + title: summary.name, + description: summary.roomListDescription, + avatar: summary.avatar) + } +} + struct MessageForwardingPayload: Hashable { /// The source items' timeline IDs. Only necessary for a rough Hashable conformance. let ids: [TimelineItemIdentifier] diff --git a/ElementX/Sources/Screens/MessageForwardingScreen/MessageForwardingScreenViewModel.swift b/ElementX/Sources/Screens/MessageForwardingScreen/MessageForwardingScreenViewModel.swift index 63c87aadc..873dff16e 100644 --- a/ElementX/Sources/Screens/MessageForwardingScreen/MessageForwardingScreenViewModel.swift +++ b/ElementX/Sources/Screens/MessageForwardingScreen/MessageForwardingScreenViewModel.swift @@ -12,6 +12,8 @@ import SwiftUI typealias MessageForwardingScreenViewModelType = StateStoreViewModel class MessageForwardingScreenViewModel: MessageForwardingScreenViewModelType, MessageForwardingScreenViewModelProtocol { + private static let maxSuggestedRoomCount = 5 + private let forwardingPayload: MessageForwardingPayload private let clientProxy: ClientProxyProtocol private let roomSummaryProvider: RoomSummaryProviderProtocol @@ -54,6 +56,8 @@ class MessageForwardingScreenViewModel: MessageForwardingScreenViewModelType, Me .store(in: &cancellables) updateRooms() + + Task { await loadSuggestedRooms() } } override func process(viewAction: MessageForwardingScreenViewAction) { @@ -83,20 +87,21 @@ class MessageForwardingScreenViewModel: MessageForwardingScreenViewModelType, Me // MARK: - Private private func updateRooms() { - var rooms = [MessageForwardingRoom]() + state.rooms = roomSummaryProvider.roomListPublisher.value + .filter { $0.id != forwardingPayload.roomID } + .map(MessageForwardingRoom.init(summary:)) + } + + /// Built from the room summaries: building room proxies is slow and would shift the list down when the section appears. + private func loadSuggestedRooms() async { + guard case let .success(roomIDs) = await clientProxy.recentlyVisitedRoomIDs() else { return } - for summary in roomSummaryProvider.roomListPublisher.value { - if summary.id == forwardingPayload.roomID { - continue - } - - rooms.append(.init(id: summary.id, - title: summary.name, - description: summary.roomListDescription, - avatar: summary.avatar)) - } - - state.rooms = rooms + state.suggestedRooms = roomIDs + .filter { $0 != forwardingPayload.roomID } + .compactMap(clientProxy.roomSummaryForIdentifier) + .filter { $0.joinRequestType == nil && !$0.isSpace && !$0.isTombstoned } + .prefix(Self.maxSuggestedRoomCount) + .map(MessageForwardingRoom.init(summary:)) } /// The actual range values don't matter as long as they contain the lower diff --git a/ElementX/Sources/Screens/MessageForwardingScreen/View/MessageForwardingScreen.swift b/ElementX/Sources/Screens/MessageForwardingScreen/View/MessageForwardingScreen.swift index 160b9d257..3f0d0fd45 100644 --- a/ElementX/Sources/Screens/MessageForwardingScreen/View/MessageForwardingScreen.swift +++ b/ElementX/Sources/Screens/MessageForwardingScreen/View/MessageForwardingScreen.swift @@ -15,19 +15,26 @@ struct MessageForwardingScreen: View { var body: some View { Form { - Section { - ForEach(context.viewState.rooms) { room in - MessageForwardingListRow(room: room, - isSelected: context.viewState.selectedRoomIDs.contains(room.id), - isDisabled: context.viewState.isAtRoomSelectionLimit && !context.viewState.selectedRoomIDs.contains(room.id), - context: context) + if context.viewState.showsSuggestions { + Section { + ForEach(context.viewState.suggestedRooms, content: row) + } header: { + Text(L10n.commonSuggestions) + .compoundListSectionHeader() } + } + + Section { + ForEach(context.viewState.chats, content: row) // Replace these with ScrollView's `scrollPosition` when dropping iOS 16. } header: { - emptyRectangle - .onAppear { - context.send(viewAction: .reachedTop) - } + if !context.viewState.chats.isEmpty { + Text(L10n.commonChats) + .compoundListSectionHeader() + .onAppear { + context.send(viewAction: .reachedTop) + } + } } footer: { emptyRectangle .onAppear { @@ -56,6 +63,15 @@ struct MessageForwardingScreen: View { .disableAutocorrection(true) } + private func row(for room: MessageForwardingRoom) -> some View { + let isSelected = context.viewState.selectedRoomIDs.contains(room.id) + + return MessageForwardingListRow(room: room, + isSelected: isSelected, + isDisabled: context.viewState.isAtRoomSelectionLimit && !isSelected, + context: context) + } + /// The greedy size of Rectangle can create an issue with the navigation bar when the search is highlighted, so is best to use a fixed frame instead of hidden() or EmptyView() private var emptyRectangle: some View { Rectangle() @@ -97,17 +113,40 @@ private struct MessageForwardingListRow: View { // MARK: - Previews struct MessageForwardingScreen_Previews: PreviewProvider, TestablePreview { + static let viewModel = makeViewModel() + static let searchingViewModel = makeViewModel(searchQuery: "Foundation") + static var previews: some View { - let summaryProvider = RoomSummaryProviderMock(.init(state: .loaded(.mockRooms))) - let viewModel = MessageForwardingScreenViewModel(forwardingPayload: .init(ids: [.randomEvent], - roomID: "", - contents: [RoomMessageEventContentWithoutRelationSDKMock()]), - userSession: UserSessionMock(.init()), - roomSummaryProvider: summaryProvider, - userIndicatorController: UserIndicatorControllerMock()) - ElementNavigationStack { MessageForwardingScreen(context: viewModel.context) } + .previewDisplayName("Suggestions") + .snapshotPreferences(expect: viewModel.context.$viewState.map(\.showsSuggestions)) + + ElementNavigationStack { + MessageForwardingScreen(context: searchingViewModel.context) + } + .previewDisplayName("Searching") + .snapshotPreferences(expect: searchingViewModel.context.$viewState.map { !$0.suggestedRooms.isEmpty && !$0.showsSuggestions }, + precision: 0.999) // The search field's clear button renders inconsistently in snapshots. + } + + static func makeViewModel(searchQuery: String? = nil) -> MessageForwardingScreenViewModel { + let clientProxy = ClientProxyMock(.init()) + clientProxy.recentlyVisitedRoomIDsReturnValue = .success(["2", "5", "3"]) + clientProxy.roomSummaryForIdentifierClosure = { roomID in [RoomSummary].mockRooms.first { $0.id == roomID } } + + let viewModel = MessageForwardingScreenViewModel(forwardingPayload: .init(ids: [.randomEvent], + roomID: "", + contents: [RoomMessageEventContentWithoutRelationSDKMock()]), + userSession: UserSessionMock(.init(clientProxy: clientProxy)), + roomSummaryProvider: RoomSummaryProviderMock(.init(state: .loaded(.mockRooms))), + userIndicatorController: UserIndicatorControllerMock()) + + if let searchQuery { + viewModel.context.searchQuery = searchQuery + } + + return viewModel } } diff --git a/ElementX/Sources/Screens/Spaces/SpaceAddRoomsScreen/SpaceAddRoomsScreenViewModel.swift b/ElementX/Sources/Screens/Spaces/SpaceAddRoomsScreen/SpaceAddRoomsScreenViewModel.swift index c68cb931f..4d7393d69 100644 --- a/ElementX/Sources/Screens/Spaces/SpaceAddRoomsScreen/SpaceAddRoomsScreenViewModel.swift +++ b/ElementX/Sources/Screens/Spaces/SpaceAddRoomsScreen/SpaceAddRoomsScreenViewModel.swift @@ -48,14 +48,11 @@ class SpaceAddRoomsScreenViewModel: SpaceAddRoomsScreenViewModelType, SpaceAddRo Task { let existingRooms = spaceRoomListProxy.spaceRoomsPublisher.value suggestedRooms = await userSession.clientProxy - .recentlyVisitedRooms { roomProxy in - // The filter is Sendable for the mock's sake but the client proxy always calls it on the main actor. - MainActor.assumeIsolated { - !roomProxy.infoPublisher.value.isDirect - && !roomProxy.infoPublisher.value.isSpace - && roomProxy.infoPublisher.value.membership == .joined - && !existingRooms.contains { $0.id == roomProxy.id } - } + .recentlyVisitedRooms { @MainActor roomProxy in + !roomProxy.infoPublisher.value.isDirect + && !roomProxy.infoPublisher.value.isSpace + && roomProxy.infoPublisher.value.membership == .joined + && !existingRooms.contains { $0.id == roomProxy.id } } .map { .init(roomProxy: $0) } diff --git a/ElementX/Sources/Services/Client/ClientProxy.swift b/ElementX/Sources/Services/Client/ClientProxy.swift index 55c50ed0b..1888faf1b 100644 --- a/ElementX/Sources/Services/Client/ClientProxy.swift +++ b/ElementX/Sources/Services/Client/ClientProxy.swift @@ -981,7 +981,17 @@ class ClientProxy: ClientProxyProtocol { } } - func recentlyVisitedRooms(filter: @Sendable (JoinedRoomProxyProtocol) -> Bool) async -> [JoinedRoomProxyProtocol] { + func recentlyVisitedRoomIDs() async -> Result<[String], ClientProxyError> { + do { + let result = try await client.getRecentlyVisitedRooms() + return .success(result) + } catch { + MXLog.error("Failed retrieving recently visited rooms with error: \(error)") + return .failure(.sdkError(error)) + } + } + + func recentlyVisitedRooms(filter: @Sendable (JoinedRoomProxyProtocol) async -> Bool) async -> [JoinedRoomProxyProtocol] { let maxResultsToReturn = 5 guard case let .success(roomIdentifiers) = await recentlyVisitedRoomIDs() else { @@ -992,7 +1002,7 @@ class ClientProxy: ClientProxyProtocol { for roomID in roomIdentifiers { guard case let .joined(roomProxy) = await roomForIdentifier(roomID), - filter(roomProxy) else { + await filter(roomProxy) else { continue } @@ -1035,16 +1045,6 @@ class ClientProxy: ClientProxyProtocol { return users.elements } - private func recentlyVisitedRoomIDs() async -> Result<[String], ClientProxyError> { - do { - let result = try await client.getRecentlyVisitedRooms() - return .success(result) - } catch { - MXLog.error("Failed retrieving recently visited rooms with error: \(error)") - return .failure(.sdkError(error)) - } - } - // MARK: Moderation & Safety func setTimelineMediaVisibility(_ value: TimelineMediaVisibility) async -> Result { diff --git a/ElementX/Sources/Services/Client/ClientProxyProtocol.swift b/ElementX/Sources/Services/Client/ClientProxyProtocol.swift index 09a57fd76..aa37bac07 100644 --- a/ElementX/Sources/Services/Client/ClientProxyProtocol.swift +++ b/ElementX/Sources/Services/Client/ClientProxyProtocol.swift @@ -283,7 +283,9 @@ protocol ClientProxyProtocol: AnyObject { func trackRecentlyVisitedRoom(_ roomID: String) async -> Result - func recentlyVisitedRooms(filter: @Sendable (JoinedRoomProxyProtocol) -> Bool) async -> [JoinedRoomProxyProtocol] + /// The identifiers of the rooms the user visited most recently, newest first. + func recentlyVisitedRoomIDs() async -> Result<[String], ClientProxyError> + func recentlyVisitedRooms(filter: @Sendable (JoinedRoomProxyProtocol) async -> Bool) async -> [JoinedRoomProxyProtocol] func recentConversationCounterparts() async -> [UserProfile] // MARK: - Crypto diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.Searching-iPad-en-GB.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.Searching-iPad-en-GB.png new file mode 100644 index 000000000..9f762f487 --- /dev/null +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.Searching-iPad-en-GB.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:90feb66a7ec36bae867ca32a2a883960a5efeab4822b0cb0136926295d8744da +size 158905 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.Searching-iPad-pseudo.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.Searching-iPad-pseudo.png new file mode 100644 index 000000000..f32e54761 --- /dev/null +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.Searching-iPad-pseudo.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:333ee064686f6d432b48a6fe38d34e801cc238ad34af28c7853f2ab794bbad46 +size 160891 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.Searching-iPhone-en-GB.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.Searching-iPhone-en-GB.png new file mode 100644 index 000000000..9d60f5b71 --- /dev/null +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.Searching-iPhone-en-GB.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:4cb33b9bb83676d00fe204736824fca62327c17edd4deb0e2722286571b30fb2 +size 107801 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.Searching-iPhone-pseudo.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.Searching-iPhone-pseudo.png new file mode 100644 index 000000000..638b49047 --- /dev/null +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.Searching-iPhone-pseudo.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:c8cf4b81d5dfc381b74728e65bec8f947e01fa6a52cfd23201a4d473f2acff3d +size 107459 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.Suggestions-iPad-en-GB.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.Suggestions-iPad-en-GB.png new file mode 100644 index 000000000..1bbae6df0 --- /dev/null +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.Suggestions-iPad-en-GB.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:175fc36dbb3f8b53610b87a06fb8f69d9a7c03416699a732678dc719ea7227e0 +size 170430 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.Suggestions-iPad-pseudo.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.Suggestions-iPad-pseudo.png new file mode 100644 index 000000000..621930925 --- /dev/null +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.Suggestions-iPad-pseudo.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:b1a433bd71d9eae827259513c29d4765202c38aac1a97c4e6e0527ca624779ac +size 174737 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.Suggestions-iPhone-en-GB.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.Suggestions-iPhone-en-GB.png new file mode 100644 index 000000000..fb762bb62 --- /dev/null +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.Suggestions-iPhone-en-GB.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:143c4962bc624c86d81996a122894b18b76acbd587942942ca736f3ac5213c5d +size 118839 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.Suggestions-iPhone-pseudo.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.Suggestions-iPhone-pseudo.png new file mode 100644 index 000000000..a4eb35bd0 --- /dev/null +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.Suggestions-iPhone-pseudo.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:78e40d11b882b09e271b345bbbe941216d15629a501191382ed5047e0f227c54 +size 119142 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.iPad-en-GB-0.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.iPad-en-GB-0.png deleted file mode 100644 index 704dfaf72..000000000 --- a/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.iPad-en-GB-0.png +++ /dev/null @@ -1,3 +0,0 @@ -version https://git-lfs.github.com/spec/v1 -oid sha256:c1466071c1f9dbee5bdd27a85a05de3f80eb0f5b09ce99739d8386dc234870ea -size 161363 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.iPad-pseudo-0.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.iPad-pseudo-0.png deleted file mode 100644 index 9917f2b82..000000000 --- a/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.iPad-pseudo-0.png +++ /dev/null @@ -1,3 +0,0 @@ -version https://git-lfs.github.com/spec/v1 -oid sha256:70f396c9aba014af49da5528bda8762ccf837de55e88b57d9f39ff923cd2fcc3 -size 162736 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.iPhone-en-GB-0.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.iPhone-en-GB-0.png deleted file mode 100644 index e529ca7c1..000000000 --- a/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.iPhone-en-GB-0.png +++ /dev/null @@ -1,3 +0,0 @@ -version https://git-lfs.github.com/spec/v1 -oid sha256:b22eda91f8a662a8ae706fc4b48a68bf39c9f607e79e52459cd247cc06865443 -size 111656 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.iPhone-pseudo-0.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.iPhone-pseudo-0.png deleted file mode 100644 index e7d957fdf..000000000 --- a/PreviewTests/Sources/__Snapshots__/PreviewTests/messageForwardingScreen.iPhone-pseudo-0.png +++ /dev/null @@ -1,3 +0,0 @@ -version https://git-lfs.github.com/spec/v1 -oid sha256:9dadf1793a775b966312dd8c84b5b7b45fb00e59ce96f325ce9644ef540b4a7b -size 110350 diff --git a/UnitTests/Sources/MessageForwardingScreenViewModelTests.swift b/UnitTests/Sources/MessageForwardingScreenViewModelTests.swift index dd35ab98a..37fc333e3 100644 --- a/UnitTests/Sources/MessageForwardingScreenViewModelTests.swift +++ b/UnitTests/Sources/MessageForwardingScreenViewModelTests.swift @@ -37,6 +37,109 @@ struct MessageForwardingScreenViewModelTests { #expect(!context.viewState.rooms.contains { $0.id == forwardingPayload.roomID }, "The source room ID shouldn't be shown") } + @Test + mutating func suggestionsShowRecentlyVisitedRooms() async throws { + viewModel = makeViewModel(recentlyVisitedRoomIDs: ["3", "5", "2"]) + context = viewModel.context + + let deferred = deferFulfillment(context.$viewState) { !$0.suggestedRooms.isEmpty } + try await deferred.fulfill() + + #expect(context.viewState.suggestedRooms.map(\.id) == ["3", "5", "2"]) + } + + @Test + mutating func suggestionsOnlyIncludeRoomsThatCanBeForwardedTo() async throws { + viewModel = makeViewModel(recentlyVisitedRoomIDs: [forwardingPayload.roomID, "space", "7", "someAwesomeRoomId1", "!unknown:matrix.org", "2"]) + context = viewModel.context + + let deferred = deferFulfillment(context.$viewState) { !$0.suggestedRooms.isEmpty } + try await deferred.fulfill() + + #expect(context.viewState.suggestedRooms.map(\.id) == ["2"]) + } + + @Test + mutating func suggestionsAreLimitedToFiveRooms() async throws { + viewModel = makeViewModel(recentlyVisitedRoomIDs: [forwardingPayload.roomID, "7", "someAwesomeRoomId1", "2", "3", "4", "5", "6", "0"]) + context = viewModel.context + + let deferred = deferFulfillment(context.$viewState) { !$0.suggestedRooms.isEmpty } + try await deferred.fulfill() + + #expect(context.viewState.suggestedRooms.map(\.id) == ["2", "3", "4", "5", "6"]) + } + + @Test + mutating func suggestedRoomsAreNotListedInChats() async throws { + viewModel = makeViewModel(recentlyVisitedRoomIDs: ["2", "5"]) + context = viewModel.context + + let deferred = deferFulfillment(context.$viewState) { !$0.suggestedRooms.isEmpty } + try await deferred.fulfill() + + let chatIDs = context.viewState.chats.map(\.id) + #expect(!chatIDs.contains("2") && !chatIDs.contains("5")) + #expect(chatIDs.contains("3")) + } + + @Test + mutating func searchingListsSuggestedRoomsInChats() async throws { + viewModel = makeViewModel(recentlyVisitedRoomIDs: ["2"]) + context = viewModel.context + + let deferredSuggestions = deferFulfillment(context.$viewState) { !$0.suggestedRooms.isEmpty } + try await deferredSuggestions.fulfill() + + let deferredSearch = deferFulfillment(context.$viewState) { $0.rooms.count == 1 } + context.searchQuery = "Empire" + try await deferredSearch.fulfill() + + #expect(context.viewState.chats.map(\.id) == ["2"]) + } + + @Test + mutating func searchingHidesTheSuggestions() async throws { + viewModel = makeViewModel(recentlyVisitedRoomIDs: ["2"]) + context = viewModel.context + + let deferredSuggestions = deferFulfillment(context.$viewState) { !$0.suggestedRooms.isEmpty } + try await deferredSuggestions.fulfill() + #expect(context.viewState.showsSuggestions) + + let deferredSearch = deferFulfillment(context.$viewState) { $0.rooms.count == 1 } + context.searchQuery = "Second" + try await deferredSearch.fulfill() + #expect(!context.viewState.showsSuggestions) + #expect(context.viewState.suggestedRooms.map(\.id) == ["2"], "The suggestions shouldn't be searched") + + context.searchQuery = "" + #expect(context.viewState.showsSuggestions) + } + + @Test + mutating func noSuggestionsAreShownWhenNoRecentRoomCanBeSuggested() async throws { + viewModel = makeViewModel(recentlyVisitedRoomIDs: [forwardingPayload.roomID, "7"]) + context = viewModel.context + + let deferred = deferFailure(context.$viewState.map(\.showsSuggestions), timeout: .seconds(1)) { $0 } + try await deferred.fulfill() + } + + @Test + mutating func noSuggestionsAreShownWhenRecentRoomsFailToLoad() async throws { + let clientProxy = ClientProxyMock(.init()) + clientProxy.recentlyVisitedRoomIDsReturnValue = .failure(.sdkError(ClientProxyMockError.generic)) + viewModel = MessageForwardingScreenViewModel(forwardingPayload: forwardingPayload, + userSession: UserSessionMock(.init(clientProxy: clientProxy)), + roomSummaryProvider: RoomSummaryProviderMock(.init(state: .loaded(.mockRooms))), + userIndicatorController: UserIndicatorControllerMock()) + context = viewModel.context + + let deferred = deferFailure(context.$viewState.map(\.showsSuggestions), timeout: .seconds(1)) { $0 } + try await deferred.fulfill() + } + @Test mutating func roomSelection() { context.send(viewAction: .selectRoom(roomID: "2")) @@ -181,9 +284,13 @@ struct MessageForwardingScreenViewModelTests { // MARK: - Helpers - private func makeViewModel(userIndicatorController: UserIndicatorControllerProtocol) -> MessageForwardingScreenViewModelProtocol { + private func makeViewModel(recentlyVisitedRoomIDs: [String] = [], + userIndicatorController: UserIndicatorControllerProtocol = UserIndicatorControllerMock()) -> MessageForwardingScreenViewModelProtocol { let clientProxy = ClientProxyMock(.init()) clientProxy.roomForIdentifierClosure = { .joined(JoinedRoomProxyMock(.init(id: $0))) } + clientProxy.recentlyVisitedRoomIDsReturnValue = .success(recentlyVisitedRoomIDs) + let knownRooms: [RoomSummary] = .mockRooms + .mockInvites + [.mock(id: "space", name: "Space", isSpace: true)] + clientProxy.roomSummaryForIdentifierClosure = { roomID in knownRooms.first { $0.id == roomID } } return MessageForwardingScreenViewModel(forwardingPayload: forwardingPayload, userSession: UserSessionMock(.init(clientProxy: clientProxy)),