SCRUM-267 design: add search result screen#33
Open
gdaegeun539 wants to merge 6 commits intoproject-lyrics:developfrom
Open
SCRUM-267 design: add search result screen#33gdaegeun539 wants to merge 6 commits intoproject-lyrics:developfrom
gdaegeun539 wants to merge 6 commits intoproject-lyrics:developfrom
Conversation
- add NoteTopic enum - represent note topic category
📝 WalkthroughWalkthroughAdds a new note search results feature: a NoteTopic enum and label extension, view state/data classes, a ViewModel with topic filtering and paginated loading, a Jetpack Compose screen with collapsible header and prefetch pagination, plus related documentation updates. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Screen as NoteSearchResultScreen
participant VM as NoteSearchResultViewModel
participant State as ViewState
User->>Screen: open screen
Screen->>VM: init / request loadInitial()
VM->>State: set status = LOADING, clear notes
VM->>VM: simulate delay -> filter dummy notes by selectedNoteTopic
VM->>State: set status = SUCCESS/EMPTY, notes = first page, nextCursor
State-->>Screen: viewState updated
Screen->>Screen: render header, filter, list
User->>Screen: select topic
Screen->>VM: selectTopic(topic)
VM->>VM: update selectedNoteTopic -> loadInitial()
User->>Screen: scroll near end
Screen->>VM: trigger loadNextPage()
VM->>VM: set isNextPageLoading, simulate delay, append next page, update nextCursor
VM->>State: update notes and loading flag
State-->>Screen: viewState updated
Screen->>Screen: append new items
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
app/src/main/java/com/lyrics/feelin/presentation/util/NoteTopicLabelExtension.kt (1)
5-10: Move these labels to string resources.Hardcoded Korean copy here makes localization and copy changes require code edits. A safer shape is mapping
NoteTopicto@StringResand resolving the text in the UI layer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/lyrics/feelin/presentation/util/NoteTopicLabelExtension.kt` around lines 5 - 10, Replace the hardcoded Korean strings in the NoteTopic.label extension with resource IDs: change the extension on NoteTopic (NoteTopic.label) to return an `@StringRes` Int and map each enum case (NoteTopic.ALL, INTERPRETATION, FREE, QUESTION) to R.string identifiers (e.g., R.string.note_topic_all, etc.), then update call sites in the UI layer to resolve the text with context.getString(noteTopic.label) or view.getString(...); ensure string entries are added to strings.xml for each label and adjust imports/annotations to use androidx.annotation.StringRes where appropriate.app/src/main/java/com/lyrics/feelin/core/domain/enum/NoteTopic.kt (1)
3-7: KeepALLout of the domain enum.
NoteTopiclives undercore.domain, butALLis a UI filter, not an actual note category. Keeping it here makes later API or persistence mappings easy to misuse once this enum is reused outside the screen. A separate presentation-layer filter type would keep the boundary cleaner.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/lyrics/feelin/core/domain/enum/NoteTopic.kt` around lines 3 - 7, Remove the UI-only value ALL from the domain enum NoteTopic and keep NoteTopic limited to actual categories (INTERPRETATION, FREE, QUESTION); introduce a separate presentation-layer filter type (e.g., NoteTopicFilter or PresentationNoteTopic) in the UI/presentation module that includes ALL and any other UI-only options, and add explicit mapping code between the new presentation filter and the domain NoteTopic where the UI interacts with the domain (update callers that relied on NoteTopic.ALL to use the new filter and convert to nullable/Optional or explicit domain values before passing into domain/persistence code).
🤖 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/lyrics/feelin/presentation/view/note/search/result/NoteSearchResultViewModel.kt`:
- Around line 27-33: When switching to the LOADING state in
NoteSearchResultViewModel, also reset the previous total note count so
ExpandedSearchHeader doesn't show the old count for the new query; update the
_viewState.value.copy call that sets status = NoteSearchResultStatus.LOADING to
include totalNoteCount = null (or 0 if your UI expects an int) alongside notes =
emptyList(), nextCursor = null, isNextPageLoading = false, and errorMessage =
null.
In
`@app/src/main/java/com/lyrics/feelin/presentation/view/note/search/result/NoteSearchResultViewState.kt`:
- Around line 30-44: The initial() companion factory for
NoteSearchResultViewState currently hardcodes a NoteSearchResultSongSummary (시퍼런
봄 / 쏜애플); change the factory to accept the selected song (e.g., a
NoteSearchResultSongSummary? or required param) instead of embedding a concrete
song, remove the hardcoded NoteSearchResultSongSummary from
NoteSearchResultViewState.initial, and update callers (such as
NoteSearchResultViewModel construction) to pass the actual selected song into
the new factory so viewState.songSummary reflects the real selection at startup.
In
`@app/src/main/java/com/lyrics/feelin/presentation/view/note/search/result/NoteSerachResultScreen.kt`:
- Around line 130-133: The NoteSearchResultScreen currently hardcodes an inert
back action; add a parameter onBackClick: () -> Unit = {} to the
NoteSearchResultScreen signature and pass it into the app bar/back-action
composable so the screen API exposes navigation to callers (also update the
other NoteSearchResultScreen declaration/usage that mirrors this
implementation). Locate the NoteSearchResultScreen function and any duplicate
definitions/usages, add the onBackClick parameter, and forward it to the app
bar's onBackClick handler so callers can supply navigation behavior.
---
Nitpick comments:
In `@app/src/main/java/com/lyrics/feelin/core/domain/enum/NoteTopic.kt`:
- Around line 3-7: Remove the UI-only value ALL from the domain enum NoteTopic
and keep NoteTopic limited to actual categories (INTERPRETATION, FREE,
QUESTION); introduce a separate presentation-layer filter type (e.g.,
NoteTopicFilter or PresentationNoteTopic) in the UI/presentation module that
includes ALL and any other UI-only options, and add explicit mapping code
between the new presentation filter and the domain NoteTopic where the UI
interacts with the domain (update callers that relied on NoteTopic.ALL to use
the new filter and convert to nullable/Optional or explicit domain values before
passing into domain/persistence code).
In
`@app/src/main/java/com/lyrics/feelin/presentation/util/NoteTopicLabelExtension.kt`:
- Around line 5-10: Replace the hardcoded Korean strings in the NoteTopic.label
extension with resource IDs: change the extension on NoteTopic (NoteTopic.label)
to return an `@StringRes` Int and map each enum case (NoteTopic.ALL,
INTERPRETATION, FREE, QUESTION) to R.string identifiers (e.g.,
R.string.note_topic_all, etc.), then update call sites in the UI layer to
resolve the text with context.getString(noteTopic.label) or view.getString(...);
ensure string entries are added to strings.xml for each label and adjust
imports/annotations to use androidx.annotation.StringRes where appropriate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 86c0ce97-9065-40a4-ac4f-978a4f02d145
📒 Files selected for processing (5)
app/src/main/java/com/lyrics/feelin/core/domain/enum/NoteTopic.ktapp/src/main/java/com/lyrics/feelin/presentation/util/NoteTopicLabelExtension.ktapp/src/main/java/com/lyrics/feelin/presentation/view/note/search/result/NoteSearchResultViewModel.ktapp/src/main/java/com/lyrics/feelin/presentation/view/note/search/result/NoteSearchResultViewState.ktapp/src/main/java/com/lyrics/feelin/presentation/view/note/search/result/NoteSerachResultScreen.kt
1 task
Update commit command and workflow documents to use the repository's English-only `type: Subject` format and staged-only flow with explicit confirmation before commit.
Reset the note count when a new search result load starts so the header does not briefly show the previous topic's total. Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
.opencode/command/suggest_commit.md (1)
1-132: Consider refactoring shared content withcommit.md.Steps 1–4 (lines 9–122) are nearly identical between
suggest_commit.mdandcommit.md. This duplication makes maintenance harder and increases the risk of inconsistencies when rules are updated.Consider extracting the shared workflow steps into a common reference document (e.g.,
.agent/rules/commit-message-generation.md) and having both command files reference it, leaving only the execution/presentation differences in each file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.opencode/command/suggest_commit.md around lines 1 - 132, The sections in suggest_commit.md and commit.md are duplicated; extract the shared Steps 1–4 content into a new shared reference file named commit-message-generation.md and replace the duplicated blocks in both suggest_commit.md and commit.md with a short include/reference line pointing to commit-message-generation.md, leaving only the command-specific differences (presentation/execution rules) in each original file; update any internal references in suggest_commit.md (e.g., "Step 1: Gather Context", "Step 2: Analyze Changes", "Step 3: Generate Commit Message", "Step 4: Present Options") to use the shared document so future edits are made in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.opencode/command/commit.md:
- Around line 98-122: In the "Suggested Commits" section update the three fenced
code blocks (Option 1 (recommended), Option 2 (minimal), Option 3 (detailed)) to
include a language specifier (e.g., text) after the opening triple-backticks so
markdownlint warnings are resolved; locate the blocks by their headings and
replace ``` with ```text for the examples under those three options.
- Around line 58-91: The commit guideline is missing the capitalization rule:
add a short bullet to the commit message rules (e.g., under "1. 커밋 메시지는 영문으로 쓴다"
or near the title rules) stating that "The first letter of the subject (after
the `type:` prefix) must be capitalized," mirroring the rule in the referenced
standard; ensure the wording is concise and consistent with existing bullets and
update any nearby examples or notes if present to reflect this capitalization
requirement.
In @.opencode/command/suggest_commit.md:
- Around line 98-122: The three fenced commit-message blocks under the headings
"Option 1 (recommended)", "Option 2 (minimal)", and "Option 3 (detailed)" are
missing language specifiers and trigger markdownlint warnings; fix by adding a
language tag (e.g., text) to each opening triple-backtick so the blocks become
```text, ensuring all three commit examples include the language specifier.
- Around line 57-92: Add the missing capitalization rule into the "Core rules"
section (near "1. 커밋 메시지는 영문으로 쓴다") by stating that the first letter of the
subject (the first character after the "type: " prefix, e.g., in "feat: Add
feature") must be capitalized; update the numbered list to include this rule and
mirror the wording from the referenced git-commit-rules.md standard so readers
know to use "feat: Add ..." not "feat: add ...".
---
Nitpick comments:
In @.opencode/command/suggest_commit.md:
- Around line 1-132: The sections in suggest_commit.md and commit.md are
duplicated; extract the shared Steps 1–4 content into a new shared reference
file named commit-message-generation.md and replace the duplicated blocks in
both suggest_commit.md and commit.md with a short include/reference line
pointing to commit-message-generation.md, leaving only the command-specific
differences (presentation/execution rules) in each original file; update any
internal references in suggest_commit.md (e.g., "Step 1: Gather Context", "Step
2: Analyze Changes", "Step 3: Generate Commit Message", "Step 4: Present
Options") to use the shared document so future edits are made in one place.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4b0c9002-7e78-4970-ab2e-3fc913928d53
📒 Files selected for processing (4)
.agent/workflows/commit.md.opencode/command/commit.md.opencode/command/suggest_commit.mdapp/src/main/java/com/lyrics/feelin/presentation/view/note/search/result/NoteSearchResultViewModel.kt
✅ Files skipped from review due to trivial changes (1)
- .agent/workflows/commit.md
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.
Please check if the PR fulfills these requirements
What kind of change does this PR introduce?
What is the current behavior?
곡을 이용한 노트 검색의 상세 화면이 구현되어 있지 않습니다.
What is the new behavior (if this is a feature change)?
곡을 이용한 노트 검색의 상세 화면을 추가합니다.
기능 연동은 로그인/회원가입 연동 이후 진행합니다.
Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
노트 카테고리를 표현하는 Enum을 추가합니다.
ScreenShots (If needed)
Other information:
#32 병합 이후 라우팅 연결을 진행하고 Draft를 해제하겠습니다.
Summary by CodeRabbit
New Features
Documentation