From af5b670bf308d91c56eeaf31e3cf17e2ef100957 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Thu, 26 Jun 2025 16:34:34 +0300 Subject: [PATCH] Be more lenient with the power levels as they can still be missing at the time the various screens are created e.g. after accepting an invite. The correct solution is to subscribe to update and update the UI accordingly when receiving them. --- .../KnockRequestsListScreenViewModel.swift | 15 +++--- .../RoomDetailsEditScreenViewModel.swift | 16 ++++-- .../RoomDetailsScreenViewModel.swift | 4 +- .../RoomMembersListScreenViewModel.swift | 9 ++-- ...omRolesAndPermissionsScreenViewModel.swift | 5 +- .../CompletionSuggestionService.swift | 19 ++++++- .../RoomScreen/RoomScreenViewModel.swift | 49 +++++++------------ .../ThreadTimelineScreenViewModel.swift | 23 ++++----- .../Screens/Timeline/TimelineViewModel.swift | 35 ++++++------- .../Sources/RoomScreenViewModelTests.swift | 36 +++++++------- 10 files changed, 108 insertions(+), 103 deletions(-) diff --git a/ElementX/Sources/Screens/KnockRequestsListScreen/KnockRequestsListScreenViewModel.swift b/ElementX/Sources/Screens/KnockRequestsListScreen/KnockRequestsListScreenViewModel.swift index 4ab609757..40640eb7f 100644 --- a/ElementX/Sources/Screens/KnockRequestsListScreen/KnockRequestsListScreenViewModel.swift +++ b/ElementX/Sources/Screens/KnockRequestsListScreen/KnockRequestsListScreenViewModel.swift @@ -26,7 +26,7 @@ class KnockRequestsListScreenViewModel: KnockRequestsListScreenViewModelType, Kn self.userIndicatorController = userIndicatorController super.init(initialViewState: KnockRequestsListScreenViewState(), mediaProvider: mediaProvider) - updateRoomInfo(roomInfo: roomProxy.infoPublisher.value) + updateRoomInfo(roomProxy.infoPublisher.value) setupSubscriptions() } @@ -185,7 +185,7 @@ class KnockRequestsListScreenViewModel: KnockRequestsListScreenViewModelType, Kn roomProxy.infoPublisher .receive(on: DispatchQueue.main) .sink { [weak self] roomInfo in - self?.updateRoomInfo(roomInfo: roomInfo) + self?.updateRoomInfo(roomInfo) } .store(in: &cancellables) @@ -210,7 +210,7 @@ class KnockRequestsListScreenViewModel: KnockRequestsListScreenViewModelType, Kn .store(in: &cancellables) } - private func updateRoomInfo(roomInfo: RoomInfoProxyProtocol) { + private func updateRoomInfo(_ roomInfo: RoomInfoProxyProtocol) { switch roomInfo.joinRule { case .knock, .knockRestricted: state.isKnockableRoom = true @@ -218,10 +218,11 @@ class KnockRequestsListScreenViewModel: KnockRequestsListScreenViewModelType, Kn state.isKnockableRoom = false } - guard let powerLevels = roomProxy.infoPublisher.value.powerLevels else { fatalError("Missing room power levels") } - state.canAccept = powerLevels.canOwnUserInvite() - state.canDecline = powerLevels.canOwnUserKick() - state.canBan = powerLevels.canOwnUserBan() + if let powerLevels = roomProxy.infoPublisher.value.powerLevels { + state.canAccept = powerLevels.canOwnUserInvite() + state.canDecline = powerLevels.canOwnUserKick() + state.canBan = powerLevels.canOwnUserBan() + } } private static let loadingIndicatorIdentifier = "\(KnockRequestsListScreenViewModel.self)-Loading" diff --git a/ElementX/Sources/Screens/RoomDetailsEditScreen/RoomDetailsEditScreenViewModel.swift b/ElementX/Sources/Screens/RoomDetailsEditScreen/RoomDetailsEditScreenViewModel.swift index 2d9dda6af..5151f3f46 100644 --- a/ElementX/Sources/Screens/RoomDetailsEditScreen/RoomDetailsEditScreenViewModel.swift +++ b/ElementX/Sources/Screens/RoomDetailsEditScreen/RoomDetailsEditScreenViewModel.swift @@ -39,6 +39,13 @@ class RoomDetailsEditScreenViewModel: RoomDetailsEditScreenViewModelType, RoomDe avatarURL: roomAvatar, bindings: .init(name: roomName ?? "", topic: roomTopic ?? "")), mediaProvider: mediaProvider) + roomProxy.infoPublisher + .receive(on: DispatchQueue.main) + .sink { [weak self] roomInfo in + self?.updateRoomInfo(roomInfo: roomInfo) + } + .store(in: &cancellables) + updateRoomInfo(roomInfo: roomProxy.infoPublisher.value) } @@ -87,10 +94,11 @@ class RoomDetailsEditScreenViewModel: RoomDetailsEditScreenViewModelType, RoomDe // MARK: - Private private func updateRoomInfo(roomInfo: RoomInfoProxyProtocol) { - guard let powerLevels = roomInfo.powerLevels else { fatalError("Missing room power levels") } - state.canEditAvatar = powerLevels.canOwnUser(sendStateEvent: .roomAvatar) - state.canEditName = powerLevels.canOwnUser(sendStateEvent: .roomName) - state.canEditTopic = powerLevels.canOwnUser(sendStateEvent: .roomTopic) + if let powerLevels = roomInfo.powerLevels { + state.canEditAvatar = powerLevels.canOwnUser(sendStateEvent: .roomAvatar) + state.canEditName = powerLevels.canOwnUser(sendStateEvent: .roomName) + state.canEditTopic = powerLevels.canOwnUser(sendStateEvent: .roomTopic) + } } private func saveRoomDetails() { diff --git a/ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenViewModel.swift b/ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenViewModel.swift index f84d4c50c..c30ce7f27 100644 --- a/ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenViewModel.swift +++ b/ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenViewModel.swift @@ -179,7 +179,7 @@ class RoomDetailsScreenViewModel: RoomDetailsScreenViewModelType, RoomDetailsScr private func setupRoomSubscription() { roomProxy.infoPublisher - .throttle(for: .milliseconds(200), scheduler: DispatchQueue.main, latest: true) + .receive(on: DispatchQueue.main) .sink { [weak self] roomInfo in self?.updateRoomInfo(roomInfo) } @@ -233,8 +233,6 @@ class RoomDetailsScreenViewModel: RoomDetailsScreenViewModelType, RoomDetailsScr state.canBanUsers = powerLevels.canOwnUserBan() state.canJoinCall = powerLevels.canOwnUserJoinCall() state.canEditRolesOrPermissions = powerLevels.suggestedRole(forUser: roomProxy.ownUserID) == .administrator - } else { - fatalError("Missing room power levels") } } diff --git a/ElementX/Sources/Screens/RoomMemberListScreen/RoomMembersListScreenViewModel.swift b/ElementX/Sources/Screens/RoomMemberListScreen/RoomMembersListScreenViewModel.swift index 350594197..e3f28fe9e 100644 --- a/ElementX/Sources/Screens/RoomMemberListScreen/RoomMembersListScreenViewModel.swift +++ b/ElementX/Sources/Screens/RoomMemberListScreen/RoomMembersListScreenViewModel.swift @@ -103,10 +103,11 @@ class RoomMembersListScreenViewModel: RoomMembersListScreenViewModelType, RoomMe bannedMembers: roomMembersDetails.bannedMembers, bindings: state.bindings) - guard let powerLevels = roomProxy.infoPublisher.value.powerLevels else { fatalError("Missing room power levels") } - self.state.canInviteUsers = powerLevels.canOwnUserInvite() - self.state.canKickUsers = powerLevels.canOwnUserKick() - self.state.canBanUsers = powerLevels.canOwnUserBan() + if let powerLevels = roomProxy.infoPublisher.value.powerLevels { + self.state.canInviteUsers = powerLevels.canOwnUserInvite() + self.state.canKickUsers = powerLevels.canOwnUserKick() + self.state.canBanUsers = powerLevels.canOwnUserBan() + } } } diff --git a/ElementX/Sources/Screens/RoomRolesAndPermissionsScreen/RoomRolesAndPermissionsScreenViewModel.swift b/ElementX/Sources/Screens/RoomRolesAndPermissionsScreen/RoomRolesAndPermissionsScreenViewModel.swift index e2db6b0d8..427494c04 100644 --- a/ElementX/Sources/Screens/RoomRolesAndPermissionsScreen/RoomRolesAndPermissionsScreenViewModel.swift +++ b/ElementX/Sources/Screens/RoomRolesAndPermissionsScreen/RoomRolesAndPermissionsScreenViewModel.swift @@ -114,8 +114,9 @@ class RoomRolesAndPermissionsScreenViewModel: RoomRolesAndPermissionsScreenViewM // MARK: - Permissions private func updateRoomInfo(roomInfo: RoomInfoProxyProtocol) { - guard let powerLevels = roomInfo.powerLevels else { fatalError("Missing room power levels") } - state.permissions = .init(powerLevels: powerLevels.values) + if let powerLevels = roomInfo.powerLevels { + state.permissions = .init(powerLevels: powerLevels.values) + } } private func editPermissions(group: RoomRolesAndPermissionsScreenPermissionsGroup) { diff --git a/ElementX/Sources/Screens/RoomScreen/ComposerToolbar/CompletionSuggestionService.swift b/ElementX/Sources/Screens/RoomScreen/ComposerToolbar/CompletionSuggestionService.swift index baf75bf3b..9cf01599a 100644 --- a/ElementX/Sources/Screens/RoomScreen/ComposerToolbar/CompletionSuggestionService.swift +++ b/ElementX/Sources/Screens/RoomScreen/ComposerToolbar/CompletionSuggestionService.swift @@ -19,10 +19,13 @@ private enum SuggestionTriggerRegex { final class CompletionSuggestionService: CompletionSuggestionServiceProtocol { private let roomProxy: JoinedRoomProxyProtocol private var canMentionAllUsers = false + private(set) var suggestionsPublisher: AnyPublisher<[SuggestionItem], Never> = Empty().eraseToAnyPublisher() private let suggestionTriggerSubject = CurrentValueSubject(nil) + private var cancellables = Set() + init(roomProxy: JoinedRoomProxyProtocol, roomListPublisher: AnyPublisher<[RoomSummary], Never>) { self.roomProxy = roomProxy @@ -47,8 +50,14 @@ final class CompletionSuggestionService: CompletionSuggestionServiceProtocol { self?.suggestionTriggerSubject.value != nil ? .milliseconds(500) : .milliseconds(0) } - guard let powerLevels = roomProxy.infoPublisher.value.powerLevels else { fatalError("Missing room power levels") } - canMentionAllUsers = powerLevels.canOwnUserTriggerRoomNotification() + roomProxy.infoPublisher + .receive(on: DispatchQueue.main) + .sink { [weak self] roomInfo in + self?.updateRoomInfo(roomInfo) + } + .store(in: &cancellables) + + updateRoomInfo(roomProxy.infoPublisher.value) } func processTextMessage(_ textMessage: String, selectedRange: NSRange) { @@ -61,6 +70,12 @@ final class CompletionSuggestionService: CompletionSuggestionServiceProtocol { // MARK: - Private + private func updateRoomInfo(_ roomInfo: RoomInfoProxyProtocol) { + if let powerLevels = roomProxy.infoPublisher.value.powerLevels { + canMentionAllUsers = powerLevels.canOwnUserTriggerRoomNotification() + } + } + private func membersSuggestions(suggestionTrigger: SuggestionTrigger, members: [RoomMemberProxyProtocol], ownUserID: String) -> [SuggestionItem] { diff --git a/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift b/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift index 27104eef3..ea95ac719 100644 --- a/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift +++ b/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift @@ -77,12 +77,12 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol super.init(initialViewState: appHooks.roomScreenHook.update(viewState), mediaProvider: mediaProvider) + updateRoomInfo(roomProxy.infoPublisher.value) + setupSubscriptions(ongoingCallRoomIDPublisher: ongoingCallRoomIDPublisher) + Task { - await handleRoomInfoUpdate(roomProxy.infoPublisher.value) await updateVerificationBadge() } - - setupSubscriptions(ongoingCallRoomIDPublisher: ongoingCallRoomIDPublisher) } override func process(viewAction: RoomScreenViewAction) { @@ -157,31 +157,14 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol appSettings.$knockingEnabled .weakAssign(to: \.state.isKnockingEnabled, on: self) .store(in: &cancellables) - - let roomInfoSubscription = roomProxy - .infoPublisher - - roomInfoSubscription - .throttle(for: .seconds(1), scheduler: DispatchQueue.main, latest: true) + + roomProxy.infoPublisher + .receive(on: DispatchQueue.main) .sink { [weak self] roomInfo in - guard let self else { return } - state.roomTitle = roomInfo.displayName ?? roomProxy.id - state.roomAvatar = roomInfo.avatar - state.hasOngoingCall = roomInfo.hasRoomCall + self?.updateRoomInfo(roomInfo) } .store(in: &cancellables) - Task { [weak self] in - for await roomInfo in roomInfoSubscription.receive(on: DispatchQueue.main).values { - guard !Task.isCancelled else { - return - } - - await self?.handleRoomInfoUpdate(roomInfo) - } - } - .store(in: &cancellables) - let identityStatusChangesPublisher = roomProxy.identityStatusChangesPublisher.receive(on: DispatchQueue.main) Task { [weak self] in @@ -332,7 +315,10 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol } } - private func handleRoomInfoUpdate(_ roomInfo: RoomInfoProxyProtocol) async { + private func updateRoomInfo(_ roomInfo: RoomInfoProxyProtocol) { + state.roomTitle = roomInfo.displayName ?? roomProxy.id + state.roomAvatar = roomInfo.avatar + state.hasOngoingCall = roomInfo.hasRoomCall state.hasSuccessor = roomInfo.successor != nil let pinnedEventIDs = roomInfo.pinnedEventIDs @@ -348,12 +334,13 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol state.isKnockableRoom = false } - guard let powerLevels = roomInfo.powerLevels else { fatalError("Missing room power levels") } - state.canSendMessage = powerLevels.canOwnUser(sendMessage: .roomMessage) - state.canJoinCall = powerLevels.canOwnUserJoinCall() - state.canAcceptKnocks = powerLevels.canOwnUserInvite() - state.canDeclineKnocks = powerLevels.canOwnUserKick() - state.canBan = powerLevels.canOwnUserBan() + if let powerLevels = roomInfo.powerLevels { + state.canSendMessage = powerLevels.canOwnUser(sendMessage: .roomMessage) + state.canJoinCall = powerLevels.canOwnUserJoinCall() + state.canAcceptKnocks = powerLevels.canOwnUserInvite() + state.canDeclineKnocks = powerLevels.canOwnUserKick() + state.canBan = powerLevels.canOwnUserBan() + } } private func setupPinnedEventsTimelineItemProviderIfNeeded() { diff --git a/ElementX/Sources/Screens/ThreadTimelineScreen/ThreadTimelineScreenViewModel.swift b/ElementX/Sources/Screens/ThreadTimelineScreen/ThreadTimelineScreenViewModel.swift index 1a808dfe2..128fb6c16 100644 --- a/ElementX/Sources/Screens/ThreadTimelineScreen/ThreadTimelineScreenViewModel.swift +++ b/ElementX/Sources/Screens/ThreadTimelineScreen/ThreadTimelineScreenViewModel.swift @@ -23,18 +23,14 @@ class ThreadTimelineScreenViewModel: ThreadTimelineScreenViewModelType, ThreadTi super.init(initialViewState: ThreadTimelineScreenViewState()) - Task { [weak self] in - for await roomInfo in roomProxy.infoPublisher.receive(on: DispatchQueue.main).values { - guard !Task.isCancelled else { - return - } - - self?.handleRoomInfoUpdate(roomInfo) + roomProxy.infoPublisher + .receive(on: DispatchQueue.main) + .sink { [weak self] roomInfo in + self?.updateRoomInfo(roomInfo) } - } - .store(in: &cancellables) + .store(in: &cancellables) - handleRoomInfoUpdate(roomProxy.infoPublisher.value) + updateRoomInfo(roomProxy.infoPublisher.value) } // MARK: - Public @@ -62,8 +58,9 @@ class ThreadTimelineScreenViewModel: ThreadTimelineScreenViewModelType, ThreadTi // MARK: - Private - private func handleRoomInfoUpdate(_ roomInfo: RoomInfoProxyProtocol) { - guard let powerLevels = roomInfo.powerLevels else { fatalError("Missing room power levels") } - state.canSendMessage = powerLevels.canOwnUser(sendMessage: .roomMessage) + private func updateRoomInfo(_ roomInfo: RoomInfoProxyProtocol) { + if let powerLevels = roomInfo.powerLevels { + state.canSendMessage = powerLevels.canOwnUser(sendMessage: .roomMessage) + } } } diff --git a/ElementX/Sources/Screens/Timeline/TimelineViewModel.swift b/ElementX/Sources/Screens/Timeline/TimelineViewModel.swift index 6616136bd..75782e83e 100644 --- a/ElementX/Sources/Screens/Timeline/TimelineViewModel.swift +++ b/ElementX/Sources/Screens/Timeline/TimelineViewModel.swift @@ -119,8 +119,6 @@ class TimelineViewModel: TimelineViewModelType, TimelineViewModelProtocol { setupSubscriptions() setupDirectRoomSubscriptionsIfNeeded() - updateRoomInfo(roomInfo: roomProxy.infoPublisher.value) - state.audioPlayerStateProvider = { [weak self] itemID -> AudioPlayerState? in guard let self else { return nil @@ -144,6 +142,7 @@ class TimelineViewModel: TimelineViewModelType, TimelineViewModelProtocol { state.timelineState.paginationState = timelineController.paginationState buildTimelineViews(timelineItems: timelineController.timelineItems) + updateRoomInfo(roomProxy.infoPublisher.value) updateMembers(roomProxy.membersPublisher.value) // Note: beware if we get to e.g. restore a reply / edit, @@ -402,16 +401,17 @@ class TimelineViewModel: TimelineViewModelType, TimelineViewModelProtocol { } } - private func updateRoomInfo(roomInfo: RoomInfoProxyProtocol) { + private func updateRoomInfo(_ roomInfo: RoomInfoProxyProtocol) { state.pinnedEventIDs = roomInfo.pinnedEventIDs - guard let powerLevels = roomInfo.powerLevels else { fatalError("Missing room power levels") } - state.canCurrentUserSendMessage = powerLevels.canOwnUser(sendMessage: .roomMessage) - state.canCurrentUserRedactOthers = powerLevels.canOwnUserRedactOther() - state.canCurrentUserRedactSelf = powerLevels.canOwnUserRedactOwn() - state.canCurrentUserPin = powerLevels.canOwnUserPinOrUnpin() - state.canCurrentUserKick = powerLevels.canOwnUserKick() - state.canCurrentUserBan = powerLevels.canOwnUserBan() + if let powerLevels = roomInfo.powerLevels { + state.canCurrentUserSendMessage = powerLevels.canOwnUser(sendMessage: .roomMessage) + state.canCurrentUserRedactOthers = powerLevels.canOwnUserRedactOther() + state.canCurrentUserRedactSelf = powerLevels.canOwnUserRedactOwn() + state.canCurrentUserPin = powerLevels.canOwnUserPinOrUnpin() + state.canCurrentUserKick = powerLevels.canOwnUserKick() + state.canCurrentUserBan = powerLevels.canOwnUserBan() + } } private func setupSubscriptions() { @@ -440,17 +440,12 @@ class TimelineViewModel: TimelineViewModelType, TimelineViewModelProtocol { } .store(in: &cancellables) - let roomInfoSubscription = roomProxy.infoPublisher - Task { [weak self] in - for await roomInfo in roomInfoSubscription.receive(on: DispatchQueue.main).values { - guard !Task.isCancelled else { - return - } - - self?.updateRoomInfo(roomInfo: roomInfo) + roomProxy.infoPublisher + .receive(on: DispatchQueue.main) + .sink { [weak self] roomInfo in + self?.updateRoomInfo(roomInfo) } - } - .store(in: &cancellables) + .store(in: &cancellables) setupAppSettingsSubscriptions() diff --git a/UnitTests/Sources/RoomScreenViewModelTests.swift b/UnitTests/Sources/RoomScreenViewModelTests.swift index b91fb9cb7..cff1688a9 100644 --- a/UnitTests/Sources/RoomScreenViewModelTests.swift +++ b/UnitTests/Sources/RoomScreenViewModelTests.swift @@ -165,15 +165,19 @@ class RoomScreenViewModelTests: XCTestCase { func testRoomInfoUpdate() async throws { var configuration = JoinedRoomProxyMockConfiguration(id: "TestID", name: "StartingName", avatarURL: nil, hasOngoingCall: false) - let infoSubject = CurrentValueSubject(RoomInfoProxyMock(configuration)) let roomProxyMock = JoinedRoomProxyMock(configuration) let powerLevelsMock = RoomPowerLevelsProxyMock(configuration: .init()) + powerLevelsMock.canUserJoinCallUserIDReturnValue = .success(false) + powerLevelsMock.canOwnUserJoinCallReturnValue = false roomProxyMock.powerLevelsReturnValue = .success(powerLevelsMock) - // setup the room proxy actions publisher - powerLevelsMock.canUserJoinCallUserIDReturnValue = .success(false) + let roomInfoProxyMock = RoomInfoProxyMock(configuration) + roomInfoProxyMock.powerLevels = powerLevelsMock + + let infoSubject = CurrentValueSubject(roomInfoProxyMock) roomProxyMock.underlyingInfoPublisher = infoSubject.asCurrentValuePublisher() + let viewModel = RoomScreenViewModel(clientProxy: ClientProxyMock(), roomProxy: roomProxyMock, initialSelectedPinnedEventID: nil, @@ -186,27 +190,25 @@ class RoomScreenViewModelTests: XCTestCase { userIndicatorController: ServiceLocator.shared.userIndicatorController) self.viewModel = viewModel - var deferred = deferFulfillment(viewModel.context.$viewState) { viewState in - viewState.roomTitle == "StartingName" && - viewState.roomAvatar == .room(id: "TestID", name: "StartingName", avatarURL: nil) && - !viewState.canJoinCall && - !viewState.hasOngoingCall - } - try await deferred.fulfill() - - configuration.name = "NewName" - configuration.avatarURL = .mockMXCAvatar - configuration.hasOngoingCall = true - powerLevelsMock.canUserJoinCallUserIDReturnValue = .success(true) - - deferred = deferFulfillment(viewModel.context.$viewState) { viewState in + XCTAssertEqual(viewModel.state.roomTitle, "StartingName") + XCTAssertEqual(viewModel.state.roomAvatar, .room(id: "TestID", name: "StartingName", avatarURL: nil)) + XCTAssertFalse(viewModel.state.canJoinCall) + XCTAssertFalse(viewModel.state.hasOngoingCall) + + let deferred = deferFulfillment(viewModel.context.$viewState) { viewState in viewState.roomTitle == "NewName" && viewState.roomAvatar == .room(id: "TestID", name: "NewName", avatarURL: .mockMXCAvatar) && viewState.canJoinCall && viewState.hasOngoingCall } + configuration.name = "NewName" + configuration.avatarURL = .mockMXCAvatar + configuration.hasOngoingCall = true + powerLevelsMock.canUserJoinCallUserIDReturnValue = .success(true) + infoSubject.send(RoomInfoProxyMock(configuration)) + try await deferred.fulfill() }