From 430798cd15971f3a888a481817fdf44604a24bba Mon Sep 17 00:00:00 2001 From: Doug Date: Mon, 27 Mar 2023 12:41:10 +0100 Subject: [PATCH 1/3] Show @room as a user suggestion when allowed. --- .../xcshareddata/xcschemes/Riot.xcscheme | 37 +++--- Riot/Modules/Room/RoomViewController.m | 18 ++- .../UserSuggestionCoordinator.swift | 34 +++++- .../UserSuggestionCoordinatorBridge.swift | 9 +- .../Service/UserSuggestionService.swift | 11 +- .../Unit/UserSuggestionServiceTests.swift | 114 +++++++++++------- .../UserSuggestionScreenState.swift | 2 + 7 files changed, 146 insertions(+), 79 deletions(-) diff --git a/Riot.xcodeproj/xcshareddata/xcschemes/Riot.xcscheme b/Riot.xcodeproj/xcshareddata/xcschemes/Riot.xcscheme index 012a5a109b..e1775adc47 100644 --- a/Riot.xcodeproj/xcshareddata/xcschemes/Riot.xcscheme +++ b/Riot.xcodeproj/xcshareddata/xcschemes/Riot.xcscheme @@ -1,11 +1,10 @@ + version = "1.3"> + buildImplicitDependencies = "YES"> @@ -35,11 +34,20 @@ + + + + @@ -52,17 +60,6 @@ - - - - - - - - - - diff --git a/Riot/Modules/Room/RoomViewController.m b/Riot/Modules/Room/RoomViewController.m index 2e641916a7..8cf5075589 100644 --- a/Riot/Modules/Room/RoomViewController.m +++ b/Riot/Modules/Room/RoomViewController.m @@ -1089,7 +1089,8 @@ - (void)displayRoom:(MXKRoomDataSource *)dataSource _voiceMessageController.roomId = dataSource.roomId; _userSuggestionCoordinator = [[UserSuggestionCoordinatorBridge alloc] initWithMediaManager:self.roomDataSource.mxSession.mediaManager - room:dataSource.room]; + room:dataSource.room + userID:self.roomDataSource.mxSession.myUserId]; _userSuggestionCoordinator.delegate = self; [self setupUserSuggestionViewIfNeeded]; @@ -8047,6 +8048,19 @@ - (void)spaceDetailPresenter:(SpaceDetailPresenter *)presenter didJoinSpaceWithI - (void)userSuggestionCoordinatorBridge:(UserSuggestionCoordinatorBridge *)coordinator didRequestMentionForMember:(MXRoomMember *)member textTrigger:(NSString *)textTrigger +{ + [self removeTriggerTextFromComposer:textTrigger]; + [self mention:member]; +} + +- (void)userSuggestionCoordinatorBridgeDidRequestMentionForRoom:(UserSuggestionCoordinatorBridge *)coordinator + textTrigger:(NSString *)textTrigger +{ + [self removeTriggerTextFromComposer:textTrigger]; + [self.inputToolbarView pasteText:@"@room "]; +} + +- (void)removeTriggerTextFromComposer:(NSString *)textTrigger { RoomInputToolbarView *toolbar = (RoomInputToolbarView *)self.inputToolbarView; if (toolbar && textTrigger.length) { @@ -8057,8 +8071,6 @@ - (void)userSuggestionCoordinatorBridge:(UserSuggestionCoordinatorBridge *)coord range:NSMakeRange(0, attributedTextMessage.length)]; [toolbar setAttributedTextMessage:attributedTextMessage]; } - - [self mention:member]; } - (void)userSuggestionCoordinatorBridge:(UserSuggestionCoordinatorBridge *)coordinator didUpdateViewHeight:(CGFloat)height diff --git a/RiotSwiftUI/Modules/Room/UserSuggestion/Coordinator/UserSuggestionCoordinator.swift b/RiotSwiftUI/Modules/Room/UserSuggestion/Coordinator/UserSuggestionCoordinator.swift index c6d86a6558..6b150436f9 100644 --- a/RiotSwiftUI/Modules/Room/UserSuggestion/Coordinator/UserSuggestionCoordinator.swift +++ b/RiotSwiftUI/Modules/Room/UserSuggestion/Coordinator/UserSuggestionCoordinator.swift @@ -21,12 +21,14 @@ import UIKit protocol UserSuggestionCoordinatorDelegate: AnyObject { func userSuggestionCoordinator(_ coordinator: UserSuggestionCoordinator, didRequestMentionForMember member: MXRoomMember, textTrigger: String?) + func userSuggestionCoordinatorDidRequestMentionForRoom(_ coordinator: UserSuggestionCoordinator, textTrigger: String?) func userSuggestionCoordinator(_ coordinator: UserSuggestionCoordinator, didUpdateViewHeight height: CGFloat) } struct UserSuggestionCoordinatorParameters { let mediaManager: MXMediaManager let room: MXRoom + let userID: String } final class UserSuggestionCoordinator: Coordinator, Presentable { @@ -56,7 +58,7 @@ final class UserSuggestionCoordinator: Coordinator, Presentable { init(parameters: UserSuggestionCoordinatorParameters) { self.parameters = parameters - roomMemberProvider = UserSuggestionCoordinatorRoomMemberProvider(room: parameters.room) + roomMemberProvider = UserSuggestionCoordinatorRoomMemberProvider(room: parameters.room, userID: parameters.userID) userSuggestionService = UserSuggestionService(roomMemberProvider: roomMemberProvider) let viewModel = UserSuggestionViewModel(userSuggestionService: userSuggestionService) @@ -73,6 +75,11 @@ final class UserSuggestionCoordinator: Coordinator, Presentable { switch result { case .selectedItemWithIdentifier(let identifier): + if identifier == "@room" { + self.delegate?.userSuggestionCoordinatorDidRequestMentionForRoom(self, textTrigger: self.userSuggestionService.currentTextTrigger) + return + } + guard let member = self.roomMemberProvider.roomMembers.filter({ $0.userId == identifier }).first else { return } @@ -130,29 +137,44 @@ final class UserSuggestionCoordinator: Coordinator, Presentable { private class UserSuggestionCoordinatorRoomMemberProvider: RoomMembersProviderProtocol { private let room: MXRoom + private let userID: String var roomMembers: [MXRoomMember] = [] + var canMentionRoom = false - init(room: MXRoom) { + init(room: MXRoom, userID: String) { self.room = room + self.userID = userID + updateWithPowerLevels() + } + + /// Gets the power levels for the room to update suggestions accordingly. + func updateWithPowerLevels() { + room.state { [weak self] state in + guard let self, let powerLevels = state?.powerLevels else { return } + let userPowerLevel = powerLevels.powerLevelOfUser(withUserID: self.userID) + let mentionRoomPowerLevel = powerLevels.minimumPowerLevel(forNotifications: kMXRoomPowerLevelNotificationsRoomKey, + defaultPower: kMXRoomPowerLevelNotificationsRoomDefault) + self.canMentionRoom = userPowerLevel >= mentionRoomPowerLevel + } } func fetchMembers(_ members: @escaping ([RoomMembersProviderMember]) -> Void) { - room.members({ [weak self] roomMembers in + room.members { [weak self] roomMembers in guard let self = self, let joinedMembers = roomMembers?.joinedMembers else { return } self.roomMembers = joinedMembers members(self.roomMembersToProviderMembers(joinedMembers)) - }, lazyLoadedMembers: { [weak self] lazyRoomMembers in + } lazyLoadedMembers: { [weak self] lazyRoomMembers in guard let self = self, let joinedMembers = lazyRoomMembers?.joinedMembers else { return } self.roomMembers = joinedMembers members(self.roomMembersToProviderMembers(joinedMembers)) - }, failure: { error in + } failure: { error in MXLog.error("[UserSuggestionCoordinatorRoomMemberProvider] Failed loading room", context: error) - }) + } } private func roomMembersToProviderMembers(_ roomMembers: [MXRoomMember]) -> [RoomMembersProviderMember] { diff --git a/RiotSwiftUI/Modules/Room/UserSuggestion/Coordinator/UserSuggestionCoordinatorBridge.swift b/RiotSwiftUI/Modules/Room/UserSuggestion/Coordinator/UserSuggestionCoordinatorBridge.swift index c5b68eeee0..ea81631063 100644 --- a/RiotSwiftUI/Modules/Room/UserSuggestion/Coordinator/UserSuggestionCoordinatorBridge.swift +++ b/RiotSwiftUI/Modules/Room/UserSuggestion/Coordinator/UserSuggestionCoordinatorBridge.swift @@ -19,6 +19,7 @@ import Foundation @objc protocol UserSuggestionCoordinatorBridgeDelegate: AnyObject { func userSuggestionCoordinatorBridge(_ coordinator: UserSuggestionCoordinatorBridge, didRequestMentionForMember member: MXRoomMember, textTrigger: String?) + func userSuggestionCoordinatorBridgeDidRequestMentionForRoom(_ coordinator: UserSuggestionCoordinatorBridge, textTrigger: String?) func userSuggestionCoordinatorBridge(_ coordinator: UserSuggestionCoordinatorBridge, didUpdateViewHeight height: CGFloat) } @@ -31,8 +32,8 @@ final class UserSuggestionCoordinatorBridge: NSObject { weak var delegate: UserSuggestionCoordinatorBridgeDelegate? - init(mediaManager: MXMediaManager, room: MXRoom) { - let parameters = UserSuggestionCoordinatorParameters(mediaManager: mediaManager, room: room) + init(mediaManager: MXMediaManager, room: MXRoom, userID: String) { + let parameters = UserSuggestionCoordinatorParameters(mediaManager: mediaManager, room: room, userID: userID) let userSuggestionCoordinator = UserSuggestionCoordinator(parameters: parameters) _userSuggestionCoordinator = userSuggestionCoordinator @@ -54,6 +55,10 @@ extension UserSuggestionCoordinatorBridge: UserSuggestionCoordinatorDelegate { func userSuggestionCoordinator(_ coordinator: UserSuggestionCoordinator, didRequestMentionForMember member: MXRoomMember, textTrigger: String?) { delegate?.userSuggestionCoordinatorBridge(self, didRequestMentionForMember: member, textTrigger: textTrigger) } + + func userSuggestionCoordinatorDidRequestMentionForRoom(_ coordinator: UserSuggestionCoordinator, textTrigger: String?) { + delegate?.userSuggestionCoordinatorBridgeDidRequestMentionForRoom(self, textTrigger: textTrigger) + } func userSuggestionCoordinator(_ coordinator: UserSuggestionCoordinator, didUpdateViewHeight height: CGFloat) { delegate?.userSuggestionCoordinatorBridge(self, didUpdateViewHeight: height) diff --git a/RiotSwiftUI/Modules/Room/UserSuggestion/Service/UserSuggestionService.swift b/RiotSwiftUI/Modules/Room/UserSuggestion/Service/UserSuggestionService.swift index bf8fa00a5f..307bfa5266 100644 --- a/RiotSwiftUI/Modules/Room/UserSuggestion/Service/UserSuggestionService.swift +++ b/RiotSwiftUI/Modules/Room/UserSuggestion/Service/UserSuggestionService.swift @@ -24,6 +24,7 @@ struct RoomMembersProviderMember { } protocol RoomMembersProviderProtocol { + var canMentionRoom: Bool { get } func fetchMembers(_ members: @escaping ([RoomMembersProviderMember]) -> Void) } @@ -100,7 +101,7 @@ class UserSuggestionService: UserSuggestionServiceProtocol { return } - self.suggestionItems = members.map { member in + self.suggestionItems = members.withRoom(self.roomMemberProvider.canMentionRoom).map { member in UserSuggestionServiceItem(userId: member.userId, displayName: member.displayName, avatarUrl: member.avatarUrl) } @@ -113,3 +114,11 @@ class UserSuggestionService: UserSuggestionServiceProtocol { } } } + +extension Array where Element == RoomMembersProviderMember { + /// Returns the array with an additional member that represents an `@room` mention. + func withRoom(_ canMentionRoom: Bool) -> Self { + guard canMentionRoom else { return self } + return self + [RoomMembersProviderMember(userId: "@room", displayName: "@room", avatarUrl: "")] + } +} diff --git a/RiotSwiftUI/Modules/Room/UserSuggestion/Test/Unit/UserSuggestionServiceTests.swift b/RiotSwiftUI/Modules/Room/UserSuggestion/Test/Unit/UserSuggestionServiceTests.swift index b32580c8de..e3e76cf3b7 100644 --- a/RiotSwiftUI/Modules/Room/UserSuggestion/Test/Unit/UserSuggestionServiceTests.swift +++ b/RiotSwiftUI/Modules/Room/UserSuggestion/Test/Unit/UserSuggestionServiceTests.swift @@ -20,87 +20,111 @@ import XCTest @testable import RiotSwiftUI class UserSuggestionServiceTests: XCTestCase { - var service: UserSuggestionService? + var service: UserSuggestionService! + var canMentionRoom = false override func setUp() { service = UserSuggestionService(roomMemberProvider: self, shouldDebounce: false) + canMentionRoom = false } func testAlice() { - service?.processTextMessage("@Al") - assert(service?.items.value.first?.displayName == "Alice") + service.processTextMessage("@Al") + XCTAssertEqual(service.items.value.first?.displayName, "Alice") - service?.processTextMessage("@al") - assert(service?.items.value.first?.displayName == "Alice") + service.processTextMessage("@al") + XCTAssertEqual(service.items.value.first?.displayName, "Alice") - service?.processTextMessage("@ice") - assert(service?.items.value.first?.displayName == "Alice") + service.processTextMessage("@ice") + XCTAssertEqual(service.items.value.first?.displayName, "Alice") - service?.processTextMessage("@Alice") - assert(service?.items.value.first?.displayName == "Alice") + service.processTextMessage("@Alice") + XCTAssertEqual(service.items.value.first?.displayName, "Alice") - service?.processTextMessage("@alice:matrix.org") - assert(service?.items.value.first?.displayName == "Alice") + service.processTextMessage("@alice:matrix.org") + XCTAssertEqual(service.items.value.first?.displayName, "Alice") } func testBob() { - service?.processTextMessage("@ob") - assert(service?.items.value.first?.displayName == "Bob") + service.processTextMessage("@ob") + XCTAssertEqual(service.items.value.first?.displayName, "Bob") - service?.processTextMessage("@ob:") - assert(service?.items.value.first?.displayName == "Bob") + service.processTextMessage("@ob:") + XCTAssertEqual(service.items.value.first?.displayName, "Bob") - service?.processTextMessage("@b:matrix") - assert(service?.items.value.first?.displayName == "Bob") + service.processTextMessage("@b:matrix") + XCTAssertEqual(service.items.value.first?.displayName, "Bob") } func testBoth() { - service?.processTextMessage("@:matrix") - assert(service?.items.value.first?.displayName == "Alice") - assert(service?.items.value.last?.displayName == "Bob") + service.processTextMessage("@:matrix") + XCTAssertEqual(service.items.value.first?.displayName, "Alice") + XCTAssertEqual(service.items.value.last?.displayName, "Bob") - service?.processTextMessage("@.org") - assert(service?.items.value.first?.displayName == "Alice") - assert(service?.items.value.last?.displayName == "Bob") + service.processTextMessage("@.org") + XCTAssertEqual(service.items.value.first?.displayName, "Alice") + XCTAssertEqual(service.items.value.last?.displayName, "Bob") } func testEmptyResult() { - service?.processTextMessage("Lorem ipsum idolor") - assert(service?.items.value.count == 0) + service.processTextMessage("Lorem ipsum idolor") + XCTAssertTrue(service.items.value.isEmpty) - service?.processTextMessage("@") - assert(service?.items.value.count == 0) + service.processTextMessage("@") + XCTAssertTrue(service.items.value.isEmpty) - service?.processTextMessage("@@") - assert(service?.items.value.count == 0) + service.processTextMessage("@@") + XCTAssertTrue(service.items.value.isEmpty) - service?.processTextMessage("alice@matrix.org") - assert(service?.items.value.count == 0) + service.processTextMessage("alice@matrix.org") + XCTAssertTrue(service.items.value.isEmpty) } func testStuff() { - service?.processTextMessage("@@") - assert(service?.items.value.count == 0) + service.processTextMessage("@@") + XCTAssertTrue(service.items.value.isEmpty) } func testWhitespaces() { - service?.processTextMessage("") - assert(service?.items.value.count == 0) + service.processTextMessage("") + XCTAssertTrue(service.items.value.isEmpty) - service?.processTextMessage(" ") - assert(service?.items.value.count == 0) + service.processTextMessage(" ") + XCTAssertTrue(service.items.value.isEmpty) - service?.processTextMessage("\n") - assert(service?.items.value.count == 0) + service.processTextMessage("\n") + XCTAssertTrue(service.items.value.isEmpty) - service?.processTextMessage(" \n ") - assert(service?.items.value.count == 0) + service.processTextMessage(" \n ") + XCTAssertTrue(service.items.value.isEmpty) - service?.processTextMessage("@A ") - assert(service?.items.value.count == 0) + service.processTextMessage("@A ") + XCTAssertTrue(service.items.value.isEmpty) - service?.processTextMessage(" @A ") - assert(service?.items.value.count == 0) + service.processTextMessage(" @A ") + XCTAssertTrue(service.items.value.isEmpty) + } + + func testRoomWithoutPower() { + // Given a user without the power to mention a room. + canMentionRoom = false + + // Given a user without the power to mention a room. + service.processTextMessage("@ro") + + // Then the completion for a room mention should not be shown. + XCTAssertTrue(service.items.value.isEmpty) + } + + func testRoomWithPower() { + // Given a user without the power to mention a room. + canMentionRoom = true + + // Given a user without the power to mention a room. + service.processTextMessage("@ro") + + // Then the completion for a room mention should be shown. + XCTAssertEqual(service.items.value.first?.userId, "@room") } } diff --git a/RiotSwiftUI/Modules/Room/UserSuggestion/UserSuggestionScreenState.swift b/RiotSwiftUI/Modules/Room/UserSuggestion/UserSuggestionScreenState.swift index a0ed202682..0a9395fa56 100644 --- a/RiotSwiftUI/Modules/Room/UserSuggestion/UserSuggestionScreenState.swift +++ b/RiotSwiftUI/Modules/Room/UserSuggestion/UserSuggestionScreenState.swift @@ -43,6 +43,8 @@ enum MockUserSuggestionScreenState: MockScreenState, CaseIterable { } extension MockUserSuggestionScreenState: RoomMembersProviderProtocol { + var canMentionRoom: Bool { false } + func fetchMembers(_ members: ([RoomMembersProviderMember]) -> Void) { if Self.members == nil { Self.members = generateUsersWithCount(10) From 0561cb81c6f15b6187caf820ff9f4b0e513bbbcd Mon Sep 17 00:00:00 2001 From: Doug Date: Mon, 27 Mar 2023 12:49:24 +0100 Subject: [PATCH 2/3] Use Everyone title. --- .../Room/UserSuggestion/Service/UserSuggestionService.swift | 2 +- changelog.d/pr-7453.change | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 changelog.d/pr-7453.change diff --git a/RiotSwiftUI/Modules/Room/UserSuggestion/Service/UserSuggestionService.swift b/RiotSwiftUI/Modules/Room/UserSuggestion/Service/UserSuggestionService.swift index 307bfa5266..4046251ef2 100644 --- a/RiotSwiftUI/Modules/Room/UserSuggestion/Service/UserSuggestionService.swift +++ b/RiotSwiftUI/Modules/Room/UserSuggestion/Service/UserSuggestionService.swift @@ -119,6 +119,6 @@ extension Array where Element == RoomMembersProviderMember { /// Returns the array with an additional member that represents an `@room` mention. func withRoom(_ canMentionRoom: Bool) -> Self { guard canMentionRoom else { return self } - return self + [RoomMembersProviderMember(userId: "@room", displayName: "@room", avatarUrl: "")] + return self + [RoomMembersProviderMember(userId: "@room", displayName: "Everyone", avatarUrl: "")] } } diff --git a/changelog.d/pr-7453.change b/changelog.d/pr-7453.change new file mode 100644 index 0000000000..8aa79f59fc --- /dev/null +++ b/changelog.d/pr-7453.change @@ -0,0 +1 @@ +Add user suggestions for @room and highlight incoming messages containing @room when the room is encrypted. \ No newline at end of file From cd44f0993666722091da9ac053880a93b9b61adb Mon Sep 17 00:00:00 2001 From: Doug Date: Tue, 28 Mar 2023 11:34:57 +0100 Subject: [PATCH 3/3] Review comments --- Riot/Modules/Room/RoomViewController.m | 2 +- .../Coordinator/UserSuggestionCoordinator.swift | 2 +- .../UserSuggestion/Service/UserSuggestionService.swift | 7 ++++++- .../Test/Unit/UserSuggestionServiceTests.swift | 2 +- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/Riot/Modules/Room/RoomViewController.m b/Riot/Modules/Room/RoomViewController.m index 8cf5075589..3819a7a49f 100644 --- a/Riot/Modules/Room/RoomViewController.m +++ b/Riot/Modules/Room/RoomViewController.m @@ -8057,7 +8057,7 @@ - (void)userSuggestionCoordinatorBridgeDidRequestMentionForRoom:(UserSuggestionC textTrigger:(NSString *)textTrigger { [self removeTriggerTextFromComposer:textTrigger]; - [self.inputToolbarView pasteText:@"@room "]; + [self.inputToolbarView pasteText:[UserSuggestionID.room stringByAppendingString:@" "]]; } - (void)removeTriggerTextFromComposer:(NSString *)textTrigger diff --git a/RiotSwiftUI/Modules/Room/UserSuggestion/Coordinator/UserSuggestionCoordinator.swift b/RiotSwiftUI/Modules/Room/UserSuggestion/Coordinator/UserSuggestionCoordinator.swift index 6b150436f9..cc3f208c39 100644 --- a/RiotSwiftUI/Modules/Room/UserSuggestion/Coordinator/UserSuggestionCoordinator.swift +++ b/RiotSwiftUI/Modules/Room/UserSuggestion/Coordinator/UserSuggestionCoordinator.swift @@ -75,7 +75,7 @@ final class UserSuggestionCoordinator: Coordinator, Presentable { switch result { case .selectedItemWithIdentifier(let identifier): - if identifier == "@room" { + if identifier == UserSuggestionID.room { self.delegate?.userSuggestionCoordinatorDidRequestMentionForRoom(self, textTrigger: self.userSuggestionService.currentTextTrigger) return } diff --git a/RiotSwiftUI/Modules/Room/UserSuggestion/Service/UserSuggestionService.swift b/RiotSwiftUI/Modules/Room/UserSuggestion/Service/UserSuggestionService.swift index 4046251ef2..2bd8a45692 100644 --- a/RiotSwiftUI/Modules/Room/UserSuggestion/Service/UserSuggestionService.swift +++ b/RiotSwiftUI/Modules/Room/UserSuggestion/Service/UserSuggestionService.swift @@ -23,6 +23,11 @@ struct RoomMembersProviderMember { var avatarUrl: String } +class UserSuggestionID: NSObject { + /// A special case added for suggesting `@room` mentions. + @objc static let room = "@room" +} + protocol RoomMembersProviderProtocol { var canMentionRoom: Bool { get } func fetchMembers(_ members: @escaping ([RoomMembersProviderMember]) -> Void) @@ -119,6 +124,6 @@ extension Array where Element == RoomMembersProviderMember { /// Returns the array with an additional member that represents an `@room` mention. func withRoom(_ canMentionRoom: Bool) -> Self { guard canMentionRoom else { return self } - return self + [RoomMembersProviderMember(userId: "@room", displayName: "Everyone", avatarUrl: "")] + return self + [RoomMembersProviderMember(userId: UserSuggestionID.room, displayName: "Everyone", avatarUrl: "")] } } diff --git a/RiotSwiftUI/Modules/Room/UserSuggestion/Test/Unit/UserSuggestionServiceTests.swift b/RiotSwiftUI/Modules/Room/UserSuggestion/Test/Unit/UserSuggestionServiceTests.swift index e3e76cf3b7..7ae0bfa39e 100644 --- a/RiotSwiftUI/Modules/Room/UserSuggestion/Test/Unit/UserSuggestionServiceTests.swift +++ b/RiotSwiftUI/Modules/Room/UserSuggestion/Test/Unit/UserSuggestionServiceTests.swift @@ -124,7 +124,7 @@ class UserSuggestionServiceTests: XCTestCase { service.processTextMessage("@ro") // Then the completion for a room mention should be shown. - XCTAssertEqual(service.items.value.first?.userId, "@room") + XCTAssertEqual(service.items.value.first?.userId, UserSuggestionID.room) } }