Fix some of the PR remarks

RiotSwiftUI/Modules/Settings/ChangePassword/ChangePasswordModels.swift

@@ -20,7 +20,7 @@ import SwiftUI
 
 enum ChangePasswordViewModelResult {
     /// Submit with old and new passwords and sign out of all devices option
-    case submit(String, String, Bool)
+    case submit(oldPassword: String, newPassword: String, signoutAllDevices: Bool)
 }
 
 // MARK: View
@@ -29,11 +29,11 @@ struct ChangePasswordViewState: BindableState {
     /// View state that can be bound to from SwiftUI.
     var bindings: ChangePasswordBindings
     
-    /// Whether the user can submit the form: old password should be entered, and new passwords should match
+    /// Whether the user can submit the form: old password and new passwords should be entered
     var canSubmit: Bool {
         !bindings.oldPassword.isEmpty
         && !bindings.newPassword1.isEmpty
-        && bindings.newPassword1 == bindings.newPassword2
+        && !bindings.newPassword2.isEmpty
     }
 }
 
@@ -60,6 +60,8 @@ enum ChangePasswordViewAction {
 enum ChangePasswordErrorType: Hashable {
     /// An error response from the homeserver.
     case mxError(String)
+    /// User entered new passwords do not match
+    case passwordsDontMatch
     /// An unknown error occurred.
     case unknown
 }

RiotSwiftUI/Modules/Settings/ChangePassword/ChangePasswordViewModel.swift

@@ -34,7 +34,7 @@ class ChangePasswordViewModel: ChangePasswordViewModelType, ChangePasswordViewMo
     init(oldPassword: String = "",
          newPassword1: String = "",
          newPassword2: String = "",
-         signoutAllDevices: Bool = true) {
+         signoutAllDevices: Bool = false) {
         let bindings = ChangePasswordBindings(oldPassword: oldPassword,
                                               newPassword1: newPassword1,
                                               newPassword2: newPassword2,
@@ -48,9 +48,13 @@ class ChangePasswordViewModel: ChangePasswordViewModelType, ChangePasswordViewMo
     override func process(viewAction: ChangePasswordViewAction) {
         switch viewAction {
         case .submit:
-            Task { await callback?(.submit(state.bindings.oldPassword,
-                                           state.bindings.newPassword1,
-                                           state.bindings.signoutAllDevices)) }
+            guard state.bindings.newPassword1 == state.bindings.newPassword2 else {
+                Task { await displayError(.passwordsDontMatch) }
+                return
+            }
+            Task { await callback?(.submit(oldPassword: state.bindings.oldPassword,
+                                           newPassword: state.bindings.newPassword1,
+                                           signoutAllDevices: state.bindings.signoutAllDevices)) }
         case .toggleSignoutAllDevices:
             state.bindings.signoutAllDevices.toggle()
         }
@@ -62,6 +66,10 @@ class ChangePasswordViewModel: ChangePasswordViewModelType, ChangePasswordViewMo
             state.bindings.alertInfo = AlertInfo(id: type,
                                                  title: VectorL10n.error,
                                                  message: message)
+        case .passwordsDontMatch:
+            state.bindings.alertInfo = AlertInfo(id: type,
+                                                 title: VectorL10n.error,
+                                                 message: VectorL10n.authPasswordDontMatch)
         case .unknown:
             state.bindings.alertInfo = AlertInfo(id: type)
         }

RiotSwiftUI/Modules/Settings/ChangePassword/Coordinator/ChangePasswordBridgePresenter.swift

@@ -56,7 +56,7 @@ final class ChangePasswordCoordinatorBridgePresenter: NSObject {
 
     // MARK: - Public
 
-    @MainActor func present(from viewController: UIViewController, animated: Bool) {
+    func present(from viewController: UIViewController, animated: Bool) {
 
         let params = ChangePasswordCoordinatorParameters(restClient: self.session.matrixRestClient)
 

RiotSwiftUI/Modules/Settings/ChangePassword/Coordinator/ChangePasswordCoordinator.swift

@@ -48,7 +48,7 @@ final class ChangePasswordCoordinator: Coordinator, Presentable {
     
     // MARK: - Setup
     
-    @MainActor init(parameters: ChangePasswordCoordinatorParameters) {
+    init(parameters: ChangePasswordCoordinatorParameters) {
         self.parameters = parameters
         
         let viewModel = ChangePasswordViewModel()
@@ -105,14 +105,10 @@ final class ChangePasswordCoordinator: Coordinator, Presentable {
             do {
                 try await parameters.restClient.changePassword(from: oldPassword, to: newPassword, logoutDevices: signoutAllDevices)
 
-                // Shouldn't be reachable but just in case, continue the flow.
-
                 guard !Task.isCancelled else { return }
 
                 self?.stopLoading()
                 self?.callback?()
-            } catch is CancellationError {
-                return
             } catch {
                 self?.stopLoading()
                 self?.handleError(error)

RiotSwiftUI/Modules/Settings/ChangePassword/MockChangePasswordScreenState.swift

@@ -42,8 +42,7 @@ enum MockChangePasswordScreenState: MockScreenState, CaseIterable {
             viewModel = ChangePasswordViewModel()
         case .cannotSubmit:
             viewModel = ChangePasswordViewModel(oldPassword: "12345678",
-                                                newPassword1: "87654321",
-                                                newPassword2: "97654321")
+                                                newPassword1: "87654321")
         case .canSubmit:
             viewModel = ChangePasswordViewModel(oldPassword: "12345678",
                                                 newPassword1: "87654321",

RiotSwiftUI/Modules/Settings/ChangePassword/Test/UI/ChangePasswordUITests.swift

@@ -62,7 +62,7 @@ class ChangePasswordUITests: MockScreenTest {
 
         let signoutAllDevicesToggle = app.switches["signoutAllDevicesToggle"]
         XCTAssertTrue(signoutAllDevicesToggle.exists, "Sign out all devices toggle should exist")
-        XCTAssertTrue(signoutAllDevicesToggle.isOn, "Sign out all devices should be checked")
+        XCTAssertFalse(signoutAllDevicesToggle.isOn, "Sign out all devices should be unchecked")
     }
     
     func verifyCannotSubmit() {
@@ -70,15 +70,15 @@ class ChangePasswordUITests: MockScreenTest {
 
         let oldPasswordTextField = app.secureTextFields["oldPasswordTextField"]
         XCTAssertTrue(oldPasswordTextField.exists, "The text field should be shown.")
-        XCTAssertEqual(oldPasswordTextField.value as? String, "••••••••", "The text field should be showing the placeholder before text is input.")
+        XCTAssertEqual(oldPasswordTextField.value as? String, "••••••••", "The text field should show the entered password secretly.")
 
         let newPasswordTextField1 = app.secureTextFields["newPasswordTextField1"]
         XCTAssertTrue(newPasswordTextField1.exists, "The text field should be shown.")
-        XCTAssertEqual(newPasswordTextField1.value as? String, "••••••••", "The text field should be showing the placeholder before text is input.")
+        XCTAssertEqual(newPasswordTextField1.value as? String, "••••••••", "The text field should show the entered password secretly.")
 
         let newPasswordTextField2 = app.secureTextFields["newPasswordTextField2"]
         XCTAssertTrue(newPasswordTextField2.exists, "The text field should be shown.")
-        XCTAssertEqual(newPasswordTextField2.value as? String, "••••••••", "The text field should be showing the placeholder before text is input.")
+        XCTAssertEqual(newPasswordTextField2.label, "confirm password", "The text field should be showing the placeholder before text is input.")
 
         let submitButton = app.buttons["submitButton"]
         XCTAssertTrue(submitButton.exists, "The submit button should be shown.")
@@ -86,7 +86,7 @@ class ChangePasswordUITests: MockScreenTest {
 
         let signoutAllDevicesToggle = app.switches["signoutAllDevicesToggle"]
         XCTAssertTrue(signoutAllDevicesToggle.exists, "Sign out all devices toggle should exist")
-        XCTAssertTrue(signoutAllDevicesToggle.isOn, "Sign out all devices should be checked")
+        XCTAssertFalse(signoutAllDevicesToggle.isOn, "Sign out all devices should be unchecked")
     }
     
     func verifyCanSubmit() {
@@ -94,15 +94,15 @@ class ChangePasswordUITests: MockScreenTest {
 
         let oldPasswordTextField = app.secureTextFields["oldPasswordTextField"]
         XCTAssertTrue(oldPasswordTextField.exists, "The text field should be shown.")
-        XCTAssertEqual(oldPasswordTextField.value as? String, "••••••••", "The text field should be showing the placeholder before text is input.")
+        XCTAssertEqual(oldPasswordTextField.value as? String, "••••••••", "The text field should show the entered password secretly.")
 
         let newPasswordTextField1 = app.secureTextFields["newPasswordTextField1"]
         XCTAssertTrue(newPasswordTextField1.exists, "The text field should be shown.")
-        XCTAssertEqual(newPasswordTextField1.value as? String, "••••••••", "The text field should be showing the placeholder before text is input.")
+        XCTAssertEqual(newPasswordTextField1.value as? String, "••••••••", "The text field should show the entered password secretly.")
 
         let newPasswordTextField2 = app.secureTextFields["newPasswordTextField2"]
         XCTAssertTrue(newPasswordTextField2.exists, "The text field should be shown.")
-        XCTAssertEqual(newPasswordTextField2.value as? String, "••••••••", "The text field should be showing the placeholder before text is input.")
+        XCTAssertEqual(newPasswordTextField2.value as? String, "••••••••", "The text field should show the entered password secretly.")
 
         let submitButton = app.buttons["submitButton"]
         XCTAssertTrue(submitButton.exists, "The submit button should be shown.")
@@ -110,7 +110,7 @@ class ChangePasswordUITests: MockScreenTest {
 
         let signoutAllDevicesToggle = app.switches["signoutAllDevicesToggle"]
         XCTAssertTrue(signoutAllDevicesToggle.exists, "Sign out all devices toggle should exist")
-        XCTAssertTrue(signoutAllDevicesToggle.isOn, "Sign out all devices should be checked")
+        XCTAssertFalse(signoutAllDevicesToggle.isOn, "Sign out all devices should be unchecked")
     }
 
     func verifyCanSubmitAndSignoutAllDevicesChecked() {
@@ -118,15 +118,15 @@ class ChangePasswordUITests: MockScreenTest {
 
         let oldPasswordTextField = app.secureTextFields["oldPasswordTextField"]
         XCTAssertTrue(oldPasswordTextField.exists, "The text field should be shown.")
-        XCTAssertEqual(oldPasswordTextField.value as? String, "••••••••", "The text field should be showing the placeholder before text is input.")
+        XCTAssertEqual(oldPasswordTextField.value as? String, "••••••••", "The text field should show the entered password secretly.")
 
         let newPasswordTextField1 = app.secureTextFields["newPasswordTextField1"]
         XCTAssertTrue(newPasswordTextField1.exists, "The text field should be shown.")
-        XCTAssertEqual(newPasswordTextField1.value as? String, "••••••••", "The text field should be showing the placeholder before text is input.")
+        XCTAssertEqual(newPasswordTextField1.value as? String, "••••••••", "The text field should show the entered password secretly.")
 
         let newPasswordTextField2 = app.secureTextFields["newPasswordTextField2"]
         XCTAssertTrue(newPasswordTextField2.exists, "The text field should be shown.")
-        XCTAssertEqual(newPasswordTextField2.value as? String, "••••••••", "The text field should be showing the placeholder before text is input.")
+        XCTAssertEqual(newPasswordTextField2.value as? String, "••••••••", "The text field should show the entered password secretly.")
 
         let submitButton = app.buttons["submitButton"]
         XCTAssertTrue(submitButton.exists, "The submit button should be shown.")