More Moderation tweaks (#2566)

* Show room member role changes in the timeline.

* Fix a bug in room flow coordinator.

* Tidy up roles and permissions flow.

* Refresh the power levels in the room details screen.

* Automatically update permissions after saving.

* Remove extra button.

* Add a short delay to the roles and permissions screen snapshots.
The permissions rows are now in a loading state initially.
This commit is contained in:
Doug 2024-03-13 15:36:38 +00:00 committed by GitHub
parent 9d68d9e250
commit 1c914c314f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
17 changed files with 212 additions and 43 deletions

View File

@ -353,7 +353,7 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol {
case (.roomDetails, .presentRolesAndPermissionsScreen, .rolesAndPermissions):
presentRolesAndPermissionsScreen()
case (.rolesAndPermissions, .dismissRolesAndPermissionsScreen, .roomDetails):
break
rolesAndPermissionsFlowCoordinator = nil
default:
fatalError("Unknown transition: \(context)")
@ -1181,7 +1181,7 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol {
coordinator.actionsPublisher.sink { [weak self] action in
switch action {
case .complete:
self?.rolesAndPermissionsFlowCoordinator = nil
self?.stateMachine.tryEvent(.dismissRolesAndPermissionsScreen)
}
}
.store(in: &cancellables)

View File

@ -43,6 +43,8 @@ class RoomRolesAndPermissionsFlowCoordinator: FlowCoordinatorProtocol {
case changingRoles
/// Changing room permissions.
case changingPermissions
/// The flow is complete and the stack has been cleaned up.
case complete
}
enum Event: EventType {
@ -89,7 +91,7 @@ class RoomRolesAndPermissionsFlowCoordinator: FlowCoordinatorProtocol {
func clearRoute(animated: Bool) {
// As we push screens on top of an existing stack, popping to root wouldn't be safe.
switch stateMachine.state {
case .initial:
case .initial, .complete:
break
case .rolesAndPermissionsScreen:
navigationStackCoordinator.pop(animated: animated)
@ -113,15 +115,15 @@ class RoomRolesAndPermissionsFlowCoordinator: FlowCoordinatorProtocol {
stateMachine.addRoutes(event: .finishedChangingRoles, transitions: [.changingRoles => .rolesAndPermissionsScreen])
stateMachine.addRoutes(event: .changePermissions, transitions: [.rolesAndPermissionsScreen => .changingPermissions]) { [weak self] context in
guard let (group, permissions) = context.userInfo as? (RoomRolesAndPermissionsScreenPermissionsGroup, RoomPermissions) else {
guard let (permissions, group) = context.userInfo as? (RoomPermissions, RoomRolesAndPermissionsScreenPermissionsGroup) else {
fatalError("Expected a group and the current permissions")
}
self?.presentChangePermissionsScreen(permissions: permissions, group: group)
}
stateMachine.addRoutes(event: .finishedChangingPermissions, transitions: [.changingPermissions => .rolesAndPermissionsScreen])
stateMachine.addHandler(event: .demotedOwnUser) { [weak self] _ in
self?.actionsSubject.send(.complete)
stateMachine.addRoutes(event: .demotedOwnUser, transitions: [.rolesAndPermissionsScreen => .complete]) { [weak self] _ in
self?.navigationStackCoordinator.pop()
}
stateMachine.addErrorHandler { context in
@ -136,8 +138,8 @@ class RoomRolesAndPermissionsFlowCoordinator: FlowCoordinatorProtocol {
switch action {
case .editRoles(let role):
stateMachine.tryEvent(.changeRoles, userInfo: role)
case .editPermissions(let group):
stateMachine.tryEvent(.changePermissions, userInfo: (group, RoomPermissions(powerLevels: .mock)))
case .editPermissions(let permissions, let group):
stateMachine.tryEvent(.changePermissions, userInfo: (permissions, group))
case .demotedOwnUser:
stateMachine.tryEvent(.demotedOwnUser)
}
@ -162,7 +164,7 @@ class RoomRolesAndPermissionsFlowCoordinator: FlowCoordinatorProtocol {
coordinator.actionsPublisher.sink { [weak self] action in
guard let self else { return }
switch action {
case .done:
case .complete:
// When discarding changes is finalised, either use an event or remove this action.
navigationStackCoordinator.pop()
}
@ -184,7 +186,7 @@ class RoomRolesAndPermissionsFlowCoordinator: FlowCoordinatorProtocol {
guard let self else { return }
switch action {
case .cancel:
case .complete:
// When discarding changes is finalised, either use an event or remove this action.
navigationStackCoordinator.pop()
}

View File

@ -434,7 +434,7 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol
state.bindings.alertInfo = AlertInfo(id: UUID(), title: L10n.errorUnknown)
case .some(.success):
userIndicatorController.submitIndicator(UserIndicator(id: UUID().uuidString,
type: .modal(progress: .none, interactiveDismissDisabled: false, allowsInteraction: false),
type: .toast,
title: L10n.commonCurrentUserLeftRoom,
iconName: "checkmark"))
actionsSubject.send(.roomLeft(roomIdentifier: roomId))

View File

@ -27,7 +27,7 @@ struct RoomChangePermissionsScreenCoordinatorParameters {
}
enum RoomChangePermissionsScreenCoordinatorAction {
case cancel
case complete
}
final class RoomChangePermissionsScreenCoordinator: CoordinatorProtocol {
@ -55,8 +55,8 @@ final class RoomChangePermissionsScreenCoordinator: CoordinatorProtocol {
guard let self else { return }
switch action {
case .cancel:
actionsSubject.send(.cancel)
case .complete:
actionsSubject.send(.complete)
}
}
.store(in: &cancellables)

View File

@ -18,7 +18,7 @@ import Foundation
import MatrixRustSDK
enum RoomChangePermissionsScreenViewModelAction {
case cancel
case complete
}
struct RoomChangePermissionsScreenViewState: BindableState {

View File

@ -21,8 +21,8 @@ import SwiftUI
typealias RoomChangePermissionsScreenViewModelType = StateStoreViewModel<RoomChangePermissionsScreenViewState, RoomChangePermissionsScreenViewAction>
class RoomChangePermissionsScreenViewModel: RoomChangePermissionsScreenViewModelType, RoomChangePermissionsScreenViewModelProtocol {
let roomProxy: RoomProxyProtocol
let userIndicatorController: UserIndicatorControllerProtocol
private let roomProxy: RoomProxyProtocol
private let userIndicatorController: UserIndicatorControllerProtocol
private var actionsSubject: PassthroughSubject<RoomChangePermissionsScreenViewModelAction, Never> = .init()
var actionsPublisher: AnyPublisher<RoomChangePermissionsScreenViewModelAction, Never> {
@ -71,14 +71,7 @@ class RoomChangePermissionsScreenViewModel: RoomChangePermissionsScreenViewModel
switch await roomProxy.applyPowerLevelChanges(changes) {
case .success:
MXLog.info("Success")
case .failure:
context.alertInfo = AlertInfo(id: .generic)
return
}
switch await roomProxy.powerLevels() {
case .success(let powerLevels):
state.currentPermissions = .init(powerLevels: powerLevels)
actionsSubject.send(.complete)
case .failure:
context.alertInfo = AlertInfo(id: .generic)
return

View File

@ -26,7 +26,7 @@ struct RoomChangeRolesScreenCoordinatorParameters {
}
enum RoomChangeRolesScreenCoordinatorAction {
case done
case complete
}
final class RoomChangeRolesScreenCoordinator: CoordinatorProtocol {
@ -54,8 +54,8 @@ final class RoomChangeRolesScreenCoordinator: CoordinatorProtocol {
guard let self else { return }
switch action {
case .done:
self.actionsSubject.send(.done)
case .complete:
self.actionsSubject.send(.complete)
}
}
.store(in: &cancellables)

View File

@ -18,7 +18,7 @@ import Collections
import Foundation
enum RoomChangeRolesScreenViewModelAction {
case done
case complete
}
struct RoomChangeRolesScreenViewState: BindableState {

View File

@ -122,9 +122,19 @@ class RoomChangeRolesScreenViewModel: RoomChangeRolesScreenViewModelType, RoomCh
let promotingUpdates = state.membersToPromote.map { ($0.id, state.mode.rustPowerLevel) }
let demotingUpdates = state.membersToDemote.map { ($0.id, Int64(0)) }
// A task we can await until the room's info gets modified with the new power levels.
let infoTask = Task { await roomProxy.actionsPublisher.values.first { $0 == .roomInfoUpdate } }
switch await roomProxy.updatePowerLevelsForUsers(promotingUpdates + demotingUpdates) {
case .success:
MXLog.info("Success")
// Call updateMembers so the count is correct on the root screen.
_ = await infoTask.value
await roomProxy.updateMembers()
actionsSubject.send(.complete)
case .failure:
context.alertInfo = AlertInfo(id: .error)
}

View File

@ -135,6 +135,7 @@ class RoomDetailsScreenViewModel: RoomDetailsScreenViewModelType, RoomDetailsScr
.throttle(for: .milliseconds(200), scheduler: DispatchQueue.main, latest: true)
.sink { [weak self] _ in
self?.updateRoomInfo()
Task { await self?.updatePowerLevelPermissions() }
}
.store(in: &cancellables)
}

View File

@ -26,7 +26,7 @@ struct RoomRolesAndPermissionsScreenCoordinatorParameters {
enum RoomRolesAndPermissionsScreenCoordinatorAction {
case editRoles(RoomRolesAndPermissionsScreenRole)
case editPermissions(RoomRolesAndPermissionsScreenPermissionsGroup)
case editPermissions(permissions: RoomPermissions, group: RoomRolesAndPermissionsScreenPermissionsGroup)
case demotedOwnUser
}
@ -52,8 +52,8 @@ final class RoomRolesAndPermissionsScreenCoordinator: CoordinatorProtocol {
switch action {
case .editRoles(let role):
actionsSubject.send(.editRoles(role))
case .editPermissions(let permissionsGroup):
actionsSubject.send(.editPermissions(permissionsGroup))
case .editPermissions(let permissions, let group):
actionsSubject.send(.editPermissions(permissions: permissions, group: group))
case .demotedOwnUser:
actionsSubject.send(.demotedOwnUser)
}

View File

@ -20,7 +20,7 @@ enum RoomRolesAndPermissionsScreenViewModelAction {
/// The user would like to edit member roles.
case editRoles(RoomRolesAndPermissionsScreenRole)
/// The user would like to edit room permissions.
case editPermissions(RoomRolesAndPermissionsScreenPermissionsGroup)
case editPermissions(permissions: RoomPermissions, group: RoomRolesAndPermissionsScreenPermissionsGroup)
/// The user has demoted themself.
case demotedOwnUser
}
@ -30,6 +30,8 @@ struct RoomRolesAndPermissionsScreenViewState: BindableState {
var administratorCount: Int?
/// The number of moderators in the room.
var moderatorCount: Int?
/// The permissions of the room when loaded.
var permissions: RoomPermissions?
var bindings = RoomRolesAndPermissionsScreenViewStateBindings()
}

View File

@ -33,12 +33,26 @@ class RoomRolesAndPermissionsScreenViewModel: RoomRolesAndPermissionsScreenViewM
self.userIndicatorController = userIndicatorController
super.init(initialViewState: RoomRolesAndPermissionsScreenViewState())
roomProxy.membersPublisher.sink { [weak self] members in
self?.updateMembers(members)
}
.store(in: &cancellables)
// Automatically update the admin/moderator counts.
roomProxy.membersPublisher
.receive(on: DispatchQueue.main)
.sink { [weak self] members in
self?.updateMembers(members)
}
.store(in: &cancellables)
updateMembers(roomProxy.membersPublisher.value)
// Automatically update the room permissions
roomProxy.actionsPublisher
.filter { $0 == .roomInfoUpdate }
.receive(on: DispatchQueue.main)
.sink { [weak self] _ in
Task { await self?.updatePermissions() }
}
.store(in: &cancellables)
Task { await updatePermissions() }
}
// MARK: - Public
@ -53,17 +67,17 @@ class RoomRolesAndPermissionsScreenViewModel: RoomRolesAndPermissionsScreenViewM
state.bindings.alertInfo = AlertInfo(id: .resetConfirmation,
title: L10n.screenRoomRolesAndPermissionsChangeMyRole,
message: L10n.screenRoomChangeRoleConfirmDemoteSelfDescription,
primaryButton: .init(title: L10n.actionCancel, role: .cancel) { },
verticalButtons: [
.init(title: L10n.screenRoomRolesAndPermissionsChangeRoleDemoteToModerator, role: .destructive) {
Task { await self.updateOwnRole(.moderator) }
},
.init(title: L10n.screenRoomRolesAndPermissionsChangeRoleDemoteToMember, role: .destructive) {
Task { await self.updateOwnRole(.user) }
},
.init(title: L10n.actionCancel, role: .cancel) { }
}
])
case .editPermissions(let permissionsGroup):
actionsSubject.send(.editPermissions(permissionsGroup))
editPermissions(group: permissionsGroup)
case .reset:
state.bindings.alertInfo = AlertInfo(id: .resetConfirmation,
title: L10n.screenRoomRolesAndPermissionsResetConfirmTitle,
@ -85,10 +99,16 @@ class RoomRolesAndPermissionsScreenViewModel: RoomRolesAndPermissionsScreenViewM
private func updateOwnRole(_ role: RoomMemberDetails.Role) async {
showSavingIndicator()
// A task we can await until the room's info gets modified with the new power levels.
let infoTask = Task { await roomProxy.actionsPublisher.values.first { $0 == .roomInfoUpdate } }
switch await roomProxy.updatePowerLevelsForUsers([(userID: roomProxy.ownUserID, powerLevel: role.rustPowerLevel)]) {
case .success:
showSuccessIndicator()
_ = await infoTask.value
await roomProxy.updateMembers()
actionsSubject.send(.demotedOwnUser)
showSuccessIndicator()
case .failure:
state.bindings.alertInfo = AlertInfo(id: .error)
}
@ -96,6 +116,26 @@ class RoomRolesAndPermissionsScreenViewModel: RoomRolesAndPermissionsScreenViewM
hideSavingIndicator()
}
// MARK: - Permissions
private func updatePermissions() async {
switch await roomProxy.powerLevels() {
case .success(let powerLevels):
state.permissions = .init(powerLevels: powerLevels)
case .failure:
break
}
}
private func editPermissions(group: RoomRolesAndPermissionsScreenPermissionsGroup) {
guard let permissions = state.permissions else {
state.bindings.alertInfo = AlertInfo(id: .error)
MXLog.error("Missing permissions.")
return
}
actionsSubject.send(.editPermissions(permissions: permissions, group: group))
}
private func resetPermissions() async {
showSavingIndicator()

View File

@ -82,24 +82,30 @@ struct RoomRolesAndPermissionsScreen: View {
Section {
ListRow(label: .default(title: L10n.screenRoomRolesAndPermissionsRoomDetails,
icon: \.info),
details: .isWaiting(context.viewState.permissions == nil),
kind: .navigationLink {
context.send(viewAction: .editPermissions(.roomDetails))
})
.accessibilityIdentifier(A11yIdentifiers.roomRolesAndPermissionsScreen.roomDetails)
.disabled(context.viewState.permissions == nil)
ListRow(label: .default(title: L10n.screenRoomRolesAndPermissionsMessagesAndContent,
icon: \.chat),
details: .isWaiting(context.viewState.permissions == nil),
kind: .navigationLink {
context.send(viewAction: .editPermissions(.messagesAndContent))
})
.accessibilityIdentifier(A11yIdentifiers.roomRolesAndPermissionsScreen.messagesAndContent)
.disabled(context.viewState.permissions == nil)
ListRow(label: .default(title: L10n.screenRoomRolesAndPermissionsMemberModeration,
icon: \.user),
details: .isWaiting(context.viewState.permissions == nil),
kind: .navigationLink {
context.send(viewAction: .editPermissions(.memberModeration))
})
.accessibilityIdentifier(A11yIdentifiers.roomRolesAndPermissionsScreen.memberModeration)
.disabled(context.viewState.permissions == nil)
} header: {
Text(L10n.screenRoomRolesAndPermissionsPermissionsHeader)
.compoundListSectionHeader()
@ -126,5 +132,6 @@ struct RoomRolesAndPermissionsScreen_Previews: PreviewProvider, TestablePreview
NavigationStack {
RoomRolesAndPermissionsScreen(context: viewModel.context)
}
.snapshot(delay: 0.2)
}
}

View File

@ -683,7 +683,6 @@ class ClientProxy: ClientProxyProtocol {
.roomHistoryVisibility,
.roomJoinRules,
.roomPinnedEvents,
.roomPowerLevels,
.roomServerAcl,
.roomTombstone,
.spaceChild,

View File

@ -151,6 +151,10 @@ struct RoomStateEventStringBuilder {
case (nil, true):
return L10n.stateEventRoomNameRemovedByYou
}
case .roomPowerLevels(let users, let previous):
let newRoles = users.compactMap { userRoleChanged(userID: $0.key, powerLevel: $0.value, previous: previous?[$0.key]) }
guard !newRoles.isEmpty else { return nil }
return newRoles.formatted(.list(type: .and))
case .roomThirdPartyInvite(let displayName):
guard let displayName else {
MXLog.error("roomThirdPartyInvite undisplayable due to missing name.")
@ -181,7 +185,7 @@ struct RoomStateEventStringBuilder {
break
case .roomJoinRules: // Doesn't provide information about the change.
break
case .roomPinnedEvents, .roomPowerLevels, .roomServerAcl: // Doesn't provide information about the change.
case .roomPinnedEvents, .roomServerAcl: // Doesn't provide information about the change.
break
case .roomTombstone: // Handle as a virtual timeline item with a link to the upgraded room.
break
@ -194,4 +198,26 @@ struct RoomStateEventStringBuilder {
MXLog.verbose("Filtering timeline item for state: \(state)")
return nil
}
private func userRoleChanged(userID: String, powerLevel: Int64, previous: Int64?) -> String? {
let previous = previous ?? 0 // TODO: Include the previous default in the SDK item.
let role = suggestedRoleForPowerLevel(powerLevel: powerLevel)
let previousRole = suggestedRoleForPowerLevel(powerLevel: previous)
guard role != previousRole else { return nil }
let isPromotion = powerLevel > previous
return switch (role, isPromotion) {
case (.user, false):
L10n.stateEventDemotedToMember(userID)
case (.moderator, true):
L10n.stateEventPromotedToModerator(userID)
case (.moderator, false):
L10n.stateEventDemotedToModerator(userID)
case (.administrator, true):
L10n.stateEventPromotedToAdministrator(userID)
default:
nil
}
}
}

View File

@ -89,4 +89,93 @@ class RoomStateEventStringBuilderTests: XCTestCase {
memberIsYou: sender.id == userID)
XCTAssertEqual(string, expectedString)
}
// MARK: - User Power Levels
let aliceID = "@alice"
let bobID = "@bob"
func testUserPowerLevelsPromotion() {
var string = stringBuilder.buildString(for: .roomPowerLevels(users: [aliceID: suggestedPowerLevelForRole(role: .moderator)],
previous: [aliceID: suggestedPowerLevelForRole(role: .user)]),
sender: TimelineItemSender(id: ""),
isOutgoing: false)
XCTAssertEqual(string, L10n.stateEventPromotedToModerator(aliceID))
string = stringBuilder.buildString(for: .roomPowerLevels(users: [aliceID: suggestedPowerLevelForRole(role: .administrator)],
previous: [aliceID: suggestedPowerLevelForRole(role: .user)]),
sender: TimelineItemSender(id: ""),
isOutgoing: false)
XCTAssertEqual(string, L10n.stateEventPromotedToAdministrator(aliceID))
string = stringBuilder.buildString(for: .roomPowerLevels(users: [aliceID: suggestedPowerLevelForRole(role: .administrator)],
previous: [aliceID: suggestedPowerLevelForRole(role: .moderator)]),
sender: TimelineItemSender(id: ""),
isOutgoing: false)
XCTAssertEqual(string, L10n.stateEventPromotedToAdministrator(aliceID))
}
func testUserPowerLevelsDemotion() {
var string = stringBuilder.buildString(for: .roomPowerLevels(users: [aliceID: suggestedPowerLevelForRole(role: .moderator)],
previous: [aliceID: suggestedPowerLevelForRole(role: .administrator)]),
sender: TimelineItemSender(id: ""),
isOutgoing: false)
XCTAssertEqual(string, L10n.stateEventDemotedToModerator(aliceID))
string = stringBuilder.buildString(for: .roomPowerLevels(users: [aliceID: suggestedPowerLevelForRole(role: .user)],
previous: [aliceID: suggestedPowerLevelForRole(role: .administrator)]),
sender: TimelineItemSender(id: ""),
isOutgoing: false)
XCTAssertEqual(string, L10n.stateEventDemotedToMember(aliceID))
string = stringBuilder.buildString(for: .roomPowerLevels(users: [aliceID: suggestedPowerLevelForRole(role: .user)],
previous: [aliceID: suggestedPowerLevelForRole(role: .moderator)]),
sender: TimelineItemSender(id: ""),
isOutgoing: false)
XCTAssertEqual(string, L10n.stateEventDemotedToMember(aliceID))
}
func testMultipleUserPowerLevels() {
let new = [aliceID: suggestedPowerLevelForRole(role: .administrator),
bobID: suggestedPowerLevelForRole(role: .user)]
let previous = [aliceID: suggestedPowerLevelForRole(role: .user),
bobID: suggestedPowerLevelForRole(role: .moderator)]
let string = stringBuilder.buildString(for: .roomPowerLevels(users: new, previous: previous),
sender: TimelineItemSender(id: ""),
isOutgoing: false)
XCTAssertEqual(string?.contains(L10n.stateEventPromotedToAdministrator(aliceID)), true)
XCTAssertEqual(string?.contains(L10n.stateEventDemotedToMember(bobID)), true)
}
func testInvalidUserPowerLevels() {
// Admin demotions aren't relevant.
var string = stringBuilder.buildString(for: .roomPowerLevels(users: [aliceID: 100],
previous: [aliceID: 200]),
sender: TimelineItemSender(id: ""),
isOutgoing: false)
XCTAssertNil(string)
// User promotions aren't relevant.
string = stringBuilder.buildString(for: .roomPowerLevels(users: [aliceID: 0],
previous: [aliceID: -100]),
sender: TimelineItemSender(id: ""),
isOutgoing: false)
XCTAssertNil(string)
// Or more generally, any change within the same role isn't relevant either.
string = stringBuilder.buildString(for: .roomPowerLevels(users: [aliceID: 75],
previous: [aliceID: 60]),
sender: TimelineItemSender(id: ""),
isOutgoing: false)
XCTAssertNil(string)
let new = [aliceID: 100,
bobID: suggestedPowerLevelForRole(role: .user)]
let previous = [aliceID: 200,
bobID: suggestedPowerLevelForRole(role: .moderator)]
string = stringBuilder.buildString(for: .roomPowerLevels(users: new, previous: previous),
sender: TimelineItemSender(id: ""),
isOutgoing: false)
XCTAssertEqual(string, L10n.stateEventDemotedToMember(bobID))
}
}