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

[nimau]

This PR fix #7497

This PR is related to this one (matrix-ios-sdk): Fix: Refreshing the poll when receiving pollEnd can break the chronological order in the store. #1776

It displays a temporary poll while fetching the pollStartEvent:

If we failed to get the start event (or to decrypt it), we'll fallback on the normal rendering:

Also, the PollAggregator delegate was not defined early enough in TimelinePollCoordinator (and so the delegate's methods were not called).
I had to make public the PollAggregator.reloadPollData() method because:

• once the PollAggregator is instantiated, it immediately loads the poll data.
  • and once the poll is loaded, the delegate is called (but not yet defined).
  • the delegate method func pollAggregatorDidUpdateData(_ aggregator: PollAggregator) needs the viewModel which is not ready at this stage because we need to instantiate it with the poll built by the PollAggregator

try pollAggregator = PollAggregator(session: parameters.session, room: parameters.room, pollEvent: parameters.pollEvent)
pollAggregator.delegate = self
        
viewModel = TimelinePollViewModel(timelinePollDetails: buildTimelinePollFrom(pollAggregator.poll))

To make it works, the PollAggregator no longer loads the poll data during its init function, and is only called when the viewModel is ready.

In this way, if the PollAggregator doesn't find the pollStartEvent in the store, it will generate a temporary poll which will be replaced by the correct one once the fetch is complete.
During this loading phase, the PollView will be in the .loading state. And once the data have been loaded, its state will be .loaded (or .invalidStartEvent)

[codecov]

Codecov Report

Patch coverage: 20.48% and project coverage change: +0.15 🎉

Comparison is base (5cfac70) 12.21% compared to head (c9e9bd0) 12.37%.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #7518      +/-   ##
===========================================
+ Coverage    12.21%   12.37%   +0.15%     
===========================================
  Files         1646     1646              
  Lines       163315   163364      +49     
  Branches     67037    67078      +41     
===========================================
+ Hits         19944    20209     +265     
+ Misses      142729   142494     -235     
- Partials       642      661      +19     

Flag	Coverage Δ
unittests	6.21% <0.00%> (+0.14%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...storyDetail/MockPollHistoryDetailScreenState.swift	96.29% <0.00%> (-3.71%)	⬇️
...History/Service/MatrixSDK/PollHistoryService.swift	0.00% <0.00%> (ø)
...linePoll/Coordinator/TimelinePollCoordinator.swift	0.00% <0.00%> (ø)
...Modules/Room/TimelinePoll/TimelinePollModels.swift	92.00% <ø> (ø)
...ules/Room/TimelinePoll/TimelinePollViewModel.swift	37.36% <0.00%> (-8.98%)	⬇️
...ules/Room/TimelinePoll/View/TimelinePollView.swift	59.25% <35.00%> (-9.24%)	⬇️
...es/Room/TimelinePoll/TimelinePollScreenState.swift	89.18% <71.42%> (-10.82%)	⬇️

... and 20 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[alfogrillo]

Great idea!
But I think we may be able to do better with additional simple changes.

Also, the PollAggregator delegate was not defined early enough in TimelinePollCoordinator (and so the delegate's methods were not called).

I think here the fix is passing the delegate in the PollAggregator init?

Also, the PollAggregator delegate was not defined early enough in TimelinePollCoordinator (and so the delegate's methods were not called).

To fix this we may try to change the dependency in the init of TimelinePollViewModel.
Right now we pass a TimelinePollDetails but since we don't have it one we may change the dependency with something like this.

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

If we do that, I think we can change the init in the TimelinePollCordinator like this:

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

Of course I also expect this type to be used somewhere in TimelinePollView.

Finally I think the Poll in the PollAggregator should now be optional to reflect this change:

public class PollAggregator {
    ....
    public private(set) var poll: PollProtocol?

What do you think?

[nimau]

Great idea! But I think we may be able to do better with additional simple changes.

Also, the PollAggregator delegate was not defined early enough in TimelinePollCoordinator (and so the delegate's methods were not called).

I think here the fix is passing the delegate in the PollAggregator init?

Also, the PollAggregator delegate was not defined early enough in TimelinePollCoordinator (and so the delegate's methods were not called).

To fix this we may try to change the dependency in the init of TimelinePollViewModel. Right now we pass a TimelinePollDetails but since we don't have it one we may change the dependency with something like this.

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

If we do that, I think we can change the init in the TimelinePollCordinator like this:

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

Of course I also expect this type to be used somewhere in TimelinePollView.

Finally I think the Poll in the PollAggregator should now be optional to reflect this change:

public class PollAggregator {
    ....
    public private(set) var poll: PollProtocol?

What do you think?

@alfogrillo I've updated the PR to reflect your comments. You're right, it is cleaner this way, let me know if it works for you. (the related matrix-ios-sdk PR has also been updated to make the poll optional on PollAggregator)

[alfogrillo]

LGTM!

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information