Knocking feature polishing part 2 (#3738)

* improved implementation of the unknown state * forget button in the banned state * error handling for forbidden access * added the forget function fixed DMs, and updated preview tests * removed banned room proxy * code polishing and test improvement
2026-11-02 20:39:24 +00:00 · 2025-02-05 14:48:24 +01:00
parent cfaa1b455a
commit 3839025caa
28 changed files with 466 additions and 101 deletions
@@ -18,19 +18,21 @@ enum JoinRoomScreenMode: Equatable {
    case joinable
    case restricted
    case inviteRequired
-    case invited
+    case invited(isDM: Bool)
    case knockable
    case knocked
    case banned(sender: String?, reason: String?)
+    case forbidden
 }

 struct JoinRoomScreenRoomDetails {
    let name: String?
    let topic: String?
    let canonicalAlias: String?
-    let avatar: RoomAvatar
-    let memberCount: Int
+    let avatar: RoomAvatar?
+    let memberCount: Int?
    let inviter: RoomInviterDetails?
+    let isDirect: Bool?
 }

 struct JoinRoomScreenViewState: BindableState {
@@ -39,28 +41,45 @@ struct JoinRoomScreenViewState: BindableState {
    var roomDetails: JoinRoomScreenRoomDetails?
    
    var mode: JoinRoomScreenMode = .loading
-    
+        
    var bindings = JoinRoomScreenViewStateBindings()
    
    var title: String {
-        roomDetails?.name ?? L10n.screenJoinRoomTitleNoPreview
+        if isDMInvite, let inviter = roomDetails?.inviter {
+            return inviter.displayName ?? inviter.id
+        } else {
+            return roomDetails?.name ?? L10n.screenJoinRoomTitleNoPreview
+        }
    }
    
    var subtitle: String? {
        switch mode {
-        case .loading:
-            nil
-        case .unknown:
-            L10n.screenJoinRoomSubtitleNoPreview
-        case .knocked:
-            nil
+        case .invited(isDM: true):
+            if let inviter = roomDetails?.inviter {
+                return inviter.displayName != nil ? inviter.id : nil
+            }
+            return nil
+        case .loading, .unknown, .knocked:
+            return nil
        default:
-            roomDetails?.canonicalAlias
+            return roomDetails?.canonicalAlias
        }
    }
    
-    var avatar: RoomAvatar {
-        roomDetails?.avatar ?? .room(id: roomID, name: title, avatarURL: nil)
+    var avatar: RoomAvatar? {
+        if isDMInvite, let inviter = roomDetails?.inviter {
+            return .room(id: roomID, name: inviter.displayName, avatarURL: inviter.avatarURL)
+        } else if let avatar = roomDetails?.avatar {
+            return avatar
+        } else if let name = roomDetails?.name {
+            return .room(id: roomID, name: name, avatarURL: nil)
+        } else {
+            return nil
+        }
+    }
+    
+    var isDMInvite: Bool {
+        mode == .invited(isDM: true)
    }
 }

@@ -80,4 +99,6 @@ enum JoinRoomScreenViewAction {
    case join
    case acceptInvite
    case declineInvite
+    case forget
+    case dismiss
 }
@@ -57,10 +57,14 @@ class JoinRoomScreenViewModel: JoinRoomScreenViewModelType, JoinRoomScreenViewMo
            Task { await joinRoom() }
        case .acceptInvite:
            Task { await joinRoom() }
+        case .forget:
+            Task { await forgetRoom() }
        case .declineInvite:
            showDeclineInviteConfirmationAlert()
        case .cancelKnock:
            showCancelKnockConfirmationAlert()
+        case .dismiss:
+            actionsSubject.send(.dismiss)
        }
    }
    
@@ -132,12 +136,20 @@ class JoinRoomScreenViewModel: JoinRoomScreenViewModelType, JoinRoomScreenViewMo
        }

        let info = roomPreview?.info ?? roomInfo
+        let avatar: RoomAvatar? = if let avatar = info?.avatar {
+            avatar
+        } else if let displayName = info?.displayName {
+            .room(id: roomID, name: displayName, avatarURL: nil)
+        } else {
+            nil
+        }
        state.roomDetails = JoinRoomScreenRoomDetails(name: info?.displayName,
                                                      topic: info?.topic,
                                                      canonicalAlias: info?.canonicalAlias,
-                                                      avatar: info?.avatar ?? .room(id: roomID, name: info?.displayName ?? "", avatarURL: nil),
-                                                      memberCount: info?.joinedMembersCount ?? 0,
-                                                      inviter: inviter)
+                                                      avatar: avatar,
+                                                      memberCount: info?.joinedMembersCount,
+                                                      inviter: inviter,
+                                                      isDirect: info?.isDirect)
        
        await updateMode()
    }
@@ -153,7 +165,7 @@ class JoinRoomScreenViewModel: JoinRoomScreenViewModelType, JoinRoomScreenViewMo
            
            switch roomPreview.info.membership {
            case .invited:
-                state.mode = .invited
+                state.mode = .invited(isDM: state.roomDetails?.isDirect == true && state.roomDetails?.memberCount == 1)
            case .knocked:
                state.mode = .knocked
            case .banned:
@@ -174,7 +186,7 @@ class JoinRoomScreenViewModel: JoinRoomScreenViewModelType, JoinRoomScreenViewMo
        } else if let room {
            switch room {
            case .invited:
-                state.mode = .invited
+                state.mode = .invited(isDM: state.roomDetails?.isDirect == true && state.roomDetails?.memberCount == 1)
            case .knocked:
                state.mode = .knocked
            case .banned:
@@ -198,16 +210,26 @@ class JoinRoomScreenViewModel: JoinRoomScreenViewModelType, JoinRoomScreenViewMo
            case .success:
                actionsSubject.send(.joined)
            case .failure(let error):
-                MXLog.error("Failed joining room alias: \(alias) with error: \(error)")
-                userIndicatorController.submitIndicator(.init(title: L10n.errorUnknown))
+                if case .forbiddenAccess = error {
+                    MXLog.error("Failed joining room alias: \(alias) forbidden access")
+                    state.mode = .forbidden
+                } else {
+                    MXLog.error("Failed joining room alias: \(alias) with error: \(error)")
+                    userIndicatorController.submitIndicator(.init(title: L10n.errorUnknown))
+                }
            }
        } else {
            switch await clientProxy.joinRoom(roomID, via: via) {
            case .success:
                actionsSubject.send(.joined)
            case .failure(let error):
-                MXLog.error("Failed joining room id: \(roomID) with error: \(error)")
-                userIndicatorController.submitIndicator(.init(title: L10n.errorUnknown))
+                if case .forbiddenAccess = error {
+                    MXLog.error("Failed joining room id: \(roomID) forbidden access")
+                    state.mode = .forbidden
+                } else {
+                    MXLog.error("Failed joining room id: \(roomID) with error: \(error)")
+                    userIndicatorController.submitIndicator(.init(title: L10n.errorUnknown))
+                }
            }
        }
    }
@@ -242,7 +264,7 @@ class JoinRoomScreenViewModel: JoinRoomScreenViewModelType, JoinRoomScreenViewMo
            }
        }
    }
-    
+        
    private func showDeclineInviteConfirmationAlert() {
        guard let roomDetails = state.roomDetails else {
            userIndicatorController.submitIndicator(.init(title: L10n.errorUnknown))
@@ -307,6 +329,27 @@ class JoinRoomScreenViewModel: JoinRoomScreenViewModelType, JoinRoomScreenViewMo
        }
    }
    
+    private func forgetRoom() async {
+        defer {
+            userIndicatorController.retractIndicatorWithId(roomID)
+        }
+        
+        userIndicatorController.submitIndicator(UserIndicator(id: roomID, type: .modal, title: L10n.commonLoading, persistent: true))
+        
+        guard case .banned = room, let roomPreview else {
+            userIndicatorController.submitIndicator(.init(title: L10n.errorUnknown))
+            return
+        }
+        
+        let result = await roomPreview.forgetRoom()
+        
+        if case .failure = result {
+            userIndicatorController.submitIndicator(.init(title: L10n.errorUnknown))
+        } else {
+            actionsSubject.send(.dismiss)
+        }
+    }
+    
    private static let loadingIndicatorIdentifier = "\(JoinRoomScreenViewModel.self)-Loading"
    
    private func showLoadingIndicator() {
@@ -50,10 +50,18 @@ struct JoinRoomScreen: View {
    @ViewBuilder
    private var defaultView: some View {
        VStack(spacing: 16) {
-            RoomAvatarImage(avatar: context.viewState.avatar,
-                            avatarSize: .room(on: .joinRoom),
-                            mediaProvider: context.mediaProvider)
-                .dynamicTypeSize(dynamicTypeSize < .accessibility1 ? dynamicTypeSize : .accessibility1)
+            if let avatar = context.viewState.avatar {
+                RoomAvatarImage(avatar: avatar,
+                                avatarSize: .room(on: .joinRoom),
+                                mediaProvider: context.mediaProvider)
+                    .dynamicTypeSize(dynamicTypeSize < .accessibility1 ? dynamicTypeSize : .accessibility1)
+            } else {
+                RoomAvatarImage(avatar: .room(id: "", name: nil, avatarURL: nil),
+                                avatarSize: .room(on: .joinRoom),
+                                mediaProvider: context.mediaProvider)
+                    .dynamicTypeSize(dynamicTypeSize < .accessibility1 ? dynamicTypeSize : .accessibility1)
+                    .hidden()
+            }
            
            VStack(spacing: 8) {
                Text(context.viewState.title)
@@ -68,7 +76,7 @@ struct JoinRoomScreen: View {
                        .multilineTextAlignment(.center)
                }
                
-                if let memberCount = context.viewState.roomDetails?.memberCount {
+                if !context.viewState.isDMInvite, let memberCount = context.viewState.roomDetails?.memberCount {
                    BadgeLabel(title: "\(memberCount)", icon: \.userProfile, isHighlighted: false)
                }
                
@@ -174,12 +182,31 @@ struct JoinRoomScreen: View {
                VStack(spacing: 16) { inviteButtons }
            }
        case .banned(let sender, let reason):
-            if let sender, let reason {
-                bottomErrorMessage(title: L10n.screenJoinRoomBanByMessage(sender),
-                                   subtitle: L10n.screenJoinRoomBanReason(reason))
-            } else {
-                bottomErrorMessage(title: L10n.screenJoinRoomBanMessage, subtitle: nil)
+            VStack(spacing: 24) {
+                if let sender, let reason {
+                    bottomErrorMessage(title: L10n.screenJoinRoomBanByMessage(sender),
+                                       subtitle: L10n.screenJoinRoomBanReason(reason))
+                } else {
+                    bottomErrorMessage(title: L10n.screenJoinRoomBanMessage, subtitle: nil)
+                }
+                
+                Button(L10n.screenJoinRoomForgetAction) {
+                    context.send(viewAction: .forget)
+                }
+                .buttonStyle(.compound(.primary))
            }
+        case .forbidden:
+            forbiddenView
+        }
+    }
+    
+    private var forbiddenView: some View {
+        VStack(spacing: 24) {
+            bottomErrorMessage(title: L10n.screenJoinRoomFailMessage, subtitle: L10n.screenJoinRoomFailReason)
+            Button(L10n.actionOk) {
+                context.send(viewAction: .dismiss)
+            }
+            .buttonStyle(.compound(.primary))
        }
    }
    
@@ -235,9 +262,11 @@ struct JoinRoomScreen: View {
    private var toolbar: some ToolbarContent {
        if context.viewState.mode == .knocked {
            ToolbarItem(placement: .principal) {
-                RoomHeaderView(roomName: context.viewState.title,
-                               roomAvatar: context.viewState.avatar,
-                               mediaProvider: context.mediaProvider)
+                if let avatar = context.viewState.avatar {
+                    RoomHeaderView(roomName: context.viewState.title,
+                                   roomAvatar: avatar,
+                                   mediaProvider: context.mediaProvider)
+                }
            }
        }
    }
@@ -250,30 +279,48 @@ struct JoinRoomScreen_Previews: PreviewProvider, TestablePreview {
    static let joinableViewModel = makeViewModel(mode: .joinable)
    static let restrictedViewModel = makeViewModel(mode: .restricted)
    static let inviteRequiredViewModel = makeViewModel(mode: .inviteRequired)
-    static let invitedViewModel = makeViewModel(mode: .invited)
+    static let invitedViewModel = makeViewModel(mode: .invited(isDM: false))
+    static let invitedDMViewModel = makeViewModel(mode: .invited(isDM: true))
    static let knockableViewModel = makeViewModel(mode: .knockable)
    static let knockedViewModel = makeViewModel(mode: .knocked)
    static let bannedViewModel = makeViewModel(mode: .banned(sender: "Bob", reason: "Spamming"))
+    static let forbiddenViewModel = makeViewModel(mode: .forbidden)
    
    static var previews: some View {
-        makePreview(viewModel: unknownViewModel, previewDisplayName: "Unknown")
-        makePreview(viewModel: joinableViewModel, previewDisplayName: "Joinable")
-        makePreview(viewModel: restrictedViewModel, previewDisplayName: "Restricted")
-        makePreview(viewModel: inviteRequiredViewModel, previewDisplayName: "InviteRequired")
-        makePreview(viewModel: invitedViewModel, previewDisplayName: "Invited")
-        makePreview(viewModel: knockableViewModel, previewDisplayName: "Knockable")
-        makePreview(viewModel: knockedViewModel, previewDisplayName: "Knocked")
-        makePreview(viewModel: bannedViewModel, previewDisplayName: "Banned")
+        makePreview(viewModel: unknownViewModel, mode: .unknown)
+        makePreview(viewModel: joinableViewModel, mode: .joinable)
+        makePreview(viewModel: restrictedViewModel, mode: .restricted)
+        makePreview(viewModel: inviteRequiredViewModel, mode: .inviteRequired)
+        makePreview(viewModel: invitedViewModel, mode: .invited(isDM: false))
+        makePreview(viewModel: invitedDMViewModel, mode: .invited(isDM: true))
+        makePreview(viewModel: knockableViewModel, mode: .knockable)
+        makePreview(viewModel: knockedViewModel, mode: .knocked)
+        makePreview(viewModel: bannedViewModel, mode: .banned(sender: nil, reason: nil))
+        makePreview(viewModel: forbiddenViewModel, mode: .forbidden)
    }
    
-    static func makePreview(viewModel: JoinRoomScreenViewModel, previewDisplayName: String) -> some View {
-        NavigationStack {
-            JoinRoomScreen(context: viewModel.context)
+    @ViewBuilder
+    static func makePreview(viewModel: JoinRoomScreenViewModel, mode: JoinRoomScreenMode) -> some View {
+        if mode == .forbidden {
+            NavigationStack {
+                JoinRoomScreen(context: viewModel.context)
+            }
+            .snapshotPreferences(expect: viewModel.context.$viewState.map { state in
+                state.mode == .forbidden
+            })
+            .onAppear {
+                forbiddenViewModel.context.send(viewAction: .join)
+            }
+            .previewDisplayName(mode.previewDisplayName)
+        } else {
+            NavigationStack {
+                JoinRoomScreen(context: viewModel.context)
+            }
+            .snapshotPreferences(expect: viewModel.context.$viewState.map { state in
+                state.roomDetails != nil
+            })
+            .previewDisplayName(mode.previewDisplayName)
        }
-        .snapshotPreferences(expect: viewModel.context.$viewState.map { state in
-            state.roomDetails != nil
-        })
-        .previewDisplayName(previewDisplayName)
    }
    
    static func makeViewModel(mode: JoinRoomScreenMode) -> JoinRoomScreenViewModel {
@@ -294,10 +341,17 @@ struct JoinRoomScreen_Previews: PreviewProvider, TestablePreview {
        case .inviteRequired:
            clientProxy.roomPreviewForIdentifierViaReturnValue = .success(RoomPreviewProxyMock.inviteRequired)
            clientProxy.roomForIdentifierReturnValue = nil
-        case .invited:
-            clientProxy.roomPreviewForIdentifierViaReturnValue = .success(RoomPreviewProxyMock.invited())
-            clientProxy.roomForIdentifierClosure = { _ in
-                .invited(InvitedRoomProxyMock(.init(avatarURL: .mockMXCAvatar)))
+        case .invited(let isDM):
+            if isDM {
+                clientProxy.roomPreviewForIdentifierViaReturnValue = .success(RoomPreviewProxyMock.inviteDM())
+                clientProxy.roomForIdentifierClosure = { _ in
+                    .invited(InvitedRoomProxyMock(.init(avatarURL: .mockMXCAvatar)))
+                }
+            } else {
+                clientProxy.roomPreviewForIdentifierViaReturnValue = .success(RoomPreviewProxyMock.invited())
+                clientProxy.roomForIdentifierClosure = { _ in
+                    .invited(InvitedRoomProxyMock(.init(avatarURL: .mockMXCAvatar)))
+                }
            }
        case .knockable:
            clientProxy.roomPreviewForIdentifierViaReturnValue = .success(RoomPreviewProxyMock.knockable)
@@ -312,6 +366,12 @@ struct JoinRoomScreen_Previews: PreviewProvider, TestablePreview {
            clientProxy.roomForIdentifierClosure = { _ in
                .banned
            }
+        case .forbidden:
+            clientProxy.roomPreviewForIdentifierViaReturnValue = .success(RoomPreviewProxyMock.restricted)
+            clientProxy.roomForIdentifierReturnValue = nil
+            clientProxy.joinRoomAliasClosure = { _ in
+                .failure(.forbiddenAccess)
+            }
        default:
            break
        }
@@ -324,3 +384,30 @@ struct JoinRoomScreen_Previews: PreviewProvider, TestablePreview {
                                       userIndicatorController: ServiceLocator.shared.userIndicatorController)
    }
 }
+
+private extension JoinRoomScreenMode {
+    var previewDisplayName: String {
+        switch self {
+        case .unknown:
+            return "Unknown"
+        case .loading:
+            return "Loading"
+        case .joinable:
+            return "Joinable"
+        case .restricted:
+            return "Restricted"
+        case .inviteRequired:
+            return "InviteRequired"
+        case .invited(isDM: let isDM):
+            return isDM ? "InvitedDM" : "Invited"
+        case .knockable:
+            return "Knockable"
+        case .knocked:
+            return "Knocked"
+        case .banned:
+            return "Banned"
+        case .forbidden:
+            return "Forbidden"
+        }
+    }
+}