Use the SDK's offline detection everywhere (except for restarting the sync loop). (#4472)

* Use the SDK's offline state to drive the offline indicator. * Only use network reachability for restarting the sync loop, use the homeserver reachability for requests. * Add a separate indicator to distinguish when the server is unreachable but the device is online.
2026-11-02 21:33:26 +00:00 · 2025-09-08 13:18:02 +01:00
parent 20b57d0ad7
commit 90504d89c8
26 changed files with 152 additions and 127 deletions
@@ -12,13 +12,13 @@ import Combine

@MainActor
 class UserSessionFlowCoordinatorTests: XCTestCase {
-    var clientProxy: ClientProxyMock!
-    var timelineControllerFactory: TimelineControllerFactoryMock!
    var userSessionFlowCoordinator: UserSessionFlowCoordinator!
    var rootCoordinator: NavigationRootCoordinator!
-    var notificationManager: NotificationManagerMock!
+    var userIndicatorController: UserIndicatorControllerMock!
    let stateMachineFactory = PublishedStateMachineFactory()
    
+    let networkReachabilitySubject: CurrentValueSubject<NetworkMonitorReachability, Never> = .init(.reachable)
+    let homeserverReachabilitySubject: CurrentValueSubject<NetworkMonitorReachability, Never> = .init(.reachable)
    var cancellables = Set<AnyCancellable>()
    
    var tabCoordinator: NavigationTabCoordinator<UserSessionFlowCoordinator.HomeTab>? { rootCoordinator?.rootCoordinator as? NavigationTabCoordinator }
@@ -28,24 +28,30 @@ class UserSessionFlowCoordinatorTests: XCTestCase {
    
    override func setUp() async throws {
        cancellables.removeAll()
-        clientProxy = ClientProxyMock(.init(userID: "hi@bob", roomSummaryProvider: RoomSummaryProviderMock(.init(state: .loaded(.mockRooms)))))
-        timelineControllerFactory = TimelineControllerFactoryMock(.init())
        
        rootCoordinator = NavigationRootCoordinator()
        
-        notificationManager = NotificationManagerMock()
+        let clientProxy = ClientProxyMock(.init(userID: "hi@bob", roomSummaryProvider: RoomSummaryProviderMock(.init(state: .loaded(.mockRooms)))))
+        clientProxy.homeserverReachabilityPublisher = homeserverReachabilitySubject.asCurrentValuePublisher()
+        
+        let networkMonitor = NetworkMonitorMock.default
+        networkMonitor.reachabilityPublisher = networkReachabilitySubject.asCurrentValuePublisher()
+        let appMediator = AppMediatorMock.default
+        appMediator.networkMonitor = networkMonitor
+        
+        userIndicatorController = UserIndicatorControllerMock()
        
        let flowParameters = CommonFlowParameters(userSession: UserSessionMock(.init(clientProxy: clientProxy)),
                                                  bugReportService: BugReportServiceMock(.init()),
                                                  elementCallService: ElementCallServiceMock(.init()),
-                                                  timelineControllerFactory: timelineControllerFactory,
+                                                  timelineControllerFactory: TimelineControllerFactoryMock(.init()),
                                                  emojiProvider: EmojiProvider(appSettings: ServiceLocator.shared.settings),
-                                                  appMediator: AppMediatorMock.default,
+                                                  appMediator: appMediator,
                                                  appSettings: ServiceLocator.shared.settings,
                                                  appHooks: AppHooks(),
                                                  analytics: ServiceLocator.shared.analytics,
-                                                  userIndicatorController: UserIndicatorControllerMock(),
-                                                  notificationManager: notificationManager,
+                                                  userIndicatorController: userIndicatorController,
+                                                  notificationManager: NotificationManagerMock(),
                                                  stateMachineFactory: stateMachineFactory)
        
        userSessionFlowCoordinator = UserSessionFlowCoordinator(isNewLogin: false,
@@ -56,6 +62,8 @@ class UserSessionFlowCoordinatorTests: XCTestCase {
        userSessionFlowCoordinator.start()
    }
    
+    // MARK: Navigation
+    
    func testInitialState() async throws {
        XCTAssertNotNil(chatsSplitCoordinator)
        XCTAssertNil(detailCoordinator)
@@ -154,6 +162,51 @@ class UserSessionFlowCoordinatorTests: XCTestCase {
        XCTAssertNil(chatsSplitCoordinator?.sheetCoordinator, "The media upload sheet shouldn't be shown when sharing text.")
    }
    
+    // MARK: Indicators
+    
+    func testReachabilityIndicators() async throws {
+        // Given a flow in its initial state.
+        try await Task.sleep(for: .milliseconds(100))
+        
+        // Then no reachability indicators should be shown.
+        XCTAssertFalse(userIndicatorController.submitIndicatorDelayCalled)
+        XCTAssertEqual(retractReachabilityIndicatorCallsCount, 1) // The initial state removes the indicator.
+        
+        // When the homeserver becomes unreachable.
+        homeserverReachabilitySubject.send(.unreachable)
+        try await Task.sleep(for: .milliseconds(100))
+        
+        // Then a server unreachable indicator should be shown.
+        XCTAssertEqual(userIndicatorController.submitIndicatorDelayCallsCount, 1)
+        XCTAssertEqual(userIndicatorController.submitIndicatorDelayReceivedArguments?.indicator.title, L10n.commonServerUnreachable)
+        XCTAssertEqual(retractReachabilityIndicatorCallsCount, 1)
+        
+        // When the network also becomes unreachable.
+        networkReachabilitySubject.send(.unreachable)
+        try await Task.sleep(for: .milliseconds(100))
+        
+        // Then the server unreachable indicator should be replaced with an offline indicator.
+        XCTAssertEqual(userIndicatorController.submitIndicatorDelayCallsCount, 2)
+        XCTAssertEqual(userIndicatorController.submitIndicatorDelayReceivedArguments?.indicator.title, L10n.commonOffline)
+        XCTAssertEqual(retractReachabilityIndicatorCallsCount, 1)
+        
+        // When the homeserver becomes reachable again.
+        homeserverReachabilitySubject.send(.reachable)
+        try await Task.sleep(for: .milliseconds(100))
+        
+        // Then the indicator should be hidden even if the network isn't reachable.
+        XCTAssertEqual(userIndicatorController.submitIndicatorDelayCallsCount, 2)
+        XCTAssertEqual(retractReachabilityIndicatorCallsCount, 2)
+        
+        // When the network becomes reachable again.
+        networkReachabilitySubject.send(.reachable)
+        try await Task.sleep(for: .milliseconds(100))
+        
+        // Then nothing else should happen.
+        XCTAssertEqual(userIndicatorController.submitIndicatorDelayCallsCount, 2)
+        XCTAssertEqual(retractReachabilityIndicatorCallsCount, 3)
+    }
+    
    // MARK: - Helpers
    
    private func process(route: AppRoute,
@@ -179,4 +232,12 @@ class UserSessionFlowCoordinatorTests: XCTestCase {
        try await deferredUserSession?.fulfill()
        try await deferredChatsState?.fulfill()
    }
+    
+    /// Other services retract indicators, so this filters based on the reachability ID.
+    private var retractReachabilityIndicatorCallsCount: Int {
+        userIndicatorController
+            .retractIndicatorWithIdReceivedInvocations
+            .filter { $0 == "io.element.elementx.reachability.notification" }
+            .count
+    }
 }