Add alerts with reason for kick and ban (#3967)

* feat: added alerts with reason for kick and ban

in the room members list screen

* fix tests

ElementX/Resources/Localizations/en-US.lproj/Localizable.strings

@@ -208,6 +208,7 @@
 "common_privacy_policy" = "Privacy policy";
 "common_reaction" = "Reaction";
 "common_reactions" = "Reactions";
+"common_reason" = "Reason";
 "common_recovery_key" = "Recovery key";
 "common_refreshing" = "Refreshing…";
 "common_replying_to" = "Replying to %1$@";
@@ -875,6 +876,9 @@
 "screen_room_member_list_ban_member_confirmation_title" = "Are you sure you want to ban this member?";
 "screen_room_member_list_banned_empty" = "There are no banned users in this room.";
 "screen_room_member_list_banning_user" = "Banning %1$@";
+"screen_room_member_list_kick_member_confirmation_action" = "Remove";
+"screen_room_member_list_kick_member_confirmation_description" = "They will be able to join this room again if invited.";
+"screen_room_member_list_kick_member_confirmation_title" = "Are you sure you want to remove this member?";
 "screen_room_member_list_manage_member_ban" = "Remove and ban member";
 "screen_room_member_list_manage_member_remove" = "Remove from room";
 "screen_room_member_list_manage_member_remove_confirmation_kick" = "Only remove member";

ElementX/Resources/Localizations/en.lproj/Localizable.strings

@@ -208,6 +208,7 @@
 "common_privacy_policy" = "Privacy policy";
 "common_reaction" = "Reaction";
 "common_reactions" = "Reactions";
+"common_reason" = "Reason";
 "common_recovery_key" = "Recovery key";
 "common_refreshing" = "Refreshing…";
 "common_replying_to" = "Replying to %1$@";
@@ -875,6 +876,9 @@
 "screen_room_member_list_ban_member_confirmation_title" = "Are you sure you want to ban this member?";
 "screen_room_member_list_banned_empty" = "There are no banned users in this room.";
 "screen_room_member_list_banning_user" = "Banning %1$@";
+"screen_room_member_list_kick_member_confirmation_action" = "Remove";
+"screen_room_member_list_kick_member_confirmation_description" = "They will be able to join this room again if invited.";
+"screen_room_member_list_kick_member_confirmation_title" = "Are you sure you want to remove this member?";
 "screen_room_member_list_manage_member_ban" = "Remove and ban member";
 "screen_room_member_list_manage_member_remove" = "Remove from room";
 "screen_room_member_list_manage_member_remove_confirmation_kick" = "Only remove member";

ElementX/Sources/Generated/Strings.swift

@@ -478,6 +478,8 @@ internal enum L10n {
   internal static var commonReaction: String { return L10n.tr("Localizable", "common_reaction") }
   /// Reactions
   internal static var commonReactions: String { return L10n.tr("Localizable", "common_reactions") }
+  /// Reason
+  internal static var commonReason: String { return L10n.tr("Localizable", "common_reason") }
   /// Recovery key
   internal static var commonRecoveryKey: String { return L10n.tr("Localizable", "common_recovery_key") }
   /// Refreshing…
@@ -2050,6 +2052,12 @@ internal enum L10n {
   internal static func screenRoomMemberListHeaderTitle(_ p1: Int) -> String {
     return L10n.tr("Localizable", "screen_room_member_list_header_title", p1)
   }
+  /// Remove
+  internal static var screenRoomMemberListKickMemberConfirmationAction: String { return L10n.tr("Localizable", "screen_room_member_list_kick_member_confirmation_action") }
+  /// They will be able to join this room again if invited.
+  internal static var screenRoomMemberListKickMemberConfirmationDescription: String { return L10n.tr("Localizable", "screen_room_member_list_kick_member_confirmation_description") }
+  /// Are you sure you want to remove this member?
+  internal static var screenRoomMemberListKickMemberConfirmationTitle: String { return L10n.tr("Localizable", "screen_room_member_list_kick_member_confirmation_title") }
   /// Remove and ban member
   internal static var screenRoomMemberListManageMemberBan: String { return L10n.tr("Localizable", "screen_room_member_list_manage_member_ban") }
   /// Remove from room

ElementX/Sources/Mocks/Generated/GeneratedMocks.swift

@@ -9077,15 +9077,15 @@ class JoinedRoomProxyMock: JoinedRoomProxyProtocol, @unchecked Sendable {
     }
     //MARK: - kickUser
 
-    var kickUserUnderlyingCallsCount = 0
+    var kickUserReasonUnderlyingCallsCount = 0
-    var kickUserCallsCount: Int {
+    var kickUserReasonCallsCount: Int {
         get {
             if Thread.isMainThread {
-                return kickUserUnderlyingCallsCount
+                return kickUserReasonUnderlyingCallsCount
             } else {
                 var returnValue: Int? = nil
                 DispatchQueue.main.sync {
-                    returnValue = kickUserUnderlyingCallsCount
+                    returnValue = kickUserReasonUnderlyingCallsCount
                 }
 
                 return returnValue!
@@ -9093,29 +9093,29 @@ class JoinedRoomProxyMock: JoinedRoomProxyProtocol, @unchecked Sendable {
         }
         set {
             if Thread.isMainThread {
-                kickUserUnderlyingCallsCount = newValue
+                kickUserReasonUnderlyingCallsCount = newValue
             } else {
                 DispatchQueue.main.sync {
-                    kickUserUnderlyingCallsCount = newValue
+                    kickUserReasonUnderlyingCallsCount = newValue
                 }
             }
         }
     }
-    var kickUserCalled: Bool {
+    var kickUserReasonCalled: Bool {
-        return kickUserCallsCount > 0
+        return kickUserReasonCallsCount > 0
     }
-    var kickUserReceivedUserID: String?
+    var kickUserReasonReceivedArguments: (userID: String, reason: String?)?
-    var kickUserReceivedInvocations: [String] = []
+    var kickUserReasonReceivedInvocations: [(userID: String, reason: String?)] = []
 
-    var kickUserUnderlyingReturnValue: Result<Void, RoomProxyError>!
+    var kickUserReasonUnderlyingReturnValue: Result<Void, RoomProxyError>!
-    var kickUserReturnValue: Result<Void, RoomProxyError>! {
+    var kickUserReasonReturnValue: Result<Void, RoomProxyError>! {
         get {
             if Thread.isMainThread {
-                return kickUserUnderlyingReturnValue
+                return kickUserReasonUnderlyingReturnValue
             } else {
                 var returnValue: Result<Void, RoomProxyError>? = nil
                 DispatchQueue.main.sync {
-                    returnValue = kickUserUnderlyingReturnValue
+                    returnValue = kickUserReasonUnderlyingReturnValue
                 }
 
                 return returnValue!
@@ -9123,39 +9123,39 @@ class JoinedRoomProxyMock: JoinedRoomProxyProtocol, @unchecked Sendable {
         }
         set {
             if Thread.isMainThread {
-                kickUserUnderlyingReturnValue = newValue
+                kickUserReasonUnderlyingReturnValue = newValue
             } else {
                 DispatchQueue.main.sync {
-                    kickUserUnderlyingReturnValue = newValue
+                    kickUserReasonUnderlyingReturnValue = newValue
                 }
             }
         }
     }
-    var kickUserClosure: ((String) async -> Result<Void, RoomProxyError>)?
+    var kickUserReasonClosure: ((String, String?) async -> Result<Void, RoomProxyError>)?
 
-    func kickUser(_ userID: String) async -> Result<Void, RoomProxyError> {
+    func kickUser(_ userID: String, reason: String?) async -> Result<Void, RoomProxyError> {
-        kickUserCallsCount += 1
+        kickUserReasonCallsCount += 1
-        kickUserReceivedUserID = userID
+        kickUserReasonReceivedArguments = (userID: userID, reason: reason)
         DispatchQueue.main.async {
-            self.kickUserReceivedInvocations.append(userID)
+            self.kickUserReasonReceivedInvocations.append((userID: userID, reason: reason))
         }
-        if let kickUserClosure = kickUserClosure {
+        if let kickUserReasonClosure = kickUserReasonClosure {
-            return await kickUserClosure(userID)
+            return await kickUserReasonClosure(userID, reason)
         } else {
-            return kickUserReturnValue
+            return kickUserReasonReturnValue
         }
     }
     //MARK: - banUser
 
-    var banUserUnderlyingCallsCount = 0
+    var banUserReasonUnderlyingCallsCount = 0
-    var banUserCallsCount: Int {
+    var banUserReasonCallsCount: Int {
         get {
             if Thread.isMainThread {
-                return banUserUnderlyingCallsCount
+                return banUserReasonUnderlyingCallsCount
             } else {
                 var returnValue: Int? = nil
                 DispatchQueue.main.sync {
-                    returnValue = banUserUnderlyingCallsCount
+                    returnValue = banUserReasonUnderlyingCallsCount
                 }
 
                 return returnValue!
@@ -9163,29 +9163,29 @@ class JoinedRoomProxyMock: JoinedRoomProxyProtocol, @unchecked Sendable {
         }
         set {
             if Thread.isMainThread {
-                banUserUnderlyingCallsCount = newValue
+                banUserReasonUnderlyingCallsCount = newValue
             } else {
                 DispatchQueue.main.sync {
-                    banUserUnderlyingCallsCount = newValue
+                    banUserReasonUnderlyingCallsCount = newValue
                 }
             }
         }
     }
-    var banUserCalled: Bool {
+    var banUserReasonCalled: Bool {
-        return banUserCallsCount > 0
+        return banUserReasonCallsCount > 0
     }
-    var banUserReceivedUserID: String?
+    var banUserReasonReceivedArguments: (userID: String, reason: String?)?
-    var banUserReceivedInvocations: [String] = []
+    var banUserReasonReceivedInvocations: [(userID: String, reason: String?)] = []
 
-    var banUserUnderlyingReturnValue: Result<Void, RoomProxyError>!
+    var banUserReasonUnderlyingReturnValue: Result<Void, RoomProxyError>!
-    var banUserReturnValue: Result<Void, RoomProxyError>! {
+    var banUserReasonReturnValue: Result<Void, RoomProxyError>! {
         get {
             if Thread.isMainThread {
-                return banUserUnderlyingReturnValue
+                return banUserReasonUnderlyingReturnValue
             } else {
                 var returnValue: Result<Void, RoomProxyError>? = nil
                 DispatchQueue.main.sync {
-                    returnValue = banUserUnderlyingReturnValue
+                    returnValue = banUserReasonUnderlyingReturnValue
                 }
 
                 return returnValue!
@@ -9193,26 +9193,26 @@ class JoinedRoomProxyMock: JoinedRoomProxyProtocol, @unchecked Sendable {
         }
         set {
             if Thread.isMainThread {
-                banUserUnderlyingReturnValue = newValue
+                banUserReasonUnderlyingReturnValue = newValue
             } else {
                 DispatchQueue.main.sync {
-                    banUserUnderlyingReturnValue = newValue
+                    banUserReasonUnderlyingReturnValue = newValue
                 }
             }
         }
     }
-    var banUserClosure: ((String) async -> Result<Void, RoomProxyError>)?
+    var banUserReasonClosure: ((String, String?) async -> Result<Void, RoomProxyError>)?
 
-    func banUser(_ userID: String) async -> Result<Void, RoomProxyError> {
+    func banUser(_ userID: String, reason: String?) async -> Result<Void, RoomProxyError> {
-        banUserCallsCount += 1
+        banUserReasonCallsCount += 1
-        banUserReceivedUserID = userID
+        banUserReasonReceivedArguments = (userID: userID, reason: reason)
         DispatchQueue.main.async {
-            self.banUserReceivedInvocations.append(userID)
+            self.banUserReasonReceivedInvocations.append((userID: userID, reason: reason))
         }
-        if let banUserClosure = banUserClosure {
+        if let banUserReasonClosure = banUserReasonClosure {
-            return await banUserClosure(userID)
+            return await banUserReasonClosure(userID, reason)
         } else {
-            return banUserReturnValue
+            return banUserReasonReturnValue
         }
     }
     //MARK: - unbanUser

ElementX/Sources/Mocks/JoinedRoomProxyMock.swift

@@ -107,8 +107,8 @@ extension JoinedRoomProxyMock {
         canUserJoinCallUserIDReturnValue = .success(configuration.canUserJoinCall)
         canUserPinOrUnpinUserIDReturnValue = .success(configuration.canUserPin)
         
-        kickUserReturnValue = .success(())
+        kickUserReasonReturnValue = .success(())
-        banUserReturnValue = .success(())
+        banUserReasonReturnValue = .success(())
         unbanUserReturnValue = .success(())
         
         let widgetDriver = ElementCallWidgetDriverMock()

ElementX/Sources/Screens/RoomMemberListScreen/RoomMembersListScreenModels.swift

@@ -111,4 +111,6 @@ enum RoomMembersListScreenViewAction {
 
 enum RoomMembersListScreenAlertType: Hashable {
     case unbanConfirmation(RoomMemberDetails)
+    case kickConfirmation
+    case banConfirmation
 }

ElementX/Sources/Screens/RoomMemberListScreen/RoomMembersListScreenViewModel.swift

@@ -51,9 +51,31 @@ class RoomMembersListScreenViewModel: RoomMembersListScreenViewModelType, RoomMe
         case .showMemberDetails(let member):
             showMemberDetails(member)
         case .kickMember(let member):
-            Task { await kickMember(member) }
+            var reason: String?
+            let binding: Binding<String> = .init(get: { reason ?? "" },
+                                                 set: { reason = $0.isEmpty ? nil : $0 })
+            state.bindings.alertInfo = .init(id: .kickConfirmation,
+                                             title: L10n.screenRoomMemberListKickMemberConfirmationTitle,
+                                             message: L10n.screenRoomMemberListKickMemberConfirmationDescription,
+                                             primaryButton: .init(title: L10n.actionCancel, role: .cancel) { },
+                                             secondaryButton: .init(title: L10n.screenRoomMemberListKickMemberConfirmationAction) { [weak self] in Task { await self?.kickMember(member, reason: reason) } },
+                                             textFields: [.init(placeholder: L10n.commonReason,
+                                                                text: binding,
+                                                                autoCapitalization: .sentences,
+                                                                autoCorrectionDisabled: false)])
         case .banMember(let member):
-            Task { await banMember(member) }
+            var reason: String?
+            let binding: Binding<String> = .init(get: { reason ?? "" },
+                                                 set: { reason = $0.isEmpty ? nil : $0 })
+            state.bindings.alertInfo = .init(id: .banConfirmation,
+                                             title: L10n.screenRoomMemberListBanMemberConfirmationTitle,
+                                             message: L10n.screenRoomMemberListBanMemberConfirmationDescription,
+                                             primaryButton: .init(title: L10n.actionCancel, role: .cancel) { },
+                                             secondaryButton: .init(title: L10n.screenRoomMemberListBanMemberConfirmationAction) { [weak self] in Task { await self?.banMember(member, reason: reason) } },
+                                             textFields: [.init(placeholder: L10n.commonReason,
+                                                                text: binding,
+                                                                autoCapitalization: .sentences,
+                                                                autoCorrectionDisabled: false)])
         case .unbanMember(let member):
             Task { await unbanMember(member) }
         case .invite:
@@ -183,11 +205,11 @@ class RoomMembersListScreenViewModel: RoomMembersListScreenViewModelType, RoomMe
     
     // MARK: - Member Management
     
-    private func kickMember(_ member: RoomMemberDetails) async {
+    private func kickMember(_ member: RoomMemberDetails, reason: String?) async {
         let indicatorTitle = L10n.screenRoomMemberListRemovingUser(member.name ?? member.id)
         showManageMemberIndicator(title: indicatorTitle)
         
-        switch await roomProxy.kickUser(member.id) {
+        switch await roomProxy.kickUser(member.id, reason: reason) {
         case .success:
             state.bindings.memberToManage = nil
             hideManageMemberIndicator(title: indicatorTitle)
@@ -197,11 +219,11 @@ class RoomMembersListScreenViewModel: RoomMembersListScreenViewModelType, RoomMe
         }
     }
     
-    private func banMember(_ member: RoomMemberDetails) async {
+    private func banMember(_ member: RoomMemberDetails, reason: String?) async {
         let indicatorTitle = L10n.screenRoomMemberListBanningUser(member.name ?? member.id)
         showManageMemberIndicator(title: indicatorTitle)
         
-        switch await roomProxy.banUser(member.id) {
+        switch await roomProxy.banUser(member.id, reason: reason) {
         case .success:
             state.bindings.memberToManage = nil
             hideManageMemberIndicator(title: indicatorTitle)

ElementX/Sources/Screens/RoomMemberListScreen/View/RoomMembersListManageMemberSheet.swift

@@ -14,8 +14,6 @@ struct RoomMembersListManageMemberSheet: View {
     
     @ObservedObject var context: RoomMembersListScreenViewModel.Context
     
-    @State private var isPresentingBanConfirmation = false
-    
     var body: some View {
         Form {
             AvatarHeaderView(member: member,
@@ -44,7 +42,7 @@ struct RoomMembersListManageMemberSheet: View {
                                             icon: \.block,
                                             role: .destructive),
                             kind: .button {
-                                isPresentingBanConfirmation = true
+                                context.send(viewAction: .banMember(member))
                             })
                 }
             }
@@ -53,14 +51,6 @@ struct RoomMembersListManageMemberSheet: View {
         .scrollBounceBehavior(.basedOnSize)
         .presentationDragIndicator(.visible)
         .presentationDetents([.large, .fraction(0.54)]) // Maybe find a way to use the ideal height somehow?
-        .alert(L10n.screenRoomMemberListBanMemberConfirmationTitle, isPresented: $isPresentingBanConfirmation) {
-            Button(L10n.actionCancel, role: .cancel) { }
-            Button(L10n.screenRoomMemberListBanMemberConfirmationAction) {
-                context.send(viewAction: .banMember(member))
-            }
-        } message: {
-            Text(L10n.screenRoomMemberListBanMemberConfirmationDescription)
-        }
     }
 }
 

ElementX/Sources/Services/Room/JoinedRoomProxy.swift

@@ -623,9 +623,9 @@ class JoinedRoomProxy: JoinedRoomProxyProtocol {
     
     // MARK: - Moderation
     
-    func kickUser(_ userID: String) async -> Result<Void, RoomProxyError> {
+    func kickUser(_ userID: String, reason: String?) async -> Result<Void, RoomProxyError> {
         do {
-            try await room.kickUser(userId: userID, reason: nil)
+            try await room.kickUser(userId: userID, reason: reason)
             return .success(())
         } catch {
             MXLog.error("Failed kicking \(userID) with error: \(error)")
@@ -633,9 +633,9 @@ class JoinedRoomProxy: JoinedRoomProxyProtocol {
         }
     }
     
-    func banUser(_ userID: String) async -> Result<Void, RoomProxyError> {
+    func banUser(_ userID: String, reason: String?) async -> Result<Void, RoomProxyError> {
         do {
-            try await room.banUser(userId: userID, reason: nil)
+            try await room.banUser(userId: userID, reason: reason)
             return .success(())
         } catch {
             MXLog.error("Failed banning \(userID) with error: \(error)")

ElementX/Sources/Services/Room/RoomProxyProtocol.swift

@@ -159,8 +159,8 @@ protocol JoinedRoomProxyProtocol: RoomProxyProtocol {
     
     // MARK: - Moderation
     
-    func kickUser(_ userID: String) async -> Result<Void, RoomProxyError>
+    func kickUser(_ userID: String, reason: String?) async -> Result<Void, RoomProxyError>
-    func banUser(_ userID: String) async -> Result<Void, RoomProxyError>
+    func banUser(_ userID: String, reason: String?) async -> Result<Void, RoomProxyError>
     func unbanUser(_ userID: String) async -> Result<Void, RoomProxyError>
     
     // MARK: - Element Call

UnitTests/Sources/RoomMembersListScreenViewModelTests.swift

@@ -249,28 +249,42 @@ class RoomMembersListScreenViewModelTests: XCTestCase {
     
     func testKickMember() async throws {
         setup(with: .allMembersAsAdmin)
-        let deferred = deferFulfillment(context.$viewState) { !$0.visibleJoinedMembers.isEmpty }
+        var deferred = deferFulfillment(context.$viewState) { !$0.visibleJoinedMembers.isEmpty }
         try await deferred.fulfill()
         
+        let expectation = XCTestExpectation(description: "Ban member")
+        roomProxy.kickUserReasonClosure = { _, _ in
+            defer { expectation.fulfill() }
+            return .success(())
+        }
+        
         context.send(viewAction: .kickMember(viewModel.state.visibleJoinedMembers[0].member))
         
-        // Calling the mock won't actually change any view state, so sleep instead.
+        deferred = deferFulfillment(context.$viewState) { $0.bindings.alertInfo != nil }
-        try await Task.sleep(for: .milliseconds(100))
+        try await deferred.fulfill()
         
-        XCTAssert(roomProxy.kickUserCalled)
+        context.alertInfo?.secondaryButton?.action?()
+        await fulfillment(of: [expectation])
     }
     
     func testBanMember() async throws {
         setup(with: .allMembersAsAdmin)
-        let deferred = deferFulfillment(context.$viewState) { !$0.visibleJoinedMembers.isEmpty }
+        var deferred = deferFulfillment(context.$viewState) { !$0.visibleJoinedMembers.isEmpty }
         try await deferred.fulfill()
         
+        let expectation = XCTestExpectation(description: "Ban member")
+        roomProxy.banUserReasonClosure = { _, _ in
+            defer { expectation.fulfill() }
+            return .success(())
+        }
+        
         context.send(viewAction: .banMember(viewModel.state.visibleJoinedMembers[0].member))
         
-        // Calling the mock won't actually change any view state, so sleep instead.
+        deferred = deferFulfillment(context.$viewState) { $0.bindings.alertInfo != nil }
-        try await Task.sleep(for: .milliseconds(100))
+        try await deferred.fulfill()
         
-        XCTAssert(roomProxy.banUserCalled)
+        context.alertInfo?.secondaryButton?.action?()
+        await fulfillment(of: [expectation])
     }
     
     func testUnbanMember() async throws {