Conversation
WalkthroughFirebase Remote Config를 도입해 최신/최소 버전 정보를 조회하고, 스플래시에서 강제 업데이트 판단, 설정에서 최신 버전 표시 및 선택적 업데이트 다이얼로그와 스토어 이동 사이드이펙트를 추가했습니다. 빌드 로직은 버전 카탈로그로 이전되고 BuildConfig 필드가 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as SettingsUi
participant P as SettingsPresenter
participant RC as RemoteConfigRepository
participant FRC as FirebaseRemoteConfig
participant PS as Play Store
UI->>P: Composition
activate P
P->>RC: getLatestVersion()
RC->>FRC: fetchAndActivate()
FRC-->>RC: success + LatestVersion
RC-->>P: Result(success: latestVersion)
P-->>UI: state.latestVersion 업데이트
UI->>P: OnVersionClick
P-->>UI: isOptionalUpdateDialogVisible = true
UI->>P: OnUpdateButtonClick
P-->>UI: sideEffect = NavigateToPlayStore
UI->>PS: openPlayStore()
deactivate P
sequenceDiagram
participant U as SplashUi
participant S as SplashPresenter
participant RC as RemoteConfigRepository
participant FRC as FirebaseRemoteConfig
participant PS as Play Store
participant Nav as Navigator
U->>S: Composition / readiness
S->>RC: shouldUpdate()
RC->>FRC: fetchAndActivate()
alt 업데이트 필요
RC-->>S: Result(success: true)
S-->>U: isForceUpdateDialogVisible = true
U->>S: OnUpdateButtonClick
S-->>U: sideEffect = NavigateToPlayStore
U->>PS: openPlayStore()
else 불필요/실패
RC-->>S: Result(false) or failure
S->>Nav: proceedToNextScreen()
end
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: 3
🧹 Nitpick comments (10)
core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/RemoteCofigRepository.kt (2)
3-5: 공용 인터페이스에서 kotlin.Result 노출은 신중 권장kotlin.Result는 에러 정보를 구조적으로 담기 어렵고, API 경계에서의 사용을 팀 내에서 지양하는 경우가 많습니다. 도메인 표준(Result/Either/Resource 등) 또는
sealed class기반의 에러 모델을 검토해 보세요.가능한 대안 예시(참고용):
sealed interface RemoteConfigResult { data class Success(val latestVersion: String) : RemoteConfigResult data class Error(val cause: Throwable) : RemoteConfigResult }interface RemoteConfigRepository { suspend fun getLatestVersion(): RemoteConfigResult }
1-5: 파일명 오탈자 수정 권장: RemoteCofigRepository.kt → RemoteConfigRepository.kt
스크립트 확인 결과, 코드 전반에서 올바른 타입명 RemoteConfigRepository만 사용 중이며 패키지 네임스페이스도 모두 com.ninecraft.booket으로 일관되어 있음을 확인했습니다.
- 변경 대상
- core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/RemoteCofigRepository.kt → RemoteConfigRepository.kt
위 파일명만 수정하시면 검색성과 유지보수성이 개선됩니다.
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/di/FirebaseModule.kt (2)
21-26: 불필요한 lazy 사용 제거 제안
configSettings는 즉시 한 번만 생성해 사용하는 값이라by lazy가 이점이 없습니다. 간결하게 변경 가능합니다.- val configSettings by lazy { - remoteConfigSettings { - minimumFetchIntervalInSeconds = if (BuildConfig.DEBUG) 0 else 60 - } - } - setConfigSettingsAsync(configSettings) + val configSettings = remoteConfigSettings { + minimumFetchIntervalInSeconds = if (BuildConfig.DEBUG) 0 else 60 + } + setConfigSettingsAsync(configSettings)
23-23: 프로덕션 fetch interval(60초) 재검토 권장릴리즈에서 60초는 다소 공격적인 설정입니다(네트워크/쿼터 소모 증가). Firebase 권장치는 보통 1시간(3600초) 이상입니다. 의도된 설정인지 확인 부탁드립니다. 의도치 않았다면 아래처럼 조정하세요.
- minimumFetchIntervalInSeconds = if (BuildConfig.DEBUG) 0 else 60 + minimumFetchIntervalInSeconds = if (BuildConfig.DEBUG) 0 else 3600feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsUiState.kt (1)
26-28: 이벤트 네이밍 가독성 제안: InitSideEffect → ClearSideEffect의도가 “초기화”보다는 “초기화(=제거)”에 가깝습니다.
ClearSideEffect가 더 자명합니다. 선택 사항입니다.- data object InitSideEffect : SettingsUiEvent + data object ClearSideEffect : SettingsUiEvent참고: 호출부와 when 분기에서도 동일한 이름으로 변경 필요.
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/HandlingSettingsSideEffect.kt (1)
15-27: sideEffect 참조를 지역 변수로 캐싱하여 재평가/널 체크 중복 최소화동일 스냅샷 내에서
state.sideEffect를 두 번 이상 참조하는 대신 지역 변수에 담아 사용하면 가독성과 안정성이 조금 개선됩니다. 기능 영향은 없습니다.- RememberedEffect(state.sideEffect) { - when (state.sideEffect) { + RememberedEffect(state.sideEffect) { + val effect = state.sideEffect + when (effect) { is SettingsSideEffect.ShowToast -> { - Toast.makeText(context, state.sideEffect.message, Toast.LENGTH_SHORT).show() + Toast.makeText(context, effect.message, Toast.LENGTH_SHORT).show() } null -> {} } - if (state.sideEffect != null) { - eventSink(SettingsUiEvent.InitSideEffect) + if (effect != null) { + eventSink(SettingsUiEvent.InitSideEffect) } }feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt (1)
48-50: 빈 문자열 방어 로직 (선택 사항)Remote Config에서 빈 값이 내려오면 UI에 공백이 노출될 수 있습니다. 기존 값 유지 또는 플레이스홀더로 대체하는 방어 로직을 고려해보세요.
- .onSuccess { version -> - latestVersion = version - } + .onSuccess { version -> + latestVersion = version.takeIf { it.isNotBlank() } ?: latestVersion + }feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsUi.kt (2)
133-139: 최신 버전 값이 비어있을 때(초기/실패) 설명 텍스트 노출 가드 권장Remote Config 결과가 비어 있거나 아직 도착하지 않은 경우 UI에 공백 문구가 노출될 수 있습니다. 빈 문자열이면 설명을 숨기는 것이 UX에 더 자연스럽습니다.
다음과 같이 조건부로 그려주세요:
- description = { - Text( - text = stringResource(R.string.latest_version, state.latestVersion), - color = ReedTheme.colors.contentTertiary, - style = ReedTheme.typography.label1Medium, - ) - }, + description = { + if (state.latestVersion.isNotBlank()) { + Text( + text = stringResource(R.string.latest_version, state.latestVersion), + color = ReedTheme.colors.contentTertiary, + style = ReedTheme.typography.label1Medium, + ) + } + },
212-213: description 슬롯의 기본값을 nullable로 바꿔 불필요한 람다 호출 방지기본값을 빈 람다로 두면 항상 호출 비용이 발생합니다. nullable 로 전환해 호출 자체를 건너뛰면 미세하지만 비용 절감과 의도 표현에 유리합니다.
- action: @Composable () -> Unit = {}, - description: @Composable () -> Unit = {}, + action: @Composable () -> Unit = {}, + description: (@Composable (() -> Unit))? = null,- description() + description?.invoke()Also applies to: 234-240
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultRemoteConfigRepository.kt (1)
27-29: 키 상수 분리 좋습니다KEY_LATEST_VERSION 상수 분리 적절합니다. 운영 중 키 변경 가능성이 낮지만, 환경별 키가 갈리는 경우를 대비해 DI로 주입하는 확장성도 고려해볼 수 있습니다.
📜 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/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/HandleException.kt(1 hunks)core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/RemoteCofigRepository.kt(1 hunks)core/data/impl/build.gradle.kts(1 hunks)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/di/FirebaseModule.kt(1 hunks)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/di/RepositoryModule.kt(2 hunks)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultRemoteConfigRepository.kt(1 hunks)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/HandlingSettingsSideEffect.kt(2 hunks)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt(3 hunks)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsUi.kt(4 hunks)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsUiState.kt(2 hunks)feature/settings/src/main/res/values/strings.xml(1 hunks)gradle/libs.versions.toml(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/HandlingSettingsSideEffect.kt (1)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/osslicenses/OssLicensesUiState.kt (1)
eventSink(6-8)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsUi.kt (1)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/HandlingSettingsSideEffect.kt (1)
HandleSettingsSideEffects(8-28)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt (3)
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/RemoteCofigRepository.kt (2)
getLatestVersion(3-5)getLatestVersion(4-4)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultRemoteConfigRepository.kt (1)
getLatestVersion(14-25)
🔇 Additional comments (10)
feature/settings/src/main/res/values/strings.xml (1)
17-17: 최신 버전 문자열 리소스 추가 적절포맷 플레이스홀더 사용이 적절하고 UI 연동 의도와 일치합니다. 별다른 이슈 없습니다.
gradle/libs.versions.toml (1)
153-153: Remote Config 라이브러리 별칭 추가 LGTMalias 명(firebase-remote-config)과 사용 지점(libs.firebase.remote.config)이 일관됩니다. BOM과 조합 사용에도 문제 없어 보입니다.
core/data/impl/build.gradle.kts (1)
21-22: Firebase BOM + Remote Config 의존성 추가 적절BOM을 먼저 선언하고 개별 라이브러리를 이어서 선언하는 순서가 맞습니다. 라이브러리 모듈에 선언해도 문제 없으며, 앱 모듈에서 google-services 플러그인을 적용하면 런타임에 정상 동작합니다.
core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/HandleException.kt (1)
22-22: 내부 Java 호출 미탐지 — @jvmoverloads 추가 불필요fd 스크립트 실행 결과 레포지토리 내에 Java에서 handleException 호출부가 없습니다. 따라서 현재로서는
@JvmOverloads를 추가하지 않아도 됩니다.
외부 Java 클라이언트에서 해당 함수를 호출해야 하는 경우에만@JvmOverloads를 적용하는 방안을 고려하세요.feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsUiState.kt (1)
13-16: latestVersion 상태 추가 LGTMUI에서 최신 버전 표시에 필요한 최소 상태만 추가되어 응집도가 좋습니다.
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/di/RepositoryModule.kt (1)
39-41: RemoteConfigRepository 바인딩 LGTMDI 구성이 일관되고, 싱글톤 스코프도 적절합니다.
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/HandlingSettingsSideEffect.kt (1)
9-12: eventSink 주입으로 사이드 이펙트 소모(consume) 흐름 분리 - 좋습니다호출부에서 eventSink를 전달받아 UI에서 소비 후 Presenter로 초기화 이벤트를 보냅니다. 순환 의존 없이 역할이 명확합니다.
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt (2)
69-72: 사이드이펙트 초기화 흐름 명확 - 좋습니다UI에서 소비 후 즉시 초기화하는 패턴이 Presenter에 잘 반영되어 있습니다.
169-171: 초기 로딩 트리거 위치 적절
LaunchedEffect(Unit)에서 최초 1회 최신 버전 조회를 트리거하는 접근이 적절합니다.feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsUi.kt (1)
50-53: 사이드이펙트 처리 시그니처 변경 적용 적절eventSink를 HandleSettingsSideEffects로 전달해 사이드이펙트 초기화 흐름을 분리한 점 좋습니다. 재구성 루프 우려도 없어 보입니다.
| import com.google.firebase.Firebase | ||
| import com.google.firebase.remoteconfig.BuildConfig | ||
| import com.google.firebase.remoteconfig.FirebaseRemoteConfig | ||
| import com.google.firebase.remoteconfig.remoteConfig | ||
| import com.google.firebase.remoteconfig.remoteConfigSettings |
There was a problem hiding this comment.
KTX import 및 BuildConfig 참조 오류로 컴파일/동작 문제가 발생합니다
- Firebase KTX 확장 사용 시 올바른 import는
com.google.firebase.ktx.Firebase,com.google.firebase.remoteconfig.ktx.remoteConfig,com.google.firebase.remoteconfig.ktx.remoteConfigSettings입니다. BuildConfig는 라이브러리의 것이 아니라 해당 모듈의BuildConfig를 참조해야 빌드 타입(디버그/릴리즈)에 따라 올바르게 분기됩니다. 현재처럼com.google.firebase.remoteconfig.BuildConfig를 쓰면 의도와 다르게 동작하거나 컴파일 오류가 납니다.
아래처럼 교체해주세요.
-import com.google.firebase.Firebase
-import com.google.firebase.remoteconfig.BuildConfig
+import com.google.firebase.ktx.Firebase
+import com.ninecraft.booket.core.data.impl.BuildConfig
import com.google.firebase.remoteconfig.FirebaseRemoteConfig
-import com.google.firebase.remoteconfig.remoteConfig
-import com.google.firebase.remoteconfig.remoteConfigSettings
+import com.google.firebase.remoteconfig.ktx.remoteConfig
+import com.google.firebase.remoteconfig.ktx.remoteConfigSettings🤖 Prompt for AI Agents
In
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/di/FirebaseModule.kt
around lines 3 to 7, the imports are incorrect for Firebase KTX and you are
referencing the wrong BuildConfig; replace com.google.firebase.Firebase with
com.google.firebase.ktx.Firebase, com.google.firebase.remoteconfig.remoteConfig
with com.google.firebase.remoteconfig.ktx.remoteConfig, and
com.google.firebase.remoteconfig.remoteConfigSettings with
com.google.firebase.remoteconfig.ktx.remoteConfigSettings, and change the
BuildConfig reference from com.google.firebase.remoteconfig.BuildConfig to this
module's BuildConfig (i.e., your package's BuildConfig) so the debug/release
branch uses the module build type.
| override suspend fun getLatestVersion(): Result<String> = suspendCancellableCoroutine { continuation -> | ||
| remoteConfig.fetchAndActivate().addOnCompleteListener { task -> | ||
| if (task.isSuccessful) { | ||
| val latestVersion = remoteConfig[KEY_LATEST_VERSION].asString() | ||
| Logger.d("LatestVersion: $latestVersion") | ||
| continuation.resume(Result.success(latestVersion)) | ||
| } else { | ||
| Logger.e(task.exception, "getLatestVersion failed") | ||
| continuation.resume(Result.failure(task.exception ?: Exception("Unknown error"))) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
코루틴 취소 시 resume 중복 호출 가능성 — 안전 가드와 캐시 폴백 추가 필요
suspendCancellableCoroutine + addOnCompleteListener 조합에서는 코루틴이 취소된 뒤에도 리스너가 호출될 수 있어, continuation.resume을 호출하면 IllegalStateException이 발생할 수 있습니다. 또한 fetch 실패 시 활성화된 기존 값으로의 폴백이 있으면 UX가 좋아집니다.
다음과 같이 isActive 가드와 캐시 폴백을 추가해 주세요:
- override suspend fun getLatestVersion(): Result<String> = suspendCancellableCoroutine { continuation ->
- remoteConfig.fetchAndActivate().addOnCompleteListener { task ->
- if (task.isSuccessful) {
- val latestVersion = remoteConfig[KEY_LATEST_VERSION].asString()
- Logger.d("LatestVersion: $latestVersion")
- continuation.resume(Result.success(latestVersion))
- } else {
- Logger.e(task.exception, "getLatestVersion failed")
- continuation.resume(Result.failure(task.exception ?: Exception("Unknown error")))
- }
- }
- }
+ override suspend fun getLatestVersion(): Result<String> = suspendCancellableCoroutine { continuation ->
+ remoteConfig.fetchAndActivate().addOnCompleteListener { task ->
+ if (!continuation.isActive) return@addOnCompleteListener
+ if (task.isSuccessful) {
+ val latestVersion = remoteConfig[KEY_LATEST_VERSION]
+ .asString()
+ .takeIf { it.isNotBlank() } ?: "Unknown"
+ Logger.d("LatestVersion: $latestVersion")
+ continuation.resume(Result.success(latestVersion))
+ } else {
+ Logger.e(task.exception, "getLatestVersion failed")
+ // fetch 실패 시에도 마지막 활성화된 값을 시도
+ val cached = remoteConfig[KEY_LATEST_VERSION].asString()
+ if (cached.isNotBlank()) {
+ continuation.resume(Result.success(cached))
+ } else {
+ continuation.resume(Result.failure(task.exception ?: Exception("Unknown error")))
+ }
+ }
+ }
+ }추가로, 필요하다면 continuation.invokeOnCancellation { /* no-op */ }를 등록해 의도를 더 분명히 할 수 있습니다.
📝 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.
| override suspend fun getLatestVersion(): Result<String> = suspendCancellableCoroutine { continuation -> | |
| remoteConfig.fetchAndActivate().addOnCompleteListener { task -> | |
| if (task.isSuccessful) { | |
| val latestVersion = remoteConfig[KEY_LATEST_VERSION].asString() | |
| Logger.d("LatestVersion: $latestVersion") | |
| continuation.resume(Result.success(latestVersion)) | |
| } else { | |
| Logger.e(task.exception, "getLatestVersion failed") | |
| continuation.resume(Result.failure(task.exception ?: Exception("Unknown error"))) | |
| } | |
| } | |
| } | |
| override suspend fun getLatestVersion(): Result<String> = suspendCancellableCoroutine { continuation -> | |
| remoteConfig.fetchAndActivate().addOnCompleteListener { task -> | |
| if (!continuation.isActive) return@addOnCompleteListener | |
| if (task.isSuccessful) { | |
| val latestVersion = remoteConfig[KEY_LATEST_VERSION] | |
| .asString() | |
| .takeIf { it.isNotBlank() } ?: "Unknown" | |
| Logger.d("LatestVersion: $latestVersion") | |
| continuation.resume(Result.success(latestVersion)) | |
| } else { | |
| Logger.e(task.exception, "getLatestVersion failed") | |
| // fetch 실패 시에도 마지막 활성화된 값을 시도 | |
| val cached = remoteConfig[KEY_LATEST_VERSION].asString() | |
| if (cached.isNotBlank()) { | |
| continuation.resume(Result.success(cached)) | |
| } else { | |
| continuation.resume(Result.failure(task.exception ?: Exception("Unknown error"))) | |
| } | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultRemoteConfigRepository.kt
around lines 14 to 25, the coroutine continuation may be resumed after
cancellation which can throw IllegalStateException and there is no fallback to
an activated cached value on fetch failure; guard resume calls with
continuation.isActive checks (only call continuation.resume(...) when isActive),
register continuation.invokeOnCancellation { /* no-op */ } to make intent
explicit, and on fetch failure try to read the currently activated cached value
(e.g. remoteConfig[KEY_LATEST_VERSION].asString()) and resume with
Result.success(fallback) if non-empty, otherwise resume with
Result.failure(task.exception ?: Exception("Unknown error")); ensure only one
resume path executes by checking isActive before each resume.
| var latestVersion by rememberRetained { mutableStateOf("") } | ||
| var sideEffect by rememberRetained { mutableStateOf<SettingsSideEffect?>(null) } | ||
|
|
||
| suspend fun getLatestVersion() { | ||
| try { | ||
| isLoading = true | ||
| remoteConfigRepository.getLatestVersion() | ||
| .onSuccess { version -> | ||
| latestVersion = version | ||
| } | ||
| .onFailure { exception -> | ||
| val handleErrorMessage = { message: String -> | ||
| Logger.e(message) | ||
| sideEffect = SettingsSideEffect.ShowToast(message) | ||
| } | ||
|
|
||
| handleException( | ||
| exception = exception, | ||
| onError = handleErrorMessage, | ||
| ) | ||
| } | ||
| } finally { | ||
| isLoading = false | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
동시 진행 작업 간 isLoading 경합으로 잘못된 로딩 상태 노출 가능
현재 getLatestVersion()과 로그아웃/회원탈퇴 흐름이 모두 동일한 isLoading을 토글합니다. 예컨대 최신 버전 조회가 끝나며 isLoading = false로 만들면, 그 사이 시작된 로그아웃/회원탈퇴 작업의 로딩 표시가 조기 종료될 수 있습니다. 각 작업별 로딩 플래그를 두고 최종 isLoading을 합성(or)하는 방식이 안전합니다.
다음과 같이 분리/합성하는 것을 권장합니다.
- var isLoading by rememberRetained { mutableStateOf(false) }
+ var isAuthInProgress by rememberRetained { mutableStateOf(false) }
+ var isLatestVersionLoading by rememberRetained { mutableStateOf(false) }
@@
- suspend fun getLatestVersion() {
+ suspend fun getLatestVersion() {
try {
- isLoading = true
+ isLatestVersionLoading = true
remoteConfigRepository.getLatestVersion()
.onSuccess { version ->
- latestVersion = version
+ // 필요 시 빈 문자열 방지(아래 별도 코멘트 참고)
+ latestVersion = version
}
.onFailure { exception ->
val handleErrorMessage = { message: String ->
Logger.e(message)
sideEffect = SettingsSideEffect.ShowToast(message)
}
handleException(
exception = exception,
onError = handleErrorMessage,
)
}
} finally {
- isLoading = false
+ isLatestVersionLoading = false
}
}로그아웃/회원탈퇴 흐름에서도 동일하게 isAuthInProgress를 사용하세요.
- isLoading = true
+ isAuthInProgress = true
@@
- isLoading = false
+ isAuthInProgress = false최종 State 반환 시에는 합성 결과를 사용합니다.
- isLoading = isLoading,
+ isLoading = isAuthInProgress || isLatestVersionLoading,📝 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.
| var latestVersion by rememberRetained { mutableStateOf("") } | |
| var sideEffect by rememberRetained { mutableStateOf<SettingsSideEffect?>(null) } | |
| suspend fun getLatestVersion() { | |
| try { | |
| isLoading = true | |
| remoteConfigRepository.getLatestVersion() | |
| .onSuccess { version -> | |
| latestVersion = version | |
| } | |
| .onFailure { exception -> | |
| val handleErrorMessage = { message: String -> | |
| Logger.e(message) | |
| sideEffect = SettingsSideEffect.ShowToast(message) | |
| } | |
| handleException( | |
| exception = exception, | |
| onError = handleErrorMessage, | |
| ) | |
| } | |
| } finally { | |
| isLoading = false | |
| } | |
| } | |
| // replace the old single `isLoading` flag with two separate flags | |
| var isAuthInProgress by rememberRetained { mutableStateOf(false) } | |
| var isLatestVersionLoading by rememberRetained { mutableStateOf(false) } | |
| var latestVersion by rememberRetained { mutableStateOf("") } | |
| var sideEffect by rememberRetained { mutableStateOf<SettingsSideEffect?>(null) } | |
| suspend fun getLatestVersion() { | |
| try { | |
| isLatestVersionLoading = true | |
| remoteConfigRepository.getLatestVersion() | |
| .onSuccess { version -> | |
| // 필요 시 빈 문자열 방지(아래 별도 코멘트 참고) | |
| latestVersion = version | |
| } | |
| .onFailure { exception -> | |
| val handleErrorMessage = { message: String -> | |
| Logger.e(message) | |
| sideEffect = SettingsSideEffect.ShowToast(message) | |
| } | |
| handleException( | |
| exception = exception, | |
| onError = handleErrorMessage, | |
| ) | |
| } | |
| } finally { | |
| isLatestVersionLoading = false | |
| } | |
| } |
🤖 Prompt for AI Agents
In
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt
around lines 41 to 65, the getLatestVersion() function toggles the shared
isLoading flag causing race conditions with logout/unregister flows; create a
dedicated flag (e.g., isGettingLatestVersion) for this function, set it
true/false inside the try/finally, and compute the public isLoading as the OR of
all per-operation flags (e.g., isGettingLatestVersion || isAuthInProgress).
Update the logout/unregister flows to use their own isAuthInProgress flag
instead of the shared isLoading, and ensure the state exposed/returned uses the
composed isLoading value. Ensure side effects and error handling remain
unchanged but reference the per-operation flag for lifecycle correctness.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/di/FirebaseModule.kt (1)
3-6: KTX import 누락으로 컴파일 실패합니다 (remoteConfig/remoteConfigSettings는 ktx 제공)
remoteConfig/remoteConfigSettings는 KTX 확장으로, 현재처럼com.google.firebase.remoteconfig.*에서 가져오면 심볼이 해석되지 않습니다. KTX 패키지로 교체가 필요합니다. 이전 코멘트와 동일한 이슈입니다.-import com.google.firebase.Firebase +import com.google.firebase.ktx.Firebase import com.google.firebase.remoteconfig.FirebaseRemoteConfig -import com.google.firebase.remoteconfig.remoteConfig -import com.google.firebase.remoteconfig.remoteConfigSettings +import com.google.firebase.remoteconfig.ktx.remoteConfig +import com.google.firebase.remoteconfig.ktx.remoteConfigSettings
🧹 Nitpick comments (3)
core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/RemoteConfigRepository.kt (1)
3-5: 메서드 의미(버전 포맷/의미) 문서화 필요외부(다른 모듈)가 의존하는 퍼블릭 인터페이스이므로 반환 값(VersionName vs VersionCode)과 포맷(예: SemVer "1.2.3")을 KDoc으로 명확히 해 두는 편이 안전합니다. 이후 비교/파싱 로직의 일관성에 도움이 됩니다.
아래처럼 KDoc을 추가해 주세요.
interface RemoteConfigRepository { - suspend fun getLatestVersion(): Result<String> + /** + * Remote Config에서 최신 앱 버전 문자열을 조회합니다. + * + * 반환되는 문자열의 포맷과 의미를 명확히 해주세요. + * - 예) VersionName: "1.2.3" (SemVer), 또는 VersionCode: "123" + * - 실패 시 Result.failure(...) 로 에러가 전달됩니다. + */ + suspend fun getLatestVersion(): Result<String> }core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/di/FirebaseModule.kt (2)
21-26: by lazy 불필요 — 즉시 val 할당으로 단순화한 번만 생성해 바로 전달하는 설정값에 lazy는 과합니다. 가독성과 비용 측면에서 즉시 생성이 더 낫습니다.
- val configSettings by lazy { - remoteConfigSettings { - minimumFetchIntervalInSeconds = if (BuildConfig.DEBUG) 0L else 60L - } - } - setConfigSettingsAsync(configSettings) + val configSettings = remoteConfigSettings { + minimumFetchIntervalInSeconds = if (BuildConfig.DEBUG) 0L else 60L + } + setConfigSettingsAsync(configSettings)
19-27: 프로덕션 fetch 간격(60초)은 과도하게 짧아 쿼터/레이트 리밋 리스크가 있습니다Firebase 권장 기본값은 12시간 수준입니다. 프로덕션에서 60초는 서버 및 클라이언트 자원 사용량을 크게 늘릴 수 있습니다. 요구사항에 따라 조정이 필요합니다.
권장 값 예시(디버그 0초, 릴리즈 12시간):
if (BuildConfig.DEBUG) 0L else 12 * 60 * 60L또한, 기본값 적용이 필요하다면
setDefaultsAsync(R.xml.remote_config_defaults)도 고려하세요. 레포지토리에서fetchAndActivate()또는 최소fetch()를 호출해 최신 값을 확보하는지 함께 확인 부탁드립니다.
📜 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)
core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/RemoteConfigRepository.kt(1 hunks)core/data/impl/build.gradle.kts(2 hunks)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/di/FirebaseModule.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- core/data/impl/build.gradle.kts
⏰ 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 (1)
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/di/FirebaseModule.kt (1)
7-7: 모듈의 BuildConfig 참조가 올바릅니다디버그/릴리즈 분기 목적에 맞게 모듈 BuildConfig를 참조하고 있어 적절합니다.
| return Firebase.remoteConfig.apply { | ||
| val configSettings by lazy { | ||
| remoteConfigSettings { | ||
| minimumFetchIntervalInSeconds = if (BuildConfig.DEBUG) 0 else 60 |
There was a problem hiding this comment.
minimumFetchIntervalInSeconds 타입(Int→Long) 불일치 가능성 — L 접미사 필요
minimumFetchIntervalInSeconds는 Long 타입입니다. 현재 0/60은 Int 리터럴이라 컴파일 에러가 발생할 수 있습니다. Long 리터럴로 수정해주세요.
- minimumFetchIntervalInSeconds = if (BuildConfig.DEBUG) 0 else 60
+ minimumFetchIntervalInSeconds = if (BuildConfig.DEBUG) 0L else 60L📝 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.
| minimumFetchIntervalInSeconds = if (BuildConfig.DEBUG) 0 else 60 | |
| minimumFetchIntervalInSeconds = if (BuildConfig.DEBUG) 0L else 60L |
🤖 Prompt for AI Agents
In
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/di/FirebaseModule.kt
around line 23, the assigned literals for minimumFetchIntervalInSeconds are Ints
but the parameter expects a Long; change the literals to Long (add the L suffix)
so use 0L when DEBUG and 60L otherwise (e.g., minimumFetchIntervalInSeconds = if
(BuildConfig.DEBUG) 0L else 60L).
사용하지 않는 stringResource 제거
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashUi.kt (1)
28-28: 오타: rememberSystemUiController import 경로가 잘못되어 컴파일 실패패키지명에 extensions 오탈자가 있습니다. 사용하는 라이브러리에 맞게 import를 수정해주세요.
다음과 같이 수정 제안드립니다:
-import tech.thdev.compose.exteions.system.ui.controller.rememberSystemUiController +import tech.thdev.compose.extensions.system.ui.controller.rememberSystemUiController라이브러리가 Accompanist라면 아래가 올바른 경우입니다(프로젝트 의존성에 따라 선택):
import com.google.accompanist.systemuicontroller.rememberSystemUiControllerAlso applies to: 33-35
feature/splash/build.gradle.kts (1)
23-28: 의존성 DSL 수정:implementations→implementation
feature/splash/build.gradle.kts23–28라인에서implementations(...)확장 함수 정의가 없어 빌드 오류가 발생합니다. 아래와 같이 변경해주세요:dependencies { - implementations( - libs.compose.system.ui.controller, - libs.logger, - ) + implementation(libs.compose.system.ui.controller) + implementation(libs.logger) }
♻️ Duplicate comments (1)
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultRemoteConfigRepository.kt (1)
15-26: 코루틴 취소 후 resume 가능성 및 캐시 폴백 부재 — 가드/폴백 추가 필요
suspendCancellableCoroutine+addOnCompleteListener조합에서는 취소 후 리스너 호출로resume중복 호출 위험이 있습니다. 또한 fetch 실패 시 활성화된 캐시 값 폴백이 있으면 UX가 개선됩니다.override suspend fun getLatestVersion(): Result<String> = suspendCancellableCoroutine { continuation -> - remoteConfig.fetchAndActivate().addOnCompleteListener { task -> - if (task.isSuccessful) { - val latestVersion = remoteConfig[KEY_LATEST_VERSION].asString() - Logger.d("LatestVersion: $latestVersion") - continuation.resume(Result.success(latestVersion)) - } else { - Logger.e(task.exception, "getLatestVersion failed") - continuation.resume(Result.failure(task.exception ?: Exception("Unknown error"))) - } - } + remoteConfig.fetchAndActivate().addOnCompleteListener { task -> + if (!continuation.isActive) return@addOnCompleteListener + if (task.isSuccessful) { + val latestVersion = remoteConfig[KEY_LATEST_VERSION].asString() + .takeIf { it.isNotBlank() } ?: "Unknown" + Logger.d("LatestVersion: $latestVersion") + continuation.resume(Result.success(latestVersion)) + } else { + Logger.e(task.exception, "getLatestVersion failed") + // fetch 실패 시 활성화된 캐시 값 폴백 + val cached = remoteConfig[KEY_LATEST_VERSION].asString() + if (cached.isNotBlank()) { + continuation.resume(Result.success(cached)) + } else { + continuation.resume(Result.failure(task.exception ?: Exception("Unknown error"))) + } + } + } }원하시면
continuation.invokeOnCancellation { /* no-op or log */ }도 함께 추가해 의도를 명확히 할 수 있습니다.
🧹 Nitpick comments (6)
feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashUiState.kt (2)
7-11: Compose 추적 최적화를 위해 SplashUiState에 @immutable 추가 권장eventSink(lamba)와 nullable sideEffect를 갖는 상태 객체는 @immutable로 마킹해두면 불필요한 재구성을 줄이는 데 도움이 됩니다.
다음 변경을 제안합니다:
+@Immutable data class SplashUiState( val isForceUpdateDialogVisible: Boolean = false, val sideEffect: SplashSideEffect? = null, val eventSink: (SplashUiEvent) -> Unit, ) : CircuitUiState
18-21: 이벤트 네이밍 가독성 개선 제안InitSideEffect는 “소비 완료” 의미에 가까워보입니다. SideEffectConsumed/ResetSideEffect 같은 이름이 의도를 더 잘 드러낼 수 있습니다. (선택사항)
feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashUi.kt (1)
90-94: 프리뷰에서 강제 업데이트 가시성 케이스도 함께 확인 제안isForceUpdateDialogVisible=true 케이스를 프리뷰에 반영하면 UI 검증이 수월합니다. (선택사항)
다음과 같이 보강 가능합니다:
- SplashUi( - state = SplashUiState( - eventSink = {}, - ), - ) + SplashUi( + state = SplashUiState( + isForceUpdateDialogVisible = true, + eventSink = {}, + ), + )feature/splash/build.gradle.kts (1)
10-16: BuildConfig 필드(PACKAGE_NAME) 추가는 현재 사용처가 없어 과잉 설정 가능성HandleSplashSideEffects.kt에서는
context.packageName를 사용 중이라 별도의 BuildConfig 상수 없이도 동작합니다. 패키지명이 빌드 변형/플레이버에 따라 달라질 수 있는 점까지 고려하면, 정적 상수로 고정하는 것은 역으로 위험할 수 있습니다. 사용 계획이 없다면 제거를 권합니다.android { namespace = "com.ninecraft.booket.feature.splash" - buildFeatures { - buildConfig = true - } - - defaultConfig { - buildConfigField("String", "PACKAGE_NAME", "\"${libs.versions.packageName.get()}\"") - } + // buildConfig / PACKAGE_NAME 상수는 현재 미사용이므로 생략 }필요 시 나중에 실제 사용 지점이 생길 때, 해당 모듈에서만 최소 범위로 재도입하는 것을 추천드립니다.
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultRemoteConfigRepository.kt (1)
41-57: 버전 비교 정규식은 상수로 재사용하고, 명확성 향상정규식을 매 호출마다 생성하지 말고 상수로 빼면 가독성과 성능에 이점이 있습니다. 또한 비교 로직은 충분히 명확하고 올바릅니다.
private fun checkMinVersion(currentVersion: String, minVersion: String): Boolean { Logger.d("checkMinVersion: current: $currentVersion, min: $minVersion") - if (!Regex("""^\d+\.\d+\.\d+$""").matches(currentVersion)) return false - if (!Regex("""^\d+\.\d+\.\d+$""").matches(minVersion)) return false + if (!SEMVER_REGEX.matches(currentVersion)) return false + if (!SEMVER_REGEX.matches(minVersion)) return false추가로, 아래 companion object에 상수 추가:
companion object { + private val SEMVER_REGEX = Regex("""^\d+\.\d+\.\d+$""") private const val KEY_LATEST_VERSION = "LatestVersion" - private const val KEY_MIN_VERSION = "MinVersion" + private const val KEY_MIN_VERSION = "MinVersion" }feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashPresenter.kt (1)
110-129: Remote Config 실패 로깅 개선 및 중복 호출 가드 제안
- 실패 시
Logger.e("${exception.message}")는 스택 트레이스를 남기지 않습니다. 예외 객체를 함께 넘겨주세요.- 강제 업데이트 다이얼로그가 이미 떠 있는 경우 중복으로 Remote Config를 재호출하지 않도록 가드를 추가하면 불필요한 호출을 줄일 수 있습니다.
LaunchedEffect(onboardingState, autoLoginState) { delay(1000L) if (onboardingState == OnboardingState.IDLE || autoLoginState == AutoLoginState.IDLE) { return@LaunchedEffect } + + // 이미 강제 업데이트 다이얼로그가 표시 중이면 재호출 생략 + if (isForceUpdateDialogVisible) { + return@LaunchedEffect + } remoteConfigRepository.shouldUpdate() .onSuccess { shouldUpdate -> if (shouldUpdate) { isForceUpdateDialogVisible = true } else { proceedToNextScreen() } } .onFailure { exception -> - Logger.e("${exception.message}") + Logger.e(exception, "shouldUpdate failed") proceedToNextScreen() } }
📜 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 (13)
build-logic/src/main/kotlin/AndroidApplicationConventionPlugin.kt(2 hunks)build-logic/src/main/kotlin/AndroidLibraryConventionPlugin.kt(2 hunks)build-logic/src/main/kotlin/com/ninecraft/booket/convention/Android.kt(1 hunks)build-logic/src/main/kotlin/com/ninecraft/booket/convention/ApplicationConstants.kt(0 hunks)core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/RemoteConfigRepository.kt(1 hunks)core/data/impl/build.gradle.kts(2 hunks)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultRemoteConfigRepository.kt(1 hunks)feature/splash/build.gradle.kts(1 hunks)feature/splash/src/main/kotlin/com/ninecraft/booket/splash/HandleSplashSideEffects.kt(1 hunks)feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashPresenter.kt(5 hunks)feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashUi.kt(3 hunks)feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashUiState.kt(1 hunks)gradle/libs.versions.toml(2 hunks)
💤 Files with no reviewable changes (1)
- build-logic/src/main/kotlin/com/ninecraft/booket/convention/ApplicationConstants.kt
🚧 Files skipped from review as they are similar to previous changes (3)
- core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/RemoteConfigRepository.kt
- gradle/libs.versions.toml
- core/data/impl/build.gradle.kts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-20T12:34:23.786Z
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)만 별도로 선언하면 된다.
Applied to files:
build-logic/src/main/kotlin/AndroidLibraryConventionPlugin.ktbuild-logic/src/main/kotlin/AndroidApplicationConventionPlugin.ktfeature/splash/build.gradle.kts
🧬 Code Graph Analysis (2)
feature/splash/src/main/kotlin/com/ninecraft/booket/splash/HandleSplashSideEffects.kt (1)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/osslicenses/OssLicensesUiState.kt (1)
eventSink(6-8)
feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashUi.kt (1)
feature/splash/src/main/kotlin/com/ninecraft/booket/splash/HandleSplashSideEffects.kt (1)
HandleSplashSideEffects(10-29)
⏰ 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 (11)
feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashUiState.kt (1)
13-16: 사이드 이펙트 모델링 적절함UI에서 처리할 원샷 동작을 SideEffect로 분리한 점 좋습니다. Compose @immutable 적용도 적절합니다.
build-logic/src/main/kotlin/com/ninecraft/booket/convention/Android.kt (2)
11-11: compileSdk/minSdk의 버전 카탈로그 이관 잘 반영됨하드코딩된 SDK 값을 제거하고 libs.versions에서 가져오도록 한 변경은 유지보수성과 일관성 측면에서 👍
Also applies to: 14-14
11-11: ApplicationConstants의 javaVersion 및 JAVA_VERSION_INT 정의 정상 유지됨 확인검증 결과, 아래와 같이 모든 필드와 설정이 올바르게 존재함을 확인했습니다. 별도 조치가 필요 없습니다.
- build-logic/src/main/kotlin/com/ninecraft/booket/convention/ApplicationConstants.kt
• const val JAVA_VERSION_INT = 17
• val javaVersion = JavaVersion.VERSION_17- gradle/libs.versions.toml
• minSdk, targetSdk, compileSdk, versionName, versionCode 모두 정의 확인- build-logic 모듈 내 libs 접근자(get() = the()) 정상 동작
build-logic/src/main/kotlin/AndroidApplicationConventionPlugin.kt (1)
6-6: 앱 버전/타깃 SDK를 버전 카탈로그로 일원화한 점 좋습니다versionName/versionCode/targetSdk를 libs.versions에서 읽어오도록 해 릴리즈 파이프라인과의 일관성이 좋아집니다. 모듈 단에서 중복 설정이 없는지만 한 번 확인해주세요.
Also applies to: 23-25
build-logic/src/main/kotlin/AndroidLibraryConventionPlugin.kt (1)
5-5: 라이브러리 타깃 SDK 카탈로그 이관 👍라이브러리도 동일한 소스에서 targetSdk를 읽도록 통일된 점 좋습니다.
Also applies to: 22-22
feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashUi.kt (2)
33-35: Circuit UI 시그니처(state 주입) 전환 적절합니다Presenter-Driven 상태 연결을 위한 state 파라미터 추가 및 DI 어노테이션 유지 모두 적절합니다.
54-57: 사이드이펙트 처리 연결이 올바릅니다UI에서 HandleSplashSideEffects로 sideEffect를 위임하고 eventSink로 소비 이벤트를 되돌리는 흐름이 명확합니다.
feature/splash/src/main/kotlin/com/ninecraft/booket/splash/HandleSplashSideEffects.kt (1)
17-28: 사이드 이펙트 처리 플로우는 명확하고 안전하게 초기화됨sideEffect 처리 후
InitSideEffect를 통해 즉시 초기화하는 패턴이 재발행 방지에 유효합니다. Compose와 잘 맞는 구현입니다.feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashPresenter.kt (3)
46-48: 상태 보존 변수 도입은 적절함
rememberRetained와mutableStateOf로 UI 상태를 안정적으로 유지하고 있어 구성 변경에 안전합니다.
70-96: 화면 전환 분기 로직은 기존 플로우를 잘 유지온보딩/자동로그인 상태에 따른 분기와 재시도 처리(
postErrorDialog)가 기존 행태를 보존하고 있습니다.
98-108: 이벤트 → 사이드이펙트 → 초기화 플로우 일관성 양호업데이트 버튼 클릭 시 사이드이펙트 발행 후 초기화하는 패턴이 명확합니다.
| override suspend fun shouldUpdate(): Result<Boolean> = suspendCancellableCoroutine { continuation -> | ||
| remoteConfig.fetchAndActivate().addOnCompleteListener { task -> | ||
| if (task.isSuccessful) { | ||
| val latestVersion = remoteConfig[KEY_MIN_VERSION].asString() | ||
| val currentVersion = BuildConfig.APP_VERSION | ||
| continuation.resume(Result.success(checkMinVersion(currentVersion, latestVersion))) | ||
| } else { | ||
| Logger.e(task.exception, "shouldUpdate: getMinVersion failed") | ||
| continuation.resume(Result.failure(task.exception ?: Exception("Unknown error"))) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
shouldUpdate()에도 동일한 취소 가드/캐시 폴백 적용 및 변수명 정정 필요
취소 가드/폴백 로직을 동일하게 적용하고, latestVersion 변수명을 minVersion으로 정정해 가독성을 개선하세요.
override suspend fun shouldUpdate(): Result<Boolean> = suspendCancellableCoroutine { continuation ->
remoteConfig.fetchAndActivate().addOnCompleteListener { task ->
- if (task.isSuccessful) {
- val latestVersion = remoteConfig[KEY_MIN_VERSION].asString()
- val currentVersion = BuildConfig.APP_VERSION
- continuation.resume(Result.success(checkMinVersion(currentVersion, latestVersion)))
- } else {
- Logger.e(task.exception, "shouldUpdate: getMinVersion failed")
- continuation.resume(Result.failure(task.exception ?: Exception("Unknown error")))
- }
+ if (!continuation.isActive) return@addOnCompleteListener
+ if (task.isSuccessful) {
+ val minVersion = remoteConfig[KEY_MIN_VERSION].asString()
+ val currentVersion = BuildConfig.APP_VERSION
+ continuation.resume(Result.success(checkMinVersion(currentVersion, minVersion)))
+ } else {
+ Logger.e(task.exception, "shouldUpdate: getMinVersion failed")
+ // fetch 실패 시 활성화된 캐시 값 폴백
+ val cachedMin = remoteConfig[KEY_MIN_VERSION].asString()
+ if (cachedMin.isNotBlank()) {
+ val currentVersion = BuildConfig.APP_VERSION
+ continuation.resume(Result.success(checkMinVersion(currentVersion, cachedMin)))
+ } else {
+ continuation.resume(Result.failure(task.exception ?: Exception("Unknown error")))
+ }
+ }
}
}🤖 Prompt for AI Agents
In
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultRemoteConfigRepository.kt
around lines 28 to 39, add the same cancellation guard and cache fallback logic
used elsewhere: before calling continuation.resume(...) check
continuation.isActive (or use continuation.resumeWith if needed) to avoid
resuming a cancelled coroutine, and on failure return a cached/minimum-version
fallback instead of immediately failing the coroutine; also rename the local
variable latestVersion to minVersion for clarity and update any subsequent
references accordingly.
easyhooon
force-pushed
the
BOOK-274-feature/#151
branch
from
cc442ea to
165c773
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashUi.kt (1)
30-30: 치명적: rememberSystemUiController import 경로 오타 수정 필요다음 3개 파일에서
tech.thdev.compose.exteions…오타로 인해 컴파일 에러가 발생합니다. 프로젝트에 맞는 올바른 패키지로 수정하세요.파일 및 위치:
- feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashUi.kt (라인 30)
- feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/ocr/OcrUi.kt (라인 71)
- feature/main/src/main/kotlin/com/ninecraft/booket/feature/main/MainActivity.kt (라인 28)
수정 예시 (Accompanist 사용 시 권장):
- import tech.thdev.compose.exteions.system.ui.controller.rememberSystemUiController + import com.google.accompanist.systemuicontroller.rememberSystemUiController• Gradle 의존성 확인:
com.google.accompanist:accompanist-systemuicontroller:<version>또는 tech.thdev 패키지 사용 시:
- import tech.thdev.compose.exteions.system.ui.controller.rememberSystemUiController + import tech.thdev.compose.extensions.system.ui.controller.rememberSystemUiController수정 후, 아래 스크립트로 정의·사용 위치를 재검증하세요.
#!/bin/bash rg -n -C2 --iglob '!**/build/**' -P '\brememberSystemUiController\s*\(' rg -n -C2 --iglob '!**/build/**' -P 'import\s+.+rememberSystemUiController'
🧹 Nitpick comments (10)
feature/splash/build.gradle.kts (2)
14-16: 실시간 패키지명 사용으로 딥링크 생성debug 빌드(
applicationIdSuffix = ".dev")에서
BuildConfig.PACKAGE_NAME("com.ninecraft.booket")을 그대로 쓰면
런타임 패키지명(com.ninecraft.booket.dev)과 불일치해
Play Store 딥링크가 동작하지 않습니다.• 확인된 위치
- app/build.gradle.kts (line 31):
applicationIdSuffix = ".dev"- feature/splash/src/main/kotlin/com/ninecraft/booket/splash/HandleSplashSideEffects.kt (line 34):
market://details?id=${BuildConfig.PACKAGE_NAME}• 권장 수정
- BuildConfig.PACKAGE_NAME 제거
- Compose:
LocalContext.current.packageName
일반 코드:context.packageName사용예시 diff:
- val intent = Intent( - Intent.ACTION_VIEW, - "market://details?id=${BuildConfig.PACKAGE_NAME}".toUri() - ) + val pkg = context.packageName + val intent = Intent( + Intent.ACTION_VIEW, + "market://details?id=$pkg".toUri() + )필요 시 앱 모듈에만
buildConfigField("String","APP_ID",…)로 상수를 정의하고
DI로 feature 모듈에 주입하는 구조를 고려하세요.
10-12: buildConfig 중복 설정 제거 권장
booket.android.feature 컨벤션 플러그인과 Android Gradle Plugin의 기본값으로 이미buildConfig = true가 적용됩니다. 명시된 설정은 불필요하니 아래 코드를 제거하세요.
- feature/splash/build.gradle.kts (10–12행):
- buildFeatures { - buildConfig = true - }feature/splash/src/main/res/values/strings.xml (1)
4-6: 국문 표기 교정 제안(띄어쓰기/합성어).사용자 노출 문구의 자연스러운 표기를 위해 아래와 같이 수정 권장합니다.
- <string name="splash_force_update_title">최신 버전이 출시 되었습니다.</string> + <string name="splash_force_update_title">최신 버전이 출시되었습니다.</string> - <string name="splash_force_update_message">최적의 사용 환경을 위해 업데이트를 해주세요.</string> + <string name="splash_force_update_message">최적의 사용 환경을 위해 업데이트해 주세요.</string> - <string name="splash_force_update_button_text">업데이트 하기</string> + <string name="splash_force_update_button_text">업데이트하기</string>feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashUi.kt (3)
3-3: 불필요하고 혼동을 줄 수 있는 import 제거.
android.R.attr.description는 사용되지 않고, 파라미터명과 혼동을 유발할 수 있어 제거 권장합니다.-import android.R.attr.description
86-95: 강제 업데이트 다이얼로그의 dismiss 동작 의도 확인 필요.현재
ReedDialog의onDismissRequest가 빈 람다이므로 백 버튼/바깥 클릭 시 닫히지 않는(눈에 그대로 남는) 형태입니다. 강제 업데이트(하드 블로킹) 의도라면 문제 없습니다. 반대로 선택적 업데이트(소프트) 의도라면 취소/나중에 버튼 또는 닫기 가능하도록 설계 변경이 필요합니다.옵션:
- 하드 블로킹 유지: ReedDialog 내부
DialogProperties에dismissOnBackPress=false, dismissOnClickOutside=false를 명시(글로벌 영향 주의). 혹은 ReedDialog에 해당 속성을 제어하는 파라미터 추가.- 소프트 업데이트:
dismissButtonText제공 및onDismissRequest에서eventSink로 닫기 이벤트 송신 → Presenter에서 다음 화면 진행.
71-75: i18n/접근성: contentDescription 하드코딩 제거 권장.
"Reed Logo"문자열을 string 리소스로 분리하면 현지화 및 접근성 관리가 수월합니다.예시:
- strings.xml
<string name="splash_logo_content_description">리드 로고</string>
- 사용부
contentDescription = stringResource(R.string.splash_logo_content_description)feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashUiState.kt (1)
7-12: Compose 안정성 힌트: State에 @immutable 부여 고려.
SplashUiState에@Immutable을 부여하면 Compose가 더 공격적으로 리컴포지션을 건너뛸 수 있습니다(함수형 프로퍼티(eventSink) 포함 구조지만, 상태 클래스로 명시적 의도를 전달하는 이점이 있습니다).+@Immutable data class SplashUiState( val isForceUpdateDialogVisible: Boolean = false, val sideEffect: SplashSideEffect? = null, val eventSink: (SplashUiEvent) -> Unit, ) : CircuitUiStatefeature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashPresenter.kt (3)
98-108: 업데이트 버튼 처리 시 다이얼로그 상태 처리 의도 확인.
OnUpdateButtonClick에서 Play 스토어 이동 사이드이펙트만 발생하고,isForceUpdateDialogVisible은 유지됩니다. 강제 업데이트라면 OK입니다. 선택적 업데이트라면 이동 직전에 다이얼로그를 닫거나 이동 후 복귀 시 다음 화면으로 진행하는 로직이 필요할 수 있습니다.선택적 업데이트 시 예시:
when (event) { SplashUiEvent.OnUpdateButtonClick -> { - sideEffect = SplashSideEffect.NavigateToPlayStore + isForceUpdateDialogVisible = false + sideEffect = SplashSideEffect.NavigateToPlayStore }
110-116: 불필요한 대기 최소화: IDLE 체크를 delay 이전으로 이동.현재는 상태가 여전히 IDLE인 경우에도 매번 1초 대기 후 반환합니다. IDLE 체크를 먼저 수행하면 불필요한 대기를 줄일 수 있습니다. 브랜드 노출 최소 시간(1초) 의도가 있다면, 두 상태가 준비된 이후에만 delay를 적용하는 편이 합리적입니다.
- LaunchedEffect(onboardingState, autoLoginState) { - delay(1000L) - - if (onboardingState == OnboardingState.IDLE || autoLoginState == AutoLoginState.IDLE) { - return@LaunchedEffect - } + LaunchedEffect(onboardingState, autoLoginState) { + if (onboardingState == OnboardingState.IDLE || autoLoginState == AutoLoginState.IDLE) { + return@LaunchedEffect + } + delay(1000L)
126-128: 로깅 시 스택트레이스 포함 권장.
Logger.e("${exception.message}")는 메시지만 출력하여 원인 분석이 어려울 수 있습니다. throwable을 함께 전달해 주세요.- Logger.e("${exception.message}") + Logger.e(exception, "RemoteConfig shouldUpdate() failed")
📜 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 (14)
build-logic/src/main/kotlin/AndroidApplicationConventionPlugin.kt(2 hunks)build-logic/src/main/kotlin/AndroidLibraryConventionPlugin.kt(2 hunks)build-logic/src/main/kotlin/com/ninecraft/booket/convention/Android.kt(1 hunks)build-logic/src/main/kotlin/com/ninecraft/booket/convention/ApplicationConstants.kt(0 hunks)core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/RemoteConfigRepository.kt(1 hunks)core/data/impl/build.gradle.kts(2 hunks)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultRemoteConfigRepository.kt(1 hunks)feature/splash/build.gradle.kts(1 hunks)feature/splash/src/main/kotlin/com/ninecraft/booket/splash/HandleSplashSideEffects.kt(1 hunks)feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashPresenter.kt(5 hunks)feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashUi.kt(5 hunks)feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashUiState.kt(1 hunks)feature/splash/src/main/res/values/strings.xml(1 hunks)gradle/libs.versions.toml(2 hunks)
💤 Files with no reviewable changes (1)
- build-logic/src/main/kotlin/com/ninecraft/booket/convention/ApplicationConstants.kt
🚧 Files skipped from review as they are similar to previous changes (7)
- gradle/libs.versions.toml
- core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/RemoteConfigRepository.kt
- build-logic/src/main/kotlin/AndroidApplicationConventionPlugin.kt
- feature/splash/src/main/kotlin/com/ninecraft/booket/splash/HandleSplashSideEffects.kt
- build-logic/src/main/kotlin/AndroidLibraryConventionPlugin.kt
- core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultRemoteConfigRepository.kt
- core/data/impl/build.gradle.kts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-20T12:34:23.786Z
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)만 별도로 선언하면 된다.
Applied to files:
feature/splash/build.gradle.kts
🧬 Code Graph Analysis (1)
feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashUi.kt (2)
feature/splash/src/main/kotlin/com/ninecraft/booket/splash/HandleSplashSideEffects.kt (1)
HandleSplashSideEffects(11-30)core/ui/src/main/kotlin/com/ninecraft/booket/core/ui/component/ReedDialog.kt (1)
ReedDialog(25-114)
🔇 Additional comments (9)
build-logic/src/main/kotlin/com/ninecraft/booket/convention/Android.kt (3)
11-11: 버전 카탈로그를 사용한 compileSdk 설정 변경 승인하드코딩된 상수 대신 Gradle 버전 카탈로그를 사용하여 compileSdk를 설정하는 것은 좋은 변경입니다. 이는 중앙화된 버전 관리를 가능하게 합니다.
14-14: 버전 카탈로그를 사용한 minSdk 설정 변경 승인하드코딩된 상수 대신 Gradle 버전 카탈로그를 사용하여 minSdk를 설정하는 것은 좋은 변경입니다. 이는 프로젝트 전반에 걸친 일관성을 보장합니다.
11-14: 라이브러리 버전 참조 확인 완료
- gradle/libs.versions.toml에
• compileSdk = "35"
• minSdk = "28"
가 정의되어 있습니다.- build-logic 모듈의 build.gradle.kts에서 version catalog( libs )가 정상적으로 적용되어 있습니다.
따라서
libs.versions.compileSdk.get().toInt()와libs.versions.minSdk.get().toInt()사용에 문제가 없습니다.feature/splash/build.gradle.kts (1)
9-9: 의미 없는 변경은 스킵합니다.빈 줄 추가만 이뤄졌습니다. 동작이나 설정에 영향이 없어 코멘트 생략합니다.
feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashUi.kt (2)
56-59: 사이드 이펙트 연결 LGTM.
HandleSplashSideEffects에state/eventSink를 넘겨 단방향 데이터 흐름을 유지합니다.
103-107: 프리뷰 업데이트 LGTM.새로운
SplashUi(state, ...)시그니처에 맞춘 미니멀 스텁 상태 주입이 적절합니다.feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashUiState.kt (1)
13-21: SideEffect/Event 모델링 LGTM.
NavigateToPlayStore사이드이펙트와OnUpdateButtonClick/InitSideEffect이벤트로 분리된 설계가 명확합니다.feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashPresenter.kt (2)
38-39: RemoteConfigRepository 주입 LGTM.DI로 원격 구성 의존성을 주입해 테스트 용이성과 관심사 분리를 확보했습니다.
46-47: rememberRetained 사용 적절.구성 변경에도 유지되어야 하는 UI 상태(
isForceUpdateDialogVisible,sideEffect)에rememberRetained를 적용한 점 좋습니다.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt (1)
37-37: isLoading 경합 가능성: 작업별 로딩 플래그로 분리 권장(기존 코멘트와 동일 이슈)로그아웃/회원탈퇴/최신버전 조회가 모두 동일한 isLoading을 토글합니다. 비동기로 중첩 실행될 경우 먼저 끝난 작업이 isLoading=false로 만들어 다른 작업의 로딩 표시가 조기 종료될 수 있습니다. 작업별 플래그를 두고 최종 isLoading을 합성(or)하는 방식이 안전합니다.
아래와 같이 변경을 제안합니다.
선언부 변경 (Line 37):
- var isLoading by rememberRetained { mutableStateOf(false) } + var isAuthInProgress by rememberRetained { mutableStateOf(false) } + var isLatestVersionLoading by rememberRetained { mutableStateOf(false) }logout() (Lines 47, 67):
- isLoading = true + isAuthInProgress = true @@ - isLoading = false + isAuthInProgress = falsewithdraw() (Lines 76, 96):
- isLoading = true + isAuthInProgress = true @@ - isLoading = false + isAuthInProgress = falsegetLatestVersion() (Lines 105, 122):
- isLoading = true + isLatestVersionLoading = true @@ - isLoading = false + isLatestVersionLoading = falsepresent 반환부 (Line 184):
- isLoading = isLoading, + isLoading = isAuthInProgress || isLatestVersionLoading,Also applies to: 44-71, 73-100, 102-125, 183-189
🧹 Nitpick comments (5)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt (4)
53-56: 로그 중복 발생: handleException 내부 로그와 중복되는 에러 로그 제거 권장handleException은 HTTP/기타 예외에서 이미 로그를 남깁니다. 현재 onFailure의 onError 콜백에서도 Logger.e를 호출해 중복 로그가 발생할 수 있습니다. 콜백 내 Logger.e는 제거하거나 수준을 낮추는 것을 권장합니다.
logout() 콜백 (Lines 53-56):
- val handleErrorMessage = { message: String -> - Logger.e(message) - sideEffect = SettingsSideEffect.ShowToast(message) - } + val handleErrorMessage = { message: String -> + sideEffect = SettingsSideEffect.ShowToast(message) + }withdraw() 콜백 (Lines 82-85):
- val handleErrorMessage = { message: String -> - Logger.e(message) - sideEffect = SettingsSideEffect.ShowToast(message) - } + val handleErrorMessage = { message: String -> + sideEffect = SettingsSideEffect.ShowToast(message) + }getLatestVersion() 콜백 (Lines 111-114):
- val handleErrorMessage = { message: String -> - Logger.e(message) - sideEffect = SettingsSideEffect.ShowToast(message) - } + val handleErrorMessage = { message: String -> + sideEffect = SettingsSideEffect.ShowToast(message) + }참고: 네트워크 예외의 경우 현재 handleException이 로그를 남기지 않습니다. 네트워크 오류에도 로그가 필요하다면 handleException 내부에서 Logger.w 정도로 처리하는 방안이 일관적입니다.
Also applies to: 82-85, 111-114
17-17: RememberedEffect 대신 Compose 표준 LaunchedEffect 사용 고려1회 실행 사이드이펙트에는 LaunchedEffect(Unit)이 더 표준적이며 외부 라이브러리 의존을 줄일 수 있습니다. 동작은 동일합니다.
imports (Line 17) 및 사용부 (Lines 179-181) 변경:
- import com.skydoves.compose.effects.RememberedEffect + import androidx.compose.runtime.LaunchedEffect- RememberedEffect(Unit) { - getLatestVersion() - } + LaunchedEffect(Unit) { + getLatestVersion() + }확인 요청: 기존 RememberedEffect 사용에는 의도된 다른 효과(예: 재구성 간 보장)가 있는지 검토 부탁드립니다. 없다면 LaunchedEffect로 단순화해도 무방해 보입니다.
Also applies to: 179-181
107-109: 빈 문자열 방지: 최신 버전 값 가드 처리 권장원격 설정에서 빈 문자열이 내려올 경우 UI에 공백으로 노출될 수 있습니다. 기존 값 유지 가드가 안전합니다.
- .onSuccess { version -> - latestVersion = version - } + .onSuccess { version -> + // 빈 문자열일 경우 기존 값 유지 + latestVersion = version.takeIf { it.isNotBlank() } ?: latestVersion + }
102-125: 테스트 보강 제안: 최신 버전 조회 흐름Presenter 단위에서 다음 시나리오 테스트를 권장합니다.
- 성공 시 latestVersion이 업데이트되는지
- 실패 시 ShowToast 사이드이펙트가 발생하는지
- isLoading 합성(분리 후)을 검증
필요하시면 Fake RemoteConfigRepository를 사용한 테스트 템플릿을 드리겠습니다.
Also applies to: 183-189
feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashPresenter.kt (1)
98-113: LaunchedEffect 내부에서 다시 scope.launch 사용 → 구조화된 동시성 어긋남(중복 실행/취소 누락 가능) — suspend 전환으로 단순화 제안LaunchedEffect가 재시작되면 내부에서 rememberCoroutineScope.launch로 시작한 작업은 취소되지 않아 중복 실행 위험이 있습니다. checkForceUpdate를 suspend 함수로 바꾸고, LaunchedEffect의 코루틴 컨텍스트에서 직접 호출하면 구조가 간결해지고 취소 전파가 보장됩니다. 또한 예외 로깅 시 Throwable을 함께 넘겨 스택트레이스를 남기면 디버깅에 유리합니다.
아래 변경을 제안합니다.
- checkForceUpdate를 suspend 함수로 전환하고, Throwable을 포함하여 로깅:
-fun checkForceUpdate() { - scope.launch { - remoteConfigRepository.shouldUpdate() - .onSuccess { shouldUpdate -> - if (shouldUpdate) { - isForceUpdateDialogVisible = true - } else { - proceedToNextScreen() - } - } - .onFailure { exception -> - Logger.e("${exception.message}") - proceedToNextScreen() - } - } -} +suspend fun checkForceUpdate() { + remoteConfigRepository.shouldUpdate() + .onSuccess { shouldUpdate -> + if (shouldUpdate) { + isForceUpdateDialogVisible = true + } else { + proceedToNextScreen() + } + } + .onFailure { exception -> + Logger.e(exception, "RemoteConfig shouldUpdate failed") + proceedToNextScreen() + } +}
- LaunchedEffect에서 suspend 함수 직접 호출 및 지연 상수화(아래 3번 참조):
-LaunchedEffect(onboardingState, autoLoginState) { - delay(1000L) +LaunchedEffect(onboardingState, autoLoginState) { + delay(SPLASH_DELAY_MS) @@ - checkForceUpdate() + checkForceUpdate() }
- 매직 넘버 제거(선택): 클래스 내부에 지연 시간 상수를 추가해 가독성과 유지보수성을 높입니다.
// 클래스 내부 어디든(예: Factory 이전) 추가 companion object { private const val SPLASH_DELAY_MS = 1_000L }Also applies to: 127-136
📜 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/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt(3 hunks)feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashPresenter.kt(5 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt (5)
core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/HandleException.kt (1)
handleException(19-46)core/network/src/main/kotlin/com/ninecraft/booket/core/network/service/ReedService.kt (2)
logout(40-41)withdraw(43-44)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultAuthRepository.kt (2)
logout(28-31)withdraw(33-36)core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/AuthRepository.kt (2)
logout(9-9)withdraw(11-11)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultRemoteConfigRepository.kt (1)
getLatestVersion(15-26)
⏰ 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 (11)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt (4)
11-11: RemoteConfigRepository 주입 및 사용 방향성 적절합니다DI로 RemoteConfigRepository를 주입해 설정 화면에서 원격 버전 정보를 취득하도록 한 점 좋습니다.
Also applies to: 31-31
129-132: 사이드 이펙트 초기화 이벤트 처리 LGTMUI에서 소비 후 초기화하는 패턴으로 일관성이 좋습니다.
102-125: 취소 안전성 확인: fetch 중 화면 이탈 시 continuation 재개 가능성DefaultRemoteConfigRepository.getLatestVersion는 suspendCancellableCoroutine에서 Task 리스너로 continuation.resume(...)을 호출합니다. 화면 이탈로 코루틴이 취소된 뒤 resume가 호출되면 IllegalStateException이 날 수 있습니다. 리포지토리 측에서 isActive 체크 후 resume하거나, invokeOnCancellation로 리스너 해제 처리가 필요합니다.
예시(리포지토리 쪽 권장 구현 스케치):
override suspend fun getLatestVersion(): Result<String> = suspendCancellableCoroutine { cont -> val task = remoteConfig.fetchAndActivate() task.addOnCompleteListener { t -> if (!cont.isActive) return@addOnCompleteListener if (t.isSuccessful) { val value = remoteConfig[KEY_LATEST_VERSION].asString() cont.resume(Result.success(value)) } else { cont.resume(Result.failure(t.exception ?: Exception("Unknown error"))) } } cont.invokeOnCancellation { // 필요 시 Task 취소/리스너 제거 } }present()에서 rememberCoroutineScope는 컴포지션 수명과 함께 취소되므로, 위 보완이 있으면 안전합니다.
188-189: UiState에 latestVersion 추가 노출 적절합니다상태에 최신 버전 값을 포함해 UI에서 단순히 구독하도록 한 점이 명확합니다.
feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashPresenter.kt (7)
12-12: RemoteConfigRepository 의존성 추가 👍설정 화면/스플래시에서 원격 업데이트 확인을 위한 DI 추가가 타당합니다.
21-21: Logger 도입 👍예외 로깅에 활용되는 Logger 추가는 적절합니다. 아래 코멘트에서 스택트레이스까지 기록하도록 로깅 형태를 약간 보완하는 제안을 드립니다.
46-47: 상태 보존에 rememberRetained 사용 적절
- 강제 업데이트 다이얼로그 표시 여부와 one-shot 사이드이펙트 상태를 retained state로 관리한 점이 Circuit 패턴과 잘 맞습니다.
70-96: 네비게이션 분기 로직 명확함온보딩/자동로그인 상태 조합에 따른 분기가 명확하며, 약관 동의 확인을 별도 비동기로 처리한 구성도 타당합니다.
115-126: 이벤트/사이드이펙트 처리 적절
- 강제 업데이트 버튼 클릭 시 외부 네비게이션을 사이드이펙트로 분리했고, 소모 후 Init 이벤트로 정리하는 패턴이 명확합니다.
137-141: UI 상태 반환 구성 적절새 필드(isForceUpdateDialogVisible, sideEffect, eventSink)를 통해 Presenter가 UI 표시/사이드이펙트/이벤트를 깔끔히 주도합니다.
34-39: RemoteConfigRepository DI 바인딩 확인됨
RemoteConfigRepository는 이미 Hilt 모듈에서 바인딩되고 있어 추가 작업이 불필요합니다.
- core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/RemoteConfigRepository.kt: 인터페이스 정의
- core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/di/RepositoryModule.kt:41
@Binds fun bindRemoteConfigRepository(defaultRemoteConfigRepository: DefaultRemoteConfigRepository): RemoteConfigRepository
|
|
||
| sealed interface SplashUiEvent : CircuitUiEvent { | ||
| data object InitSideEffect : SplashUiEvent | ||
| data object OnUpdateButtonClick : SplashUiEvent |
There was a problem hiding this comment.
클릭 이벤트를 SideEffect로 처리하는 기준이 있을까요??
seoyoon513
left a comment
seoyoon513
left a comment
There was a problem hiding this comment.
혼돈의 Splash 고생하셨습니다 👍👍
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultRemoteConfigRepository.kt (2)
16-27: 코루틴 취소 시 resume 중복 호출 및 실패 시 캐시 폴백 부재 — 안전 가드/폴백 추가 필요suspendCancellableCoroutine + addOnCompleteListener 조합에서는 취소 후에도 리스너가 호출될 수 있어 IllegalStateException 위험이 있습니다. 또한 fetch 실패 시 활성화된 값으로 폴백하면 UX가 개선됩니다. 성공 시 빈 문자열도 "Unknown" 등으로 정규화하세요.
override suspend fun getLatestVersion(): Result<String> = suspendCancellableCoroutine { continuation -> - remoteConfig.fetchAndActivate().addOnCompleteListener { task -> - if (task.isSuccessful) { - val latestVersion = remoteConfig[KEY_LATEST_VERSION].asString() - Logger.d("LatestVersion: $latestVersion") - continuation.resume(Result.success(latestVersion)) - } else { - Logger.e(task.exception, "getLatestVersion failed") - continuation.resume(Result.failure(task.exception ?: Exception("Unknown error"))) - } - } + remoteConfig.fetchAndActivate().addOnCompleteListener { task -> + if (!continuation.isActive) return@addOnCompleteListener + if (task.isSuccessful) { + val latestVersion = remoteConfig[KEY_LATEST_VERSION] + .asString() + .takeIf { it.isNotBlank() } ?: "Unknown" + Logger.d("LatestVersion: $latestVersion") + continuation.resume(Result.success(latestVersion)) + } else { + Logger.e(task.exception, "getLatestVersion failed") + // fetch 실패 시 현재 활성화된 값으로 폴백 + val cached = remoteConfig[KEY_LATEST_VERSION].asString() + if (cached.isNotBlank()) { + continuation.resume(Result.success(cached)) + } else { + continuation.resume(Result.failure(task.exception ?: Exception("Unknown error"))) + } + } + } }추가로 의도를 명확히 하려면 다음을 등록해도 좋습니다(선택):
continuation.invokeOnCancellation { /* no-op: listener가 나중에 와도 무시됨 */ }
29-40: shouldUpdate()도 동일 취소 가드/캐시 폴백 필요최소 지원 버전 판단 경로에도 동일한 가드를 적용해야 안전합니다. 또한 빈 문자열을 최소 버전으로 받으면 항상 '업데이트 불필요'가 되어 버리므로 폴백을 고려하세요.
override suspend fun shouldUpdate(): Result<Boolean> = suspendCancellableCoroutine { continuation -> - remoteConfig.fetchAndActivate().addOnCompleteListener { task -> - if (task.isSuccessful) { - val minVersion = remoteConfig[KEY_MIN_VERSION].asString() - val currentVersion = BuildConfig.APP_VERSION - continuation.resume(Result.success(isUpdateRequired(currentVersion, minVersion))) - } else { - Logger.e(task.exception, "shouldUpdate: getMinVersion failed") - continuation.resume(Result.failure(task.exception ?: Exception("Unknown error"))) - } - } + remoteConfig.fetchAndActivate().addOnCompleteListener { task -> + if (!continuation.isActive) return@addOnCompleteListener + if (task.isSuccessful) { + val minVersion = remoteConfig[KEY_MIN_VERSION] + .asString() + .takeIf { it.isNotBlank() } ?: "0.0.0" + val currentVersion = BuildConfig.APP_VERSION + continuation.resume(Result.success(isUpdateRequired(currentVersion, minVersion))) + } else { + Logger.e(task.exception, "shouldUpdate: getMinVersion failed") + val cachedMin = remoteConfig[KEY_MIN_VERSION].asString() + if (cachedMin.isNotBlank()) { + val currentVersion = BuildConfig.APP_VERSION + continuation.resume(Result.success(isUpdateRequired(currentVersion, cachedMin))) + } else { + continuation.resume(Result.failure(task.exception ?: Exception("Unknown error"))) + } + } + } }feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt (1)
37-37: 여러 작업이 공유 isLoading을 토글 → 로딩 상태 경합 위험 (분리/합성 권장)로그아웃/탈퇴/버전조회가 하나의 isLoading을 토글하여 서로의 로딩 표시를 끌 수 있습니다. 작업별 플래그로 분리해 최종 isLoading을 OR 합성하세요.
- var isLoading by rememberRetained { mutableStateOf(false) } + var isAuthInProgress by rememberRetained { mutableStateOf(false) } + var isLatestVersionLoading by rememberRetained { mutableStateOf(false) } @@ - fun logout() { + fun logout() { scope.launch { try { - isLoading = true + isAuthInProgress = true authRepository.logout() @@ } finally { - isLoading = false + isAuthInProgress = false isLogoutDialogVisible = false } } } @@ - fun withdraw() { + fun withdraw() { scope.launch { try { - isLoading = true + isAuthInProgress = true authRepository.withdraw() @@ } finally { - isLoading = false + isAuthInProgress = false isWithdrawBottomSheetVisible = false } } } @@ - fun getLatestVersion() { + fun getLatestVersion() { scope.launch { try { - isLoading = true + isLatestVersionLoading = true remoteConfigRepository.getLatestVersion() @@ - } finally { - isLoading = false + } finally { + isLatestVersionLoading = false } } } @@ - return SettingsUiState( - isLoading = isLoading, + return SettingsUiState( + isLoading = isAuthInProgress || isLatestVersionLoading, isLogoutDialogVisible = isLogoutDialogVisible, isWithdrawBottomSheetVisible = isWithdrawBottomSheetVisible, isWithdrawConfirmed = isWithdrawConfirmed, latestVersion = latestVersion, isOptionalUpdateDialogVisible = isOptionalUpdateDialogVisible, sideEffect = sideEffect, eventSink = ::handleEvent, )Also applies to: 45-71, 74-101, 103-126, 196-205
🧹 Nitpick comments (12)
core/common/src/main/kotlin/com/ninecraft/booket/core/common/extensions/Context.kt (1)
3-11: Play Store 미설치 단말/비-Activity Context 안전성 보강을 권장합니다현재 구현은:
- Play Store 앱이 없는 환경(중국/에뮬레이터 등)에서 ActivityNotFoundException 발생 가능
- Activity가 아닌 Context(예: Application)에서 호출 시 FLAG_ACTIVITY_NEW_TASK 미지정으로 예외 발생 가능
아래처럼 예외 처리 + 웹 URL 폴백 + NEW_TASK 플래그를 추가하면 안정성이 크게 올라갑니다.
-import android.content.Context -import android.content.Intent -import androidx.core.net.toUri +import android.app.Activity +import android.content.ActivityNotFoundException +import android.content.Context +import android.content.Intent +import android.net.Uri import com.ninecraft.booket.core.common.BuildConfig fun Context.openPlayStore() { - val intent = Intent(Intent.ACTION_VIEW, "market://details?id=${BuildConfig.PACKAGE_NAME}".toUri()) - startActivity(intent) + val appId = BuildConfig.PACKAGE_NAME + val marketUri = Uri.parse("market://details?id=$appId") + val webUri = Uri.parse("https://play.google.com/store/apps/details?id=$appId") + + val marketIntent = Intent(Intent.ACTION_VIEW, marketUri).apply { + // Play Store 앱 우선 + setPackage("com.android.vending") + if (this@openPlayStore !is Activity) addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) + } + try { + startActivity(marketIntent) + } catch (_: ActivityNotFoundException) { + // Play Store 앱이 없으면 웹으로 폴백 + val webIntent = Intent(Intent.ACTION_VIEW, webUri).apply { + if (this@openPlayStore !is Activity) addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) + } + startActivity(webIntent) + } }feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/component/SettingItem.kt (1)
24-30: Modifier 분기 간소화로 가독성 개선 제안현재 if-else로 Modifier를 두 갈래로 구성하고 있습니다. 한 줄 체인으로 정리하면 가독성이 좋아집니다.
- val combinedModifier = if (isClickable) { - modifier - .fillMaxWidth() - .clickableSingle { onItemClick() } - } else { - modifier.fillMaxWidth() - } + val combinedModifier = modifier + .fillMaxWidth() + .let { if (isClickable) it.clickableSingle { onItemClick() } else it }또한 접근성 향상을 위해 클릭 가능 시 role(Button) 부여를 고려해 주세요(가능하다면 clickableSingle 내부에서 or 여기서 semantics { role = Role.Button }).
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsUiState.kt (2)
13-15: UI 상태 필드 추가 방향성 OK — 파생 가능 속성 여부만 검토
- latestVersion, isUpdateAvailable, isOptionalUpdateDialogVisible 추가로 요구사항을 충족합니다.
- isUpdateAvailable는 latestVersion과 앱 버전 비교에서 파생될 수 있는 값이라, 프레젠터에서 계산해 주입하거나 파생 속성으로 관리하는 대안도 있습니다(현재 구조도 문제 없음).
30-45: (선택) Compose 안정성 힌트: @immutable 주석 범위 확대
- SettingsSideEffect엔 @immutable이 적용되어 있으나, SettingsUiEvent에도 동일 주석을 부여하면(모든 멤버가 안정 타입이므로) 리컴포지션 힌트를 통해 미세 최적화에 도움이 됩니다.
feature/settings/src/main/res/values/strings.xml (1)
17-20: 문구/포맷 깔끔합니다 — 미세 워딩 제안 1건
- latest_version: 포맷 인자 사용 적절. 디자인 톤앤매너에 따라 콜론 포함 형태('최신 버전: %1$s')가 더 읽기 쉬울 수 있어 참고만 남깁니다.
core/common/src/main/kotlin/com/ninecraft/booket/core/common/util/VersionUtils.kt (1)
15-31: 세 자리 고정/엄격한 정규식으로 인한 잘못된 비교 가능성 — 유연한 파싱으로 개선 권장현재 "숫자.숫자.숫자" 형식이 아니면 0을 반환해 '같음'으로 판단됩니다. 실제 앱 버전은 "1.2" 또는 "1.2.3-beta01" 같이 다양합니다. 빈 문자열/접미사(β, rc) 등에서도 의도치 않은 결과가 나올 수 있어, 숫자 토큰만 추출해 부족한 자리는 0으로 패딩하는 방식이 안전합니다. 또한 산술 뺄셈 대신 compareTo를 써 오버플로/가독성을 개선합니다.
아래처럼 변경을 제안합니다.
fun compareVersions(version1: String, version2: String): Int { - Logger.d("compareVersions: version1: $version1, version2: $version2") - - if (!Regex("""^\d+\.\d+\.\d+$""").matches(version1)) return 0 - if (!Regex("""^\d+\.\d+\.\d+$""").matches(version2)) return 0 - - val v1 = version1.split('.').map { it.toInt() } - val v2 = version2.split('.').map { it.toInt() } - - // 메이저 버전 비교 - if (v1[0] != v2[0]) return v1[0] - v2[0] - - // 마이너 버전 비교 - if (v1[1] != v2[1]) return v1[1] - v2[1] - - // 패치 버전 비교 - return v1[2] - v2[2] + Logger.d("compareVersions: version1: $version1, version2: $version2") + + // 숫자 토큰만 추출(예: "1.2.3-beta1" -> [1,2,3,1]) + val extract = { v: String -> + Regex("""\d+""").findAll(v).map { it.value.toInt() }.toList() + } + val v1 = extract(version1).let { it + List((3 - it.size).coerceAtLeast(0)) { 0 } }.take(3) + val v2 = extract(version2).let { it + List((3 - it.size).coerceAtLeast(0)) { 0 } }.take(3) + + repeat(3) { idx -> + val cmp = v1[idx].compareTo(v2[idx]) + if (cmp != 0) return cmp + } + return 0 }feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsUi.kt (4)
67-69: 빈 latestVersion 대비 및 불필요 비교/로그 최소화latestVersion이 공백일 때 compareVersions 호출은 의미가 없고 로그만 남습니다. 공백 체크 후 비교하도록 가볍게 방어하세요.
- val isUpdateAvailable = remember(appVersion, state.latestVersion) { - compareVersions(state.latestVersion, appVersion) > 0 - } + val isUpdateAvailable = remember(appVersion, state.latestVersion) { + state.latestVersion.isNotBlank() && + compareVersions(state.latestVersion, appVersion) > 0 + }
96-101: 장식용 아이콘의 contentDescription 제거(접근성 개선)해당 화살표는 순수 장식 목적이라 보이며, 스크린리더 노이즈를 줄이기 위해 contentDescription을 null로 두는 것이 권장됩니다.
- Icon( - imageVector = ImageVector.vectorResource(id = designR.drawable.ic_chevron_right), - contentDescription = "Right Chevron Icon", - tint = Color.Unspecified, - ) + Icon( + imageVector = ImageVector.vectorResource(id = designR.drawable.ic_chevron_right), + contentDescription = null, + tint = Color.Unspecified, + )같은 변경을 해당 파일 내 다른 오른쪽 화살표 아이콘에도 적용해 주세요.
Also applies to: 109-114, 122-128, 146-151
139-144: 업데이트 불가(비클릭) 상태에서는 강조색 대신 기본 텍스트 색 추천업데이트 불가인데도 브랜드 색을 유지하면 클릭 기대감을 줄 수 있습니다. 상태에 따라 색을 분기하면 더 명확합니다.
- Text( - text = appVersion, - style = ReedTheme.typography.body1Medium, - color = ReedTheme.colors.contentBrand, - ) + Text( + text = appVersion, + style = ReedTheme.typography.body1Medium, + color = if (isUpdateAvailable) + ReedTheme.colors.contentBrand + else + ReedTheme.colors.contentPrimary, + )
155-159: 최신 버전 문자열 공백 시 플레이스홀더 표기Remote Config가 비어 있으면 "최신 버전: "만 보여 UX가 어색합니다. 공백이면 대시(—) 등으로 보강하세요.
- Text( - text = stringResource(R.string.latest_version, state.latestVersion), + Text( + text = if (state.latestVersion.isNotBlank()) + stringResource(R.string.latest_version, state.latestVersion) + else + stringResource(R.string.latest_version, "—"), color = ReedTheme.colors.contentTertiary, style = ReedTheme.typography.label1Medium, )feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt (2)
107-111: 빈 최신 버전 문자열 정규화Remote Config가 빈 문자열을 반환할 수 있습니다. UI/비교 로직을 안정화하려면 공백을 보정하세요(예: "Unknown" 또는 "—").
- .onSuccess { version -> - latestVersion = version - } + .onSuccess { version -> + latestVersion = version.takeIf { it.isNotBlank() } ?: "Unknown" + }
186-188: 업데이트 버튼 클릭 시 다이얼로그 닫기 추가 제안스토어로 이동 트리거와 함께 다이얼로그를 닫아 복귀 시 중복 노출을 방지하는 것이 일반적입니다.
- is SettingsUiEvent.OnUpdateButtonClick -> { - sideEffect = SettingsSideEffect.NavigateToPlayStore - } + is SettingsUiEvent.OnUpdateButtonClick -> { + isOptionalUpdateDialogVisible = false + sideEffect = SettingsSideEffect.NavigateToPlayStore + }
📜 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/common/build.gradle.kts(1 hunks)core/common/src/main/kotlin/com/ninecraft/booket/core/common/extensions/Context.kt(1 hunks)core/common/src/main/kotlin/com/ninecraft/booket/core/common/util/VersionUtils.kt(1 hunks)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultRemoteConfigRepository.kt(1 hunks)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/HandlingSettingsSideEffect.kt(2 hunks)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt(3 hunks)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsUi.kt(9 hunks)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsUiState.kt(3 hunks)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/component/SettingItem.kt(1 hunks)feature/settings/src/main/res/values/strings.xml(1 hunks)feature/splash/src/main/kotlin/com/ninecraft/booket/splash/HandleSplashSideEffects.kt(1 hunks)feature/splash/src/main/res/values/strings.xml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- feature/splash/src/main/res/values/strings.xml
- feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/HandlingSettingsSideEffect.kt
- feature/splash/src/main/kotlin/com/ninecraft/booket/splash/HandleSplashSideEffects.kt
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-20T12:34:23.786Z
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)만 별도로 선언하면 된다.
Applied to files:
core/common/build.gradle.kts
🧬 Code graph analysis (4)
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultRemoteConfigRepository.kt (1)
core/common/src/main/kotlin/com/ninecraft/booket/core/common/util/VersionUtils.kt (1)
isUpdateRequired(41-43)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/component/SettingItem.kt (1)
core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/theme/Theme.kt (1)
ReedTheme(14-21)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt (5)
core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/HandleException.kt (1)
handleException(19-46)core/network/src/main/kotlin/com/ninecraft/booket/core/network/service/ReedService.kt (2)
logout(40-41)withdraw(43-44)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultAuthRepository.kt (2)
logout(28-31)withdraw(33-36)core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/AuthRepository.kt (2)
logout(9-9)withdraw(11-11)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultRemoteConfigRepository.kt (1)
getLatestVersion(16-27)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsUi.kt (4)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/HandlingSettingsSideEffect.kt (1)
HandleSettingsSideEffects(9-33)core/common/src/main/kotlin/com/ninecraft/booket/core/common/util/VersionUtils.kt (1)
compareVersions(15-32)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/component/SettingItem.kt (1)
SettingItem(15-54)core/ui/src/main/kotlin/com/ninecraft/booket/core/ui/component/ReedDialog.kt (1)
ReedDialog(25-114)
🔇 Additional comments (10)
core/common/build.gradle.kts (2)
13-19: BuildConfig 활성화 및 PACKAGE_NAME 상수 주입 접근, 목적에 부합합니다
- 라이브러리 모듈에서 BuildConfig 생성 + 고정 패키지명을 상수로 노출해 Play Store 이동에 일관된 ID를 쓰는 전략은 디버그/내부 빌드(애플리케이션 ID 서픽스 존재)에서도 실제 상용 패키지 상세 페이지로 유도한다는 점에서 합리적입니다.
- 버전 카탈로그(libs.versions.packageName)로 단일 소스 유지도 좋습니다.
현재 구조 그대로 진행해도 무방합니다.
17-19: PACKAGE_NAME 버전 카탈로그 정의 확인 완료
- gradle/libs.versions.toml 84행에
packageName = "com.ninecraft.booket"으로 정의되어 있어, 해당 키가 정상적으로 존재함을 확인했습니다.- 현 시점에서는 화이트라벨/멀티 패키지 전략 도입 시점까지 이 값으로 문제없이 사용 가능할 것으로 보입니다.
추가 조치 불필요합니다.
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/component/SettingItem.kt (1)
15-54: 재사용성 높은 컴포저블 구성, 슬롯 구조와 클릭 제어가 깔끔합니다
- title/description/action 슬롯 구성으로 확장성 확보
- isClickable + clickableSingle로 중복 탭 방지 기대
- weight(1f) 배치로 trailing 액션 정렬 자연스러움
현 상태로 도입해도 문제 없습니다.
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsUiState.kt (1)
27-28: NavigateToPlayStore 사이드이펙트 추가 적합
- Settings와 스플래시 모두에서 동일한 내비게이션 패턴을 재사용할 수 있어 일관성이 좋아집니다.
core/common/src/main/kotlin/com/ninecraft/booket/core/common/util/VersionUtils.kt (1)
41-43: isUpdateRequired 구현 적합현재/최소 버전 비교 결과를 그대로 활용하는 단순·명확한 로직입니다. 별다른 우려 없습니다.
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultRemoteConfigRepository.kt (1)
42-45: Remote Config 키 상수 적정KEY_LATEST_VERSION/KEY_MIN_VERSION이 분리되어 있어 혼선이 해소되었습니다. Console의 실제 키명과 동일한지 마지막으로만 확인 부탁드립니다.
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsUi.kt (2)
52-55: 사이드이펙트 초기화 연동 OKHandleSettingsSideEffects에 eventSink를 전달해 소비 후 초기화하는 패턴이 명확합니다.
224-236: 선택적 업데이트 다이얼로그 구성 적절확인/취소, 설명/제목 구성과 사이드이펙트 연동이 깔끔합니다.
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt (2)
130-133: 사이드이펙트 초기화 이벤트 처리 적절소비 후 null로 리셋하는 패턴은 재발행 방지에 효과적입니다.
192-194: 초기 버전 조회 트리거 적절RememberedEffect로 1회 트리거 패턴이 명확합니다.
| } | ||
|
|
||
| sealed interface SettingsUiEvent : CircuitUiEvent { | ||
| data object InitSideEffect : SettingsUiEvent |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
신규 이벤트들의 처리 누락 여부 점검 요청
- InitSideEffect, OnVersionClick, OnOptionalUpdateDialogDismiss, OnUpdateButtonClick가 추가되었습니다. 프레젠터/핸들러에서 when 처리 누락 시 런타임 미동작 위험이 있습니다.
아래 스크립트로 처리부를 빠르게 훑을 수 있습니다(예상: 각 이벤트가 SettingsPresenter/HandlingSettingsSideEffect 등에서 최소 1회 이상 매칭).
Also applies to: 42-45
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 이벤트 정의 위치 재확인
rg -nP 'sealed interface\s+SettingsUiEvent\b' -C2 --type=kt
# 각 이벤트 핸들링(when 분기 포함) 검색
for ev in InitSideEffect OnVersionClick OnOptionalUpdateDialogDismiss OnUpdateButtonClick; do
echo "== Searching handlers for $ev =="
rg -nP "$ev\b" -C3 --type=kt | rg -nP -C2 'when\s*\(|handle|on\w+\(' || true
doneLength of output: 108
rg가 --type=kt를 인식하지 못해 검색이 실행되지 않았습니다. 파일 타입 필터 대신 확장자 필터(-g '*.kt')를 사용해 다시 검사해보세요. 아래 스크립트를 실행해 주세요.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== 이벤트 정의 위치 재확인 =="
rg -nP 'sealed interface\s+SettingsUiEvent\b' -C2 -g '*.kt'
echo "== 이벤트 정의 확인 =="
rg -nP 'data object\s+(InitSideEffect|OnVersionClick|OnOptionalUpdateDialogDismiss|OnUpdateButtonClick)\b' -C2 -g '*.kt'
echo "== 핸들러 클래스 위치 확인 =="
rg -nP 'class\s+(SettingsPresenter|HandlingSettingsSideEffect)\b' -C2 -g '*.kt'
for ev in InitSideEffect OnVersionClick OnOptionalUpdateDialogDismiss OnUpdateButtonClick; do
echo "== 핸들링 검색: $ev =="
rg -nP "$ev\b" -C3 -g '*.kt' | rg -nP -C2 'when\s*\(|handle|on[A-Z]\w*\(' || echo " -> '$ev' 처리부 미발견"
doneLength of output: 8946
핸들링 누락 이벤트 확인 및 추가 구현 필요
다음 신규 이벤트들이 SettingsUiState.kt(lines 42–44)에 추가되었으나, SettingsPresenter.handleEvent에서는 InitSideEffect만 처리되고 다른 분기가 없습니다. 누락 시 해당 동작(버전 클릭, 업데이트 다이얼로그 해제, 업데이트 버튼 클릭)이 런타임에서 무시됩니다.
• 누락 이벤트
- OnVersionClick (SettingsUiState.kt 42)
- OnOptionalUpdateDialogDismiss (SettingsUiState.kt 43)
- OnUpdateButtonClick (SettingsUiState.kt 44)
• 확인된 처리부 위치
- feature/settings/src/main/kotlin/.../SettingsPresenter.kt
fun handleEvent(event: SettingsUiEvent) { when (event) { is SettingsUiEvent.InitSideEffect -> { sideEffect = null } // ↓ 여기에 각 이벤트 분기 추가 필요 } }
요구사항
- 위 세 이벤트에 대한
when분기 추가 - 각 분기에서 적절한 로직 또는
sideEffect설정 구현
🤖 Prompt for AI Agents
In
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt
around lines 31–44, the when in handleEvent only handles InitSideEffect and is
missing branches for OnVersionClick, OnOptionalUpdateDialogDismiss and
OnUpdateButtonClick; add three when branches for these events and in each set
the appropriate sideEffect or state change (e.g., OnVersionClick should set
sideEffect to trigger showing the version/details dialog or navigation,
OnOptionalUpdateDialogDismiss should clear the update dialog sideEffect and any
related flags, and OnUpdateButtonClick should set sideEffect to start the update
flow or emit an UpdateTriggered side effect), ensuring sideEffect is
reset/cleared where appropriate to avoid stale effects.
# Conflicts: # core/common/src/main/kotlin/com/ninecraft/booket/core/common/extensions/Context.kt
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
core/common/src/main/kotlin/com/ninecraft/booket/core/common/extensions/Context.kt (2)
47-48: 가독성 소폭 개선 제안(선택)동작에는 문제 없습니다. 다만 두 줄로 개행된 대입을 한 줄로 합치면 읽기성이 좋아집니다.
- val imageUri = - contentResolver.insert(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, contentValues) + val imageUri = contentResolver.insert( + MediaStore.Images.Media.EXTERNAL_CONTENT_URI, + contentValues + )
5-7: BuildConfig.PACKAGE_NAME 주입 위치 및 applicationIdSuffix 충돌 위험 확인 완료검증 결과
- libs.versions.toml:
packageName = "com.ninecraft.booket"- core/common/build.gradle.kts (라인 18):
buildConfigField("String", "PACKAGE_NAME", "\"${libs.versions.packageName.get()}\"")- app/build.gradle.kts (라인 31):
applicationIdSuffix = ".dev"위 설정으로 인해
- dev/staging 빌드 시 런타임
context.packageName이"com.ninecraft.booket.dev"등이 되지만BuildConfig.PACKAGE_NAME은 항상"com.ninecraft.booket"으로 고정 주입되어, 마켓 URL(market://details?id=…)을 호출할 때 프로덕션(기본) 패키지로 연결됩니다.✔️ 확인 및 고려 사항
- Play Console에서 실제 릴리스 applicationId(
com.ninecraft.booket)와libs.versions.toml의 동기화 여부 재확인- 환경별(개발/테스트)로 별도 스토어 리스팅을 운영해야 한다면,
Context.openPlayStore()확장 함수에packageName: String = BuildConfig.PACKAGE_NAME파라미터를 추가하여 유연하게 처리
📜 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)
core/common/src/main/kotlin/com/ninecraft/booket/core/common/extensions/Context.kt(3 hunks)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsUi.kt(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsUi.kt
| fun Context.openPlayStore() { | ||
| val intent = | ||
| Intent(Intent.ACTION_VIEW, "market://details?id=${BuildConfig.PACKAGE_NAME}".toUri()) | ||
| startActivity(intent) | ||
| } |
There was a problem hiding this comment.
Play 스토어 미탑재 기기/에뮬레이터에서 ActivityNotFoundException로 크래시 가능 — 핸들러 확인, 웹 URL 폴백, NEW_TASK 플래그 추가 필요
market 스킴을 처리할 앱이 없는 환경(일부 AOSP 기기/중국 ROM/에뮬레이터 등)에서 startActivity가 예외를 던질 수 있습니다. 또한 비-Activity Context에서 호출될 수 있는 확장 함수 특성상 NEW_TASK 플래그가 필요합니다. 아래와 같이 안전하게 처리해 주세요.
-fun Context.openPlayStore() {
- val intent =
- Intent(Intent.ACTION_VIEW, "market://details?id=${BuildConfig.PACKAGE_NAME}".toUri())
- startActivity(intent)
-}
+fun Context.openPlayStore() {
+ val marketUri = "market://details?id=${BuildConfig.PACKAGE_NAME}".toUri()
+ val marketIntent = Intent(Intent.ACTION_VIEW, marketUri).apply {
+ if (this@openPlayStore !is Activity) addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)
+ }
+
+ val pm = packageManager
+ if (marketIntent.resolveActivity(pm) != null) {
+ startActivity(marketIntent)
+ return
+ }
+
+ // Fallback: 웹 URL
+ val webUri = "https://play.google.com/store/apps/details?id=${BuildConfig.PACKAGE_NAME}".toUri()
+ val webIntent = Intent(Intent.ACTION_VIEW, webUri).apply {
+ if (this@openPlayStore !is Activity) addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)
+ }
+ try {
+ startActivity(webIntent)
+ } catch (e: ActivityNotFoundException) {
+ Logger.w("No handler for Play Store intents: ${e.message}")
+ }
+}위 변경에 따라 필요한 import(파일 상단)에 아래를 추가해 주세요.
+import android.app.Activity
+import android.content.ActivityNotFoundException📝 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.
| fun Context.openPlayStore() { | |
| val intent = | |
| Intent(Intent.ACTION_VIEW, "market://details?id=${BuildConfig.PACKAGE_NAME}".toUri()) | |
| startActivity(intent) | |
| } | |
| // Add at the top of the file: | |
| import android.app.Activity | |
| import android.content.ActivityNotFoundException | |
| fun Context.openPlayStore() { | |
| val marketUri = "market://details?id=${BuildConfig.PACKAGE_NAME}".toUri() | |
| val marketIntent = Intent(Intent.ACTION_VIEW, marketUri).apply { | |
| if (this@openPlayStore !is Activity) { | |
| addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) | |
| } | |
| } | |
| val pm = packageManager | |
| if (marketIntent.resolveActivity(pm) != null) { | |
| startActivity(marketIntent) | |
| return | |
| } | |
| // Fallback to web URL if no market handler is available | |
| val webUri = "https://play.google.com/store/apps/details?id=${BuildConfig.PACKAGE_NAME}".toUri() | |
| val webIntent = Intent(Intent.ACTION_VIEW, webUri).apply { | |
| if (this@openPlayStore !is Activity) { | |
| addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) | |
| } | |
| } | |
| try { | |
| startActivity(webIntent) | |
| } catch (e: ActivityNotFoundException) { | |
| Logger.w("No handler for Play Store intents: ${e.message}") | |
| } | |
| } |
🤖 Prompt for AI Agents
In
core/common/src/main/kotlin/com/ninecraft/booket/core/common/extensions/Context.kt
around lines 65–69, wrap the startActivity call in a try/catch for
android.content.ActivityNotFoundException and, when caught, fall back to opening
the Play Store web URL
(https://play.google.com/store/apps/details?id=<PACKAGE_NAME>), and ensure the
Intent uses FLAG_ACTIVITY_NEW_TASK so non-Activity Contexts can launch it; also
add the needed imports (android.content.ActivityNotFoundException and
Intent.FLAG_ACTIVITY_NEW_TASK or android.content.Intent) at the file top.
2 participants
🔗 관련 이슈
📙 작업 설명
🧪 테스트 내역 (선택)
📸 스크린샷 또는 시연 영상 (선택)
💬 추가 설명 or 리뷰 포인트 (선택)
Summary by CodeRabbit
신기능
UI
현지화
기타