feat: Google Analytics 화면별, 이벤트별 태그 적용#157
Conversation
LaunchedEffect 를 사용하지 않아도 되는 부분들 RememberedEffect 로 교체
# Conflicts: # feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashPresenter.kt
WalkthroughFirebase Analytics가 추가되어 core.common에 DI 모듈과 AnalyticsHelper가 도입되었습니다. 다수 Presenter와 UI 컴포넌트에 AnalyticsHelper 주입 및 ImpressionEffect/logEvent 호출이 삽입되었고, 일부 Compose 일회성 효과는 LaunchedEffect→RememberedEffect로 교체되었으며 화면명은 ScreenNames로 중앙화되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Presenter as Presenter / UI
participant Effect as ImpressionEffect
participant AH as AnalyticsHelper
participant FA as FirebaseAnalytics
User->>Presenter: 화면 진입
Presenter->>Effect: ImpressionEffect 실행
Effect->>AH: logScreenView(screenName)
AH->>FA: logEvent("screen_view", params)
User->>Presenter: 사용자 액션(예: 삭제/저장/검색)
Presenter->>AH: logEvent(eventName)
AH->>FA: logEvent(eventName)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
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. ✨ 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: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/register/RecordRegisterPresenter.kt (1)
263-268: 중복 스크린 뷰 로깅 제거 필요저장 확인 다이얼로그에서
RECORD_DETAIL스크린 뷰를 미리 찍은 뒤, 실제로RecordDetailScreen으로 이동하면 해당 화면의 Presenter에서 다시 Impression이 찍혀 2회 집계됩니다. 사전에 찍는 스크린 뷰는 제거하세요.- is RecordRegisterUiEvent.OnRecordSavedDialogConfirm -> { - analyticsHelper.logScreenView(RECORD_DETAIL) + is RecordRegisterUiEvent.OnRecordSavedDialogConfirm -> { isRecordSavedDialogVisible = false navigator.pop() navigator.goTo(RecordDetailScreen(event.recordId)) }feature/edit/src/main/kotlin/com/ninecraft/booket/feature/edit/record/RecordEditPresenter.kt (1)
51-56: 페이지 번호 검증 누락으로 잘못된 값(비숫자/0)이 저장될 수 있음현재
isPageError는 상한만 검사하고, 저장 시toIntOrNull() ?: 0으로 0을 전달합니다. "abc"처럼 비숫자 입력도 통과해 0페이지가 저장될 수 있습니다. 범위(1..MAX_PAGE) 및 숫자여부 검증을 강화하세요.적용 Diff:
- val isPageError by remember { - derivedStateOf { - val page = recordPageState.text.toString().toIntOrNull() ?: 0 - page > MAX_PAGE - } - } + val isPageError by remember { + derivedStateOf { + val page = recordPageState.text.toString().toIntOrNull() + page == null || page !in 1..MAX_PAGE + } + } @@ RecordEditUiEvent.OnSaveButtonClick -> { - editRecord( - readingRecordId = recordInfo.id, - pageNumber = recordPageState.text.toString().toIntOrNull() ?: 0, - quote = recordQuoteState.text.toString(), - emotionTags = recordInfo.emotionTags, - impression = recordImpressionState.text.toString(), - onSuccess = { - navigator.pop() - }, - ) + // isPageError가 false인 경우에만 진입 가능하지만, 방어적으로 다시 한 번 변환 확인 + val page = recordPageState.text.toString().toIntOrNull() + if (page != null && page in 1..MAX_PAGE) { + editRecord( + readingRecordId = recordInfo.id, + pageNumber = page, + quote = recordQuoteState.text.toString(), + emotionTags = recordInfo.emotionTags, + impression = recordImpressionState.text.toString(), + onSuccess = { navigator.pop() }, + ) + } else { + sideEffect = RecordEditSideEffect.ShowToast("페이지 번호를 확인해 주세요.") + } }Also applies to: 66-74, 131-142
🧹 Nitpick comments (18)
core/ui/src/main/kotlin/com/ninecraft/booket/core/ui/component/InfinityLazyColumn.kt (1)
96-102: 가독성/계산 중복 감소 제안: index·상수 캐싱으로 조건식 단순화
lastVisibleItem?.index를 여러 번 안전 호출하고 있어 가독성이 떨어집니다. 한 번만 안전 해제하고 상수(마지막 인덱스, 트리거 인덱스)를 캐싱하면 읽기와 유지보수가 쉬워집니다.private fun LazyListState.reachedBottom( limitCount: Int = LIMIT_COUNT, triggerOnEnd: Boolean = false, ): Boolean { - val lastVisibleItem = layoutInfo.visibleItemsInfo.lastOrNull() - return (triggerOnEnd && lastVisibleItem?.index == layoutInfo.totalItemsCount - 1) || lastVisibleItem?.index != 0 && lastVisibleItem?.index == layoutInfo.totalItemsCount - (limitCount + 1) + val lastIndex = layoutInfo.totalItemsCount - 1 + val triggerIndex = layoutInfo.totalItemsCount - (limitCount + 1) + val i = layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: return false + return (triggerOnEnd && i == lastIndex) || (i != 0 && i == triggerIndex) }feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/ocr/OcrUi.kt (2)
124-138: Lifecycle 옵저버 구독 키를 lifecycleOwner로 변경 제안
DisposableEffect(Unit)대신DisposableEffect(lifecycleOwner)로 키를 두면 LifecycleOwner 교체(예: 구성 변경) 시 옵저버가 안전하게 재구성됩니다.아래처럼 수정 가능합니다.
- DisposableEffect(Unit) { + DisposableEffect(lifecycleOwner) { val observer = LifecycleEventObserver { _, event -> ... } lifecycleOwner.lifecycle.addObserver(observer) onDispose { lifecycleOwner.lifecycle.removeObserver(observer) } }
144-148: Analyzer 내부에서 eventSink 캡처 고정 이슈 — rememberUpdatedState로 최신 참조 보장
remember { ImageAnalysis.Analyzer { ... state.eventSink(...) } }는 최초 컴포지션 시점의eventSink를 캡처합니다. Presenter 재생성/재구성으로eventSink가 교체되면 구(舊) 참조를 호출할 수 있습니다.rememberUpdatedState로 최신 콜백을 참조하도록 바꾸는 것을 권장합니다.+ val updatedEventSink = rememberUpdatedState(newValue = state.eventSink) val imageAnalyzer = remember { ImageAnalysis.Analyzer { imageProxy -> - state.eventSink(OcrUiEvent.OnFrameReceived(imageProxy)) + updatedEventSink.value(OcrUiEvent.OnFrameReceived(imageProxy)) } }추가로 파일 상단에 다음 import가 필요합니다.
import androidx.compose.runtime.rememberUpdatedStatefeature/onboarding/src/main/kotlin/com/ninecraft/booket/feature/onboarding/OnboardingPresenter.kt (1)
53-55: 온보딩 화면 노출 로그 OK — 이벤트(다음/완료) 로그도 함께 남기면 분석 품질이 올라갑니다화면 뷰는 잘 남고 있습니다. PM 요구가 “화면별 + 이벤트별”이라면 다음 행동도 추천합니다.
- 다음 버튼 클릭 시: 현재 페이지 인덱스 파라미터 포함
- 마지막 페이지 완료 시: 완료 이벤트
적용 예시는 아래와 같습니다.
fun handleEvent(event: OnboardingUiEvent) { when (event) { is OnboardingUiEvent.OnNextButtonClick -> { - if (event.currentPage == 2) { + if (event.currentPage == ONBOARDING_STEPS_COUNT - 1) { + analyticsHelper.logEvent("onboarding_complete") scope.launch { repository.setOnboardingCompleted(true) navigator.resetRoot(LoginScreen) } } else { + analyticsHelper.logEvent( + "onboarding_next", + mapOf("page_index" to event.currentPage) + ) pagerState.let { state -> scope.launch { state.animateScrollToPage(event.currentPage + 1) } } } } } }추가로 “매직 넘버 2” 대신
ONBOARDING_STEPS_COUNT - 1을 사용하도록 함께 정리했습니다.feature/login/src/main/kotlin/com/ninecraft/booket/feature/termsagreement/TermsAgreementPresenter.kt (1)
88-90: 화면 노출 로그 OK — 약관 상호작용 이벤트도 함께 로깅 제안동의/토글/문서 열람/시작 버튼과 같은 핵심 상호작용을 이벤트로 남기면 전환 퍼널 분석에 유용합니다. PII는 포함하지 않고, 인덱스/결과 등 비식별 파라미터만 권장합니다.
예시:
fun handleEvent(event: TermsAgreementUiEvent) { when (event) { is TermsAgreementUiEvent.OnAllTermsAgreedClick -> { val toggleAgreed = !isAllAgreed agreedTerms = agreedTerms.map { toggleAgreed }.toPersistentList() + analyticsHelper.logEvent( + "terms_select_all", + mapOf("selected" to toggleAgreed) + ) } is TermsAgreementUiEvent.OnTermItemClick -> { agreedTerms = agreedTerms.set(event.index, !agreedTerms[event.index]) + analyticsHelper.logEvent( + "terms_toggle", + mapOf("index" to event.index, "selected" to agreedTerms[event.index]) + ) } is TermsAgreementUiEvent.OnPolicyClick -> { val policy = WebViewConstants.PRIVACY_POLICY navigator.goTo(WebViewScreen(url = policy.url, title = policy.title)) + analyticsHelper.logEvent("terms_policy_open") } is TermsAgreementUiEvent.OnTermClick -> { val terms = WebViewConstants.TERMS_OF_SERVICE navigator.goTo(WebViewScreen(url = terms.url, title = terms.title)) + analyticsHelper.logEvent("terms_terms_open") } is TermsAgreementUiEvent.OnStartButtonClick -> { + analyticsHelper.logEvent("terms_start_submit") scope.launch { userRepository.agreeTerms(true) .onSuccess { + analyticsHelper.logEvent("terms_start_success") navigator.resetRoot(HomeScreen) }.onFailure { exception -> + analyticsHelper.logEvent( + "terms_start_failure", + mapOf("reason" to (exception::class.simpleName ?: "Unknown")) + ) exception.message?.let { Logger.e(it) } sideEffect = exception.message?.let { TermsAgreementSideEffect.ShowToast(it) } } } } } }이벤트 명은 스네이크 케이스로 통일했고, 파라미터는 비식별 정보만 포함했습니다.
core/common/src/main/kotlin/com/ninecraft/booket/core/common/analytics/AnalyticsHelper.kt (1)
19-22: 이벤트 파라미터 확장 고려(선택)현재
logEvent(eventName)는 파라미터가 없어 실패 원인, 컨텍스트 등을 담기 어렵습니다. 향후 분석 품질을 위해logEvent(eventName: String, params: Map<String, Any?> = emptyMap())오버로드 추가를 제안합니다. 이번 PR 범위를 넘는다면 추후 작업으로 분리해도 됩니다.feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/library/LibrarySearchPresenter.kt (1)
93-93: 에러 이벤트 로깅은 적절합니다만 컨텍스트 파라미터 확장을 고려
ERROR_SEARCH전송은 유용합니다. 차후AnalyticsHelper가 파라미터를 지원하게 되면 네트워크/검증/권한 등 에러 유형 정도만(PII 제외) 파라미터로 추가하면 분석 품질이 올라갑니다.feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/book/BookDetailPresenter.kt (2)
56-58: 이벤트 상수 네이밍/범위 점검 권장
BOOK_DELETE,BOOK_DELETE_COMPLETE는 “도서 삭제” 플로우로 읽히는데, 하단에서 “기록 삭제”와 혼용되어 사용됩니다. 도메인 단위를 명확히 분리(record_delete,record_delete_complete등)하면 대시보드에서 해석이 쉬워집니다.
356-359: 도서 삭제 다이얼로그 오픈 시점에 시작 이벤트를 함께 로깅하면 퍼널 완성삭제 완료만 로깅 중입니다. 시작 이벤트(확인 다이얼로그 오픈)를 함께 기록하면 시작→완료 퍼널을 분석할 수 있습니다.
is BookDetailUiEvent.OnDeleteBookClick -> { isDetailMenuBottomSheetVisible = false isBookDeleteDialogVisible = true + analyticsHelper.logEvent(BOOK_DELETE) }feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/ocr/OcrPresenter.kt (1)
29-31: 문장 선택 화면 스크린 이름 상수의 중앙집중화 제안현재
RECORD_OCR_SENTENCE가 로컬 상수로 정의되어 있는데, 다른 화면명은ScreenNames로 일원화되어 있습니다. 분석 리포팅의 오탈자/불일치 리스크를 줄이기 위해 다음 중 하나로 정리하는 것을 권장합니다.
- 선택 1:
ScreenNames에OCR_SENTENCE를 추가하고 여기서는 그 값을 사용- 선택 2: 화면/스텝 전용 네임 스페이스(예:
object AnalyticsScreenNames.Record)를 만들어 모든 스텝명을 모으기큰 변경은 아니므로 본 PR 내에서도 안전하게 반영 가능합니다.
가능한 변경 예시(개념적 예):
- companion object { - private const val RECORD_OCR_SENTENCE = "record_OCR_sentence" - } + // 로컬 상수 제거, ScreenNames.OCR_SENTENCE 사용 @@ - analyticsHelper.logScreenView(RECORD_OCR_SENTENCE) + analyticsHelper.logScreenView(ScreenNames.OCR_SENTENCE)추가로 필요한 변경(다른 파일):
feature/screens/.../ScreenNames.kt에const val OCR_SENTENCE = "record_OCR_sentence"추가- 본 파일 상단에
import com.ninecraft.booket.feature.screens.ScreenNames추가Also applies to: 89-89
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt (1)
29-29: 일관성: navigator 가시성 private로 정렬 권장다른 Presenter들과 동일하게
navigator를private val로 두어 외부 노출을 피하는 것이 좋습니다. 기능엔 영향 없으며 코드 일관성 향상입니다.- @Assisted val navigator: Navigator, + @Assisted private val navigator: Navigator,feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/register/RecordRegisterPresenter.kt (3)
199-206: BottomSheet 열림은 이벤트로 기록하는 것이 적절합니다
OnImpressionGuideButtonClick은 화면 전환이 아닌 보조 UI 노출입니다. 스크린 뷰보다logEvent로 기록하는 편이 해석과 대시보드 구성에 적합합니다.- analyticsHelper.logScreenView(RECORD_INPUT_HELP) + analyticsHelper.logEvent(RECORD_INPUT_HELP)
279-286: 스텝별 스크린 네이밍 중앙집중화/타입세이프 제안스텝 이름 문자열이 분산되면 오탈자 리스크가 높습니다.
ScreenNames하위에object Record를 두거나 별도의AnalyticsScreenNames.Record로 모으는 방법을 고려해 주세요. sealed interface로 타입세이프하게 다루는 것도 옵션입니다.예시(개념):
// ScreenNames.kt object ScreenNames { object Record { const val INPUT_SENTENCE = "record_input_sentence" const val SELECT_EMOTION = "record_select_emotion" const val INPUT_OPINION = "record_input_opinion" } }그 후 본 파일:
- val screenName = when (currentStep) { - RecordStep.QUOTE -> RECORD_INPUT_SENTENCE - RecordStep.EMOTION -> RECORD_SELECT_EMOTION - RecordStep.IMPRESSION -> RECORD_INPUT_OPINION - } + val screenName = when (currentStep) { + RecordStep.QUOTE -> ScreenNames.Record.INPUT_SENTENCE + RecordStep.EMOTION -> ScreenNames.Record.SELECT_EMOTION + RecordStep.IMPRESSION -> ScreenNames.Record.INPUT_OPINION + }
47-48: MAX_PAGE(4032) 상수 검증 및 공용 상수 리팩터링 검토 요청MAX_PAGE 값이 4032로 두 파일에 중복 정의되어 있으며, 해당 값이 도메인 요구사항에 부합하는지 확인이 필요합니다. 만약 책 페이지 상한이 비즈니스 로직상 별도로 정해진 값이라면, 공용 도메인/설정 영역으로 이동하여 관리 일관성을 높일 것을 권장드립니다.
검토 대상 위치:
- feature/edit/src/main/kotlin/com/ninecraft/booket/feature/edit/record/RecordEditPresenter.kt
• companion object 내private const val MAX_PAGE = 4032
•derivedStateOf { … page > MAX_PAGE }검증 로직- feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/register/RecordRegisterPresenter.kt
• companion object 내private const val MAX_PAGE = 4032
•derivedStateOf { … page > MAX_PAGE }검증 로직제안 사항:
- 4032 값의 근거(도메인 상한, 요구사항 등을 문서 또는 담당자에게 확인)
- 공용 상수로 이동 (예:
core/common/constants/RecordConstants.kt생성 후 통합 관리)feature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/ScreenNames.kt (1)
13-16: OCR 문장 선택(결과) 스크린명 추가 제안OCR 플로우에서 문장 선택(결과) 화면을 별도 스크린명으로 집계하려면 여기에도 상수를 추가해두면 일관성이 높아집니다.
object ScreenNames { @@ - const val OCR = "record_OCR_camera" + const val OCR = "record_OCR_camera" + // OCR 결과(문장 선택) 화면 + const val OCR_SENTENCE = "record_OCR_sentence" }그 후
OcrPresenter에서는 로컬 상수 대신ScreenNames.OCR_SENTENCE를 사용하세요.feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt (1)
100-101: 이벤트 스팸 방지 및 속성(파라미터) 확장을 고려해주세요
SEARCH_BOOK_RESULT는 페이지네이션 로딩 때마다 중복 기록됩니다. 최초 검색에만 한정하면 분석 품질이 좋아집니다.- 가능하다면 결과 수, 검색어 길이/유형, 실패 사유(예외 클래스) 등 속성 값을 함께 기록하세요. 향후 퍼널/세그먼트 분석에 유용합니다.
예시 Diff(최초 검색 시에만 결과 이벤트 로깅):
- analyticsHelper.logEvent(SEARCH_BOOK_RESULT) + if (startIndex == START_INDEX) { + analyticsHelper.logEvent(SEARCH_BOOK_RESULT) + }추가로,
AnalyticsHelper에logEvent(name: String, params: Map<String, Any?>)오버로드를 추가하는 것도 검토 바랍니다. 없다면 본 코멘트는 참고만 하세요.Also applies to: 104-105, 126-126, 138-139, 178-179, 218-219
feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginPresenter.kt (1)
68-72: 실패 이벤트 로깅은 좋습니다. 성공/시도 이벤트도 보강을 검토하세요
- 현재
error_login만 존재합니다. 퍼널 분석을 위해login_attempt,login_success등 추가 이벤트를 고려하면 좋습니다.- 필요 시 에러 유형(예: 네트워크/자격증명/서버)을 파라미터로 구분해 두면 대시보드에서 필터링이 쉬워집니다.
AnalyticsHelper에 파라미터 지원이 없다면 참고만 하세요.Also applies to: 81-85
feature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/Screens.kt (1)
13-74: ScreenNames 적용 일관성 개선 제안대부분의 화면이
ScreenNames로 통일되었지만 아래 네 개는 여전히 리터럴입니다:OssLicensesScreen,RecordEditScreen,EmotionEditScreen,WebViewScreen. 스크린명 표준화를 위해ScreenNames에 상수 추가 후 이 파일에서도 사용하도록 정리하면 좋습니다. 향후 Analytics/딥링크/로그 필터링에서 이점이 큽니다.
📜 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 (24)
core/common/build.gradle.kts(1 hunks)core/common/src/main/kotlin/com/ninecraft/booket/core/common/analytics/AnalyticsHelper.kt(1 hunks)core/common/src/main/kotlin/com/ninecraft/booket/core/common/analytics/di/AnalyticsModule.kt(1 hunks)core/ui/build.gradle.kts(1 hunks)core/ui/src/main/kotlin/com/ninecraft/booket/core/ui/component/InfinityLazyColumn.kt(2 hunks)feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/book/BookDetailPresenter.kt(6 hunks)feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/record/RecordDetailPresenter.kt(6 hunks)feature/edit/src/main/kotlin/com/ninecraft/booket/feature/edit/record/RecordEditPresenter.kt(5 hunks)feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomePresenter.kt(3 hunks)feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryPresenter.kt(4 hunks)feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginPresenter.kt(6 hunks)feature/login/src/main/kotlin/com/ninecraft/booket/feature/termsagreement/TermsAgreementPresenter.kt(4 hunks)feature/onboarding/src/main/kotlin/com/ninecraft/booket/feature/onboarding/OnboardingPresenter.kt(3 hunks)feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/ocr/OcrPresenter.kt(4 hunks)feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/ocr/OcrUi.kt(2 hunks)feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/register/RecordRegisterPresenter.kt(8 hunks)feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/step/ImpressionStep.kt(2 hunks)feature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/ScreenNames.kt(1 hunks)feature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/Screens.kt(2 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt(8 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/library/LibrarySearchPresenter.kt(5 hunks)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt(7 hunks)feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashPresenter.kt(4 hunks)gradle/libs.versions.toml(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-31T23:30:37.547Z
Learnt from: easyhooon
PR: YAPP-Github/Reed-Android#88
File: feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/record/RecordDetailPresenter.kt:18-47
Timestamp: 2025-07-31T23:30:37.547Z
Learning: In Circuit architecture, presenters receive the Screen object directly as a constructor parameter (e.g., Assisted private val screen: RecordDetailScreen), and screen parameters are accessed through this screen object (e.g., screen.recordId). Screen parameters should not be added as separate constructor parameters.
Applied to files:
feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/register/RecordRegisterPresenter.ktfeature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/record/RecordDetailPresenter.ktfeature/edit/src/main/kotlin/com/ninecraft/booket/feature/edit/record/RecordEditPresenter.ktfeature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/Screens.kt
🧬 Code graph analysis (2)
feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryPresenter.kt (2)
core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/BookRepository.kt (1)
filterLibraryBooks(29-33)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultBookRepository.kt (1)
filterLibraryBooks(45-47)
feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/record/RecordDetailPresenter.kt (2)
core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/RecordRepository.kt (1)
getRecordDetail(24-26)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultRecordRepository.kt (1)
getRecordDetail(33-35)
⏰ 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 (18)
core/ui/build.gradle.kts (1)
18-18: compose-effects 의존성 직접 선언 필요 (feature 모듈)core/ui 모듈에
implementation(libs.compose.effects)선언은 적절하게 적용되어 있습니다.
그러나 RememberedEffect를 사용 중인 모든 feature 모듈에서는 트랜지티브하게 해당 라이브러리가 노출되지 않으므로, 각 모듈의build.gradle.kts에 직접 의존성을 추가해야 컴파일 오류를 피할 수 있습니다.– 적용 대상 feature 모듈
• feature/splash
• feature/settings
• feature/edit
• feature/record
• feature/search
• feature/library
• feature/login
• feature/home
• feature/detail각 모듈의
dependencies블록에 아래 한 줄을 추가해주세요:dependencies { implementation(libs.compose.effects) }⛔ Skipped due to learnings
Learnt from: easyhooon PR: YAPP-Github/Reed-Android#61 File: feature/webview/build.gradle.kts:17-21 Timestamp: 2025-07-20T12:34:23.786Z Learning: Reed-Android 프로젝트에서는 `booket.android.feature` convention plugin을 사용하여 feature 모듈들의 공통 의존성을 관리한다. 이 plugin은 Circuit, Compose, 그리고 core 모듈들의 의존성을 자동으로 포함하므로, 각 feature 모듈의 build.gradle.kts에서는 특별한 의존성(예: libs.logger, libs.kakao.auth)만 별도로 선언하면 된다.gradle/libs.versions.toml (1)
75-75: Firebase BoM 기반 버전 위임 일관성 확인 완료
- 제시된
rg -nP -g '!**/build/**' 'com\.google\.firebase:firebase-[a-z-]+:[0-9]'스크립트 실행 결과, 개별 Firebase 라이브러리에 직접 버전이 지정된 부분이 없습니다.- 따라서
gradle/libs.versions.toml에서firebase-bom = "34.1.0"으로만 버전을 관리하고, 개별 라이브러리에 버전을 지정하지 않는 현재 방식이 적절함을 확인했습니다.feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/step/ImpressionStep.kt (1)
63-68: 초기 1회 포커스/키보드 표시 용도로 RememberedEffect 사용 적합
Unit키로 최초 컴포지션 한 번만 실행되어 의도(빈 텍스트 시 자동 포커스 및 키보드 표시)에 부합합니다. UI 플리커 없이 동작할 가능성이 높습니다.feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/ocr/OcrUi.kt (1)
117-122: 권한 1회 요청에 RememberedEffect 적용, 적절합니다초기 진입 시 한 번만 실행되어 권한 요청을 트리거해야 하는 요구에 정확히 부합합니다.
LaunchedEffect(Unit)대비 코루틴이 불필요한 케이스라 더 간결합니다.feature/onboarding/src/main/kotlin/com/ninecraft/booket/feature/onboarding/OnboardingPresenter.kt (1)
23-27: AnalyticsHelper 주입 추가, DI 구성에는 문제 없어 보입니다
@AssistedInject컨스트럭터에AnalyticsHelper를 추가한 형태가 Hilt + Circuit 패턴과 잘 맞습니다.Factory시그니처 변경이 필요 없는 점도 👍feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashPresenter.kt (2)
35-40: AnalyticsHelper 주입 추가, 구성 타당합니다Singleton 범위 Analytics를 Presenter에서 사용하는 패턴으로 문제 없습니다.
105-107: Splash screen_view 수집 여부 PM/데이터 전략 확인 권장스플래시는 체류 시간이 짧아 세션/화면 수치가 과대집계될 수 있어 많은 팀이 screen_view에서 제외합니다. 이번 PR이 “PM 제공 태그 전부 적용” 목적이라면, 스플래시 포함이 의도인지 재확인 부탁드립니다. 의도치 않았다면 해당 블록 제거만으로 안전하게 제외할 수 있습니다.
feature/login/src/main/kotlin/com/ninecraft/booket/feature/termsagreement/TermsAgreementPresenter.kt (1)
30-34: AnalyticsHelper 주입 추가, DI와 호출 지점 모두 적절합니다Terms Agreement 화면에서의 주입/사용 패턴이 다른 Presenter들과 일관됩니다.
feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryPresenter.kt (1)
157-163: 초기 로딩 트리거를 RememberedEffect로 교체한 점 좋습니다재컴포지션/구성 복원 시 중복 호출을 방지하는 의도가 명확하고, 초기 페이지 로드에 적합합니다.
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomePresenter.kt (1)
91-93: 한 번만 실행되는 초기 로딩 트리거 적절합니다
RememberedEffect(true)로 초기 페치가 중복 실행되지 않도록 한 부분 👍feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/book/BookDetailPresenter.kt (1)
226-227: 삭제 완료 시점 이벤트 로깅 적절성공 콜백에서만
BOOK_DELETE_COMPLETE를 전송하는 구조는 정확합니다.feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/ocr/OcrPresenter.kt (1)
123-125: ImpressionEffect로 화면 노출 1회 로깅 적용 — 좋습니다첫 진입 시점에만 OCR 카메라 화면 노출을 로깅하도록 잘 배치되었습니다. Circuit 재구성에도 중복 로깅이 발생하지 않겠습니다.
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt (1)
150-152: Settings 화면 Impression 로깅 — 좋습니다ImpressionEffect로 화면 노출 시점에 단 한 번 스크린 뷰가 찍히도록 잘 구성되어 있습니다.
feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/record/RecordDetailPresenter.kt (1)
81-82: 삭제 플로우 및 화면 노출 로깅 배치 — 적절합니다
- 삭제 버튼 클릭 시점과 성공 시점의 이벤트를 분리해 로깅한 점이 좋습니다.
- 화면 노출은 ImpressionEffect로 1회만 찍히도록 했습니다.
Also applies to: 142-144, 164-166
feature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/ScreenNames.kt (1)
3-17: 스크린 이름 단일 출처 구축 — 좋습니다대부분의 상위 화면명을 이 파일로 집중시켜 오탈자/불일치를 줄일 수 있게 되었습니다. Analytics/내비게이션 양측에서 재사용하기에 적합합니다.
feature/edit/src/main/kotlin/com/ninecraft/booket/feature/edit/record/RecordEditPresenter.kt (1)
96-99: 저장 성공 이벤트 로깅은 적절합니다
RECORD_EDIT_SAVE를 성공 콜백에서만 로깅하는 위치 선정이 적절합니다. 실패 케이스에 별도의 에러 이벤트가 필요하면 확장 고려만 해두세요.feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginPresenter.kt (1)
96-99: ImpressionEffect로 스크린 뷰 로깅 — 일관성 OK
LoginScreen.name을 사용한 스크린 뷰 로깅이 다른 화면들과 컨벤션이 일치합니다.feature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/Screens.kt (1)
13-74: 스크린 이름 변경 검증 완료
레포지토리 내 모든.kt파일에서"Record"리터럴,.name == "Record",toString() == "Record"비교를 검색한 결과 해당 패턴이 전혀 발견되지 않아(오직ReedScreen(name = "<다른 리터럴>")만 존재) RecordScreen의 이름을 직접 비교하는 로직이 없음을 확인했습니다.
따라서 화면 이름을ScreenNames.RECORD로 변경해도 런타임 동작이나 대시보드 그룹핑에 영향을 주지 않습니다.
| platform(libs.firebase.bom), | ||
| libs.firebase.analytics, | ||
| libs.logger, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Firebase BoM + Analytics 추가 OK. KTX 전환 영향(확장 API 사용 여부)만 확인 부탁드립니다.
이번 변경으로 firebase-analytics-ktx → firebase-analytics로 전환되었습니다. 최신 BoM에서는 KTX 확장이 기본 아티팩트로 통합된 경우가 많지만, 코드에서 Firebase.analytics 등의 KTX 확장을 사용한다면 컴파일이 깨지지 않는지 점검이 필요합니다. 특히 AnalyticsHelper/AnalyticsModule에서 어떤 API를 쓰는지 확인해 주세요.
다음 스크립트로 KTX 확장 사용 흔적과 잔존 -ktx 의존성 유무를 빠르게 확인할 수 있습니다.
🏁 Script executed:
#!/bin/bash
# 1) KTX 확장 사용 여부 확인
rg -nP --type=kotlin -C2 '\bFirebaseAnalytics\.getInstance\(|\bFirebase\.analytics\b'
# 2) -ktx 아티팩트가 남아있는지 확인
rg -nP -g '!**/build/**' 'firebase-(analytics|crashlytics)-ktx'Length of output: 746
KTX 확장 함수 의존성 누락으로 인한 컴파일 오류 위험
AnalyticsModule.kt에서 Firebase.analytics 확장 프로퍼티를 사용하고 있는데, firebase-analytics-ktx 의존성을 제거하면서 컴파일 오류가 발생할 수 있습니다. 다음 중 하나로 수정해 주세요:
- core/common/build.gradle.kts
libs.firebase.analytics대신 KTX 아티팩트인libs.firebase.analytics.ktx를 추가
- 또는 core/common/src/main/kotlin/com/ninecraft/booket/core/common/analytics/di/AnalyticsModule.kt (19행)
Firebase.analytics대신FirebaseAnalytics.getInstance(context)호출로 변경
🤖 Prompt for AI Agents
In core/common/build.gradle.kts around lines 21 to 23, the KTX extension
dependency for Firebase Analytics was removed causing potential compile errors
in AnalyticsModule.kt where Firebase.analytics is used; fix by either replacing
libs.firebase.analytics with the KTX artifact libs.firebase.analytics.ktx in
this build file, or alternatively modify
core/common/src/main/kotlin/com/ninecraft/booket/core/common/analytics/di/AnalyticsModule.kt
at line 19 to stop using the Firebase.analytics extension and instead call
FirebaseAnalytics.getInstance(context) to obtain the analytics instance.
| @Provides | ||
| @Singleton | ||
| fun provideFirebaseAnalytics(): FirebaseAnalytics { | ||
| return Firebase.analytics | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Firebase.analytics 확장 프로퍼티 사용 시 KTX 의존성 확인 필요
Firebase.analytics는 KTX 확장에 의해 제공됩니다. 만약 의존성을 firebase-analytics(비-KTX)로만 교체했다면 컴파일이 실패할 수 있습니다. 두 가지 중 하나를 선택해 주세요.
옵션 A) KTX 유지
- Gradle에
firebase-analytics-ktx가 포함되어 있는지 확인
검증 스크립트:
옵션 B) KTX 미사용 (대안 제공자)
- 애플리케이션 컨텍스트 기반으로 인스턴스를 획득
import android.content.Context
import com.google.firebase.Firebase
import com.google.firebase.analytics.FirebaseAnalytics
// import com.google.firebase.analytics.analytics // 제거
import dagger.Module
import dagger.Provides
import dagger.hilt.InstallIn
import dagger.hilt.android.qualifiers.ApplicationContext
import dagger.hilt.components.SingletonComponent
import javax.inject.Singleton
@Module
@InstallIn(SingletonComponent::class)
object AnalyticsModule {
@Provides
@Singleton
fun provideFirebaseAnalytics(@ApplicationContext context: Context): FirebaseAnalytics =
FirebaseAnalytics.getInstance(context)
}팀 규칙(ktx 사용 여부)에 맞게 한 가지로 통일해 두면 혼선을 줄일 수 있습니다.
🏁 Script executed:
#!/bin/bash
rg -nP --glob '**/build.gradle.kts' 'firebase-analytics-ktx|libs\.firebase\.analyticsKtx'Length of output: 91
스크립트가 build.gradle.kts에서 KTX 의존성을 찾지 못했습니다. Groovy DSL(.gradle) 또는 버전 카탈로그(.toml)에 정의됐을 수 있으므로, 전체 빌드 파일과 버전 카탈로그에서 firebase-analytics 및 firebase-analytics-ktx 항목을 검색해 주세요.
🏁 Script executed:
#!/bin/bash
# 1. 모든 Gradle 빌드 스크립트(.gradle, .gradle.kts)에서 firebase-analytics(-ktx) 의존성 검색
rg -nP --glob '**/build.gradle*' 'firebase-analytics(-ktx)?'
# 2. 버전 카탈로그(gradle/libs.versions.toml 등)에서 analytics 관련 항목 검색
rg -nP --glob '**/libs.versions.toml' 'analytics'Length of output: 226
Firebase.analytics KTX 의존성 누락 확인 및 조치 필요
다음 사항을 확인했습니다.
gradle/libs.versions.toml(151행)에firebase-analytics만 정의되어 있으며,firebase-analytics-ktx관련 alias 또는 의존성은 없습니다.- 코드에서
Firebase.analytics확장 프로퍼티를 사용하고 있으므로, KTX 모듈이 없으면 컴파일 에러가 발생합니다.
따라서 아래 두 가지 옵션 중 하나를 반드시 선택하여 반영해 주세요.
옵션 A) KTX 유지
Gradle 의존성에 KTX 모듈을 추가
// build.gradle.kts
dependencies {
implementation("com.google.firebase:firebase-analytics-ktx")
}또는 버전 카탈로그에 alias 등록
# gradle/libs.versions.toml
firebase-analytics-ktx = { group = "com.google.firebase", name = "firebase-analytics-ktx", version.ref = "firebase" }옵션 B) KTX 미사용 (대안 제공자)
확장 프로퍼티 대신 애플리케이션 컨텍스트 기반으로 인스턴스 획득
import com.google.firebase.Firebase
import com.google.firebase.analytics.FirebaseAnalytics
// import com.google.firebase.analytics.analytics // 제거
import dagger.hilt.android.qualifiers.ApplicationContext
import dagger.Module
import dagger.Provides
import dagger.hilt.InstallIn
import dagger.hilt.components.SingletonComponent
import javax.inject.Singleton
import android.content.Context
@Module
@InstallIn(SingletonComponent::class)
object AnalyticsModule {
@Provides
@Singleton
fun provideFirebaseAnalytics(@ApplicationContext context: Context): FirebaseAnalytics =
FirebaseAnalytics.getInstance(context)
}위 두 옵션 중 팀 정책에 맞는 방식을 선택하여 일관성 있게 적용해 주세요.
🤖 Prompt for AI Agents
In core/common/src/main/kotlin/com/ninecraft/booket/core/common/analytics/di/AnalyticsModule.kt
around lines 16 to 21, the code uses the Firebase KTX extension property
Firebase.analytics but the Gradle version catalog does not include
firebase-analytics-ktx, causing a compile error; fix by either (A) adding the
KTX dependency to the project (add implementation
com.google.firebase:firebase-analytics-ktx or register an alias in
gradle/libs.versions.toml) so Firebase.analytics is available, or (B) stop using
the KTX extension and obtain FirebaseAnalytics via the application Context
(e.g., FirebaseAnalytics.getInstance(context)) and update the provider
accordingly; choose the team-approved option and apply it consistently across
build files and this provider.
| RememberedEffect(reached) { | ||
| if (reached && layoutInfo.totalItemsCount > limitCount) action() | ||
| } |
There was a problem hiding this comment.
무한 스크롤 트리거가 한 번만 실행되는 회귀 가능성: RememberedEffect(Boolean) 키 사용
RememberedEffect(reached)는 키 값이 동일하게 재방문될 때(예: true 재진입) 재실행되지 않습니다. 무한 스크롤에서는 스크롤 임계 도달 시마다 action()이 재호출되어야 하므로, boolean 키 기반의 RememberedEffect는 이후 페이지에서 트리거가 멈출 수 있습니다. LaunchedEffect + 키 변화 감지로 복구하는 것을 제안합니다.
아래처럼 교체하면 false → true 전이 때마다 한 번씩 호출됩니다.
- RememberedEffect(reached) {
- if (reached && layoutInfo.totalItemsCount > limitCount) action()
- }
+ LaunchedEffect(reached) {
+ if (reached && layoutInfo.totalItemsCount > limitCount) {
+ action()
+ }
+ }추가로, 더 견고하게 하려면 snapshotFlow로 전이를 명시적으로 다루는 방식도 가능합니다(선택):
// 추가 import: import kotlinx.coroutines.flow.*
LaunchedEffect(limitCount, loadOnBottom) {
snapshotFlow { reachedBottom(limitCount, loadOnBottom) to layoutInfo.totalItemsCount }
.distinctUntilChanged() // (reached, count) 변화만 반응
.filter { (reached, count) -> reached && count > limitCount }
.collect { action() }
}필요 시 isLoading 플래그를 인자로 받아 중복 호출을 더 강하게 차단하는 것도 고려해 주세요.
🤖 Prompt for AI Agents
In
core/ui/src/main/kotlin/com/ninecraft/booket/core/ui/component/InfinityLazyColumn.kt
around lines 85 to 87, the RememberedEffect(reached) usage will not re-run when
the boolean key remains the same on re-entry, causing infinite-scroll triggers
to stop after the first hit; replace it with a LaunchedEffect keyed on relevant
inputs (e.g., limitCount and loadOnBottom) that observes transitions or uses
snapshotFlow to detect reached→true transitions (optionally include
layoutInfo.totalItemsCount in the observed tuple), filter for reached &&
totalItemsCount > limitCount and then call action(); also consider passing an
isLoading flag to prevent duplicate calls.
| is BookDetailUiEvent.OnDeleteRecordClick -> { | ||
| isRecordMenuBottomSheetVisible = false | ||
| isRecordDeleteDialogVisible = true | ||
| analyticsHelper.logScreenView(BOOK_DELETE) | ||
| } |
There was a problem hiding this comment.
오류: 기록 삭제 트리거에서 logScreenView와 도서 삭제 이벤트를 사용하고 있습니다
- 기록 삭제 UI 진입인데 이벤트 키가
library_book_delete로 되어 있어 의미가 불일치합니다. - 게다가
logScreenView를 호출하고 있어 이벤트가 스크린 뷰로 집계될 우려가 있습니다.
최소 수정(함수 오용 수정):
- analyticsHelper.logScreenView(BOOK_DELETE)
+ analyticsHelper.logEvent(BOOK_DELETE)권장 수정(의미 일치 + 함수 오용 모두 해결):
@@
- private const val BOOK_DELETE = "library_book_delete"
- private const val BOOK_DELETE_COMPLETE = "library_book_delete_complete"
+ private const val BOOK_DELETE = "library_book_delete"
+ private const val BOOK_DELETE_COMPLETE = "library_book_delete_complete"
+ private const val RECORD_DELETE = "library_record_delete"
@@
- analyticsHelper.logScreenView(BOOK_DELETE)
+ analyticsHelper.logEvent(RECORD_DELETE)📝 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.
| is BookDetailUiEvent.OnDeleteRecordClick -> { | |
| isRecordMenuBottomSheetVisible = false | |
| isRecordDeleteDialogVisible = true | |
| analyticsHelper.logScreenView(BOOK_DELETE) | |
| } | |
| // --- constants in BookDetailPresenter.kt --- | |
| private const val BOOK_DELETE = "library_book_delete" | |
| private const val BOOK_DELETE_COMPLETE = "library_book_delete_complete" | |
| private const val RECORD_DELETE = "library_record_delete" | |
| // ... later in the same file, inside the event handler: | |
| is BookDetailUiEvent.OnDeleteRecordClick -> { | |
| isRecordMenuBottomSheetVisible = false | |
| isRecordDeleteDialogVisible = true | |
| analyticsHelper.logEvent(RECORD_DELETE) | |
| } |
🤖 Prompt for AI Agents
In
feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/book/BookDetailPresenter.kt
around lines 326-330, the handler for OnDeleteRecordClick currently calls
analyticsHelper.logScreenView(BOOK_DELETE) which mismatches the intent (record
deletion, not a screen view) and uses the wrong analytics function; change the
call to use the event logging API with a semantically correct event key (e.g.,
analyticsHelper.logEvent(BOOK_RECORD_DELETE) or the existing constant that
represents a record-delete event), and ensure the event key name reflects
"record delete" rather than "library_book_delete" so the logged event accurately
represents the action.
| import com.ninecraft.booket.feature.record.component.ImpressionGuideBottomSheet | ||
| import com.ninecraft.booket.feature.record.register.RecordRegisterUiEvent | ||
| import com.ninecraft.booket.feature.record.register.RecordRegisterUiState | ||
| import com.skydoves.compose.effects.RememberedEffect |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
이 모듈의 build.gradle에도 compose-effects 의존성이 선언되어 있는지 확인 필요
현재 파일은 feature/record 모듈이며, RememberedEffect를 직접 import합니다. core/ui에만 libs.compose.effects가 추가되어 있다면 이 모듈에서 컴파일 에러가 발생할 수 있습니다. 사용 모듈마다 직접 의존을 선언하거나, 의도적으로 transitive export하려면 export 측 모듈에서 api(...)를 사용해야 합니다.
빠른 점검 스크립트:
원하시면 feature/record/build.gradle.kts에 의존성 추가 패치를 드리겠습니다.
🏁 Script executed:
#!/bin/bash
# 1) 본 모듈 파일 경로를 기준으로 최상위 build.gradle(.kts) 확인
fd -tf -a 'build.gradle.kts' | rg -n 'feature/record/.*/build.gradle.kts' -n
# 2) 해당 빌드 파일에 compose-effects 선언이 있는지 확인
rg -n 'libs\.compose\.effects' feature/recordLength of output: 124
feature/record 모듈의 build.gradle.kts 파일 위치와 libs.compose.effects 의존성 선언 여부를 명확히 확인하기 위해 아래 스크립트를 실행해주세요.
🏁 Script executed:
#!/bin/bash
# 1) feature/record 모듈 내 build.gradle.kts 파일 위치 확인
find feature/record -maxdepth 2 -type f -name "build.gradle.kts"
# 2) 해당 파일의 상위 200줄 출력 (의존성 블록 확인용)
sed -n '1,200p' feature/record/build.gradle.kts
# 3) compose-effects 라이브러리 선언 여부 조회
grep -n 'compose\\.effects' feature/record/build.gradle.kts || echo "libs.compose.effects not found in feature/record/build.gradle.kts"Length of output: 947
Compose-effects 의존성 추가 필요
feature/record/build.gradle.kts 모듈에 libs.compose.effects 의존성이 선언되어 있지 않아 RememberedEffect 사용 시 컴파일 오류가 발생합니다. 아래와 같이 dependencies 블록에 libs.compose.effects 를 추가해주세요.
— feature/record/build.gradle.kts
dependencies {
implementations(
projects.core.ocr,
libs.compose.system.ui.controller,
+ libs.compose.effects,
libs.androidx.activity.compose,
libs.androidx.camera.camera2,
libs.androidx.camera.lifecycle,
libs.androidx.camera.view,
libs.logger,
)
}📝 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.
| import com.skydoves.compose.effects.RememberedEffect | |
| // feature/record/build.gradle.kts | |
| dependencies { | |
| implementations( | |
| projects.core.ocr, | |
| libs.compose.system.ui.controller, | |
| libs.compose.effects, | |
| libs.androidx.activity.compose, | |
| libs.androidx.camera.camera2, | |
| libs.androidx.camera.lifecycle, | |
| libs.androidx.camera.view, | |
| libs.logger, | |
| ) | |
| } |
🤖 Prompt for AI Agents
In
feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/step/ImpressionStep.kt
around line 46, the import of RememberedEffect requires the compose-effects
dependency which is missing from the module; open
feature/record/build.gradle.kts and inside the dependencies block add the
libs.compose.effects dependency (implementation or api consistent with the
module's convention), then sync/refresh Gradle so the import compiles.
| private const val SEARCH_BOOK_INPUT = "search_book_input" | ||
| private const val SEARCH_BOOK_RESULT = "search_book_result" | ||
| private const val SEARCH_BOOK_NO_RESULT = "search_book_noresult" | ||
| private const val ERROR_SEARCH_LOADING = "error_search_loading" | ||
| private const val REGISTER_BOOK_OPTION = "register_book_option" | ||
| private const val REGISTER_BOOK_COMPLETE = "register_book_complete" | ||
| private const val ERROR_REGISTER_BOOK = "error_register_book" |
There was a problem hiding this comment.
이벤트 키 네이밍 불일치(오타) 수정 필요
SEARCH_BOOK_NO_RESULT의 값이 "search_book_noresult"로 언더스코어가 빠져 있습니다. 다른 키들과 컨벤션을 맞추고 대시보드에서의 필터링 혼선을 방지하기 위해 수정하세요.
적용 Diff:
- private const val SEARCH_BOOK_NO_RESULT = "search_book_noresult"
+ private const val SEARCH_BOOK_NO_RESULT = "search_book_no_result"📝 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.
| private const val SEARCH_BOOK_INPUT = "search_book_input" | |
| private const val SEARCH_BOOK_RESULT = "search_book_result" | |
| private const val SEARCH_BOOK_NO_RESULT = "search_book_noresult" | |
| private const val ERROR_SEARCH_LOADING = "error_search_loading" | |
| private const val REGISTER_BOOK_OPTION = "register_book_option" | |
| private const val REGISTER_BOOK_COMPLETE = "register_book_complete" | |
| private const val ERROR_REGISTER_BOOK = "error_register_book" | |
| private const val SEARCH_BOOK_INPUT = "search_book_input" | |
| private const val SEARCH_BOOK_RESULT = "search_book_result" | |
| private const val SEARCH_BOOK_NO_RESULT = "search_book_no_result" | |
| private const val ERROR_SEARCH_LOADING = "error_search_loading" | |
| private const val REGISTER_BOOK_OPTION = "register_book_option" | |
| private const val REGISTER_BOOK_COMPLETE = "register_book_complete" | |
| private const val ERROR_REGISTER_BOOK = "error_register_book" |
🤖 Prompt for AI Agents
In
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt
around lines 47 to 53, the constant SEARCH_BOOK_NO_RESULT has a typo value
"search_book_noresult" missing the underscore; change its string value to
"search_book_no_result" to match the naming convention of the other event keys
so dashboard filtering and conventions remain consistent.
| ImpressionEffect { | ||
| analyticsHelper.logScreenView(LibrarySearchScreen.name) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
스크린 뷰 로깅 표준화 권장
여기도 Helper가 screen_view + screen_name 파라미터 방식으로 바뀌면, 호출부는 그대로 유지해도 표준 리포트에 반영됩니다.
🤖 Prompt for AI Agents
In
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/library/LibrarySearchPresenter.kt
around lines 161-163, replace the current ImpressionEffect block that calls
analyticsHelper.logScreenView(LibrarySearchScreen.name) with a standardized
screen_view event call so it reports as "screen_view" with a "screen_name"
parameter (e.g. call analyticsHelper.logEvent("screen_view", mapOf("screen_name"
to LibrarySearchScreen.name")) or the project's equivalent helper signature).
Ensure the event name is "screen_view" and the payload includes "screen_name"
set to LibrarySearchScreen.name so standardized reports pick it up.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt (1)
48-48: 이벤트 키 네이밍 오타 수정 필요: 언더스코어 누락대시보드 필터링 혼선을 방지하려면
search_book_noresult→search_book_no_result로 수정해야 합니다. 이전 리뷰에서도 동일 지적이 있었습니다.적용 diff:
- private const val SEARCH_BOOK_NO_RESULT = "search_book_noresult" + private const val SEARCH_BOOK_NO_RESULT = "search_book_no_result"feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt (1)
34-38: 이전 리뷰 지적 사항(오타) 반영 확인 — 이벤트 상수 값 수정 완료SETTINGS_WITHDRAWAL_COMPLETE가 WARNING과 동일했던 오타가 수정되어 정상 분리되었습니다. 더 이상 잘못된 이벤트명이 로깅되지 않겠습니다.
🧹 Nitpick comments (12)
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt (8)
42-42: 화면 노출(Impression) 태깅 추가 제안PR 목표(“화면별 태그 적용”)에 맞춰 해당 화면 최초 진입 시 1회 screen_view 로깅을 권장합니다. Circuit의 ImpressionEffect(또는 remember/LaunchedEffect 1회성 패턴)를 사용하면 중복 로깅을 막을 수 있습니다.
예시(참고용):
// present() 내부, state 초기화 이후 ImpressionEffect(key1 = Unit) { // ScreenNames 등 중앙화된 화면명 사용 권장 analyticsHelper.logScreenView("search_book") }
46-52: 이벤트 키 문자열 중앙화/타입 세이프티 제안문자열 상수로 이벤트를 관리하면 오탈자·중복 정의 리스크가 큽니다. 공통 모듈에 타입 안정화된 이벤트 정의(예: sealed interface/object, enum + name 프로퍼티)를 두고 사용을 권장합니다.
예시(개념):
sealed interface AnalyticsEvent { val name: String } object SearchEvents { object Input : AnalyticsEvent { override val name = "search_book_input" } object Result : AnalyticsEvent { override val name = "search_book_result" } object NoResult : AnalyticsEvent { override val name = "search_book_no_result" } } // 사용 analyticsHelper.logEvent(SearchEvents.Result.name)
98-103: 검색 결과/무결과 이벤트에 핵심 파라미터 포함 제안분석 활용도를 높이기 위해 페이지 인덱스/결과 수/마지막 페이지 여부를 함께 전송하세요. AnalyticsHelper에 파라미터 오버로드가 없다면 추가를 권장합니다.
예시 diff(오버로드가 있다는 가정):
- analyticsHelper.logEvent(SEARCH_BOOK_RESULT) + analyticsHelper.logEvent( + SEARCH_BOOK_RESULT, + mapOf( + "page_index" to startIndex, + "result_count" to result.books.size, + "is_last_page" to result.lastPage + ) + ) - if (startIndex == START_INDEX && result.books.isEmpty()) { - analyticsHelper.logEvent(SEARCH_BOOK_NO_RESULT) - } + if (startIndex == START_INDEX && result.books.isEmpty()) { + analyticsHelper.logEvent( + SEARCH_BOOK_NO_RESULT, + mapOf("page_index" to startIndex) + ) + }
104-113: 에러 로깅에 에러 타입/컨텍스트 포함 제안오류 분석을 위해 예외 타입, 초기 페이지 여부 등 최소 메타데이터를 함께 로깅하세요. 원문 메시지·스택트레이스는 PII/보안 이슈가 될 수 있으니 피하고, 안전한 요약값을 권장합니다.
예시 diff(오버로드가 있다는 가정):
- analyticsHelper.logEvent(ERROR_SEARCH_LOADING) + analyticsHelper.logEvent( + ERROR_SEARCH_LOADING, + mapOf( + "error_type" to (exception::class.simpleName ?: "Unknown"), + "is_first_page" to (startIndex == START_INDEX) + ) + )
128-133: 등록 성공 이벤트에 컨텍스트 파라미터 포함 및 PII 안전성 고려등록된 상태 값 등 비식별 컨텍스트를 함께 전송하면 퍼널 분석에 유용합니다. userBookId 같은 식별자는 절대 전송하지 말고, 필요하면 존재 여부(boolean)만 전송하세요.
예시 diff(오버로드가 있다는 가정):
- analyticsHelper.logEvent(REGISTER_BOOK_COMPLETE) + analyticsHelper.logEvent( + REGISTER_BOOK_COMPLETE, + mapOf( + "book_status" to bookStatus, // upsertBook 파라미터 + "isbn_present" to selectedBookIsbn.isNotEmpty(), + "user_book_id_present" to it.userBookId.isNotBlank() + ) + )
140-153: 등록 실패 이벤트에 에러 타입/도메인 파라미터 포함 제안실패 원인 분류를 위해 예외 타입, 선택된 상태값 등을 함께 로깅하세요.
예시 diff(오버로드가 있다는 가정):
- analyticsHelper.logEvent(ERROR_REGISTER_BOOK) + analyticsHelper.logEvent( + ERROR_REGISTER_BOOK, + mapOf( + "error_type" to (exception::class.simpleName ?: "Unknown"), + "book_status" to (selectedBookStatus?.value ?: "unknown") + ) + )
178-182: 검색 입력 이벤트 로깅 위치 적절 — 추가로 ‘최근 검색 클릭’ 경로도 태깅 고려직접 입력만 집계되므로, 최근 검색 항목 클릭으로 시작된 검색도 동일 이벤트에
source=recent등으로 구분해 로깅하면 퍼널 왜곡을 줄일 수 있습니다. 쿼리 원문은 PII/규정 이슈가 있어 수집하지 않는 현 방침이 적절합니다.예시(개념):
// OnRecentSearchClick 분기 내 analyticsHelper.logEvent( SEARCH_BOOK_INPUT, mapOf("source" to "recent") )
219-222: 옵션 선택 이벤트에 선택값 파라미터 포함 제안선택한 독서 상태값을 함께 전송하여 전환 분석 품질을 높이세요.
예시 diff(오버로드가 있다는 가정):
- analyticsHelper.logEvent(REGISTER_BOOK_OPTION) + analyticsHelper.logEvent( + REGISTER_BOOK_OPTION, + mapOf("book_status" to event.bookStatus.value) + )feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt (4)
34-38: 이벤트 키 중앙 관리 제안여러 Presenter에서 동일 키를 사용할 가능성이 높으니, 공용 모듈에 AnalyticsEvents(또는 SettingsAnalytics) 같은 객체로 상수들을 중앙화하면 유지보수성이 올라갑니다. 추후 키 변경 시 단일 지점에서 반영 가능합니다.
78-82: OnBottomSheetDismissed가 로그아웃 다이얼로그까지 닫고 있음바텀시트 dismiss 이벤트에서 isLogoutDialogVisible까지 false로 만드는 것은 관심사가 섞일 여지가 있습니다. 의도치 않게 로그아웃 다이얼로그가 닫힐 수 있으므로 바텀시트 관련 상태만 재설정하는 편이 안전합니다.
아래 패치처럼 최소 수정 권장:
is SettingsUiEvent.OnBottomSheetDismissed -> { - isLogoutDialogVisible = false isWithdrawBottomSheetVisible = false isWithdrawConfirmed = false }
74-76: 분석 이벤트 호출에 대한 최소 단위 테스트 추가 권장회귀 방지용으로 AnalyticsHelper 호출 여부/키값을 검증하는 단위 테스트를 붙여두면 안전합니다. Fake AnalyticsHelper를 주입해 이벤트 키와 호출 순서를 검증하세요.
예시(요지):
- OnWithdrawClick → SETTINGS_WITHDRAWAL_WARNING 1회
- Logout 성공 → SETTINGS_LOGOUT_COMPLETE 1회
- Withdraw 성공 → SETTINGS_WITHDRAWAL_COMPLETE 1회
필요 시 제가 Fake/Spy 구현과 테스트 템플릿을 드릴 수 있습니다. 원하시면 알려주세요.
Also applies to: 94-96, 124-126
74-76: [선택] 이벤트 파라미터 확장 여지분석팀이 원한다면 이벤트에 source(예: "settings"), 결과(success/failure), 이유(reason) 등의 파라미터를 추가할 수 있도록 AnalyticsHelper에 Bundle/Map 인자를 받는 오버로드를 열어두는 것을 권장합니다. 현재 스코프에서는 필수는 아니며 확장 포인트 제안입니다.
라인 내 적용 예(Helper에 오버로드 추가 시):
-analyticsHelper.logEvent(SETTINGS_WITHDRAWAL_WARNING) +analyticsHelper.logEvent(SETTINGS_WITHDRAWAL_WARNING, mapOf("source" to "settings")) -analyticsHelper.logEvent(SETTINGS_LOGOUT_COMPLETE) +analyticsHelper.logEvent(SETTINGS_LOGOUT_COMPLETE, mapOf("source" to "settings", "result" to "success")) -analyticsHelper.logEvent(SETTINGS_WITHDRAWAL_COMPLETE) +analyticsHelper.logEvent(SETTINGS_WITHDRAWAL_COMPLETE, mapOf("source" to "settings", "result" to "success"))Helper 측(다른 파일) 보강 예시:
class AnalyticsHelper @Inject constructor( private val firebase: FirebaseAnalytics ) { fun logEvent(name: String, params: Map<String, Any?> = emptyMap()) { val bundle = Bundle().apply { params.forEach { (k, v) -> when (v) { null -> {} is String -> putString(k, v) is Int -> putInt(k, v) is Long -> putLong(k, v) is Double -> putDouble(k, v) is Float -> putFloat(k, v) is Boolean -> putString(k, v.toString()) else -> putString(k, v.toString()) } } } firebase.logEvent(name, bundle) } }Also applies to: 94-96, 124-126
📜 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 (2)
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt(7 hunks)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt(7 hunks)
⏰ 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 (10)
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt (3)
11-11: AnalyticsHelper import 도입 — LGTM계측 주입을 위한 import 추가가 적절하며 사이드이펙트 없음으로 보입니다.
42-42: Presenter 레벨 DI로 AnalyticsHelper 주입 — LGTM관심사 분리 및 테스트 용이성에 도움이 됩니다.
98-103: NO_RESULT 로깅 위치가 정확해졌습니다 — LGTM초기 페이지(START_INDEX)에서 결과가 비었을 때만 로깅하여 중복·오탐을 방지합니다. 이전 제안과 일치합니다.
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt (7)
8-8: AnalyticsHelper import 추가 적절DI로 주입된 AnalyticsHelper 사용을 위한 import가 정확합니다. 별다른 이슈 없습니다.
21-21: ImpressionEffect import 추가 OKCircuit의 ImpressionEffect 사용 준비(import) 문제 없습니다.
74-76: 회원탈퇴 경고 이벤트 로깅 위치 적절바텀시트 노출 전에 WARNING 이벤트를 먼저 적재하는 순서가 자연스럽습니다. UX/분석 의도에 부합합니다.
94-96: 로그아웃 성공 이벤트를 네비게이션 이전에 로깅 — 순서 적절성공 콜백 내에서 resetRoot 이전에 로그 적재한 점 좋습니다. 이벤트 유실 가능성을 낮춥니다.
124-126: 회원탈퇴 성공 이벤트 로깅 순서 적절성공 시 COMPLETE 이벤트 후 화면 전환으로 일관성 유지되어 있습니다.
150-152: ImpressionEffect를 통한 단일 화면 노출 로깅 패턴 적합
SettingsScreen.name이 ScreenNames.SETTINGS(= "settings_main")과 일치함을 확인했습니다. 화면명 매핑 일관성에 문제가 없습니다.
31-31: AnalyticsHelper DI 구성 및 Module 바인딩 확인 완료 — 이상 없음
core/common/src/main/kotlin/com/ninecraft/booket/core/common/analytics/AnalyticsHelper.kt에@Singleton및@Inject constructor가 적용되어 있어 Hilt가 자동으로 바인딩합니다.core/common/src/main/kotlin/com/ninecraft/booket/core/common/analytics/di/AnalyticsModule.kt에서FirebaseAnalytics를@Provides(@Singleton)로 등록하고 있으므로,AnalyticsHelper가 요구하는 의존성도 모두 해결됩니다.모든 DI 바인딩이 정상적이므로 추가 조치 없이 코드 변경을 승인합니다.
# Conflicts: # feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/record/RecordDetailPresenter.kt # feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/register/RecordRegisterPresenter.kt # feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/step/ImpressionStep.kt # feature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/Screens.kt # feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt # feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt # feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashPresenter.kt # gradle/libs.versions.toml
# Conflicts: # feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/ocr/OcrPresenter.kt
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/register/RecordRegisterPresenter.kt (1)
85-97: 페이지 입력 유효성 검사 강화 필요현재
isPageError는toIntOrNull() ?: 0후page > MAX_PAGE만 검사하여
- 비숫자 입력(예: “abc”)은
page == 0으로 간주되어 에러 처리되지 않고,- 음수 입력(예: “-1”) 또한
page == -1로 간주되어 에러 처리되지 않습니다.따라서 아래와 같이 1..MAX_PAGE 범위를 벗어나거나 비숫자일 경우 에러로 처리하도록 수정이 필요합니다.
대상 위치:
- 파일:
feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/register/RecordRegisterPresenter.kt- 라인: 85–97 (상기
isPageError정의)제안 수정안:
- val isPageError by remember { - derivedStateOf { - val page = recordPageState.text.toString().toIntOrNull() ?: 0 - page > MAX_PAGE - } - } + val isPageError by remember { + derivedStateOf { + val page = recordPageState.text.toString().toIntOrNull() + // 비숫자(null) 또는 유효 범위를 벗어날 경우 에러(true) + page == null || page !in 1..MAX_PAGE + } + }추가로,
postRecord호출부에도 동일한 1..MAX_PAGE 유효성 검사를 중복 적용하여
서버로 잘못된 페이지 번호가 전송되지 않도록 방어적 코드를 권장합니다.
♻️ Duplicate comments (3)
feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/book/BookDetailPresenter.kt (3)
397-399: 스크린 뷰 로깅 표준화(중복 제안) — event+params 포맷으로 통일 권장대시보드에서 화면 노출을 단일 이벤트로 집계하려면
logScreenView(name)대신logEvent("screen_view", params)포맷으로 맞추는 편이 안정적입니다. 이전 코멘트와 동일 제안입니다.ImpressionEffect { - analyticsHelper.logScreenView(screen.name) + analyticsHelper.logEvent( + "screen_view", + mapOf("screen_name" to screen.name) + ) }
334-338: 기록 삭제 트리거에서 ‘도서 삭제’ 이벤트 사용 — 의미 오류 및 과거 코멘트 중복기록 삭제 메뉴 진입에
BOOK_DELETE를 쓰고 있습니다. 이는 “도서 삭제”용 키라서 분석이 왜곡됩니다. 기록 삭제 전용 키로 교체하고, 실제 삭제 성공 시 완료 이벤트도 추가해 주세요.case is BookDetailUiEvent.OnDeleteRecordClick -> { isRecordMenuBottomSheetVisible = false isRecordDeleteDialogVisible = true - analyticsHelper.logEvent(BOOK_DELETE) + analyticsHelper.logEvent(RECORD_DELETE) } // 삭제 API 성공 시점에도 완료 이벤트 로깅 @@ fun deleteRecord(readingRecordId: String, onSuccess: () -> Unit) { scope.launch { recordRepository.deleteRecord(readingRecordId = readingRecordId) .onSuccess { + analyticsHelper.logEvent(RECORD_DELETE_COMPLETE) onSuccess() }
56-58: 이벤트 키 의미 불일치: 기록 삭제 플로우에 ‘도서 삭제’ 키 사용 중
BOOK_DELETE,BOOK_DELETE_COMPLETE는 “도서 삭제”를 뜻합니다. 이후 핸들러에서 기록 삭제(Record) 단계에 이 키가 쓰여 의미가 뒤섞입니다. 기록 삭제 전용 키를 분리해 주세요.companion object { private const val PAGE_SIZE = 20 private const val START_INDEX = 0 - private const val BOOK_DELETE = "library_book_delete" - private const val BOOK_DELETE_COMPLETE = "library_book_delete_complete" + // Book + private const val BOOK_DELETE = "library_book_delete" + private const val BOOK_DELETE_COMPLETE = "library_book_delete_complete" + // Record + private const val RECORD_DELETE = "library_record_delete" + private const val RECORD_DELETE_COMPLETE = "library_record_delete_complete" }
🧹 Nitpick comments (7)
feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/book/BookDetailPresenter.kt (2)
221-223: 도서 삭제 퍼널 보강: 시작/완료 이벤트 모두 로깅완료 시점(
BOOK_DELETE_COMPLETE)만 로깅되고 있어 클릭→확인→완료까지의 전환율을 보기가 어렵습니다. 시작 시점에도 이벤트를 쌓아 퍼널을 닫아주세요. PR 설명상 네트워크 에러 태깅은 제외했으므로 실패 태깅은 선택사항으로 둡니다.case is BookDetailUiEvent.OnDeleteBookClick -> { isDetailMenuBottomSheetVisible = false isBookDeleteDialogVisible = true + // 사용자 의도(삭제 시도) 로깅 + analyticsHelper.logEvent(BOOK_DELETE) } case is BookDetailUiEvent.OnDeleteBook -> { isBookDeleteDialogVisible = false deleteBook( userBookId = screen.userBookId, onSuccess = { - navigator.pop() + navigator.pop() }, ) } // 성공 시점은 유지 .onSuccess { analyticsHelper.logEvent(BOOK_DELETE_COMPLETE) onSuccess() }검증 포인트
- 대시보드 상 “삭제 시도 → 삭제 완료” 전환율 확인이 목적이라면 위 2개 이벤트만으로도 OK.
- 이벤트 명세가 PM과 합의된 snake_case/namespace 규약과 일치하는지 최종 확인 바랍니다.
Also applies to: 364-367, 373-381
56-58: 이벤트 키 중앙화 제안: 오타/불일치 방지여러 Presenter에서 상수로 이벤트 키를 중복 선언하면 오타·불일치 위험이 큽니다.
core/common/analytics/AnalyticsEvents(object or enum/sealed)로 키를 중앙화하고, 타입 세이프하게 참조하도록 리팩터링을 검토해 주세요.원하시면 이벤트 키 레지스트리 초안과 사용 예시까지 포함해 PR에 소규모 리팩터를 추가해 드리겠습니다.
Also applies to: 334-338, 194-201
feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/register/RecordRegisterPresenter.kt (5)
44-53: Analytics 키를 로컬 companion object에 두기보다 중앙 집중화를 권장화면/이벤트 이름이 분산되면 오타·불일치로 분석 지표가 분절될 수 있습니다. 공용 객체(예: AnalyticsScreenNames/ScreenNames/Events 등)로 모으고 여기서는 참조만 하도록 리팩터링을 제안합니다.
135-140: 성공/실패 이벤트에 맥락 파라미터를 추가해 분석 품질을 높이세요현재는 이벤트 이름만 로깅합니다. AnalyticsHelper가 지원한다면, 비식별 파라미터를 함께 보내면 유용합니다(PII/콘텐츠 원문은 금지).
예시:
- RECORD_COMPLETE: { source: "record_register", hasEmotion: true/false, stepCount: 3 }
- ERROR_RECORD_SAVE: { error_class: exception::class, isNetwork: true/false }
민감정보/콘텐츠(quote, impression 텍스트 등)는 절대 전송하지 않도록 주의해 주세요.
202-202: BottomSheet를 screen_view로 집계할지 정책 확인 필요BottomSheet(도움말 선택)는 일반적으로 화면 전환이 아닌 UI 요소로 분류하는 경우가 많습니다. 조직의 측정 설계가 BottomSheet도 별도 screen_view로 본다면 OK, 아니라면 이벤트(logEvent)로 전환하는 편이 일관됩니다.
282-289: ImpressionEffect로 단계별 screen_view 로깅: 구현은 적절합니다. 소량의 메타 파라미터 추가를 고려하세요현재 step에 따라 이름만 기록합니다. 파라미터 지원 시 { step: "QUOTE" | "EMOTION" | "IMPRESSION", step_index: 0/1/2 } 같은 맥락을 포함하면 퍼널 분석이 수월합니다. 재조합에 의한 중복 호출은 ImpressionEffect가 방지하지만, 매우 빠른 step 왕복에서 중복이 문제되면 로컬 lastLoggedStep을 기억해 추가 방지할 수 있습니다.
267-271: RecordDetail 화면 로그 중복 방지 리팩토링 제안
RecordRegisterPresenter.kt에서 화면 전환 시analyticsHelper.logScreenView(RECORD_DETAIL)를 호출하고,
이후RecordDetailPresenter.kt에서도 다시logScreenView(screen.name)를 호출하여 동일한 화면 노출이 중복 집계될 수 있습니다.
일반적인 패턴에 맞게 “네비게이션 이벤트는 이벤트로, 실제 화면 진입 시에만 screen_view 로깅”하도록 분리할 것을 권장드립니다.• 변경 대상
- feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/register/RecordRegisterPresenter.kt (267행)
- feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/record/RecordDetailPresenter.kt (180행)
• 제안하는 수정 예시
--- a/feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/register/RecordRegisterPresenter.kt +++ b/feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/register/RecordRegisterPresenter.kt @@ 264,7c264,7 - analyticsHelper.logScreenView(RECORD_DETAIL) + analyticsHelper.logEvent(NAVIGATE_RECORD_DETAIL) isRecordSavedDialogVisible = false navigator.pop() navigator.goTo(RecordDetailScreen(event.recordId))• companion object에 네비게이션 이벤트 상수 추가
--- a/feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/register/RecordRegisterPresenter.kt +++ b/feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/register/RecordRegisterPresenter.kt @@ companion object { private const val ERROR_RECORD_SAVE = "error_record_save" + private const val NAVIGATE_RECORD_DETAIL = "navigate_record_detail" }이렇게 구성하면
- 네비게이션 발생 시
navigate_record_detail이벤트만 기록- 실제
RecordDetailPresenter진입 시에만screen_view = record_detail기록
되어 중복 집계 없이 명확한 로그를 남길 수 있습니다.
📜 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 (10)
core/common/build.gradle.kts(1 hunks)feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/book/BookDetailPresenter.kt(6 hunks)feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/record/RecordDetailPresenter.kt(6 hunks)feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/register/RecordRegisterPresenter.kt(7 hunks)feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/step/ImpressionStep.kt(2 hunks)feature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/Screens.kt(2 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt(6 hunks)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt(4 hunks)feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashPresenter.kt(3 hunks)gradle/libs.versions.toml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- core/common/build.gradle.kts
- feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashPresenter.kt
- feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt
- feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt
- feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/step/ImpressionStep.kt
- feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/record/RecordDetailPresenter.kt
- feature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/Screens.kt
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-31T23:30:37.547Z
Learnt from: easyhooon
PR: YAPP-Github/Reed-Android#88
File: feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/record/RecordDetailPresenter.kt:18-47
Timestamp: 2025-07-31T23:30:37.547Z
Learning: In Circuit architecture, presenters receive the Screen object directly as a constructor parameter (e.g., Assisted private val screen: RecordDetailScreen), and screen parameters are accessed through this screen object (e.g., screen.recordId). Screen parameters should not be added as separate constructor parameters.
Applied to files:
feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/register/RecordRegisterPresenter.kt
🔇 Additional comments (4)
gradle/libs.versions.toml (1)
75-75: 검증 완료: Firebase BoM(34.1.0) 적용 여부 확인됨
- 스크립트 실행 결과, Firebase 의존성(analytics, crashlytics, remote-config)을 사용하는 모듈은 오직
core/common/build.gradle.kts였으며, 해당 파일에서platform(libs.firebase.bom)선언이 정상적으로 적용되어 있음을 확인했습니다.- 그 외 모듈에는 Firebase 의존성이 없어 별도 BoM 선언이 필요하지 않습니다.
따라서 모든 Firebase 의존성 사용 모듈에 BoM 선언이 올바르게 적용되어 있으므로, 추가 조치나 수정은 필요 없습니다.
feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/book/BookDetailPresenter.kt (1)
46-52: AnalyticsHelper DI 주입 LGTMPresenter 레벨에서 Helper 주입 방식 일관성 유지되어 좋습니다.
feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/register/RecordRegisterPresenter.kt (2)
13-13: AnalyticsHelper 주입/임포트 추가는 적절합니다Hilt Assisted 주입 관례에 맞고, Presenter에선 화면/네비만 Assisted로 받고 나머지는 DI에서 공급되므로 스코프 충돌 우려도 없어 보입니다.
Also applies to: 41-41
29-29: ImpressionEffect 도입 적절CircuitX의 ImpressionEffect 사용으로 단계 전환 시점에만 노출 로그가 찍히도록 한 점 좋습니다.
| firebase-analytics = { group = "com.google.firebase", name = "firebase-analytics" } | ||
| firebase-crashlytics = { group = "com.google.firebase", name = "firebase-crashlytics" } | ||
| firebase-remote-config = { group = "com.google.firebase", name = "firebase-config" } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
KTX → non-KTX 전환: KTX 확장 API 사용 여부 확인 필요
현재 카탈로그가 firebase-analytics-ktx, firebase-crashlytics-ktx, firebase-config-ktx 대신 non-KTX 아티팩트를 가리키고 있습니다. 코드에서 아래 KTX 확장/프로퍼티를 사용 중이라면 컴파일 에러가 납니다:
Firebase.analytics,Firebase.crashlytics,Firebase.remoteConfig- KTX 제공 확장 함수들
두 가지 중 하나로 정리해 주세요.
옵션 A) KTX API를 사용한다면 의존성 되돌리기:
-firebase-analytics = { group = "com.google.firebase", name = "firebase-analytics" }
-firebase-crashlytics = { group = "com.google.firebase", name = "firebase-crashlytics" }
-firebase-remote-config = { group = "com.google.firebase", name = "firebase-config" }
+firebase-analytics = { group = "com.google.firebase", name = "firebase-analytics-ktx" }
+firebase-crashlytics = { group = "com.google.firebase", name = "firebase-crashlytics-ktx" }
+firebase-remote-config = { group = "com.google.firebase", name = "firebase-config-ktx" }옵션 B) non-KTX 유지 시 코드도 non-KTX API로 교체:
- Analytics:
FirebaseAnalytics.getInstance(context) - Crashlytics:
FirebaseCrashlytics.getInstance() - RemoteConfig:
FirebaseRemoteConfig.getInstance()
빠른 확인용 스크립트:
🏁 Script executed:
#!/bin/bash
# KTX 전용 API 사용 여부 검사
rg -n -C1 -g '!**/build/**' -g '!**/.gradle/**' -e '\bFirebase\.(analytics|crashlytics|remoteConfig)\b' || trueLength of output: 905
libs.versions.toml 내 Firebase KTX 의존성 일관성 확인 필요
코드에서 아래 KTX 확장 프로퍼티를 사용 중이므로, non-KTX 아티팩트를 그대로 두면 컴파일 에러가 발생합니다.
- core/common/src/main/kotlin/com/ninecraft/booket/core/common/analytics/di/AnalyticsModule.kt (19행):
return Firebase.analytics - core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/di/FirebaseModule.kt (20행):
return Firebase.remoteConfig…
두 가지 중 하나를 선택해 주세요.
옵션 A) KTX API 유지: gradle/libs.versions.toml을 KTX 아티팩트로 롤백
-firebase-analytics = { group = "com.google.firebase", name = "firebase-analytics" }
+firebase-analytics = { group = "com.google.firebase", name = "firebase-analytics-ktx" }
-firebase-remote-config = { group = "com.google.firebase", name = "firebase-config" }
+firebase-remote-config = { group = "com.google.firebase", name = "firebase-config-ktx" }(※ firebase-crashlytics는 코드 내 KTX 프로퍼티 사용 흔적이 없으므로 현 상태 유지 가능)
옵션 B) non-KTX API 전환: 코드에서 KTX 호출을 non-KTX 메서드로 변경
- Analytics:
FirebaseAnalytics.getInstance(context) - RemoteConfig:
FirebaseRemoteConfig.getInstance()
필요한 방향으로 정리해 주세요.
📝 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.
| firebase-analytics = { group = "com.google.firebase", name = "firebase-analytics" } | |
| firebase-crashlytics = { group = "com.google.firebase", name = "firebase-crashlytics" } | |
| firebase-remote-config = { group = "com.google.firebase", name = "firebase-config" } | |
| firebase-analytics = { group = "com.google.firebase", name = "firebase-analytics-ktx" } | |
| firebase-crashlytics = { group = "com.google.firebase", name = "firebase-crashlytics" } | |
| firebase-remote-config = { group = "com.google.firebase", name = "firebase-config-ktx" } |
🤖 Prompt for AI Agents
In gradle/libs.versions.toml around lines 159-161, the project uses KTX
extension properties in code, so restore KTX artifacts: replace
firebase-analytics and firebase-remote-config entries with their KTX equivalents
(firebase-analytics-ktx and firebase-config-ktx) while leaving
firebase-crashlytics unchanged; then run Gradle sync and fix imports if needed
to ensure Firebase.analytics and Firebase.remoteConfig resolve.
versionName: 1.0.0 -> 1.1.0 versionCode: 3 -> 4
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt (6)
37-41: 이벤트 키 상수화 OK + 공용 정의로의 이전을 검토
- 지난 코멘트의 오타 이슈가 해결되었고(SETTINGS_WITHDRAWAL_COMPLETE 값 수정), 현재 키 네이밍도 일관적입니다.
- 다만 각 Presenter에 흩어진 이벤트 키는 오탈자/중복 위험이 있습니다. core/common 쪽(예: AnalyticsEvents.Settings)으로 중앙화하면 추후 유지보수가 쉬워집니다.
아래와 같이 공용 객체로 이동하는 방안을 제안합니다(예시):
// core/common/analytics/AnalyticsEvents.kt object AnalyticsEvents { object Settings { const val LOGOUT_COMPLETE = "settings_logout_complete" const val WITHDRAWAL_COMPLETE = "settings_withdrawal_complete" const val WITHDRAWAL_WARNING = "settings_withdrawal_warning" // (선택) 실패 이벤트 등도 여기에서 관리 } }그리고 본 파일에서는:
- private const val SETTINGS_LOGOUT_COMPLETE = "settings_logout_complete" - private const val SETTINGS_WITHDRAWAL_COMPLETE = "settings_withdrawal_complete" - private const val SETTINGS_WITHDRAWAL_WARNING = "settings_withdrawal_warning" + // AnalyticsEvents.Settings.* 로 참조
90-92: 탈퇴 성공 로깅 OK — 실패 시 진단용 이벤트도 고려성공 로깅은 적절합니다. 추가로 실패 케이스(onFailure)에도 진단용 이벤트를 남기면 운영/분석에 유용합니다(예: 네트워크/권한/서버 에러 비율 파악).
아래와 같이 실패 이벤트를 보강하는 패치를 제안합니다(키는 예시, 상수화 권장):
authRepository.withdraw() .onSuccess { analyticsHelper.logEvent(SETTINGS_WITHDRAWAL_COMPLETE) navigator.resetRoot(LoginScreen) } .onFailure { exception -> + analyticsHelper.logEvent("settings_withdrawal_failed") val handleErrorMessage = { message: String -> Logger.e(message) sideEffect = SettingsSideEffect.ShowToast(message) }동일 아이디어를 로그아웃 실패에도 적용할 수 있습니다.
168-170: 경고(바텀시트 오픈) 시점 로깅 적절 — 다중 오픈 중복 로깅은 의도인지 확인 권장바텀시트 진입 시 경고 이벤트를 남기는 포인트는 직관적입니다. UX 상 동일 세션에서 여러 번 열릴 수 있다면, 중복 로깅이 의도인지(혹은 1회만 로깅할지) 정책 확인을 권장합니다.
200-202: ImpressionEffect로 화면 뷰 로깅: 위치·타이밍 적절화면 진입 시점 로깅이 명확합니다. 팀 내 표준이 ScreenNames.SETTINGS 같은 중앙화된 상수를 쓰는 것이라면, SettingsScreen.name 대신 해당 상수를 사용해 전역 일관성을 맞추는 것도 고려해 주세요.
64-68: 에러 메시지 노출 정책 점검(사용자 토스트 및 로그)현재 handleErrorMessage가 전달받은 message를 그대로 Logger 및 사용자 토스트로 노출합니다. handleException이 이미 사용자 친화적·비식별화 메시지를 보장한다면 문제 없지만, 예외 원문이 전달될 수 있다면 민감 정보/내부 상세가 노출될 우려가 있습니다. 사용자 메시지와 내부 로그 메시지를 분리하거나, 공통 에러 매퍼를 통해 정제된 메시지만 UI에 노출하는지 확인 바랍니다.
Also applies to: 94-98, 123-126
51-53: 미사용isOptionalUpdateDialogVisible상태 제거 권장
- SettingsUiState.kt (15행):
val isOptionalUpdateDialogVisible: Boolean = false선언- SettingsPresenter.kt (51행):
var isOptionalUpdateDialogVisible by rememberRetained { mutableStateOf(false) }선언- SettingsPresenter.kt (210행): UIState 매핑 시
isOptionalUpdateDialogVisible = isOptionalUpdateDialogVisible사용위 플래그는 레포 전체에서 토글되거나 실제 소비되는 곳이 없어 항상 기본값(false)으로만 전달됩니다. 다이얼로그 표시 기능이 더 이상 필요치 않다면 관련 선언과 매핑을 제거해 코드 단순화를 권장합니다.
반대로 향후 사용 의도가 있다면, 의도를 설명하는 주석을 추가해 주세요.
📜 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 (7)
feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/ocr/OcrPresenter.kt(4 hunks)feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/ocr/OcrUi.kt(2 hunks)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt(7 hunks)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsUi.kt(0 hunks)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsUiState.kt(0 hunks)feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashPresenter.kt(4 hunks)gradle/libs.versions.toml(2 hunks)
💤 Files with no reviewable changes (2)
- feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsUi.kt
- feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsUiState.kt
🚧 Files skipped from review as they are similar to previous changes (4)
- feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/ocr/OcrUi.kt
- feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashPresenter.kt
- gradle/libs.versions.toml
- feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/ocr/OcrPresenter.kt
⏰ 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 (2)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt (2)
30-35: AnalyticsHelper DI 주입 적절 — PR 목적에 부합Presenter 레벨에서 AnalyticsHelper를 주입해 화면·이벤트 로깅을 담당하게 한 방향성 좋습니다. 테스트 용이성과 교차 화면 일관성에 도움이 됩니다.
60-62: 로그아웃 성공 시 이벤트 선로깅 배치 좋습니다성공 분기에서 Navigation 이전에 logEvent 호출되어 드랍 가능성을 줄였습니다. 현재 수준이면 충분합니다.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/card/RecordCardPresenter.kt (3)
25-28: 이벤트 키를 전역 상수(또는 타입세이프 enum)로 중앙화 제안현재 파일 로컬
companion object상수는 시작은 좋지만, 여러 화면에서 유사 이벤트가 생기면 오타/중복 위험이 큽니다.core/common/analytics/Events.kt(가칭)로 이동하거나 sealed class/enum으로 타이핑을 권장합니다.
예:AnalyticsEvent.RecordCard.Save,AnalyticsEvent.RecordCard.Share
61-65: 공유 채널/결과 파라미터 확장 고려공유는 채널(예: 시스템 공유 시트 선택 결과)과 성공/취소 여부가 분석에 중요합니다.
logEvent(RECORD_CARD_SHARE, params)형태로channel,result(success|cancel|error) 같은 경량 파라미터를 추가할 여지를 남겨두면 이후 리포트 구성이 수월합니다. PII(문구 전문/도서명 등)는 전송하지 않는 방향을 유지하는 게 좋습니다.
69-72: ImpressionEffect 사용 OK. key 지정(선택)으로 중복 방지 강화 가능
ImpressionEffect만으로도 최초 컴포지션 시 1회 호출되지만, 동일 컴포지션 트리 내에서 다른RecordCardScreen인스턴스로 교체되는 경우를 대비해 key를 부여하는 방법도 고려해 볼 수 있습니다. 예)ImpressionEffect(/* key = */ screen.name) { ... }
라이브러리 시그니처와 팀 규칙에 따라 선택 사항입니다.feature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/Screens.kt (2)
34-35: 일부 화면명이 여전히 리터럴 문자열입니다 — 일관성 정리 권장
OssLicensesScreen,RecordEditScreen,EmotionEditScreen,WebViewScreen이 네 곳만 아직 리터럴을 사용하고 있습니다. 가능하면ScreenNames로 편입해 일관성을 유지하세요. (없다면ScreenNames에 상수 추가)예시(diff는 ScreenNames에 상수가 추가된다는 가정하):
- data object OssLicensesScreen : ReedScreen(name = "OssLicenses()") + data object OssLicensesScreen : ReedScreen(name = ScreenNames.OSS_LICENSES) - data class RecordEditScreen(val recordInfo: RecordEditArgs) : ReedScreen(name = "RecordEdit()") + data class RecordEditScreen(val recordInfo: RecordEditArgs) : ReedScreen(name = ScreenNames.RECORD_EDIT) - data class EmotionEditScreen(val emotion: String) : ReedScreen(name = "EmotionEdit()") { + data class EmotionEditScreen(val emotion: String) : ReedScreen(name = ScreenNames.EMOTION_EDIT) { - ) : ReedScreen(name = "WebView()") + ) : ReedScreen(name = ScreenNames.WEB_VIEW)Also applies to: 49-53, 58-62
70-70: 온보딩 화면명 변경에 따른 대시보드 영향 점검
ScreenNames.ONBOARDING의 실제 문자열 값이 과거 값(예:onboarding_1_search)에서 변경되었다면, 기존 리포트가 분리될 수 있습니다. 의도된 버전업이라면 OK이고, 아니라면 GA에서 alias 처리(커스텀 차트), 또는 일시적으로 두 이름을 이벤트 파라미터로 병기하는 방법도 있습니다.필요하시면 전체 리포트 영향 범위를 빠르게 점검하는 체크리스트/SQL을 정리해드릴게요.
📜 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 (3)
feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/card/RecordCardPresenter.kt(3 hunks)feature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/ScreenNames.kt(1 hunks)feature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/Screens.kt(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- feature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/ScreenNames.kt
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-31T23:30:37.547Z
Learnt from: easyhooon
PR: YAPP-Github/Reed-Android#88
File: feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/record/RecordDetailPresenter.kt:18-47
Timestamp: 2025-07-31T23:30:37.547Z
Learning: In Circuit architecture, presenters receive the Screen object directly as a constructor parameter (e.g., Assisted private val screen: RecordDetailScreen), and screen parameters are accessed through this screen object (e.g., screen.recordId). Screen parameters should not be added as separate constructor parameters.
Applied to files:
feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/card/RecordCardPresenter.ktfeature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/Screens.kt
⏰ 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 (6)
feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/card/RecordCardPresenter.kt (3)
20-23: AssistedInject 구성 적절합니다 + DI 스코프만 확인 부탁드려요
@Assisted가 필요한screen,navigator만 Assisted로 받고AnalyticsHelper는 Hilt 주입으로 받는 패턴이 Circuit 표준과 일치합니다. 굿.- 단,
AnalyticsHelper제공 모듈의 스코프가ActivityRetainedComponent이상(예:SingletonComponent)으로 맞춰져 있는지 한 번만 확인해 주세요. Presenter 수명과 어긋나면 재생성 시 인스턴스가 달라질 수 있습니다.
55-59: Save 이벤트의 의미(시도 vs 성공) PM 명세 확인 필요
SaveRecordCard에서 이벤트를 쏘고 있는데, 이 시점이 “저장 시도”인지 “저장 성공”인지 명확히 합의되어야 리텐션/퍼널이 왜곡되지 않습니다. 저장 실패 케이스(권한 거부, I/O 오류 등)도 향후 요구사항에 따라..._fail이벤트를 분리할지 확인 부탁드립니다.
7-7: 분석 도입을 위한 import 추가, 문제 없습니다
AnalyticsHelper,ImpressionEffectimport 추가는 변경 의도와 일치하며 불필요한 의존성 증가는 보이지 않습니다.Also applies to: 13-13
feature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/Screens.kt (3)
13-31: 화면명 상수(ScreenNames)로의 일괄 전환 잘 되었습니다주요 화면의
name을ScreenNames로 통일해 GA 대시보드 차트/필터 일관성이 좋아집니다. 다른 모듈에서도 동일 패턴 유지 부탁드립니다.Also applies to: 37-47, 67-74, 80-80
37-37: ID 파라미터와 PII 가이드 재확인(선택)
RecordScreen(userBookId),BookDetailScreen(userBookId, isbn13)등 식별자가 존재하는 화면은, 화면뷰 로그에는 이름만 보내고 식별자는 이벤트 파라미터로도 보내지 않는 현재 정책을 유지하는지 확인해 주세요. 내부 분석이 꼭 필요하면 해시/버킷 처리 등 비식별화 후 전달을 권장합니다.Also applies to: 64-68
8-10: toString()가 화면명에 매핑되는 설계는 유지해 주세요로그나 디버깅 시 화면명이 곧
toString()으로 표기되므로, 화면명 변경이 문자열 비교 로직에 영향을 주지 않도록 주의가 필요합니다. 현재 변경 범위에서는 문제 없어 보입니다.
| data object HomeScreen : ReedScreen(name = ScreenNames.HOME) | ||
|
|
||
| @Parcelize | ||
| data object LibraryScreen : ReedScreen(name = "Library()") | ||
| data object LibraryScreen : ReedScreen(name = ScreenNames.LIBRARY) | ||
|
|
||
| @Parcelize | ||
| data object LoginScreen : ReedScreen(name = "Login()") | ||
| data object LoginScreen : ReedScreen(name = ScreenNames.LOGIN) | ||
|
|
||
| @Parcelize | ||
| data object SearchScreen : ReedScreen(name = "Search()") | ||
| data object SearchScreen : ReedScreen(name = ScreenNames.SEARCH) | ||
|
|
||
| @Parcelize | ||
| data object LibrarySearchScreen : ReedScreen(name = "LibrarySearch()") | ||
| data object LibrarySearchScreen : ReedScreen(name = ScreenNames.LIBRARY_SEARCH) | ||
|
|
||
| @Parcelize | ||
| data object TermsAgreementScreen : ReedScreen(name = "TermsAgreement()") | ||
| data object TermsAgreementScreen : ReedScreen(name = ScreenNames.TERMS_AGREEMENT) | ||
|
|
||
| @Parcelize | ||
| data object SettingsScreen : ReedScreen(name = "Settings()") | ||
| data object SettingsScreen : ReedScreen(name = ScreenNames.SETTINGS) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
마이그레이션 누락 여부 빠른 점검 스크립트
프로젝트 전체에서 리터럴 화면명이 남아있는 위치를 스캔해볼 수 있는 스크립트입니다. (리뷰 참고용)
Also applies to: 37-47, 67-74, 80-80
🏁 Script executed:
#!/bin/bash
# 리터럴 화면명 사용 흔적 점검
rg -nP -C2 '\bReedScreen\s*\(\s*name\s*=\s*"[^"]+"\s*\)'
# ScreenNames 사용 현황 샘플
rg -nP -C1 '\bReedScreen\s*\(\s*name\s*=\s*ScreenNames\.'
# 과거 온보딩 화면명 흔적 검사(있다면 대시보드 영향 검토)
rg -n 'onboarding_1_search|onboarding' --glob '!*build*' -C2Length of output: 25487
리터럴 화면명 마이그레이션 필요: ReedScreen(name = …)에 ScreenNames 상수 사용
스크립트 검사 결과, 아래 화면들이 여전히 문자열 리터럴로 지정되어 있어 ScreenNames 상수로 교체해야 합니다.
- feature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/Screens.kt
• 34행:OssLicensesScreen : ReedScreen(name = "OssLicenses()")→ScreenNames.OSS_LICENSES
• 49행:RecordEditScreen(val recordInfo: RecordEditArgs) : ReedScreen(name = "RecordEdit()")→ScreenNames.RECORD_EDIT
• 52행:EmotionEditScreen(val emotion: String) : ReedScreen(name = "EmotionEdit()")→ScreenNames.EMOTION_EDIT
• 61행:WebViewScreen(url: String, title: String) : ReedScreen(name = "WebView()")→ScreenNames.WEB_VIEW
위 네 곳에 대응하는 상수를 ScreenNames.kt에 추가하고, 리터럴을 상수 참조로 교체해 주세요.
🤖 Prompt for AI Agents
In
feature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/Screens.kt
(addresses occurrences around lines 34, 49, 52, 61), replace the hard-coded name
string literals used in ReedScreen(...) with the corresponding ScreenNames
constants (OSS_LICENSES, RECORD_EDIT, EMOTION_EDIT, WEB_VIEW): add those four
constants to ScreenNames.kt and update the four ReedScreen(name = "...")
instances to use ScreenNames.OSS_LICENSES, ScreenNames.RECORD_EDIT,
ScreenNames.EMOTION_EDIT, and ScreenNames.WEB_VIEW respectively so all screen
names reference the shared constants.
2 participants
🔗 관련 이슈
📙 작업 설명
🧪 테스트 내역 (선택)
📸 스크린샷 또는 시연 영상 (선택)
💬 추가 설명 or 리뷰 포인트 (선택)
Summary by CodeRabbit
신기능
버그 수정
작업