From 761824fa0dc70fae24cceb7444de3c8b45bb7cc4 Mon Sep 17 00:00:00 2001 From: Doug <6060466+pixlwave@users.noreply.github.com> Date: Mon, 8 Apr 2024 16:44:03 +0100 Subject: [PATCH] Tweak internal/external deeplink handling (#2664) * Add a childRoom AppRoute. * Add a tests for child room routes. --- .../Sources/Application/AppCoordinator.swift | 10 ++++- .../Application/AppCoordinatorProtocol.swift | 2 +- .../Sources/Application/Application.swift | 4 +- .../Application/Navigation/AppRoutes.swift | 11 +++++ .../RoomFlowCoordinator.swift | 21 ++++++---- .../UserSessionFlowCoordinator.swift | 6 +++ .../UITests/UITestsAppCoordinator.swift | 2 +- .../UnitTests/UnitTestsAppCoordinator.swift | 2 +- .../Sources/RoomFlowCoordinatorTests.swift | 42 +++++++++++++++++++ 9 files changed, 86 insertions(+), 14 deletions(-) diff --git a/ElementX/Sources/Application/AppCoordinator.swift b/ElementX/Sources/Application/AppCoordinator.swift index 095a5802d..af79325a1 100644 --- a/ElementX/Sources/Application/AppCoordinator.swift +++ b/ElementX/Sources/Application/AppCoordinator.swift @@ -176,7 +176,7 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg ) } - func handleDeepLink(_ url: URL) -> Bool { + func handleDeepLink(_ url: URL, isExternalURL: Bool) -> Bool { // Parse into an AppRoute to redirect these in a type safe way. if let route = appRouteURLParser.route(from: url) { @@ -193,8 +193,14 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg } else { navigationRootCoordinator.setSheetCoordinator(GenericCallLinkCoordinator(parameters: .init(url: url))) } - case .roomMemberDetails, .room: + case .roomMemberDetails: userSessionFlowCoordinator?.handleAppRoute(route, animated: true) + case .room(let roomID): + if isExternalURL { + userSessionFlowCoordinator?.handleAppRoute(route, animated: true) + } else { + userSessionFlowCoordinator?.handleAppRoute(.childRoom(roomID: roomID), animated: true) + } default: break } diff --git a/ElementX/Sources/Application/AppCoordinatorProtocol.swift b/ElementX/Sources/Application/AppCoordinatorProtocol.swift index aa8649d3f..5ac3381c5 100644 --- a/ElementX/Sources/Application/AppCoordinatorProtocol.swift +++ b/ElementX/Sources/Application/AppCoordinatorProtocol.swift @@ -18,5 +18,5 @@ import Foundation protocol AppCoordinatorProtocol: CoordinatorProtocol { var windowManager: WindowManagerProtocol { get } - @discardableResult func handleDeepLink(_ url: URL) -> Bool + @discardableResult func handleDeepLink(_ url: URL, isExternalURL: Bool) -> Bool } diff --git a/ElementX/Sources/Application/Application.swift b/ElementX/Sources/Application/Application.swift index b7db31a3c..434e265bf 100644 --- a/ElementX/Sources/Application/Application.swift +++ b/ElementX/Sources/Application/Application.swift @@ -40,14 +40,14 @@ struct Application: App { appCoordinator.toPresentable() .statusBarHidden(shouldHideStatusBar) .environment(\.openURL, OpenURLAction { url in - if appCoordinator.handleDeepLink(url) { + if appCoordinator.handleDeepLink(url, isExternalURL: false) { return .handled } return .systemAction }) .onOpenURL { - if !appCoordinator.handleDeepLink($0) { + if !appCoordinator.handleDeepLink($0, isExternalURL: true) { openURLInSystemBrowser($0) } } diff --git a/ElementX/Sources/Application/Navigation/AppRoutes.swift b/ElementX/Sources/Application/Navigation/AppRoutes.swift index a6664bf12..bcc313ad0 100644 --- a/ElementX/Sources/Application/Navigation/AppRoutes.swift +++ b/ElementX/Sources/Application/Navigation/AppRoutes.swift @@ -17,13 +17,24 @@ import Foundation enum AppRoute: Equatable { + /// The callback used to complete login with OIDC. case oidcCallback(url: URL) + /// The app's home screen. case roomList + /// A room, shown as the root of the stack (popping any child rooms). case room(roomID: String) + /// A room, pushed as a child of any existing rooms on the stack. + case childRoom(roomID: String) + /// The information about a particular room. case roomDetails(roomID: String) + /// The profile of a member within the current room. + /// (This can be specialised into 2 routes when we support user permalinks). case roomMemberDetails(userID: String) + /// An Element Call link generated outside of a chat room. case genericCallLink(url: URL) + /// The settings screen. case settings + /// The setting screen for key backup. case chatBackupSettings } diff --git a/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift index 918604776..fee3c4a8b 100644 --- a/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift +++ b/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift @@ -109,13 +109,24 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol { case .room(let roomID): guard roomID == roomProxy.id else { fatalError("Navigation route doesn't belong to this room flow.") } stateMachine.tryEvent(.presentRoom, userInfo: EventUserInfo(animated: animated)) + case .childRoom(let roomID): + if case .presentingChild = stateMachine.state, let childRoomFlowCoordinator { + childRoomFlowCoordinator.handleAppRoute(appRoute, animated: animated) + } else { + stateMachine.tryEvent(.presentChildRoom(roomID: roomID), userInfo: EventUserInfo(animated: animated)) + } case .roomDetails(let roomID): guard roomID == roomProxy.id else { fatalError("Navigation route doesn't belong to this room flow.") } stateMachine.tryEvent(.presentRoomDetails, userInfo: EventUserInfo(animated: animated)) case .roomList: stateMachine.tryEvent(.dismissRoom, userInfo: EventUserInfo(animated: animated)) case .roomMemberDetails(let userID): - stateMachine.tryEvent(.presentRoomMemberDetails(userID: userID), userInfo: EventUserInfo(animated: animated)) + // Always assume this will be presented on the child, external permalinks to a user aren't for a room member. + if case .presentingChild = stateMachine.state, let childRoomFlowCoordinator { + childRoomFlowCoordinator.handleAppRoute(appRoute, animated: animated) + } else { + stateMachine.tryEvent(.presentRoomMemberDetails(userID: userID), userInfo: EventUserInfo(animated: animated)) + } case .genericCallLink, .oidcCallback, .settings, .chatBackupSettings: break } @@ -136,7 +147,7 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol { switch (fromState, event) { case (_, .presentRoom): return .room - case (.room, .dismissRoom): + case (_, .dismissRoom): return .complete case (.initial, .presentRoomDetails): @@ -145,8 +156,6 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol { return .roomDetails(isRoot: false) case (.roomDetails, .dismissRoomDetails): return .room - case (.roomDetails, .dismissRoom): - return .complete case (.roomDetails, .presentRoomDetailsEditScreen): return .roomDetailsEditScreen @@ -254,7 +263,7 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol { switch (context.fromState, context.event, context.toState) { case (_, .presentRoom, .room): Task { await self.presentRoom(animated: animated) } - case (.room, .dismissRoom, .complete): + case (_, .dismissRoom, .complete): dismissFlow(animated: animated) case (.initial, .presentRoomDetails, .roomDetails(let isRoot)), @@ -263,8 +272,6 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol { Task { await self.presentRoomDetails(isRoot: isRoot, animated: animated) } case (.roomDetails, .dismissRoomDetails, .room): break - case (.roomDetails, .dismissRoom, .complete): - dismissFlow(animated: animated) case (.roomDetails, .presentRoomDetailsEditScreen, .roomDetailsEditScreen): presentRoomDetailsEditScreen() diff --git a/ElementX/Sources/FlowCoordinators/UserSessionFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/UserSessionFlowCoordinator.swift index f7dda17f2..6b56584df 100644 --- a/ElementX/Sources/FlowCoordinators/UserSessionFlowCoordinator.swift +++ b/ElementX/Sources/FlowCoordinators/UserSessionFlowCoordinator.swift @@ -179,6 +179,12 @@ class UserSessionFlowCoordinator: FlowCoordinatorProtocol { switch appRoute { case .room(let roomID): Task { await self.handleRoomRoute(roomID: roomID, animated: animated) } + case .childRoom(let roomID): + if let roomFlowCoordinator { + roomFlowCoordinator.handleAppRoute(appRoute, animated: animated) + } else { + Task { await self.handleRoomRoute(roomID: roomID, animated: animated) } + } case .roomDetails(let roomID): if stateMachine.state.selectedRoomID == roomID { roomFlowCoordinator?.handleAppRoute(appRoute, animated: animated) diff --git a/ElementX/Sources/UITests/UITestsAppCoordinator.swift b/ElementX/Sources/UITests/UITestsAppCoordinator.swift index c537d970d..c4eadd824 100644 --- a/ElementX/Sources/UITests/UITestsAppCoordinator.swift +++ b/ElementX/Sources/UITests/UITestsAppCoordinator.swift @@ -66,7 +66,7 @@ class UITestsAppCoordinator: AppCoordinatorProtocol, WindowManagerDelegate { navigationRootCoordinator.toPresentable() } - func handleDeepLink(_ url: URL) -> Bool { + func handleDeepLink(_ url: URL, isExternalURL: Bool) -> Bool { fatalError("Not implemented.") } diff --git a/ElementX/Sources/UnitTests/UnitTestsAppCoordinator.swift b/ElementX/Sources/UnitTests/UnitTestsAppCoordinator.swift index d8ee1972a..6940f5b2c 100644 --- a/ElementX/Sources/UnitTests/UnitTestsAppCoordinator.swift +++ b/ElementX/Sources/UnitTests/UnitTestsAppCoordinator.swift @@ -38,7 +38,7 @@ class UnitTestsAppCoordinator: AppCoordinatorProtocol { AnyView(ProgressView("Running Unit Tests")) } - func handleDeepLink(_ url: URL) -> Bool { + func handleDeepLink(_ url: URL, isExternalURL: Bool) -> Bool { fatalError("Not implemented.") } } diff --git a/UnitTests/Sources/RoomFlowCoordinatorTests.swift b/UnitTests/Sources/RoomFlowCoordinatorTests.swift index 88503e498..487558f40 100644 --- a/UnitTests/Sources/RoomFlowCoordinatorTests.swift +++ b/UnitTests/Sources/RoomFlowCoordinatorTests.swift @@ -72,6 +72,30 @@ class RoomFlowCoordinatorTests: XCTestCase { XCTAssert(navigationStackCoordinator.stackCoordinators.first is RoomDetailsScreenCoordinator) } + func testChildRoomFlow() async throws { + await setupViewModel() + + try await process(route: .room(roomID: "1")) + XCTAssert(navigationStackCoordinator.rootCoordinator is RoomScreenCoordinator) + XCTAssertEqual(navigationStackCoordinator.stackCoordinators.count, 0) + + try await process(route: .childRoom(roomID: "2")) + try await Task.sleep(for: .milliseconds(100)) + XCTAssertEqual(navigationStackCoordinator.stackCoordinators.count, 1) + XCTAssert(navigationStackCoordinator.stackCoordinators.first is RoomScreenCoordinator) + + try await process(route: .childRoom(roomID: "3")) + try await Task.sleep(for: .milliseconds(100)) + XCTAssertEqual(navigationStackCoordinator.stackCoordinators.count, 2) + XCTAssert(navigationStackCoordinator.stackCoordinators.first is RoomScreenCoordinator) + XCTAssert(navigationStackCoordinator.stackCoordinators.last is RoomScreenCoordinator) + + try await process(route: .roomList, expectedAction: .finished) + XCTAssertNil(navigationStackCoordinator.rootCoordinator) + XCTAssertEqual(navigationStackCoordinator.stackCoordinators.count, 0) + } + + /// Tests the child flow teardown in isolation of it's parent. func testChildFlowTearDown() async throws { await setupViewModel(asChildFlow: true) navigationStackCoordinator.setRootCoordinator(BlankFormCoordinator()) @@ -90,6 +114,24 @@ class RoomFlowCoordinatorTests: XCTestCase { XCTAssertTrue(navigationStackCoordinator.stackCoordinators.last is RoomDetailsScreenCoordinator, "A child room flow should leave its parent to clean up the stack.") } + func testChildRoomMemberDetails() async throws { + await setupViewModel() + + try await process(route: .room(roomID: "1")) + XCTAssert(navigationStackCoordinator.rootCoordinator is RoomScreenCoordinator) + XCTAssertEqual(navigationStackCoordinator.stackCoordinators.count, 0) + + try await process(route: .childRoom(roomID: "2")) + try await Task.sleep(for: .milliseconds(100)) + XCTAssertEqual(navigationStackCoordinator.stackCoordinators.count, 1) + XCTAssert(navigationStackCoordinator.stackCoordinators.first is RoomScreenCoordinator) + + try await process(route: .roomMemberDetails(userID: RoomMemberProxyMock.mockMe.userID)) + XCTAssertEqual(navigationStackCoordinator.stackCoordinators.count, 2) + XCTAssert(navigationStackCoordinator.stackCoordinators.first is RoomScreenCoordinator) + XCTAssert(navigationStackCoordinator.stackCoordinators.last is RoomMemberDetailsScreenCoordinator) + } + // MARK: - Private private func process(route: AppRoute) async throws {