Conversation
|
Caution Review failedThe pull request is closed. Walkthrough등록 도서 삭제 API(네트워크/리포지토리/구현)를 추가하고, 상세 화면에 상세 메뉴 바텀시트와 도서 삭제 확인 다이얼로그 및 관련 상태/이벤트를 도입했다. 부수적으로 최근 검색 삭제 명칭 변경, Compose @immutable 주석 및 IME 인셋 처리 개선이 포함됨. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as 사용자
participant UI as BookDetailUi
participant P as BookDetailPresenter
participant R as BookRepository
participant S as ReedService
participant SV as Server
U->>UI: 우측 메뉴 클릭
UI->>P: OnDetailMenuClick
P-->>UI: show bottom sheet
U->>UI: 바텀시트에서 "도서 삭제" 선택
UI->>P: OnDeleteBookClick
P-->>UI: show delete confirmation dialog
U->>UI: 삭제 확인 (OnDeleteBook)
UI->>P: OnDeleteBook
P->>R: deleteBook(userBookId)
R->>S: DELETE /api/v1/books/my-library/{userBookId}
S->>SV: 요청 전달
SV-->>S: 응답 (200 / 401 / error)
alt 성공
S-->>R: Unit
R-->>P: Result.success
P-->>UI: dismiss dialog, navigator.pop()
else 로그인 필요 (401)
S-->>R: 401
R-->>P: Result.failure(LoginRequired)
P->>UI: navigate(Login)
else 기타 오류
S-->>R: Error
R-->>P: Result.failure
P->>UI: showToast + handleException
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (2)
✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 (15)
feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/step/ImpressionStep.kt (1)
73-75: 선택: 컨텐츠 전체 이동이 아닌 CTA만 이동시키고 싶다면현 구조는 키보드 등장 시 Box 전체가
imePadding으로 밀립니다. 만약 입력 중에도 본문 스크롤 영역의 레이아웃 이동을 최소화하고, 하단 CTA만 올리고 싶다면imePadding()을 CTA(Button)의 modifier로 이동하는 방법도 있습니다. UX 취향 차이이므로 필수 사항은 아닙니다.예시 변경(참고용):
- Box( - modifier = modifier - .fillMaxSize() - .background(White) - .imePadding(), - ) { + Box( + modifier = modifier + .fillMaxSize() + .background(White), + ) { ... ReedButton( onClick = { state.eventSink(RecordRegisterUiEvent.OnNextButtonClick) }, colorStyle = ReedButtonColorStyle.PRIMARY, sizeStyle = largeButtonStyle, modifier = Modifier .fillMaxWidth() .align(Alignment.BottomCenter) .padding(horizontal = ReedTheme.spacing.spacing5) - .padding(bottom = ReedTheme.spacing.spacing4), + .padding(bottom = ReedTheme.spacing.spacing4) + .imePadding(), enabled = state.isNextButtonEnabled, text = stringResource(R.string.record_next_button), multipleEventsCutterEnabled = state.currentStep == RecordStep.IMPRESSION, )Also applies to: 130-135
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/library/LibrarySearchUiState.kt (1)
31-36: SideEffect에도 @immutable 부여해 일관성/최적화 정렬 제안동일 모듈의 HomeUiState처럼 SideEffect도 @immutable로 명시하면 타입 안정성과 리컴포지션 힌팅이 일관됩니다.
아래와 같이 주석 없이 바로 애노테이션만 추가해도 충분합니다:
-sealed interface LibrarySearchSideEffect { +@Immutable +sealed interface LibrarySearchSideEffect { data class ShowToast( val message: String, private val key: String = UUID.randomUUID().toString(), ) : LibrarySearchSideEffect }feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/record/RecordDetailUiState.kt (1)
41-43: 이벤트 네이밍 명확화 제안: OnDelete → OnConfirmDelete 또는 OnDeleteRecord이미
OnDeleteRecordClick가 있어OnDelete는 의미가 모호합니다. 확인(Confirm) 액션을 표현하려면OnConfirmDelete로, 일관성을 맞추려면OnDeleteRecord로 변경을 고려해 주세요.예시:
- data object OnDelete : RecordDetailUiEvent + data object OnConfirmDelete : RecordDetailUiEventfeature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchUiState.kt (1)
15-21: UiState에 @immutable 추가는 적절합니다. 단, Error(exception: Throwable)의 안정성은 한 번 점검 권장sealed UiState가 모두 불변 객체이므로 @immutable 힌트는 합당합니다. 다만 Error가 Throwable을 보유하고 있어 Compose의 엄밀한 불변성 분석 기준으로는 완전한 불변 타입으로 간주되지 않을 수 있습니다. 실제 문제로 이어질 가능성은 낮지만, 엄격하게 가려면 다음 중 하나를 고려해볼 수 있습니다:
- 예외 전체 대신 message/code 등 불변 값만 보유하도록 축약
- 또는 @stable 사용 검토
core/network/src/main/kotlin/com/ninecraft/booket/core/network/service/ReedService.kt (1)
83-87: DELETE 엔드포인트 추가 적절. 상태코드 분기 필요 시 Response 반환 고려현재 Unit 반환으로도 예외 기반 에러 처리(runSuspendCatching)와 잘 맞습니다. 다만 상태코드별 분기(예: 404/409 처리)나 헤더 확인이 필요하다면 Response으로 받아서 해석하는 선택지도 있습니다. 선택 사항입니다.
가능한 대안 시그니처 예시:
- @DELETE("api/v1/books/my-library/{userBookId}") - suspend fun deleteBook( - @Path("userBookId") userBookId: String, - ) + @DELETE("api/v1/books/my-library/{userBookId}") + suspend fun deleteBook( + @Path("userBookId") userBookId: String, + ): retrofit2.Response<Unit>core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/BookRepository.kt (1)
46-48: 레포지토리 API에 deleteBook 추가 — 일관된 Result 계약 유지, 좋습니다네트워크 계층 동작과 맞물린 Result 계약이 기존 패턴과 일관적입니다. 가독성을 위해 간단한 KDoc로 성공/실패 시 의미를 명시하면 활용 측에서 더 안전합니다(예: 404 시 실패 반환 여부, 인증 오류 처리 기대치 등).
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultBookRepository.kt (1)
68-70: 구현 일관성 양호. 후속 동기화 전략(캐시/목록 무효화) 고려 여지runSuspendCatching으로 예외를 Result로 감싸는 기존 패턴과 완전히 일치합니다. 한 가지 운영 관점 제안으로, 삭제 성공 이후 라이브러리 목록 캐싱/프리패치 전략이 있거나 화면 단에서 부분 갱신이 필요하다면 레포지토리 레벨에서 무효화 훅을 두는 것도 고려해볼 만합니다. 현재 구조에선 프레젠터/화면에서 재요청으로 커버해도 무방합니다.
feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/book/BookDetailUiState.kt (2)
16-22: @immutable 어노테이션 적용: 개념적으로 타당하나 Error 서브타입의 불변성 확인 권장UiState에 @immutable을 부여한 방향은 좋습니다. 다만 UiState.Error가 Throwable을 들고 있어 런타임 안정성 추론과의 미세한 괴리가 있을 수 있습니다. 현재 구조에서 문제를 일으킬 가능성은 낮지만, UiState의 불변성 계약을 유지한다는 관점에서 추후 Throwable 교체 여부(예: 메시지 스냅샷 저장) 검토를 권장합니다.
77-83: 이벤트 네이밍 일관성: OnDeleteDialogDismiss → OnBookDeleteDialogDismiss 권장Record 삭제 관련 이벤트(OnRecordDeleteDialogDismiss)와 달리, 도서 삭제의 다이얼로그 dismiss는 OnDeleteDialogDismiss로 범용 명칭이라 혼동 여지가 있습니다. OnBookDeleteDialogDismiss로 명확화하면 가독성이 올라갑니다.
적용 예(이 파일 내):
- data object OnDeleteDialogDismiss : BookDetailUiEvent + data object OnBookDeleteDialogDismiss : BookDetailUiEvent다른 파일의 참조도 함께 변경 필요합니다:
- BookDetailUi.kt: state.eventSink(BookDetailUiEvent.OnBookDeleteDialogDismiss)
- BookDetailPresenter.kt: when 분기에서 OnBookDeleteDialogDismiss 처리
feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/book/BookDetailUi.kt (2)
165-175: DetailMenuBottomSheet에 전용 SheetState 사용 및 닫힘 애니메이션 일관화 제안현재 recordMenuBottomSheetState를 재사용하고 있어 향후 두 시트가 확장될 경우 상태 간섭 가능성이 있습니다. 다른 바텀시트처럼 전용 state를 두는 편이 안전합니다. 또한 onDeleteBookClick 시 presenter에서 가시성 플래그만 내리는 방식이라, UI 레벨에서 hide 애니메이션을 보장하지 못합니다. 다른 시트(onEditRecordClick)처럼 hide 후 이벤트 발행을 권장합니다.
선택 영역 적용 diff(전용 state 사용 및 hide 애니메이션):
- DetailMenuBottomSheet( + DetailMenuBottomSheet( onDismissRequest = { state.eventSink(BookDetailUiEvent.OnDetailMenuBottomSheetDismiss) }, - sheetState = recordMenuBottomSheetState, + sheetState = detailMenuBottomSheetState, onDeleteBookClick = { - state.eventSink(BookDetailUiEvent.OnDeleteBookClick) + coroutineScope.launch { + detailMenuBottomSheetState.hide() + state.eventSink(BookDetailUiEvent.OnDeleteBookClick) + } }, )선택 영역 밖에 추가해야 할 코드(상단 remember 구간에 전용 state 선언):
// 추가 필요 val detailMenuBottomSheetState = rememberModalBottomSheetState()
228-231: 아이콘 설명 문자열 하드코딩 대신 리소스 사용 권장"More Vertical Icon"은 사용자 노출 가능성이 있는 접근성 설명입니다. 문자열 리소스로 분리해 현지화/일관성을 확보하세요.
예시:
endIconDescription = stringResource(R.string.more_vertical_icon_desc)strings.xml:
<string name="more_vertical_icon_desc">더보기</string>feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/book/BookDetailPresenter.kt (3)
217-238: 도서 삭제 로직 전반적으로 적절Repository → handleException → Login 리다이렉트 플로우가 기존 패턴과 일치합니다. 성공 시 navigator.pop()은 사용자 흐름에도 자연스럽습니다. 필요하면 성공 토스트(예: “도서를 삭제했어요.”)를 SideEffect로 추가하는 방안도 고려할 수 있습니다.
325-335: 레코드 삭제 후 총 개수 동기화 권장삭제 후 readingRecordsTotalCount를 감소시키지 않아 헤더(totalCount)와 목록 개수가 불일치할 수 있습니다. UI 일관성을 위해 카운트 감소를 권장합니다.
적용 diff:
is BookDetailUiEvent.OnDeleteRecord -> { isRecordDeleteDialogVisible = false deleteRecord( readingRecordId = selectedRecordInfo.id, onSuccess = { readingRecords = readingRecords .filterNot { it.id == selectedRecordInfo.id } .toPersistentList() + if (readingRecordsTotalCount > 0) { + readingRecordsTotalCount -= 1 + } }, ) }
354-356: 이벤트 네이밍 일관성: OnDeleteDialogDismiss → OnBookDeleteDialogDismiss 제안UiState와 동일하게 구체적인 이벤트명으로 변경하면 가독성이 향상됩니다.
적용 diff:
- is BookDetailUiEvent.OnDeleteDialogDismiss -> { + is BookDetailUiEvent.OnBookDeleteDialogDismiss -> { isBookDeleteDialogVisible = false }연쇄 변경:
- BookDetailUiState.kt: 이벤트 선언명 변경
- BookDetailUi.kt: onDismissRequest 쪽 이벤트 발행명 변경
feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/book/component/DetailMenuBottomSheet.kt (1)
67-90: 접근성(a11y) 개선 제안: Row에 역할/라벨 부여noRippleClickable을 사용하면 터치 피드백은 깔끔하지만 접근성 세만틱(role/label)이 부족할 수 있습니다. Row에 semantics로 Role.Button을 주고, label을 제공하면 TalkBack 사용자 경험이 개선됩니다. 아이콘의 contentDescription 대신 텍스트 라벨이 존재하므로 아이콘 설명은 null로 두어 중복 낭독을 피하는 것도 방법입니다.
적용 예:
Row( - modifier = modifier + modifier = modifier .fillMaxWidth() .noRippleClickable { onClick() } + .semantics { + this.role = Role.Button + this.contentDescription = label + } .padding( vertical = ReedTheme.spacing.spacing5, horizontal = ReedTheme.spacing.spacing6, ), ) { Icon( - imageVector = ImageVector.vectorResource(iconResId), - contentDescription = iconDescription, + imageVector = ImageVector.vectorResource(iconResId), + contentDescription = null, // 텍스트 라벨로 대체 tint = color, )필요 import:
import androidx.compose.ui.semantics.Role import androidx.compose.ui.semantics.semantics import androidx.compose.ui.semantics.contentDescription
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (18)
core/common/src/main/kotlin/com/ninecraft/booket/core/common/constants/ErrorDialogSpec.kt(1 hunks)core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/BookRepository.kt(1 hunks)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultBookRepository.kt(1 hunks)core/network/src/main/kotlin/com/ninecraft/booket/core/network/TokenInterceptor.kt(0 hunks)core/network/src/main/kotlin/com/ninecraft/booket/core/network/service/ReedService.kt(1 hunks)core/ui/src/main/kotlin/com/ninecraft/booket/core/ui/component/InfinityLazyColumn.kt(1 hunks)feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/book/BookDetailPresenter.kt(5 hunks)feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/book/BookDetailUi.kt(3 hunks)feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/book/BookDetailUiState.kt(3 hunks)feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/book/component/DetailMenuBottomSheet.kt(1 hunks)feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/record/RecordDetailUiState.kt(1 hunks)feature/detail/src/main/res/values/strings.xml(1 hunks)feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUiState.kt(1 hunks)feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryUiState.kt(1 hunks)feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/register/RecordRegisterUi.kt(2 hunks)feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/step/ImpressionStep.kt(2 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchUiState.kt(1 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/library/LibrarySearchUiState.kt(2 hunks)
💤 Files with no reviewable changes (1)
- core/network/src/main/kotlin/com/ninecraft/booket/core/network/TokenInterceptor.kt
🧰 Additional context used
🧬 Code Graph Analysis (4)
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)
feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/book/component/DetailMenuBottomSheet.kt (1)
core/ui/src/main/kotlin/com/ninecraft/booket/core/ui/component/ReedBottomSheet.kt (1)
ReedBottomSheet(21-40)
feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/book/BookDetailPresenter.kt (4)
core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/HandleException.kt (1)
handleException(19-46)core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/BookRepository.kt (1)
deleteBook(47-47)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultBookRepository.kt (1)
deleteBook(68-70)core/network/src/main/kotlin/com/ninecraft/booket/core/network/service/ReedService.kt (1)
deleteBook(83-86)
feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/book/BookDetailUi.kt (3)
feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/book/component/DetailMenuBottomSheet.kt (1)
DetailMenuBottomSheet(28-56)core/ui/src/main/kotlin/com/ninecraft/booket/core/ui/component/ReedDialog.kt (1)
ReedDialog(25-114)core/ui/src/main/kotlin/com/ninecraft/booket/core/ui/component/ReedTopAppBar.kt (1)
ReedTopAppBar(28-84)
⏰ 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 (14)
feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/register/RecordRegisterUi.kt (1)
47-51: Scaffold에서 IME 인셋 제외 패턴 적절합니다
contentWindowInsets = ScaffoldDefaults.contentWindowInsets.exclude(WindowInsets.ime)로 IME 인셋을 상위에서 제거하고, 하위에서 필요한 화면만imePadding()으로 처리하는 구조가 중복 인셋을 깔끔하게 방지합니다. 현재 ImpressionStep에서imePadding()을 적용하고 있으므로 의도대로 동작할 것으로 보입니다.feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/step/ImpressionStep.kt (1)
10-10: imePadding 추가로 중복 인셋 없이 안전하게 키보드 대응됨상위 Scaffold에서 IME 인셋을 제외한 뒤, 본문 컨테이너(Box)에
imePadding()을 적용한 구성은 중복 인셋 없이 하단 CTA가 키보드 위로 자연스럽게 올라오게 합니다. 현재 구조와 잘 맞습니다.Also applies to: 73-75
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/library/LibrarySearchUiState.kt (1)
13-19: UiState에 @immutable 적용: LGTMCompose에서 불변 모델로 명시해 리컴포지션 안정성과 퍼포먼스에 도움이 됩니다. 변경 취지에 잘 맞습니다.
core/ui/src/main/kotlin/com/ninecraft/booket/core/ui/component/InfinityLazyColumn.kt (1)
75-88: 하드코딩(6) 제거하고 상수(LIMIT_COUNT) 참조로 단일 소스화: 좋습니다기본 파라미터를 상수로 통일해 유지보수성과 가독성이 개선되었습니다. 동작 변화 없이 의도만 명확해졌습니다.
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUiState.kt (1)
12-18: UiState @immutable 적용: 문제 없습니다다른 화면과의 일관성 유지에도 도움이 됩니다. SideEffect에도 이미 @immutable이 적용되어 있어 일관성 측면에서도 좋습니다.
feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/record/RecordDetailUiState.kt (1)
9-15: UiState @immutable 적용: LGTM레코드 상세 상태 모델에 대해서도 불변성 힌트를 제공해 Compose 추론이 개선됩니다.
core/common/src/main/kotlin/com/ninecraft/booket/core/common/constants/ErrorDialogSpec.kt (1)
3-9: buttonLabelResId에 @stringres 추가: 타입 안정성 강화, LGTM리소스 타입 실수를 컴파일 타임에 방지할 수 있어 안전성이 향상됩니다. 사용처에서도 의도가 명확해집니다.
feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryUiState.kt (1)
13-19: LGTM — UiState에 @immutable 추가로 재구성 힌트 명확화불변 상태 객체임을 명시해 Compose 재구성 최적화에 도움 됩니다. 변경 사항 문제 없습니다.
feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/book/BookDetailUiState.kt (1)
42-43: 상태 플래그 추가 LGTMisDetailMenuBottomSheetVisible, isBookDeleteDialogVisible 추가로 UI 흐름이 명확해졌습니다. 프리젠터와 Ui의 연결도 일관적입니다.
feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/book/BookDetailUi.kt (2)
45-45: TopAppBar 교체 및 메뉴 아이콘 핸들링 LGTMReedTopAppBar 도입과 좌/우 아이콘 콜백 연결이 명확합니다. 뒤로가기와 메뉴 오픈 이벤트가 일관적으로 처리됩니다.
Also applies to: 50-50, 221-232
151-157: 레코드 삭제 확인 이벤트 연결 적절확인 버튼에서 OnDeleteRecord를 발행하는 흐름이 프리젠터 처리와 일치합니다.
feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/book/BookDetailPresenter.kt (2)
81-83: UI 가시성 상태 추가 LGTMDetail 메뉴/도서 삭제 다이얼로그 가시성 상태 추가가 UI 흐름 분리에 도움이 됩니다.
358-366: 도서 삭제 confirm 핸들링 LGTM다이얼로그 비가시화 → 삭제 → pop의 순서가 깔끔합니다.
feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/book/component/DetailMenuBottomSheet.kt (1)
28-56: BottomSheet 구성 LGTM단일 액션(도서 삭제)으로 명확하며, 색상/아이콘/레이블 사용도 적절합니다.
| if (state.isBookDeleteDialogVisible) { | ||
| ReedDialog( | ||
| title = stringResource(R.string.record_delete_dialog_title), | ||
| confirmButtonText = stringResource(R.string.record_delete_dialog_delete), | ||
| onConfirmRequest = { | ||
| state.eventSink(BookDetailUiEvent.OnDeleteBook) | ||
| }, | ||
| dismissButtonText = stringResource(R.string.record_delete_dialog_cancel), | ||
| onDismissRequest = { | ||
| state.eventSink(BookDetailUiEvent.OnDeleteDialogDismiss) | ||
| }, | ||
| ) | ||
| } |
There was a problem hiding this comment.
도서 삭제 다이얼로그 문구 수정 필요: ‘기록’ → ‘도서’
현재 도서 삭제 다이얼로그가 기록 삭제 문자열을 사용하고 있어 사용자에게 혼동을 줄 수 있습니다. strings.xml에 제안한 book_delete_dialog_title을 사용하도록 변경해 주세요. 또한 앞선 코멘트대로 이벤트 명칭을 OnBookDeleteDialogDismiss로 바꿀 경우, 참조도 함께 조정합니다.
적용 diff:
- ReedDialog(
- title = stringResource(R.string.record_delete_dialog_title),
- confirmButtonText = stringResource(R.string.record_delete_dialog_delete),
+ ReedDialog(
+ title = stringResource(R.string.book_delete_dialog_title),
+ confirmButtonText = stringResource(R.string.record_delete_dialog_delete),
onConfirmRequest = {
state.eventSink(BookDetailUiEvent.OnDeleteBook)
},
dismissButtonText = stringResource(R.string.record_delete_dialog_cancel),
onDismissRequest = {
- state.eventSink(BookDetailUiEvent.OnDeleteDialogDismiss)
+ state.eventSink(BookDetailUiEvent.OnBookDeleteDialogDismiss)
},
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (state.isBookDeleteDialogVisible) { | |
| ReedDialog( | |
| title = stringResource(R.string.record_delete_dialog_title), | |
| confirmButtonText = stringResource(R.string.record_delete_dialog_delete), | |
| onConfirmRequest = { | |
| state.eventSink(BookDetailUiEvent.OnDeleteBook) | |
| }, | |
| dismissButtonText = stringResource(R.string.record_delete_dialog_cancel), | |
| onDismissRequest = { | |
| state.eventSink(BookDetailUiEvent.OnDeleteDialogDismiss) | |
| }, | |
| ) | |
| } | |
| if (state.isBookDeleteDialogVisible) { | |
| ReedDialog( | |
| title = stringResource(R.string.book_delete_dialog_title), | |
| confirmButtonText = stringResource(R.string.record_delete_dialog_delete), | |
| onConfirmRequest = { | |
| state.eventSink(BookDetailUiEvent.OnDeleteBook) | |
| }, | |
| dismissButtonText = stringResource(R.string.record_delete_dialog_cancel), | |
| onDismissRequest = { | |
| state.eventSink(BookDetailUiEvent.OnBookDeleteDialogDismiss) | |
| }, | |
| ) | |
| } |
🤖 Prompt for AI Agents
In
feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/book/BookDetailUi.kt
around lines 177-189, the delete dialog currently uses record-centric strings
and the old dismiss event name; change title and button text to use
stringResource(R.string.book_delete_dialog_title) (and corresponding
book_delete_dialog_delete/book_delete_dialog_cancel if available) instead of the
record_* keys, and update the dismiss handler reference to the renamed event
(BookDetailUiEvent.OnBookDeleteDialogDismiss) so all usages match the new event
name.
| <string name="record_delete_dialog_title">삭제하면 기록을 복구할 수 없어요.\n정말 삭제하시겠어요?</string> | ||
| <string name="record_delete_dialog_delete">삭제</string> | ||
| <string name="record_delete_dialog_cancel">취소</string> | ||
| <string name="book_delete">도서 삭제하기</string> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
도서 삭제 확인 다이얼로그용 문자열 추가 권장
현재 도서 삭제 다이얼로그에서 기록 삭제용 문자열(record_delete_dialog_title)을 재사용하고 있어 사용자 문구가 부정확합니다. 도서 삭제 전용 타이틀을 추가해 주세요.
적용 diff:
<string name="record_delete_dialog_cancel">취소</string>
<string name="book_delete">도서 삭제하기</string>
+ <string name="book_delete_dialog_title">삭제하면 도서를 복구할 수 없어요.\n정말 삭제하시겠어요?</string>추가로 BookDetailUi.kt의 도서 삭제 다이얼로그에서 title을 book_delete_dialog_title로 교체해야 합니다(해당 파일 코멘트 참고).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <string name="book_delete">도서 삭제하기</string> | |
| <string name="record_delete_dialog_cancel">취소</string> | |
| <string name="book_delete">도서 삭제하기</string> | |
| <string name="book_delete_dialog_title">삭제하면 도서를 복구할 수 없어요.\n정말 삭제하시겠어요?</string> |
🤖 Prompt for AI Agents
In feature/detail/src/main/res/values/strings.xml around line 19, add a new
string resource named book_delete_dialog_title with the appropriate Korean text
for the book-delete confirmation dialog (e.g., "도서 삭제하기" or contextual wording),
and then update BookDetailUi.kt to use R.string.book_delete_dialog_title instead
of reusing record_delete_dialog_title for the delete confirmation dialog title;
ensure resource name matches exactly and rebuild to pick up the new string.
| import androidx.compose.foundation.layout.WindowInsets | ||
| import androidx.compose.foundation.layout.exclude | ||
| import androidx.compose.foundation.layout.fillMaxSize | ||
| import androidx.compose.foundation.layout.height | ||
| import androidx.compose.foundation.layout.ime | ||
| import androidx.compose.foundation.layout.padding | ||
| import androidx.compose.material3.Scaffold | ||
| import androidx.compose.material3.ScaffoldDefaults | ||
| import androidx.compose.runtime.Composable |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
다른 Step(Quote/Emotion)에서도 IME 인셋 직접 처리 여부 확인 필요
Scaffold에서 IME 인셋을 제외했기 때문에, 텍스트 입력이 있는 모든 Step 화면은 각자 imePadding() 또는 windowInsetsPadding(WindowInsets.ime)를 적용해야 키보드에 가려지지 않습니다. ImpressionStep은 반영되어 있으나, QuoteStep/EmotionStep에도 동일 패턴이 적용되어 있는지 확인 부탁드립니다.
다음 스크립트로 관련 적용 여부를 빠르게 점검할 수 있습니다:
Also applies to: 50-50
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "[1] 입력 Step 정의 위치 확인 (Quote/Emotion)"
rg -nP -C2 --type=kt '(?m)^\s*fun\s+(QuoteStep|EmotionStep)\s*\('
echo
echo "[2] 각 Step에서 IME 인셋 처리(imePadding/windowInsetsPadding(WindowInsets.ime)) 사용 여부 확인"
rg -nP -C3 --type=kt '(imePadding\s*\(|windowInsetsPadding\s*\(\s*WindowInsets\.ime)' || trueLength of output: 197
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "[1] 입력 Step 정의 위치 확인 (QuoteStep/EmotionStep)"
rg -nP -C2 -g '*.kt' 'fun\s+(QuoteStep|EmotionStep)\s*\(' || true
echo ""
echo "[2] IME 인셋 처리 사용 여부 확인 (imePadding 또는 windowInsetsPadding(WindowInsets.ime))"
rg -nP -C3 -g '*.kt' '(imePadding\s*\(|windowInsetsPadding\s*\(\s*WindowInsets\.ime\))' || trueLength of output: 4014
EmotionStep에 IME 인셋 처리 누락
QuoteStep과 ImpressionStep에는 Modifier에 .imePadding()이 적용되어 있어 키보드에 가려지지 않지만, EmotionStep에는 해당 처리가 빠져 있습니다. 아래 위치에 IME 인셋 패딩을 적용해주세요:
• 파일:
feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/step/EmotionStep.kt
• 위치:
@Composable
fun EmotionStep(
state: RecordRegisterUiState,
modifier: Modifier = Modifier, // ← 여기에
) {
Box(
modifier = modifier // ← 이 modifier에 `.imePadding()` 추가
.fillMaxSize()
.background(White)
) {
…
}
}• 제안:
modifier = modifier.imePadding()
또는windowInsetsPadding(WindowInsets.ime)
🤖 Prompt for AI Agents
In
feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/step/EmotionStep.kt
around the EmotionStep composable declaration (the modifier parameter and the
Box modifier usage), the IME inset handling is missing; update the modifier so
the Box uses IME padding by changing the parameter or its use to apply ime
padding (e.g., set modifier = modifier.imePadding() or add
.imePadding()/.windowInsetsPadding(WindowInsets.ime) to the Box modifier chain)
and add the required import for imePadding or windowInsetsPadding.
delete 1표! |
dataStore 랑 kotlin collections, Android Lifecyle API 들에서 내부적으로 remove를 써가지고 통일성을 위해 이부분들 remove로 썼었는데 이 부분들 전부 delete로 바꿨슴다 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/datasource/DefaultBookRecentSearchDataSource.kt (1)
70-91: 로그 메시지와 가드 보완 제안 (delete 시맨틱에 맞추기)
- deleteRecentSearch 내부 로그 문구가 여전히 "adding"을 언급합니다. 혼동 방지를 위해 "deleting"으로 정정 권장.
- query가 공백일 때 조기 반환을 추가하면 불필요한 I/O를 피할 수 있습니다(추가 쪽과 일관성).
다음 패치를 제안합니다:
@@ - override suspend fun deleteRecentSearch(query: String) { + override suspend fun deleteRecentSearch(query: String) { + if (query.isBlank()) return dataStore.edit { prefs -> val currentSearches = prefs[BOOK_RECENT_SEARCHES]?.let { jsonString -> try { Json.decodeFromString<List<String>>(jsonString).toMutableList() } catch (e: SerializationException) { - Logger.e(e, "Failed to deserialize recent searches for adding") + Logger.e(e, "Failed to deserialize recent searches for deleting") mutableListOf() } catch (e: Exception) { - Logger.e(e, "Unexpected error while adding recent search") + Logger.e(e, "Unexpected error while deleting recent search") mutableListOf() } } ?: mutableListOf() currentSearches.remove(query) try { prefs[BOOK_RECENT_SEARCHES] = Json.encodeToString(currentSearches) } catch (e: SerializationException) { - Logger.e(e, "Failed to serialize recent searches after removal") + Logger.e(e, "Failed to serialize recent searches after deletion") } } }feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/common/component/SearchItem.kt (1)
50-56: 접근성 문구도 delete 용어로 갱신 권장UI 전반의 용어를 delete로 통일한 흐름에 맞춰 contentDescription도 갱신해 주세요. 가능하면 stringResource로 현지화도 고려 바랍니다.
다음 최소 변경을 권장합니다:
- contentDescription = "Remove Icon", + contentDescription = "Delete Icon",추가로, 접근성과 머티리얼 인터랙션 일관성을 위해 Icon+clickable 대신 IconButton 사용도 고려해보세요.
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt (1)
154-157: 삭제 요청 실패 시 사용자 피드백 처리 추가 제안예외가 발생해도 조용히 실패합니다. 간단한 try/catch로 토스트 등 피드백을 주면 UX가 개선됩니다.
아래처럼 최소한의 예외 처리와 토스트를 추가하는 것을 권장합니다:
- is BookSearchUiEvent.OnRecentSearchDeleteClick -> { - scope.launch { - repository.deleteBookRecentSearch(query = event.query) - } - } + is BookSearchUiEvent.OnRecentSearchDeleteClick -> { + scope.launch { + try { + repository.deleteBookRecentSearch(query = event.query) + } catch (e: Exception) { + Logger.e(e, "Failed to delete recent search") + sideEffect = BookSearchSideEffect.ShowToast("최근 검색어 삭제에 실패했어요. 잠시 후 다시 시도해주세요.") + } + } + }core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/datasource/DefaultLibraryRecentSearchDataSource.kt (1)
70-91: 용어 통일(“delete”) 및 사소한 가드 추가 제안메서드명이 deleteRecentSearch로 바뀐 것과 달리, 내부 로그 메시지는 여전히 "removal"/"removing"을 사용 중입니다. PR에서 전반적으로 “delete”로 용어를 통일하는 흐름에 맞춰 로그 메시지도 맞추는 것을 권장합니다. 추가로, 빈 문자열 입력에 대해 불필요한 I/O를 피하기 위해 빠른 반환 가드를 두는 것도 좋습니다. 기능 동작에는 영향 없으나 일관성/가독성 측면에서 개선 포인트입니다.
아래와 같이 수정해보세요.
override suspend fun deleteRecentSearch(query: String) { - dataStore.edit { prefs -> + if (query.isBlank()) return + dataStore.edit { prefs -> val currentSearches = prefs[LIBRARY_RECENT_SEARCHES]?.let { jsonString -> try { Json.decodeFromString<List<String>>(jsonString).toMutableList() } catch (e: SerializationException) { - Logger.e(e, "Failed to deserialize recent searches for removal") + Logger.e(e, "Failed to deserialize recent searches for deletion") mutableListOf() } catch (e: Exception) { - Logger.e(e, "Unexpected error while removing recent search") + Logger.e(e, "Unexpected error while deleting recent search") mutableListOf() } } ?: mutableListOf() currentSearches.remove(query) try { prefs[LIBRARY_RECENT_SEARCHES] = Json.encodeToString(currentSearches) } catch (e: SerializationException) { - Logger.e(e, "Failed to serialize recent searches after removal") + Logger.e(e, "Failed to serialize recent searches after deletion") } } }참고: 위 범위를 벗어나지만, Line 54의 주석(“제거”)도 “삭제”로 바꾸면 더욱 일관적입니다.
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/library/LibrarySearchPresenter.kt (1)
120-123: 최근 검색 삭제 처리에 실패 핸들링(로그/토스트) 추가 제안현재는 삭제 실패 시 조용히 무시됩니다. 다른 분기들처럼 최소한의 try/catch로 로깅 및 사용자 피드백(토스트)을 주면 UX와 디버깅이 좋아집니다.
- is LibrarySearchUiEvent.OnRecentSearchDeleteClick -> { - scope.launch { - repository.deleteLibraryRecentSearch(event.query) - } - } + is LibrarySearchUiEvent.OnRecentSearchDeleteClick -> { + scope.launch { + try { + repository.deleteLibraryRecentSearch(event.query) + } catch (e: Exception) { + Logger.e(e, "Failed to delete recent search") + sideEffect = LibrarySearchSideEffect.ShowToast("최근 검색어 삭제에 실패했습니다.") + } + } + }feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/library/LibrarySearchUiState.kt (1)
13-19: @immutable 부착 위치 재검토 권장 (UiState 뿐 아니라 화면 상태에도 부착)UiState에 @immutable 부착은 좋지만, 실제로 Compose가 관찰하는 것은 LibrarySearchUiState이므로 해당 data class에도 @immutable(또는 @stable) 부착을 고려해 주세요. 또한 UiState.Error가 Throwable을 직접 들고 있으면 엄밀한 의미의 불변성 보장이 약해집니다(예: stack trace 등). UI에는 사용자 친화적 메시지/코드 등 불변 데이터만 전달하는 구조가 안전합니다.
아래처럼 적용을 제안합니다(참고용, 선택사항):
@Immutable data class LibrarySearchUiState( val uiState: UiState = UiState.Idle, // ... ) : CircuitUiState sealed interface UiState { data object Idle : UiState data object Loading : UiState data object Success : UiState data class Error(val message: String) : UiState // 또는 별도 UiError 타입 }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (12)
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/repository/DefaultBookRepository.kt(3 hunks)core/datastore/api/src/main/kotlin/com/ninecraft/booket/core/datastore/api/datasource/RecentSearchDataSource.kt(1 hunks)core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/datasource/DefaultBookRecentSearchDataSource.kt(1 hunks)core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/datasource/DefaultLibraryRecentSearchDataSource.kt(1 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt(1 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchUi.kt(1 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchUiState.kt(2 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/common/component/SearchItem.kt(3 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/library/LibrarySearchPresenter.kt(1 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/library/LibrarySearchUi.kt(1 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/library/LibrarySearchUiState.kt(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
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)
🔇 Additional comments (12)
core/datastore/api/src/main/kotlin/com/ninecraft/booket/core/datastore/api/datasource/RecentSearchDataSource.kt (1)
8-8: 확인 완료:remove*명칭이 모두delete로 통일되었습니다
- 스크립트 실행 결과 기존
removeRecentSearch,removeBookRecentSearch,removeLibraryRecentSearch,OnRecentSearchRemoveClick,onRemoveIconClick등 모든 구 명칭이 더 이상 코드에 존재하지 않습니다.추가 검증 없이 안전하게 머지해도 좋습니다.
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchUiState.kt (2)
15-15: UiState에 @immutable 추가: 👍Compose에서 불필요한 재구성을 줄이는 데 도움이 됩니다.
51-51: 이벤트 명칭 변경(OnRecentSearchDeleteClick): 👍상위/하위 레이어의 delete* 네이밍과 잘 맞습니다.
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/common/component/SearchItem.kt (2)
27-27: 파라미터명 onDeleteIconClick으로 통일: 👍상위 호출부/이벤트 명칭과 일관됩니다.
67-68: 프리뷰 콜백명 변경 반영: 👍feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/library/LibrarySearchUi.kt (1)
139-141: onDeleteIconClick → OnRecentSearchDeleteClick 연결 변경 LGTMUI 콜백과 이벤트 명칭이 “delete”로 통일되었고, 이벤트 싱크 라우팅도 올바르게 연결되었습니다.
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchUi.kt (1)
157-160: 이벤트/콜백의 “delete” 명칭 반영 LGTMSearchItem의 파라미터 및 UI 이벤트가 통일된 명칭으로 변경되었으며, 라우팅도 적절합니다.
core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/BookRepository.kt (1)
20-21: deleteBook 추가 및 remove → delete 통일 확인됨 — 구현체·엔드포인트 반영 완료
- ‘remove(Book|Library)RecentSearch’ 잔존 참조 없음
- core/data/api…/BookRepository.kt에
suspend fun deleteBook(userBookId: String): Result<Unit>시그니처 존재- core/data/impl…/DefaultBookRepository.kt에
override suspend fun deleteBook(userBookId: String)구현 확인- core/network…/ReedService.kt에
@DELETE("api/v1/books/my-library/{userBookId}")와suspend fun deleteBook엔드포인트 존재- Presenter/UI 레이어에서
deleteBook(...)호출로 일관되게 사용 중LGTM!
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/library/LibrarySearchUiState.kt (2)
4-4: @immutable 도입 LGTMCompose 안정성 힌트 추가 잘 반영되었습니다.
41-41: 네이밍 변경 일관성 확인 및 승인
이전 이름(OnRecentSearchRemoveClick, onRemoveIconClick)에 대한 참조가 모두 제거되었으므로 변경 사항을 승인합니다.core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultBookRepository.kt (2)
68-70: deleteBook 위임 로직 확인 필요: 서비스/인터페이스 반환 타입 검증 요청
runSuspendCatching으로Result<Unit>반환 패턴은 기존 스타일과 일치합니다.
아래 두 가지를 직접 확인해 주세요:
- ReedService.deleteBook의 선언부가 실제로
Unit을 반환하는지- BookRepository 인터페이스의 deleteBook 시그니처와 정확히 일치하는지
33-35: remove → delete 리네이밍 검증 완료 — 문제 없습니다
이전 시그니처(removeBookRecentSearch,removeLibraryRecentSearch)는 모두 제거되었고,deleteBookRecentSearch/deleteLibraryRecentSearch가 API, 구현체(DefaultBookRepository), 호출부(BookSearchPresenter, LibrarySearchPresenter)에 일관되게 반영되었습니다. 추가 검토 없이 머지하셔도 됩니다.
2 participants
🔗 관련 이슈
📙 작업 설명
🧪 테스트 내역 (선택)
📸 스크린샷 또는 시연 영상 (선택)
default.mp4
💬 추가 설명 or 리뷰 포인트 (선택)
Summary by CodeRabbit
신기능
UI/UX
현지화