From 9c23948ce55633fde80f17e8cbf23c233f889246 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Wed, 11 Jan 2023 11:12:55 +0200 Subject: [PATCH 1/7] Fix information leaking on RoomSummaryDetails logging --- .../Services/Room/RoomSummary/RoomSummaryDetails.swift | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryDetails.swift b/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryDetails.swift index 35fc248bc6..49d03cc53a 100644 --- a/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryDetails.swift +++ b/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryDetails.swift @@ -25,3 +25,9 @@ struct RoomSummaryDetails { let lastMessageTimestamp: Date? let unreadNotificationCount: UInt } + +extension RoomSummaryDetails: CustomStringConvertible { + var description: String { + "id: \"\(id)\", isDirect: \"\(isDirect)\", unreadNotificationCount: \"\(unreadNotificationCount)\"" + } +} From b4e852e2420b7f75701d174cb175775b3f9b96ef Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Wed, 11 Jan 2023 12:58:37 +0200 Subject: [PATCH 2/7] Prevent crashes when force quitting the application --- ElementX/Sources/Application/AppCoordinator.swift | 5 +++++ .../Sources/Application/AppCoordinatorStateMachine.swift | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/ElementX/Sources/Application/AppCoordinator.swift b/ElementX/Sources/Application/AppCoordinator.swift index f9ae849dfa..b699edd460 100644 --- a/ElementX/Sources/Application/AppCoordinator.swift +++ b/ElementX/Sources/Application/AppCoordinator.swift @@ -85,6 +85,11 @@ class AppCoordinator: AppCoordinatorProtocol { } func start() { + guard stateMachine.state == .initial else { + MXLog.error("Received a start request when already started") + return + } + stateMachine.processEvent(userSessionStore.hasSessions ? .startWithExistingSession : .startWithAuthentication) } diff --git a/ElementX/Sources/Application/AppCoordinatorStateMachine.swift b/ElementX/Sources/Application/AppCoordinatorStateMachine.swift index dc8afb0e27..1f17eba034 100644 --- a/ElementX/Sources/Application/AppCoordinatorStateMachine.swift +++ b/ElementX/Sources/Application/AppCoordinatorStateMachine.swift @@ -61,6 +61,10 @@ class AppCoordinatorStateMachine { private let stateMachine: StateMachine + var state: AppCoordinatorStateMachine.State { + stateMachine.state + } + init() { stateMachine = StateMachine(state: .initial) configure() From 5a11aeddaf2b1b96cbc9f93d3661b8e1926f9091 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Wed, 11 Jan 2023 13:05:29 +0200 Subject: [PATCH 3/7] Cleanup crash detected alert presentation and exposed home screen view model actions --- .../HomeScreen/HomeScreenCoordinator.swift | 41 +++++++------------ .../Screens/HomeScreen/HomeScreenModels.swift | 9 ++-- .../HomeScreen/HomeScreenViewModel.swift | 25 ++++++++--- .../HomeScreenViewModelProtocol.swift | 2 +- .../UserSessionFlowCoordinator.swift | 2 +- 5 files changed, 43 insertions(+), 36 deletions(-) diff --git a/ElementX/Sources/Screens/HomeScreen/HomeScreenCoordinator.swift b/ElementX/Sources/Screens/HomeScreen/HomeScreenCoordinator.swift index d36336f7a1..dd9d39ad74 100644 --- a/ElementX/Sources/Screens/HomeScreen/HomeScreenCoordinator.swift +++ b/ElementX/Sources/Screens/HomeScreen/HomeScreenCoordinator.swift @@ -25,7 +25,7 @@ struct HomeScreenCoordinatorParameters { } enum HomeScreenCoordinatorAction { - case presentRoomScreen(roomIdentifier: String) + case presentRoom(roomIdentifier: String) case presentSettingsScreen case presentFeedbackScreen case presentSessionVerificationScreen @@ -50,12 +50,18 @@ final class HomeScreenCoordinator: CoordinatorProtocol { guard let self else { return } switch action { - case .selectRoom(let roomIdentifier): - self.callback?(.presentRoomScreen(roomIdentifier: roomIdentifier)) - case .userMenu(let action): - self.processUserMenuAction(action) - case .verifySession: + case .presentRoom(let roomIdentifier): + self.callback?(.presentRoom(roomIdentifier: roomIdentifier)) + case .presentFeedbackScreen: + self.callback?(.presentFeedbackScreen) + case .presentSettingsScreen: + self.callback?(.presentSettingsScreen) + case .presentInviteFriendsScreen: + self.presentInviteFriends() + case .presentSessionVerificationScreen: self.callback?(.presentSessionVerificationScreen) + case .signOut: + self.callback?(.signOut) } } } @@ -63,15 +69,11 @@ final class HomeScreenCoordinator: CoordinatorProtocol { // MARK: - Public func start() { + #if !DEBUG if parameters.bugReportService.crashedLastRun { - viewModel.presentAlert( - AlertInfo(id: UUID(), - title: ElementL10n.sendBugReportAppCrashed, - primaryButton: .init(title: ElementL10n.no, action: nil), - secondaryButton: .init(title: ElementL10n.yes) { [weak self] in - self?.callback?(.presentFeedbackScreen) - })) + viewModel.presentCrashedLastRunAlert() } + #endif } func toPresentable() -> AnyView { @@ -79,19 +81,6 @@ final class HomeScreenCoordinator: CoordinatorProtocol { } // MARK: - Private - - private func processUserMenuAction(_ action: HomeScreenViewUserMenuAction) { - switch action { - case .settings: - callback?(.presentSettingsScreen) - case .inviteFriends: - presentInviteFriends() - case .feedback: - callback?(.presentFeedbackScreen) - case .signOut: - callback?(.signOut) - } - } private func presentInviteFriends() { parameters.navigationStackCoordinator.setSheetCoordinator(InviteFriendsCoordinator(userId: parameters.userSession.userID)) diff --git a/ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift b/ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift index 774a366b23..fbc771af36 100644 --- a/ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift +++ b/ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift @@ -18,9 +18,12 @@ import Foundation import UIKit enum HomeScreenViewModelAction { - case selectRoom(roomIdentifier: String) - case userMenu(action: HomeScreenViewUserMenuAction) - case verifySession + case presentRoom(roomIdentifier: String) + case presentSessionVerificationScreen + case presentSettingsScreen + case presentInviteFriendsScreen + case presentFeedbackScreen + case signOut } enum HomeScreenViewUserMenuAction { diff --git a/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift b/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift index ae197dd7c4..3955d52046 100644 --- a/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift +++ b/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift @@ -112,6 +112,7 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol // MARK: - Public + // swiftlint:disable:next cyclomatic_complexity override func process(viewAction: HomeScreenViewAction) async { switch viewAction { case .loadRoomData(let roomIdentifier): @@ -119,11 +120,20 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol loadDataForRoomIdentifier(roomIdentifier) } case .selectRoom(let roomIdentifier): - callback?(.selectRoom(roomIdentifier: roomIdentifier)) + callback?(.presentRoom(roomIdentifier: roomIdentifier)) case .userMenu(let action): - callback?(.userMenu(action: action)) + switch action { + case .feedback: + callback?(.presentFeedbackScreen) + case .settings: + callback?(.presentSettingsScreen) + case .inviteFriends: + callback?(.presentInviteFriendsScreen) + case .signOut: + callback?(.signOut) + } case .verifySession: - callback?(.verifySession) + callback?(.presentSessionVerificationScreen) case .skipSessionVerification: state.showSessionVerificationBanner = false case .updatedVisibleItemIdentifiers(let identifiers): @@ -131,8 +141,13 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol } } - func presentAlert(_ alertInfo: AlertInfo) { - state.bindings.alertInfo = alertInfo + func presentCrashedLastRunAlert() { + state.bindings.alertInfo = AlertInfo(id: UUID(), + title: ElementL10n.sendBugReportAppCrashed, + primaryButton: .init(title: ElementL10n.no, action: nil), + secondaryButton: .init(title: ElementL10n.yes) { [weak self] in + self?.callback?(.presentFeedbackScreen) + }) } // MARK: - Private diff --git a/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModelProtocol.swift b/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModelProtocol.swift index 54d15d17b1..dfa6b035db 100644 --- a/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModelProtocol.swift +++ b/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModelProtocol.swift @@ -23,5 +23,5 @@ protocol HomeScreenViewModelProtocol { var context: HomeScreenViewModelType.Context { get } - func presentAlert(_ alert: AlertInfo) + func presentCrashedLastRunAlert() } diff --git a/ElementX/Sources/Services/UserSession/UserSessionFlowCoordinator.swift b/ElementX/Sources/Services/UserSession/UserSessionFlowCoordinator.swift index 92a3c14669..d4b1d211f9 100644 --- a/ElementX/Sources/Services/UserSession/UserSessionFlowCoordinator.swift +++ b/ElementX/Sources/Services/UserSession/UserSessionFlowCoordinator.swift @@ -125,7 +125,7 @@ class UserSessionFlowCoordinator: CoordinatorProtocol { guard let self else { return } switch action { - case .presentRoomScreen(let roomIdentifier): + case .presentRoom(let roomIdentifier): self.stateMachine.processEvent(.selectRoom(roomId: roomIdentifier)) case .presentSettingsScreen: self.stateMachine.processEvent(.showSettingsScreen) From ee5c35466e5a80e74ba8991ab9fd91763fd6e8d1 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Wed, 11 Jan 2023 13:10:08 +0200 Subject: [PATCH 4/7] Fixes #340 - Wait for logout confirmation before changing the app state --- .../Sources/Application/AppCoordinator.swift | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/ElementX/Sources/Application/AppCoordinator.swift b/ElementX/Sources/Application/AppCoordinator.swift index b699edd460..060bf11a9b 100644 --- a/ElementX/Sources/Application/AppCoordinator.swift +++ b/ElementX/Sources/Application/AppCoordinator.swift @@ -260,21 +260,22 @@ class AppCoordinator: AppCoordinatorProtocol { deobserveUserSessionChanges() - if !isSoftLogout { - Task { - // first log out from the server - _ = await userSession.clientProxy.logout() - - // regardless of the result, clear user data - userSessionStore.logout(userSession: userSession) - userSession = nil - notificationManager?.delegate = nil - notificationManager = nil - } + Task { + showLoadingIndicator() + + // first log out from the server + _ = await userSession.clientProxy.logout() + + // regardless of the result, clear user data + userSessionStore.logout(userSession: userSession) + userSession = nil + notificationManager?.delegate = nil + notificationManager = nil + + stateMachine.processEvent(.completedSigningOut) + + hideLoadingIndicator() } - - // complete logging out - stateMachine.processEvent(.completedSigningOut) } private func presentSplashScreen(isSoftLogout: Bool = false) { From 422eca75a436e55b59ba49623ff6162affa42b0e Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Wed, 11 Jan 2023 13:12:09 +0200 Subject: [PATCH 5/7] Add changelogs --- changelog.d/340.bugfix | 1 + changelog.d/pr-437.bugfix | 1 + 2 files changed, 2 insertions(+) create mode 100644 changelog.d/340.bugfix create mode 100644 changelog.d/pr-437.bugfix diff --git a/changelog.d/340.bugfix b/changelog.d/340.bugfix new file mode 100644 index 0000000000..6c03a21da1 --- /dev/null +++ b/changelog.d/340.bugfix @@ -0,0 +1 @@ +Wait for logout confirmation before changing the app state \ No newline at end of file diff --git a/changelog.d/pr-437.bugfix b/changelog.d/pr-437.bugfix new file mode 100644 index 0000000000..8a5fe1a284 --- /dev/null +++ b/changelog.d/pr-437.bugfix @@ -0,0 +1 @@ +Prevent crash popups when force quitting the application \ No newline at end of file From 208f3c036d39a6cb6e51a12e107cca92894bb14e Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Wed, 11 Jan 2023 13:37:12 +0200 Subject: [PATCH 6/7] Fix unit tests --- UnitTests/Sources/HomeScreenViewModelTests.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/UnitTests/Sources/HomeScreenViewModelTests.swift b/UnitTests/Sources/HomeScreenViewModelTests.swift index b6220566b2..3304aeeae5 100644 --- a/UnitTests/Sources/HomeScreenViewModelTests.swift +++ b/UnitTests/Sources/HomeScreenViewModelTests.swift @@ -35,7 +35,7 @@ class HomeScreenViewModelTests: XCTestCase { var selectedRoomId = "" viewModel.callback = { result in switch result { - case .selectRoom(let roomId): + case .presentRoom(let roomId): correctResult = true selectedRoomId = roomId default: @@ -53,8 +53,8 @@ class HomeScreenViewModelTests: XCTestCase { var correctResult = false viewModel.callback = { result in switch result { - case .userMenu(let action): - correctResult = action == .settings + case .presentSettingsScreen: + correctResult = true default: break } From 6d7374041fcc7bf3e9b5693b433388f05b7a182d Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Wed, 11 Jan 2023 13:39:35 +0200 Subject: [PATCH 7/7] Add missing softLogout logout handling --- ElementX/Sources/Application/AppCoordinator.swift | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ElementX/Sources/Application/AppCoordinator.swift b/ElementX/Sources/Application/AppCoordinator.swift index 060bf11a9b..b9da6e44c9 100644 --- a/ElementX/Sources/Application/AppCoordinator.swift +++ b/ElementX/Sources/Application/AppCoordinator.swift @@ -260,6 +260,11 @@ class AppCoordinator: AppCoordinatorProtocol { deobserveUserSessionChanges() + guard !isSoftLogout else { + stateMachine.processEvent(.completedSigningOut) + return + } + Task { showLoadingIndicator()