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

Use /sources/stats in the home #133

Merged
AmineB merged 9 commits from davidoskky/ReaderForSelfoss-multiplatform:sources into master 2023-03-13 16:26:57 +00:00
Conversation 2 Commits 9 Files Changed 9 +142 -95
9 changed files with 142 additions and 95 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 0c942f7a80 - Show all commits

2
androidApp/src/main/java/bou/amine/apps/readerforselfossv2/android/SourcesActivity.kt
View File

@ -49,7 +49,7 @@ class SourcesActivity : AppCompatActivity(), DIAware {
super.onResume()
val mLayoutManager = LinearLayoutManager(this)
var items: ArrayList<SelfossModel.Source>
var items: ArrayList<SelfossModel.SourceDetail>
binding.recyclerView.setHasFixedSize(true)
binding.recyclerView.layoutManager = mLayoutManager

2
androidApp/src/main/java/bou/amine/apps/readerforselfossv2/android/UpsertSourceActivity.kt
View File

@ -24,7 +24,7 @@ import org.kodein.di.instance
class UpsertSourceActivity : AppCompatActivity(), DIAware {
private var existingSource: SelfossModel.Source? = null
private var existingSource: SelfossModel.SourceDetail? = null
private var mSpoutsValue: String? = null
private lateinit var binding: ActivityUpsertSourceBinding

2
androidApp/src/main/java/bou/amine/apps/readerforselfossv2/android/adapters/SourcesListAdapter.kt
View File

@ -31,7 +31,7 @@ import org.kodein.di.instance
class SourcesListAdapter(
private val app: Activity,
private val items: ArrayList<SelfossModel.Source>
private val items: ArrayList<SelfossModel.SourceDetail>
) : RecyclerView.Adapter<SourcesListAdapter.ViewHolder>(), DIAware {
private val c: Context = app.baseContext
private val generator: ColorGenerator = ColorGenerator.MATERIAL

2
androidApp/src/main/java/bou/amine/apps/readerforselfossv2/android/fragments/FilterSheetFragment.kt
View File

@ -84,7 +84,7 @@ class FilterSheetFragment : BottomSheetDialogFragment(), DIAware {
) {
val sourceGroup = binding.sourcesGroup
repository.getSourcesStats().forEach { source ->
repository.getSourcesDetailsOrStats().forEach { source ->
val c = Chip(context)
c.ellipsize = TextUtils.TruncateAt.END

148
shared/src/commonMain/kotlin/bou/amine/apps/readerforselfossv2/model/SelfossModel.kt
View File

@ -1,5 +1,6 @@
package bou.amine.apps.readerforselfossv2.model
import bou.amine.apps.readerforselfossv2.dao.SOURCE
import bou.amine.apps.readerforselfossv2.utils.DateUtils
import bou.amine.apps.readerforselfossv2.utils.getHtmlDecoded
import kotlinx.serialization.KSerializer
@ -63,70 +64,117 @@ class SelfossModel {
fun isPublicModeEnabled() = publicMode ?: false
}
data class Source(
val id: Int,
AmineB marked this conversation as resolved
AmineB commented 2023-02-07 21:04:24 +00:00
Review
Copy Link

Source data class should be changed to an interface

 public interface Source {
        val id: Int
        val title: String
        var unread: Int?
        val icon: String?
    }

SourceDetail and SourceStats should implement it.

`Source` data class should be changed to an interface ``` public interface Source { val id: Int val title: String var unread: Int? val icon: String? } ``` `SourceDetail` and `SourceStats` should implement it.
👍 1
val title: String,
var unread: Int?,
var tags: List<String>?,
var spout: String?,
var error: String?,
var icon: String?,
var params: SourceParams?
) {
interface Source {
val id: Int
var title: String
var unread: Int?
var error: String?
var icon: String?
constructor(sourceDetail: SourceDetail) : this(
id = sourceDetail.id,
title = sourceDetail.title,
unread = null,
tags = sourceDetail.tags,
spout = sourceDetail.spout,
error = sourceDetail.error,
icon = sourceDetail.icon,
params = sourceDetail.params
)
constructor(sourceStat: SourceStats) : this(
id = sourceStat.id,
title = sourceStat.title,
unread = sourceStat.unread,
tags = null,
spout = null,
error = null,
icon = null,
params = null
)
fun checkSameSource(source: Source): Boolean {
AmineB marked this conversation as resolved
AmineB commented 2023-03-12 19:14:05 +00:00
Review
Copy Link

I don't understand why this is needed ?

I don't understand why this is needed ?
return this.id != source.id
}
AmineB marked this conversation as resolved
AmineB commented 2023-02-07 20:55:00 +00:00
Review
Copy Link

This should be an extension function fun SourceDetail.toSource(): Source

This should be an extension function `fun SourceDetail.toSource(): Source`
AmineB commented 2023-02-07 21:03:37 +00:00
Review
Copy Link

Ignore this.

Ignore this.
fun update(source: Source) {
AmineB marked this conversation as resolved
AmineB commented 2023-03-12 19:14:47 +00:00
Review
Copy Link

This should be an "abstract" method.

This should be an "abstract" method.
if (this.id != source.id || this.title != source.title) {
if (source is SourceDetail) {
this.update(source)
} else if (source is SourceStats) {
this.update(source)
}
}
fun update(source: SourceStats)
AmineB marked this conversation as resolved
AmineB commented 2023-03-12 19:15:02 +00:00
Review
Copy Link

This should not be here.

This should not be here.
fun update(source: SourceDetail)
AmineB marked this conversation as resolved
AmineB commented 2023-03-12 19:15:06 +00:00
Review
Copy Link

This should not be here.

This should not be here.
AmineB marked this conversation as resolved
AmineB commented 2023-02-07 20:55:21 +00:00
Review
Copy Link

This should be an extension function fun SourceStats.toSource(): Source

This should be an extension function `fun SourceStats.toSource(): Source`
AmineB commented 2023-02-07 21:03:46 +00:00
Review
Copy Link

Ignore this.

Ignore this.
fun toEntity(): SOURCE
operator fun component1(): Int { return id }
AmineB marked this conversation as resolved
AmineB commented 2023-03-12 19:15:31 +00:00
Review
Copy Link

What are these two methods ?

What are these two methods ?
operator fun component2(): String { return title }
}
@Serializable
data class SourceStats(
override val id: Int,
override var title: String,
override var unread: Int?,
override var error: String? = null,
override var icon: String? = null
) : Source {
override fun update(source: SourceStats) {
if (checkSameSource(source)) {
throw Exception()
}
this.title = source.title
this.unread = source.unread
}
override fun update(source: SourceDetail) {
AmineB marked this conversation as resolved
AmineB commented 2023-03-12 19:16:14 +00:00
Review
Copy Link

SourceStats class should not contain SourceDetail update.

`SourceStats` class should not contain `SourceDetail` update.
if (checkSameSource(source)) {
throw Exception()
}
this.title = source.title
this.error = source.error
this.icon = source.icon
}
override fun toEntity(): SOURCE {
return SOURCE(
this.id.toString(),
this.title.getHtmlDecoded(),
null,
null,
this.error,
this.icon,
null
)
}
}
@Serializable
data class SourceDetail(
override val id: Int,
override var title: String,
override var unread: Int? = null,
@Serializable(with = TagsListSerializer::class)
var tags: List<String>?,
var spout: String?,
override var error: String?,
override var icon: String?,
var params: SourceParams?
) : Source {
override fun update(source: SourceStats) {
AmineB marked this conversation as resolved
AmineB commented 2023-03-12 19:16:44 +00:00
Review
Copy Link

SourceDetail should not contain SourceStats update

`SourceDetail` should not contain `SourceStats` update
if (checkSameSource(source)) {
throw Exception()
}
this.title = source.title
this.unread = source.unread
}
override fun update(source: SourceDetail) {
if (checkSameSource(source)) {
throw Exception()
}
this.title = source.title
this.tags = source.tags
this.spout = source.spout
this.error = source.error
this.icon = source.icon
this.params = source.params
}
override fun toEntity(): SOURCE {
return SOURCE(
this.id.toString(),
this.title.getHtmlDecoded(),
this.tags?.joinToString(","),
this.spout,
this.error,
this.icon.orEmpty(),
this.params?.url
)
}
}
@Serializable
data class SourceStats(
val id: Int,
val title: String,
val unread: Int
)
@Serializable
data class SourceDetail(
val id: Int,
val title: String,
@Serializable(with = TagsListSerializer::class)
val tags: List<String>,
val spout: String,
val error: String,
val icon: String?,
val params: SourceParams?
)
@Serializable
data class SourceParams(
val url: String

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

@ -44,12 +44,11 @@ class Repository(
private val _badgeStarred = MutableStateFlow(0)
val badgeStarred = _badgeStarred.asStateFlow()
private var sources = ArrayList<SelfossModel.Source>()
private var fetchedTags = false
AmineB marked this conversation as resolved
AmineB commented 2023-02-07 21:23:22 +00:00
Review
Copy Link

This shouldn't be changed/added.

This shouldn't be changed/added.
private var fetchedSources = false
private var _readerItems = ArrayList<SelfossModel.Item>()
private var _selectedSource: SelfossModel.Source? = null
private var _selectedSource: SelfossModel.SourceDetail? = null
suspend fun getNewerItems(): ArrayList<SelfossModel.Item> {
var fetchedItems: StatusAndData<List<SelfossModel.Item>> = StatusAndData.error()
@ -181,47 +180,62 @@ class Repository(
}
}
AmineB marked this conversation as resolved
AmineB commented 2023-02-07 21:07:38 +00:00
Review
Copy Link

There should be two methods:

suspend fun getSourceDetailsOrStats(): ArrayList<SelfossModel.Source> {
  if (IS PUBLIC MODE) {
    return api.sourcesStats()
  } else {
    copy/past the content of the old `getSources()` method
  }
}

suspend fun getSourcesDetails(): List<SelfossModel.SourceDetail> { // This will be called in the sources activity
    if (isNetworkAvailable()) {
        return api.sourcesDetailed().data.orEmpty()
    } 
    return emptyList<>()
}
There should be two methods: ``` suspend fun getSourceDetailsOrStats(): ArrayList<SelfossModel.Source> { if (IS PUBLIC MODE) { return api.sourcesStats() } else { copy/past the content of the old `getSources()` method } } suspend fun getSourcesDetails(): List<SelfossModel.SourceDetail> { // This will be called in the sources activity if (isNetworkAvailable()) { return api.sourcesDetailed().data.orEmpty() } return emptyList<>() } ```
davidoskky commented 2023-02-11 17:02:43 +00:00
Review
Copy Link

I'm not sure about this. Doing so is fine only as long as we don't care about the unread count. That is only available in the stats.
If we don't plan on displaying the unread count, this is fine; otherwise the amount of unread articles should be available in the home activity.

I'm not sure about this. Doing so is fine only as long as we don't care about the unread count. That is only available in the stats. If we don't plan on displaying the unread count, this is fine; otherwise the amount of unread articles should be available in the home activity.
AmineB commented 2023-02-12 19:41:48 +00:00
Review
Copy Link

Unread count was complicated code-wise. We have no idea if the feature was used/useful, so for now, let's not add it back.

Unread count was complicated code-wise. We have no idea if the feature was used/useful, so for now, let's not add it back.
👍 1
davidoskky commented 2023-02-13 08:44:58 +00:00
Review
Copy Link

I did find the tags unread count useful. I will not add this here; we can always implement it later.

I did find the tags unread count useful. I will not add this here; we can always implement it later.
private fun updateSources(newSources: ArrayList<SelfossModel.Source>) {
private fun updateSources(newSources: List<SelfossModel.Source>): List<SelfossModel.Source> {
val isDatabaseEnabled =
appSettingsService.isItemCachingEnabled() || !appSettingsService.isUpdateSourcesEnabled()
val newIds = newSources.map {it.id}
val filteredSources = sources.filterNot { it.id in newIds }
for (source in filteredSources) {
val newSource = newSources.find { it.id == source.id }
source.update(newSource!!)
}
sources = (filteredSources + newSources).distinctBy { it.id } as ArrayList<SelfossModel.Source>
return if (isDatabaseEnabled) {
AmineB marked this conversation as resolved
AmineB commented 2023-03-12 19:25:09 +00:00
Review
Copy Link

All this is overly complicated.

All this is overly complicated.
var sources = getDBSources().map { it.toView() }
for (source in sources) {
val newSource = newSources.find { it.id == source.id }
if (newSource != null) {
source.update(newSource)
}
}
sources = (sources + newSources).distinctBy { it.id }
if (isDatabaseEnabled) {
resetDBSourcesWithData(sources)
sources
} else {
newSources
}
}
suspend fun getSourcesStats(): ArrayList<SelfossModel.Source> {
suspend fun getSourcesDetailsOrStats(): ArrayList<SelfossModel.Source> {
var sources = ArrayList<SelfossModel.Source>()
val isDatabaseEnabled =
appSettingsService.isItemCachingEnabled() || !appSettingsService.isUpdateSourcesEnabled()
if (isNetworkAvailable() && sources.none { it.unread != null }) {
val apiSources = api.sourcesStats()
if (apiSources.success && apiSources.data != null) {
updateSources(apiSources.data.map { SelfossModel.Source(it) } as ArrayList)
val shouldFetch = if (!appSettingsService.isUpdateSourcesEnabled()) !fetchedSources else true
if (shouldFetch && isNetworkAvailable()) {
if (appSettingsService.getPublicAccess()) {
val apiSources = api.sourcesStats()
if (apiSources.success && apiSources.data != null) {
fetchedSources = true
sources = updateSources(apiSources.data) as ArrayList<SelfossModel.Source>
AmineB marked this conversation as resolved
AmineB commented 2023-03-12 19:25:38 +00:00
Review
Copy Link

We only update the DB from getSourcesDetails

We only update the DB from `getSourcesDetails`
}
} else {
sources = getSourcesDetails() as ArrayList<SelfossModel.Source>
}
} else if (isDatabaseEnabled) {
updateSources(getDBSources().map { it.toView() } as ArrayList)
sources = getDBSources().map { it.toView() } as ArrayList<SelfossModel.Source>
}
return sources
}
suspend fun getSourcesDetails(): ArrayList<SelfossModel.Source> {
suspend fun getSourcesDetails(): ArrayList<SelfossModel.SourceDetail> {
var sources = ArrayList<SelfossModel.SourceDetail>()
val isDatabaseEnabled =
appSettingsService.isItemCachingEnabled() || !appSettingsService.isUpdateSourcesEnabled()
if (isNetworkAvailable() && sources.none { it.spout != null }) {
val shouldFetch = if (!appSettingsService.isUpdateSourcesEnabled()) !fetchedSources else true
if (shouldFetch && isNetworkAvailable()) {
val apiSources = api.sourcesDetailed()
if (apiSources.success && apiSources.data != null) {
updateSources(apiSources.data.map { SelfossModel.Source(it) } as ArrayList)
fetchedSources = true
sources = updateSources(apiSources.data) as ArrayList<SelfossModel.SourceDetail>
}
} else if (isDatabaseEnabled) {
updateSources(getDBSources().map { it.toView() } as ArrayList)
sources = getDBSources().map { it.toView() } as ArrayList<SelfossModel.SourceDetail>
}
return sources
}
@ -400,7 +414,6 @@ class Repository(
items = ArrayList(items.filter { it.sourcetitle != title })
setReaderItems(items)
db.itemsQueries.deleteItemsWhereSource(title)
sources.removeAll { it.id == id }
}
return success
@ -620,7 +633,7 @@ class Repository(
ReaderForSelfossDB.Schema.migrate(driverFactory.createDriver(), 0, 1)
}
fun setSelectedSource(source: SelfossModel.Source) {
fun setSelectedSource(source: SelfossModel.SourceDetail) {
_selectedSource = source
}
@ -628,7 +641,7 @@ class Repository(
_selectedSource = null
}
fun getSelectedSource(): SelfossModel.Source? {
fun getSelectedSource(): SelfossModel.SourceDetail? {
return _selectedSource
}
}

16
shared/src/commonMain/kotlin/bou/amine/apps/readerforselfossv2/utils/EntityUtils.kt
View File

@ -13,10 +13,10 @@ fun TAG.toView(): SelfossModel.Tag =
)
fun SOURCE.toView(): SelfossModel.Source =
SelfossModel.Source(
SelfossModel.SourceDetail(
this.id.toInt(),
this.title,
this.unread?.toInt(),
null,
this.tags?.split(","),
this.spout,
this.error,
@ -24,18 +24,6 @@ fun SOURCE.toView(): SelfossModel.Source =
if (this.url != null) SelfossModel.SourceParams(this.url) else null
)
fun SelfossModel.Source.toEntity(): SOURCE =
SOURCE(
this.id.toString(),
this.title.getHtmlDecoded(),
this.unread?.toLong(),
this.tags?.joinToString(","),
this.spout,
this.error,
this.icon.orEmpty(),
this.params?.url
)
fun SelfossModel.Tag.toEntity(): TAG =
TAG(
this.tag,

1
shared/src/commonMain/sqldelight/bou/amine/apps/readerforselfossv2/dao/3.sqm
View File

@ -1 +0,0 @@
ALTER TABLE SOURCE ADD COLUMN `unread` INTEGER;

1
shared/src/commonMain/sqldelight/bou/amine/apps/readerforselfossv2/dao/Sources.sq
View File

@ -1,7 +1,6 @@
CREATE TABLE SOURCE (
`id` TEXT NOT NULL,
`title` TEXT NOT NULL,
`unread` INTEGER,
`tags` TEXT,
AmineB marked this conversation as resolved
AmineB commented 2023-02-07 21:31:50 +00:00
Review
Copy Link

As unread is only in the public mode, let's not change this.

As `unread` is only in the public mode, let's not change this.
👍 1
`spout` TEXT,
`error` TEXT,