From e4d961225a28699ccf449d1deb53290006c23166 Mon Sep 17 00:00:00 2001 From: Doug <6060466+pixlwave@users.noreply.github.com> Date: Mon, 19 Jan 2026 15:16:16 +0000 Subject: [PATCH] Silence some warnings. (#4969) --- .../SpacesTabFlowCoordinator.swift | 1 - .../View/CreateRoomScreen.swift | 4 +-- .../QRCodeLoginScreenModels.swift | 21 +++++++----- .../QRCodeLoginScreenViewModel.swift | 33 +++++++------------ .../Audio/Recorder/AudioRecorder.swift | 2 +- .../AuthenticationService.swift | 3 ++ .../Authentication/LinkNewDeviceService.swift | 12 ++++--- .../Media/MediaUploadingPreprocessor.swift | 10 +++--- UnitTests/Sources/DateTests.swift | 5 ++- UnitTests/Sources/LoggingTests.swift | 10 +++--- .../NotificationManagerTests.swift | 2 +- .../RoomDetailsEditScreenViewModelTests.swift | 2 +- 12 files changed, 50 insertions(+), 55 deletions(-) diff --git a/ElementX/Sources/FlowCoordinators/SpacesTabFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/SpacesTabFlowCoordinator.swift index bec3a2fad..de88d065a 100644 --- a/ElementX/Sources/FlowCoordinators/SpacesTabFlowCoordinator.swift +++ b/ElementX/Sources/FlowCoordinators/SpacesTabFlowCoordinator.swift @@ -96,7 +96,6 @@ class SpacesTabFlowCoordinator: FlowCoordinatorProtocol { // MARK: - Private - // swiftlint:disable:next cyclomatic_complexity private func configureStateMachine() { stateMachine.addRoutes(event: .start, transitions: [.initial => .spacesScreen(selectedSpaceID: nil)]) { [weak self] _ in self?.presentSpacesScreen() diff --git a/ElementX/Sources/Screens/CreateRoomScreen/View/CreateRoomScreen.swift b/ElementX/Sources/Screens/CreateRoomScreen/View/CreateRoomScreen.swift index ecd9567f7..86bf2f3df 100644 --- a/ElementX/Sources/Screens/CreateRoomScreen/View/CreateRoomScreen.swift +++ b/ElementX/Sources/Screens/CreateRoomScreen/View/CreateRoomScreen.swift @@ -307,7 +307,7 @@ struct CreateRoom_Previews: PreviewProvider, TestablePreview { analytics: ServiceLocator.shared.analytics, userIndicatorController: UserIndicatorControllerMock(), appSettings: ServiceLocator.shared.settings) - viewModel.updateAvatar(fileURL: Bundle.main.url(forResource: "preview_avatar_room", withExtension: "jpg")!) + viewModel.updateAvatar(fileURL: Bundle.main.url(forResource: "preview_avatar_room", withExtension: "jpg") ?? .picturesDirectory) return viewModel }() @@ -331,7 +331,7 @@ struct CreateRoom_Previews: PreviewProvider, TestablePreview { analytics: ServiceLocator.shared.analytics, userIndicatorController: UserIndicatorControllerMock(), appSettings: ServiceLocator.shared.settings) - viewModel.updateAvatar(fileURL: Bundle.main.url(forResource: "preview_avatar_room", withExtension: "jpg")!) + viewModel.updateAvatar(fileURL: Bundle.main.url(forResource: "preview_avatar_room", withExtension: "jpg") ?? .picturesDirectory) return viewModel }() diff --git a/ElementX/Sources/Screens/QRCodeLoginScreen/QRCodeLoginScreenModels.swift b/ElementX/Sources/Screens/QRCodeLoginScreen/QRCodeLoginScreenModels.swift index 2f95afefa..33472e411 100644 --- a/ElementX/Sources/Screens/QRCodeLoginScreen/QRCodeLoginScreenModels.swift +++ b/ElementX/Sources/Screens/QRCodeLoginScreen/QRCodeLoginScreenModels.swift @@ -37,33 +37,36 @@ enum QRCodeLoginScreenMode { struct QRCodeLoginScreenViewState: BindableState { var state: QRCodeLoginState + let mode: QRCodeLoginScreenMode /// Whether or not it is possible for the screen to start the manual sign in flow. This was added to avoid /// having to handle server configuration when ``AppSettings.allowOtherAccountProviders`` is false. let canSignInManually: Bool - let isPresentedModally: Bool let instructions = QRCodeLoginScreenInstructions() var bindings = QRCodeLoginScreenViewStateBindings() var shouldDisplayCancelButton: Bool { - // TODO: Simplify/validate these assumptions. - if isPresentedModally { + switch mode { + case .login: switch state { case .loginInstructions, .scan, .error(.noCameraPermission): true - default: false + case .error: false + case .linkDesktopInstructions, .displayCode, .displayQR, .confirmCode: false // Unreachable states. } - } else { + case .linkDesktop, .linkMobile: switch state { case .displayCode, .confirmCode, .scan, .error(.noCameraPermission): true - case .loginInstructions, .linkDesktopInstructions, .displayQR, .error: false + case .linkDesktopInstructions, .displayQR, .error: false + case .loginInstructions: false // Unreachable state. } } } var shouldDisplayBackButton: Bool { - if isPresentedModally { - false - } else { + switch mode { + case .login: + false // Login is presented modally, never show the back button. + case .linkDesktop, .linkMobile: switch state { case .loginInstructions, .linkDesktopInstructions, .displayQR: true case .displayCode, .confirmCode, .scan, .error: false diff --git a/ElementX/Sources/Screens/QRCodeLoginScreen/QRCodeLoginScreenViewModel.swift b/ElementX/Sources/Screens/QRCodeLoginScreen/QRCodeLoginScreenViewModel.swift index 4e2580640..40e93c328 100644 --- a/ElementX/Sources/Screens/QRCodeLoginScreen/QRCodeLoginScreenViewModel.swift +++ b/ElementX/Sources/Screens/QRCodeLoginScreen/QRCodeLoginScreenViewModel.swift @@ -12,7 +12,6 @@ import Foundation typealias QRCodeLoginScreenViewModelType = StateStoreViewModel class QRCodeLoginScreenViewModel: QRCodeLoginScreenViewModelType, QRCodeLoginScreenViewModelProtocol { - private let mode: QRCodeLoginScreenMode private let appMediator: AppMediatorProtocol private let actionsSubject: PassthroughSubject = .init() @@ -26,24 +25,19 @@ class QRCodeLoginScreenViewModel: QRCodeLoginScreenViewModelType, QRCodeLoginScr init(mode: QRCodeLoginScreenMode, canSignInManually: Bool, appMediator: AppMediatorProtocol) { - self.mode = mode self.appMediator = appMediator - let initialState: QRCodeLoginScreenViewState = switch mode { - case .login: - .init(state: .loginInstructions, canSignInManually: canSignInManually, isPresentedModally: true) - case .linkDesktop: - .init(state: .linkDesktopInstructions, canSignInManually: canSignInManually, isPresentedModally: false) + let initialViewState: QRCodeLoginState = switch mode { + case .login: .loginInstructions + case .linkDesktop: .linkDesktopInstructions case .linkMobile(let progressPublisher): switch progressPublisher.value { - case .qrReady(let image): - .init(state: .displayQR(image), canSignInManually: canSignInManually, isPresentedModally: false) - default: - .init(state: .error(.unknown), canSignInManually: canSignInManually, isPresentedModally: false) + case .qrReady(let image): .displayQR(image) + default: .error(.unknown) } } - super.init(initialViewState: initialState) + super.init(initialViewState: .init(state: initialViewState, mode: mode, canSignInManually: canSignInManually)) setupSubscriptions() if case .linkMobile(let progressPublisher) = mode { @@ -62,7 +56,7 @@ class QRCodeLoginScreenViewModel: QRCodeLoginScreenViewModelType, QRCodeLoginScr case .sendCheckCode: Task { await sendCheckCode() } case .errorAction(.startOver): - switch mode { + switch state.mode { case .login: Task { await startScanIfPossible() } case .linkDesktop, .linkMobile: @@ -88,7 +82,7 @@ class QRCodeLoginScreenViewModel: QRCodeLoginScreenViewModelType, QRCodeLoginScr .receive(on: DispatchQueue.main) .sink { [weak self] qrData in guard let self else { return } - switch mode { + switch state.mode { case .login(let qrCodeLoginService): handleScan(qrData: qrData, loginService: qrCodeLoginService) case .linkDesktop(let linkNewDeviceService): @@ -152,8 +146,6 @@ class QRCodeLoginScreenViewModel: QRCodeLoginScreenViewModelType, QRCodeLoginScr } } - // TODO: when user cancels in UI then the underlying login needs to be cancelled too. It's unclear if we have that exposed in the bindings yet. - private func handleScan(qrData: Data, linkService: LinkNewDeviceServiceProtocol) { guard currentTask == nil else { return } @@ -314,24 +306,23 @@ class QRCodeLoginScreenViewModel: QRCodeLoginScreenViewModelType, QRCodeLoginScr } /// Only for mocking initial states - fileprivate init(state: QRCodeLoginState, canSignInManually: Bool, isPresentedModally: Bool, checkCodeInput: String) { - mode = .login(QRCodeLoginServiceMock()) + fileprivate init(state: QRCodeLoginState, mode: QRCodeLoginScreenMode, canSignInManually: Bool, checkCodeInput: String) { appMediator = AppMediatorMock.default super.init(initialViewState: .init(state: state, + mode: mode, canSignInManually: canSignInManually, - isPresentedModally: isPresentedModally, bindings: .init(checkCodeInput: checkCodeInput))) } } extension QRCodeLoginScreenViewModel { static func mock(state: QRCodeLoginState, + mode: QRCodeLoginScreenMode = .login(QRCodeLoginServiceMock()), canSignInManually: Bool = true, - isPresentedModally: Bool = true, checkCodeInput: String = "") -> QRCodeLoginScreenViewModel { QRCodeLoginScreenViewModel(state: state, + mode: mode, canSignInManually: canSignInManually, - isPresentedModally: isPresentedModally, checkCodeInput: checkCodeInput) } } diff --git a/ElementX/Sources/Services/Audio/Recorder/AudioRecorder.swift b/ElementX/Sources/Services/Audio/Recorder/AudioRecorder.swift index 4057c7071..7120fe81c 100644 --- a/ElementX/Sources/Services/Audio/Recorder/AudioRecorder.swift +++ b/ElementX/Sources/Services/Audio/Recorder/AudioRecorder.swift @@ -113,7 +113,7 @@ class AudioRecorder: AudioRecorderProtocol { MXLog.info("setup audio session") try audioSession.setAllowHapticsAndSystemSoundsDuringRecording(true) - try audioSession.setCategory(.playAndRecord, mode: .default, options: [.allowBluetooth]) + try audioSession.setCategory(.playAndRecord, mode: .default, options: [.allowBluetoothHFP]) try audioSession.setActive(true) addObservers() } diff --git a/ElementX/Sources/Services/Authentication/AuthenticationService.swift b/ElementX/Sources/Services/Authentication/AuthenticationService.swift index 68c3282a8..6a003a7dc 100644 --- a/ElementX/Sources/Services/Authentication/AuthenticationService.swift +++ b/ElementX/Sources/Services/Authentication/AuthenticationService.swift @@ -160,6 +160,9 @@ class AuthenticationService: AuthenticationServiceProtocol { return progressSubject.asCurrentValuePublisher() } + // At some stage the SDK will have a `qrCodeData.intent` which we should check before continuing here. + // Note the equivalent check will also happen for linking a device by QR in the LinkNewDeviceService. + guard let scannedServerName = qrData.serverName() else { MXLog.error("The QR code is from a device that is not yet signed in.") progressSubject.send(completion: .failure(.qrCodeError(.deviceNotSignedIn))) diff --git a/ElementX/Sources/Services/Authentication/LinkNewDeviceService.swift b/ElementX/Sources/Services/Authentication/LinkNewDeviceService.swift index 19dab69a9..4caadf800 100644 --- a/ElementX/Sources/Services/Authentication/LinkNewDeviceService.swift +++ b/ElementX/Sources/Services/Authentication/LinkNewDeviceService.swift @@ -63,7 +63,7 @@ class LinkNewDeviceService: LinkNewDeviceServiceProtocol { Task { do { - // TODO: we need a way to cancel the in progress grant if the user hit the cancel button + // Note: The SDK doesn't provide us with a way to cancel the grant if the user hit the cancel button 🤷‍♂️ try await grantLoginHandler.generate(progressListener: listener) // The success state is handled by the listener. } catch let error as HumanQrGrantLoginError { MXLog.error("QR code reciprocate error: \(error)") @@ -98,12 +98,12 @@ class LinkNewDeviceService: LinkNewDeviceServiceProtocol { return progressSubject.asCurrentValuePublisher() } - #warning("Check intent/server name??") - #warning("Check Element Pro here??") + // At some stage the SDK will have a `qrCodeData.intent` which we should check before continuing here. + // Note the equivalent check will also happen for sign in with QR code in the AuthenticationService. Task { do { - // TODO: it would be nice to be able to cancel the grant at the SDK level if the user hits the cancel button + // Note: The SDK doesn't provide us with a way to cancel the grant if the user hit the cancel button 🤷‍♂️ try await grantLoginHandler.scan(qrCodeData: qrCodeData, progressListener: listener) // The success state is handled by the listener. } catch let error as HumanQrGrantLoginError { MXLog.error("QR code reciprocate error: \(error)") @@ -216,7 +216,9 @@ class CheckCodeSenderProxy: Equatable { try await underlyingSender.send(code: code) } - #warning("Waiting for an SDK update to use the underlying sender.") + // Bypassed for now whilst we wait for an SDK update (however its worth noting that + // things should still fail if the wrong code is provided, just not necessarily with + // the right error being shown). https://github.com/matrix-org/matrix-rust-sdk/pull/5957 func validate(checkCode: UInt8) -> Bool { true } } diff --git a/ElementX/Sources/Services/Media/MediaUploadingPreprocessor.swift b/ElementX/Sources/Services/Media/MediaUploadingPreprocessor.swift index 5b361021d..2d4a4fca5 100644 --- a/ElementX/Sources/Services/Media/MediaUploadingPreprocessor.swift +++ b/ElementX/Sources/Services/Media/MediaUploadingPreprocessor.swift @@ -451,9 +451,6 @@ struct MediaUploadingPreprocessor { try? FileManager.default.removeItem(at: outputURL) - exportSession.outputURL = outputURL - exportSession.outputFileType = AVFileType.mp4 - guard exportSession.supportedFileTypes.contains(AVFileType.mp4) else { throw .failedConvertingVideo } @@ -463,9 +460,10 @@ struct MediaUploadingPreprocessor { exportSession.fileLengthLimit = Int64(Double(targetFileSize) * 0.9) } - await exportSession.export() - - guard exportSession.status == .completed else { + do { + try await exportSession.export(to: outputURL, as: .mp4) + } catch { + MXLog.error("Video conversion failed: \(error)") throw .failedConvertingVideo } diff --git a/UnitTests/Sources/DateTests.swift b/UnitTests/Sources/DateTests.swift index 66fead9bc..99eae0d88 100644 --- a/UnitTests/Sources/DateTests.swift +++ b/UnitTests/Sources/DateTests.swift @@ -50,9 +50,8 @@ class DateTests: XCTestCase { XCTAssertEqual(threeDaysAgo.formattedDateSeparator(), threeDaysAgo.formatted(.dateTime.weekday(.wide))) // This test will fail during the first 6 days of the year. - let startOfTheYear = calendar.date(bySetting: .dayOfYear, value: 1, of: startOfToday)! - // FIXME: Uncomment on the 7th Jan. - // XCTAssertEqual(startOfTheYear.formattedDateSeparator(), startOfTheYear.formatted(.dateTime.weekday(.wide).day().month(.wide))) + let startOfTheYear = calendar.dateInterval(of: .year, for: startOfToday)!.start + XCTAssertEqual(startOfTheYear.formattedDateSeparator(), startOfTheYear.formatted(.dateTime.weekday(.wide).day().month(.wide))) let theMillennium = calendar.date(from: DateComponents(year: 2000, month: 1, day: 1))! XCTAssertEqual(theMillennium.formattedDateSeparator(), theMillennium.formatted(.dateTime.weekday(.wide).day().month(.wide).year())) diff --git a/UnitTests/Sources/LoggingTests.swift b/UnitTests/Sources/LoggingTests.swift index 482780d27..5a95fb7a4 100644 --- a/UnitTests/Sources/LoggingTests.swift +++ b/UnitTests/Sources/LoggingTests.swift @@ -34,7 +34,7 @@ class LoggingTests: XCTestCase { return } - try XCTAssertTrue(String(contentsOf: logFile).contains(infoLog)) + try XCTAssertTrue(String(contentsOf: logFile, encoding: .utf8).contains(infoLog)) } func testLogLevels() throws { @@ -48,7 +48,7 @@ class LoggingTests: XCTestCase { return } - try XCTAssertFalse(String(contentsOf: logFile).contains(verboseLog)) + try XCTAssertFalse(String(contentsOf: logFile, encoding: .utf8).contains(verboseLog)) } /// This is meant to test the `Target.tests.configure(…)`, but at this stage the test is somewhat pointless @@ -103,7 +103,7 @@ class LoggingTests: XCTestCase { return } - let content = try String(contentsOf: logFile) + let content = try String(contentsOf: logFile, encoding: .utf8) XCTAssertTrue(content.contains(roomSummary.id)) XCTAssertFalse(content.contains(roomName)) XCTAssertFalse(content.contains(lastMessage)) @@ -185,7 +185,7 @@ class LoggingTests: XCTestCase { return } - let content = try String(contentsOf: logFile) + let content = try String(contentsOf: logFile, encoding: .utf8) XCTAssertTrue(content.contains(textMessage.id.uniqueID.value)) XCTAssertFalse(content.contains(textMessage.body)) XCTAssertFalse(content.contains(textAttributedString)) @@ -254,7 +254,7 @@ class LoggingTests: XCTestCase { return } - let content = try String(contentsOf: logFile) + let content = try String(contentsOf: logFile, encoding: .utf8) XCTAssertTrue(content.contains(String(describing: TextMessageContent.self))) XCTAssertFalse(content.contains(textString)) diff --git a/UnitTests/Sources/NotificationManager/NotificationManagerTests.swift b/UnitTests/Sources/NotificationManager/NotificationManagerTests.swift index a6313eb55..e37bdb6e0 100644 --- a/UnitTests/Sources/NotificationManager/NotificationManagerTests.swift +++ b/UnitTests/Sources/NotificationManager/NotificationManagerTests.swift @@ -226,7 +226,7 @@ final class NotificationManagerTests: XCTestCase { } } -extension NotificationManagerTests: NotificationManagerDelegate { +extension NotificationManagerTests: @MainActor NotificationManagerDelegate { func registerForRemoteNotifications() { authorizationStatusWasGranted = true registerForRemoteNotificationsDelegateCalled?() diff --git a/UnitTests/Sources/RoomDetailsEditScreenViewModelTests.swift b/UnitTests/Sources/RoomDetailsEditScreenViewModelTests.swift index eb762a2bf..bed52fa8f 100644 --- a/UnitTests/Sources/RoomDetailsEditScreenViewModelTests.swift +++ b/UnitTests/Sources/RoomDetailsEditScreenViewModelTests.swift @@ -98,7 +98,7 @@ class RoomDetailsEditScreenViewModelTests: XCTestCase { XCTAssertFalse(context.viewState.canSave) XCTAssertNil(context.alertInfo) - var deferred = deferFulfillment(viewModel.actions) { $0 == .cancel } + let deferred = deferFulfillment(viewModel.actions) { $0 == .cancel } context.send(viewAction: .cancel) try await deferred.fulfill() XCTAssertNil(context.alertInfo)