diff --git a/CHANGES.md b/CHANGES.md index e4d8ef4d8..3484fcf48 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,15 @@ +## Changes in 1.9.17 (2023-01-26) + +🙌 Improvements + +- Analytics: Ensure E2EE never tracks UnknownError ([#7304](https://github.com/vector-im/element-ios/pull/7304)) + +🐛 Bugfixes + +- Fix a deadlock when updating the summary of a room that has a voice broadcast. ([#7300](https://github.com/vector-im/element-ios/pull/7300)) +- Space Switcher: Fix a bug where the avatars would all be the same. ([#7305](https://github.com/vector-im/element-ios/issues/7305)) + + ## Changes in 1.9.16 (2023-01-24) ✨ Features diff --git a/Config/AppVersion.xcconfig b/Config/AppVersion.xcconfig index 210603b23..250f5e373 100644 --- a/Config/AppVersion.xcconfig +++ b/Config/AppVersion.xcconfig @@ -15,5 +15,5 @@ // // Version -MARKETING_VERSION = 1.9.17 -CURRENT_PROJECT_VERSION = 1.9.17 +MARKETING_VERSION = 1.9.18 +CURRENT_PROJECT_VERSION = 1.9.18 diff --git a/Riot/Assets/en.lproj/Vector.strings b/Riot/Assets/en.lproj/Vector.strings index c5d6c22ae..1a1ff5f2d 100644 --- a/Riot/Assets/en.lproj/Vector.strings +++ b/Riot/Assets/en.lproj/Vector.strings @@ -2304,6 +2304,7 @@ Tap the + to start adding people."; "poll_history_no_past_poll_period_text" = "There are no past polls for the past %@ days. Load more polls to view polls for previous months"; "poll_history_detail_view_in_timeline" = "View poll in timeline"; "poll_history_load_more" = "Load more polls"; +"poll_history_fetching_error" = "Error fetching polls."; // MARK: - Polls diff --git a/Riot/Generated/Strings.swift b/Riot/Generated/Strings.swift index d3a740965..494fa3b89 100644 --- a/Riot/Generated/Strings.swift +++ b/Riot/Generated/Strings.swift @@ -4821,7 +4821,11 @@ public class VectorL10n: NSObject { } /// View poll in timeline public static var pollHistoryDetailViewInTimeline: String { - return VectorL10n.tr("Vector", "poll_history_detail_view_in_timeline") + return VectorL10n.tr("Vector", "poll_history_detail_view_in_timeline") + } + /// Error fetching polls. + public static var pollHistoryFetchingError: String { + return VectorL10n.tr("Vector", "poll_history_fetching_error") } /// Load more polls public static var pollHistoryLoadMore: String { diff --git a/Riot/Modules/Analytics/DecryptionFailure.swift b/Riot/Modules/Analytics/DecryptionFailure.swift index 2f33f6369..1c991db88 100644 --- a/Riot/Modules/Analytics/DecryptionFailure.swift +++ b/Riot/Modules/Analytics/DecryptionFailure.swift @@ -16,12 +16,10 @@ import AnalyticsEvents -/// Failure reasons as defined in https://docs.google.com/document/d/1es7cTCeJEXXfRCTRgZerAM2Wg5ZerHjvlpfTW-gsOfI. @objc enum DecryptionFailureReason: Int { case unspecified case olmKeysNotSent case olmIndexError - case unexpected var errorName: AnalyticsEvent.Error.Name { switch self { @@ -31,8 +29,6 @@ import AnalyticsEvents return .OlmKeysNotSentError case .olmIndexError: return .OlmIndexError - case .unexpected: - return .UnknownError } } } diff --git a/Riot/Modules/Analytics/DecryptionFailureTracker.m b/Riot/Modules/Analytics/DecryptionFailureTracker.m index d175f9fd1..4a749b71a 100644 --- a/Riot/Modules/Analytics/DecryptionFailureTracker.m +++ b/Riot/Modules/Analytics/DecryptionFailureTracker.m @@ -105,12 +105,9 @@ NSString *const kDecryptionFailureTrackerAnalyticsCategory = @"e2e.failure"; reason = DecryptionFailureReasonOlmIndexError; break; - case MXDecryptingErrorEncryptionNotEnabledCode: - case MXDecryptingErrorUnableToDecryptCode: - reason = DecryptionFailureReasonUnexpected; - break; - default: + // All other error codes will be tracked as `OlmUnspecifiedError` and will include `context` containing + // the actual error code and localized description reason = DecryptionFailureReasonUnspecified; break; } diff --git a/Riot/Modules/Analytics/Helpers/MXCallHangupReason+Analytics.swift b/Riot/Modules/Analytics/Helpers/MXCallHangupReason+Analytics.swift index 4b8911ce8..c60f35446 100644 --- a/Riot/Modules/Analytics/Helpers/MXCallHangupReason+Analytics.swift +++ b/Riot/Modules/Analytics/Helpers/MXCallHangupReason+Analytics.swift @@ -21,6 +21,9 @@ extension __MXCallHangupReason { switch self { case .userHangup: return .VoipUserHangup + case .userBusy: + // There is no dedicated analytics event for `userBusy` error + return .UnknownError case .inviteTimeout: return .VoipInviteTimeout case .iceFailed: @@ -32,6 +35,9 @@ extension __MXCallHangupReason { case .unknownError: return .UnknownError default: + MXLog.failure("Unknown or unhandled hangup reason", context: [ + "reason": rawValue + ]) return .UnknownError } } diff --git a/Riot/Utils/EventFormatter.m b/Riot/Utils/EventFormatter.m index 21cd496dd..db526607a 100644 --- a/Riot/Utils/EventFormatter.m +++ b/Riot/Utils/EventFormatter.m @@ -567,7 +567,8 @@ static NSString *const kEventFormatterTimeFormat = @"HH:mm"; return [self session:session updateRoomSummary:summary withVoiceBroadcastInfoStateEvent:lastVoiceBroadcastInfoEvent - voiceBroadcastInfoStartedEvent:voiceBroadcastInfoStartedEvent roomState:roomState]; + voiceBroadcastInfoStartedEvent:voiceBroadcastInfoStartedEvent + roomState:roomState]; } } @@ -624,22 +625,8 @@ withVoiceBroadcastInfoStateEvent:lastVoiceBroadcastInfoEvent } else { - dispatch_group_t group = dispatch_group_create(); - dispatch_group_enter(group); - - __block MXEvent *voiceBroadcastInfoStartedEvent; - - [session eventWithEventId:voiceBroadcastInfo.voiceBroadcastId inRoom:roomId success:^(MXEvent *resultEvent) { - voiceBroadcastInfoStartedEvent = resultEvent; - dispatch_group_leave(group); - } failure:^(NSError *error) { - MXLogErrorDetails(@"[EventFormatter] Fetch eventWithEventId with error = %@", error.description); - dispatch_group_leave(group); - }]; - - dispatch_group_wait(group, DISPATCH_TIME_FOREVER); - - return voiceBroadcastInfoStartedEvent; + // Search for the event only in the store to avoid network calls while updating the room summary (this a synchronous process and we cannot delay it). + return [mxSession.store eventWithEventId:voiceBroadcastInfo.voiceBroadcastId inRoom:roomId]; } } diff --git a/RiotSwiftUI/Modules/Common/Avatar/View/AvatarImage.swift b/RiotSwiftUI/Modules/Common/Avatar/View/AvatarImage.swift index b143d4d30..3ddd4d472 100644 --- a/RiotSwiftUI/Modules/Common/Avatar/View/AvatarImage.swift +++ b/RiotSwiftUI/Modules/Common/Avatar/View/AvatarImage.swift @@ -26,9 +26,11 @@ struct AvatarImage: View { var displayName: String? var size: AvatarSize + @State private var avatar: AvatarViewState = .empty + var body: some View { Group { - switch viewModel.viewState { + switch avatar { case .empty: ProgressView() case .placeholder(let firstCharacter, let colorIndex): @@ -42,13 +44,16 @@ struct AvatarImage: View { .frame(maxWidth: CGFloat(size.rawValue), maxHeight: CGFloat(size.rawValue)) .clipShape(Circle()) .onAppear { - viewModel.loadAvatar( - mxContentUri: mxContentUri, - matrixItemId: matrixItemId, - displayName: displayName, - colorCount: theme.colors.namesAndAvatars.count, - avatarSize: size - ) + avatar = viewModel.placeholderAvatar(matrixItemId: matrixItemId, + displayName: displayName, + colorCount: theme.colors.namesAndAvatars.count) + viewModel.loadAvatar(mxContentUri: mxContentUri, + matrixItemId: matrixItemId, + displayName: displayName, + colorCount: theme.colors.namesAndAvatars.count, + avatarSize: size ) { newState in + avatar = newState + } } } } diff --git a/RiotSwiftUI/Modules/Common/Avatar/View/SpaceAvatarImage.swift b/RiotSwiftUI/Modules/Common/Avatar/View/SpaceAvatarImage.swift index 2662831e1..708c26a2b 100644 --- a/RiotSwiftUI/Modules/Common/Avatar/View/SpaceAvatarImage.swift +++ b/RiotSwiftUI/Modules/Common/Avatar/View/SpaceAvatarImage.swift @@ -26,9 +26,11 @@ struct SpaceAvatarImage: View { var displayName: String? var size: AvatarSize + @State private var avatar: AvatarViewState = .empty + var body: some View { Group { - switch viewModel.viewState { + switch avatar { case .empty: ProgressView() case .placeholder(let firstCharacter, let colorIndex): @@ -48,23 +50,27 @@ struct SpaceAvatarImage: View { .clipShape(RoundedRectangle(cornerRadius: 8)) } } - .onChange(of: displayName, perform: { value in - viewModel.loadAvatar( - mxContentUri: mxContentUri, - matrixItemId: matrixItemId, - displayName: value, - colorCount: theme.colors.namesAndAvatars.count, - avatarSize: size - ) - }) + .onChange(of: displayName) { value in + guard case .placeholder = avatar else { return } + viewModel.loadAvatar(mxContentUri: mxContentUri, + matrixItemId: matrixItemId, + displayName: value, + colorCount: theme.colors.namesAndAvatars.count, + avatarSize: size) { newState in + avatar = newState + } + } .onAppear { - viewModel.loadAvatar( - mxContentUri: mxContentUri, - matrixItemId: matrixItemId, - displayName: displayName, - colorCount: theme.colors.namesAndAvatars.count, - avatarSize: size - ) + avatar = viewModel.placeholderAvatar(matrixItemId: matrixItemId, + displayName: displayName, + colorCount: theme.colors.namesAndAvatars.count) + viewModel.loadAvatar(mxContentUri: mxContentUri, + matrixItemId: matrixItemId, + displayName: displayName, + colorCount: theme.colors.namesAndAvatars.count, + avatarSize: size) { newState in + avatar = newState + } } } } diff --git a/RiotSwiftUI/Modules/Common/Avatar/ViewModel/AvatarViewModel.swift b/RiotSwiftUI/Modules/Common/Avatar/ViewModel/AvatarViewModel.swift index 10055738d..68037f552 100644 --- a/RiotSwiftUI/Modules/Common/Avatar/ViewModel/AvatarViewModel.swift +++ b/RiotSwiftUI/Modules/Common/Avatar/ViewModel/AvatarViewModel.swift @@ -22,14 +22,22 @@ import Foundation final class AvatarViewModel: ObservableObject { private let avatarService: AvatarServiceProtocol - @Published private(set) var viewState = AvatarViewState.empty - init(avatarService: AvatarServiceProtocol) { self.avatarService = avatarService } private var cancellables = Set() + func placeholderAvatar(matrixItemId: String, + displayName: String?, + colorCount: Int) -> AvatarViewState { + let placeholderViewModel = PlaceholderAvatarViewModel(displayName: displayName, + matrixItemId: matrixItemId, + colorCount: colorCount) + + return .placeholder(placeholderViewModel.firstCharacterCapitalized, placeholderViewModel.stableColorIndex) + } + /// Load an avatar /// - Parameters: /// - mxContentUri: The matrix content URI of the avatar. @@ -41,14 +49,10 @@ final class AvatarViewModel: ObservableObject { matrixItemId: String, displayName: String?, colorCount: Int, - avatarSize: AvatarSize) { - let placeholderViewModel = PlaceholderAvatarViewModel(displayName: displayName, - matrixItemId: matrixItemId, - colorCount: colorCount) - - viewState = .placeholder(placeholderViewModel.firstCharacterCapitalized, placeholderViewModel.stableColorIndex) - + avatarSize: AvatarSize, + avatarCompletion: @escaping (AvatarViewState) -> Void) { guard let mxContentUri = mxContentUri, mxContentUri.count > 0 else { + avatarCompletion(placeholderAvatar(matrixItemId: matrixItemId, displayName: displayName, colorCount: colorCount)) return } @@ -56,8 +60,9 @@ final class AvatarViewModel: ObservableObject { .sink { completion in guard case let .failure(error) = completion else { return } UILog.error("[AvatarService] Failed to retrieve avatar", context: error) + // No need to call the completion, there's nothing we can do and the error is logged. } receiveValue: { image in - self.viewState = .avatar(image) + avatarCompletion(.avatar(image)) } .store(in: &cancellables) } diff --git a/RiotSwiftUI/Modules/Room/PollHistory/MockPollHistoryScreenState.swift b/RiotSwiftUI/Modules/Room/PollHistory/MockPollHistoryScreenState.swift index d939dab2f..a1c12e5f3 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/MockPollHistoryScreenState.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/MockPollHistoryScreenState.swift @@ -26,8 +26,11 @@ enum MockPollHistoryScreenState: MockScreenState, CaseIterable { // mock that screen. case active case past - case activeEmpty - case pastEmpty + case activeNoMoreContent + case contentLoading + case empty + case emptyLoading + case emptyNoMoreContent case loading /// The associated screen @@ -37,34 +40,40 @@ enum MockPollHistoryScreenState: MockScreenState, CaseIterable { /// Generate the view struct for the screen state. var screenView: ([Any], AnyView) { - let pollHistoryMode: PollHistoryMode + var pollHistoryMode: PollHistoryMode = .active let pollService = MockPollHistoryService() switch self { case .active: pollHistoryMode = .active + case .activeNoMoreContent: + pollHistoryMode = .active + pollService.hasNextBatch = false case .past: pollHistoryMode = .past - case .activeEmpty: + case .contentLoading: + pollService.nextBatchPublishers.append(MockPollPublisher.loadingPolls) + case .empty: pollHistoryMode = .active - pollService.nextBatchPublisher = Empty(completeImmediately: true, - outputType: TimelinePollDetails.self, - failureType: Error.self).eraseToAnyPublisher() - case .pastEmpty: - pollHistoryMode = .past - pollService.nextBatchPublisher = Empty(completeImmediately: true, - outputType: TimelinePollDetails.self, - failureType: Error.self).eraseToAnyPublisher() + pollService.nextBatchPublishers = [MockPollPublisher.emptyPolls] + case .emptyLoading: + pollService.nextBatchPublishers = [MockPollPublisher.emptyPolls, MockPollPublisher.loadingPolls] + case .emptyNoMoreContent: + pollService.hasNextBatch = false + pollService.nextBatchPublishers = [MockPollPublisher.emptyPolls] case .loading: - pollHistoryMode = .active - pollService.nextBatchPublisher = Empty(completeImmediately: false, - outputType: TimelinePollDetails.self, - failureType: Error.self).eraseToAnyPublisher() + pollService.nextBatchPublishers = [MockPollPublisher.loadingPolls] } let viewModel = PollHistoryViewModel(mode: pollHistoryMode, pollService: pollService) // can simulate service and viewModel actions here if needs be. + switch self { + case .contentLoading, .emptyLoading: + viewModel.process(viewAction: .loadMoreContent) + default: + break + } return ( [pollHistoryMode, viewModel], @@ -73,3 +82,17 @@ enum MockPollHistoryScreenState: MockScreenState, CaseIterable { ) } } + +enum MockPollPublisher { + static var emptyPolls: AnyPublisher { + Empty(completeImmediately: true).eraseToAnyPublisher() + } + + static var loadingPolls: AnyPublisher { + Empty(completeImmediately: false).eraseToAnyPublisher() + } + + static var failure: AnyPublisher { + Fail(error: NSError(domain: "fake", code: 1)).eraseToAnyPublisher() + } +} diff --git a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift index 68c30e064..4f4f2a606 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift @@ -33,6 +33,7 @@ enum PollHistoryMode: CaseIterable { struct PollHistoryViewBindings { var mode: PollHistoryMode + var alertInfo: AlertInfo? } struct PollHistoryViewState: BindableState { @@ -44,10 +45,13 @@ struct PollHistoryViewState: BindableState { var isLoading = false var canLoadMoreContent = true var polls: [TimelinePollDetails]? + var syncStartDate: Date = .init() + var syncedUpTo: Date = .distantFuture } enum PollHistoryViewAction { case viewAppeared case segmentDidChange case showPollDetail(poll: TimelinePollDetails) + case loadMoreContent } diff --git a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift index 0d1834c73..fa02f1a4f 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift @@ -29,6 +29,7 @@ final class PollHistoryViewModel: PollHistoryViewModelType, PollHistoryViewModel init(mode: PollHistoryMode, pollService: PollHistoryServiceProtocol) { self.pollService = pollService super.init(initialViewState: PollHistoryViewState(mode: mode)) + state.canLoadMoreContent = pollService.hasNextBatch } // MARK: - Public @@ -37,32 +38,47 @@ final class PollHistoryViewModel: PollHistoryViewModelType, PollHistoryViewModel switch viewAction { case .viewAppeared: setupUpdateSubscriptions() - fetchFirstBatch() + fetchContent() case .segmentDidChange: updateViewState() case .showPollDetail(let poll): completion?(.showPollDetail(poll: poll)) + case .loadMoreContent: + fetchContent() } } } private extension PollHistoryViewModel { - func fetchFirstBatch() { + func fetchContent() { state.isLoading = true pollService .nextBatch() .collect() - .sink { [weak self] _ in - #warning("Handle errors") - self?.state.isLoading = false + .sink { [weak self] completion in + self?.handleBatchEnded(completion: completion) } receiveValue: { [weak self] polls in - self?.polls = polls - self?.updateViewState() + self?.add(polls: polls) } .store(in: &subcriptions) } + func handleBatchEnded(completion: Subscribers.Completion) { + state.isLoading = false + state.canLoadMoreContent = pollService.hasNextBatch + + switch completion { + case .finished: + break + case .failure: + polls = polls ?? [] + state.bindings.alertInfo = .init(id: true, title: VectorL10n.pollHistoryFetchingError) + } + + updateViewState() + } + func setupUpdateSubscriptions() { subcriptions.removeAll() @@ -75,9 +91,15 @@ private extension PollHistoryViewModel { .store(in: &subcriptions) pollService - .pollErrors - .sink { detail in - #warning("Handle errors") + .fetchedUpTo + .weakAssign(to: \.state.syncedUpTo, on: self) + .store(in: &subcriptions) + + pollService + .livePolls + .sink { [weak self] livePoll in + self?.add(polls: [livePoll]) + self?.updateViewState() } .store(in: &subcriptions) } @@ -90,6 +112,10 @@ private extension PollHistoryViewModel { polls?[pollIndex] = poll } + func add(polls: [TimelinePollDetails]) { + self.polls = (self.polls ?? []) + polls + } + func updateViewState() { let renderedPolls: [TimelinePollDetails]? @@ -106,17 +132,22 @@ private extension PollHistoryViewModel { extension PollHistoryViewModel.Context { var emptyPollsText: String { - let days = PollHistoryConstants.chunkSizeInDays - switch (viewState.bindings.mode, viewState.canLoadMoreContent) { case (.active, true): - return VectorL10n.pollHistoryNoActivePollPeriodText("\(days)") + return VectorL10n.pollHistoryNoActivePollPeriodText("\(syncedPastDays)") case (.active, false): return VectorL10n.pollHistoryNoActivePollText case (.past, true): - return VectorL10n.pollHistoryNoPastPollPeriodText("\(days)") + return VectorL10n.pollHistoryNoPastPollPeriodText("\(syncedPastDays)") case (.past, false): return VectorL10n.pollHistoryNoPastPollText } } + + var syncedPastDays: Int { + guard let days = Calendar.current.dateComponents([.day], from: viewState.syncedUpTo, to: viewState.syncStartDate).day else { + return 0 + } + return max(0, days) + } } diff --git a/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift b/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift index a57731cdd..7f6d8c5f6 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift @@ -22,58 +22,105 @@ final class PollHistoryService: PollHistoryServiceProtocol { private let room: MXRoom private let timeline: MXEventTimeline private let chunkSizeInDays: UInt - private var timelineListener: Any? - private let updatesSubject: PassthroughSubject = .init() - private let pollErrorsSubject: PassthroughSubject = .init() + private var timelineListener: Any? + private var roomListener: Any? - private var pollAggregators: [String: PollAggregator] = [:] - private var targetTimestamp: Date - private var oldestEventDate: Date = .distantFuture + // polls aggregation + private var pollAggregationContexts: [String: PollAggregationContext] = [:] + + // polls private var currentBatchSubject: PassthroughSubject? + private var livePollsSubject: PassthroughSubject = .init() + + // polls updates + private let updatesSubject: PassthroughSubject = .init() + + // timestamps + private var targetTimestamp: Date = .init() + private var oldestEventDateSubject: CurrentValueSubject = .init(.init()) var updates: AnyPublisher { updatesSubject.eraseToAnyPublisher() } - var pollErrors: AnyPublisher { - pollErrorsSubject.eraseToAnyPublisher() - } - init(room: MXRoom, chunkSizeInDays: UInt) { self.room = room self.chunkSizeInDays = chunkSizeInDays timeline = MXRoomEventTimeline(room: room, andInitialEventId: nil) - targetTimestamp = Date().addingTimeInterval(-TimeInterval(chunkSizeInDays) * Constants.oneDayInSeconds) - setup(timeline: timeline) + setupTimeline() + setupLiveUpdates() } func nextBatch() -> AnyPublisher { currentBatchSubject?.eraseToAnyPublisher() ?? startPagination() } + + var hasNextBatch: Bool { + timeline.canPaginate(.backwards) + } + + var fetchedUpTo: AnyPublisher { + oldestEventDateSubject.eraseToAnyPublisher() + } + + var livePolls: AnyPublisher { + livePollsSubject.eraseToAnyPublisher() + } + + deinit { + guard let roomListener = roomListener else { + return + } + room.removeListener(roomListener) + } + + class PollAggregationContext { + var pollAggregator: PollAggregator? + let isLivePoll: Bool + var published: Bool + + init(pollAggregator: PollAggregator? = nil, isLivePoll: Bool, published: Bool = false) { + self.pollAggregator = pollAggregator + self.isLivePoll = isLivePoll + self.published = published + } + } } private extension PollHistoryService { enum Constants { - static let pageSize: UInt = 500 - static let oneDayInSeconds: TimeInterval = 8.6 * 10e3 + static let pageSize: UInt = 250 } - func setup(timeline: MXEventTimeline) { + func setupTimeline() { + timeline.resetPagination() + timelineListener = timeline.listenToEvents { [weak self] event, _, _ in if event.eventType == .pollStart { - self?.aggregatePoll(pollStartEvent: event) + self?.aggregatePoll(pollStartEvent: event, isLivePoll: false) } self?.updateTimestamp(event: event) } } + func setupLiveUpdates() { + roomListener = room.listen(toEventsOfTypes: [kMXEventTypeStringPollStart, kMXEventTypeStringPollStartMSC3381]) { [weak self] event, _, _ in + if event.eventType == .pollStart { + self?.aggregatePoll(pollStartEvent: event, isLivePoll: true) + } + } + } + func updateTimestamp(event: MXEvent) { oldestEventDate = min(event.originServerDate, oldestEventDate) } func startPagination() -> AnyPublisher { + let startingTimestamp = oldestEventDate + targetTimestamp = startingTimestamp.subtractingDays(chunkSizeInDays) ?? startingTimestamp + let batchSubject = PassthroughSubject() currentBatchSubject = batchSubject @@ -81,14 +128,13 @@ private extension PollHistoryService { guard let self = self else { return } - self.timeline.resetPagination() - self.paginate(timeline: self.timeline) + self.paginate() } return batchSubject.eraseToAnyPublisher() } - func paginate(timeline: MXEventTimeline) { + func paginate() { timeline.paginate(Constants.pageSize, direction: .backwards, onlyFromStore: false) { [weak self] response in guard let self = self else { return @@ -96,8 +142,8 @@ private extension PollHistoryService { switch response { case .success: - if timeline.canPaginate(.backwards), self.timestampTargetReached == false { - self.paginate(timeline: timeline) + if self.timeline.canPaginate(.backwards), self.timestampTargetReached == false { + self.paginate() } else { self.completeBatch(completion: .finished) } @@ -112,21 +158,41 @@ private extension PollHistoryService { currentBatchSubject = nil } - func aggregatePoll(pollStartEvent: MXEvent) { - guard pollAggregators[pollStartEvent.eventId] == nil else { + func aggregatePoll(pollStartEvent: MXEvent, isLivePoll: Bool) { + let eventId: String = pollStartEvent.eventId + + guard pollAggregationContexts[eventId] == nil else { return } - guard let aggregator = try? PollAggregator(session: room.mxSession, room: room, pollEvent: pollStartEvent, delegate: self) else { - return - } + let newContext: PollAggregationContext = .init(isLivePoll: isLivePoll) + pollAggregationContexts[eventId] = newContext - pollAggregators[pollStartEvent.eventId] = aggregator + do { + newContext.pollAggregator = try PollAggregator(session: room.mxSession, room: room, pollEvent: pollStartEvent, delegate: self) + } catch { + pollAggregationContexts.removeValue(forKey: eventId) + } } var timestampTargetReached: Bool { oldestEventDate <= targetTimestamp } + + var oldestEventDate: Date { + get { + oldestEventDateSubject.value + } + set { + oldestEventDateSubject.send(newValue) + } + } +} + +private extension Date { + func subtractingDays(_ days: UInt) -> Date? { + Calendar.current.date(byAdding: DateComponents(day: -Int(days)), to: self) + } } private extension MXEvent { @@ -138,17 +204,30 @@ private extension MXEvent { // MARK: - PollAggregatorDelegate extension PollHistoryService: PollAggregatorDelegate { - func pollAggregatorDidStartLoading(_ aggregator: PollAggregator) {} + func pollAggregatorDidStartLoading(_ aggregator: PollAggregator) { } + + func pollAggregator(_ aggregator: PollAggregator, didFailWithError: Error) { } func pollAggregatorDidEndLoading(_ aggregator: PollAggregator) { - currentBatchSubject?.send(.init(poll: aggregator.poll, represent: .started)) - } - - func pollAggregator(_ aggregator: PollAggregator, didFailWithError: Error) { - pollErrorsSubject.send(didFailWithError) + guard let context = pollAggregationContexts[aggregator.poll.id], context.published == false else { + return + } + + context.published = true + + let newPoll: TimelinePollDetails = .init(poll: aggregator.poll, represent: .started) + + if context.isLivePoll { + livePollsSubject.send(newPoll) + } else { + currentBatchSubject?.send(newPoll) + } } func pollAggregatorDidUpdateData(_ aggregator: PollAggregator) { + guard let context = pollAggregationContexts[aggregator.poll.id], context.published else { + return + } updatesSubject.send(.init(poll: aggregator.poll, represent: .started)) } } diff --git a/RiotSwiftUI/Modules/Room/PollHistory/Service/Mock/MockPollHistoryService.swift b/RiotSwiftUI/Modules/Room/PollHistory/Service/Mock/MockPollHistoryService.swift index acd9543e3..c98f4e136 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/Service/Mock/MockPollHistoryService.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/Service/Mock/MockPollHistoryService.swift @@ -17,29 +17,38 @@ import Combine final class MockPollHistoryService: PollHistoryServiceProtocol { + lazy var nextBatchPublishers: [AnyPublisher] = [ + (activePollsData + pastPollsData) + .publisher + .setFailureType(to: Error.self) + .eraseToAnyPublisher() + ] + + func nextBatch() -> AnyPublisher { + nextBatchPublishers.isEmpty ? Empty().eraseToAnyPublisher() : nextBatchPublishers.removeFirst() + } + var updatesPublisher: AnyPublisher = Empty().eraseToAnyPublisher() var updates: AnyPublisher { updatesPublisher } - var pollErrorPublisher: AnyPublisher = Empty().eraseToAnyPublisher() - var pollErrors: AnyPublisher { - pollErrorPublisher + var hasNextBatch = true + + var fetchedUpToPublisher: AnyPublisher = Just(.init()).eraseToAnyPublisher() + var fetchedUpTo: AnyPublisher { + fetchedUpToPublisher } - lazy var nextBatchPublisher: AnyPublisher = (activePollsData + pastPollsData) - .publisher - .setFailureType(to: Error.self) - .eraseToAnyPublisher() - - func nextBatch() -> AnyPublisher { - nextBatchPublisher + var livePollsPublisher: AnyPublisher = Empty().eraseToAnyPublisher() + var livePolls: AnyPublisher { + livePollsPublisher } } private extension MockPollHistoryService { var activePollsData: [TimelinePollDetails] { - (1...10) + (1...3) .map { index in TimelinePollDetails(id: "a\(index)", question: "Do you like the active poll number \(index)?", @@ -56,7 +65,7 @@ private extension MockPollHistoryService { } var pastPollsData: [TimelinePollDetails] { - (1...10) + (1...3) .map { index in TimelinePollDetails(id: "p\(index)", question: "Do you like the active poll number \(index)?", diff --git a/RiotSwiftUI/Modules/Room/PollHistory/Service/PollHistoryServiceProtocol.swift b/RiotSwiftUI/Modules/Room/PollHistory/Service/PollHistoryServiceProtocol.swift index 637ba393f..5132478cc 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/Service/PollHistoryServiceProtocol.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/Service/PollHistoryServiceProtocol.swift @@ -21,10 +21,17 @@ protocol PollHistoryServiceProtocol { /// Implementations should return the same publisher if `nextBatch()` is called again before the previous publisher completes. func nextBatch() -> AnyPublisher - /// Publishes updates for the polls previously pusblished by the `nextBatch()` publishers. + /// Publishes updates for the polls previously pusblished by the `nextBatch()` or `livePolls` publishers. var updates: AnyPublisher { get } - /// Publishes errors regarding poll aggregations. - /// Note: `nextBatch()` will continue to publish new polls even if some poll isn't being aggregated correctly. - var pollErrors: AnyPublisher { get } + /// Publishes live polls not related with the current batch. + var livePolls: AnyPublisher { get } + + /// Returns true every time the service can fetch another batch. + /// There is no guarantee the `nextBatch()` returned publisher will publish something anyway. + var hasNextBatch: Bool { get } + + /// Publishes the date up to the service is synced (in the past). + /// This date doesn't need to be related with any poll event. + var fetchedUpTo: AnyPublisher { get } } diff --git a/RiotSwiftUI/Modules/Room/PollHistory/Test/UI/PollHistoryUITests.swift b/RiotSwiftUI/Modules/Room/PollHistory/Test/UI/PollHistoryUITests.swift index 986fd37bd..ddce4978c 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/Test/UI/PollHistoryUITests.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/Test/UI/PollHistoryUITests.swift @@ -24,6 +24,7 @@ final class PollHistoryUITests: MockScreenTestCase { let emptyText = app.staticTexts["PollHistory.emptyText"] let items = app.staticTexts["PollListItem.title"] let selectedSegment = app.buttons[VectorL10n.pollHistoryActiveSegmentTitle] + let loadMoreButton = app.buttons["PollHistory.loadMore"] let winningOption = app.staticTexts["PollListData.winningOption"] XCTAssertEqual(title, VectorL10n.pollHistoryTitle) @@ -31,6 +32,7 @@ final class PollHistoryUITests: MockScreenTestCase { XCTAssertFalse(emptyText.exists) XCTAssertTrue(selectedSegment.exists) XCTAssertEqual(selectedSegment.value as? String, VectorL10n.accessibilitySelected) + XCTAssertTrue(loadMoreButton.exists) XCTAssertFalse(winningOption.exists) } @@ -40,6 +42,7 @@ final class PollHistoryUITests: MockScreenTestCase { let emptyText = app.staticTexts["PollHistory.emptyText"] let items = app.staticTexts["PollListItem.title"] let selectedSegment = app.buttons[VectorL10n.pollHistoryPastSegmentTitle] + let loadMoreButton = app.buttons["PollHistory.loadMore"] let winningOption = app.buttons["PollAnswerOption0"] XCTAssertEqual(title, VectorL10n.pollHistoryTitle) @@ -47,33 +50,66 @@ final class PollHistoryUITests: MockScreenTestCase { XCTAssertFalse(emptyText.exists) XCTAssertTrue(selectedSegment.exists) XCTAssertEqual(selectedSegment.value as? String, VectorL10n.accessibilitySelected) + XCTAssertTrue(loadMoreButton.exists) XCTAssertTrue(winningOption.exists) } - func testPastPollHistoryIsEmpty() { - app.goToScreenWithIdentifier(MockPollHistoryScreenState.pastEmpty.title) + func testActivePollHistoryHasContentAndCantLoadMore() { + app.goToScreenWithIdentifier(MockPollHistoryScreenState.activeNoMoreContent.title) + let emptyText = app.staticTexts["PollHistory.emptyText"] + let items = app.staticTexts["PollListItem.title"] + let loadMoreButton = app.buttons["PollHistory.loadMore"] + + XCTAssertTrue(items.exists) + XCTAssertFalse(emptyText.exists) + XCTAssertFalse(loadMoreButton.exists) + } + + func testActivePollHistoryHasContentAndCanLoadMore() { + app.goToScreenWithIdentifier(MockPollHistoryScreenState.contentLoading.title) let title = app.navigationBars.firstMatch.identifier let emptyText = app.staticTexts["PollHistory.emptyText"] let items = app.staticTexts["PollListItem.title"] - let selectedSegment = app.buttons[VectorL10n.pollHistoryPastSegmentTitle] - let winningOption = app.staticTexts["PollListData.winningOption"] + let loadMoreButton = app.buttons["PollHistory.loadMore"] - XCTAssertEqual(title, VectorL10n.pollHistoryTitle) - XCTAssertFalse(items.exists) - XCTAssertTrue(emptyText.exists) - XCTAssertTrue(selectedSegment.exists) - XCTAssertEqual(selectedSegment.value as? String, VectorL10n.accessibilitySelected) - XCTAssertFalse(winningOption.exists) + XCTAssertTrue(items.exists) + XCTAssertFalse(emptyText.exists) + XCTAssertTrue(loadMoreButton.exists) + XCTAssertFalse(loadMoreButton.isEnabled) } - func testLoaderIsShowing() { - app.goToScreenWithIdentifier(MockPollHistoryScreenState.loading.title) - let title = app.navigationBars.firstMatch.identifier - let loadingText = app.staticTexts["PollHistory.loadingText"] + func testActivePollHistoryEmptyAndCanLoadMore() { + app.goToScreenWithIdentifier(MockPollHistoryScreenState.empty.title) + let emptyText = app.staticTexts["PollHistory.emptyText"] let items = app.staticTexts["PollListItem.title"] + let loadMoreButton = app.buttons["PollHistory.loadMore"] - XCTAssertEqual(title, VectorL10n.pollHistoryTitle) XCTAssertFalse(items.exists) - XCTAssertTrue(loadingText.exists) + XCTAssertTrue(emptyText.exists) + XCTAssertTrue(loadMoreButton.exists) + XCTAssertTrue(loadMoreButton.isEnabled) + } + + func testActivePollHistoryEmptyAndLoading() { + app.goToScreenWithIdentifier(MockPollHistoryScreenState.emptyLoading.title) + let emptyText = app.staticTexts["PollHistory.emptyText"] + let items = app.staticTexts["PollListItem.title"] + let loadMoreButton = app.buttons["PollHistory.loadMore"] + + XCTAssertFalse(items.exists) + XCTAssertTrue(emptyText.exists) + XCTAssertTrue(loadMoreButton.exists) + XCTAssertFalse(loadMoreButton.isEnabled) + } + + func testActivePollHistoryEmptyAndCantLoadMore() { + app.goToScreenWithIdentifier(MockPollHistoryScreenState.emptyNoMoreContent.title) + let emptyText = app.staticTexts["PollHistory.emptyText"] + let items = app.staticTexts["PollListItem.title"] + let loadMoreButton = app.buttons["PollHistory.loadMore"] + + XCTAssertFalse(items.exists) + XCTAssertTrue(emptyText.exists) + XCTAssertFalse(loadMoreButton.exists) } } diff --git a/RiotSwiftUI/Modules/Room/PollHistory/Test/Unit/PollHistoryViewModelTests.swift b/RiotSwiftUI/Modules/Room/PollHistory/Test/Unit/PollHistoryViewModelTests.swift index e2eff7475..efce641d4 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/Test/Unit/PollHistoryViewModelTests.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/Test/Unit/PollHistoryViewModelTests.swift @@ -1,4 +1,4 @@ -// +// // Copyright 2023 New Vector Ltd // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -42,9 +42,11 @@ final class PollHistoryViewModelTests: XCTestCase { func testLoadingStateIsTrueWhileLoading() { XCTAssertFalse(viewModel.state.isLoading) - pollHistoryService.nextBatchPublisher = Empty(completeImmediately: false, outputType: TimelinePollDetails.self, failureType: Error.self).eraseToAnyPublisher() + pollHistoryService.nextBatchPublishers = [MockPollPublisher.loadingPolls, MockPollPublisher.emptyPolls] viewModel.process(viewAction: .viewAppeared) XCTAssertTrue(viewModel.state.isLoading) + viewModel.process(viewAction: .viewAppeared) + XCTAssertFalse(viewModel.state.isLoading) } func testUpdatesAreHandled() throws { @@ -79,6 +81,35 @@ final class PollHistoryViewModelTests: XCTestCase { let pollDates = try polls.map(\.startDate) XCTAssertEqual(pollDates, pollDates.sorted(by: { $0 > $1 })) } + + func testLivePollsAreHandled() throws { + pollHistoryService.nextBatchPublishers = [MockPollPublisher.emptyPolls] + pollHistoryService.livePollsPublisher = Just(mockPoll).eraseToAnyPublisher() + viewModel.process(viewAction: .viewAppeared) + XCTAssertEqual(viewModel.state.polls?.count, 1) + XCTAssertEqual(viewModel.state.polls?.first?.id, "id") + } + + func testLivePollsDontChangeLoadingState() throws { + let livePolls = PassthroughSubject() + pollHistoryService.nextBatchPublishers = [MockPollPublisher.loadingPolls] + pollHistoryService.livePollsPublisher = livePolls.eraseToAnyPublisher() + viewModel.process(viewAction: .viewAppeared) + XCTAssertTrue(viewModel.state.isLoading) + XCTAssertNil(viewModel.state.polls) + livePolls.send(mockPoll) + XCTAssertTrue(viewModel.state.isLoading) + XCTAssertNotNil(viewModel.state.polls) + XCTAssertEqual(viewModel.state.polls?.count, 1) + } + + func testAfterFailureCompletionIsCalled() throws { + pollHistoryService.nextBatchPublishers = [MockPollPublisher.failure] + viewModel.process(viewAction: .viewAppeared) + XCTAssertFalse(viewModel.state.isLoading) + XCTAssertNotNil(viewModel.state.polls) + XCTAssertNotNil(viewModel.state.bindings.alertInfo) + } } private extension PollHistoryViewModelTests { @@ -87,4 +118,18 @@ private extension PollHistoryViewModelTests { try XCTUnwrap(viewModel.state.polls) } } + + var mockPoll: TimelinePollDetails { + .init(id: "id", + question: "Do you like polls?", + answerOptions: [], + closed: false, + startDate: .init(), + totalAnswerCount: 3, + type: .undisclosed, + eventType: .started, + maxAllowedSelections: 1, + hasBeenEdited: false, + hasDecryptionError: false) + } } diff --git a/RiotSwiftUI/Modules/Room/PollHistory/View/PollHistory.swift b/RiotSwiftUI/Modules/Room/PollHistory/View/PollHistory.swift index 0c819bf3a..da9ba02da 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/View/PollHistory.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/View/PollHistory.swift @@ -44,6 +44,9 @@ struct PollHistory: View { .onChange(of: viewModel.mode) { _ in viewModel.send(viewAction: .segmentDidChange) } + .alert(item: $viewModel.alertInfo) { + $0.alert + } } @ViewBuilder @@ -76,19 +79,23 @@ struct PollHistory: View { } } + @ViewBuilder private var loadMoreButton: some View { - HStack(spacing: 8) { - if viewModel.viewState.isLoading { - spinner + if viewModel.viewState.canLoadMoreContent { + HStack(spacing: 8) { + if viewModel.viewState.isLoading { + spinner + } + + Button { + viewModel.send(viewAction: .loadMoreContent) + } label: { + Text(VectorL10n.pollHistoryLoadMore) + .font(theme.fonts.body) + } + .accessibilityIdentifier("PollHistory.loadMore") + .disabled(viewModel.viewState.isLoading) } - - Button { - #warning("handle action in next ticket") - } label: { - Text(VectorL10n.pollHistoryLoadMore) - .font(theme.fonts.body) - } - .disabled(viewModel.viewState.isLoading) } } diff --git a/RiotSwiftUI/Modules/Room/TimelinePoll/View/TimelinePollAnswerOptionButton.swift b/RiotSwiftUI/Modules/Room/TimelinePoll/View/TimelinePollAnswerOptionButton.swift index 6b0765a49..1488911bd 100644 --- a/RiotSwiftUI/Modules/Room/TimelinePoll/View/TimelinePollAnswerOptionButton.swift +++ b/RiotSwiftUI/Modules/Room/TimelinePoll/View/TimelinePollAnswerOptionButton.swift @@ -58,17 +58,19 @@ struct TimelinePollAnswerOptionButton: View { .font(theme.fonts.body) .foregroundColor(theme.colors.primaryContent) .accessibilityIdentifier("PollAnswerOption\(optionIndex)Label") + .frame(maxWidth: .infinity, alignment: .leading) - if poll.closed, answerOption.winner { - Spacer() - Image(uiImage: Asset.Images.pollWinnerIcon.image) - } - - if poll.shouldDiscloseResults { - Text(answerOption.count == 1 ? VectorL10n.pollTimelineOneVote : VectorL10n.pollTimelineVotesCount(Int(answerOption.count))) - .font(theme.fonts.footnote) - .foregroundColor(poll.closed && answerOption.winner ? theme.colors.accent : theme.colors.secondaryContent) - .accessibilityIdentifier("PollAnswerOption\(optionIndex)Count") + HStack(spacing: 6) { + if poll.closed, answerOption.winner { + Image(uiImage: Asset.Images.pollWinnerIcon.image) + } + + if poll.shouldDiscloseResults { + Text(answerOption.count == 1 ? VectorL10n.pollTimelineOneVote : VectorL10n.pollTimelineVotesCount(Int(answerOption.count))) + .font(theme.fonts.footnote) + .foregroundColor(poll.closed && answerOption.winner ? theme.colors.accent : theme.colors.secondaryContent) + .accessibilityIdentifier("PollAnswerOption\(optionIndex)Count") + } } } diff --git a/changelog.d/pr-7303.change b/changelog.d/pr-7303.change new file mode 100644 index 000000000..fc6bbdb3a --- /dev/null +++ b/changelog.d/pr-7303.change @@ -0,0 +1 @@ +Poll: add a feature to load more polls in the poll history.