Louvorg/ReaderForSelfoss-multiplatform
Louvorg/ReaderForSelfoss-multiplatform
3
2
Fork 2
Code Issues 16 Pull Requests Projects 1 Releases 65 Wiki Activity

WIP: HomeActivity cleanup #40

Closed
davidoskky wants to merge 3 commits from davidoskky/ReaderForSelfoss-multiplatform:dispatchers into master
pull from: davidoskky/ReaderForSelfoss-multiplatform:dispatchers
merge into: Louvorg:master
Conversation 6 Commits 3 Files Changed 4 +217 -206
4 changed files with 217 additions and 206 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

288
androidApp/src/main/java/bou/amine/apps/readerforselfossv2/android/HomeActivity.kt
View File

@ -17,6 +17,7 @@ import androidx.appcompat.app.AppCompatActivity
import androidx.appcompat.widget.SearchView
import androidx.core.view.doOnNextLayout
import androidx.drawerlayout.widget.DrawerLayout
import androidx.lifecycle.lifecycleScope
import androidx.recyclerview.widget.*
import androidx.work.Constraints
import androidx.work.ExistingPeriodicWorkPolicy
@ -34,10 +35,13 @@ import bou.amine.apps.readerforselfossv2.android.utils.Config
import bou.amine.apps.readerforselfossv2.android.utils.bottombar.maybeShow
import bou.amine.apps.readerforselfossv2.android.utils.bottombar.removeBadge
import bou.amine.apps.readerforselfossv2.android.utils.customtabs.CustomTabActivityHelper
import bou.amine.apps.readerforselfossv2.dao.ACTION
import bou.amine.apps.readerforselfossv2.repository.Repository
import bou.amine.apps.readerforselfossv2.android.viewmodel.AppViewModel
import bou.amine.apps.readerforselfossv2.model.SelfossModel
import bou.amine.apps.readerforselfossv2.utils.*
import bou.amine.apps.readerforselfossv2.repository.Repository
import bou.amine.apps.readerforselfossv2.utils.ItemType
import bou.amine.apps.readerforselfossv2.utils.getHtmlDecoded
import bou.amine.apps.readerforselfossv2.utils.getIcon
import bou.amine.apps.readerforselfossv2.utils.longHash
import com.ashokvarma.bottomnavigation.BottomNavigationBar
import com.ashokvarma.bottomnavigation.BottomNavigationItem
import com.ashokvarma.bottomnavigation.TextBadgeItem
@ -66,7 +70,6 @@ import org.kodein.di.DIAware
import org.kodein.di.android.closestDI
import org.kodein.di.instance
import java.util.concurrent.TimeUnit
import kotlin.concurrent.thread
class HomeActivity : AppCompatActivity(), SearchView.OnQueryTextListener, DIAware {
@ -118,6 +121,7 @@ class HomeActivity : AppCompatActivity(), SearchView.OnQueryTextListener, DIAwar
override val di by closestDI()
private val repository : Repository by instance()
private val viewModel: AppViewModel by instance()
data class DrawerData(val tags: List<SelfossModel.Tag>?, val sources: List<SelfossModel.Source>?)
@ -142,6 +146,11 @@ class HomeActivity : AppCompatActivity(), SearchView.OnQueryTextListener, DIAwar
}
setContentView(view)
lifecycleScope.launch {
viewModel.refreshingIndicatorProvider.collect { showRefresh ->
binding.swipeRefreshLayout.isRefreshing = showRefresh
AmineB commented 2022-08-25 12:02:28 +00:00
Review
Copy Link

The view/activity/fragment should be the one that knows when to display the refresh loading or not.

The view/activity/fragment should be the one that knows when to display the refresh loading or not.
davidoskky commented 2022-08-25 15:56:20 +00:00
Review
Copy Link

Alright, I'll find a different way to handle this then.

Alright, I'll find a different way to handle this then.
}
}
handleThemeBinding()
@ -153,18 +162,25 @@ class HomeActivity : AppCompatActivity(), SearchView.OnQueryTextListener, DIAwar
mDrawerToggle.syncState()
customTabActivityHelper = CustomTabActivityHelper()
handleSettings()
lifecycleScope.launch {
AmineB commented 2022-08-25 12:03:35 +00:00
Review
Copy Link

The view should call the repository and get the data from there.

This seams too complicated.

The view should call the repository and get the data from there. This seams too complicated.
davidoskky commented 2022-08-25 15:57:55 +00:00
Review
Copy Link

Storing the items would allow sharing the same data across all activities without passing it around and it can be used to prevent making an api call every time you close an article and come back to the article list; which is in fact unnecessary.

Storing the items would allow sharing the same data across all activities without passing it around and it can be used to prevent making an api call every time you close an article and come back to the article list; which is in fact unnecessary.
AmineB commented 2022-08-25 16:30:17 +00:00
Review
Copy Link

it can be used to prevent making an api call every time you close an article and come back to the article list

this is intended to fetch the new articles as soon as possible.

> it can be used to prevent making an api call every time you close an article and come back to the article list this is intended to fetch the new articles as soon as possible.
davidoskky commented 2022-08-25 18:35:33 +00:00
Review
Copy Link

Personal use case: selfoss fetches new articles every 15 minutes, in the application I will be reading an article in a couple of minutes and then going back to the articles list to select another one.
Every time I go back to the articles list the fetching starts, which is annoying because of the loading spinner appearing at the top each time.
I have no need for it to be updating so often, and most of those updates are useless anyways because most of the times no new items will be available on the server.
I can manually update the articles list at any time if I wish or if I finished reading all the available ones.

I remember someone else opened an issue some time ago complaining about the frequency of updates as well.

Personal use case: selfoss fetches new articles every 15 minutes, in the application I will be reading an article in a couple of minutes and then going back to the articles list to select another one. Every time I go back to the articles list the fetching starts, which is annoying because of the loading spinner appearing at the top each time. I have no need for it to be updating so often, and most of those updates are useless anyways because most of the times no new items will be available on the server. I can manually update the articles list at any time if I wish or if I finished reading all the available ones. I remember someone else opened an issue some time ago complaining about the frequency of updates as well.
AmineB commented 2022-08-25 18:43:11 +00:00
Review
Copy Link

Every time I go back to the articles list the fetching starts, which is annoying because of the loading spinner appearing at the top each time.

Why don't you swipe between the articles ?

I remember someone else opened an issue some time ago complaining about the frequency of updates as well.

That user in particular had issues with the tags and sources loading. They had a lot of them. That's why the update_sources setting was added.

> Every time I go back to the articles list the fetching starts, which is annoying because of the loading spinner appearing at the top each time. Why don't you swipe between the articles ? > I remember someone else opened an issue some time ago complaining about the frequency of updates as well. That user in particular had issues with the tags and sources loading. They had a lot of them. That's why the update_sources setting was added.
davidoskky commented 2022-08-25 19:17:02 +00:00
Review
Copy Link

Why don't you swipe between the articles ?

I have many articles to read and I do not read them in order, it is easier for me to get back to the list and select from there what I want to read rather than swiping through until I find something interesting.

> Why don't you swipe between the articles ? I have many articles to read and I do not read them in order, it is easier for me to get back to the list and select from there what I want to read rather than swiping through until I find something interesting.
AmineB commented 2022-08-26 07:42:34 +00:00
Review
Copy Link

That makes sens, but I still thinks the app needs to refresh onReload.

We could try to find a way to differenciate between "going back from another activity of the app" and "relaunching the app after it being in the background". This would solve both our use cases.

That makes sens, but I still thinks the app needs to refresh `onReload`. We could try to find a way to differenciate between "going back from another activity of the app" and "relaunching the app after it being in the background". This would solve both our use cases.
davidoskky commented 2022-08-26 11:23:50 +00:00
Review
Copy Link

Yes, I feel this could be a great way to solve this issue.

Yes, I feel this could be a great way to solve this issue.
viewModel.items.collect { fetchedItems ->
items = fetchedItems
handleListResult()
}
}
handleBottomBar()
handleDrawer()
handleSwipeRefreshLayout()
handleSettings()
getElementsAccordingToTab()
handleBadgesContent()
CoroutineScope(Dispatchers.Main).launch {
CoroutineScope(Dispatchers.IO).launch {
repository.tryToCacheItemsAndGetNewOnes()
}
@ -180,10 +196,7 @@ class HomeActivity : AppCompatActivity(), SearchView.OnQueryTextListener, DIAwar
repository.offlineOverride = false
lastFetchDone = false
handleDrawerItems()
CoroutineScope(Dispatchers.Main).launch {
getElementsAccordingToTab()
binding.swipeRefreshLayout.isRefreshing = false
}
viewModel.getItems(false, elementsShown)
}
val simpleItemTouchCallback =
@ -219,8 +232,6 @@ class HomeActivity : AppCompatActivity(), SearchView.OnQueryTextListener, DIAwar
adapter.handleItemAtIndex(position)
reloadBadgeContent()
val tagHashes = i.tags.map { it.longHash() }
tagsBadge = tagsBadge.map {
if (tagHashes.contains(it.key)) {
@ -252,16 +263,23 @@ class HomeActivity : AppCompatActivity(), SearchView.OnQueryTextListener, DIAwar
tabNewBadge = TextBadgeItem()
.setText("")
.setHideOnSelect(false).hide(false)
.setHideOnSelect(false)
.setBackgroundColor(appColors.colorPrimary)
if (!displayUnreadCount) {
tabNewBadge.hide(false)
}
tabArchiveBadge = TextBadgeItem()
.setText("")
.setHideOnSelect(false).hide(false)
.setHideOnSelect(false)
.setBackgroundColor(appColors.colorPrimary)
tabStarredBadge = TextBadgeItem()
.setText("")
.setHideOnSelect(false).hide(false)
.setHideOnSelect(false)
.setBackgroundColor(appColors.colorPrimary)
if (!displayAllCount) {
tabArchiveBadge.hide(false)
tabStarredBadge.hide(false)
}
val tabNew =
BottomNavigationItem(
@ -318,7 +336,7 @@ class HomeActivity : AppCompatActivity(), SearchView.OnQueryTextListener, DIAwar
handleRecurringTask()
CoroutineScope(Dispatchers.Main).launch {
CoroutineScope(Dispatchers.IO).launch {
repository.handleDBActions()
}
@ -453,7 +471,7 @@ class HomeActivity : AppCompatActivity(), SearchView.OnQueryTextListener, DIAwar
// TODO: refactor this.
private fun handleDrawerItems() {
tagsBadge = emptyMap()
fun handleDrawerData(maybeDrawerData: DrawerData?, loadedFromCache: Boolean = false) {
AmineB commented 2022-08-25 12:05:09 +00:00
Review
Copy Link

Why did you remove this ?

I think that this will break some features.

Why did you remove this ? I think that this will break some features.
AmineB commented 2022-08-25 12:05:38 +00:00
Review
Copy Link

I already started refactoring this, so I'll handle this.

I already started refactoring this, so I'll handle this.
fun handleDrawerData(maybeDrawerData: DrawerData?) {
fun createDrawerItem(
it: SelfossModel.Tag
) {
@ -495,12 +513,10 @@ class HomeActivity : AppCompatActivity(), SearchView.OnQueryTextListener, DIAwar
fun handleTags(maybeTags: List<SelfossModel.Tag>?) {
if (maybeTags == null) {
if (loadedFromCache) {
binding.mainDrawer.itemAdapter.add(
SecondaryDrawerItem()
.apply { nameRes = R.string.drawer_error_loading_tags; isSelectable = false }
)
}
binding.mainDrawer.itemAdapter.add(
SecondaryDrawerItem()
.apply { nameRes = R.string.drawer_error_loading_tags; isSelectable = false }
)
} else {
val filteredTags = maybeTags
.filterNot { hiddenTags.contains(it.tag) }
@ -515,14 +531,12 @@ class HomeActivity : AppCompatActivity(), SearchView.OnQueryTextListener, DIAwar
fun handleHiddenTags(maybeTags: List<SelfossModel.Tag>?) {
if (maybeTags == null) {
if (loadedFromCache) {
binding.mainDrawer.itemAdapter.add(
SecondaryDrawerItem().apply {
nameRes = R.string.drawer_error_loading_tags
isSelectable = false
}
)
}
binding.mainDrawer.itemAdapter.add(
SecondaryDrawerItem().apply {
nameRes = R.string.drawer_error_loading_tags
isSelectable = false
}
)
} else {
val filteredHiddenTags: List<SelfossModel.Tag> =
maybeTags.filter { hiddenTags.contains(it.tag) }
@ -536,14 +550,12 @@ class HomeActivity : AppCompatActivity(), SearchView.OnQueryTextListener, DIAwar
fun handleSources(maybeSources: List<SelfossModel.Source>?) {
if (maybeSources == null) {
if (loadedFromCache) {
binding.mainDrawer.itemAdapter.add(
SecondaryDrawerItem().apply {
nameRes = R.string.drawer_error_loading_sources
isSelectable = false
}
)
}
binding.mainDrawer.itemAdapter.add(
SecondaryDrawerItem().apply {
nameRes = R.string.drawer_error_loading_sources
isSelectable = false
}
)
} else {
for (source in maybeSources) {
val item = PrimaryDrawerItem().apply {
@ -632,61 +644,19 @@ class HomeActivity : AppCompatActivity(), SearchView.OnQueryTextListener, DIAwar
}
)
if (!loadedFromCache) {
if (maybeDrawerData.tags != null) {
thread {
repository.resetDBTagsWithData(maybeDrawerData.tags)
}
}
if (maybeDrawerData.sources != null) {
thread {
repository.resetDBSourcesWithData(maybeDrawerData.sources)
}
}
}
} else {
if (!loadedFromCache) {
binding.mainDrawer.itemAdapter.add(
PrimaryDrawerItem().apply {
nameRes = R.string.no_tags_loaded
identifier = DRAWER_ID_TAGS
isSelectable = false
},
PrimaryDrawerItem().apply {
nameRes = R.string.no_sources_loaded
identifier = DRAWER_ID_SOURCES
isSelectable = false
}
)
}
}
}
fun drawerApiCalls(maybeDrawerData: DrawerData?) {
var tags: List<SelfossModel.Tag>? = null
var sources: List<SelfossModel.Source>?
fun sourcesApiCall() {
CoroutineScope(Dispatchers.Main).launch {
val response = repository.getSources()
if (response != null) {
sources = response
val apiDrawerData = DrawerData(tags, sources)
if ((maybeDrawerData != null && maybeDrawerData != apiDrawerData) || maybeDrawerData == null) {
handleDrawerData(apiDrawerData)
}
} else {
val apiDrawerData = DrawerData(tags, null)
if ((maybeDrawerData != null && maybeDrawerData != apiDrawerData) || maybeDrawerData == null) {
handleDrawerData(apiDrawerData)
}
binding.mainDrawer.itemAdapter.add(
PrimaryDrawerItem().apply {
nameRes = R.string.no_tags_loaded
identifier = DRAWER_ID_TAGS
isSelectable = false
},
PrimaryDrawerItem().apply {
nameRes = R.string.no_sources_loaded
identifier = DRAWER_ID_SOURCES
isSelectable = false
}
}
}
CoroutineScope(Dispatchers.IO).launch {
tags = repository.getTags()
sourcesApiCall()
)
}
}
@ -697,12 +667,11 @@ class HomeActivity : AppCompatActivity(), SearchView.OnQueryTextListener, DIAwar
}
)
thread {
val drawerData = DrawerData(repository.getDBTags().map { it.toView() },
repository.getDBSources().map { it.toView() })
CoroutineScope(Dispatchers.IO).launch {
val drawerData = DrawerData(repository.getTags(),
repository.getSources())
runOnUiThread {
handleDrawerData(drawerData, loadedFromCache = true)
drawerApiCalls(drawerData)
handleDrawerData(drawerData)
}
}
}
@ -839,21 +808,7 @@ class HomeActivity : AppCompatActivity(), SearchView.OnQueryTextListener, DIAwar
}
firstVisible = if (appendResults) firstVisible else 0
getItems(appendResults, elementsShown)
}
private fun getItems(appendResults: Boolean, itemType: ItemType) {
CoroutineScope(Dispatchers.Main).launch {
binding.swipeRefreshLayout.isRefreshing = true
repository.displayedItems = itemType
items = if (appendResults) {
repository.getOlderItems()
} else {
repository.getNewerItems()
}
binding.swipeRefreshLayout.isRefreshing = false
handleListResult()
}
viewModel.getItems(appendResults, elementsShown)
}
private fun handleListResult(appendResults: Boolean = false) {
@ -915,26 +870,50 @@ class HomeActivity : AppCompatActivity(), SearchView.OnQueryTextListener, DIAwar
private fun reloadBadges() {
if (displayUnreadCount || displayAllCount) {
CoroutineScope(Dispatchers.Main).launch {
CoroutineScope(Dispatchers.IO).launch {
repository.reloadBadges()
reloadBadgeContent()
}
}
}
private fun reloadBadgeContent() {
private fun handleBadgesContent() {
AmineB commented 2022-08-25 12:00:26 +00:00
Review
Copy Link

This way better !

This way better !
if (displayUnreadCount) {
tabNewBadge
.setText(repository.badgeUnread.toString())
.maybeShow()
lifecycleScope.launch {
repository.badgeUnread.collect { unreadCount ->
if (unreadCount > 0) {
tabNewBadge
.setText(unreadCount.toString())
.maybeShow()
} else {
tabNewBadge.removeBadge()
}
}
}
}
if (displayAllCount) {
tabArchiveBadge
.setText(repository.badgeAll.toString())
.maybeShow()
tabStarredBadge
.setText(repository.badgeStarred.toString())
.maybeShow()
lifecycleScope.launch {
repository.badgeAll.collect { itemsCount ->
if (itemsCount > 0) {
tabArchiveBadge
.setText(itemsCount.toString())
.maybeShow()
} else {
tabArchiveBadge.removeBadge()
}
}
}
lifecycleScope.launch {
repository.badgeStarred.collect { starredCount ->
if (starredCount > 0) {
tabStarredBadge
.setText(starredCount.toString())
.maybeShow()
} else {
tabStarredBadge.removeBadge()
}
}
}
}
}
@ -992,56 +971,14 @@ class HomeActivity : AppCompatActivity(), SearchView.OnQueryTextListener, DIAwar
when (item.itemId) {
R.id.refresh -> {
needsConfirmation(R.string.menu_home_refresh, R.string.refresh_dialog_message) {
Toast.makeText(this, R.string.refresh_in_progress, Toast.LENGTH_SHORT).show()
// TODO: Use Dispatchers.IO
CoroutineScope(Dispatchers.Main).launch {
val updatedRemote = repository.updateRemote()
if (updatedRemote) {
// TODO: Send toast messages from the repository
Toast.makeText(
this@HomeActivity,
R.string.refresh_success_response, Toast.LENGTH_LONG
)
.show()
} else {
Toast.makeText(
this@HomeActivity,
R.string.refresh_failer_message,
Toast.LENGTH_SHORT
).show()
}
}
viewModel.updateRemote()
}
return true
}
R.id.readAll -> {
if (elementsShown == ItemType.UNREAD) {
needsConfirmation(R.string.readAll, R.string.markall_dialog_message) {
binding.swipeRefreshLayout.isRefreshing = true
CoroutineScope(Dispatchers.Main).launch {
val success = repository.markAllAsRead(items)
if (success) {
Toast.makeText(
this@HomeActivity,
R.string.all_posts_read,
Toast.LENGTH_SHORT
).show()
tabNewBadge.removeBadge()
handleDrawerItems()
getElementsAccordingToTab()
} else {
Toast.makeText(
this@HomeActivity,
R.string.all_posts_not_read,
Toast.LENGTH_SHORT
).show()
}
handleListResult()
binding.swipeRefreshLayout.isRefreshing = false
}
viewModel.markAllAsRead()
}
}
return true
@ -1055,10 +992,10 @@ class HomeActivity : AppCompatActivity(), SearchView.OnQueryTextListener, DIAwar
private fun maxItemNumber(): Int =
when (elementsShown) {
ItemType.UNREAD -> repository.badgeUnread
ItemType.ALL -> repository.badgeAll
ItemType.STARRED -> repository.badgeStarred
else -> repository.badgeUnread // if !elementsShown then unread are fetched.
ItemType.UNREAD -> repository.badgeUnread.value
ItemType.ALL -> repository.badgeAll.value
ItemType.STARRED -> repository.badgeStarred.value
else -> repository.badgeUnread.value // if !elementsShown then unread are fetched.
}
private fun updateItems(adapterItems: ArrayList<SelfossModel.Item>) {
@ -1082,9 +1019,4 @@ class HomeActivity : AppCompatActivity(), SearchView.OnQueryTextListener, DIAwar
WorkManager.getInstance(baseContext).enqueueUniquePeriodicWork("selfoss-loading", ExistingPeriodicWorkPolicy.KEEP, backgroundWork)
}
}
private fun handleOfflineActions() {
}
}

9
androidApp/src/main/java/bou/amine/apps/readerforselfossv2/android/MyApp.kt
View File

@ -84,6 +84,15 @@ class MyApp : MultiDexApplication(), DIAware {
).show()
}
}
CoroutineScope(Dispatchers.Main).launch {
AmineB commented 2022-08-25 11:59:52 +00:00
Review
Copy Link

I don't think that this should be here. Each activity/fragment should know what they want to display, and how.

I don't think that this should be here. Each activity/fragment should know what they want to display, and how.
viewModel.toastMessageProvider.collect { toastMessage ->
Toast.makeText(
applicationContext,
toastMessage,
Toast.LENGTH_SHORT
).show()
}
}
}

53
androidApp/src/main/java/bou/amine/apps/readerforselfossv2/android/viewmodel/AppViewModel.kt
View File

@ -3,16 +3,29 @@ package bou.amine.apps.readerforselfossv2.android.viewmodel
import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import bou.amine.apps.readerforselfossv2.android.R
import bou.amine.apps.readerforselfossv2.model.SelfossModel
import bou.amine.apps.readerforselfossv2.repository.Repository
import bou.amine.apps.readerforselfossv2.utils.ItemType
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.asSharedFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.launch
class AppViewModel(private val repository: Repository) : ViewModel() {
private val _networkAvailableProvider = MutableSharedFlow<Boolean>()
val networkAvailableProvider = _networkAvailableProvider.asSharedFlow()
private val _refreshingIndicatorProvider = MutableSharedFlow<Boolean>()
val refreshingIndicatorProvider = _refreshingIndicatorProvider.asSharedFlow()
private val _toastMessageProvider = MutableSharedFlow<Int>()
val toastMessageProvider = _toastMessageProvider.asSharedFlow()
private var wasConnected = true
private val _items = MutableStateFlow(ArrayList<SelfossModel.Item>())
val items = _items.asStateFlow()
init {
viewModelScope.launch {
repository.isConnectionAvailable.collect { isConnected ->
@ -28,4 +41,44 @@ class AppViewModel(private val repository: Repository) : ViewModel() {
}
}
}
fun updateRemote() {
AmineB commented 2022-08-25 11:55:57 +00:00
Review
Copy Link

Why were these moved here ?

Why were these moved here ?
davidoskky commented 2022-08-25 16:02:38 +00:00
Review
Copy Link

updateRemote doesn't necessarily have to be there, I have done it because I think it makes it easier to read through the activity code.
getItems in the viewmodel allows storing the items in the viewmodel which might provide some advantages.

updateRemote doesn't necessarily have to be there, I have done it because I think it makes it easier to read through the activity code. getItems in the viewmodel allows storing the items in the viewmodel which might provide some advantages.
AmineB commented 2022-08-25 18:18:22 +00:00
Review
Copy Link

updateRemote doesn't necessarily have to be there, I have done it because I think it makes it easier to read through the activity code.

I find it more difficult to read. Moving some code that's only available and usable in a specific activity in a shared ViewModel isn't necessary.

getItems in the viewmodel allows storing the items in the viewmodel which might provide some advantages.

There is no need to refactor thing this soon. If there is currently no need to have the items in the viewmodel, there is no need to have them here.

> updateRemote doesn't necessarily have to be there, I have done it because I think it makes it easier to read through the activity code. I find it more difficult to read. Moving some code that's only available and usable in a specific activity in a shared `ViewModel` isn't necessary. > getItems in the viewmodel allows storing the items in the viewmodel which *might* provide some advantages. There is no need to refactor thing this soon. If there is currently no need to have the items in the viewmodel, there is no need to have them here.
👍 1
CoroutineScope(Dispatchers.IO).launch {
_toastMessageProvider.emit(R.string.refresh_in_progress)
val updatedRemote = repository.updateRemote()
if (updatedRemote) {
_toastMessageProvider.emit(R.string.refresh_success_response)
} else {
_toastMessageProvider.emit(R.string.refresh_failer_message)
}
}
}
fun getItems(appendResults: Boolean, itemType: ItemType) {
// TODO: Find a way to use Dispatchers.IO without creating conflicts
CoroutineScope(Dispatchers.Main).launch {
_refreshingIndicatorProvider.emit(true)
repository.displayedItems = itemType
val items = if (appendResults) {
repository.getOlderItems()
} else {
repository.getNewerItems()
}
_items.emit(items)
_refreshingIndicatorProvider.emit(false)
}
}
fun markAllAsRead() {
CoroutineScope(Dispatchers.IO).launch {
_refreshingIndicatorProvider.emit(true)
val success = repository.markAllAsRead(items.value)
if (success) {
_toastMessageProvider.emit(R.string.all_posts_read)
} else {
_toastMessageProvider.emit(R.string.all_posts_not_read)
}
_refreshingIndicatorProvider.emit(false)
}
}
}

73
shared/src/commonMain/kotlin/bou/amine/apps/readerforselfossv2/repository/RepositoryImpl.kt
View File

@ -11,6 +11,8 @@ import com.russhwolf.settings.Settings
import io.github.aakira.napier.Napier
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.launch
class Repository(private val api: SelfossApi, private val apiDetails: ApiDetailsService, private val connectivityStatus: ConnectivityStatus, private val db: ReaderForSelfossDB) {
@ -33,18 +35,23 @@ class Repository(private val api: SelfossApi, private val apiDetails: ApiDetails
var offlineOverride = false
var apiMajorVersion = 0
var badgeUnread = 0
set(value) {field = if (value < 0) { 0 } else { value } }
var badgeAll = 0
set(value) {field = if (value < 0) { 0 } else { value } }
var badgeStarred = 0
set(value) {field = if (value < 0) { 0 } else { value } }
private val _badgeUnread = MutableStateFlow(0)
val badgeUnread = _badgeUnread.asStateFlow()
private val _badgeAll = MutableStateFlow(0)
val badgeAll = _badgeAll.asStateFlow()
private val _badgeStarred = MutableStateFlow(0)
val badgeStarred = _badgeStarred.asStateFlow()
init {
// TODO: Dispatchers.IO not available in KMM, an alternative solution should be found
CoroutineScope(Dispatchers.Main).launch {
updateApiVersion()
AmineB commented 2022-08-25 11:53:34 +00:00
Review
Copy Link

updateApiVersion() and reloadBadges() both check the network availability. This change isn't needed.

`updateApiVersion()` and `reloadBadges()` both check the network availability. This change isn't needed.
davidoskky commented 2022-08-25 16:00:24 +00:00
Review
Copy Link

When init is run the connectivity check always fails.
This allows retrieving the badges every time connectivity gets back.
We could include here all operations that should be run as soon as network connectivity is available, such as fetching sources and tags.

When init is run the connectivity check always fails. This allows retrieving the badges every time connectivity gets back. We could include here all operations that should be run as soon as network connectivity is available, such as fetching sources and tags.
AmineB commented 2022-08-25 18:21:40 +00:00
Review
Copy Link

When init is run the connectivity check always fails.

So this is a bug that should be fixed instead of having this workaround.

This allows retrieving the badges every time connectivity gets back.

This isn't a good thing, since items arn't refreshed on connectivity change. We would have a difference in the items count and the badge count.

We could include here all operations that should be run as soon as network connectivity is available, such as fetching sources and tags.

Fetching sources and tags can be limited via a update_sources. It's also something that shouldn't be done as much as items fetching.

> When init is run the connectivity check always fails. So this is a bug that should be fixed instead of having this workaround. > This allows retrieving the badges every time connectivity gets back. This isn't a good thing, since items arn't refreshed on connectivity change. We would have a difference in the items count and the badge count. > We could include here all operations that should be run as soon as network connectivity is available, *such as fetching sources and tags.* Fetching sources and tags can be limited via a `update_sources`. It's also something that shouldn't be done as much as items fetching.
reloadBadges()
isConnectionAvailable.collect { connectionAvailable ->
if (connectionAvailable) {
updateApiVersion()
reloadBadges()
}
}
}
}
@ -125,27 +132,31 @@ class Repository(private val api: SelfossApi, private val apiDetails: ApiDetails
if (isNetworkAvailable()) {
val response = api.stats()
if (response != null) {
badgeUnread = response.unread
badgeAll = response.total
badgeStarred = response.starred
_badgeUnread.value = response.unread
_badgeAll.value = response.total
_badgeStarred.value = response.starred
success = true
}
} else {
} else if (itemsCaching) {
// TODO: do this differently, because it's not efficient
val dbItems = getDBItems()
badgeUnread = dbItems.filter { item -> item.unread }.size
badgeStarred = dbItems.filter { item -> item.starred }.size
badgeAll = items.size
_badgeUnread.value = dbItems.filter { item -> item.unread }.size
_badgeStarred.value = dbItems.filter { item -> item.starred }.size
_badgeAll.value = dbItems.size
}
return success
}
suspend fun getTags(): List<SelfossModel.Tag>? {
return if (isNetworkAvailable()) {
val tags = if (isNetworkAvailable()) {
api.tags()
} else {
getDBTags().map { it.toView() }
}
if (tags != null) {
AmineB commented 2022-08-25 11:52:04 +00:00
Review
Copy Link

resetDBTagsWithData should only be done if network the api call was done, and if update_sources setting is set to false.

`resetDBTagsWithData` should only be done if network the api call was done, and if `update_sources` setting is set to false.
👍 1
resetDBTagsWithData(tags)
}
return tags
}
suspend fun getSpouts(): Map<String, SelfossModel.Spout>? {
@ -157,12 +168,15 @@ class Repository(private val api: SelfossApi, private val apiDetails: ApiDetails
}
suspend fun getSources(): ArrayList<SelfossModel.Source>? {
return if (isNetworkAvailable()) {
val sources = if (isNetworkAvailable()) {
api.sources()
} else {
ArrayList(getDBSources().map { it.toView() })
}
if (sources != null) {
AmineB commented 2022-08-25 11:51:49 +00:00
Review
Copy Link

resetDBSourcesWithData should only be done if network the api call was done, and if update_sources setting is set to false.

`resetDBSourcesWithData` should only be done if network the api call was done, and if `update_sources` setting is set to false.
resetDBSourcesWithData(sources)
}
return sources
}
suspend fun markAsRead(item: SelfossModel.Item): Boolean {
@ -253,7 +267,7 @@ class Repository(private val api: SelfossApi, private val apiDetails: ApiDetails
private fun markAsReadLocally(item: SelfossModel.Item) {
if (item.unread) {
item.unread = false
badgeUnread -= 1
_badgeUnread.value -= 1
}
CoroutineScope(Dispatchers.Main).launch {
@ -264,7 +278,7 @@ class Repository(private val api: SelfossApi, private val apiDetails: ApiDetails
private fun unmarkAsReadLocally(item: SelfossModel.Item) {
if (!item.unread) {
item.unread = true
badgeUnread += 1
_badgeUnread.value += 1
}
CoroutineScope(Dispatchers.Main).launch {
@ -275,7 +289,7 @@ class Repository(private val api: SelfossApi, private val apiDetails: ApiDetails
private fun starrLocally(item: SelfossModel.Item) {
if (!item.starred) {
item.starred = true
badgeStarred += 1
_badgeStarred.value += 1
}
CoroutineScope(Dispatchers.Main).launch {
@ -286,7 +300,7 @@ class Repository(private val api: SelfossApi, private val apiDetails: ApiDetails
private fun unstarrLocally(item: SelfossModel.Item) {
if (item.starred) {
item.starred = false
badgeStarred -= 1
_badgeStarred.value -= 1
}
CoroutineScope(Dispatchers.Main).launch {
@ -428,13 +442,16 @@ class Repository(private val api: SelfossApi, private val apiDetails: ApiDetails
suspend fun tryToCacheItemsAndGetNewOnes(): List<SelfossModel.Item>? {
try {
val newItems = getMaxItemsForBackground(ItemType.UNREAD)
val allItems = getMaxItemsForBackground(ItemType.ALL)
val starredItems = getMaxItemsForBackground(ItemType.STARRED)
insertDBItems(newItems.orEmpty() + allItems.orEmpty() + starredItems.orEmpty())
return newItems
} catch (e: Throwable) {}
if (itemsCaching) {
try {
val newItems = getMaxItemsForBackground(ItemType.UNREAD)
val allItems = getMaxItemsForBackground(ItemType.ALL)
val starredItems = getMaxItemsForBackground(ItemType.STARRED)
insertDBItems(newItems.orEmpty() + allItems.orEmpty() + starredItems.orEmpty())
return newItems
} catch (e: Throwable) {
}
}
return emptyList()
}