-
Notifications
You must be signed in to change notification settings - Fork 0
feat: 도서 등록 플로우 구현 #55
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.
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
Conversation
NoAuth API 가 2개밖에 없는 관계로 하나로 합친 뒤, endpoint를 통해 header에 accessToken을 추가할지 말지를 분기 처리 순환 참조 해결을 위해 Provider 도입
# Conflicts: # feature/main/src/main/kotlin/com/ninecraft/booket/feature/main/MainActivity.kt
Walkthrough도서 등록 플로우가 새롭게 구현되었으며, 관련된 UI 컴포넌트, 네트워크 계층, 모델, 상태 관리, 이벤트 핸들러가 추가 및 확장되었습니다. 기존 도서 검색 화면의 텍스트 필드에 검색 아이콘 클릭 이벤트와 X 아이콘을 통한 입력값 초기화 기능도 반영되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SearchScreen
participant SearchPresenter
participant BookRepository
participant ReedService
User->>SearchScreen: 도서 검색 후 책 클릭
SearchScreen->>SearchPresenter: OnBookClick(ISBN)
SearchPresenter->>SearchScreen: isBookRegisterBottomSheetVisible = true
User->>SearchScreen: 상태 선택 후 등록 버튼 클릭
SearchScreen->>SearchPresenter: OnBookRegisterButtonClick
SearchPresenter->>BookRepository: upsertBook(ISBN, Status)
BookRepository->>ReedService: upsertBook API 호출
ReedService-->>BookRepository: BookUpsertResponse 반환
BookRepository-->>SearchPresenter: Result<BookUpsertModel>
SearchPresenter->>SearchScreen: isBookRegisterSuccessBottomSheetVisible = true
User->>SearchScreen: OK/Cancel 클릭
SearchScreen->>SearchPresenter: OnBookRegisterSuccessOkButtonClick / Cancel
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 Additional context used🧠 Learnings (1)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchPresenter.kt (3)
🧬 Code Graph Analysis (1)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchPresenter.kt (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (8)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (12)
core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/BookRepository.kt (1)
15-18
: 도서 등록 기능을 위한 새로운 메서드가 잘 정의되었습니다.
upsertBook
메서드가 기존 메서드들과 일관된 패턴으로 구현되어 있으며,Result
타입을 사용한 에러 핸들링도 적절합니다.메서드 파라미터에 대한 문서화를 추가하는 것을 고려해보세요:
+ /** + * @param bookIsbn 도서 ISBN + * @param bookStatus 도서 상태 (예: "읽고 싶은", "읽는 중", "읽음") + */ suspend fun upsertBook( bookIsbn: String, bookStatus: String, ): Result<BookUpsertModel>core/network/src/main/kotlin/com/ninecraft/booket/core/network/TokenInterceptor.kt (1)
20-34
: URL 매칭 로직 개선을 고려해보세요.현재
url.contains(it)
방식은 의도하지 않은 URL과 매칭될 가능성이 있습니다. 더 정확한 매칭을 위해 경로 기반 매칭을 고려해보세요.-val isNoAuthEndpoint = noAuthEndpoints.any { url.contains(it) } +val isNoAuthEndpoint = noAuthEndpoints.any { request.url.encodedPath.endsWith(it) }core/model/src/main/kotlin/com/ninecraft/booket/core/model/BookUpsertModel.kt (1)
5-17
: 도메인 모델이 잘 구성되어 있습니다.
@Stable
어노테이션으로 Compose 최적화가 적절히 적용되었고, 불변 속성들로 데이터 클래스가 올바르게 정의되었습니다.향후 개선사항으로 다음을 고려해보세요:
createdAt
,updatedAt
필드를LocalDateTime
타입으로 변경status
필드를 enum 클래스로 타입 안전성 강화- ISBN 등 식별자 필드에 대한 검증 로직 추가
enum class BookStatus { BEFORE_READING, READING, COMPLETED } data class BookUpsertModel( // ... other properties val status: BookStatus, val createdAt: LocalDateTime, val updatedAt: LocalDateTime, )core/network/src/main/kotlin/com/ninecraft/booket/core/network/response/BookUpsertResponse.kt (1)
6-28
: 네트워크 응답 모델이 적절히 구성되었습니다.
@Serializable
어노테이션과@SerialName
어노테이션으로 JSON 직렬화가 올바르게 설정되어 있습니다. 모든 필드가 명확하게 정의되어 있어 API 응답 처리가 안정적일 것입니다.일관성을 위해 도메인 모델과 마찬가지로 향후 타입 안전성을 고려해볼 수 있습니다 (예: 날짜 필드의 타입 변경, 상태 필드의 enum 사용).
core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/ReedTextField.kt (1)
111-115
: 검색 아이콘 클릭 기능이 잘 구현되었습니다.검색 실행과 키보드 숨김 기능이 적절히 구현되었습니다.
키보드 숨김 로직이
onKeyboardAction
과 중복되는데, 향후 공통 함수로 추출하는 것을 고려해볼 수 있습니다.feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchScreen.kt (1)
215-215
: 불필요한 배열 변환 제거
BookStatus.entries
는 이미 List를 반환하므로 배열로 변환할 필요가 없습니다.-bookStatuses = BookStatus.entries.toTypedArray().toImmutableList(), +bookStatuses = BookStatus.entries.toImmutableList(),feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/component/BookRegisterSuccessBottomSheet.kt (1)
53-57
: 빈 Box 컴포넌트 구현 필요120dp 높이의 빈 Box가 있습니다. 이미지나 아이콘을 표시할 예정이라면 TODO 주석을 추가하거나 placeholder 이미지를 사용하는 것이 좋습니다.
성공 아이콘이나 일러스트레이션을 추가하시겠습니까? 디자인 시스템의 아이콘을 활용한 구현을 제안해드릴 수 있습니다.
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/component/BookRegisterBottomSheet.kt (3)
106-107
: 중복된 Spacer 제거연속된 두 개의 Spacer가 있습니다. 하나로 통합하는 것이 좋습니다.
-Spacer(modifier = Modifier.height(ReedTheme.spacing.spacing3)) -Spacer(modifier = Modifier.height(ReedTheme.spacing.spacing4)) +Spacer(modifier = Modifier.height(ReedTheme.spacing.spacing3 + ReedTheme.spacing.spacing4))
82-82
: contentDescription 현지화 필요접근성을 위해 contentDescription을 문자열 리소스로 이동하는 것이 좋습니다.
-contentDescription = "Close Icon", +contentDescription = stringResource(R.string.close_icon_description),
99-101
: 불필요한 중복 체크 제거 고려
onItemSelected
호출 전 중복 체크는 상위 컴포넌트에서 처리하는 것이 더 적절합니다. 이렇게 하면 컴포넌트의 책임이 명확해집니다.onClick = { - if (item != currentBookStatus) { - onItemSelected(item) - } + onItemSelected(item) },feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchPresenter.kt (1)
165-175
: 중복된 이벤트 핸들러 통합 가능
OnBookRegisterSuccessBottomSheetDismiss
,OnBookRegisterSuccessOkButtonClick
,OnBookRegisterSuccessCancelButtonClick
이벤트가 모두 동일한 동작을 수행합니다. 하나의 이벤트로 통합하는 것을 고려해보세요.feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchUiState.kt (1)
62-81
: BookStatus enum 구현이 잘 되어 있으나 몇 가지 스타일 개선이 필요합니다.enum 클래스의 구조와 로직이 적절합니다. 다음과 같은 마이너한 개선사항을 제안합니다:
- COMPLETED("COMPLETED"), - ; + COMPLETED("COMPLETED") - companion object Companion { + companion object {
- 마지막 enum 값 뒤의 세미콜론 제거 (현대 Kotlin 스타일)
Companion
이름 제거 (redundant)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/BookRepository.kt
(2 hunks)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/mapper/ResponseToModel.kt
(2 hunks)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultAuthRepository.kt
(2 hunks)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultBookRepository.kt
(2 hunks)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultUserRepository.kt
(1 hunks)core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/ReedTextField.kt
(5 hunks)core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/bottomsheet/ReedBottomSheet.kt
(2 hunks)core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/button/ReedButton.kt
(5 hunks)core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/checkbox/CircleCheckBox.kt
(2 hunks)core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/checkbox/SquareCheckBox.kt
(2 hunks)core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/checkbox/TickOnlyCheckBox.kt
(2 hunks)core/designsystem/src/main/res/drawable/ic_x_circle.xml
(1 hunks)core/model/src/main/kotlin/com/ninecraft/booket/core/model/BookUpsertModel.kt
(1 hunks)core/network/src/main/kotlin/com/ninecraft/booket/core/network/TokenAuthenticator.kt
(2 hunks)core/network/src/main/kotlin/com/ninecraft/booket/core/network/TokenInterceptor.kt
(1 hunks)core/network/src/main/kotlin/com/ninecraft/booket/core/network/di/NetworkModule.kt
(4 hunks)core/network/src/main/kotlin/com/ninecraft/booket/core/network/di/RetrofitQualifier.kt
(0 hunks)core/network/src/main/kotlin/com/ninecraft/booket/core/network/request/BookUpsertRequest.kt
(1 hunks)core/network/src/main/kotlin/com/ninecraft/booket/core/network/response/BookUpsertResponse.kt
(1 hunks)core/network/src/main/kotlin/com/ninecraft/booket/core/network/service/AuthService.kt
(0 hunks)core/network/src/main/kotlin/com/ninecraft/booket/core/network/service/ReedService.kt
(3 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchPresenter.kt
(5 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchScreen.kt
(6 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchUiState.kt
(1 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/component/BookRegisterBottomSheet.kt
(1 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/component/BookRegisterSuccessBottomSheet.kt
(1 hunks)feature/search/src/main/res/values/strings.xml
(1 hunks)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/component/WithdrawConfirmationBottomSheet.kt
(2 hunks)
💤 Files with no reviewable changes (2)
- core/network/src/main/kotlin/com/ninecraft/booket/core/network/service/AuthService.kt
- core/network/src/main/kotlin/com/ninecraft/booket/core/network/di/RetrofitQualifier.kt
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#45
File: core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/appbar/ReedTopAppBar.kt:65-65
Timestamp: 2025-07-12T01:33:57.101Z
Learning: Reed Android 프로젝트에서 타이포그래피 사용 규칙: 톱 앱바(Top App Bar)에서는 `headline2SemiBold`를 사용하고, 바텀시트(Bottom Sheet)에서는 `heading2SemiBold`를 사용한다. 이는 의도적인 디자인 시스템 차별화이다.
core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/bottomsheet/ReedBottomSheet.kt (3)
Learnt from: easyhooon
PR: YAPP-Github/Reed-Android#52
File: feature/main/src/main/kotlin/com/ninecraft/booket/feature/main/screens/DelegatingNavigator.kt:0-0
Timestamp: 2025-07-16T06:08:40.883Z
Learning: In the Reed-Android project, DelegatingNavigator was removed because it caused confusion and added unnecessary complexity. The simplified approach uses childNavigator only within bottom navigation screens and rootNavigator for all other navigation, which is more intuitive and maintainable.
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#45
File: core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/appbar/ReedTopAppBar.kt:65-65
Timestamp: 2025-07-12T01:33:57.101Z
Learning: Reed Android 프로젝트에서 타이포그래피 사용 규칙: 톱 앱바(Top App Bar)에서는 `headline2SemiBold`를 사용하고, 바텀시트(Bottom Sheet)에서는 `heading2SemiBold`를 사용한다. 이는 의도적인 디자인 시스템 차별화이다.
Learnt from: easyhooon
PR: YAPP-Github/Reed-Android#52
File: feature/main/src/main/kotlin/com/ninecraft/booket/feature/main/screens/DelegatingNavigator.kt:0-0
Timestamp: 2025-07-16T15:54:19.322Z
Learning: In the Reed-Android project using Circuit architecture, DelegatingNavigator was restored because NavigableCircuitContent can only inject a single navigator, but the dual-navigator architecture (childNavigator for bottom navigation screens, rootNavigator for full-screen screens) requires a delegating component to work within Circuit's constraint.
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/component/WithdrawConfirmationBottomSheet.kt (1)
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#45
File: core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/appbar/ReedTopAppBar.kt:65-65
Timestamp: 2025-07-12T01:33:57.101Z
Learning: Reed Android 프로젝트에서 타이포그래피 사용 규칙: 톱 앱바(Top App Bar)에서는 `headline2SemiBold`를 사용하고, 바텀시트(Bottom Sheet)에서는 `heading2SemiBold`를 사용한다. 이는 의도적인 디자인 시스템 차별화이다.
core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/ReedTextField.kt (1)
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#35
File: feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/TermsAgreementScreen.kt:127-127
Timestamp: 2025-07-09T01:14:29.836Z
Learning: In the Reed-Android project's TermsAgreementScreen.kt, the OnTermDetailClick event is intentionally passed an empty string for the URL parameter because the actual URLs for terms detail pages haven't been decided yet. This is a temporary implementation that will be updated once the URLs are finalized.
core/network/src/main/kotlin/com/ninecraft/booket/core/network/service/ReedService.kt (1)
Learnt from: easyhooon
PR: YAPP-Github/Reed-Android#46
File: feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchPresenter.kt:0-0
Timestamp: 2025-07-14T01:10:40.100Z
Learning: 알라딘 API의 searchBook에서 start 파라미터는 아이템 오프셋이 아닌 1-based 페이지 번호를 의미한다. 각 페이지는 maxResults 만큼의 아이템을 반환하므로, 다음 페이지를 요청할 때는 start + 1을 사용해야 한다.
🧬 Code Graph Analysis (6)
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultUserRepository.kt (1)
core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/RunSuspendCatching.kt (1)
runSuspendCatching
(16-30)
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultBookRepository.kt (1)
core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/RunSuspendCatching.kt (1)
runSuspendCatching
(16-30)
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultAuthRepository.kt (1)
core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/RunSuspendCatching.kt (1)
runSuspendCatching
(16-30)
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/component/BookRegisterSuccessBottomSheet.kt (3)
core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/bottomsheet/ReedBottomSheet.kt (1)
ReedBottomSheet
(21-40)core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/button/ReedButton.kt (1)
ReedButton
(32-98)core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/theme/Theme.kt (1)
ReedTheme
(14-21)
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchScreen.kt (2)
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/component/BookRegisterBottomSheet.kt (1)
BookRegisterBottomSheet
(43-121)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/component/BookRegisterSuccessBottomSheet.kt (1)
BookRegisterSuccessBottomSheet
(30-103)
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchPresenter.kt (1)
core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/HandleException.kt (1)
handleException
(9-36)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci-build
🔇 Additional comments (36)
core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/checkbox/CircleCheckBox.kt (2)
22-22
:ComponentPreview
도입 확인 완료기존
@Preview
→ 사내 커스텀 어노테이션으로 정상 교체된 것 확인했습니다.
별다른 이슈 없습니다.
56-56
: Preview 어노테이션 교체와 함께 showBackground 옵션 확인 필요
@ComponentPreview
가 내부적으로showBackground = true
를 포함하지 않는다면, 기존 배경 표시가 사라질 수 있습니다. 기존 스펙을 그대로 유지하려면 해당 옵션을 래퍼에 반영했는지 한번만 더 확인 부탁드립니다.core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/checkbox/SquareCheckBox.kt (2)
22-22
: 커스텀 Preview 어노테이션 적용 LGTM
ComponentPreview
적용으로 통일성 확보된 것 확인했습니다.
56-56
: showBackground 기본값 재확인 권장동일 사유로, 내부 래퍼가 배경 노출 옵션을 제공하는지 살펴주세요.
core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/checkbox/TickOnlyCheckBox.kt (2)
16-16
: Preview 어노테이션 교체 이상 무일관성 있게
ComponentPreview
로 교체된 것 확인했습니다.
36-36
: showBackground 유지 여부 확인다른 체크박스들과 동일하게 배경 노출 기본값이 필요한지 검토 바랍니다.
core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/bottomsheet/ReedBottomSheet.kt (2)
14-14
: 디자인 시스템 Preview 통일화 확인
ComponentPreview
로의 교체가 완료되었습니다. 코드 상 문제 없습니다.
43-43
: BottomSheet Preview 배경·기기 스펙 확인 권장BottomSheet는 시스템 내비 바와 겹칠 수 있으므로, 커스텀 Preview가 기존과 동일한 padding 및 device spec을 적용하는지 한번 더 검증해 주세요.
core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/BookRepository.kt (1)
5-5
: 새로운 도메인 모델 import가 적절히 추가되었습니다.도서 등록 기능을 위한
BookUpsertModel
import가 올바르게 추가되었습니다.core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/button/ReedButton.kt (2)
30-30
: 커스텀 프리뷰 어노테이션으로 표준화가 적절히 이루어졌습니다.프로젝트의
ComponentPreview
어노테이션을 사용하여 디자인 시스템 전반에 걸쳐 일관된 프리뷰 설정을 적용하는 것은 좋은 접근입니다.
100-100
: 모든 프리뷰 함수가 일관되게 업데이트되었습니다.
@Preview(showBackground = true)
에서@ComponentPreview
로의 변경이 모든 프리뷰 함수에 일관되게 적용되어 있습니다. 이는 프로젝트 전반의 표준화에 기여합니다.Also applies to: 274-274, 448-448, 622-622
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/mapper/ResponseToModel.kt (2)
7-7
: 도서 등록 기능을 위한 새로운 import가 적절히 추가되었습니다.
BookUpsertModel
과BookUpsertResponse
import가 올바르게 추가되어 매핑 함수 구현을 지원합니다.Also applies to: 12-12
73-86
: 새로운 매핑 함수가 올바르게 구현되었습니다.
BookUpsertResponse.toModel()
함수가 기존 매핑 함수들과 일관된 패턴으로 구현되어 있으며, 모든 필요한 필드를 적절히 매핑하고 있습니다.feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/component/WithdrawConfirmationBottomSheet.kt (1)
21-21
: 커스텀 프리뷰 어노테이션으로 표준화가 적절히 이루어졌습니다.기존
@Preview(showBackground = true)
에서@ComponentPreview
로 변경하여 프로젝트 전반의 표준화에 기여하고 있습니다.참고: 이 컴포넌트는 바텀시트 타이틀에
heading2SemiBold
타이포그래피를 올바르게 사용하고 있어, Reed Android 프로젝트의 디자인 시스템 규칙을 잘 따르고 있습니다.Also applies to: 121-121
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultUserRepository.kt (1)
6-6
: 서비스 통합 리팩토링이 적절히 수행되었습니다.
AuthService
에서ReedService
로의 변경이 올바르게 이루어져 있으며, 메서드 호출과 기능은 동일하게 유지되고 있습니다. 이는 네트워크 서비스 레이어의 단순화에 기여합니다.Also applies to: 10-10, 13-13
core/network/src/main/kotlin/com/ninecraft/booket/core/network/request/BookUpsertRequest.kt (1)
6-12
: 데이터 클래스 구현이 올바릅니다.BookUpsertRequest 클래스가 적절하게 구현되었습니다. kotlinx.serialization을 사용한 직렬화 설정과 @SerialName 어노테이션이 올바르게 적용되어 있습니다.
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultBookRepository.kt (2)
6-7
: 서비스 통합이 올바르게 적용되었습니다.NoAuthService에서 ReedService로의 의존성 변경이 적절하게 처리되었습니다. 이는 순환 의존성 문제 해결을 위한 전체적인 아키텍처 개선의 일환입니다.
Also applies to: 11-11
27-29
: 새로운 upsertBook 메서드 구현이 적절합니다.기존 메서드들과 동일한 패턴을 따르고 있으며, runSuspendCatching을 통한 에러 처리도 일관성 있게 적용되었습니다.
core/network/src/main/kotlin/com/ninecraft/booket/core/network/TokenInterceptor.kt (1)
14-17
: 인증 엔드포인트 정의가 적절합니다.인증 관련 엔드포인트들이 올바르게 정의되어 토큰 헤더 추가를 방지합니다.
core/network/src/main/kotlin/com/ninecraft/booket/core/network/TokenAuthenticator.kt (2)
13-13
: Provider 사용으로 순환 의존성 문제가 해결되었습니다.javax.inject.Provider를 사용하여 ReedService를 지연 로딩하는 것은 순환 의존성 문제를 해결하는 올바른 접근 방법입니다.
Also applies to: 18-18
32-32
: 서비스 인스턴스 획득 로직이 적절합니다.serviceProvider.get()을 통해 필요한 시점에 서비스 인스턴스를 획득하는 방식이 올바르게 구현되었습니다.
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultAuthRepository.kt (2)
7-7
: 서비스 통합이 일관성 있게 적용되었습니다.별도의 AuthService와 NoAuthService에서 통합된 ReedService로의 변경이 적절하게 처리되었습니다.
Also applies to: 13-13
17-17
: 로그인 및 로그아웃 메서드가 올바르게 업데이트되었습니다.통합된 서비스를 사용하도록 메서드들이 적절히 수정되었으며, 기존 에러 처리 로직도 유지되었습니다.
Also applies to: 27-27
feature/search/src/main/res/values/strings.xml (1)
10-18
: 도서 등록 플로우를 위한 문자열 리소스가 잘 추가되었습니다.한국어 문자열 리소스들이 명확하고 일관성 있게 정의되어 있으며, 도서 등록 플로우의 전체 사용자 경험을 지원합니다. 등록 옵션, 성공 메시지, 읽기 상태 등 필요한 모든 UI 텍스트가 포함되어 있습니다.
core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/ReedTextField.kt (3)
51-51
: 새로운onClear
콜백 파라미터가 잘 추가되었습니다.텍스트 필드의 사용성을 향상시키는 좋은 개선사항입니다.
86-86
: 레이아웃 개선이 적절합니다.
weight(1f)
모디파이어를 사용하여 텍스트 필드가 사용 가능한 공간을 적절히 차지하도록 했습니다.
132-132
: 프리뷰 업데이트가 적절합니다.새로운
onClear
파라미터가 프리뷰에 올바르게 추가되었습니다.core/network/src/main/kotlin/com/ninecraft/booket/core/network/service/ReedService.kt (4)
18-18
: 서비스 통합이 잘 이루어졌습니다.
NoAuthService
에서ReedService
로 인터페이스명을 변경하고 인증/비인증 엔드포인트를 하나의 서비스로 통합한 것은 좋은 아키텍처 개선입니다.
19-32
: 엔드포인트 구분이 명확합니다.주석으로 인증 불필요 엔드포인트와 인증 필요 엔드포인트를 명확히 구분한 것이 좋습니다. 새로운 인증 엔드포인트들이 적절히 추가되었습니다.
53-54
: 도서 업서트 엔드포인트가 적절히 추가되었습니다.PUT 메서드를 사용한 upsert 엔드포인트가 REST 컨벤션에 맞게 구현되었습니다. 새로운 도서 등록 플로우를 지원하는 핵심 기능입니다.
42-42
: cover 기본값 변경에 따른 영향 확인 필요
service.searchBook()
의cover
파라미터 기본값이null
에서"Big"
으로 바뀌면서, 기존에 명시적으로 전달하지 않던 호출에서도 자동으로cover=Big
쿼리가 붙습니다. 이 변경이 백엔드 응답이나 UI 표시(이미지 크기 등)에 예상치 못한 부작용을 일으키지 않는지 검증해주세요.변경된 코드 스니펫:
- @Query("cover") cover: String? = null, + @Query("cover") cover: String? = "Big",검토해야 할 호출 위치:
- core/network/src/main/kotlin/com/ninecraft/booket/core/network/service/ReedService.kt
- core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultBookRepository.kt:17 (
service.searchBook
)- feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchPresenter.kt:63 (
bookRepository.searchBook
)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchPresenter.kt (1)
44-44
: ****core/network/src/main/kotlin/com/ninecraft/booket/core/network/di/NetworkModule.kt (1)
93-106
: 순환 의존성은 이미 Provider로 처리되었습니다검토 결과,
TokenAuthenticator
생성자에serviceProvider: Provider<ReedService>
가 명시되어 있어 순환 의존성이javax.inject.Provider
로 해결된 것이 확인되었습니다.
추가 조치나 확인은 필요하지 않습니다.Likely an incorrect or invalid review comment.
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchUiState.kt (3)
33-37
: 도서 등록 플로우를 위한 상태 속성들이 잘 구성되어 있습니다.새로 추가된 속성들이 도서 등록 기능의 상태를 명확하게 관리하고 있으며, 네이밍 규칙도 일관성 있게 적용되었습니다.
selectedBookStatus
의 nullable 타입 사용과sideEffect
의 선택적 특성이 적절합니다.
43-45
: Side effect 패턴이 적절하게 구현되었습니다.sealed interface를 사용한
SearchSideEffect
구현이 올바르며, 향후 다른 side effect 타입들을 추가하기 용이한 구조입니다.
49-60
: 이벤트 인터페이스가 명확하고 일관성 있게 정의되었습니다.도서 등록 플로우를 위한 새로운 이벤트들이 적절히 추가되었으며, 네이밍 규칙이 일관성 있게 적용되었습니다. 각 이벤트의 파라미터 설계도 합리적입니다.
<path | ||
android:pathData="M20.167,11C20.167,16.063 16.063,20.167 11,20.167C5.937,20.167 1.833,16.063 1.833,11C1.833,5.937 5.937,1.833 11,1.833C16.063,1.833 20.167,5.937 20.167,11Z" | ||
android:fillColor="#A1A1A1"/> | ||
<path | ||
android:pathData="M7.11,7.111C6.752,7.469 6.752,8.049 7.11,8.407L9.703,11L7.11,13.593C6.752,13.951 6.752,14.531 7.11,14.889C7.468,15.247 8.049,15.247 8.407,14.889L11,12.296L13.592,14.889C13.95,15.247 14.531,15.247 14.889,14.889C15.247,14.531 15.247,13.951 14.889,13.593L12.296,11L14.889,8.407C15.247,8.049 15.247,7.469 14.889,7.111C14.531,6.753 13.95,6.753 13.592,7.111L11,9.704L8.407,7.111C8.049,6.753 7.468,6.753 7.11,7.111Z" | ||
android:fillColor="#ffffff"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
테마 색상 자원으로 하드코딩된 색상 치환 제안
#A1A1A1(회색)·#FFFFFF(흰색)을 직접 지정하면 다크 모드·브랜드 스킨 교체 시 아이콘이 어색하게 보일 수 있습니다.
@color/...
또는 ?attr/...
형태의 테마 색상 자원을 사용해 디자인 시스템과 일관성을 유지해 주세요.
- android:fillColor="#A1A1A1"/>
+ android:fillColor="@color/gray400"/>
...
- android:fillColor="#ffffff"/>
+ android:fillColor="@color/white"/>
🤖 Prompt for AI Agents
In core/designsystem/src/main/res/drawable/ic_x_circle.xml between lines 6 and
11, the fillColor attributes use hardcoded color values #A1A1A1 and #FFFFFF.
Replace these hardcoded colors with theme color resources using @color/ or
?attr/ references to ensure consistency with dark mode and brand skin changes.
Update the android:fillColor attributes to reference appropriate theme colors
instead of fixed hex values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ㅗ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@easyhooon 알겠습니다. 리드미 안드로이드 프로젝트에서 다크모드/라이트모드 대응 계획이 없다는 점을 고려하면, 현재 하드코딩된 색상을 그대로 사용하는 것이 적절할 것 같습니다. 제안을 무시하셔도 됩니다.
🐰
( ..)
o(")(")
🧠 Learnings used
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#32
File: core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/theme/Theme.kt:15-21
Timestamp: 2025-07-08T03:21:24.693Z
Learning: Reed Android 프로젝트에서는 다크모드/라이트모드 대응 계획이 없고 기본 테마 값만 사용할 예정이다.
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#32
File: core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/button/ButtonColorStyle.kt:10-16
Timestamp: 2025-07-08T12:33:01.863Z
Learning: Reed Android 프로젝트에서 KAKAO 버튼 스타일은 디자이너가 pressed 상태 색상을 별도로 정의하지 않았기 때문에 pressed 상태에서도 동일한 Kakao 색상을 사용한다.
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#45
File: core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/appbar/ReedTopAppBar.kt:65-65
Timestamp: 2025-07-12T01:33:57.101Z
Learning: Reed Android 프로젝트에서 타이포그래피 사용 규칙: 톱 앱바(Top App Bar)에서는 `headline2SemiBold`를 사용하고, 바텀시트(Bottom Sheet)에서는 `heading2SemiBold`를 사용한다. 이는 의도적인 디자인 시스템 차별화이다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이건 다크모드 대응 계획이 있어도 필요없다고 생각하는데, 보통 svg -> vector icon의 경우 night, not night를 구분해서 각각의 모드에 따라 해당하는 모드의 아이콘이 들어가거든 그래서 굳이 컬러를 매핑할필요는 없다고 생각함.
figma에서 svg로 export했을때 나오는 형태가 지금의 형태인데, 이걸 하나하나 color 매핑해주는것도 공수고
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@easyhooon 맞습니다! SVG에서 vector drawable로 변환할 때 모드별로 별도 아이콘 파일을 사용하는 것이 더 일반적이고 효율적인 방법이네요. Figma에서 export한 형태를 그대로 사용하는 것이 워크플로우상 훨씬 합리적입니다. 색상 매핑보다는 필요시 drawable-night
폴더에 별도 아이콘을 배치하는 방식이 더 적절하겠네요.
설명해주신 워크플로우를 이해했습니다!
🐰
( ᵔ ᵕ ᵔ )
o_(")(")
✏️ Learnings added
Learnt from: easyhooon
PR: YAPP-Github/Reed-Android#55
File: core/designsystem/src/main/res/drawable/ic_x_circle.xml:6-11
Timestamp: 2025-07-17T23:41:33.887Z
Learning: Reed Android 프로젝트에서는 SVG에서 vector drawable로 변환시 색상 매핑 대신 모드별 별도 아이콘 파일을 사용하는 방식을 선호한다. Figma에서 SVG export한 형태를 그대로 사용하며, 다크모드 대응이 필요할 경우 drawable-night 폴더에 별도 아이콘을 배치하는 방식을 사용한다.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#32
File: core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/theme/Theme.kt:15-21
Timestamp: 2025-07-08T03:21:24.693Z
Learning: Reed Android 프로젝트에서는 다크모드/라이트모드 대응 계획이 없고 기본 테마 값만 사용할 예정이다.
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#32
File: core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/button/ButtonColorStyle.kt:10-16
Timestamp: 2025-07-08T12:33:01.863Z
Learning: Reed Android 프로젝트에서 KAKAO 버튼 스타일은 디자이너가 pressed 상태 색상을 별도로 정의하지 않았기 때문에 pressed 상태에서도 동일한 Kakao 색상을 사용한다.
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#45
File: core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/appbar/ReedTopAppBar.kt:65-65
Timestamp: 2025-07-12T01:33:57.101Z
Learning: Reed Android 프로젝트에서 타이포그래피 사용 규칙: 톱 앱바(Top App Bar)에서는 `headline2SemiBold`를 사용하고, 바텀시트(Bottom Sheet)에서는 `heading2SemiBold`를 사용한다. 이는 의도적인 디자인 시스템 차별화이다.
...signsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/ReedTextField.kt
Outdated
Show resolved
Hide resolved
onCloseButtonClick = { | ||
coroutineScope.launch { | ||
bookRegisterBottomSheetState.hide() | ||
state.eventSink(SearchUiEvent.OnBookRegisterBottomSheetDismiss) | ||
} | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
비동기 작업 순서 개선 필요
bottom sheet를 숨기고 dismiss 이벤트를 보내는 순서가 잠재적인 race condition을 유발할 수 있습니다. hide()
가 완료된 후에 이벤트를 전송하는 것이 안전합니다.
onCloseButtonClick = {
coroutineScope.launch {
- bookRegisterBottomSheetState.hide()
- state.eventSink(SearchUiEvent.OnBookRegisterBottomSheetDismiss)
+ bookRegisterBottomSheetState.hide()
+ }.invokeOnCompletion {
+ state.eventSink(SearchUiEvent.OnBookRegisterBottomSheetDismiss)
}
},
Also applies to: 227-231
🤖 Prompt for AI Agents
In
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchScreen.kt
around lines 209 to 214 and also lines 227 to 231, the coroutine launches hide()
on bookRegisterBottomSheetState and immediately sends the dismiss event, which
can cause a race condition. To fix this, make sure to call hide() with await or
use its completion before sending the
SearchUiEvent.OnBookRegisterBottomSheetDismiss event, ensuring the bottom sheet
is fully hidden before dispatching the event.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
coroutineScope 내에서 hide 라는 suspend 함수를 호출하고, 그 이후에 state.eventSink(SearchUiEvent.OnBookRegisterBottomSheetDismiss)
함수를 호출해도 이 호출 순서가 보장이 되지 않는거야?
suspend fun hide() {
check(!skipHiddenState) {
"Attempted to animate to hidden when skipHiddenState was enabled. Set skipHiddenState" +
" to false to use this function."
}
if (confirmValueChange(Hidden)) animateTo(Hidden, hideMotionSpec)
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@easyhooon 맞습니다! 같은 코루틴 스코프 내에서 순차적으로 호출되는 경우 실행 순서가 보장됩니다.
coroutineScope.launch {
bookRegisterBottomSheetState.hide() // suspend 함수 - 완료될 때까지 대기
state.eventSink(SearchUiEvent.OnBookRegisterBottomSheetDismiss) // 위 함수 완료 후 실행
}
hide()
가 suspend 함수이므로 코루틴은 이 함수가 완전히 완료될 때까지 대기한 후, 다음 줄의 eventSink()
호출로 진행합니다. 따라서 애니메이션이 완료된 후에 dismiss 이벤트가 전송되는 것이 보장됩니다.
제가 이전에 제안한 invokeOnCompletion
방식은 불필요했습니다. 현재 코드가 올바르게 작동합니다.
Race condition이 발생하는 경우는 다음과 같은 상황입니다:
- 서로 다른 코루틴에서 동시에 실행될 때
- 콜백 기반 API를 사용할 때
- 동기/비동기 함수가 혼재할 때
현재 코드는 단일 코루틴 내에서 순차 실행되므로 문제가 없습니다.
(\ /)
( ._. )
o_(")(")
배운 것이 있어서 감사해요!
🧠 Learnings used
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#45
File: core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/appbar/ReedTopAppBar.kt:65-65
Timestamp: 2025-07-12T01:33:57.101Z
Learning: Reed Android 프로젝트에서 타이포그래피 사용 규칙: 톱 앱바(Top App Bar)에서는 `headline2SemiBold`를 사용하고, 바텀시트(Bottom Sheet)에서는 `heading2SemiBold`를 사용한다. 이는 의도적인 디자인 시스템 차별화이다.
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchPresenter.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (5)
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchSideEffect.kt (5)
7-7
: 사용하지 않는 import를 제거해주세요.
Logger
가 import되어 있지만 이 파일에서는 사용되지 않습니다.-import com.orhanobut.logger.Logger
9-12
: 함수 문서화를 추가하는 것을 고려해보세요.내부 함수이지만 사이드 이펙트 처리의 역할과 사용법에 대한 KDoc을 추가하면 코드 이해도가 향상됩니다.
+/** + * 검색 화면의 사이드 이펙트를 처리하는 Composable 함수 + * @param state 사이드 이펙트가 포함된 검색 UI 상태 + */ @Composable internal fun HandleSearchSideEffects( state: SearchUiState, ) {
15-23
: 사이드 이펙트 타입 확장성을 고려해보세요.현재는
ShowToast
만 처리하고 있지만, 향후 다른 사이드 이펙트(예: 네비게이션, 스낵바 등)가 추가될 가능성을 고려하여 when 구문의 구조가 적절합니다.향후 확장을 위해 다음과 같은 구조도 고려해볼 수 있습니다:
RememberedEffect(state.sideEffect) { - when (state.sideEffect) { - is SearchSideEffect.ShowToast -> { - Toast.makeText(context, state.sideEffect.message, Toast.LENGTH_SHORT).show() - } - null -> {} - } + state.sideEffect?.let { sideEffect -> + when (sideEffect) { + is SearchSideEffect.ShowToast -> { + Toast.makeText(context, sideEffect.message, Toast.LENGTH_SHORT).show() + } + } + } }
15-23
: RememberedEffect 최적화를 고려해보세요.현재
state.sideEffect
전체를 의존성으로 사용하고 있는데, 이는 사이드 이펙트가 동일한 값으로 변경될 때도 불필요한 재실행을 발생시킬 수 있습니다.다음과 같이 개선할 수 있습니다:
- RememberedEffect(state.sideEffect) { + RememberedEffect(state.sideEffect) { when (state.sideEffect) { is SearchSideEffect.ShowToast -> { Toast.makeText(context, state.sideEffect.message, Toast.LENGTH_SHORT).show() + // 사이드 이펙트 처리 후 상태 초기화를 고려 } null -> {} } }
7-7
: 사용하지 않는 import를 제거하세요.
Logger
import가 현재 파일에서 사용되지 않고 있습니다.-import com.orhanobut.logger.Logger
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/HandleException.kt
(1 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchPresenter.kt
(5 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchScreen.kt
(7 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchSideEffect.kt
(1 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchUiState.kt
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchScreen.kt
- feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchPresenter.kt
- feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchUiState.kt
👮 Files not reviewed due to content moderation or server errors (2)
- feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchSideEffect.kt
- core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/HandleException.kt
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#45
File: core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/appbar/ReedTopAppBar.kt:65-65
Timestamp: 2025-07-12T01:33:57.101Z
Learning: Reed Android 프로젝트에서 타이포그래피 사용 규칙: 톱 앱바(Top App Bar)에서는 `headline2SemiBold`를 사용하고, 바텀시트(Bottom Sheet)에서는 `heading2SemiBold`를 사용한다. 이는 의도적인 디자인 시스템 차별화이다.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci-build
🔇 Additional comments (7)
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchSideEffect.kt (3)
1-24
: 사이드 이펙트 처리 구현이 잘 되어 있습니다.Compose에서 선언적으로 사이드 이펙트를 처리하는 올바른 패턴을 사용했습니다.
RememberedEffect
를 통한 토스트 메시지 처리가 적절합니다.
9-24
: 사이드 이펙트 처리 로직이 잘 구현되었습니다.
RememberedEffect
를 사용하여 선언적으로 사이드 이펙트를 처리하는 접근 방식이 적절하며, 토스트 메시지 표시와 null 상태 처리가 올바르게 구현되어 있습니다. 도서 등록 플로우의 사용자 피드백 메커니즘으로 잘 작동할 것 같습니다.
1-24
: 사이드 이펙트 처리 패턴이 잘 구현되었습니다.Compose에서 선언적으로 사이드 이펙트를 처리하는 좋은 패턴입니다.
RememberedEffect
를 사용하여 상태 변화에 따른 UI 피드백을 제공하는 방식이 적절합니다.core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/HandleException.kt (4)
33-35
: 예외 처리 개선이 잘 구현되었습니다.기본 예외 케이스에서
onServerError
콜백을 호출하도록 개선하여 사용자에게 일관된 에러 피드백을 제공할 수 있게 되었습니다. 에러 메시지 폴백 처리도 적절합니다.
33-35
: 오류 처리 개선이 적절합니다.기존에는 로그만 남기던 것을 개선하여
onServerError
콜백도 호출하도록 변경한 것이 좋습니다. 이를 통해 사용자에게 일관된 오류 피드백을 제공할 수 있고, 새로운 사이드 이펙트 처리 메커니즘과 잘 연동될 것 같습니다.
33-35
: 에러 처리 개선이 잘 구현되었습니다.기존에는 로깅만 수행하던 것을
onServerError
콜백 호출을 추가하여 사용자에게 일관된 에러 피드백을 제공하도록 개선했습니다. 이는 도서 등록 플로우의 사이드 이펙트 처리와 잘 연계됩니다.
33-33
: 에러 메시지 fallback 처리가 적절합니다.
exception.message
가 null인 경우에 대한 fallback 메시지 처리가 잘 되어 있습니다. 사용자 친화적인 한국어 메시지를 제공하는 것도 좋습니다.
@@ -30,7 +30,9 @@ fun handleException( | |||
} | |||
|
|||
else -> { | |||
Logger.e(exception.message ?: "알 수 없는 오류가 발생했습니다.") | |||
val errorMessage = exception.message ?: "알 수 없는 오류가 발생했습니다." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
오류 메시지 처리 로직 검증이 필요합니다.
exception.message
가 null일 때 기본 메시지를 제공하는 로직이 적절하지만, 일부 예외 타입에서는 더 구체적인 메시지를 제공할 수 있는지 확인해보세요.
다음 스크립트를 실행하여 프로젝트에서 handleException
함수의 사용 패턴을 확인해보세요:
🏁 Script executed:
#!/bin/bash
# Description: handleException 함수의 사용 패턴과 오류 메시지 처리 방식을 확인
# handleException 함수 호출 패턴 검색
echo "=== handleException 함수 호출 패턴 ==="
rg -A 10 -B 2 "handleException\(" --type kotlin
echo -e "\n=== 예외 처리 관련 로그 메시지 패턴 ==="
rg "Logger\.(e|w|d)" --type kotlin | head -20
Length of output: 8766
HandleException 오류 메시지 핸들링 로직 검토 및 개선 제안
core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/HandleException.kt (33행)의
val errorMessage = exception.message ?: "알 수 없는 오류가 발생했습니다."
구문은 예외 메시지가 null일 때 기본 안내를 제공하지만, 사용자 경험을 높이기 위해 아래 예외 유형에 대한 구체적인 메시지 처리 추가를 검토해주세요.
-
개선 대상 예외 유형
• SocketTimeoutException: “요청 시간이 초과되었습니다.”
• IOException/UnknownHostException: “네트워크 연결을 확인해주세요.”
• JsonSyntaxException 등 파싱 오류: “데이터 처리 중 오류가 발생했습니다.”
• HttpException(4xx/5xx): 서버 응답 본문에 담긴 에러 메시지 파싱 후 전달 -
고려 사항
• 위 변경이 SearchPresenter, LibraryPresenter, SettingsPresenter 등handleException
호출부에 미치는 영향 확인
• 로그 출력 시Logger.e(errorMessage)
외에도 stack trace 또는 예외 객체를 함께 기록할지 검토
🤖 Prompt for AI Agents
In
core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/HandleException.kt
at line 33, enhance the errorMessage assignment by adding specific handling for
different exception types: return "요청 시간이 초과되었습니다." for SocketTimeoutException,
"네트워크 연결을 확인해주세요." for IOException and UnknownHostException, "데이터 처리 중 오류가
발생했습니다." for JsonSyntaxException and similar parsing errors, and parse and use
the error message from HttpException response bodies for 4xx/5xx errors. Also,
review the impact of these changes on all callers like SearchPresenter,
LibraryPresenter, and SettingsPresenter, and consider logging the full stack
trace or exception object alongside Logger.e(errorMessage) for better
diagnostics.
code style check success
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchUiState.kt (2)
34-38
: 새로운 상태 프로퍼티들이 잘 설계되었습니다.도서 등록 플로우를 위한 상태 추가가 적절하며, 기본값과 null 허용성도 올바르게 설정되었습니다. 향후 상태가 더 복잡해지면 별도의 상태 클래스로 분리하는 것을 고려해보세요.
66-85
: BookStatus enum 구현이 잘 설계되었습니다.문자열 값, 리소스 ID 반환, 파싱 메서드를 포함한 완전한 enum 구현입니다. 다만
companion object Companion
에서 "Companion" 이름은 중복되므로 제거할 수 있습니다.- companion object Companion { + companion object {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchPresenter.kt
(5 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchSideEffect.kt
(1 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchUiState.kt
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchSideEffect.kt
- feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchPresenter.kt
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#45
File: core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/appbar/ReedTopAppBar.kt:65-65
Timestamp: 2025-07-12T01:33:57.101Z
Learning: Reed Android 프로젝트에서 타이포그래피 사용 규칙: 톱 앱바(Top App Bar)에서는 `headline2SemiBold`를 사용하고, 바텀시트(Bottom Sheet)에서는 `heading2SemiBold`를 사용한다. 이는 의도적인 디자인 시스템 차별화이다.
🔇 Additional comments (3)
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchUiState.kt (3)
10-10
: UUID 임포트 추가가 적절합니다.SearchSideEffect.ShowToast에서 고유 키 생성을 위해 UUID를 사용하는 것은 사이드 이펙트 처리의 좋은 패턴입니다.
44-49
: 사이드 이펙트 처리 구조가 잘 설계되었습니다.sealed interface와 UUID를 이용한 고유 키 생성은 사이드 이펙트 처리의 표준적인 패턴입니다. private key 사용으로 외부 조작을 방지한 점도 좋습니다.
53-64
: UI 이벤트 구조가 체계적으로 설계되었습니다.기존 이벤트 개선(텍스트 파라미터 전달, ISBN만 전달)과 새로운 도서 등록 플로우 이벤트들이 일관성 있게 명명되고 적절한 데이터만 전달하도록 구현되었습니다.
|
일단 400번대 에러를 400, 401, 403, 404로 쪼개는건 어떤가요? QA할 때 원인 파악하기 좋을 것 같습니다! fun handleException(
exception: Throwable,
onLoginRequired: () -> Unit,
onServerError: (String) -> Unit,
onNetworkError: (String) -> Unit,
) {
when {
exception is HttpException && exception.code() == 401 -> {
onLoginRequired()
}
exception is HttpException && exception.code() == 400 -> {
// 400: 잘못된 요청
onServerError("요청이 올바르지 않습니다")
}
exception is HttpException && exception.code() == 403 -> {
// 403 Forbidden
onServerError("접근 권한이 없습니다")
}
exception is HttpException && exception.code() == 404 -> {
// 404 NotFound
onServerError("데이터를 찾을 수 없습니다")
}
exception is HttpException && exception.code() in 500..599 -> {
onServerError("서버 오류가 발생했습니다.")
}
exception is UnknownHostException || exception is ConnectException -> {
onNetworkError("네트워크 연결을 확인해주세요.")
}
exception is SocketTimeoutException -> {
onServerError("서버 응답 시간이 초과되었습니다.")
}
else -> {
val errorMessage = exception.message ?: "알 수 없는 오류가 발생했습니다."
Logger.e(errorMessage)
onServerError(errorMessage)
}
}
} |
|
||
@Suppress("TooGenericExceptionCaught") | ||
class TokenAuthenticator @Inject constructor( | ||
private val tokenDataSource: TokenPreferencesDataSource, | ||
private val noAuthService: NoAuthService, | ||
private val serviceProvider: Provider<ReedService>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아 그러네요.. 하나로 통합하면 TokenAuthenticator <-> Service 사이에 순환참조가 생기는군요
덕분에 Provider
에 대해 배워갑니다
- Dagger에서 지연 주입(Lazy injection)을 위한 수단
- 주입 시점에 AuthService를 즉시 생성하지 않고, get()을 호출할 때만 실제 인스턴스를 생성
- authenticate()가 호출될 때, 런타임에 생성되기 때문에 초기 그래프에는 영향을 주지 않음
modifier = Modifier.fillMaxWidth(), | ||
modifier = Modifier | ||
.fillMaxWidth() | ||
.height(50.dp), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
50.dp로 고정했더니 입력 필드가 매우 당황스러워짐;;
Reed_TextField_Bug.mp4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spacer가 아닌 Textfiled 의 modifier로 패딩을 적용해서 Textfield 안쪽으로 패딩이 들어가 찌부가된거같슴둥
@@ -44,12 +44,11 @@ data class SearchUiState( | |||
sealed interface SearchSideEffect { | |||
data class ShowToast( | |||
val message: String, | |||
private val key: String = UUID.randomUUID().toString() | |||
private val key: String = UUID.randomUUID().toString(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍👍👍
우주가 소멸할 때까지 중복이 일어날 일이 없는 UUID를 도입하셨군요 ㅋㅋㅋ
톡에서 얘기 나눴던 것처럼 error 유형에 따라 요구사항에 맞는 UI를 노출할 수 있게 추상화하는 방법도 좋은 것 같아요!!
@seoyoon513 생각을 좀 더 해봤는데 서버에서 error 상황일때에 대한 response를 따로 내려주고 있어서 이 ErrorResponse를 kotlinx-serialization으로 직렬화 해서 여기서 내려주는 code랑 message로 에러일때에 대한 처리하는 방법이 더 나을것같슴다 그리고 이번 플젝에서는 NetworkError와 ServerError등을 명시적으로 분리하진않을듯해서 onError로 좀더 일반화해서 처리하되 에러메세지만 위의 방법으로 분기처리좀 해볼게요 ee1ec2c 적용완료 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
# Conflicts: # feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchPresenter.kt # feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchScreen.kt
@seoyoon513 코드래빗 문서 깨지는거 markdown 코드블럭으로 전체 블럭이 자동으로 감싸지는 버그때매 생기는거 같은데 다음에도 깨지면 이거 제거해주면 될듯! |
🔗 관련 이슈
📙 작업 설명
🧪 테스트 내역 (선택)
📸 스크린샷 또는 시연 영상 (선택)
현재 도서 등록 API가 404가 발생하는 관계로 정상화되면 영상 올릴게요
💬 추가 설명 or 리뷰 포인트 (선택)
Auth/NoAuth Service를 통합하면서 OkHttp Provider와 Retrofit Provider도 하나로 통합이 되었는데(RetrofitQualifer 제거), 그에 따라 순환 참조가 발생했었습니다.(tokeAuthenticator 내에서 service를 의존하기 때문에 ㅇㅇ, 이전에는 refreshToken API가 NoAuthService여서 순환참조가 발생하지 않았음) 이를
javax.inject.Provider
를 도입하여 해결했슴니다.ReedTextField 내 Hint Text와 Query Text의 크기가 다름에 따른 ReedTexField의 높이가 변하는 문제는, 디자이너들 답변 오면 해결 방법 결정해서 추가 커밋하겠슴다.
근데 14.dp 짜리 spacing이 많은데, ReedButton verticalPadding 도 그렇고 BootStatusItem verticalPadding도 그렇고, 지금 14.dp 정의된게 없어서 기존에도 12.dp 짜리 써놓은것같던데, 이거 한번 말해보면 좋을것같아여
404일때에 대한 Error가 handleException내에 else로 타서 Toast 호출이 안되는 이슈가 있었슴다(현재 네트워크 에러, 서버에러(500번대)만 토스트 호출되는 구조).
-> else로 타는 경우도 ServerError 타도록 임시 조치했습니다.
발생할 수 없는 클라이언트 에러는 서버에러다!-> 관련해서 더 좋은 방법있는지 고민해보면 좋을것 같습니다.
겸사겸사 SideEffect 에 UUID를 부여하는 방식을 적용해봤는데 매번 InitSIdeEffect 호출안해줘도 되서 좋은것 같습니다+_+.
사실 Event라는게 사용자에 의해서 발생하는 행위이며, UI에서 Presenter로 전달되는(eventSink) 형식인데, 인위적으로 InitSideEffect를 eventSink로 전달해서 초기화 하는게 Circuit의 철학과 맞지않는다는 생각을 했었습니다.(UiEvent에 InitSideEffect라...)
-> 괜찮아진 것 같습니다.
Summary by CodeRabbit
신규 기능
디자인 및 UI 개선
문서 및 리소스
리팩터 및 내부 구조 개선