Simplify sources and tags handling #70
@ -15,9 +15,6 @@ import androidx.activity.result.contract.ActivityResultContracts
|
||||
import androidx.appcompat.app.ActionBarDrawerToggle
|
||||
import androidx.appcompat.app.AlertDialog
|
||||
import androidx.appcompat.app.AppCompatActivity
|
||||
import androidx.appcompat.app.AppCompatDelegate
|
||||
import androidx.appcompat.app.AppCompatDelegate.MODE_NIGHT_NO
|
||||
import androidx.appcompat.app.AppCompatDelegate.MODE_NIGHT_YES
|
||||
import androidx.appcompat.widget.SearchView
|
||||
import androidx.core.view.doOnNextLayout
|
||||
import androidx.drawerlayout.widget.DrawerLayout
|
||||
@ -98,8 +95,6 @@ class HomeActivity : AppCompatActivity(), SearchView.OnQueryTextListener, DIAwar
|
||||
private val repository : Repository by instance()
|
||||
private val appSettingsService : AppSettingsService by instance()
|
||||
|
||||
data class DrawerData(val tags: List<SelfossModel.Tag>?, val sources: List<SelfossModel.Source>?)
|
||||
|
||||
override fun onCreate(savedInstanceState: Bundle?) {
|
||||
super.onCreate(savedInstanceState)
|
||||
binding = ActivityHomeBinding.inflate(layoutInflater)
|
||||
@ -352,28 +347,15 @@ class HomeActivity : AppCompatActivity(), SearchView.OnQueryTextListener, DIAwar
|
||||
)
|
||||
|
||||
|
||||
CoroutineScope(Dispatchers.IO).launch {
|
||||
val drawerData = DrawerData(repository.getDBTags().map { it.toView() },
|
||||
AmineB marked this conversation as resolved
Outdated
AmineB
commented
Removing this forces the app to make 2 api calls (to the tags and sources routes) even if we have data in the database. Not sure if we want to do this. Removing this forces the app to make 2 api calls (to the tags and sources routes) even if we have data in the database.
Not sure if we want to do this.
davidoskky
commented
There is a change in behaviour effectively. There is a change in behaviour effectively.
If this is the behaviour we desire, it should be specified in the repository.
I'm not sure this is good, because doing this you will never update sources after you stored them once in the database.
AmineB
commented
The condition The condition `drawerData.sources?.isEmpty() == true || appSettingsService.isUpdateSourcesEnabled()` was false if there was already data in the DB and `isUpdateSourcesEnabled` was false, thus not making an api call as I stated in my first comment.
davidoskky
commented
Yes, I made a mistake. You were right in the first comment. However this behavior never update sources once they've been stored in the database. Yes, I made a mistake. You were right in the first comment. However this behavior never update sources once they've been stored in the database.
AmineB
commented
The pull to refresh was updating them, if I remember correctly The pull to refresh was updating them, if I remember correctly
davidoskky
commented
As far as I can see this is not the case; the pull to refresh just calls I'm not sure this is the preferred behavior, I guess an user expects the application to have sources and tags up to date once they start it up. Moreover it's a very small amount of data being transferred when compared to the data downloaded for actual articles. Also, I'm not sure about how this would work with the number of unread items in each tag; it would probably be out of date when you first start the application. As far as I can see this is not the case; the pull to refresh just calls `handleDrawerData()`.
What I could do, is change the repository implementation of `getTags()` and `getSources()` so that they default to fetching from the database rather than the api unless a parameter to force using the api is set; then I could include a call to this method with the force flag in the pull to refresh.
I'm not sure this is the preferred behavior, I guess an user expects the application to have sources and tags up to date once they start it up. Moreover it's a very small amount of data being transferred when compared to the data downloaded for actual articles.
Also, I'm not sure about how this would work with the number of unread items in each tag; it would probably be out of date when you first start the application.
AmineB
commented
For now, we'll let this like you implemented it. We'll see later how to handle things. For now, we'll let this like you implemented it. We'll see later how to handle things.
|
||||
repository.getDBSources().map { it.toView() })
|
||||
val tags = repository.getTags()
|
||||
val sources = repository.getSources()
|
||||
runOnUiThread {
|
||||
// TODO: All this logic should be handled by the repository, simplify and remove direct DB access
|
||||
// Only refresh if there is no data in the DB, or if the `UpdateSources` setting is enabled
|
||||
if (drawerData.sources?.isEmpty() == true || appSettingsService.isUpdateSourcesEnabled()) {
|
||||
drawerApiCalls(drawerData)
|
||||
} else {
|
||||
handleDrawerData(drawerData, loadedFromCache = true)
|
||||
}
|
||||
handleDrawerData(tags, sources)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private fun drawerApiCalls(drawerData: DrawerData) {
|
||||
CoroutineScope(Dispatchers.Main).launch {
|
||||
val apiDrawerData = DrawerData(repository.getTags(), repository.getSources())
|
||||
handleDrawerData(if (drawerData != apiDrawerData) apiDrawerData else drawerData)
|
||||
}
|
||||
}
|
||||
|
||||
private fun handleDrawerData(drawerData: DrawerData, loadedFromCache: Boolean = false) {
|
||||
private fun handleDrawerData(tags: List<SelfossModel.Tag>, sources: List<SelfossModel.Source>) {
|
||||
binding.mainDrawer.itemAdapter.clear()
|
||||
|
||||
// Filters title with clear action
|
||||
@ -387,24 +369,24 @@ class HomeActivity : AppCompatActivity(), SearchView.OnQueryTextListener, DIAwar
|
||||
}
|
||||
|
||||
// Hidden tags
|
||||
if (drawerData.tags != null && drawerData.tags.isNotEmpty() && appSettingsService.getHiddenTags().isNotEmpty()) {
|
||||
if (tags.isNotEmpty() && appSettingsService.getHiddenTags().isNotEmpty()) {
|
||||
secondaryItem(
|
||||
withDivider = true,
|
||||
R.string.drawer_item_hidden_tags,
|
||||
DRAWER_ID_HIDDEN_TAGS
|
||||
)
|
||||
handleHiddenTags(drawerData.tags)
|
||||
handleHiddenTags(tags)
|
||||
}
|
||||
|
||||
// Tags
|
||||
secondaryItem(withDivider = true, R.string.drawer_item_tags, DRAWER_ID_TAGS)
|
||||
if (drawerData.tags == null && !loadedFromCache) {
|
||||
if (tags.isEmpty()) {
|
||||
binding.mainDrawer.itemAdapter.add(
|
||||
SecondaryDrawerItem()
|
||||
.apply { nameRes = R.string.drawer_error_loading_tags; isSelectable = false }
|
||||
)
|
||||
} else {
|
||||
handleTags(drawerData.tags!!)
|
||||
handleTags(tags)
|
||||
}
|
||||
|
||||
// Sources
|
||||
@ -412,15 +394,15 @@ class HomeActivity : AppCompatActivity(), SearchView.OnQueryTextListener, DIAwar
|
||||
startActivity(Intent(v!!.context, SourcesActivity::class.java))
|
||||
false
|
||||
}
|
||||
if (drawerData.sources == null && !loadedFromCache) {
|
||||
if (sources.isEmpty()) {
|
||||
binding.mainDrawer.itemAdapter.add(
|
||||
SecondaryDrawerItem().apply {
|
||||
nameRes = R.string.drawer_error_loading_tags
|
||||
nameRes = R.string.drawer_error_loading_sources
|
||||
isSelectable = false
|
||||
}
|
||||
)
|
||||
} else {
|
||||
handleSources(drawerData.sources!!)
|
||||
handleSources(sources)
|
||||
}
|
||||
|
||||
// About action
|
||||
|
@ -75,7 +75,6 @@ class MyApp : MultiDexApplication(), DIAware {
|
||||
).show()
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
AmineB marked this conversation as resolved
Outdated
AmineB
commented
Why was this added ? Why was this added ?
davidoskky
commented
This starts tracking the connectivity status before the repository is initiated; it fixes a problem with the repository not correctly executing actions in the init method. This starts tracking the connectivity status before the repository is initiated; it fixes a problem with the repository not correctly executing actions in the init method.
|
||||
|
||||
private fun handleNotificationChannels() {
|
||||
|
@ -132,4 +132,5 @@
|
||||
<string name="mode_dark">Dark mode</string>
|
||||
<string name="mode_system">Follow the system setting</string>
|
||||
<string name="mode_light">Light mode</string>
|
||||
<string name="drawer_error_loading_sources">Error loading sources…</string>
|
||||
</resources>
|
||||
|
@ -132,4 +132,5 @@
|
||||
<string name="mode_dark">Dark mode</string>
|
||||
<string name="mode_system">Follow the system setting</string>
|
||||
<string name="mode_light">Light mode</string>
|
||||
<string name="drawer_error_loading_sources">Error loading sources…</string>
|
||||
</resources>
|
||||
|
@ -132,4 +132,5 @@
|
||||
<string name="mode_dark">Dark mode</string>
|
||||
<string name="mode_system">Follow the system setting</string>
|
||||
<string name="mode_light">Light mode</string>
|
||||
<string name="drawer_error_loading_sources">Error loading sources…</string>
|
||||
</resources>
|
||||
|
@ -132,4 +132,5 @@
|
||||
<string name="mode_dark">Dark mode</string>
|
||||
<string name="mode_system">Follow the system setting</string>
|
||||
<string name="mode_light">Light mode</string>
|
||||
<string name="drawer_error_loading_sources">Error loading sources…</string>
|
||||
</resources>
|
||||
|
@ -132,4 +132,5 @@
|
||||
<string name="mode_dark">Thème sombre</string>
|
||||
<string name="mode_system">Utiliser les paramètres système</string>
|
||||
<string name="mode_light">Thème clair</string>
|
||||
<string name="drawer_error_loading_sources">Error loading sources…</string>
|
||||
</resources>
|
||||
|
@ -132,4 +132,5 @@
|
||||
<string name="mode_dark">Dark mode</string>
|
||||
<string name="mode_system">Follow the system setting</string>
|
||||
<string name="mode_light">Light mode</string>
|
||||
<string name="drawer_error_loading_sources">Error loading sources…</string>
|
||||
</resources>
|
||||
|
@ -132,4 +132,5 @@
|
||||
<string name="mode_dark">Dark mode</string>
|
||||
<string name="mode_system">Follow the system setting</string>
|
||||
<string name="mode_light">Light mode</string>
|
||||
<string name="drawer_error_loading_sources">Error loading sources…</string>
|
||||
</resources>
|
||||
|
@ -132,4 +132,5 @@
|
||||
<string name="mode_dark">Dark mode</string>
|
||||
<string name="mode_system">Follow the system setting</string>
|
||||
<string name="mode_light">Light mode</string>
|
||||
<string name="drawer_error_loading_sources">Error loading sources…</string>
|
||||
</resources>
|
||||
|
@ -132,4 +132,5 @@
|
||||
<string name="mode_dark">Dark mode</string>
|
||||
<string name="mode_system">Follow the system setting</string>
|
||||
<string name="mode_light">Light mode</string>
|
||||
<string name="drawer_error_loading_sources">Error loading sources…</string>
|
||||
</resources>
|
||||
|
@ -132,4 +132,5 @@
|
||||
<string name="mode_dark">Dark mode</string>
|
||||
<string name="mode_system">Follow the system setting</string>
|
||||
<string name="mode_light">Light mode</string>
|
||||
<string name="drawer_error_loading_sources">Error loading sources…</string>
|
||||
</resources>
|
||||
|
@ -132,4 +132,5 @@
|
||||
<string name="mode_dark">Dark mode</string>
|
||||
<string name="mode_system">Follow the system setting</string>
|
||||
<string name="mode_light">Light mode</string>
|
||||
<string name="drawer_error_loading_sources">Error loading sources…</string>
|
||||
</resources>
|
||||
|
@ -132,4 +132,5 @@
|
||||
<string name="mode_dark">Dark mode</string>
|
||||
<string name="mode_system">Follow the system setting</string>
|
||||
<string name="mode_light">Light mode</string>
|
||||
<string name="drawer_error_loading_sources">Error loading sources…</string>
|
||||
</resources>
|
||||
|
@ -132,4 +132,5 @@
|
||||
<string name="mode_dark">Dark mode</string>
|
||||
<string name="mode_system">Follow the system setting</string>
|
||||
<string name="mode_light">Light mode</string>
|
||||
<string name="drawer_error_loading_sources">Error loading sources…</string>
|
||||
</resources>
|
||||
|
@ -132,4 +132,5 @@
|
||||
<string name="mode_dark">Dark mode</string>
|
||||
<string name="mode_system">Follow the system setting</string>
|
||||
<string name="mode_light">Light mode</string>
|
||||
<string name="drawer_error_loading_sources">Error loading sources…</string>
|
||||
</resources>
|
||||
|
@ -132,4 +132,5 @@
|
||||
<string name="mode_dark">深色模式</string>
|
||||
<string name="mode_system">遵循系统设置</string>
|
||||
<string name="mode_light">浅色模式</string>
|
||||
<string name="drawer_error_loading_sources">Error loading sources…</string>
|
||||
</resources>
|
||||
|
@ -132,4 +132,5 @@
|
||||
<string name="mode_dark">Dark mode</string>
|
||||
<string name="mode_system">Follow the system setting</string>
|
||||
<string name="mode_light">Light mode</string>
|
||||
<string name="drawer_error_loading_sources">Error loading sources…</string>
|
||||
</resources>
|
||||
|
@ -63,6 +63,7 @@
|
||||
<string name="card_height_off">Card height will be fixed</string>
|
||||
<string name="source_code">Source code</string>
|
||||
<string name="drawer_error_loading_tags">Error loading tags…</string>
|
||||
<string name="drawer_error_loading_sources">Error loading sources…</string>
|
||||
AmineB marked this conversation as resolved
AmineB
commented
Adding this will make the build fail. It should be added to all the translation files with the same value. Adding this will make the build fail.
It should be added to all the translation files with the same value.
|
||||
<string name="drawer_item_filters">Filters</string>
|
||||
<string name="drawer_action_clear">clear</string>
|
||||
<string name="drawer_item_tags">Tags</string>
|
||||
@ -109,7 +110,7 @@
|
||||
<string name="pref_switch_periodic_refresh_on">Articles will periodically be synced</string>
|
||||
<string name="pref_periodic_refresh_minutes_title"><![CDATA[Sync interval ( >= 15 minutes)]]></string>
|
||||
<string name="pref_switch_refresh_when_charging">Only refresh when phone is charging</string>
|
||||
<string name="loading_notification_title">Loading ...</string>
|
||||
<string name="loading_notification_title">Loading …</string>
|
||||
<string name="loading_notification_text">Selfoss is syncing your articles</string>
|
||||
<string name="notification_channel_sync">Sync notification</string>
|
||||
<string name="new_items_channel_sync">New items notification</string>
|
||||
|
@ -42,6 +42,7 @@ class Repository(private val api: SelfossApi, private val appSettingsService: Ap
|
||||
|
||||
init {
|
||||
// TODO: Dispatchers.IO not available in KMM, an alternative solution should be found
|
||||
connectivityStatus.start()
|
||||
runBlocking {
|
||||
updateApiVersion()
|
||||
dateUtils = DateUtils(appSettingsService)
|
||||
@ -410,11 +411,9 @@ class Repository(private val api: SelfossApi, private val appSettingsService: Ap
|
||||
private fun deleteDBAction(action: ACTION) =
|
||||
db.actionsQueries.deleteAction(action.id)
|
||||
|
||||
// TODO: This function should be private
|
||||
fun getDBTags(): List<TAG> = db.tagsQueries.tags().executeAsList()
|
||||
private fun getDBTags(): List<TAG> = db.tagsQueries.tags().executeAsList()
|
||||
|
||||
// TODO: This function should be private
|
||||
fun getDBSources(): List<SOURCE> = db.sourcesQueries.sources().executeAsList()
|
||||
private fun getDBSources(): List<SOURCE> = db.sourcesQueries.sources().executeAsList()
|
||||
|
||||
private fun resetDBTagsWithData(tagEntities: List<SelfossModel.Tag>) {
|
||||
db.tagsQueries.deleteAllTags()
|
||||
|
@ -42,6 +42,8 @@ class RepositoryTest() {
|
||||
every { db.tagsQueries.deleteAllTags() } returns Unit
|
||||
every { db.tagsQueries.transaction(any(), any()) } returns Unit
|
||||
every { db.tagsQueries.insertTag(any()) } returns Unit
|
||||
|
||||
every { connectivityStatus.start() } returns Unit
|
||||
}
|
||||
|
||||
@Test
|
||||
|
This doesn't worked as expected since the itemAdapter is cleared immediately after.