Moderation tweaks (#2548)

* Only allow admins to see the roles and permissions screen. * Hide the selection checkbox on Admins when changing roles. * Show an empty state for banned users. * Add separate actions for ban and remove. * Implement reset permissions and demote self alerts. * Add tests for resetting permissions and demoting self. * Add a warning when promoting someone to administrator.
element-hq · Mar 11, 2024 · 71721f0 · 71721f0
1 parent 9ce12e6
commit 71721f0
Show file tree

Hide file tree

Showing 37 changed files with 411 additions and 104 deletions.
diff --git a/ElementX/Resources/Localizations/en.lproj/Localizable.strings b/ElementX/Resources/Localizations/en.lproj/Localizable.strings
@@ -75,6 +75,7 @@
 "action_reply_in_thread" = "Reply in thread";
 "action_report_bug" = "Report bug";
 "action_report_content" = "Report content";
+"action_reset" = "Reset";
 "action_retry" = "Retry";
 "action_retry_decryption" = "Retry decryption";
 "action_save" = "Save";
@@ -547,15 +548,17 @@
 "screen_room_member_list_ban_member_confirmation_action" = "Ban";
 "screen_room_member_list_ban_member_confirmation_description" = "They won’t be able to join this room again if invited.";
 "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_manage_member_remove" = "Remove 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_ban" = "Remove and ban member";
 "screen_room_member_list_manage_member_remove_confirmation_kick" = "Only remove member";
 "screen_room_member_list_manage_member_remove_confirmation_title" = "Remove member and ban from joining in the future?";
 "screen_room_member_list_manage_member_unban_action" = "Unban";
 "screen_room_member_list_manage_member_unban_message" = "They will be able to join this room again if invited.";
 "screen_room_member_list_manage_member_unban_title" = "Unban user";
-"screen_room_member_list_manage_member_user_info" = "See user info";
+"screen_room_member_list_manage_member_user_info" = "View profile";
 "screen_room_member_list_mode_banned" = "Banned";
 "screen_room_member_list_mode_members" = "Members";
 "screen_room_member_list_pending_header_title" = "Pending";
@@ -592,6 +595,8 @@
 "screen_room_roles_and_permissions_moderators" = "Moderators";
 "screen_room_roles_and_permissions_permissions_header" = "Permissions";
 "screen_room_roles_and_permissions_reset" = "Reset permissions";
+"screen_room_roles_and_permissions_reset_confirm_description" = "Once you reset permissions, you will lose your current settings.";
+"screen_room_roles_and_permissions_reset_confirm_title" = "Reset permissions?";
 "screen_room_roles_and_permissions_roles_header" = "Roles";
 "screen_room_roles_and_permissions_room_details" = "Room details";
 "screen_room_roles_and_permissions_title" = "Roles and permissions";

diff --git a/ElementX/Sources/FlowCoordinators/RoomRolesAndPermissionsFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/RoomRolesAndPermissionsFlowCoordinator.swift
@@ -54,8 +54,10 @@ class RoomRolesAndPermissionsFlowCoordinator: FlowCoordinatorProtocol {
         case finishedChangingRoles
         /// The user would like to change room permissions.
         case changePermissions
-        /// The user finished changing room permissions
+        /// The user finished changing room permissions.
         case finishedChangingPermissions
+        /// The user has demoted themself.
+        case demotedOwnUser
     }
 
     private let stateMachine: StateMachine<State, Event>
@@ -118,20 +120,26 @@ class RoomRolesAndPermissionsFlowCoordinator: FlowCoordinatorProtocol {
         }
         stateMachine.addRoutes(event: .finishedChangingPermissions, transitions: [.changingPermissions => .rolesAndPermissionsScreen])
 
+        stateMachine.addHandler(event: .demotedOwnUser) { [weak self] _ in
+            self?.actionsSubject.send(.complete)
+        }
+
         stateMachine.addErrorHandler { context in
             fatalError("Unexpected transition: \(context)")
         }
     }
 
     private func presentRolesAndPermissionsScreen() {
-        let parameters = RoomRolesAndPermissionsScreenCoordinatorParameters(roomProxy: roomProxy)
+        let parameters = RoomRolesAndPermissionsScreenCoordinatorParameters(roomProxy: roomProxy, userIndicatorController: userIndicatorController)
         let coordinator = RoomRolesAndPermissionsScreenCoordinator(parameters: parameters)
-        coordinator.actions.sink { [stateMachine] action in
+        coordinator.actionsPublisher.sink { [stateMachine] action in
             switch action {
             case .editRoles(let role):
                 stateMachine.tryEvent(.changeRoles, userInfo: role)
             case .editPermissions(let group):
                 stateMachine.tryEvent(.changePermissions, userInfo: (group, RoomPermissions(powerLevels: .mock)))
+            case .demotedOwnUser:
+                stateMachine.tryEvent(.demotedOwnUser)
             }
         }
         .store(in: &cancellables)

diff --git a/ElementX/Sources/Generated/Strings.swift b/ElementX/Sources/Generated/Strings.swift
@@ -184,6 +184,8 @@ internal enum L10n {
   internal static var actionReportBug: String { return L10n.tr("Localizable", "action_report_bug") }
   /// Report content
   internal static var actionReportContent: String { return L10n.tr("Localizable", "action_report_content") }
+  /// Reset
+  internal static var actionReset: String { return L10n.tr("Localizable", "action_reset") }
   /// Retry
   internal static var actionRetry: String { return L10n.tr("Localizable", "action_retry") }
   /// Retry decryption
@@ -1341,6 +1343,8 @@ internal enum L10n {
   internal static var screenRoomMemberListBanMemberConfirmationDescription: String { return L10n.tr("Localizable", "screen_room_member_list_ban_member_confirmation_description") }
   /// Are you sure you want to ban this member?
   internal static var screenRoomMemberListBanMemberConfirmationTitle: String { return L10n.tr("Localizable", "screen_room_member_list_ban_member_confirmation_title") }
+  /// There are no banned users in this room.
+  internal static var screenRoomMemberListBannedEmpty: String { return L10n.tr("Localizable", "screen_room_member_list_banned_empty") }
   /// Banning %1$@
   internal static func screenRoomMemberListBanningUser(_ p1: Any) -> String {
     return L10n.tr("Localizable", "screen_room_member_list_banning_user", String(describing: p1))
@@ -1349,7 +1353,9 @@ internal enum L10n {
   internal static func screenRoomMemberListHeaderTitle(_ p1: Int) -> String {
     return L10n.tr("Localizable", "screen_room_member_list_header_title", p1)
   }
-  /// Remove member
+  /// Remove and ban member
+  internal static var screenRoomMemberListManageMemberBan: String { return L10n.tr("Localizable", "screen_room_member_list_manage_member_ban") }
+  /// Remove from room
   internal static var screenRoomMemberListManageMemberRemove: String { return L10n.tr("Localizable", "screen_room_member_list_manage_member_remove") }
   /// Remove and ban member
   internal static var screenRoomMemberListManageMemberRemoveConfirmationBan: String { return L10n.tr("Localizable", "screen_room_member_list_manage_member_remove_confirmation_ban") }
@@ -1363,7 +1369,7 @@ internal enum L10n {
   internal static var screenRoomMemberListManageMemberUnbanMessage: String { return L10n.tr("Localizable", "screen_room_member_list_manage_member_unban_message") }
   /// Unban user
   internal static var screenRoomMemberListManageMemberUnbanTitle: String { return L10n.tr("Localizable", "screen_room_member_list_manage_member_unban_title") }
-  /// See user info
+  /// View profile
   internal static var screenRoomMemberListManageMemberUserInfo: String { return L10n.tr("Localizable", "screen_room_member_list_manage_member_user_info") }
   /// Banned
   internal static var screenRoomMemberListModeBanned: String { return L10n.tr("Localizable", "screen_room_member_list_mode_banned") }
@@ -1449,6 +1455,10 @@ internal enum L10n {
   internal static var screenRoomRolesAndPermissionsPermissionsHeader: String { return L10n.tr("Localizable", "screen_room_roles_and_permissions_permissions_header") }
   /// Reset permissions
   internal static var screenRoomRolesAndPermissionsReset: String { return L10n.tr("Localizable", "screen_room_roles_and_permissions_reset") }
+  /// Once you reset permissions, you will lose your current settings.
+  internal static var screenRoomRolesAndPermissionsResetConfirmDescription: String { return L10n.tr("Localizable", "screen_room_roles_and_permissions_reset_confirm_description") }
+  /// Reset permissions?
+  internal static var screenRoomRolesAndPermissionsResetConfirmTitle: String { return L10n.tr("Localizable", "screen_room_roles_and_permissions_reset_confirm_title") }
   /// Roles
   internal static var screenRoomRolesAndPermissionsRolesHeader: String { return L10n.tr("Localizable", "screen_room_roles_and_permissions_roles_header") }
   /// Room details

diff --git a/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift b/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift
@@ -2750,6 +2750,44 @@ class RoomProxyMock: RoomProxyProtocol {
             return applyPowerLevelChangesReturnValue
         }
     }
+    //MARK: - resetPowerLevels
+
+    var resetPowerLevelsCallsCount = 0
+    var resetPowerLevelsCalled: Bool {
+        return resetPowerLevelsCallsCount > 0
+    }
+    var resetPowerLevelsReturnValue: Result<RoomPowerLevels, RoomProxyError>!
+    var resetPowerLevelsClosure: (() async -> Result<RoomPowerLevels, RoomProxyError>)?
+
+    func resetPowerLevels() async -> Result<RoomPowerLevels, RoomProxyError> {
+        resetPowerLevelsCallsCount += 1
+        if let resetPowerLevelsClosure = resetPowerLevelsClosure {
+            return await resetPowerLevelsClosure()
+        } else {
+            return resetPowerLevelsReturnValue
+        }
+    }
+    //MARK: - suggestedRole
+
+    var suggestedRoleForCallsCount = 0
+    var suggestedRoleForCalled: Bool {
+        return suggestedRoleForCallsCount > 0
+    }
+    var suggestedRoleForReceivedUserID: String?
+    var suggestedRoleForReceivedInvocations: [String] = []
+    var suggestedRoleForReturnValue: Result<RoomMemberRole, RoomProxyError>!
+    var suggestedRoleForClosure: ((String) async -> Result<RoomMemberRole, RoomProxyError>)?
+
+    func suggestedRole(for userID: String) async -> Result<RoomMemberRole, RoomProxyError> {
+        suggestedRoleForCallsCount += 1
+        suggestedRoleForReceivedUserID = userID
+        suggestedRoleForReceivedInvocations.append(userID)
+        if let suggestedRoleForClosure = suggestedRoleForClosure {
+            return await suggestedRoleForClosure(userID)
+        } else {
+            return suggestedRoleForReturnValue
+        }
+    }
     //MARK: - updatePowerLevelsForUsers
 
     var updatePowerLevelsForUsersCallsCount = 0

diff --git a/ElementX/Sources/Mocks/RoomProxyMock.swift b/ElementX/Sources/Mocks/RoomProxyMock.swift
@@ -42,7 +42,6 @@ struct RoomProxyMockConfiguration {
     }()
 
     var members: [RoomMemberProxyMock] = .allMembers
-    var memberForID: RoomMemberProxyMock = .mockMe
     var ownUserID = RoomMemberProxyMock.mockMe.userID
 
     var canUserInvite = true
@@ -81,7 +80,12 @@ extension RoomProxyMock {
         underlyingActionsPublisher = Empty(completeImmediately: false).eraseToAnyPublisher()
         setNameClosure = { _ in .success(()) }
         setTopicClosure = { _ in .success(()) }
-        getMemberUserIDReturnValue = .success(configuration.memberForID)
+        getMemberUserIDClosure = { [weak self] userID in
+            guard let member = self?.membersPublisher.value.first(where: { $0.userID == userID }) else {
+                return .failure(.failedRetrievingMember)
+            }
+            return .success(member)
+        }
 
         flagAsUnreadReturnValue = .success(())
         markAsReadReceiptTypeReturnValue = .success(())
@@ -90,6 +94,13 @@ extension RoomProxyMock {
 
         powerLevelsReturnValue = .success(.mock)
         applyPowerLevelChangesReturnValue = .success(())
+        resetPowerLevelsReturnValue = .success(.mock)
+        suggestedRoleForClosure = { [weak self] userID in
+            guard case .success(let member) = await self?.getMember(userID: userID) else {
+                return .failure(.failedCheckingPermission)
+            }
+            return .success(member.role)
+        }
         updatePowerLevelsForUsersReturnValue = .success(())
         canUserUserIDSendStateEventClosure = { [weak self] userID, _ in
             .success(self?.membersPublisher.value.first { $0.userID == userID }?.role ?? .user != .user)

diff --git a/ElementX/Sources/Screens/RoomChangeRolesScreen/RoomChangeRolesScreenModels.swift b/ElementX/Sources/Screens/RoomChangeRolesScreen/RoomChangeRolesScreenModels.swift
@@ -83,12 +83,14 @@ struct RoomChangeRolesScreenViewState: BindableState {
 struct RoomChangeRolesScreenViewStateBindings {
     var searchQuery = ""
     /// Information about the currently displayed alert.
-    var alertInfo: AlertInfo<RoomChangePermissionsScreenAlertType>?
+    var alertInfo: AlertInfo<RoomChangeRolesScreenAlertType>?
 }
 
 enum RoomChangeRolesScreenAlertType {
+    /// A warning that a particular promotion can't be undone.
+    case promotionWarning
     /// The generic error message.
-    case generic
+    case error
 }
 
 enum RoomChangeRolesScreenViewAction {

diff --git a/ElementX/Sources/Screens/RoomChangeRolesScreen/RoomChangeRolesScreenViewModel.swift b/ElementX/Sources/Screens/RoomChangeRolesScreen/RoomChangeRolesScreenViewModel.swift
@@ -64,7 +64,11 @@ class RoomChangeRolesScreenViewModel: RoomChangeRolesScreenViewModelType, RoomCh
         case .demoteMember(let member):
             demoteMember(member)
         case .save:
-            Task { await save() }
+            if state.mode == .administrator, !state.membersToPromote.isEmpty {
+                showPromotionWarning()
+            } else {
+                Task { await save() }
+            }
         }
     }
 
@@ -99,6 +103,16 @@ class RoomChangeRolesScreenViewModel: RoomChangeRolesScreenViewModelType, RoomCh
         }
     }
 
+    private func showPromotionWarning() {
+        context.alertInfo = AlertInfo(id: .promotionWarning,
+                                      title: L10n.screenRoomChangeRoleConfirmAddAdminTitle,
+                                      message: L10n.screenRoomChangeRoleConfirmAddAdminDescription,
+                                      primaryButton: .init(title: L10n.actionContinue) {
+                                          Task { await self.save() }
+                                      },
+                                      secondaryButton: .init(title: L10n.actionCancel, role: .cancel, action: nil))
+    }
+
     private func save() async {
         showSavingIndicator()
 
@@ -112,7 +126,7 @@ class RoomChangeRolesScreenViewModel: RoomChangeRolesScreenViewModelType, RoomCh
         case .success:
             MXLog.info("Success")
         case .failure:
-            context.alertInfo = AlertInfo(id: .generic)
+            context.alertInfo = AlertInfo(id: .error)
         }
     }
 

diff --git a/ElementX/Sources/Screens/RoomChangeRolesScreen/View/RoomChangeRolesScreen.swift b/ElementX/Sources/Screens/RoomChangeRolesScreen/View/RoomChangeRolesScreen.swift
@@ -77,10 +77,10 @@ struct RoomChangeRolesScreen: View {
                 ForEach(context.viewState.visibleMembers, id: \.id) { member in
                     RoomChangeRolesScreenRow(member: member,
                                              imageProvider: context.imageProvider,
-                                             kind: .multiSelection(isSelected: context.viewState.isMemberSelected(member)) {
-                                                 context.send(viewAction: .toggleMember(member))
-                                             })
-                                             .disabled(member.role == .administrator)
+                                             isSelected: context.viewState.isMemberSelected(member)) {
+                        context.send(viewAction: .toggleMember(member))
+                    }
+                    .disabled(member.role == .administrator)
                 }
             } header: {
                 Text(L10n.screenRoomMemberListRoomMembersHeaderTitle)

diff --git a/ElementX/Sources/Screens/RoomChangeRolesScreen/View/RoomChangeRolesScreenRow.swift b/ElementX/Sources/Screens/RoomChangeRolesScreen/View/RoomChangeRolesScreenRow.swift
@@ -19,16 +19,19 @@ import MatrixRustSDK
 import SwiftUI
 
 struct RoomChangeRolesScreenRow: View {
+    @Environment(\.isEnabled) private var isEnabled
+
     let member: RoomMemberDetails
     let imageProvider: ImageProviderProtocol?
 
-    let kind: ListRow<LoadableAvatarImage, EmptyView, EmptyView, Bool>.Kind<EmptyView, Bool>
+    let isSelected: Bool
+    let action: () -> Void
 
     var body: some View {
         ListRow(label: .avatar(title: member.name ?? member.id,
                                description: member.name == nil ? nil : member.id,
                                icon: avatar),
-                kind: kind)
+                kind: isEnabled ? .multiSelection(isSelected: isSelected, action: action) : .label)
     }
 
     var avatar: LoadableAvatarImage {
@@ -47,25 +50,30 @@ struct RoomChangeRolesScreenRow_Previews: PreviewProvider, TestablePreview {
         Form {
             RoomChangeRolesScreenRow(member: .init(withProxy: RoomMemberProxyMock.mockAlice),
                                      imageProvider: MockMediaProvider(),
-                                     kind: .multiSelection(isSelected: true, action: action))
+                                     isSelected: true,
+                                     action: action)
 
             RoomChangeRolesScreenRow(member: .init(withProxy: RoomMemberProxyMock.mockBob),
                                      imageProvider: MockMediaProvider(),
-                                     kind: .multiSelection(isSelected: false, action: action))
+                                     isSelected: false,
+                                     action: action)
 
             RoomChangeRolesScreenRow(member: .init(withProxy: RoomMemberProxyMock.mockCharlie),
                                      imageProvider: MockMediaProvider(),
-                                     kind: .multiSelection(isSelected: true, action: action))
+                                     isSelected: true,
+                                     action: action)
                 .disabled(true)
 
             RoomChangeRolesScreenRow(member: .init(withProxy: RoomMemberProxyMock(with: .init(userID: "@someone:matrix.org", membership: .join))),
                                      imageProvider: MockMediaProvider(),
-                                     kind: .multiSelection(isSelected: false, action: action))
+                                     isSelected: false,
+                                     action: action)
                 .disabled(true)
 
             RoomChangeRolesScreenRow(member: .init(withProxy: RoomMemberProxyMock(with: .init(userID: "@someone:matrix.org", membership: .join))),
                                      imageProvider: MockMediaProvider(),
-                                     kind: .multiSelection(isSelected: false, action: action))
+                                     isSelected: false,
+                                     action: action)
         }
         .compoundList()
     }

diff --git a/ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenViewModel.swift b/ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenViewModel.swift
@@ -178,7 +178,7 @@ class RoomDetailsScreenViewModel: RoomDetailsScreenViewModelType, RoomDetailsScr
         state.canEditRoomTopic = await roomProxy.canUser(userID: roomProxy.ownUserID, sendStateEvent: .roomTopic) == .success(true)
         state.canEditRoomAvatar = await roomProxy.canUser(userID: roomProxy.ownUserID, sendStateEvent: .roomAvatar) == .success(true)
         if appSettings.roomModerationEnabled {
-            state.canEditRolesOrPermissions = await roomProxy.canUser(userID: roomProxy.ownUserID, sendStateEvent: .roomPowerLevels) == .success(true)
+            state.canEditRolesOrPermissions = await roomProxy.suggestedRole(for: roomProxy.ownUserID) == .success(.administrator)
         }
         state.canInviteUsers = await canInviteUsers
     }

diff --git a/ElementX/Sources/Screens/RoomMemberListScreen/RoomMembersListScreenViewModel.swift b/ElementX/Sources/Screens/RoomMemberListScreen/RoomMembersListScreenViewModel.swift
@@ -114,10 +114,6 @@ class RoomMembersListScreenViewModel: RoomMembersListScreenViewModelType, RoomMe
             self.state.canKickUsers = await canKickUsers == .success(true)
             self.state.canBanUsers = await canBanUsers == .success(true)
 
-            if state.bindings.mode == .banned, roomMembersDetails.bannedMembers.isEmpty {
-                state.bindings.mode = .members
-            }
-
             hideLoader()
         }
     }