Better handling for editing alias in case of different HS (#3695)

* better handling for aliases from different HS

* insert the alias at the top

* removing the old homeserver alias

* code improvement

* always remove the old canonical alias found on the server if exists

* added extensive testing for all the

possible cases on how the save is handled given the various context of the existing room alias

ElementX/Sources/Screens/EditRoomAddressScreen/EditRoomAddressScreenViewModel.swift

@@ -129,13 +129,13 @@ class EditRoomAddressScreenViewModel: EditRoomAddressScreenViewModelType, EditRo
             hideLoadingIndicator()
         }
         
-        guard let canonicalAlias = String.makeCanonicalAlias(aliasLocalPart: state.bindings.desiredAliasLocalPart, serverName: state.serverName),
+        guard let desiredCanonicalAlias = String.makeCanonicalAlias(aliasLocalPart: state.bindings.desiredAliasLocalPart, serverName: state.serverName),
-              isRoomAliasFormatValid(alias: canonicalAlias) else {
+              isRoomAliasFormatValid(alias: desiredCanonicalAlias) else {
             state.aliasErrors = [.invalidSymbols]
             return
         }
         
-        switch await clientProxy.isAliasAvailable(canonicalAlias) {
+        switch await clientProxy.isAliasAvailable(desiredCanonicalAlias) {
         case .success(true):
             break
         case .success(false):
@@ -146,24 +146,44 @@ class EditRoomAddressScreenViewModel: EditRoomAddressScreenViewModelType, EditRo
             return
         }
         
-        let oldAlias = roomProxy.infoPublisher.value.firstAliasMatching(serverName: clientProxy.userIDServerName, useFallback: false)
+        let savedAliasFromHomeserver = roomProxy.infoPublisher.value.firstAliasMatching(serverName: state.serverName, useFallback: false)
+        let savedCanonicalAlias = roomProxy.infoPublisher.value.canonicalAlias
         
-        // First publish the new alias
+        // First publish the desired new alias in the room directory
-        if case .failure = await roomProxy.publishRoomAliasInRoomDirectory(canonicalAlias) {
+        if case .failure = await roomProxy.publishRoomAliasInRoomDirectory(desiredCanonicalAlias) {
             userIndicatorController.submitIndicator(.init(title: L10n.errorUnknown))
             return
         }
         
-        // Then set it as the main alias
+        // Then try remove the old alias from the room directory on our current HS
-        if case .failure = await roomProxy.updateCanonicalAlias(canonicalAlias, altAliases: []) {
+        if let savedAliasFromHomeserver {
-            userIndicatorController.submitIndicator(.init(title: L10n.errorUnknown))
+            if case .failure = await roomProxy.removeRoomAliasFromRoomDirectory(savedAliasFromHomeserver) {
-            return
+                userIndicatorController.submitIndicator(.init(title: L10n.errorUnknown))
+                return
+            }
         }
         
-        // And finally delete the old one
+        // Finally update the canonical alias state..
-        if let oldAlias, case .failure = await roomProxy.removeRoomAliasFromRoomDirectory(oldAlias) {
+        // Allow to update the canonical alias only if the saved canonical alias matches the homeserver or if there is no canonical alias
-            userIndicatorController.submitIndicator(.init(title: L10n.errorUnknown))
+        if savedCanonicalAlias == nil || savedCanonicalAlias?.hasSuffix(state.serverName) == true {
-            return
+            var newAlternativeAliases = roomProxy.infoPublisher.value.alternativeAliases
+            newAlternativeAliases.removeAll { $0 == savedAliasFromHomeserver }
+
+            if case .failure = await roomProxy.updateCanonicalAlias(desiredCanonicalAlias, altAliases: newAlternativeAliases) {
+                userIndicatorController.submitIndicator(.init(title: L10n.errorUnknown))
+                return
+            }
+            // Otherwise, update the alternative aliases and keep the current canonical alias
+        } else {
+            var newAlternativeAliases = roomProxy.infoPublisher.value.alternativeAliases
+            // We also remove the existing saved alias from our homeserver if exists
+            newAlternativeAliases.removeAll { $0 == savedAliasFromHomeserver }
+            newAlternativeAliases.insert(desiredCanonicalAlias, at: 0)
+            
+            if case .failure = await roomProxy.updateCanonicalAlias(savedCanonicalAlias, altAliases: newAlternativeAliases) {
+                userIndicatorController.submitIndicator(.init(title: L10n.errorUnknown))
+                return
+            }
         }
         
         actionsSubject.send(.dismiss)

UnitTests/Sources/EditRoomAddressScreenViewModelTests.swift

@@ -65,19 +65,103 @@ class EditRoomAddressScreenViewModelTests: XCTestCase {
         try await deferred.fulfill()
     }
     
-    func testCorrectMethodsCalledOnSave() async throws {
+    func testCorrectMethodsCalledOnSaveWhenNoAliasExists() 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())
         
+        XCTAssertNil(roomProxy.infoPublisher.value.canonicalAlias)
+        XCTAssertEqual(viewModel.context.viewState.bindings.desiredAliasLocalPart, "room-name")
+
+        let publishingExpectation = expectation(description: "Wait for publishing")
+        roomProxy.publishRoomAliasInRoomDirectoryClosure = { roomAlias in
+            defer { publishingExpectation.fulfill() }
+            XCTAssertEqual(roomAlias, "#room-name:matrix.org")
+            return .success(true)
+        }
+        
+        let updateAliasExpectation = expectation(description: "Wait for alias update")
+        roomProxy.updateCanonicalAliasAltAliasesClosure = { roomAlias, altAliases in
+            defer { updateAliasExpectation.fulfill() }
+            XCTAssertEqual(altAliases, [])
+            XCTAssertEqual(roomAlias, "#room-name:matrix.org")
+            return .success(())
+        }
+        
         context.send(viewAction: .save)
+        await fulfillment(of: [publishingExpectation, updateAliasExpectation], timeout: 1.0)
+        XCTAssertFalse(roomProxy.removeRoomAliasFromRoomDirectoryCalled)
+    }
+    
+    func testCorrectMethodsCalledOnSaveWhenAliasOnSameHomeserverExists() async throws {
+        let clientProxy = ClientProxyMock(.init(userIDServerName: "matrix.org"))
+        clientProxy.isAliasAvailableReturnValue = .success(true)
+        let roomProxy = JoinedRoomProxyMock(.init(name: "Room Name", canonicalAlias: "#old-room-name:matrix.org"))
+        
+        viewModel = EditRoomAddressScreenViewModel(roomProxy: roomProxy,
+                                                   clientProxy: clientProxy,
+                                                   userIndicatorController: UserIndicatorControllerMock())
+        
+        context.desiredAliasLocalPart = "room-name"
+
+        let publishingExpectation = expectation(description: "Wait for publishing")
+        roomProxy.publishRoomAliasInRoomDirectoryClosure = { roomAlias in
+            defer { publishingExpectation.fulfill() }
+            XCTAssertEqual(roomAlias, "#room-name:matrix.org")
+            return .success(true)
+        }
+        
+        let updateAliasExpectation = expectation(description: "Wait for alias update")
+        roomProxy.updateCanonicalAliasAltAliasesClosure = { roomAlias, altAliases in
+            defer { updateAliasExpectation.fulfill() }
+            XCTAssertEqual(altAliases, [])
+            XCTAssertEqual(roomAlias, "#room-name:matrix.org")
+            return .success(())
+        }
+        
+        let removeAliasExpectation = expectation(description: "Wait for alias removal")
+        roomProxy.removeRoomAliasFromRoomDirectoryClosure = { roomAlias in
+            defer { removeAliasExpectation.fulfill() }
+            XCTAssertEqual(roomAlias, "#old-room-name:matrix.org")
+            return .success(true)
+        }
+        
+        context.send(viewAction: .save)
+        await fulfillment(of: [publishingExpectation, updateAliasExpectation, removeAliasExpectation], timeout: 1.0)
+    }
+    
+    func testCorrectMethodsCalledOnSaveWhenAliasOnOtherHomeserverExists() async throws {
+        let clientProxy = ClientProxyMock(.init(userIDServerName: "matrix.org"))
+        clientProxy.isAliasAvailableReturnValue = .success(true)
+        let roomProxy = JoinedRoomProxyMock(.init(name: "Room Name", canonicalAlias: "#old-room-name:element.io"))
+        
+        viewModel = EditRoomAddressScreenViewModel(roomProxy: roomProxy,
+                                                   clientProxy: clientProxy,
+                                                   userIndicatorController: UserIndicatorControllerMock())
+        
+        context.desiredAliasLocalPart = "room-name"
+
+        let publishingExpectation = expectation(description: "Wait for publishing")
+        roomProxy.publishRoomAliasInRoomDirectoryClosure = { roomAlias in
+            defer { publishingExpectation.fulfill() }
+            XCTAssertEqual(roomAlias, "#room-name:matrix.org")
+            return .success(true)
+        }
+        
+        let updateAliasExpectation = expectation(description: "Wait for alias update")
+        roomProxy.updateCanonicalAliasAltAliasesClosure = { roomAlias, altAliases in
+            defer { updateAliasExpectation.fulfill() }
+            XCTAssertEqual(altAliases, ["#room-name:matrix.org"])
+            XCTAssertEqual(roomAlias, "#old-room-name:element.io")
+            return .success(())
+        }
+        
+        context.send(viewAction: .save)
+        await fulfillment(of: [publishingExpectation, updateAliasExpectation], timeout: 1.0)
+        XCTAssertFalse(roomProxy.removeRoomAliasFromRoomDirectoryCalled)
     }
 }