Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix: allow to render a TimelinePoll even if the poll is loading #7518

Merged
merged 3 commits into from
Apr 27, 2023
Merged

Fix: allow to render a TimelinePoll even if the poll is loading #7518

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
27fd56d
Fix: allow to render a TimelinePoll even if the poll is loading
nimau Apr 25, 2023
6776a8f
Fix: TimelinePoll code refactoring
nimau Apr 26, 2023
c9e9bd0
Update RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHis…
nimau Apr 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Riot/Assets/en.lproj/Vector.strings
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2405,6 +2405,8 @@ Tap the + to start adding people.";

"poll_timeline_reply_ended_poll" = "Ended poll";

"poll_timeline_loading" = "Loading...";

// MARK: - Location sharing

"location_sharing_title" = "Location";
Expand Down
4 changes: 4 additions & 0 deletions Riot/Generated/Strings.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4923,6 +4923,10 @@ public class VectorL10n: NSObject {
public static var pollTimelineEndedText: String {
return VectorL10n.tr("Vector", "poll_timeline_ended_text")
}
/// Loading...
public static var pollTimelineLoading: String {
return VectorL10n.tr("Vector", "poll_timeline_loading")
}
/// Please try again
public static var pollTimelineNotClosedSubtitle: String {
return VectorL10n.tr("Vector", "poll_timeline_not_closed_subtitle")
Expand Down
2 changes: 1 addition & 1 deletion ...SwiftUI/Modules/Room/PollHistory/PollHistoryDetail/MockPollHistoryDetailScreenState.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ enum MockPollHistoryDetailScreenState: MockScreenState, CaseIterable {
}

var screenView: ([Any], AnyView) {
let timelineViewModel = TimelinePollViewModel(timelinePollDetails: poll)
let timelineViewModel = TimelinePollViewModel(timelinePollDetailsState: .loaded(poll))
let viewModel = PollHistoryDetailViewModel(poll: poll)

return ([viewModel], AnyView(PollHistoryDetail(viewModel: viewModel.context, contentPoll: TimelinePollView(viewModel: timelineViewModel.context))))
Expand Down
9 changes: 5 additions & 4 deletions RiotSwiftUI/Modules/Room/PollHistory/Service/MatrixSDK/PollHistoryService.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,13 +209,14 @@ extension PollHistoryService: PollAggregatorDelegate {
func pollAggregator(_ aggregator: PollAggregator, didFailWithError: Error) { }

func pollAggregatorDidEndLoading(_ aggregator: PollAggregator) {
guard let context = pollAggregationContexts[aggregator.poll.id], context.published == false else {

nimau marked this conversation as resolved.
Show resolved Hide resolved
guard let poll = aggregator.poll, let context = pollAggregationContexts[poll.id], context.published == false else {
return
}

context.published = true

let newPoll: TimelinePollDetails = .init(poll: aggregator.poll, represent: .started)
let newPoll: TimelinePollDetails = .init(poll: poll, represent: .started)

if context.isLivePoll {
livePollsSubject.send(newPoll)
Expand All @@ -225,9 +226,9 @@ extension PollHistoryService: PollAggregatorDelegate {
}

func pollAggregatorDidUpdateData(_ aggregator: PollAggregator) {
guard let context = pollAggregationContexts[aggregator.poll.id], context.published else {
guard let poll = aggregator.poll, let context = pollAggregationContexts[poll.id], context.published else {
return
}
updatesSubject.send(.init(poll: aggregator.poll, represent: .started))
updatesSubject.send(.init(poll: poll, represent: .started))
}
}
26 changes: 17 additions & 9 deletions RiotSwiftUI/Modules/Room/TimelinePoll/Coordinator/TimelinePollCoordinator.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ final class TimelinePollCoordinator: Coordinator, Presentable, PollAggregatorDel
private let parameters: TimelinePollCoordinatorParameters
private let selectedAnswerIdentifiersSubject = PassthroughSubject<[String], Never>()

private var pollAggregator: PollAggregator
private var pollAggregator: PollAggregator!
private(set) var viewModel: TimelinePollViewModelProtocol!
private var cancellables = Set<AnyCancellable>()

Expand All @@ -46,10 +46,9 @@ final class TimelinePollCoordinator: Coordinator, Presentable, PollAggregatorDel
init(parameters: TimelinePollCoordinatorParameters) throws {
self.parameters = parameters

try pollAggregator = PollAggregator(session: parameters.session, room: parameters.room, pollEvent: parameters.pollEvent)
pollAggregator.delegate = self
viewModel = TimelinePollViewModel(timelinePollDetailsState: .loading)
try pollAggregator = PollAggregator(session: parameters.session, room: parameters.room, pollEvent: parameters.pollEvent, delegate: self)

viewModel = TimelinePollViewModel(timelinePollDetails: buildTimelinePollFrom(pollAggregator.poll))
viewModel.completion = { [weak self] result in
guard let self = self else { return }

Expand Down Expand Up @@ -92,11 +91,11 @@ final class TimelinePollCoordinator: Coordinator, Presentable, PollAggregatorDel
}

func canEndPoll() -> Bool {
pollAggregator.poll.isClosed == false
pollAggregator.poll?.isClosed == false
}

func canEditPoll() -> Bool {
pollAggregator.poll.isClosed == false && pollAggregator.poll.totalAnswerCount == 0
pollAggregator.poll?.isClosed == false && pollAggregator.poll?.totalAnswerCount == 0
}

func endPoll() {
Expand All @@ -108,14 +107,23 @@ final class TimelinePollCoordinator: Coordinator, Presentable, PollAggregatorDel
// MARK: - PollAggregatorDelegate

func pollAggregatorDidUpdateData(_ aggregator: PollAggregator) {
viewModel.updateWithPollDetails(buildTimelinePollFrom(aggregator.poll))
if let poll = aggregator.poll {
viewModel.updateWithPollDetailsState(.loaded(buildTimelinePollFrom(poll)))
}
}

func pollAggregatorDidStartLoading(_ aggregator: PollAggregator) { }

func pollAggregatorDidEndLoading(_ aggregator: PollAggregator) { }
func pollAggregatorDidEndLoading(_ aggregator: PollAggregator) {
guard let poll = aggregator.poll else {
return
}
viewModel.updateWithPollDetailsState(.loaded(buildTimelinePollFrom(poll)))
}

func pollAggregator(_ aggregator: PollAggregator, didFailWithError: Error) { }
func pollAggregator(_ aggregator: PollAggregator, didFailWithError: Error) {
viewModel.updateWithPollDetailsState(.errored)
}

// MARK: - Private

Expand Down
115 changes: 69 additions & 46 deletions RiotSwiftUI/Modules/Room/TimelinePoll/Test/Unit/TimelinePollViewModelTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,111 +41,134 @@ class TimelinePollViewModelTests: XCTestCase {
hasBeenEdited: false,
hasDecryptionError: false)

viewModel = TimelinePollViewModel(timelinePollDetails: timelinePoll)
viewModel = TimelinePollViewModel(timelinePollDetailsState: .loaded(timelinePoll))
context = viewModel.context
}

func testInitialState() {
XCTAssertEqual(context.viewState.poll.answerOptions.count, 3)
XCTAssertFalse(context.viewState.poll.closed)
XCTAssertEqual(context.viewState.poll.type, .disclosed)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions.count, 3)
XCTAssertEqual(context.viewState.pollState.poll?.closed, false)
XCTAssertEqual(context.viewState.pollState.poll?.type, .disclosed)
}

func testSingleSelectionOnMax1Allowed() {
context.send(viewAction: .selectAnswerOptionWithIdentifier("1"))

XCTAssertTrue(context.viewState.poll.answerOptions[0].selected)
XCTAssertFalse(context.viewState.poll.answerOptions[1].selected)
XCTAssertFalse(context.viewState.poll.answerOptions[2].selected)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions[0].selected, true)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions[1].selected, false)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions[2].selected, false)
}

func testSingleReselectionOnMax1Allowed() {
context.send(viewAction: .selectAnswerOptionWithIdentifier("1"))
context.send(viewAction: .selectAnswerOptionWithIdentifier("1"))

XCTAssertTrue(context.viewState.poll.answerOptions[0].selected)
XCTAssertFalse(context.viewState.poll.answerOptions[1].selected)
XCTAssertFalse(context.viewState.poll.answerOptions[2].selected)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions[0].selected, true)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions[1].selected, false)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions[2].selected, false)
}

func testMultipleSelectionOnMax1Allowed() {
context.send(viewAction: .selectAnswerOptionWithIdentifier("1"))
context.send(viewAction: .selectAnswerOptionWithIdentifier("3"))

XCTAssertFalse(context.viewState.poll.answerOptions[0].selected)
XCTAssertFalse(context.viewState.poll.answerOptions[1].selected)
XCTAssertTrue(context.viewState.poll.answerOptions[2].selected)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions[0].selected, false)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions[1].selected, false)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions[2].selected, true)
}

func testMultipleReselectionOnMax1Allowed() {
context.send(viewAction: .selectAnswerOptionWithIdentifier("1"))
context.send(viewAction: .selectAnswerOptionWithIdentifier("3"))
context.send(viewAction: .selectAnswerOptionWithIdentifier("3"))

XCTAssertFalse(context.viewState.poll.answerOptions[0].selected)
XCTAssertFalse(context.viewState.poll.answerOptions[1].selected)
XCTAssertTrue(context.viewState.poll.answerOptions[2].selected)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions[0].selected, false)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions[1].selected, false)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions[2].selected, true)
}

func testClosedSelection() {
viewModel.state.poll.closed = true
guard case var .loaded(poll) = context.viewState.pollState else {
return XCTFail()
}
poll.closed = true
viewModel.updateWithPollDetailsState(.loaded(poll))

context.send(viewAction: .selectAnswerOptionWithIdentifier("1"))
context.send(viewAction: .selectAnswerOptionWithIdentifier("3"))

XCTAssertFalse(context.viewState.poll.answerOptions[0].selected)
XCTAssertFalse(context.viewState.poll.answerOptions[1].selected)
XCTAssertFalse(context.viewState.poll.answerOptions[2].selected)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions[0].selected, false)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions[1].selected, false)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions[2].selected, false)
}

func testSingleSelectionOnMax2Allowed() {
viewModel.state.poll.maxAllowedSelections = 2
guard case var .loaded(poll) = context.viewState.pollState else {
return XCTFail()
}
poll.maxAllowedSelections = 2
viewModel.updateWithPollDetailsState(.loaded(poll))

context.send(viewAction: .selectAnswerOptionWithIdentifier("1"))

XCTAssertTrue(context.viewState.poll.answerOptions[0].selected)
XCTAssertFalse(context.viewState.poll.answerOptions[1].selected)
XCTAssertFalse(context.viewState.poll.answerOptions[2].selected)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions[0].selected, true)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions[1].selected, false)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions[2].selected, false)
}

func testSingleReselectionOnMax2Allowed() {
viewModel.state.poll.maxAllowedSelections = 2
guard case var .loaded(poll) = context.viewState.pollState else {
return XCTFail()
}
poll.maxAllowedSelections = 2
viewModel.updateWithPollDetailsState(.loaded(poll))

context.send(viewAction: .selectAnswerOptionWithIdentifier("1"))
context.send(viewAction: .selectAnswerOptionWithIdentifier("1"))

XCTAssertFalse(context.viewState.poll.answerOptions[0].selected)
XCTAssertFalse(context.viewState.poll.answerOptions[1].selected)
XCTAssertFalse(context.viewState.poll.answerOptions[2].selected)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions[0].selected, false)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions[1].selected, false)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions[2].selected, false)
}

func testMultipleSelectionOnMax2Allowed() {
viewModel.state.poll.maxAllowedSelections = 2

guard case var .loaded(poll) = context.viewState.pollState else {
return XCTFail()
}
poll.maxAllowedSelections = 2
viewModel.updateWithPollDetailsState(.loaded(poll))

context.send(viewAction: .selectAnswerOptionWithIdentifier("1"))
context.send(viewAction: .selectAnswerOptionWithIdentifier("3"))
context.send(viewAction: .selectAnswerOptionWithIdentifier("2"))

XCTAssertTrue(context.viewState.poll.answerOptions[0].selected)
XCTAssertFalse(context.viewState.poll.answerOptions[1].selected)
XCTAssertTrue(context.viewState.poll.answerOptions[2].selected)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions[0].selected, true)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions[1].selected, false)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions[2].selected, true)

context.send(viewAction: .selectAnswerOptionWithIdentifier("1"))

XCTAssertFalse(context.viewState.poll.answerOptions[0].selected)
XCTAssertFalse(context.viewState.poll.answerOptions[1].selected)
XCTAssertTrue(context.viewState.poll.answerOptions[2].selected)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions[0].selected, false)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions[1].selected, false)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions[2].selected, true)

context.send(viewAction: .selectAnswerOptionWithIdentifier("2"))

XCTAssertFalse(context.viewState.poll.answerOptions[0].selected)
XCTAssertTrue(context.viewState.poll.answerOptions[1].selected)
XCTAssertTrue(context.viewState.poll.answerOptions[2].selected)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions[0].selected, false)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions[1].selected, true)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions[2].selected, true)

context.send(viewAction: .selectAnswerOptionWithIdentifier("3"))

XCTAssertFalse(context.viewState.poll.answerOptions[0].selected)
XCTAssertTrue(context.viewState.poll.answerOptions[1].selected)
XCTAssertFalse(context.viewState.poll.answerOptions[2].selected)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions[0].selected, false)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions[1].selected, true)
XCTAssertEqual(context.viewState.pollState.poll?.answerOptions[2].selected, false)
}
}

private extension TimelinePollDetailsState {
var poll: TimelinePollDetails? {
switch self {
case .loaded(let poll):
return poll
default:
return nil
}
}
}
8 changes: 7 additions & 1 deletion RiotSwiftUI/Modules/Room/TimelinePoll/TimelinePollModels.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ enum TimelinePollEventType {
case ended
}

enum TimelinePollDetailsState {
case loading
case loaded(TimelinePollDetails)
case errored
}

struct TimelinePollAnswerOption: Identifiable {
var id: String
var text: String
Expand Down Expand Up @@ -94,7 +100,7 @@ struct TimelinePollDetails {
extension TimelinePollDetails: Identifiable { }

struct TimelinePollViewState: BindableState {
var poll: TimelinePollDetails
var pollState: TimelinePollDetailsState
var bindings: TimelinePollViewStateBindings
}

Expand Down
18 changes: 17 additions & 1 deletion RiotSwiftUI/Modules/Room/TimelinePoll/TimelinePollScreenState.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ enum MockTimelinePollScreenState: MockScreenState, CaseIterable {
case openUndisclosed
case closedUndisclosed
case closedPollEnded
case loading
case invalidStartEvent
case withAlert

var screenType: Any.Type {
TimelinePollDetails.self
Expand All @@ -45,7 +48,20 @@ enum MockTimelinePollScreenState: MockScreenState, CaseIterable {
hasBeenEdited: false,
hasDecryptionError: false)

let viewModel = TimelinePollViewModel(timelinePollDetails: poll)
let viewModel: TimelinePollViewModel

switch self {
case .loading:
viewModel = TimelinePollViewModel(timelinePollDetailsState: .loading)
case .invalidStartEvent:
viewModel = TimelinePollViewModel(timelinePollDetailsState: .errored)
default:
viewModel = TimelinePollViewModel(timelinePollDetailsState: .loaded(poll))
}

if self == .withAlert {
viewModel.showAnsweringFailure()
}

return ([viewModel], AnyView(TimelinePollView(viewModel: viewModel.context)))
}
Expand Down
Loading