Fix loading when site model is incomplete#9353
Conversation
Generated by 🚫 dangerJS |
0nko
left a comment
There was a problem hiding this comment.
Looks great. I hope the SiteProvider will be used everywhere, eventually. I've left a couple of comments in the code. Let me know what you think.
...ress/src/main/java/org/wordpress/android/ui/stats/refresh/lists/sections/BaseStatsUseCase.kt
Show resolved
Hide resolved
...ress/src/main/java/org/wordpress/android/ui/stats/refresh/lists/sections/BaseStatsUseCase.kt
Outdated
Show resolved
Hide resolved
...ress/src/main/java/org/wordpress/android/ui/stats/refresh/lists/sections/BaseStatsUseCase.kt
Outdated
Show resolved
Hide resolved
|
|
||
| @Subscribe(threadMode = ThreadMode.MAIN) | ||
| fun onSiteChanged(event: OnSiteChanged) { | ||
| val site = siteStore.getSiteByLocalId(siteModel.id) |
There was a problem hiding this comment.
This looks a bit cumbersome. When updating the SiteProvider's site value, you must already know what it is :). Would it be possible to update the OnSiteChanged to include the new site value? That way, it wouldn't be necessary to call start(site) and the dispatcher registration could happen in the constructor.
There was a problem hiding this comment.
You're totally right and I think it should be done this way. I think it's out of scope for this PR. At least we're containing this logic in one place. I think it might make sense to rename this SiteModelProvider to something like StatsSiteProvider because it shouldn't be used anywhere else right now. I think adding a full reusable SiteModelProvider should be a separate task. Let me know what you think :-)
|
@0nko This should be ready for another try |
Fixes #9352
This PR introduces a
SiteModelProvider, which provides an up-to-date version of the SiteModel and triggers a data refresh, when the SiteModel is updated. I've replaced theSiteModelthat was passed around as a parameter all around the use cases with the provider. The only place where we're still retrieving theSiteModelis inside of theStatsFragmentwhere we initialize theSiteModelProviderin thestartmethod of theStatsViewModel.To test: