From 218958d7c055d92802c4b7af84eadee4a48bba1f Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Fri, 20 Dec 2024 15:43:01 +0100 Subject: [PATCH] implemented error checking when editing the address --- .../CreateRoom/CreateRoomViewModel.swift | 2 +- .../EditRoomAddressScreenModels.swift | 14 ++-- .../EditRoomAddressScreenViewModel.swift | 79 ++++++++++++++++++- .../View/EditRoomAddressScreen.swift | 35 +++++--- .../SecurityAndPrivacyScreenViewModel.swift | 1 - 5 files changed, 111 insertions(+), 20 deletions(-) diff --git a/ElementX/Sources/Screens/CreateRoom/CreateRoomViewModel.swift b/ElementX/Sources/Screens/CreateRoom/CreateRoomViewModel.swift index 8f9a5d8dc5..c8790687d2 100644 --- a/ElementX/Sources/Screens/CreateRoom/CreateRoomViewModel.swift +++ b/ElementX/Sources/Screens/CreateRoom/CreateRoomViewModel.swift @@ -271,7 +271,7 @@ class CreateRoomViewModel: CreateRoomViewModelType, CreateRoomViewModelProtocol } } - func canonicalAlias(aliasLocalPart: String?) -> String? { + private func canonicalAlias(aliasLocalPart: String?) -> String? { guard let aliasLocalPart, !aliasLocalPart.isEmpty else { return nil diff --git a/ElementX/Sources/Screens/EditRoomAddressScreen/EditRoomAddressScreenModels.swift b/ElementX/Sources/Screens/EditRoomAddressScreen/EditRoomAddressScreenModels.swift index 7a55102245..c5d1bcaea6 100644 --- a/ElementX/Sources/Screens/EditRoomAddressScreen/EditRoomAddressScreenModels.swift +++ b/ElementX/Sources/Screens/EditRoomAddressScreen/EditRoomAddressScreenModels.swift @@ -14,18 +14,22 @@ enum EditRoomAddressScreenViewModelAction { struct EditRoomAddressScreenViewState: BindableState { let serverName: String var currentAliasLocalPart: String? - var desiredAliasLocalPart: String var aliasErrors: Set = [] var canSave: Bool { - currentAliasLocalPart != desiredAliasLocalPart && - !aliasErrors.isEmpty && - !desiredAliasLocalPart.isEmpty + currentAliasLocalPart != bindings.desiredAliasLocalPart && + aliasErrors.isEmpty && + !bindings.desiredAliasLocalPart.isEmpty } + + var bindings: EditRoomAddressScreenViewStateBindings +} + +struct EditRoomAddressScreenViewStateBindings { + var desiredAliasLocalPart: String } enum EditRoomAddressScreenViewAction { case save case cancel - case updateAliasLocalPart(String) } diff --git a/ElementX/Sources/Screens/EditRoomAddressScreen/EditRoomAddressScreenViewModel.swift b/ElementX/Sources/Screens/EditRoomAddressScreen/EditRoomAddressScreenViewModel.swift index 171ca51a80..d027febf6a 100644 --- a/ElementX/Sources/Screens/EditRoomAddressScreen/EditRoomAddressScreenViewModel.swift +++ b/ElementX/Sources/Screens/EditRoomAddressScreen/EditRoomAddressScreenViewModel.swift @@ -20,6 +20,8 @@ class EditRoomAddressScreenViewModel: EditRoomAddressScreenViewModelType, EditRo var actionsPublisher: AnyPublisher { actionsSubject.eraseToAnyPublisher() } + + @CancellableTask private var checkAliasAvailabilityTask: Task? init(roomProxy: JoinedRoomProxyProtocol, clientProxy: ClientProxyProtocol, @@ -36,7 +38,8 @@ class EditRoomAddressScreenViewModel: EditRoomAddressScreenViewModelType, EditRo } super.init(initialViewState: EditRoomAddressScreenViewState(serverName: clientProxy.userIDServerName ?? "", - desiredAliasLocalPart: aliasLocalPart)) + bindings: .init(desiredAliasLocalPart: aliasLocalPart))) + setupSubscriptions() } // MARK: - Public @@ -46,12 +49,80 @@ class EditRoomAddressScreenViewModel: EditRoomAddressScreenViewModelType, EditRo switch viewAction { case .save: - // TODO: Handle the save action - break + Task { await save() } case .cancel: actionsSubject.send(.cancel) - case .updateAliasLocalPart(let updatedValue): + } + } + + private func setupSubscriptions() { + context.$viewState + .map(\.bindings.desiredAliasLocalPart) + .removeDuplicates() + .debounce(for: 1, scheduler: DispatchQueue.main) + .sink { [weak self] aliasLocalPart in + guard let self else { + return + } + + guard let canonicalAlias = canonicalAlias(aliasLocalPart: aliasLocalPart) else { + // While is empty don't display the errors, since the save button is already disabled + state.aliasErrors.removeAll() + 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.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 save() async { + guard let canonicalAlias = canonicalAlias(aliasLocalPart: state.bindings.desiredAliasLocalPart), + isRoomAliasFormatValid(alias: canonicalAlias) else { + state.aliasErrors = [.invalidSymbols] + return + } + + switch await clientProxy.isAliasAvailable(canonicalAlias) { + case .success(true): break + case .success(false): + state.aliasErrors = [.alreadyExists] + return + case .failure: + userIndicatorController.submitIndicator(.init(title: L10n.errorUnknown)) + return + } + + // TODO: API calls to edit/add the alias and maybe also dismiss the view? (check with design) + } + + private func canonicalAlias(aliasLocalPart: String) -> String? { + guard !aliasLocalPart.isEmpty else { + return nil } + return "#\(aliasLocalPart):\(state.serverName)" } } diff --git a/ElementX/Sources/Screens/EditRoomAddressScreen/View/EditRoomAddressScreen.swift b/ElementX/Sources/Screens/EditRoomAddressScreen/View/EditRoomAddressScreen.swift index f0808a5a05..824fb4c3fb 100644 --- a/ElementX/Sources/Screens/EditRoomAddressScreen/View/EditRoomAddressScreen.swift +++ b/ElementX/Sources/Screens/EditRoomAddressScreen/View/EditRoomAddressScreen.swift @@ -11,19 +11,14 @@ import SwiftUI struct EditRoomAddressScreen: View { @ObservedObject var context: EditRoomAddressScreenViewModel.Context - private var aliasBinding: Binding { - .init(get: { - context.viewState.desiredAliasLocalPart - }, set: { - context.send(viewAction: .updateAliasLocalPart($0)) - }) - } - var body: some View { Form { Section { - EditRoomAddressListRow(aliasLocalPart: aliasBinding, + EditRoomAddressListRow(aliasLocalPart: $context.desiredAliasLocalPart, serverName: context.viewState.serverName, shouldDisplayError: context.viewState.aliasErrors.errorDescription != nil) + .onChange(of: context.desiredAliasLocalPart) { _, newAliasLocalPart in + context.desiredAliasLocalPart = newAliasLocalPart.lowercased() + } } footer: { VStack(alignment: .leading, spacing: 12) { if let errorDescription = context.viewState.aliasErrors.errorDescription { @@ -70,6 +65,18 @@ struct EditRoomAddressScreen_Previews: PreviewProvider, TestablePreview { clientProxy: ClientProxyMock(.init(userIDServerName: "matrix.org")), userIndicatorController: UserIndicatorControllerMock()) + static let invalidSymbolsViewModel = EditRoomAddressScreenViewModel(roomProxy: JoinedRoomProxyMock(.init(name: "Room Name", canonicalAlias: "#room#-alias:matrix.org")), + clientProxy: ClientProxyMock(.init(userIDServerName: "matrix.org")), + userIndicatorController: UserIndicatorControllerMock()) + + static let alreadyExistingViewModel = { + let clientProxy = ClientProxyMock(.init(userIDServerName: "matrix.org")) + clientProxy.isAliasAvailableReturnValue = .success(false) + return EditRoomAddressScreenViewModel(roomProxy: JoinedRoomProxyMock(.init(name: "Room Name")), + clientProxy: clientProxy, + userIndicatorController: UserIndicatorControllerMock()) + }() + static var previews: some View { NavigationStack { EditRoomAddressScreen(context: noAliasviewModel.context) @@ -80,5 +87,15 @@ struct EditRoomAddressScreen_Previews: PreviewProvider, TestablePreview { EditRoomAddressScreen(context: aliasviewModel.context) } .previewDisplayName("With alias") + + NavigationStack { + EditRoomAddressScreen(context: invalidSymbolsViewModel.context) + } + .previewDisplayName("Invalid symbols") + + NavigationStack { + EditRoomAddressScreen(context: alreadyExistingViewModel.context) + } + .previewDisplayName("Already existing") } } diff --git a/ElementX/Sources/Screens/SecurityAndPrivacyScreen/SecurityAndPrivacyScreenViewModel.swift b/ElementX/Sources/Screens/SecurityAndPrivacyScreen/SecurityAndPrivacyScreenViewModel.swift index a93be2500b..bd6048ad83 100644 --- a/ElementX/Sources/Screens/SecurityAndPrivacyScreen/SecurityAndPrivacyScreenViewModel.swift +++ b/ElementX/Sources/Screens/SecurityAndPrivacyScreen/SecurityAndPrivacyScreenViewModel.swift @@ -27,7 +27,6 @@ class SecurityAndPrivacyScreenViewModel: SecurityAndPrivacyScreenViewModelType, self.roomProxy = roomProxy self.clientProxy = clientProxy self.userIndicatorController = userIndicatorController - let canonicalAlias = roomProxy.infoPublisher.value.canonicalAlias super.init(initialViewState: SecurityAndPrivacyScreenViewState(serverName: clientProxy.userIDServerName ?? "", accessType: roomProxy.infoPublisher.value.roomAccessType, isEncryptionEnabled: roomProxy.isEncrypted,