Skip to content

refactor(read-state): rename persisted isUnread fields to isRead #38

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
johan456789 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

refactor(read-state): rename persisted isUnread fields to isRead #38

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/src/main/java/me/ash/reader/domain/data/DiffMapHolder.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ class DiffMapHolder @Inject constructor(
articleId = diff.articleId,
accountId = accountId,
feedId = diff.feedId,
isUnread = !diff.isRead,
isRead = diff.isRead,
)
}

Expand Down
10 changes: 2 additions & 8 deletions app/src/main/java/me/ash/reader/domain/model/article/Article.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ data class Article(
var feedId: String,
@field:ColumnInfo(index = true)
var accountId: Int,
@field:ColumnInfo
var isUnread: Boolean = true,
@field:ColumnInfo(name = "isUnread")
var isRead: Boolean = false,
Comment on lines +44 to +45

@gemini-code-assist gemini-code-assist Bot Apr 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

This change introduces a logic inversion bug. By renaming the field to isRead but mapping it to the existing isUnread database column via @ColumnInfo(name = "isUnread"), the boolean logic is flipped upon persistence.

In the database, isUnread = 1 (true) represents an unread article. When Room loads this value into the new isRead field, it will set isRead = true, meaning the application will treat unread articles as read.

To fix this while avoiding database migrations, you should keep the underlying field named isUnread (matching the database semantics) and use a computed property for isRead (reverting the changes in lines 57-61). If you truly want to rename the persisted field, you must provide a database migration to rename the column and invert all existing boolean values (isRead = NOT isUnread).

Comment on lines +44 to +45

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve unread inversion in Article entity mapping

Article.isRead is now bound directly to the legacy isUnread column, but Room does not invert booleans. That means rows stored as unread (isUnread = 1) are materialized as isRead = true, and inserts with isRead = true persist back as unread. Code paths that trust article.isRead (for example read/unread UI state and diff toggling) will therefore treat unread items as read and can apply the opposite action. Keep an explicit inversion layer when mapping this column.

Useful? React with 👍 / 👎.

@field:ColumnInfo
var isStarred: Boolean = false,
@field:ColumnInfo
Expand All @@ -53,10 +53,4 @@ data class Article(

@Ignore
var dateString: String? = null

var isRead: Boolean
get() = !isUnread
set(value) {
isUnread = !value
}
}
12 changes: 3 additions & 9 deletions app/src/main/java/me/ash/reader/domain/model/article/ArticleMeta.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,8 @@ import androidx.room.PrimaryKey
data class ArticleMeta(
@field:PrimaryKey
var id: String,
@field:ColumnInfo
var isUnread: Boolean = true,
@field:ColumnInfo(name = "isUnread")
var isRead: Boolean = false,
Comment on lines +13 to +14

@gemini-code-assist gemini-code-assist Bot Apr 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

Similar to the Article model, renaming this field to isRead while mapping it to the isUnread column inverts the logic. Articles that are unread in the database (isUnread = 1) will be loaded as isRead = true. This will break the read/unread state tracking throughout the application.

Comment on lines +13 to +14

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Restore correct read semantics for ArticleMeta

ArticleMeta.isRead is now also mapped directly from isUnread, so metadata-based sync logic reads inverted states. In GoogleReaderRssService, local unread/read sets are built from queryMetadataAll via filterNot { it.isRead } and filter { it.isRead }; with this mapping those sets are swapped, so reconciliation can miss true local-vs-remote read mismatches and skip necessary updates. ArticleMeta.isRead must remain inverted from the stored unread flag (or SQL should project an inverted value).

Useful? React with 👍 / 👎.

@field:ColumnInfo
var isStarred: Boolean = false,
) {
var isRead: Boolean
get() = !isUnread
set(value) {
isUnread = !value
}
}
)
9 changes: 3 additions & 6 deletions app/src/main/java/me/ash/reader/domain/model/article/PendingReadStateOp.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,8 @@ data class PendingReadStateOp(
val accountId: Int,
@ColumnInfo
val feedId: String,
@ColumnInfo
val isUnread: Boolean,
@ColumnInfo(name = "isUnread")
val isRead: Boolean,
Comment on lines +20 to +21

@gemini-code-assist gemini-code-assist Bot Apr 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

Renaming this field to isRead while mapping it to the isUnread column inverts the logic. Pending operations stored in the database will be interpreted with the opposite meaning when loaded, leading to incorrect synchronization with remote services.

@ColumnInfo
val updatedAt: Date = Date(),
) {
val isRead: Boolean
get() = !isUnread
}
)
78 changes: 39 additions & 39 deletions app/src/main/java/me/ash/reader/domain/repository/ArticleDao.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,15 @@ interface ArticleDao {

@Query(
"""
UPDATE article SET isUnread = :storedUnread
UPDATE article SET isUnread = NOT :isRead
WHERE accountId = :accountId
AND id in (:ids)
"""
)
fun markAsReadByIdSet(
accountId: Int,
ids: Set<String>,
storedUnread: Boolean,
isRead: Boolean,
): Int

@Transaction
Expand Down Expand Up @@ -92,7 +92,7 @@ interface ArticleDao {
AND feedId IN (
SELECT id FROM feed WHERE groupId = :groupId
)
AND isUnread = :isUnread
AND isUnread = NOT :isRead
AND (
title LIKE '%' || :text || '%'
OR shortDescription LIKE '%' || :text || '%'
Expand All @@ -107,7 +107,7 @@ interface ArticleDao {
accountId: Int,
text: String,
groupId: String,
isUnread: Boolean,
isRead: Boolean,

@gemini-code-assist gemini-code-assist Bot Apr 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The parameter has been renamed to isRead, but the method name searchArticleByGroupIdWhenIsUnread still contains 'WhenIsUnread'. This creates a confusing API where passing isRead = true to a 'WhenIsUnread' method returns read articles. If the refactor proceeds (after fixing the logic inversion in the models), these methods should be renamed to something more neutral like searchArticleByGroupIdByReadState to maintain clarity.

sortAscending: Boolean = false
): PagingSource<Int, ArticleWithFeed>

Expand Down Expand Up @@ -166,7 +166,7 @@ interface ArticleDao {
SELECT * FROM article
WHERE accountId = :accountId
AND feedId = :feedId
AND isUnread = :isUnread
AND isUnread = NOT :isRead
AND (
title LIKE '%' || :text || '%'
OR shortDescription LIKE '%' || :text || '%'
Expand All @@ -181,7 +181,7 @@ interface ArticleDao {
accountId: Int,
text: String,
feedId: String,
isUnread: Boolean,
isRead: Boolean,
sortAscending: Boolean = false
): PagingSource<Int, ArticleWithFeed>

Expand Down Expand Up @@ -235,7 +235,7 @@ interface ArticleDao {
"""
SELECT * FROM article
WHERE accountId = :accountId
AND isUnread = :isUnread
AND isUnread = NOT :isRead
AND (
title LIKE '%' || :text || '%'
OR shortDescription LIKE '%' || :text || '%'
Expand All @@ -247,7 +247,7 @@ interface ArticleDao {
"""
)
fun searchArticleWhenIsUnread(
accountId: Int, text: String, isUnread: Boolean, sortAscending: Boolean = false
accountId: Int, text: String, isRead: Boolean, sortAscending: Boolean = false
): PagingSource<Int, ArticleWithFeed>

@Transaction
Expand Down Expand Up @@ -321,67 +321,67 @@ interface ArticleDao {
@Transaction
@Query(
"""
UPDATE article SET isUnread = :storedUnread
UPDATE article SET isUnread = NOT :isRead
WHERE accountId = :accountId
AND date < :before
AND isUnread != :storedUnread
AND isUnread != NOT :isRead
"""
)
suspend fun markAllAsRead(
accountId: Int,
storedUnread: Boolean,
isRead: Boolean,
before: Date,
)

@Transaction
@Query(
"""
UPDATE article SET isUnread = :storedUnread
UPDATE article SET isUnread = NOT :isRead
WHERE feedId IN (
SELECT id FROM feed
WHERE groupId = :groupId
)
AND accountId = :accountId
AND isUnread != :storedUnread
AND isUnread != NOT :isRead
AND date < :before
"""
)
suspend fun markAllAsReadByGroupId(
accountId: Int,
groupId: String,
storedUnread: Boolean,
isRead: Boolean,
before: Date,
)

@Transaction
@Query(
"""
UPDATE article SET isUnread = :storedUnread
UPDATE article SET isUnread = NOT :isRead
WHERE feedId = :feedId
AND accountId = :accountId
AND isUnread != :storedUnread
AND isUnread != NOT :isRead
AND date < :before
"""
)
suspend fun markAllAsReadByFeedId(
accountId: Int,
feedId: String,
storedUnread: Boolean,
isRead: Boolean,
before: Date,
)

@Transaction
@Query(
"""
UPDATE article SET isUnread = :storedUnread
UPDATE article SET isUnread = NOT :isRead
WHERE id = :articleId
AND accountId = :accountId
"""
)
suspend fun markAsReadByArticleId(
accountId: Int,
articleId: String,
storedUnread: Boolean,
isRead: Boolean,
)

@Query(
Expand Down Expand Up @@ -436,14 +436,14 @@ interface ArticleDao {
"""
SELECT feedId, COUNT(*) AS important
FROM article
WHERE isUnread = :isUnread
WHERE isUnread = NOT :isRead
AND accountId = :accountId
GROUP BY feedId
"""
)
fun queryImportantCountWhenIsUnread(
accountId: Int,
isUnread: Boolean,
isRead: Boolean,
): Flow<Map<@MapColumn("feedId") String, @MapColumn("important") Int>>

@Transaction
Expand Down Expand Up @@ -507,15 +507,15 @@ interface ArticleDao {
@Query(
"""
SELECT * FROM article
WHERE isUnread = :isUnread
WHERE isUnread = NOT :isRead
AND accountId = :accountId
ORDER BY
CASE WHEN :sortAscending = 1 THEN date END ASC,
CASE WHEN :sortAscending = 0 THEN date END DESC
"""
)
fun queryArticleWithFeedWhenIsUnread(
accountId: Int, isUnread: Boolean, sortAscending: Boolean = false
accountId: Int, isRead: Boolean, sortAscending: Boolean = false
): PagingSource<Int, ArticleWithFeed>

@Transaction
Expand Down Expand Up @@ -572,15 +572,15 @@ interface ArticleDao {
LEFT JOIN feed AS b ON b.id = a.feedId
LEFT JOIN `group` AS c ON c.id = b.groupId
WHERE c.id = :groupId
AND a.isUnread = :isUnread
AND a.isUnread = NOT :isRead
AND a.accountId = :accountId
ORDER BY
CASE WHEN :sortAscending = 1 THEN a.date END ASC,
CASE WHEN :sortAscending = 0 THEN a.date END DESC
"""
)
fun queryArticleWithFeedByGroupIdWhenIsUnread(
accountId: Int, groupId: String, isUnread: Boolean, sortAscending: Boolean = false
accountId: Int, groupId: String, isRead: Boolean, sortAscending: Boolean = false
): PagingSource<Int, ArticleWithFeed>

@Transaction
Expand Down Expand Up @@ -619,15 +619,15 @@ interface ArticleDao {
"""
SELECT * FROM article
WHERE feedId = :feedId
AND isUnread = :isUnread
AND isUnread = NOT :isRead
AND accountId = :accountId
ORDER BY
CASE WHEN :sortAscending = 1 THEN date END ASC,
CASE WHEN :sortAscending = 0 THEN date END DESC
"""
)
fun queryArticleWithFeedByFeedIdWhenIsUnread(
accountId: Int, feedId: String, isUnread: Boolean, sortAscending: Boolean = false
accountId: Int, feedId: String, isRead: Boolean, sortAscending: Boolean = false
): PagingSource<Int, ArticleWithFeed>


Expand Down Expand Up @@ -704,14 +704,14 @@ interface ArticleDao {
"""
SELECT id, isUnread, isStarred FROM article
WHERE accountId = :accountId
AND isUnread = :isUnread
AND isUnread = NOT :isRead
ORDER BY
CASE WHEN :sortAscending = 1 THEN date END ASC,
CASE WHEN :sortAscending = 0 THEN date END DESC
"""
)
fun queryMetadataAll(
accountId: Int, isUnread: Boolean, sortAscending: Boolean = false
accountId: Int, isRead: Boolean, sortAscending: Boolean = false
): List<ArticleMeta>

@Transaction
Expand All @@ -734,15 +734,15 @@ interface ArticleDao {
"""
SELECT id, isUnread, isStarred FROM article
WHERE accountId = :accountId
AND isUnread = :isUnread
AND isUnread = NOT :isRead
AND date < :before
ORDER BY
CASE WHEN :sortAscending = 1 THEN date END ASC,
CASE WHEN :sortAscending = 0 THEN date END DESC
"""
)
fun queryMetadataAll(
accountId: Int, isUnread: Boolean, before: Date, sortAscending: Boolean = false
accountId: Int, isRead: Boolean, before: Date, sortAscending: Boolean = false
): List<ArticleMeta>

@Transaction
Expand All @@ -751,14 +751,14 @@ interface ArticleDao {
SELECT id, isUnread, isStarred FROM article
WHERE accountId = :accountId
AND feedId = :feedId
AND isUnread = :isUnread
AND isUnread = NOT :isRead
ORDER BY
CASE WHEN :sortAscending = 1 THEN date END ASC,
CASE WHEN :sortAscending = 0 THEN date END DESC
"""
)
fun queryMetadataByFeedId(
accountId: Int, feedId: String, isUnread: Boolean, sortAscending: Boolean = false
accountId: Int, feedId: String, isRead: Boolean, sortAscending: Boolean = false
): List<ArticleMeta>

@Transaction
Expand All @@ -767,7 +767,7 @@ interface ArticleDao {
SELECT id, isUnread, isStarred FROM article
WHERE accountId = :accountId
AND feedId = :feedId
AND isUnread = :isUnread
AND isUnread = NOT :isRead
AND date < :before
ORDER BY
CASE WHEN :sortAscending = 1 THEN date END ASC,
Expand All @@ -777,7 +777,7 @@ interface ArticleDao {
fun queryMetadataByFeedId(
accountId: Int,
feedId: String,
isUnread: Boolean,
isRead: Boolean,
before: Date,
sortAscending: Boolean = false
): List<ArticleMeta>
Expand All @@ -791,14 +791,14 @@ interface ArticleDao {
LEFT JOIN `group` AS c ON c.id = b.groupId
WHERE c.id = :groupId
AND a.accountId = :accountId
AND a.isUnread = :isUnread
AND a.isUnread = NOT :isRead
ORDER BY
CASE WHEN :sortAscending = 1 THEN a.date END ASC,
CASE WHEN :sortAscending = 0 THEN a.date END DESC
"""
)
fun queryMetadataByGroupIdWhenIsUnread(
accountId: Int, groupId: String, isUnread: Boolean, sortAscending: Boolean = false
accountId: Int, groupId: String, isRead: Boolean, sortAscending: Boolean = false
): List<ArticleMeta>

@Transaction
Expand All @@ -810,7 +810,7 @@ interface ArticleDao {
LEFT JOIN `group` AS c ON c.id = b.groupId
WHERE c.id = :groupId
AND a.accountId = :accountId
AND a.isUnread = :isUnread
AND a.isUnread = NOT :isRead
AND a.date < :before
ORDER BY
CASE WHEN :sortAscending = 1 THEN a.date END ASC,
Expand All @@ -820,7 +820,7 @@ interface ArticleDao {
fun queryMetadataByGroupIdWhenIsUnread(
accountId: Int,
groupId: String,
isUnread: Boolean,
isRead: Boolean,
before: Date,
sortAscending: Boolean = false
): List<ArticleMeta>
Expand Down
Loading