Add alias to public room creation (#3450)

* added the address section * updated code and strings * syncing name and address * improved code * added a way to reset the state * better documentation * update strings * handling the alias * alias error state * update strings * error handling * improved the error handling * new preview tests, even if they do not work well * improved tests * unit tests * pr comments and using the correct value * fix * pr comments * to improve safety and control of the FF * fixed a test * updated tests * update SDK
2026-11-05 03:48:27 +00:00 · 2024-11-08 15:49:20 +01:00
parent c567901ec5
commit e7d4ea0d35
39 changed files with 592 additions and 115 deletions
@@ -25,20 +25,32 @@ enum CreateRoomViewModelAction {
 }

 struct CreateRoomViewState: BindableState {
+    var roomName: String
+    let serverName: String
    let isKnockingFeatureEnabled: Bool
    var selectedUsers: [UserProfileProxy]
+    var aliasLocalPart: String
    var bindings: CreateRoomViewStateBindings
    var avatarURL: URL?
    var canCreateRoom: Bool {
-        !bindings.roomName.isEmpty
+        !roomName.isEmpty && aliasErrors.isEmpty
+    }
+
+    var aliasErrors: Set<CreateRoomAliasErrorState> = []
+    var aliasErrorDescription: String? {
+        if aliasErrors.contains(.alreadyExists) {
+            return L10n.screenCreateRoomRoomAddressNotAvailableErrorDescription
+        } else if aliasErrors.contains(.invalidSymbols) {
+            return L10n.screenCreateRoomRoomAddressInvalidSymbolsErrorDescription
+        }
+        return nil
    }
 }

 struct CreateRoomViewStateBindings {
-    var roomName: String
    var roomTopic: String
    var isRoomPrivate: Bool
-    var isKnockingOnly = false
+    var isKnockingOnly: Bool
    var showAttachmentConfirmationDialog = false
    
    /// Information describing the currently displayed alert.
@@ -51,4 +63,11 @@ enum CreateRoomViewAction {
    case displayCameraPicker
    case displayMediaPicker
    case removeImage
+    case updateRoomName(String)
+    case updateAliasLocalPart(String)
+}
+
+enum CreateRoomAliasErrorState {
+    case alreadyExists
+    case invalidSymbols
 }
@@ -6,6 +6,7 @@
 //

 import Combine
+import MatrixRustSDK
 import SwiftUI

 typealias CreateRoomViewModelType = StateStoreViewModel<CreateRoomViewState, CreateRoomViewAction>
@@ -15,6 +16,8 @@ class CreateRoomViewModel: CreateRoomViewModelType, CreateRoomViewModelProtocol
    private var createRoomParameters: CreateRoomFlowParameters
    private let analytics: AnalyticsService
    private let userIndicatorController: UserIndicatorControllerProtocol
+    private var syncNameAndAlias = true
+    @CancellableTask private var checkAliasAvailabilityTask: Task<Void, Never>?
    
    private var actionsSubject: PassthroughSubject<CreateRoomViewModelAction, Never> = .init()
    
@@ -35,9 +38,17 @@ class CreateRoomViewModel: CreateRoomViewModelType, CreateRoomViewModelProtocol
        self.analytics = analytics
        self.userIndicatorController = userIndicatorController
        
-        let bindings = CreateRoomViewStateBindings(roomName: parameters.name, roomTopic: parameters.topic, isRoomPrivate: parameters.isRoomPrivate)
+        let bindings = CreateRoomViewStateBindings(roomTopic: parameters.topic,
+                                                   isRoomPrivate: parameters.isRoomPrivate,
+                                                   isKnockingOnly: appSettings.knockingEnabled ? parameters.isKnockingOnly : false)

-        super.init(initialViewState: CreateRoomViewState(isKnockingFeatureEnabled: appSettings.knockingEnabled, selectedUsers: selectedUsers.value, bindings: bindings), mediaProvider: userSession.mediaProvider)
+        super.init(initialViewState: CreateRoomViewState(roomName: parameters.name,
+                                                         serverName: userSession.clientProxy.userIDServerName ?? "",
+                                                         isKnockingFeatureEnabled: appSettings.knockingEnabled,
+                                                         selectedUsers: selectedUsers.value,
+                                                         aliasLocalPart: parameters.aliasLocalPart ?? roomAliasNameFromRoomDisplayName(roomName: parameters.name),
+                                                         bindings: bindings),
+                   mediaProvider: userSession.mediaProvider)
        
        createRoomParameters
            .map(\.avatarImageMedia)
@@ -80,31 +91,111 @@ class CreateRoomViewModel: CreateRoomViewModelType, CreateRoomViewModelProtocol
            actionsSubject.send(.displayMediaPicker)
        case .removeImage:
            actionsSubject.send(.removeImage)
+        case .updateAliasLocalPart(let aliasLocalPart):
+            state.aliasLocalPart = aliasLocalPart
+            // If this has been called this means that the user wants a custom address not necessarily reflecting the name
+            // So we disable the two from syncing.
+            syncNameAndAlias = false
+        case .updateRoomName(let name):
+            // Reset the syncing if the name is fully cancelled
+            if name.isEmpty {
+                syncNameAndAlias = true
+            }
+            state.roomName = name
+            if syncNameAndAlias {
+                state.aliasLocalPart = roomAliasNameFromRoomDisplayName(roomName: name)
+            }
        }
    }
    
    // MARK: - Private

    private func setupBindings() {
+        // Reset the state related to public rooms if the user choses the room to be empty
+        context.$viewState
+            .dropFirst()
+            .map(\.bindings.isRoomPrivate)
+            .removeDuplicates()
+            .filter { $0 }
+            .receive(on: DispatchQueue.main)
+            .sink { [weak self] _ in
+                guard let self else { return }
+                state.bindings.isKnockingOnly = false
+                state.aliasErrors = []
+                state.aliasLocalPart = roomAliasNameFromRoomDisplayName(roomName: state.roomName)
+                syncNameAndAlias = true
+            }
+            .store(in: &cancellables)
+        
        context.$viewState
-            .map(\.bindings)
            .throttle(for: 0.5, scheduler: DispatchQueue.main, latest: true)
            .removeDuplicates { old, new in
-                old.roomName == new.roomName && old.roomTopic == new.roomTopic && old.isRoomPrivate == new.isRoomPrivate
+                old.roomName == new.roomName &&
+                    old.bindings.roomTopic == new.bindings.roomTopic &&
+                    old.bindings.isRoomPrivate == new.bindings.isRoomPrivate &&
+                    old.bindings.isKnockingOnly == new.bindings.isKnockingOnly &&
+                    old.aliasLocalPart == new.aliasLocalPart
            }
-            .sink { [weak self] bindings in
+            .sink { [weak self] state in
                guard let self else { return }
-                updateParameters(bindings: bindings)
+                updateParameters(state: state)
                actionsSubject.send(.updateDetails(createRoomParameters))
            }
            .store(in: &cancellables)
+        
+        context.$viewState
+            .map(\.aliasLocalPart)
+            .removeDuplicates()
+            .debounce(for: 1, scheduler: DispatchQueue.main)
+            .sink { [weak self] aliasLocalPart in
+                guard let self else {
+                    return
+                }
+                
+                guard state.isKnockingFeatureEnabled,
+                      !state.bindings.isRoomPrivate,
+                      let canonicalAlias = canonicalAlias(aliasLocalPart: aliasLocalPart) else {
+                    // While is empty or private room we don't change or display the error
+                    return
+                }
+                
+                if !isRoomAliasFormatValid(alias: canonicalAlias) {
+                    state.aliasErrors.insert(.invalidSymbols)
+                    // If the alias is invalid we don't need to check for availability
+                    state.aliasErrors.remove(.alreadyExists)
+                    checkAliasAvailabilityTask = nil
+                    return
+                }
+                
+                state.aliasErrors.remove(.invalidSymbols)
+                
+                checkAliasAvailabilityTask = Task { [weak self] in
+                    guard let self else {
+                        return
+                    }
+                    
+                    if case .success(false) = await self.userSession.clientProxy.isAliasAvailable(canonicalAlias) {
+                        guard !Task.isCancelled else { return }
+                        state.aliasErrors.insert(.alreadyExists)
+                    } else {
+                        guard !Task.isCancelled else { return }
+                        state.aliasErrors.remove(.alreadyExists)
+                    }
+                }
+            }
+            .store(in: &cancellables)
    }
    
-    private func updateParameters(bindings: CreateRoomViewStateBindings) {
-        createRoomParameters.name = bindings.roomName
-        createRoomParameters.topic = bindings.roomTopic
-        createRoomParameters.isRoomPrivate = bindings.isRoomPrivate
-        createRoomParameters.isKnockingOnly = bindings.isKnockingOnly
+    private func updateParameters(state: CreateRoomViewState) {
+        createRoomParameters.name = state.roomName
+        createRoomParameters.topic = state.bindings.roomTopic
+        createRoomParameters.isRoomPrivate = state.bindings.isRoomPrivate
+        createRoomParameters.isKnockingOnly = state.bindings.isKnockingOnly
+        if state.isKnockingFeatureEnabled, !state.aliasLocalPart.isEmpty {
+            createRoomParameters.aliasLocalPart = state.aliasLocalPart
+        } else {
+            createRoomParameters.aliasLocalPart = nil
+        }
    }
    
    private func createRoom() async {
@@ -114,7 +205,28 @@ class CreateRoomViewModel: CreateRoomViewModelType, CreateRoomViewModelProtocol
        showLoadingIndicator()
        
        // Since the parameters are throttled, we need to make sure that the latest values are used
-        updateParameters(bindings: state.bindings)
+        updateParameters(state: state)
+        
+        // Better to double check the errors also when trying to create the room
+        if state.isKnockingFeatureEnabled, !createRoomParameters.isRoomPrivate {
+            guard let canonicalAlias = canonicalAlias(aliasLocalPart: createRoomParameters.aliasLocalPart),
+                  isRoomAliasFormatValid(alias: canonicalAlias) else {
+                state.aliasErrors = [.invalidSymbols]
+                return
+            }
+            
+            switch await userSession.clientProxy.isAliasAvailable(canonicalAlias) {
+            case .success(true):
+                break
+            case .success(false):
+                state.aliasErrors = [.alreadyExists]
+                return
+            case .failure:
+                state.bindings.alertInfo = AlertInfo(id: .unknown)
+                return
+            }
+        }
+        
        let avatarURL: URL?
        if let media = createRoomParameters.avatarImageMedia {
            switch await userSession.clientProxy.uploadMedia(media) {
@@ -147,7 +259,8 @@ class CreateRoomViewModel: CreateRoomViewModelType, CreateRoomViewModelProtocol
                                                        // As of right now we don't want to make private rooms with the knock rule
                                                        isKnockingOnly: createRoomParameters.isRoomPrivate ? false : createRoomParameters.isKnockingOnly,
                                                        userIDs: state.selectedUsers.map(\.userID),
-                                                        avatarURL: avatarURL) {
+                                                        avatarURL: avatarURL,
+                                                        aliasLocalPart: createRoomParameters.isRoomPrivate ? nil : createRoomParameters.aliasLocalPart) {
        case .success(let roomId):
            analytics.trackCreatedRoom(isDM: false)
            actionsSubject.send(.openRoom(withIdentifier: roomId))
@@ -158,6 +271,14 @@ class CreateRoomViewModel: CreateRoomViewModelType, CreateRoomViewModelProtocol
        }
    }
    
+    func canonicalAlias(aliasLocalPart: String?) -> String? {
+        guard let aliasLocalPart,
+              !aliasLocalPart.isEmpty else {
+            return nil
+        }
+        return "#\(aliasLocalPart):\(state.serverName)"
+    }
+    
    // MARK: Loading indicator
    
    private static let loadingIndicatorIdentifier = "\(CreateRoomViewModel.self)-Loading"
@@ -16,7 +16,23 @@ struct CreateRoomScreen: View {
        case name
        case topic
    }
-
+    
+    private var aliasBinding: Binding<String> {
+        .init(get: {
+            context.viewState.aliasLocalPart
+        }, set: {
+            context.send(viewAction: .updateAliasLocalPart($0))
+        })
+    }
+    
+    private var roomNameBinding: Binding<String> {
+        .init(get: {
+            context.viewState.roomName
+        }, set: {
+            context.send(viewAction: .updateRoomName($0))
+        })
+    }
+    
    var body: some View {
        Form {
            roomSection
@@ -25,6 +41,7 @@ struct CreateRoomScreen: View {
            if context.viewState.isKnockingFeatureEnabled,
               !context.isRoomPrivate {
                roomAccessSection
+                roomAliasSection
            }
        }
        .compoundList()
@@ -48,7 +65,7 @@ struct CreateRoomScreen: View {
                        .compoundListSectionHeader()
                    
                    TextField(L10n.screenCreateRoomRoomNameLabel,
-                              text: $context.roomName,
+                              text: roomNameBinding,
                              prompt: Text(L10n.commonRoomNamePlaceholder).foregroundColor(.compound.textPlaceholder),
                              axis: .horizontal)
                        .focused($focus, equals: .name)
@@ -150,25 +167,65 @@ struct CreateRoomScreen: View {
                                    iconAlignment: .top),
                    kind: .selection(isSelected: !context.isRoomPrivate) { context.isRoomPrivate = false })
        } header: {
-            Text(L10n.commonSecurity.uppercased())
+            Text(L10n.screenCreateRoomRoomVisibilitySectionTitle)
                .compoundListSectionHeader()
        }
    }
    
    private var roomAccessSection: some View {
        Section {
-            ListRow(label: .plain(title: L10n.screenCreateRoomAccessSectionAnyoneOptionTitle,
-                                  description: L10n.screenCreateRoomAccessSectionAnyoneOptionDescription),
+            ListRow(label: .plain(title: L10n.screenCreateRoomRoomAccessSectionAnyoneOptionTitle,
+                                  description: L10n.screenCreateRoomRoomAccessSectionAnyoneOptionDescription),
                    kind: .selection(isSelected: !context.isKnockingOnly) { context.isKnockingOnly = false })
-            ListRow(label: .plain(title: L10n.screenCreateRoomAccessSectionKnockingOptionTitle,
-                                  description: L10n.screenCreateRoomAccessSectionKnockingOptionDescription),
+            ListRow(label: .plain(title: L10n.screenCreateRoomRoomAccessSectionKnockingOptionTitle,
+                                  description: L10n.screenCreateRoomRoomAccessSectionKnockingOptionDescription),
                    kind: .selection(isSelected: context.isKnockingOnly) { context.isKnockingOnly = true })
        } header: {
-            Text(L10n.screenCreateRoomAccessSectionHeader.uppercased())
+            Text(L10n.screenCreateRoomRoomAccessSectionHeader)
                .compoundListSectionHeader()
        }
    }
    
+    private var roomAliasSection: some View {
+        Section {
+            ListRow(kind: .custom {
+                HStack(spacing: 0) {
+                    Text("#")
+                        .font(.compound.bodyLG)
+                        .foregroundStyle(.compound.textSecondary)
+                    
+                    TextField("", text: aliasBinding)
+                        .autocapitalization(.none)
+                        .textCase(.lowercase)
+                        .tint(.compound.iconAccentTertiary)
+                        .font(.compound.bodyLG)
+                        .foregroundStyle(.compound.textPrimary)
+                        .padding(.horizontal, 8)
+                    Text(":\(context.viewState.serverName)")
+                        .font(.compound.bodyLG)
+                        .foregroundStyle(.compound.textSecondary)
+                }
+                .padding(ListRowPadding.textFieldInsets)
+                .environment(\.layoutDirection, .leftToRight)
+                .errorBackground(!context.viewState.aliasErrors.isEmpty)
+            })
+        } header: {
+            Text(L10n.screenCreateRoomRoomAddressSectionTitle)
+                .compoundListSectionHeader()
+        } footer: {
+            VStack(alignment: .leading, spacing: 12) {
+                if let errorDescription = context.viewState.aliasErrorDescription {
+                    Label(errorDescription, icon: \.error, iconSize: .xSmall, relativeTo: .compound.bodySM)
+                        .foregroundStyle(.compound.textCriticalPrimary)
+                        .font(.compound.bodySM)
+                }
+                Text(L10n.screenCreateRoomRoomAddressSectionFooter)
+                    .compoundListSectionFooter()
+                    .font(.compound.bodySM)
+            }
+        }
+    }
+    
    private var toolbar: some ToolbarContent {
        ToolbarItem(placement: .confirmationAction) {
            Button(L10n.actionCreate) {
@@ -180,6 +237,15 @@ struct CreateRoomScreen: View {
    }
 }

+private extension View {
+    func errorBackground(_ shouldDisplay: Bool) -> some View {
+        listRowBackground(shouldDisplay ? AnyView(RoundedRectangle(cornerRadius: 10)
+                .inset(by: 1)
+                .fill(.compound.bgCriticalSubtleHovered)
+                .stroke(Color.compound.borderCriticalPrimary)) : AnyView(Color.compound.bgCanvasDefaultLevel1))
+    }
+}
+
 // MARK: - Previews

 struct CreateRoom_Previews: PreviewProvider, TestablePreview {
@@ -208,7 +274,7 @@ struct CreateRoom_Previews: PreviewProvider, TestablePreview {
    }()
    
    static let publicRoomViewModel = {
-        let userSession = UserSessionMock(.init(clientProxy: ClientProxyMock(.init(userID: "@userid:example.com"))))
+        let userSession = UserSessionMock(.init(clientProxy: ClientProxyMock(.init(userIDServerName: "example.org", userID: "@userid:example.com"))))
        let parameters = CreateRoomFlowParameters(isRoomPrivate: false)
        let selectedUsers: [UserProfileProxy] = [.mockAlice, .mockBob, .mockCharlie]
        ServiceLocator.shared.settings.knockingEnabled = true
@@ -220,6 +286,32 @@ struct CreateRoom_Previews: PreviewProvider, TestablePreview {
                                   appSettings: ServiceLocator.shared.settings)
    }()
    
+    static let publicRoomInvalidAliasViewModel = {
+        let userSession = UserSessionMock(.init(clientProxy: ClientProxyMock(.init(userIDServerName: "example.org", userID: "@userid:example.com"))))
+        let parameters = CreateRoomFlowParameters(isRoomPrivate: false, aliasLocalPart: "#:")
+        ServiceLocator.shared.settings.knockingEnabled = true
+        return CreateRoomViewModel(userSession: userSession,
+                                   createRoomParameters: .init(parameters),
+                                   selectedUsers: .init([]),
+                                   analytics: ServiceLocator.shared.analytics,
+                                   userIndicatorController: UserIndicatorControllerMock(),
+                                   appSettings: ServiceLocator.shared.settings)
+    }()
+    
+    static let publicRoomExistingAliasViewModel = {
+        let clientProxy = ClientProxyMock(.init(userIDServerName: "example.org", userID: "@userid:example.com"))
+        clientProxy.isAliasAvailableReturnValue = .success(false)
+        let userSession = UserSessionMock(.init(clientProxy: clientProxy))
+        let parameters = CreateRoomFlowParameters(isRoomPrivate: false, aliasLocalPart: "existing")
+        ServiceLocator.shared.settings.knockingEnabled = true
+        return CreateRoomViewModel(userSession: userSession,
+                                   createRoomParameters: .init(parameters),
+                                   selectedUsers: .init([]),
+                                   analytics: ServiceLocator.shared.analytics,
+                                   userIndicatorController: UserIndicatorControllerMock(),
+                                   appSettings: ServiceLocator.shared.settings)
+    }()
+
    static var previews: some View {
        NavigationStack {
            CreateRoomScreen(context: viewModel.context)
@@ -233,5 +325,15 @@ struct CreateRoom_Previews: PreviewProvider, TestablePreview {
            CreateRoomScreen(context: publicRoomViewModel.context)
        }
        .previewDisplayName("Create Public Room")
+        NavigationStack {
+            CreateRoomScreen(context: publicRoomInvalidAliasViewModel.context)
+        }
+        .snapshotPreferences(delay: 1.5)
+        .previewDisplayName("Create Public Room, invalid alias")
+        NavigationStack {
+            CreateRoomScreen(context: publicRoomExistingAliasViewModel.context)
+        }
+        .snapshotPreferences(delay: 1.5)
+        .previewDisplayName("Create Public Room, existing alias")
    }
 }
@@ -239,13 +239,13 @@ struct SecureBackupRecoveryKeyScreen_Previews: PreviewProvider, TestablePreview
            SecureBackupRecoveryKeyScreen(context: generatingViewModel.context)
        }
        .previewDisplayName("Generating")
-        .snapshot(delay: 0.25)
+        .snapshotPreferences(delay: 0.25)
        
        NavigationStack {
            SecureBackupRecoveryKeyScreen(context: setupViewModel.context)
        }
        .previewDisplayName("Set up")
-        .snapshot(delay: 0.25)
+        .snapshotPreferences(delay: 0.25)

        NavigationStack {
            SecureBackupRecoveryKeyScreen(context: incompleteViewModel.context)