Conversation
Contributor
📝 WalkthroughWalkthroughThe PR converts DAO and model layers from LiveData to Kotlin Flow, introduces debounced search via a Flow-based SearchResult, adds Flow-to-LiveData conversion helpers, and optimizes adapter updates to refresh only affected items during keyword changes. Changes
Sequence DiagramsequenceDiagram
participant User as User Input
participant SearchFrag as SearchFragment
participant SearchResult as SearchResult
participant DAO as BaseNoteDao
participant Adapter as BaseNoteAdapter
participant UI as UI List
User->>SearchFrag: type query
SearchFrag->>SearchResult: fetch(keyword, folder, label)
activate SearchResult
SearchResult->>SearchResult: update searchParams (MutableStateFlow)
SearchResult->>SearchResult: debounce -> flatMapLatest -> map(transform)
deactivate SearchResult
SearchResult->>DAO: request matching notes (Flow)
activate DAO
DAO-->>SearchResult: Flow<List<BaseNote>>
deactivate DAO
SearchResult->>SearchFrag: emit results (as LiveData)
SearchFrag->>Adapter: setSearchKeyword(keyword)
activate Adapter
Adapter->>Adapter: matchesKeyword per item
Adapter->>UI: notifyItemChanged (targeted)
deactivate Adapter
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
app/src/main/java/com/philkes/notallyx/presentation/view/main/BaseNoteAdapter.kt (1)
74-95: Consolidate duplicated keyword-matching logic.
matchesKeywordis duplicated here and inapp/src/main/java/com/philkes/notallyx/data/dao/BaseNoteDao.kt(Line 326-344). Keeping two implementations in sync is error-prone and can desync filtering vs highlighting behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/philkes/notallyx/presentation/view/main/BaseNoteAdapter.kt` around lines 74 - 95, The duplicated keyword-matching logic in the matchesKeyword function should be consolidated into a single reusable routine: extract the current implementation into a shared function (for example a top-level function like fun matchesKeyword(baseNote: BaseNote, keyword: String): Boolean or an extension fun BaseNote.matchesKeyword(keyword: String): Boolean) that preserves behavior (return false when keyword.isBlank(), case-insensitive checks against baseNote.title, baseNote.body, each label, and each item.body), then replace the local matchesKeyword in BaseNoteAdapter and the duplicate implementation in BaseNoteDao with calls to that single shared function so both filtering and highlighting use the same code path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/com/philkes/notallyx/data/model/SearchResult.kt`:
- Around line 37-47: The loading flag can get stuck true if
baseNoteDao.getBaseNotesByKeyword throws before emitting; inside the
flatMapLatest branch where you call
baseNoteDao.getBaseNotesByKeyword(params.keyword, params.folder, params.label)
(and then .map { transform(it) }.onEach { _isLoading.value = false }), add error
and completion handling so _isLoading.value is set to false on both error and
normal completion — e.g. attach a .catch { /* set _isLoading.value = false and
rethrow/log if needed */ } and/or an .onCompletion { _isLoading.value = false }
after the DAO flow (or replace onEach with onCompletion to guarantee cleanup),
ensuring references to _isLoading, baseNoteDao.getBaseNotesByKeyword, transform,
flatMapLatest, and onEach are preserved and updated accordingly.
In
`@app/src/main/java/com/philkes/notallyx/presentation/activity/main/fragment/NotallyFragment.kt`:
- Around line 225-228: The callback is using requireNotNull(text, ...) which can
crash if Android passes a null Editable; change the null handling to a null-safe
conversion by replacing the requireNotNull usage with text?.toString().orEmpty()
so newKeyword becomes a safe String, then keep the existing
comparison/assignment to model.keyword (i.e., update the code around newKeyword
and the if (model.keyword != newKeyword) block to use the null-safe value).
In
`@app/src/main/java/com/philkes/notallyx/presentation/activity/main/fragment/SearchFragment.kt`:
- Line 53: The line binding?.ImageView?.isVisible = !isLoading incorrectly shows
the empty-state image whenever loading finishes regardless of results; change
the visibility logic so the empty image is only visible when loading is false
AND the results list is empty (e.g., use the fragment's adapter.itemCount == 0
or viewModel.searchResults.value?.isEmpty() check). Update the code in
SearchFragment.kt where binding?.ImageView is set to compute isVisible =
!isLoading && (adapter.itemCount == 0 or searchResults.isEmpty()), and ensure
you update this whenever the adapter or searchResults change.
---
Nitpick comments:
In
`@app/src/main/java/com/philkes/notallyx/presentation/view/main/BaseNoteAdapter.kt`:
- Around line 74-95: The duplicated keyword-matching logic in the matchesKeyword
function should be consolidated into a single reusable routine: extract the
current implementation into a shared function (for example a top-level function
like fun matchesKeyword(baseNote: BaseNote, keyword: String): Boolean or an
extension fun BaseNote.matchesKeyword(keyword: String): Boolean) that preserves
behavior (return false when keyword.isBlank(), case-insensitive checks against
baseNote.title, baseNote.body, each label, and each item.body), then replace the
local matchesKeyword in BaseNoteAdapter and the duplicate implementation in
BaseNoteDao with calls to that single shared function so both filtering and
highlighting use the same code path.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/build.gradle.ktsapp/src/main/java/com/philkes/notallyx/data/dao/BaseNoteDao.ktapp/src/main/java/com/philkes/notallyx/data/model/Content.ktapp/src/main/java/com/philkes/notallyx/data/model/SearchResult.ktapp/src/main/java/com/philkes/notallyx/presentation/activity/main/fragment/NotallyFragment.ktapp/src/main/java/com/philkes/notallyx/presentation/activity/main/fragment/SearchFragment.ktapp/src/main/java/com/philkes/notallyx/presentation/view/main/BaseNoteAdapter.ktapp/src/main/java/com/philkes/notallyx/presentation/viewmodel/BaseNoteModel.kt
app/src/main/java/com/philkes/notallyx/presentation/activity/main/fragment/NotallyFragment.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/philkes/notallyx/presentation/activity/main/fragment/SearchFragment.kt
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/java/com/philkes/notallyx/data/model/SearchResult.kt (1)
84-85: TheisEmptyextension returnsfalsewhenvalueisnull, which may be misleading.The expression
this?.value?.isEmpty() == truereturnsfalsewhenthisis null or whenvalueis null. This means aSearchResultwith no results yet (null value) is considered "not empty", which could cause the empty-state image to not show during initial load or after errors.Consider whether the semantics should treat null as empty:
♻️ Suggested alternative if null should be treated as empty
val SearchResult?.isEmpty: Boolean - get() = this?.value?.isEmpty() == true + get() = this?.value?.isEmpty() != false🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/philkes/notallyx/data/model/SearchResult.kt` around lines 84 - 85, The current SearchResult?.isEmpty extension treats a null SearchResult or a null value as not empty; change the logic so null is considered empty by returning true when the receiver is null or when value is null/empty — update the getter on SearchResult?.isEmpty (referring to the SearchResult class and its value property) to return true if this == null || this.value.isNullOrEmpty(), otherwise false, so the empty-state UI behaves correctly on initial load or after errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/main/java/com/philkes/notallyx/data/model/SearchResult.kt`:
- Around line 84-85: The current SearchResult?.isEmpty extension treats a null
SearchResult or a null value as not empty; change the logic so null is
considered empty by returning true when the receiver is null or when value is
null/empty — update the getter on SearchResult?.isEmpty (referring to the
SearchResult class and its value property) to return true if this == null ||
this.value.isNullOrEmpty(), otherwise false, so the empty-state UI behaves
correctly on initial load or after errors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6c3cd53c-0589-4b33-99e6-a61a66be200b
📒 Files selected for processing (8)
app/build.gradle.ktsapp/src/main/java/com/philkes/notallyx/data/dao/BaseNoteDao.ktapp/src/main/java/com/philkes/notallyx/data/model/Content.ktapp/src/main/java/com/philkes/notallyx/data/model/SearchResult.ktapp/src/main/java/com/philkes/notallyx/presentation/activity/main/fragment/NotallyFragment.ktapp/src/main/java/com/philkes/notallyx/presentation/activity/main/fragment/SearchFragment.ktapp/src/main/java/com/philkes/notallyx/presentation/view/main/BaseNoteAdapter.ktapp/src/main/java/com/philkes/notallyx/presentation/viewmodel/BaseNoteModel.kt
🚧 Files skipped from review as they are similar to previous changes (3)
- app/src/main/java/com/philkes/notallyx/data/model/Content.kt
- app/build.gradle.kts
- app/src/main/java/com/philkes/notallyx/presentation/view/main/BaseNoteAdapter.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #879
Summary by CodeRabbit
New Features
Bug Fixes
Refactor