From a47d7dcff0f12cbf9555bb95e46a4b10f9267562 Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Wed, 25 Jan 2023 11:41:16 +0100 Subject: [PATCH 01/23] Fix TimelinePollAnswerOptionButton layout --- .../View/TimelinePollAnswerOptionButton.swift | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/RiotSwiftUI/Modules/Room/TimelinePoll/View/TimelinePollAnswerOptionButton.swift b/RiotSwiftUI/Modules/Room/TimelinePoll/View/TimelinePollAnswerOptionButton.swift index 6b0765a496..1488911bd6 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") + } } } From 085ee66a027305e0cefc50e7894150c127db4e20 Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Wed, 25 Jan 2023 12:05:28 +0100 Subject: [PATCH 02/23] Support load more in PollHistoryService --- .../Room/PollHistory/PollHistoryModels.swift | 1 + .../PollHistory/PollHistoryViewModel.swift | 21 +++++++++++++++++++ .../MatrixSDK/PollHistoryService.swift | 20 ++++++++++++++---- .../Room/PollHistory/View/PollHistory.swift | 2 +- 4 files changed, 39 insertions(+), 5 deletions(-) diff --git a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift index 7331dc3eda..542f78f4a1 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift @@ -49,4 +49,5 @@ struct PollHistoryViewState: BindableState { enum PollHistoryViewAction { case viewAppeared case segmentDidChange + case loadMoreContent } diff --git a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift index eed1e6c6cf..995464ed81 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift @@ -40,6 +40,8 @@ final class PollHistoryViewModel: PollHistoryViewModelType, PollHistoryViewModel fetchFirstBatch() case .segmentDidChange: updateViewState() + case .loadMoreContent: + fetchMoreContent() } } } @@ -61,6 +63,21 @@ private extension PollHistoryViewModel { .store(in: &subcriptions) } + func fetchMoreContent() { + state.isLoading = true + + pollService + .nextBatch() + .sink { [weak self] _ in + #warning("Handle errors") + self?.state.isLoading = false + } receiveValue: { [weak self] poll in + self?.add(poll: poll) + self?.updateViewState() + } + .store(in: &subcriptions) + } + func setupUpdateSubscriptions() { subcriptions.removeAll() @@ -88,6 +105,10 @@ private extension PollHistoryViewModel { polls?[pollIndex] = poll } + func add(poll: TimelinePollDetails) { + polls?.append(poll) + } + func updateViewState() { let renderedPolls: [TimelinePollDetails]? diff --git a/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift b/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift index a57731cdd4..2c684a598d 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift @@ -28,7 +28,7 @@ final class PollHistoryService: PollHistoryServiceProtocol { private let pollErrorsSubject: PassthroughSubject = .init() private var pollAggregators: [String: PollAggregator] = [:] - private var targetTimestamp: Date + private var targetTimestamp: Date? private var oldestEventDate: Date = .distantFuture private var currentBatchSubject: PassthroughSubject? @@ -44,7 +44,6 @@ final class PollHistoryService: PollHistoryServiceProtocol { self.room = room self.chunkSizeInDays = chunkSizeInDays timeline = MXRoomEventTimeline(room: room, andInitialEventId: nil) - targetTimestamp = Date().addingTimeInterval(-TimeInterval(chunkSizeInDays) * Constants.oneDayInSeconds) setup(timeline: timeline) } @@ -56,7 +55,6 @@ final class PollHistoryService: PollHistoryServiceProtocol { private extension PollHistoryService { enum Constants { static let pageSize: UInt = 500 - static let oneDayInSeconds: TimeInterval = 8.6 * 10e3 } func setup(timeline: MXEventTimeline) { @@ -74,6 +72,9 @@ private extension PollHistoryService { } func startPagination() -> AnyPublisher { + let startingTimestamp = targetTimestamp ?? .init() + targetTimestamp = startingTimestamp.subtractingDays(chunkSizeInDays) + let batchSubject = PassthroughSubject() currentBatchSubject = batchSubject @@ -125,7 +126,18 @@ private extension PollHistoryService { } var timestampTargetReached: Bool { - oldestEventDate <= targetTimestamp + guard let targetTimestamp = targetTimestamp else { + return true + } + return oldestEventDate <= targetTimestamp + } +} + +private extension Date { + private static let oneDayInSeconds: TimeInterval = 8.6 * 10e3 + + func subtractingDays(_ days: UInt) -> Date { + addingTimeInterval(-TimeInterval(days) * Self.oneDayInSeconds) } } diff --git a/RiotSwiftUI/Modules/Room/PollHistory/View/PollHistory.swift b/RiotSwiftUI/Modules/Room/PollHistory/View/PollHistory.swift index 49ce11a5a2..163918bfa8 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/View/PollHistory.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/View/PollHistory.swift @@ -80,7 +80,7 @@ struct PollHistory: View { } Button { - #warning("handle action in next ticket") + viewModel.send(viewAction: .loadMoreContent) } label: { Text(VectorL10n.pollHistoryLoadMore) .font(theme.fonts.body) From a1a885e2038939a17459c796ef58cc6a0eaf318d Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Wed, 25 Jan 2023 12:35:51 +0100 Subject: [PATCH 03/23] Handle number of batches / last batch --- .../Room/PollHistory/PollHistoryModels.swift | 1 + .../PollHistory/PollHistoryViewModel.swift | 25 +++++++++++++------ .../MatrixSDK/PollHistoryService.swift | 4 +++ .../Service/Mock/MockPollHistoryService.swift | 18 +++++++------ .../Service/PollHistoryServiceProtocol.swift | 4 +++ 5 files changed, 37 insertions(+), 15 deletions(-) diff --git a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift index 542f78f4a1..d861959c78 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift @@ -44,6 +44,7 @@ struct PollHistoryViewState: BindableState { var isLoading = false var canLoadMoreContent = true var polls: [TimelinePollDetails]? + var numberOfFetchedBatches: UInt = 0 } enum PollHistoryViewAction { diff --git a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift index 995464ed81..91a8201d51 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 @@ -53,9 +54,8 @@ private extension PollHistoryViewModel { 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() @@ -68,9 +68,8 @@ private extension PollHistoryViewModel { pollService .nextBatch() - .sink { [weak self] _ in - #warning("Handle errors") - self?.state.isLoading = false + .sink { [weak self] completion in + self?.handleBatchEnded(completion: completion) } receiveValue: { [weak self] poll in self?.add(poll: poll) self?.updateViewState() @@ -78,6 +77,18 @@ private extension PollHistoryViewModel { .store(in: &subcriptions) } + func handleBatchEnded(completion: Subscribers.Completion) { + state.isLoading = false + state.canLoadMoreContent = pollService.hasNextBatch + + switch completion { + case .finished: + state.numberOfFetchedBatches += 1 + case .failure(_): + #warning("Handle errors") + } + } + func setupUpdateSubscriptions() { subcriptions.removeAll() @@ -125,7 +136,7 @@ private extension PollHistoryViewModel { extension PollHistoryViewModel.Context { var emptyPollsText: String { - let days = PollHistoryConstants.chunkSizeInDays + let days = PollHistoryConstants.chunkSizeInDays * viewState.numberOfFetchedBatches switch (viewState.bindings.mode, viewState.canLoadMoreContent) { case (.active, true): diff --git a/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift b/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift index 2c684a598d..b3fc094c48 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift @@ -50,6 +50,10 @@ final class PollHistoryService: PollHistoryServiceProtocol { func nextBatch() -> AnyPublisher { currentBatchSubject?.eraseToAnyPublisher() ?? startPagination() } + + var hasNextBatch: Bool { + timeline.canPaginate(.backwards) + } } private extension PollHistoryService { diff --git a/RiotSwiftUI/Modules/Room/PollHistory/Service/Mock/MockPollHistoryService.swift b/RiotSwiftUI/Modules/Room/PollHistory/Service/Mock/MockPollHistoryService.swift index acd9543e3c..d6b12c3d26 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/Service/Mock/MockPollHistoryService.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/Service/Mock/MockPollHistoryService.swift @@ -17,6 +17,15 @@ import Combine final class MockPollHistoryService: PollHistoryServiceProtocol { + lazy var nextBatchPublisher: AnyPublisher = (activePollsData + pastPollsData) + .publisher + .setFailureType(to: Error.self) + .eraseToAnyPublisher() + + func nextBatch() -> AnyPublisher { + nextBatchPublisher + } + var updatesPublisher: AnyPublisher = Empty().eraseToAnyPublisher() var updates: AnyPublisher { updatesPublisher @@ -27,14 +36,7 @@ final class MockPollHistoryService: PollHistoryServiceProtocol { pollErrorPublisher } - lazy var nextBatchPublisher: AnyPublisher = (activePollsData + pastPollsData) - .publisher - .setFailureType(to: Error.self) - .eraseToAnyPublisher() - - func nextBatch() -> AnyPublisher { - nextBatchPublisher - } + var hasNextBatch: Bool = true } private extension MockPollHistoryService { diff --git a/RiotSwiftUI/Modules/Room/PollHistory/Service/PollHistoryServiceProtocol.swift b/RiotSwiftUI/Modules/Room/PollHistory/Service/PollHistoryServiceProtocol.swift index 637ba393f8..85f0a91372 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/Service/PollHistoryServiceProtocol.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/Service/PollHistoryServiceProtocol.swift @@ -27,4 +27,8 @@ protocol PollHistoryServiceProtocol { /// 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 } + + /// 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 } } From cbededa4494e0770f311c8ebd5fdae34e858cb49 Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Wed, 25 Jan 2023 12:56:36 +0100 Subject: [PATCH 04/23] Disable load more button when there is no content --- RiotSwiftUI/Modules/Room/PollHistory/View/PollHistory.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RiotSwiftUI/Modules/Room/PollHistory/View/PollHistory.swift b/RiotSwiftUI/Modules/Room/PollHistory/View/PollHistory.swift index 163918bfa8..c0dfd5c12c 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/View/PollHistory.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/View/PollHistory.swift @@ -85,7 +85,7 @@ struct PollHistory: View { Text(VectorL10n.pollHistoryLoadMore) .font(theme.fonts.body) } - .disabled(viewModel.viewState.isLoading) + .disabled(viewModel.viewState.isLoading || !viewModel.viewState.canLoadMoreContent) } } From 619b0cd3a20cb1bd0f86841d63be733a98a4896c Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Wed, 25 Jan 2023 15:12:19 +0100 Subject: [PATCH 05/23] Add live synced days --- .../Room/PollHistory/PollHistoryModels.swift | 6 ++++-- .../PollHistory/PollHistoryViewModel.swift | 18 +++++++++++++----- .../MatrixSDK/PollHistoryService.swift | 19 +++++++++++++++---- .../Service/Mock/MockPollHistoryService.swift | 5 +++++ .../Service/PollHistoryServiceProtocol.swift | 4 ++++ 5 files changed, 41 insertions(+), 11 deletions(-) diff --git a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift index d861959c78..e88d98367d 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift @@ -17,7 +17,8 @@ // MARK: View model enum PollHistoryConstants { - static let chunkSizeInDays: UInt = 30 + static let chunkSizeInDays: UInt = 10 + static let oneDayInSeconds: TimeInterval = 8.6 * 10e3 } enum PollHistoryViewModelResult: Equatable { @@ -44,7 +45,8 @@ struct PollHistoryViewState: BindableState { var isLoading = false var canLoadMoreContent = true var polls: [TimelinePollDetails]? - var numberOfFetchedBatches: UInt = 0 + var syncStartDate: Date = .init() + var syncedUpTo: Date = .distantFuture } enum PollHistoryViewAction { diff --git a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift index 91a8201d51..ef7599830b 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift @@ -83,7 +83,7 @@ private extension PollHistoryViewModel { switch completion { case .finished: - state.numberOfFetchedBatches += 1 + break case .failure(_): #warning("Handle errors") } @@ -106,6 +106,11 @@ private extension PollHistoryViewModel { #warning("Handle errors") } .store(in: &subcriptions) + + pollService + .fetchedUpTo + .weakAssign(to: \.state.syncedUpTo, on: self) + .store(in: &subcriptions) } func update(poll: TimelinePollDetails) { @@ -136,17 +141,20 @@ private extension PollHistoryViewModel { extension PollHistoryViewModel.Context { var emptyPollsText: String { - let days = PollHistoryConstants.chunkSizeInDays * viewState.numberOfFetchedBatches - 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: UInt { + let timeDelta = max(viewState.syncStartDate.timeIntervalSince(viewState.syncedUpTo), 0) + return UInt((timeDelta / PollHistoryConstants.oneDayInSeconds).rounded()) + } } diff --git a/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift b/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift index b3fc094c48..e6569174a2 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift @@ -29,7 +29,7 @@ final class PollHistoryService: PollHistoryServiceProtocol { private var pollAggregators: [String: PollAggregator] = [:] private var targetTimestamp: Date? - private var oldestEventDate: Date = .distantFuture + private var oldestEventDateSubject: CurrentValueSubject = .init(Date.distantFuture) private var currentBatchSubject: PassthroughSubject? var updates: AnyPublisher { @@ -54,6 +54,10 @@ final class PollHistoryService: PollHistoryServiceProtocol { var hasNextBatch: Bool { timeline.canPaginate(.backwards) } + + var fetchedUpTo: AnyPublisher { + oldestEventDateSubject.eraseToAnyPublisher() + } } private extension PollHistoryService { @@ -135,13 +139,20 @@ private extension PollHistoryService { } return oldestEventDate <= targetTimestamp } + + var oldestEventDate: Date { + get { + oldestEventDateSubject.value + } + set { + oldestEventDateSubject.send(newValue) + } + } } private extension Date { - private static let oneDayInSeconds: TimeInterval = 8.6 * 10e3 - func subtractingDays(_ days: UInt) -> Date { - addingTimeInterval(-TimeInterval(days) * Self.oneDayInSeconds) + addingTimeInterval(-TimeInterval(days) * PollHistoryConstants.oneDayInSeconds) } } diff --git a/RiotSwiftUI/Modules/Room/PollHistory/Service/Mock/MockPollHistoryService.swift b/RiotSwiftUI/Modules/Room/PollHistory/Service/Mock/MockPollHistoryService.swift index d6b12c3d26..8b4ab063b0 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/Service/Mock/MockPollHistoryService.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/Service/Mock/MockPollHistoryService.swift @@ -37,6 +37,11 @@ final class MockPollHistoryService: PollHistoryServiceProtocol { } var hasNextBatch: Bool = true + + var fetchedUpToPublisher: AnyPublisher = Just(.init()).eraseToAnyPublisher() + var fetchedUpTo: AnyPublisher { + fetchedUpToPublisher + } } private extension MockPollHistoryService { diff --git a/RiotSwiftUI/Modules/Room/PollHistory/Service/PollHistoryServiceProtocol.swift b/RiotSwiftUI/Modules/Room/PollHistory/Service/PollHistoryServiceProtocol.swift index 85f0a91372..661c2dce47 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/Service/PollHistoryServiceProtocol.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/Service/PollHistoryServiceProtocol.swift @@ -31,4 +31,8 @@ protocol PollHistoryServiceProtocol { /// 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 } } From 69891f141724ef1f6cdda4fc1ce3139f9a794702 Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Wed, 25 Jan 2023 16:04:47 +0100 Subject: [PATCH 06/23] Disable load more button if needed --- .../Room/PollHistory/View/PollHistory.swift | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/RiotSwiftUI/Modules/Room/PollHistory/View/PollHistory.swift b/RiotSwiftUI/Modules/Room/PollHistory/View/PollHistory.swift index c0dfd5c12c..9289740e0c 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/View/PollHistory.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/View/PollHistory.swift @@ -73,19 +73,22 @@ struct PollHistory: View { } } + @ViewBuilder private var loadMoreButton: some View { - HStack(spacing: 8) { - if viewModel.viewState.isLoading { - spinner - } - - Button { - viewModel.send(viewAction: .loadMoreContent) - } label: { - Text(VectorL10n.pollHistoryLoadMore) - .font(theme.fonts.body) + 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) + } + .disabled(viewModel.viewState.isLoading) } - .disabled(viewModel.viewState.isLoading || !viewModel.viewState.canLoadMoreContent) } } From 8ab4d1b15e0510ee47fd2e86de7f99a5b047c8ec Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Wed, 25 Jan 2023 16:06:16 +0100 Subject: [PATCH 07/23] Restore default constants --- RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift | 2 +- .../Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift index e88d98367d..a20d37205c 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift @@ -17,7 +17,7 @@ // MARK: View model enum PollHistoryConstants { - static let chunkSizeInDays: UInt = 10 + static let chunkSizeInDays: UInt = 30 static let oneDayInSeconds: TimeInterval = 8.6 * 10e3 } diff --git a/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift b/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift index e6569174a2..830d799b89 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift @@ -62,7 +62,7 @@ final class PollHistoryService: PollHistoryServiceProtocol { private extension PollHistoryService { enum Constants { - static let pageSize: UInt = 500 + static let pageSize: UInt = 250 } func setup(timeline: MXEventTimeline) { From 6dcebf785a5315eb5e32ea6e0ebaeddbdf3893a0 Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Wed, 25 Jan 2023 16:11:36 +0100 Subject: [PATCH 08/23] Remove dynamic poll updates --- .../PollHistory/PollHistoryViewModel.swift | 26 +++++-------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift index ef7599830b..3d6ba34fc4 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift @@ -38,17 +38,17 @@ final class PollHistoryViewModel: PollHistoryViewModelType, PollHistoryViewModel switch viewAction { case .viewAppeared: setupUpdateSubscriptions() - fetchFirstBatch() + fetchContent() case .segmentDidChange: updateViewState() case .loadMoreContent: - fetchMoreContent() + fetchContent() } } } private extension PollHistoryViewModel { - func fetchFirstBatch() { + func fetchContent() { state.isLoading = true pollService @@ -57,21 +57,7 @@ private extension PollHistoryViewModel { .sink { [weak self] completion in self?.handleBatchEnded(completion: completion) } receiveValue: { [weak self] polls in - self?.polls = polls - self?.updateViewState() - } - .store(in: &subcriptions) - } - - func fetchMoreContent() { - state.isLoading = true - - pollService - .nextBatch() - .sink { [weak self] completion in - self?.handleBatchEnded(completion: completion) - } receiveValue: { [weak self] poll in - self?.add(poll: poll) + self?.add(polls: polls) self?.updateViewState() } .store(in: &subcriptions) @@ -121,8 +107,8 @@ private extension PollHistoryViewModel { polls?[pollIndex] = poll } - func add(poll: TimelinePollDetails) { - polls?.append(poll) + func add(polls: [TimelinePollDetails]) { + self.polls = (self.polls ?? []) + polls } func updateViewState() { From 78df44d2638ad7cd8d949b6ea0e227fa7b3b5428 Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Wed, 25 Jan 2023 16:23:38 +0100 Subject: [PATCH 09/23] Fix pagination reset --- .../Service/MatrixSDK/PollHistoryService.swift | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift b/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift index 830d799b89..57bcd5e9b6 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift @@ -44,7 +44,7 @@ final class PollHistoryService: PollHistoryServiceProtocol { self.room = room self.chunkSizeInDays = chunkSizeInDays timeline = MXRoomEventTimeline(room: room, andInitialEventId: nil) - setup(timeline: timeline) + setupTimeline() } func nextBatch() -> AnyPublisher { @@ -65,7 +65,9 @@ private extension PollHistoryService { 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) @@ -90,14 +92,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 @@ -105,8 +106,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) } From 63ee508e368898ae5b6bc1603b349755cef3a62c Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Wed, 25 Jan 2023 17:52:39 +0100 Subject: [PATCH 10/23] Handle live polls --- .../PollHistory/PollHistoryViewModel.swift | 8 +++ .../MatrixSDK/PollHistoryService.swift | 56 +++++++++++++++++-- .../Service/Mock/MockPollHistoryService.swift | 5 ++ .../Service/PollHistoryServiceProtocol.swift | 3 + 4 files changed, 67 insertions(+), 5 deletions(-) diff --git a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift index 3d6ba34fc4..13c4418ead 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift @@ -97,6 +97,14 @@ private extension PollHistoryViewModel { .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) } func update(poll: TimelinePollDetails) { diff --git a/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift b/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift index 57bcd5e9b6..bceaed266c 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift @@ -22,15 +22,26 @@ 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? + // polls aggregation private var pollAggregators: [String: PollAggregator] = [:] + private var livePollsIDs: Set = .init() + private var publishedPollsIDs: Set = .init() + + // polls + private var currentBatchSubject: PassthroughSubject? + private var livePollsSubject: PassthroughSubject = .init() + + // polls updates + private let updatesSubject: PassthroughSubject = .init() + private let pollErrorsSubject: PassthroughSubject = .init() + + // timestamps private var targetTimestamp: Date? private var oldestEventDateSubject: CurrentValueSubject = .init(Date.distantFuture) - private var currentBatchSubject: PassthroughSubject? var updates: AnyPublisher { updatesSubject.eraseToAnyPublisher() @@ -45,6 +56,7 @@ final class PollHistoryService: PollHistoryServiceProtocol { self.chunkSizeInDays = chunkSizeInDays timeline = MXRoomEventTimeline(room: room, andInitialEventId: nil) setupTimeline() + setupLiveUpdates() } func nextBatch() -> AnyPublisher { @@ -58,6 +70,17 @@ final class PollHistoryService: PollHistoryServiceProtocol { var fetchedUpTo: AnyPublisher { oldestEventDateSubject.eraseToAnyPublisher() } + + var livePolls: AnyPublisher { + livePollsSubject.eraseToAnyPublisher() + } + + deinit { + guard let roomListener = roomListener else { + return + } + room.removeListener(roomListener) + } } private extension PollHistoryService { @@ -77,6 +100,15 @@ private extension PollHistoryService { } } + func setupLiveUpdates() { + roomListener = room.listen(toEventsOfTypes: [kMXEventTypeStringPollStart, kMXEventTypeStringPollStartMSC3381]) { [weak self] event, _, _ in + if event.eventType == .pollStart { + self?.livePollsIDs.insert(event.eventId) + self?.aggregatePoll(pollStartEvent: event) + } + } + } + func updateTimestamp(event: MXEvent) { oldestEventDate = min(event.originServerDate, oldestEventDate) } @@ -169,7 +201,21 @@ extension PollHistoryService: PollAggregatorDelegate { func pollAggregatorDidStartLoading(_ aggregator: PollAggregator) {} func pollAggregatorDidEndLoading(_ aggregator: PollAggregator) { - currentBatchSubject?.send(.init(poll: aggregator.poll, represent: .started)) + let pollID = aggregator.poll.id + + guard publishedPollsIDs.contains(pollID) == false else { + return + } + + publishedPollsIDs.insert(pollID) + + let newPoll: TimelinePollDetails = .init(poll: aggregator.poll, represent: .started) + + if livePollsIDs.contains(newPoll.id) { + livePollsSubject.send(newPoll) + } else { + currentBatchSubject?.send(newPoll) + } } func pollAggregator(_ aggregator: PollAggregator, didFailWithError: Error) { diff --git a/RiotSwiftUI/Modules/Room/PollHistory/Service/Mock/MockPollHistoryService.swift b/RiotSwiftUI/Modules/Room/PollHistory/Service/Mock/MockPollHistoryService.swift index 8b4ab063b0..89bee9b832 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/Service/Mock/MockPollHistoryService.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/Service/Mock/MockPollHistoryService.swift @@ -42,6 +42,11 @@ final class MockPollHistoryService: PollHistoryServiceProtocol { var fetchedUpTo: AnyPublisher { fetchedUpToPublisher } + + var livePollsPublisher: AnyPublisher = Empty().eraseToAnyPublisher() + var livePolls: AnyPublisher { + livePollsPublisher + } } private extension MockPollHistoryService { diff --git a/RiotSwiftUI/Modules/Room/PollHistory/Service/PollHistoryServiceProtocol.swift b/RiotSwiftUI/Modules/Room/PollHistory/Service/PollHistoryServiceProtocol.swift index 661c2dce47..6335d5d635 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/Service/PollHistoryServiceProtocol.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/Service/PollHistoryServiceProtocol.swift @@ -28,6 +28,9 @@ protocol PollHistoryServiceProtocol { /// 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 } From ce52d5bf7cce2bb03d70053276cc65372c442ba7 Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Wed, 25 Jan 2023 18:19:48 +0100 Subject: [PATCH 11/23] Cleanup code --- .../MatrixSDK/PollHistoryService.swift | 46 ++++++++++++------- 1 file changed, 29 insertions(+), 17 deletions(-) diff --git a/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift b/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift index bceaed266c..395972a47f 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift @@ -27,9 +27,7 @@ final class PollHistoryService: PollHistoryServiceProtocol { private var roomListener: Any? // polls aggregation - private var pollAggregators: [String: PollAggregator] = [:] - private var livePollsIDs: Set = .init() - private var publishedPollsIDs: Set = .init() + private var pollAggregationContexts: [String: PollAggregationContext] = [:] // polls private var currentBatchSubject: PassthroughSubject? @@ -81,6 +79,18 @@ final class PollHistoryService: PollHistoryServiceProtocol { } room.removeListener(roomListener) } + + class PollAggregationContext { + var pollAggregator: PollAggregator? + let isLivePoll: Bool + var published: Bool = false + + init(pollAggregator: PollAggregator? = nil, isLivePoll: Bool, published: Bool = false) { + self.pollAggregator = pollAggregator + self.isLivePoll = isLivePoll + self.published = published + } + } } private extension PollHistoryService { @@ -93,7 +103,7 @@ private extension PollHistoryService { 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) @@ -103,8 +113,7 @@ private extension PollHistoryService { func setupLiveUpdates() { roomListener = room.listen(toEventsOfTypes: [kMXEventTypeStringPollStart, kMXEventTypeStringPollStartMSC3381]) { [weak self] event, _, _ in if event.eventType == .pollStart { - self?.livePollsIDs.insert(event.eventId) - self?.aggregatePoll(pollStartEvent: event) + self?.aggregatePoll(pollStartEvent: event, isLivePoll: true) } } } @@ -154,16 +163,21 @@ private extension PollHistoryService { currentBatchSubject = nil } - func aggregatePoll(pollStartEvent: MXEvent) { - guard pollAggregators[pollStartEvent.eventId] == nil else { - return - } + func aggregatePoll(pollStartEvent: MXEvent, isLivePoll: Bool) { + let eventId: String = pollStartEvent.eventId - guard let aggregator = try? PollAggregator(session: room.mxSession, room: room, pollEvent: pollStartEvent, delegate: self) else { + guard pollAggregationContexts[eventId] == nil else { return } - pollAggregators[pollStartEvent.eventId] = aggregator + let newContext: PollAggregationContext = .init(isLivePoll: isLivePoll) + pollAggregationContexts[eventId] = newContext + + do { + newContext.pollAggregator = try PollAggregator(session: room.mxSession, room: room, pollEvent: pollStartEvent, delegate: self) + } catch { + pollAggregationContexts.removeValue(forKey: eventId) + } } var timestampTargetReached: Bool { @@ -201,17 +215,15 @@ extension PollHistoryService: PollAggregatorDelegate { func pollAggregatorDidStartLoading(_ aggregator: PollAggregator) {} func pollAggregatorDidEndLoading(_ aggregator: PollAggregator) { - let pollID = aggregator.poll.id - - guard publishedPollsIDs.contains(pollID) == false else { + guard let context = pollAggregationContexts[aggregator.poll.id], !context.published else { return } - publishedPollsIDs.insert(pollID) + context.published = true let newPoll: TimelinePollDetails = .init(poll: aggregator.poll, represent: .started) - if livePollsIDs.contains(newPoll.id) { + if context.isLivePoll { livePollsSubject.send(newPoll) } else { currentBatchSubject?.send(newPoll) From a69c4d24e9cf1f79fc673f0a1194a34cfbe6bb8a Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Wed, 25 Jan 2023 18:40:09 +0100 Subject: [PATCH 12/23] Add alert on error --- Riot/Assets/en.lproj/Vector.strings | 1 + Riot/Generated/Strings.swift | 4 ++++ .../Coordinator/PollHistoryCoordinator.swift | 17 ++++++++++++++++- .../Room/PollHistory/PollHistoryModels.swift | 2 +- .../Room/PollHistory/PollHistoryViewModel.swift | 11 ++--------- 5 files changed, 24 insertions(+), 11 deletions(-) diff --git a/Riot/Assets/en.lproj/Vector.strings b/Riot/Assets/en.lproj/Vector.strings index 49b6fb73c3..40cc62e9c4 100644 --- a/Riot/Assets/en.lproj/Vector.strings +++ b/Riot/Assets/en.lproj/Vector.strings @@ -2303,6 +2303,7 @@ Tap the + to start adding people."; "poll_history_no_active_poll_period_text" = "There are no active polls for the past %@ days. Load more polls to view polls for previous months"; "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_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 44ea603d04..ab9d80f24c 100644 --- a/Riot/Generated/Strings.swift +++ b/Riot/Generated/Strings.swift @@ -4819,6 +4819,10 @@ public class VectorL10n: NSObject { public static var pollHistoryActiveSegmentTitle: String { return VectorL10n.tr("Vector", "poll_history_active_segment_title") } + /// Error fetching polls. + public static var pollHistoryFetchingError: String { + return VectorL10n.tr("Vector", "poll_history_fetching_error") + } /// Load more polls public static var pollHistoryLoadMore: String { return VectorL10n.tr("Vector", "poll_history_load_more") diff --git a/RiotSwiftUI/Modules/Room/PollHistory/Coordinator/PollHistoryCoordinator.swift b/RiotSwiftUI/Modules/Room/PollHistory/Coordinator/PollHistoryCoordinator.swift index 0311cda4e3..e0ee3f54ca 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/Coordinator/PollHistoryCoordinator.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/Coordinator/PollHistoryCoordinator.swift @@ -44,7 +44,10 @@ final class PollHistoryCoordinator: Coordinator, Presentable { func start() { MXLog.debug("[PollHistoryCoordinator] did start.") pollHistoryViewModel.completion = { [weak self] result in - self?.completion?() + switch result { + case .genericError: + self?.showErrorAlert() + } } } @@ -52,3 +55,15 @@ final class PollHistoryCoordinator: Coordinator, Presentable { pollHistoryHostingController } } + +private extension PollHistoryCoordinator { + func showErrorAlert() { + let alert = UIAlertController(title: VectorL10n.pollHistoryFetchingError, + message: nil, + preferredStyle: .alert) + + let cancelAction = UIAlertAction(title: VectorL10n.ok, style: .cancel) + alert.addAction(cancelAction) + pollHistoryHostingController.present(alert, animated: true, completion: nil) + } +} diff --git a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift index a20d37205c..0b6213787c 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift @@ -22,7 +22,7 @@ enum PollHistoryConstants { } enum PollHistoryViewModelResult: Equatable { - #warning("e.g. show poll detail") + case genericError } // MARK: View diff --git a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift index 13c4418ead..1687cf2f45 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift @@ -70,8 +70,8 @@ private extension PollHistoryViewModel { switch completion { case .finished: break - case .failure(_): - #warning("Handle errors") + case .failure: + self.completion?(.genericError) } } @@ -86,13 +86,6 @@ private extension PollHistoryViewModel { } .store(in: &subcriptions) - pollService - .pollErrors - .sink { detail in - #warning("Handle errors") - } - .store(in: &subcriptions) - pollService .fetchedUpTo .weakAssign(to: \.state.syncedUpTo, on: self) From d51938e5442c7e6f7135bfb7a6ce3d4c03e0875c Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Wed, 25 Jan 2023 19:21:56 +0100 Subject: [PATCH 13/23] Improve MockPollHistoryScreenState --- .../MockPollHistoryScreenState.swift | 25 +++++++++---------- .../Service/Mock/MockPollHistoryService.swift | 6 ++--- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/RiotSwiftUI/Modules/Room/PollHistory/MockPollHistoryScreenState.swift b/RiotSwiftUI/Modules/Room/PollHistory/MockPollHistoryScreenState.swift index d939dab2fc..6d15ca9878 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/MockPollHistoryScreenState.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/MockPollHistoryScreenState.swift @@ -26,8 +26,8 @@ enum MockPollHistoryScreenState: MockScreenState, CaseIterable { // mock that screen. case active case past - case activeEmpty - case pastEmpty + case empty + case emptyNoMoreContent case loading /// The associated screen @@ -37,7 +37,7 @@ 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 { @@ -45,21 +45,20 @@ enum MockPollHistoryScreenState: MockScreenState, CaseIterable { pollHistoryMode = .active case .past: pollHistoryMode = .past - case .activeEmpty: + case .empty: pollHistoryMode = .active pollService.nextBatchPublisher = Empty(completeImmediately: true, - outputType: TimelinePollDetails.self, - failureType: Error.self).eraseToAnyPublisher() - case .pastEmpty: - pollHistoryMode = .past + outputType: TimelinePollDetails.self, + failureType: Error.self).eraseToAnyPublisher() + case .emptyNoMoreContent: + pollService.hasNextBatch = false pollService.nextBatchPublisher = Empty(completeImmediately: true, - outputType: TimelinePollDetails.self, - failureType: Error.self).eraseToAnyPublisher() + outputType: TimelinePollDetails.self, + failureType: Error.self).eraseToAnyPublisher() case .loading: - pollHistoryMode = .active pollService.nextBatchPublisher = Empty(completeImmediately: false, - outputType: TimelinePollDetails.self, - failureType: Error.self).eraseToAnyPublisher() + outputType: TimelinePollDetails.self, + failureType: Error.self).eraseToAnyPublisher() } let viewModel = PollHistoryViewModel(mode: pollHistoryMode, pollService: pollService) diff --git a/RiotSwiftUI/Modules/Room/PollHistory/Service/Mock/MockPollHistoryService.swift b/RiotSwiftUI/Modules/Room/PollHistory/Service/Mock/MockPollHistoryService.swift index 89bee9b832..a75195a0f7 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/Service/Mock/MockPollHistoryService.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/Service/Mock/MockPollHistoryService.swift @@ -36,7 +36,7 @@ final class MockPollHistoryService: PollHistoryServiceProtocol { pollErrorPublisher } - var hasNextBatch: Bool = true + var hasNextBatch = true var fetchedUpToPublisher: AnyPublisher = Just(.init()).eraseToAnyPublisher() var fetchedUpTo: AnyPublisher { @@ -51,7 +51,7 @@ final class MockPollHistoryService: PollHistoryServiceProtocol { 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)?", @@ -68,7 +68,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)?", From 51f612186ac6161e7fe693c742faeb4e56480c44 Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Wed, 25 Jan 2023 19:41:48 +0100 Subject: [PATCH 14/23] Add more MockPollHistoryScreenState cases --- .../MockPollHistoryScreenState.swift | 38 ++++++++++++++----- .../Service/Mock/MockPollHistoryService.swift | 12 +++--- 2 files changed, 36 insertions(+), 14 deletions(-) diff --git a/RiotSwiftUI/Modules/Room/PollHistory/MockPollHistoryScreenState.swift b/RiotSwiftUI/Modules/Room/PollHistory/MockPollHistoryScreenState.swift index 6d15ca9878..15e217e4fd 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/MockPollHistoryScreenState.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/MockPollHistoryScreenState.swift @@ -25,8 +25,11 @@ enum MockPollHistoryScreenState: MockScreenState, CaseIterable { // with specific, minimal associated data that will allow you // mock that screen. case active + case activeNoMoreContent case past + case contentLoading case empty + case emptyLoading case emptyNoMoreContent case loading @@ -43,27 +46,34 @@ enum MockPollHistoryScreenState: MockScreenState, CaseIterable { switch self { case .active: pollHistoryMode = .active + case .activeNoMoreContent: + pollHistoryMode = .active + pollService.hasNextBatch = false case .past: pollHistoryMode = .past + case .contentLoading: + pollService.nextBatchPublishers.append(loadingPolls) case .empty: pollHistoryMode = .active - pollService.nextBatchPublisher = Empty(completeImmediately: true, - outputType: TimelinePollDetails.self, - failureType: Error.self).eraseToAnyPublisher() + pollService.nextBatchPublishers = [noPolls] + case .emptyLoading: + pollService.nextBatchPublishers = [noPolls, loadingPolls] case .emptyNoMoreContent: pollService.hasNextBatch = false - pollService.nextBatchPublisher = Empty(completeImmediately: true, - outputType: TimelinePollDetails.self, - failureType: Error.self).eraseToAnyPublisher() + pollService.nextBatchPublishers = [noPolls] case .loading: - pollService.nextBatchPublisher = Empty(completeImmediately: false, - outputType: TimelinePollDetails.self, - failureType: Error.self).eraseToAnyPublisher() + pollService.nextBatchPublishers = [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], @@ -72,3 +82,13 @@ enum MockPollHistoryScreenState: MockScreenState, CaseIterable { ) } } + +private extension MockPollHistoryScreenState { + var noPolls: AnyPublisher { + Empty(completeImmediately: true).eraseToAnyPublisher() + } + + var loadingPolls: AnyPublisher { + Empty(completeImmediately: false).eraseToAnyPublisher() + } +} diff --git a/RiotSwiftUI/Modules/Room/PollHistory/Service/Mock/MockPollHistoryService.swift b/RiotSwiftUI/Modules/Room/PollHistory/Service/Mock/MockPollHistoryService.swift index a75195a0f7..277c6a647d 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/Service/Mock/MockPollHistoryService.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/Service/Mock/MockPollHistoryService.swift @@ -17,13 +17,15 @@ import Combine final class MockPollHistoryService: PollHistoryServiceProtocol { - lazy var nextBatchPublisher: AnyPublisher = (activePollsData + pastPollsData) - .publisher - .setFailureType(to: Error.self) - .eraseToAnyPublisher() + lazy var nextBatchPublishers: [AnyPublisher] = [ + (activePollsData + pastPollsData) + .publisher + .setFailureType(to: Error.self) + .eraseToAnyPublisher() + ] func nextBatch() -> AnyPublisher { - nextBatchPublisher + nextBatchPublishers.isEmpty ? Empty().eraseToAnyPublisher() : nextBatchPublishers.removeFirst() } var updatesPublisher: AnyPublisher = Empty().eraseToAnyPublisher() From 48e80b5e26119211b6312564817bb8afa0145cc4 Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Wed, 25 Jan 2023 19:58:02 +0100 Subject: [PATCH 15/23] Add ui tests --- .../MockPollHistoryScreenState.swift | 2 +- .../Test/UI/PollHistoryUITests.swift | 64 +++++++++++++++---- .../Room/PollHistory/View/PollHistory.swift | 1 + 3 files changed, 52 insertions(+), 15 deletions(-) diff --git a/RiotSwiftUI/Modules/Room/PollHistory/MockPollHistoryScreenState.swift b/RiotSwiftUI/Modules/Room/PollHistory/MockPollHistoryScreenState.swift index 15e217e4fd..e2e8278ea2 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/MockPollHistoryScreenState.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/MockPollHistoryScreenState.swift @@ -25,8 +25,8 @@ enum MockPollHistoryScreenState: MockScreenState, CaseIterable { // with specific, minimal associated data that will allow you // mock that screen. case active - case activeNoMoreContent case past + case activeNoMoreContent case contentLoading case empty case emptyLoading diff --git a/RiotSwiftUI/Modules/Room/PollHistory/Test/UI/PollHistoryUITests.swift b/RiotSwiftUI/Modules/Room/PollHistory/Test/UI/PollHistoryUITests.swift index 986fd37bdc..ddce4978ca 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"] + + XCTAssertTrue(items.exists) + XCTAssertFalse(emptyText.exists) + XCTAssertTrue(loadMoreButton.exists) + XCTAssertFalse(loadMoreButton.isEnabled) + } + + 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(emptyText.exists) - XCTAssertTrue(selectedSegment.exists) - XCTAssertEqual(selectedSegment.value as? String, VectorL10n.accessibilitySelected) - XCTAssertFalse(winningOption.exists) + XCTAssertTrue(loadMoreButton.exists) + XCTAssertTrue(loadMoreButton.isEnabled) } - func testLoaderIsShowing() { - app.goToScreenWithIdentifier(MockPollHistoryScreenState.loading.title) - let title = app.navigationBars.firstMatch.identifier - let loadingText = app.staticTexts["PollHistory.loadingText"] + 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"] - XCTAssertEqual(title, VectorL10n.pollHistoryTitle) XCTAssertFalse(items.exists) - XCTAssertTrue(loadingText.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/View/PollHistory.swift b/RiotSwiftUI/Modules/Room/PollHistory/View/PollHistory.swift index 9289740e0c..43e1b28e8b 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/View/PollHistory.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/View/PollHistory.swift @@ -87,6 +87,7 @@ struct PollHistory: View { Text(VectorL10n.pollHistoryLoadMore) .font(theme.fonts.body) } + .accessibilityIdentifier("PollHistory.loadMore") .disabled(viewModel.viewState.isLoading) } } From ef6910a5f99839cb30220a61c78d1191beb2f29d Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Wed, 25 Jan 2023 20:16:24 +0100 Subject: [PATCH 16/23] Add UTs --- .../Test/Unit/PollHistoryViewModelTests.swift | 34 ++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/RiotSwiftUI/Modules/Room/PollHistory/Test/Unit/PollHistoryViewModelTests.swift b/RiotSwiftUI/Modules/Room/PollHistory/Test/Unit/PollHistoryViewModelTests.swift index e2eff74753..86487c1d1e 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/Test/Unit/PollHistoryViewModelTests.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/Test/Unit/PollHistoryViewModelTests.swift @@ -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 = [loadingPublisher, emptyPublisher] viewModel.process(viewAction: .viewAppeared) XCTAssertTrue(viewModel.state.isLoading) + viewModel.process(viewAction: .viewAppeared) + XCTAssertFalse(viewModel.state.isLoading) } func testUpdatesAreHandled() throws { @@ -79,6 +81,14 @@ final class PollHistoryViewModelTests: XCTestCase { let pollDates = try polls.map(\.startDate) XCTAssertEqual(pollDates, pollDates.sorted(by: { $0 > $1 })) } + + func testLivePollsAreHandled() throws { + pollHistoryService.nextBatchPublishers = [emptyPublisher] + pollHistoryService.livePollsPublisher = Just(mockPoll).eraseToAnyPublisher() + viewModel.process(viewAction: .viewAppeared) + XCTAssertEqual(viewModel.state.polls?.count, 1) + XCTAssertEqual(viewModel.state.polls?.first?.id, "id") + } } private extension PollHistoryViewModelTests { @@ -87,4 +97,26 @@ private extension PollHistoryViewModelTests { try XCTUnwrap(viewModel.state.polls) } } + + var loadingPublisher: AnyPublisher { + Empty(completeImmediately: false, outputType: TimelinePollDetails.self, failureType: Error.self).eraseToAnyPublisher() + } + + var emptyPublisher: AnyPublisher { + Empty(completeImmediately: true, outputType: TimelinePollDetails.self, failureType: Error.self).eraseToAnyPublisher() + } + + 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) + } } From 31ca4e3e380e62c92db15073ffb680b313795af1 Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Thu, 26 Jan 2023 10:09:28 +0100 Subject: [PATCH 17/23] Refine timestamp logics --- .../Service/MatrixSDK/PollHistoryService.swift | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift b/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift index 395972a47f..35208a9543 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift @@ -38,8 +38,8 @@ final class PollHistoryService: PollHistoryServiceProtocol { private let pollErrorsSubject: PassthroughSubject = .init() // timestamps - private var targetTimestamp: Date? - private var oldestEventDateSubject: CurrentValueSubject = .init(Date.distantFuture) + private var targetTimestamp: Date = .init() + private var oldestEventDateSubject: CurrentValueSubject = .init(.init()) var updates: AnyPublisher { updatesSubject.eraseToAnyPublisher() @@ -83,7 +83,7 @@ final class PollHistoryService: PollHistoryServiceProtocol { class PollAggregationContext { var pollAggregator: PollAggregator? let isLivePoll: Bool - var published: Bool = false + var published: Bool init(pollAggregator: PollAggregator? = nil, isLivePoll: Bool, published: Bool = false) { self.pollAggregator = pollAggregator @@ -123,7 +123,7 @@ private extension PollHistoryService { } func startPagination() -> AnyPublisher { - let startingTimestamp = targetTimestamp ?? .init() + let startingTimestamp = oldestEventDate targetTimestamp = startingTimestamp.subtractingDays(chunkSizeInDays) let batchSubject = PassthroughSubject() @@ -181,10 +181,7 @@ private extension PollHistoryService { } var timestampTargetReached: Bool { - guard let targetTimestamp = targetTimestamp else { - return true - } - return oldestEventDate <= targetTimestamp + oldestEventDate <= targetTimestamp } var oldestEventDate: Date { @@ -212,7 +209,7 @@ private extension MXEvent { // MARK: - PollAggregatorDelegate extension PollHistoryService: PollAggregatorDelegate { - func pollAggregatorDidStartLoading(_ aggregator: PollAggregator) {} + func pollAggregatorDidStartLoading(_ aggregator: PollAggregator) { } func pollAggregatorDidEndLoading(_ aggregator: PollAggregator) { guard let context = pollAggregationContexts[aggregator.poll.id], !context.published else { From 8138410e1e83064d63bc8febef156d9c3c997641 Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Thu, 26 Jan 2023 10:09:47 +0100 Subject: [PATCH 18/23] Improve error handling --- .../Modules/Room/PollHistory/PollHistoryViewModel.swift | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift index 1687cf2f45..c73e2ea322 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift @@ -58,7 +58,6 @@ private extension PollHistoryViewModel { self?.handleBatchEnded(completion: completion) } receiveValue: { [weak self] polls in self?.add(polls: polls) - self?.updateViewState() } .store(in: &subcriptions) } @@ -71,8 +70,11 @@ private extension PollHistoryViewModel { case .finished: break case .failure: + polls = polls ?? [] self.completion?(.genericError) } + + updateViewState() } func setupUpdateSubscriptions() { From f9c570e187fe61957357ef30739863f2450806f9 Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Thu, 26 Jan 2023 10:14:26 +0100 Subject: [PATCH 19/23] Cleanup unused code --- .../Service/MatrixSDK/PollHistoryService.swift | 16 ++++++---------- .../Service/Mock/MockPollHistoryService.swift | 5 ----- .../Service/PollHistoryServiceProtocol.swift | 6 +----- 3 files changed, 7 insertions(+), 20 deletions(-) diff --git a/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift b/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift index 35208a9543..eb73ff7327 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift @@ -35,7 +35,6 @@ final class PollHistoryService: PollHistoryServiceProtocol { // polls updates private let updatesSubject: PassthroughSubject = .init() - private let pollErrorsSubject: PassthroughSubject = .init() // timestamps private var targetTimestamp: Date = .init() @@ -45,10 +44,6 @@ final class PollHistoryService: PollHistoryServiceProtocol { updatesSubject.eraseToAnyPublisher() } - var pollErrors: AnyPublisher { - pollErrorsSubject.eraseToAnyPublisher() - } - init(room: MXRoom, chunkSizeInDays: UInt) { self.room = room self.chunkSizeInDays = chunkSizeInDays @@ -211,8 +206,10 @@ private extension MXEvent { extension PollHistoryService: PollAggregatorDelegate { func pollAggregatorDidStartLoading(_ aggregator: PollAggregator) { } + func pollAggregator(_ aggregator: PollAggregator, didFailWithError: Error) { } + func pollAggregatorDidEndLoading(_ aggregator: PollAggregator) { - guard let context = pollAggregationContexts[aggregator.poll.id], !context.published else { + guard let context = pollAggregationContexts[aggregator.poll.id], context.published == false else { return } @@ -227,11 +224,10 @@ extension PollHistoryService: PollAggregatorDelegate { } } - func pollAggregator(_ aggregator: PollAggregator, didFailWithError: Error) { - pollErrorsSubject.send(didFailWithError) - } - 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 277c6a647d..c98f4e136d 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/Service/Mock/MockPollHistoryService.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/Service/Mock/MockPollHistoryService.swift @@ -33,11 +33,6 @@ final class MockPollHistoryService: PollHistoryServiceProtocol { updatesPublisher } - var pollErrorPublisher: AnyPublisher = Empty().eraseToAnyPublisher() - var pollErrors: AnyPublisher { - pollErrorPublisher - } - var hasNextBatch = true var fetchedUpToPublisher: AnyPublisher = Just(.init()).eraseToAnyPublisher() diff --git a/RiotSwiftUI/Modules/Room/PollHistory/Service/PollHistoryServiceProtocol.swift b/RiotSwiftUI/Modules/Room/PollHistory/Service/PollHistoryServiceProtocol.swift index 6335d5d635..5132478cc8 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/Service/PollHistoryServiceProtocol.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/Service/PollHistoryServiceProtocol.swift @@ -21,13 +21,9 @@ 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 } From 0a4b507ffb15dbcfd630a17e8355224b63e80bd6 Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Thu, 26 Jan 2023 10:24:36 +0100 Subject: [PATCH 20/23] Improve tests --- .../MockPollHistoryScreenState.swift | 20 ++++++---- .../Test/Unit/PollHistoryViewModelTests.swift | 40 ++++++++++++++----- 2 files changed, 42 insertions(+), 18 deletions(-) diff --git a/RiotSwiftUI/Modules/Room/PollHistory/MockPollHistoryScreenState.swift b/RiotSwiftUI/Modules/Room/PollHistory/MockPollHistoryScreenState.swift index e2e8278ea2..a1c12e5f33 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/MockPollHistoryScreenState.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/MockPollHistoryScreenState.swift @@ -52,17 +52,17 @@ enum MockPollHistoryScreenState: MockScreenState, CaseIterable { case .past: pollHistoryMode = .past case .contentLoading: - pollService.nextBatchPublishers.append(loadingPolls) + pollService.nextBatchPublishers.append(MockPollPublisher.loadingPolls) case .empty: pollHistoryMode = .active - pollService.nextBatchPublishers = [noPolls] + pollService.nextBatchPublishers = [MockPollPublisher.emptyPolls] case .emptyLoading: - pollService.nextBatchPublishers = [noPolls, loadingPolls] + pollService.nextBatchPublishers = [MockPollPublisher.emptyPolls, MockPollPublisher.loadingPolls] case .emptyNoMoreContent: pollService.hasNextBatch = false - pollService.nextBatchPublishers = [noPolls] + pollService.nextBatchPublishers = [MockPollPublisher.emptyPolls] case .loading: - pollService.nextBatchPublishers = [loadingPolls] + pollService.nextBatchPublishers = [MockPollPublisher.loadingPolls] } let viewModel = PollHistoryViewModel(mode: pollHistoryMode, pollService: pollService) @@ -83,12 +83,16 @@ enum MockPollHistoryScreenState: MockScreenState, CaseIterable { } } -private extension MockPollHistoryScreenState { - var noPolls: AnyPublisher { +enum MockPollPublisher { + static var emptyPolls: AnyPublisher { Empty(completeImmediately: true).eraseToAnyPublisher() } - var loadingPolls: AnyPublisher { + 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/Test/Unit/PollHistoryViewModelTests.swift b/RiotSwiftUI/Modules/Room/PollHistory/Test/Unit/PollHistoryViewModelTests.swift index 86487c1d1e..95a6e7e721 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/Test/Unit/PollHistoryViewModelTests.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/Test/Unit/PollHistoryViewModelTests.swift @@ -42,7 +42,7 @@ final class PollHistoryViewModelTests: XCTestCase { func testLoadingStateIsTrueWhileLoading() { XCTAssertFalse(viewModel.state.isLoading) - pollHistoryService.nextBatchPublishers = [loadingPublisher, emptyPublisher] + pollHistoryService.nextBatchPublishers = [MockPollPublisher.loadingPolls, MockPollPublisher.emptyPolls] viewModel.process(viewAction: .viewAppeared) XCTAssertTrue(viewModel.state.isLoading) viewModel.process(viewAction: .viewAppeared) @@ -83,12 +83,40 @@ final class PollHistoryViewModelTests: XCTestCase { } func testLivePollsAreHandled() throws { - pollHistoryService.nextBatchPublishers = [emptyPublisher] + 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 { + let expectation = expectation(description: #function) + + pollHistoryService.nextBatchPublishers = [MockPollPublisher.failure] + viewModel.completion = { event in + XCTAssertEqual(event, .genericError) + expectation.fulfill() + } + viewModel.process(viewAction: .viewAppeared) + XCTAssertFalse(viewModel.state.isLoading) + XCTAssertNotNil(viewModel.state.polls) + + wait(for: [expectation], timeout: 1.0) + } } private extension PollHistoryViewModelTests { @@ -98,14 +126,6 @@ private extension PollHistoryViewModelTests { } } - var loadingPublisher: AnyPublisher { - Empty(completeImmediately: false, outputType: TimelinePollDetails.self, failureType: Error.self).eraseToAnyPublisher() - } - - var emptyPublisher: AnyPublisher { - Empty(completeImmediately: true, outputType: TimelinePollDetails.self, failureType: Error.self).eraseToAnyPublisher() - } - var mockPoll: TimelinePollDetails { .init(id: "id", question: "Do you like polls?", From 13a317e5eb5bef8b7555e76c44297bf1652b23ce Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Thu, 26 Jan 2023 10:38:41 +0100 Subject: [PATCH 21/23] Add changelog.d file --- changelog.d/pr-7303.change | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/pr-7303.change diff --git a/changelog.d/pr-7303.change b/changelog.d/pr-7303.change new file mode 100644 index 0000000000..fc6bbdb3a8 --- /dev/null +++ b/changelog.d/pr-7303.change @@ -0,0 +1 @@ +Poll: add a feature to load more polls in the poll history. From 6caf8d707d7e9f516cadbf1f638b980965a5f25c Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Thu, 26 Jan 2023 14:52:33 +0100 Subject: [PATCH 22/23] Fix alert presentation --- .../Coordinator/PollHistoryCoordinator.swift | 19 ++----------------- .../Room/PollHistory/PollHistoryModels.swift | 3 ++- .../PollHistory/PollHistoryViewModel.swift | 2 +- .../Test/Unit/PollHistoryViewModelTests.swift | 11 ++--------- .../Room/PollHistory/View/PollHistory.swift | 3 +++ 5 files changed, 10 insertions(+), 28 deletions(-) diff --git a/RiotSwiftUI/Modules/Room/PollHistory/Coordinator/PollHistoryCoordinator.swift b/RiotSwiftUI/Modules/Room/PollHistory/Coordinator/PollHistoryCoordinator.swift index e0ee3f54ca..13ed8e3d23 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/Coordinator/PollHistoryCoordinator.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/Coordinator/PollHistoryCoordinator.swift @@ -43,11 +43,8 @@ final class PollHistoryCoordinator: Coordinator, Presentable { func start() { MXLog.debug("[PollHistoryCoordinator] did start.") - pollHistoryViewModel.completion = { [weak self] result in - switch result { - case .genericError: - self?.showErrorAlert() - } + pollHistoryViewModel.completion = { _ in + } } @@ -55,15 +52,3 @@ final class PollHistoryCoordinator: Coordinator, Presentable { pollHistoryHostingController } } - -private extension PollHistoryCoordinator { - func showErrorAlert() { - let alert = UIAlertController(title: VectorL10n.pollHistoryFetchingError, - message: nil, - preferredStyle: .alert) - - let cancelAction = UIAlertAction(title: VectorL10n.ok, style: .cancel) - alert.addAction(cancelAction) - pollHistoryHostingController.present(alert, animated: true, completion: nil) - } -} diff --git a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift index 0b6213787c..f342d50af5 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift @@ -22,7 +22,7 @@ enum PollHistoryConstants { } enum PollHistoryViewModelResult: Equatable { - case genericError + } // MARK: View @@ -34,6 +34,7 @@ enum PollHistoryMode: CaseIterable { struct PollHistoryViewBindings { var mode: PollHistoryMode + var alertInfo: AlertInfo? } struct PollHistoryViewState: BindableState { diff --git a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift index c73e2ea322..9dbffb7a88 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift @@ -71,7 +71,7 @@ private extension PollHistoryViewModel { break case .failure: polls = polls ?? [] - self.completion?(.genericError) + state.bindings.alertInfo = .init(id: true, title: VectorL10n.pollHistoryFetchingError) } updateViewState() diff --git a/RiotSwiftUI/Modules/Room/PollHistory/Test/Unit/PollHistoryViewModelTests.swift b/RiotSwiftUI/Modules/Room/PollHistory/Test/Unit/PollHistoryViewModelTests.swift index 95a6e7e721..efce641d4e 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"); @@ -104,18 +104,11 @@ final class PollHistoryViewModelTests: XCTestCase { } func testAfterFailureCompletionIsCalled() throws { - let expectation = expectation(description: #function) - pollHistoryService.nextBatchPublishers = [MockPollPublisher.failure] - viewModel.completion = { event in - XCTAssertEqual(event, .genericError) - expectation.fulfill() - } viewModel.process(viewAction: .viewAppeared) XCTAssertFalse(viewModel.state.isLoading) XCTAssertNotNil(viewModel.state.polls) - - wait(for: [expectation], timeout: 1.0) + XCTAssertNotNil(viewModel.state.bindings.alertInfo) } } diff --git a/RiotSwiftUI/Modules/Room/PollHistory/View/PollHistory.swift b/RiotSwiftUI/Modules/Room/PollHistory/View/PollHistory.swift index 43e1b28e8b..703390ac35 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 From fc8ee97c1b106c20b75cecfebef73c35e036dbec Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Thu, 26 Jan 2023 15:16:48 +0100 Subject: [PATCH 23/23] Use Calendar to compute target dates --- .../Modules/Room/PollHistory/PollHistoryModels.swift | 1 - .../Modules/Room/PollHistory/PollHistoryViewModel.swift | 8 +++++--- .../Service/MatrixSDK/PollHistoryService.swift | 6 +++--- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift index f342d50af5..fda7b0b3b8 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryModels.swift @@ -18,7 +18,6 @@ enum PollHistoryConstants { static let chunkSizeInDays: UInt = 30 - static let oneDayInSeconds: TimeInterval = 8.6 * 10e3 } enum PollHistoryViewModelResult: Equatable { diff --git a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift index 9dbffb7a88..4fe1f49f2f 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/PollHistoryViewModel.swift @@ -142,8 +142,10 @@ extension PollHistoryViewModel.Context { } } - var syncedPastDays: UInt { - let timeDelta = max(viewState.syncStartDate.timeIntervalSince(viewState.syncedUpTo), 0) - return UInt((timeDelta / PollHistoryConstants.oneDayInSeconds).rounded()) + 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 eb73ff7327..7f6d8c5f6c 100644 --- a/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift +++ b/RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift @@ -119,7 +119,7 @@ private extension PollHistoryService { func startPagination() -> AnyPublisher { let startingTimestamp = oldestEventDate - targetTimestamp = startingTimestamp.subtractingDays(chunkSizeInDays) + targetTimestamp = startingTimestamp.subtractingDays(chunkSizeInDays) ?? startingTimestamp let batchSubject = PassthroughSubject() currentBatchSubject = batchSubject @@ -190,8 +190,8 @@ private extension PollHistoryService { } private extension Date { - func subtractingDays(_ days: UInt) -> Date { - addingTimeInterval(-TimeInterval(days) * PollHistoryConstants.oneDayInSeconds) + func subtractingDays(_ days: UInt) -> Date? { + Calendar.current.date(byAdding: DateComponents(day: -Int(days)), to: self) } }