Improve how alias settings are handled, add unit tests (#3686)

ElementX/Sources/Mocks/JoinedRoomProxyMock.swift

@@ -25,6 +25,7 @@ struct JoinedRoomProxyMockConfiguration {
     var isEncrypted = true
     var hasOngoingCall = true
     var canonicalAlias: String?
+    var alternativeAliases: [String] = []
     var pinnedEventIDs: Set<String> = []
     
     var timelineStartReached = false
@@ -146,7 +147,7 @@ extension RoomInfo {
                   isTombstoned: false,
                   isFavourite: false,
                   canonicalAlias: configuration.canonicalAlias,
-                  alternativeAliases: [],
+                  alternativeAliases: configuration.alternativeAliases,
                   membership: .joined,
                   inviter: configuration.inviter.map { RoomMember(userId: $0.userID,
                                                                   displayName: $0.displayName,

ElementX/Sources/Screens/EditRoomAddressScreen/EditRoomAddressScreenModels.swift

@@ -22,11 +22,11 @@ struct EditRoomAddressScreenViewState: BindableState {
             !bindings.desiredAliasLocalPart.isEmpty
     }
     
-    var bindings: EditRoomAddressScreenViewStateBindings
+    var bindings = EditRoomAddressScreenViewStateBindings()
 }
 
 struct EditRoomAddressScreenViewStateBindings {
-    var desiredAliasLocalPart: String
+    var desiredAliasLocalPart = ""
 }
 
 enum EditRoomAddressScreenViewAction {

ElementX/Sources/Screens/EditRoomAddressScreen/EditRoomAddressScreenViewModel.swift

@@ -31,23 +31,33 @@ class EditRoomAddressScreenViewModel: EditRoomAddressScreenViewModelType, EditRo
         self.clientProxy = clientProxy
         self.userIndicatorController = userIndicatorController
         
-        var aliasLocalPart = ""
-        if let canonicalAlias = roomProxy.infoPublisher.value.canonicalAlias {
-            aliasLocalPart = canonicalAlias.dropFirst().split(separator: ":").first.flatMap(String.init) ?? ""
-        } else if let displayName = roomProxy.infoPublisher.value.displayName {
-            aliasLocalPart = roomAliasNameFromRoomDisplayName(roomName: displayName)
-        }
-        
         if let initialViewState {
             super.init(initialViewState: initialViewState)
         } else {
-            super.init(initialViewState: EditRoomAddressScreenViewState(serverName: clientProxy.userIDServerName ?? "",
-                                                                        currentAliasLocalPart: aliasLocalPart,
-                                                                        bindings: .init(desiredAliasLocalPart: aliasLocalPart)))
+            super.init(initialViewState: EditRoomAddressScreenViewState(serverName: clientProxy.userIDServerName ?? ""))
+            
+            state.currentAliasLocalPart = localPartForMatchingAlias(computeFromDisplayName: false)
+            state.bindings.desiredAliasLocalPart = localPartForMatchingAlias(computeFromDisplayName: true) ?? ""
         }
+        
         setupSubscriptions()
     }
     
+    /// Give priority to aliases from the current user's homeserver as remote ones
+    /// cannot be edited. If none match then don't fallback and show an empty alias
+    /// instead so that the user can add one sepecific to this homeserver.
+    private func localPartForMatchingAlias(computeFromDisplayName: Bool) -> String? {
+        if let matchingAlias = roomProxy.infoPublisher.value.firstAliasMatching(serverName: clientProxy.userIDServerName, useFallback: false) {
+            return matchingAlias.aliasLocalPart
+        }
+        
+        guard computeFromDisplayName, let displayName = roomProxy.infoPublisher.value.displayName else {
+            return nil
+        }
+        
+        return roomAliasNameFromRoomDisplayName(roomName: displayName)
+    }
+    
     // MARK: - Public
     
     override func process(viewAction: EditRoomAddressScreenViewAction) {
@@ -136,7 +146,7 @@ class EditRoomAddressScreenViewModel: EditRoomAddressScreenViewModelType, EditRo
             return
         }
         
-        let oldAlias = roomProxy.infoPublisher.value.canonicalAlias
+        let oldAlias = roomProxy.infoPublisher.value.firstAliasMatching(serverName: clientProxy.userIDServerName, useFallback: false)
         
         // First publish the new alias
         if case .failure = await roomProxy.publishRoomAliasInRoomDirectory(canonicalAlias) {
@@ -172,3 +182,9 @@ class EditRoomAddressScreenViewModel: EditRoomAddressScreenViewModelType, EditRo
         userIndicatorController.retractIndicatorWithId(Self.loadingIndicatorIdentifier)
     }
 }
+
+private extension String {
+    var aliasLocalPart: String {
+        dropFirst().split(separator: ":").first.flatMap(String.init) ?? ""
+    }
+}

ElementX/Sources/Screens/SecurityAndPrivacyScreen/SecurityAndPrivacyScreenViewModel.swift

@@ -79,8 +79,18 @@ class SecurityAndPrivacyScreenViewModel: SecurityAndPrivacyScreenViewModelType,
             }
             .store(in: &cancellables)
         
+        let userIDServerName = clientProxy.userIDServerName
+        
         roomProxy.infoPublisher
-            .map(\.canonicalAlias)
+            .compactMap { roomInfo in
+                guard let userIDServerName else {
+                    return nil
+                }
+                
+                // Give priority to aliases from the current user's homeserver as remote ones
+                // cannot be edited.
+                return roomInfo.firstAliasMatching(serverName: userIDServerName, useFallback: true)
+            }
             .removeDuplicates()
             .receive(on: DispatchQueue.main)
             .weakAssign(to: \.state.canonicalAlias, on: self)

ElementX/Sources/Services/Room/RoomInfoProxy.swift

@@ -67,6 +67,37 @@ struct RoomInfoProxy: BaseRoomInfoProxyProtocol {
     var pinnedEventIDs: Set<String> { Set(roomInfo.pinnedEventIds) }
     var joinRule: JoinRule? { roomInfo.joinRule }
     var historyVisibility: RoomHistoryVisibility { roomInfo.historyVisibility }
+    
+    /// Find the first alias that matches the given homeserver
+    /// - Parameters:
+    ///   - serverName: the homserver in question
+    ///   - useFallback: whether to return any alias if none match
+    func firstAliasMatching(serverName: String?, useFallback: Bool) -> String? {
+        guard let serverName else { return nil }
+        
+        // Check if the canonical alias matches the homeserver
+        if let canonicalAlias = roomInfo.canonicalAlias,
+           canonicalAlias.range(of: serverName) != nil {
+            return canonicalAlias
+        }
+        
+        // Otherwise check the alternative aliases and return the first one that matches
+        if let matchingAlternativeAlias = roomInfo.alternativeAliases.filter({ $0.range(of: serverName) != nil }).first {
+            return matchingAlternativeAlias
+        }
+        
+        guard useFallback else {
+            return nil
+        }
+        
+        // Or just return the canonical alias if any
+        if let canonicalAlias = roomInfo.canonicalAlias {
+            return canonicalAlias
+        }
+        
+        // And finally return whatever the first alternative alias is
+        return roomInfo.alternativeAliases.first
+    }
 }
 
 struct RoomPreviewInfoProxy: BaseRoomInfoProxyProtocol {

PreviewTests/Sources/__Snapshots__/PreviewTests/test_editRoomAddressScreen-iPad-en-GB.No-alias.png

@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:dd9afbafee47a33dfa211d290b3737873fb55e8706ec7b6cf3c74f1820da86ce
-size 103048
+oid sha256:b37dd8a18de99e48b75dee74174e64a096db0362d66abf7162906f5e9138e174
+size 103033

PreviewTests/Sources/__Snapshots__/PreviewTests/test_editRoomAddressScreen-iPad-pseudo.No-alias.png

@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:ef1930d51d5f0634aa06250abe7740029b3d8fc2f3ba319653e54aaf3bacccfb
-size 111971
+oid sha256:6fb2d86369c003b01ba5647c2bcbd2fdd19dcc652bdbdd4e824f8cc706da8e32
+size 112083

PreviewTests/Sources/__Snapshots__/PreviewTests/test_editRoomAddressScreen-iPhone-16-en-GB.No-alias.png

@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:b8bb258d0a65f5ab6bd36ce68909417e220b7336bac504ed9ba1c9ca77e3dc8b
-size 54967
+oid sha256:825e6d71ddfad3f418a251f5dbd4a117867f2b9c098a2624ca43fb4b508651cf
+size 54829

PreviewTests/Sources/__Snapshots__/PreviewTests/test_editRoomAddressScreen-iPhone-16-pseudo.No-alias.png

@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:7dc96671de7a8e204694533122addb9a64f705bf1818d6a0f46a1ba535571a1e
-size 68334
+oid sha256:03760870affe24024075c15eabfe8ffe8b3dfcd9c24a99473dedc9818e106df1
+size 68418

UnitTests/Sources/EditRoomAddressScreenViewModelTests.swift

@@ -17,5 +17,67 @@ class EditRoomAddressScreenViewModelTests: XCTestCase {
         viewModel.context
     }
     
-    override func setUpWithError() throws { }
+    func testCanonicalAliasChosen() async throws {
+        let roomProxy = JoinedRoomProxyMock(.init(name: "Room Name", canonicalAlias: "#room-name:matrix.org",
+                                                  alternativeAliases: ["#beta:homeserver.io",
+                                                                       "#alternative-room-name:matrix.org"]))
+        
+        viewModel = EditRoomAddressScreenViewModel(roomProxy: roomProxy,
+                                                   clientProxy: ClientProxyMock(.init(userIDServerName: "matrix.org")),
+                                                   userIndicatorController: UserIndicatorControllerMock())
+        
+        let deferred = deferFulfillment(context.$viewState) { state in
+            state.bindings.desiredAliasLocalPart == "room-name"
+        }
+        
+        try await deferred.fulfill()
+    }
+    
+    /// Priority should be given to aliases from the current user's homeserver as they can edit those.
+    func testAlternativeAliasChosen() async throws {
+        let roomProxy = JoinedRoomProxyMock(.init(name: "Room Name", canonicalAlias: "#alpha:homeserver.io",
+                                                  alternativeAliases: ["#beta:homeserver.io",
+                                                                       "#room-name:matrix.org",
+                                                                       "#alternative-room-name:matrix.org"]))
+        
+        viewModel = EditRoomAddressScreenViewModel(roomProxy: roomProxy,
+                                                   clientProxy: ClientProxyMock(.init(userIDServerName: "matrix.org")),
+                                                   userIndicatorController: UserIndicatorControllerMock())
+        
+        let deferred = deferFulfillment(context.$viewState) { state in
+            state.bindings.desiredAliasLocalPart == "room-name"
+        }
+        
+        try await deferred.fulfill()
+    }
+    
+    func testBuildAliasFromDisplayName() async throws {
+        let roomProxy = JoinedRoomProxyMock(.init(name: "Room Name"))
+        
+        viewModel = EditRoomAddressScreenViewModel(roomProxy: roomProxy,
+                                                   clientProxy: ClientProxyMock(.init(userIDServerName: "matrix.org")),
+                                                   userIndicatorController: UserIndicatorControllerMock())
+        
+        let deferred = deferFulfillment(context.$viewState) { state in
+            state.bindings.desiredAliasLocalPart == "room-name"
+        }
+        
+        try await deferred.fulfill()
+    }
+    
+    func testCorrectMethodsCalledOnSave() async throws {
+        let clientProxy = ClientProxyMock(.init(userIDServerName: "matrix.org"))
+        clientProxy.isAliasAvailableReturnValue = .success(true)
+        
+        let roomProxy = JoinedRoomProxyMock(.init(name: "Room Name"))
+        roomProxy.publishRoomAliasInRoomDirectoryReturnValue = .success(true)
+        roomProxy.updateCanonicalAliasAltAliasesReturnValue = .success(())
+        roomProxy.removeRoomAliasFromRoomDirectoryReturnValue = .success(true)
+        
+        viewModel = EditRoomAddressScreenViewModel(roomProxy: roomProxy,
+                                                   clientProxy: clientProxy,
+                                                   userIndicatorController: UserIndicatorControllerMock())
+        
+        context.send(viewAction: .save)
+    }
 }