pr suggestions and some code improvements

This commit is contained in:
Mauro Romito
2025-10-02 15:39:36 +02:00
committed by Mauro
parent 8f31671e10
commit c4aebc47ee
6 changed files with 60 additions and 58 deletions
@@ -68,7 +68,7 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol {
private var roomProxy: JoinedRoomProxyProtocol! private var roomProxy: JoinedRoomProxyProtocol!
private var roomScreenCoordinator: RoomScreenCoordinator? private var roomScreenCoordinator: RoomScreenCoordinator?
private var childThreads: [ThreadTimelineScreenCoordinator] = [] private var childThreadScreenCoordinators: [ThreadTimelineScreenCoordinator] = []
private weak var joinRoomScreenCoordinator: JoinRoomScreenCoordinator? private weak var joinRoomScreenCoordinator: JoinRoomScreenCoordinator?
// periphery:ignore - used to avoid deallocation // periphery:ignore - used to avoid deallocation
@@ -167,27 +167,27 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol {
showLoadingIndicator(delay: .seconds(0.5)) showLoadingIndicator(delay: .seconds(0.5))
Task { Task {
defer { hideLoadingIndicator() } defer { hideLoadingIndicator() }
switch await roomProxy.loadOrFetchEvent(for: eventID) { switch await roomProxy.loadOrFetchEventDetails(for: eventID) {
case .success(let event): case .success(let event):
if flowParameters.appSettings.threadsEnabled, let threadRootEventID = event.threadRootEventId() { if flowParameters.appSettings.threadsEnabled, let threadRootEventID = event.threadRootEventId() {
if case .thread(threadRootEventID: threadRootEventID, _) = stateMachine.state, let threadCoordinator = childThreads.last { if case .thread(threadRootEventID: threadRootEventID, _) = stateMachine.state, let threadCoordinator = childThreadScreenCoordinators.last {
threadCoordinator.focusOnEvent(eventID: eventID) threadCoordinator.focusOnEvent(eventID: eventID)
} else { } else {
// If we are showing the room timeline, we want to focus the thread root // If we are showing the room timeline, we want to focus the thread root.
if childThreads.isEmpty { if childThreadScreenCoordinators.isEmpty {
roomScreenCoordinator?.focusOnEvent(.init(eventID: threadRootEventID, shouldSetPin: false)) roomScreenCoordinator?.focusOnEvent(.init(eventID: threadRootEventID, shouldSetPin: false))
} }
stateMachine.tryEvent(.presentThread(threadRootEventID: threadRootEventID, focusEventID: eventID)) stateMachine.tryEvent(.presentThread(threadRootEventID: threadRootEventID, focusEventID: eventID))
} }
} else if !childThreads.isEmpty { } else if !childThreadScreenCoordinators.isEmpty {
// If we are showing a child thread, and we are navigating to a non threaded event // If we are showing a child thread and we are navigating to a non threaded event
// of the same room, we want to push the room on top of the thread // of the same room, we want to push the room on top of the thread.
stateMachine.tryEvent(.startChildFlow(roomID: roomID, via: via, entryPoint: .eventID(eventID)), userInfo: EventUserInfo(animated: animated)) stateMachine.tryEvent(.startChildFlow(roomID: roomID, via: via, entryPoint: .eventID(eventID)), userInfo: EventUserInfo(animated: animated))
} else { } else {
roomScreenCoordinator?.focusOnEvent(.init(eventID: eventID, shouldSetPin: false)) roomScreenCoordinator?.focusOnEvent(.init(eventID: eventID, shouldSetPin: false))
} }
case .failure: case .failure:
showError() showErrorIndicator()
} }
} }
} }
@@ -258,22 +258,24 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol {
switch room { switch room {
case .joined(let roomProxy): case .joined(let roomProxy):
await storeAndSubscribeToRoomProxy(roomProxy) await storeAndSubscribeToRoomProxy(roomProxy)
if case let .eventFocus(focusEvent) = presentationAction { guard case let .eventFocus(focusEvent) = presentationAction else {
switch await roomProxy.loadOrFetchEvent(for: focusEvent.eventID) { // If is not a focus event just handle the presentation action directly in `presentRoom`
case .success(let event): stateMachine.tryEvent(.presentRoom(presentationAction: presentationAction), userInfo: EventUserInfo(animated: animated))
guard flowParameters.appSettings.threadsEnabled, let threadRootEventID = event.threadRootEventId() else { return
break }
} // Otherwise check if the focussed event exists to handle a possible error or theaded event.
switch await roomProxy.loadOrFetchEventDetails(for: focusEvent.eventID) {
case .success(let event):
if flowParameters.appSettings.threadsEnabled, let threadRootEventID = event.threadRootEventId() {
stateMachine.tryEvent(.presentRoom(presentationAction: .eventFocus(.init(eventID: threadRootEventID, shouldSetPin: false))), userInfo: EventUserInfo(animated: animated)) stateMachine.tryEvent(.presentRoom(presentationAction: .eventFocus(.init(eventID: threadRootEventID, shouldSetPin: false))), userInfo: EventUserInfo(animated: animated))
stateMachine.tryEvent(.presentThread(threadRootEventID: threadRootEventID, focusEventID: focusEvent.eventID), userInfo: EventUserInfo(animated: false)) stateMachine.tryEvent(.presentThread(threadRootEventID: threadRootEventID, focusEventID: focusEvent.eventID), userInfo: EventUserInfo(animated: false))
return } else {
case .failure: stateMachine.tryEvent(.presentRoom(presentationAction: presentationAction), userInfo: EventUserInfo(animated: animated))
showError()
stateMachine.tryEvent(.presentRoom(presentationAction: nil), userInfo: EventUserInfo(animated: animated))
return
} }
case .failure:
showErrorIndicator()
stateMachine.tryEvent(.presentRoom(presentationAction: nil), userInfo: EventUserInfo(animated: animated))
} }
stateMachine.tryEvent(.presentRoom(presentationAction: presentationAction), userInfo: EventUserInfo(animated: animated))
default: default:
stateMachine.tryEvent(.presentJoinRoomScreen(via: via), userInfo: EventUserInfo(animated: animated)) stateMachine.tryEvent(.presentJoinRoomScreen(via: via), userInfo: EventUserInfo(animated: animated))
} }
@@ -737,10 +739,10 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol {
navigationStackCoordinator.push(coordinator, animated: animated) { [weak self] in navigationStackCoordinator.push(coordinator, animated: animated) { [weak self] in
guard let self else { return } guard let self else { return }
stateMachine.tryEvent(.dismissThread) stateMachine.tryEvent(.dismissThread)
childThreads.removeAll { $0 === coordinator } childThreadScreenCoordinators.removeAll { $0 === coordinator }
} }
childThreads.append(coordinator) childThreadScreenCoordinators.append(coordinator)
} }
private func presentJoinRoomScreen(via: [String], animated: Bool) { private func presentJoinRoomScreen(via: [String], animated: Bool) {
@@ -1625,7 +1627,7 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol {
flowParameters.userIndicatorController.retractIndicatorWithId(Self.loadingIndicatorID) flowParameters.userIndicatorController.retractIndicatorWithId(Self.loadingIndicatorID)
} }
private func showError() { private func showErrorIndicator() {
flowParameters.userIndicatorController.submitIndicator(.init(title: L10n.errorUnknown)) flowParameters.userIndicatorController.submitIndicator(.init(title: L10n.errorUnknown))
} }
} }
+2 -2
View File
@@ -97,11 +97,11 @@ extension ClientProxyMock {
roomForIdentifierClosure = { [weak self] identifier in roomForIdentifierClosure = { [weak self] identifier in
if let room = self?.roomSummaryProvider.roomListPublisher.value.first(where: { $0.id == identifier }) { if let room = self?.roomSummaryProvider.roomListPublisher.value.first(where: { $0.id == identifier }) {
let roomProxy = await JoinedRoomProxyMock(.init(id: room.id, name: room.name)) let roomProxy = await JoinedRoomProxyMock(.init(id: room.id, name: room.name))
roomProxy.loadOrFetchEventForReturnValue = .success(TimelineEventSDKMock()) roomProxy.loadOrFetchEventDetailsForReturnValue = .success(TimelineEventSDKMock())
return .joined(roomProxy) return .joined(roomProxy)
} else if let spaceRoomProxy = configuration.joinedSpaceRooms.first(where: { $0.id == identifier }) { } else if let spaceRoomProxy = configuration.joinedSpaceRooms.first(where: { $0.id == identifier }) {
let roomProxy = await JoinedRoomProxyMock(.init(id: spaceRoomProxy.id, name: spaceRoomProxy.name)) let roomProxy = await JoinedRoomProxyMock(.init(id: spaceRoomProxy.id, name: spaceRoomProxy.name))
roomProxy.loadOrFetchEventForReturnValue = .success(TimelineEventSDKMock()) roomProxy.loadOrFetchEventDetailsForReturnValue = .success(TimelineEventSDKMock())
return .joined(roomProxy) return .joined(roomProxy)
} else { } else {
return nil return nil
@@ -6483,17 +6483,17 @@ class JoinedRoomProxyMock: JoinedRoomProxyProtocol, @unchecked Sendable {
return threadTimelineEventIDReturnValue return threadTimelineEventIDReturnValue
} }
} }
//MARK: - loadOrFetchEvent //MARK: - loadOrFetchEventDetails
var loadOrFetchEventForUnderlyingCallsCount = 0 var loadOrFetchEventDetailsForUnderlyingCallsCount = 0
var loadOrFetchEventForCallsCount: Int { var loadOrFetchEventDetailsForCallsCount: Int {
get { get {
if Thread.isMainThread { if Thread.isMainThread {
return loadOrFetchEventForUnderlyingCallsCount return loadOrFetchEventDetailsForUnderlyingCallsCount
} else { } else {
var returnValue: Int? = nil var returnValue: Int? = nil
DispatchQueue.main.sync { DispatchQueue.main.sync {
returnValue = loadOrFetchEventForUnderlyingCallsCount returnValue = loadOrFetchEventDetailsForUnderlyingCallsCount
} }
return returnValue! return returnValue!
@@ -6501,29 +6501,29 @@ class JoinedRoomProxyMock: JoinedRoomProxyProtocol, @unchecked Sendable {
} }
set { set {
if Thread.isMainThread { if Thread.isMainThread {
loadOrFetchEventForUnderlyingCallsCount = newValue loadOrFetchEventDetailsForUnderlyingCallsCount = newValue
} else { } else {
DispatchQueue.main.sync { DispatchQueue.main.sync {
loadOrFetchEventForUnderlyingCallsCount = newValue loadOrFetchEventDetailsForUnderlyingCallsCount = newValue
} }
} }
} }
} }
var loadOrFetchEventForCalled: Bool { var loadOrFetchEventDetailsForCalled: Bool {
return loadOrFetchEventForCallsCount > 0 return loadOrFetchEventDetailsForCallsCount > 0
} }
var loadOrFetchEventForReceivedEventID: String? var loadOrFetchEventDetailsForReceivedEventID: String?
var loadOrFetchEventForReceivedInvocations: [String] = [] var loadOrFetchEventDetailsForReceivedInvocations: [String] = []
var loadOrFetchEventForUnderlyingReturnValue: Result<TimelineEvent, RoomProxyError>! var loadOrFetchEventDetailsForUnderlyingReturnValue: Result<TimelineEvent, RoomProxyError>!
var loadOrFetchEventForReturnValue: Result<TimelineEvent, RoomProxyError>! { var loadOrFetchEventDetailsForReturnValue: Result<TimelineEvent, RoomProxyError>! {
get { get {
if Thread.isMainThread { if Thread.isMainThread {
return loadOrFetchEventForUnderlyingReturnValue return loadOrFetchEventDetailsForUnderlyingReturnValue
} else { } else {
var returnValue: Result<TimelineEvent, RoomProxyError>? = nil var returnValue: Result<TimelineEvent, RoomProxyError>? = nil
DispatchQueue.main.sync { DispatchQueue.main.sync {
returnValue = loadOrFetchEventForUnderlyingReturnValue returnValue = loadOrFetchEventDetailsForUnderlyingReturnValue
} }
return returnValue! return returnValue!
@@ -6531,26 +6531,26 @@ class JoinedRoomProxyMock: JoinedRoomProxyProtocol, @unchecked Sendable {
} }
set { set {
if Thread.isMainThread { if Thread.isMainThread {
loadOrFetchEventForUnderlyingReturnValue = newValue loadOrFetchEventDetailsForUnderlyingReturnValue = newValue
} else { } else {
DispatchQueue.main.sync { DispatchQueue.main.sync {
loadOrFetchEventForUnderlyingReturnValue = newValue loadOrFetchEventDetailsForUnderlyingReturnValue = newValue
} }
} }
} }
} }
var loadOrFetchEventForClosure: ((String) async -> Result<TimelineEvent, RoomProxyError>)? var loadOrFetchEventDetailsForClosure: ((String) async -> Result<TimelineEvent, RoomProxyError>)?
func loadOrFetchEvent(for eventID: String) async -> Result<TimelineEvent, RoomProxyError> { func loadOrFetchEventDetails(for eventID: String) async -> Result<TimelineEvent, RoomProxyError> {
loadOrFetchEventForCallsCount += 1 loadOrFetchEventDetailsForCallsCount += 1
loadOrFetchEventForReceivedEventID = eventID loadOrFetchEventDetailsForReceivedEventID = eventID
DispatchQueue.main.async { DispatchQueue.main.async {
self.loadOrFetchEventForReceivedInvocations.append(eventID) self.loadOrFetchEventDetailsForReceivedInvocations.append(eventID)
} }
if let loadOrFetchEventForClosure = loadOrFetchEventForClosure { if let loadOrFetchEventDetailsForClosure = loadOrFetchEventDetailsForClosure {
return await loadOrFetchEventForClosure(eventID) return await loadOrFetchEventDetailsForClosure(eventID)
} else { } else {
return loadOrFetchEventForReturnValue return loadOrFetchEventDetailsForReturnValue
} }
} }
//MARK: - messageFilteredTimeline //MARK: - messageFilteredTimeline
@@ -190,7 +190,7 @@ class JoinedRoomProxy: JoinedRoomProxyProtocol {
} }
} }
func loadOrFetchEvent(for eventID: String) async -> Result<TimelineEvent, RoomProxyError> { func loadOrFetchEventDetails(for eventID: String) async -> Result<TimelineEvent, RoomProxyError> {
do { do {
let event = try await room.loadOrFetchEvent(eventId: eventID) let event = try await room.loadOrFetchEvent(eventId: eventID)
return .success(event) return .success(event)
@@ -97,7 +97,7 @@ protocol JoinedRoomProxyProtocol: RoomProxyProtocol {
func threadTimeline(eventID: String) async -> Result<TimelineProxyProtocol, RoomProxyError> func threadTimeline(eventID: String) async -> Result<TimelineProxyProtocol, RoomProxyError>
func loadOrFetchEvent(for eventID: String) async -> Result<TimelineEvent, RoomProxyError> func loadOrFetchEventDetails(for eventID: String) async -> Result<TimelineEvent, RoomProxyError>
func messageFilteredTimeline(focus: TimelineFocus, func messageFilteredTimeline(focus: TimelineFocus,
allowedMessageTypes: [TimelineAllowedMessageType], allowedMessageTypes: [TimelineAllowedMessageType],
@@ -235,7 +235,7 @@ class RoomFlowCoordinatorTests: XCTestCase {
var mockedEvent = TimelineEventSDKMock() var mockedEvent = TimelineEventSDKMock()
mockedEvent.threadRootEventIdReturnValue = "1" mockedEvent.threadRootEventIdReturnValue = "1"
roomProxy.loadOrFetchEventForReturnValue = .success(mockedEvent) roomProxy.loadOrFetchEventDetailsForReturnValue = .success(mockedEvent)
clientProxy.roomForIdentifierClosure = { _ in clientProxy.roomForIdentifierClosure = { _ in
.joined(roomProxy) .joined(roomProxy)
@@ -258,7 +258,7 @@ class RoomFlowCoordinatorTests: XCTestCase {
// From the thread screen, navigate to another threaded event in the same room, but in a different thread. // From the thread screen, navigate to another threaded event in the same room, but in a different thread.
mockedEvent = TimelineEventSDKMock() mockedEvent = TimelineEventSDKMock()
mockedEvent.threadRootEventIdReturnValue = "4" mockedEvent.threadRootEventIdReturnValue = "4"
roomProxy.loadOrFetchEventForReturnValue = .success(mockedEvent) roomProxy.loadOrFetchEventDetailsForReturnValue = .success(mockedEvent)
try await process(route: .childEvent(eventID: "5", roomID: "1", via: [])) try await process(route: .childEvent(eventID: "5", roomID: "1", via: []))
XCTAssert(navigationStackCoordinator.rootCoordinator is RoomScreenCoordinator) XCTAssert(navigationStackCoordinator.rootCoordinator is RoomScreenCoordinator)
XCTAssertEqual(navigationStackCoordinator.stackCoordinators.count, 2) XCTAssertEqual(navigationStackCoordinator.stackCoordinators.count, 2)
@@ -274,7 +274,7 @@ class RoomFlowCoordinatorTests: XCTestCase {
mockedEvent = TimelineEventSDKMock() mockedEvent = TimelineEventSDKMock()
mockedEvent.threadRootEventIdReturnValue = "1" mockedEvent.threadRootEventIdReturnValue = "1"
roomProxy.loadOrFetchEventForReturnValue = .success(mockedEvent) roomProxy.loadOrFetchEventDetailsForReturnValue = .success(mockedEvent)
clientProxy.roomForIdentifierClosure = { _ in clientProxy.roomForIdentifierClosure = { _ in
.joined(roomProxy) .joined(roomProxy)
@@ -291,7 +291,7 @@ class RoomFlowCoordinatorTests: XCTestCase {
// From the thread screen, navigate to an event of the same room that is not threaded // From the thread screen, navigate to an event of the same room that is not threaded
mockedEvent = TimelineEventSDKMock() mockedEvent = TimelineEventSDKMock()
mockedEvent.threadRootEventIdReturnValue = nil mockedEvent.threadRootEventIdReturnValue = nil
roomProxy.loadOrFetchEventForReturnValue = .success(mockedEvent) roomProxy.loadOrFetchEventDetailsForReturnValue = .success(mockedEvent)
try await process(route: .childEvent(eventID: "3", roomID: "2", via: [])) try await process(route: .childEvent(eventID: "3", roomID: "2", via: []))
XCTAssert(navigationStackCoordinator.rootCoordinator is RoomScreenCoordinator) XCTAssert(navigationStackCoordinator.rootCoordinator is RoomScreenCoordinator)