Conversation
|
Looking for one thing? Review this PR in Change Stack to search files, summaries, diffs, and code without losing your place. Warning Review limit reached
More reviews will be available in 27 minutes and 43 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Walkthrough이 PR은 일반 탐색 화면에 서버 기반 최근 검색어 조회, 단건 삭제, 전체 삭제 기능을 구현합니다. 데이터 계층(DTO, Entity, API, Repository, 매퍼)부터 UI 계층(Model, Adapter, ViewModel, Activity)까지 일관되게 최근 검색 기능을 적층하고, 검색 기록 API 응답을 UI에 표시합니다. Changes최근 검색어 기능
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
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)
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}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/main/res/layout/item_normal_explore_recent_search.xml (1)
39-39: ⚡ Quick win접근성을 위한 contentDescription 추가를 권장합니다.
삭제 버튼에
contentDescription="@null"이 설정되어 있어 스크린 리더 사용자가 버튼의 기능을 알 수 없습니다. "최근 검색어 삭제"와 같은 설명 텍스트를 추가하는 것이 좋습니다.♿ 접근성 개선 제안
strings.xml에 새 문자열 추가:
<string name="normal_explore_recent_search_delete_content_description">최근 검색어 삭제</string>item_normal_explore_recent_search.xml 수정:
<ImageView android:id="@+id/iv_normal_explore_recent_search_delete" android:layout_width="16dp" android:layout_height="16dp" android:layout_marginStart="6dp" - android:contentDescription="`@null`" + android:contentDescription="`@string/normal_explore_recent_search_delete_content_description`" android:padding="4dp" android:src="`@drawable/ic_normal_explore_recent_search_delete`" />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/res/layout/item_normal_explore_recent_search.xml` at line 39, Add an accessible contentDescription for the delete button by creating a new string resource named normal_explore_recent_search_delete_content_description in strings.xml (value: "최근 검색어 삭제") and then replace android:contentDescription="`@null`" in item_normal_explore_recent_search.xml with android:contentDescription="`@string/normal_explore_recent_search_delete_content_description`" so screen readers announce the button purpose.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@app/src/main/java/com/into/websoso/ui/normalExplore/NormalExploreViewModel.kt`:
- Around line 156-157: The ktlint violation is due to method chaining formatting
on _recentSearches update; ensure each chained call is on its own line by moving
the .filterNot call to a new line after invoking orEmpty() so the chain in the
assignment to _recentSearches.value follows ktlint's one-call-per-line rule
(reference: _recentSearches, orEmpty(), filterNot, recentSearchId).
---
Nitpick comments:
In `@app/src/main/res/layout/item_normal_explore_recent_search.xml`:
- Line 39: Add an accessible contentDescription for the delete button by
creating a new string resource named
normal_explore_recent_search_delete_content_description in strings.xml (value:
"최근 검색어 삭제") and then replace android:contentDescription="`@null`" in
item_normal_explore_recent_search.xml with
android:contentDescription="`@string/normal_explore_recent_search_delete_content_description`"
so screen readers announce the button purpose.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 35a8b386-967a-4c99-9d7e-4b325999b2ef
📒 Files selected for processing (15)
app/src/main/java/com/into/websoso/data/mapper/NovelMapper.ktapp/src/main/java/com/into/websoso/data/model/RecentSearchesEntity.ktapp/src/main/java/com/into/websoso/data/remote/api/NovelApi.ktapp/src/main/java/com/into/websoso/data/remote/response/RecentSearchesResponseDto.ktapp/src/main/java/com/into/websoso/data/repository/NovelRepository.ktapp/src/main/java/com/into/websoso/ui/mapper/NovelMapper.ktapp/src/main/java/com/into/websoso/ui/normalExplore/NormalExploreActivity.ktapp/src/main/java/com/into/websoso/ui/normalExplore/NormalExploreViewModel.ktapp/src/main/java/com/into/websoso/ui/normalExplore/adapter/RecentSearchAdapter.ktapp/src/main/java/com/into/websoso/ui/normalExplore/model/NormalExploreModel.ktapp/src/main/res/drawable/bg_normal_explore_recent_search_chip.xmlapp/src/main/res/drawable/ic_normal_explore_recent_search_delete.xmlapp/src/main/res/layout/activity_normal_explore.xmlapp/src/main/res/layout/item_normal_explore_recent_search.xmlcore/resource/src/main/res/values/strings.xml
Sadturtleman
left a comment
Sadturtleman
left a comment
There was a problem hiding this comment.
수고하셨습니다!
다만 repo가 class로 되있는데 interface로 바꾸는건 어떨까 한번 여쭤봅니다!
m6z1
left a comment
m6z1
left a comment
There was a problem hiding this comment.
@Sadturtleman 오 repo 를 추상화 했을 때의 장점이 무엇일까요 ?? !! ㅎㅎ
이제 하루가 아키텍처를 바꾸셔도 됩니다
There was a problem hiding this comment.
수고하셨습니다 뭉치! 👍🏻
repository를 추상화 하는 의견 좋네요 ! 저도 그에 대한 생각을 남기자면,,
interface 분리의 장점은 이해하지만 현재 NovelRepository는 구현체가 하나뿐이고 별도 대체 구현이나 명확한 분리가 필요한 상황은 아닌 것 같아 보입니다...! 지금 분리하면 테스트성,확장성 이점보다는 Repository/Impl 구조와 Hilt binding이 추가되는 비용이 더 커 보여서요...!
추후 local/remote 분리나 fake 구현이 필요해지는 시점에 분리해도 괜찮지 않을까요?
이에 대한 @m6z1 @Sadturtleman 뭉치와 하루의 의견이 궁금하네요 ㅎ ㅎ
물론 그래도 되지만 이미 feed와 같이 리팩토링을 하면서 hilt binding을 추가하기도 했고, 아키텍쳐의 일관성이 깨지는 것 같아 바꾸는게 좋지 않을까? 라는 의견이었습니다! |
3 participants
📌𝘐𝘴𝘴𝘶𝘦𝘴
📎𝘞𝘰𝘳𝘬 𝘋𝘦𝘴𝘤𝘳𝘪𝘱𝘵𝘪𝘰𝘯
📷𝘚𝘤𝘳𝘦𝘦𝘯𝘴𝘩𝘰𝘵
default.mp4
💬𝘛𝘰 𝘙𝘦𝘷𝘪𝘦𝘸𝘦𝘳𝘴
DELETE /novels/recent-searches/{recentSearchId},DELETE /novels/recent-searches기준으로 구현했습니다.Summary by CodeRabbit
새로운 기능