From 0ff1c123aa16d516c486a27ed34236d6ccf5831f Mon Sep 17 00:00:00 2001 From: Doug <6060466+pixlwave@users.noreply.github.com> Date: Tue, 9 Apr 2024 11:11:14 +0100 Subject: [PATCH] Fix child presentation of the same room. (#2669) --- .../RoomFlowCoordinator.swift | 4 ++- .../Sources/RoomFlowCoordinatorTests.swift | 32 ++++++++++++++++--- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift index fee3c4a8b..311128650 100644 --- a/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift +++ b/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift @@ -112,8 +112,10 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol { case .childRoom(let roomID): if case .presentingChild = stateMachine.state, let childRoomFlowCoordinator { childRoomFlowCoordinator.handleAppRoute(appRoute, animated: animated) - } else { + } else if roomID != roomProxy.id { stateMachine.tryEvent(.presentChildRoom(roomID: roomID), userInfo: EventUserInfo(animated: animated)) + } else { + MXLog.info("Ignoring presentation of the same room as a child of this one.") } case .roomDetails(let roomID): guard roomID == roomProxy.id else { fatalError("Navigation route doesn't belong to this room flow.") } diff --git a/UnitTests/Sources/RoomFlowCoordinatorTests.swift b/UnitTests/Sources/RoomFlowCoordinatorTests.swift index 487558f40..afff9847a 100644 --- a/UnitTests/Sources/RoomFlowCoordinatorTests.swift +++ b/UnitTests/Sources/RoomFlowCoordinatorTests.swift @@ -80,12 +80,10 @@ class RoomFlowCoordinatorTests: XCTestCase { 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) @@ -122,7 +120,6 @@ class RoomFlowCoordinatorTests: XCTestCase { 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) @@ -132,11 +129,38 @@ class RoomFlowCoordinatorTests: XCTestCase { XCTAssert(navigationStackCoordinator.stackCoordinators.last is RoomMemberDetailsScreenCoordinator) } + func testChildRoomIgnoresDirectDuplicate() 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: "1")) + XCTAssertEqual(navigationStackCoordinator.stackCoordinators.count, 0, + "A room flow shouldn't present a direct child for the same room.") + + try await process(route: .childRoom(roomID: "2")) + XCTAssertEqual(navigationStackCoordinator.stackCoordinators.count, 1) + XCTAssert(navigationStackCoordinator.stackCoordinators.first is RoomScreenCoordinator) + + try await process(route: .childRoom(roomID: "1")) + XCTAssertEqual(navigationStackCoordinator.stackCoordinators.count, 2, + "Presenting the same room multiple times should be allowed when it's not a direct child of itself.") + XCTAssert(navigationStackCoordinator.stackCoordinators.first is RoomScreenCoordinator) + XCTAssert(navigationStackCoordinator.stackCoordinators.last is RoomScreenCoordinator) + } + // MARK: - Private private func process(route: AppRoute) async throws { roomFlowCoordinator.handleAppRoute(route, animated: true) - await Task.yield() + if case .childRoom = route { + // A single yield isn't enough when creating the new flow coordinator. + try await Task.sleep(for: .milliseconds(100)) + } else { + await Task.yield() + } } private func process(route: AppRoute, expectedAction: RoomFlowCoordinatorAction) async throws {