Conversation
…toryimpl 디렉토리로 통합하여 도메인 중심 구조로 정리
…ncy-cleanup [REFACTOR] 모듈간의 의존성을 깔끔하게 정리해보자
- 모든 유닛 테스트에 대해 Jacoco 설정 자동 적용 - Android 모듈(Application, Library)에 유닛 테스트 커버리지 활성화 - 커버리지 리포트를 생성하는 generateTestCoverageReport Task 등록 - .exec 파일 미존재 시 createDebugUnitTestCoverageReport Task 자동 스킵
[FEAT] Junit4 테스트 환경을 설정해보자.
…-status [REFACTOR] 운세 생성 상태 관리용 FortuneCreateStatusFlow 도입
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (17)
feature/mission/src/main/java/com/yapp/mission/MissionViewModel.kt (2)
133-136: 마지막 스텝에서 햅틱이 이중으로 재생됩니다handleMissionProgress(마지막 단계)에서 이미 성공 햅틱을 발생시키고, completeMission에서도 다시 호출합니다. 중복 진동을 제거하세요.
적용 diff:
private fun completeMission(type: String) = intent { - performHapticSuccess() logMissionSuccess(type)
137-140: 변수명 오타(fortuneCreateStaus) → fortuneCreateStatus가독성을 위해 정정 권장합니다.
- val fortuneCreateStaus = fortuneRepository.fortuneCreateStatusFlow.first() + val fortuneCreateStatus = fortuneRepository.fortuneCreateStatusFlow.first() - when (fortuneCreateStaus) { + when (fortuneCreateStatus) {core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmInteractionActivityReceiver.kt (3)
34-44: 미션 데이터 유효성: missionCount=0 처리 여부 확인현재 -1만 배제합니다. 0 회차가 유효하지 않다면 조건을 > 0으로 강화하세요.
- missionCount != -1 + missionCount > 0
55-82: 중복 Fortune 인텐트 생성 제거 권장두 분기(Creating, Success+hasUnseenFortune)에서 동일 인텐트를 중복 생성합니다. 헬퍼로 추출해 중복 제거하세요.
적용 diff(해당 범위 치환):
- is FortuneCreateStatus.Creating -> { - context?.let { ctx -> - val uri = "orbitapp://fortune".toUri() - val fortuneIntent = Intent(Intent.ACTION_VIEW, uri).apply { - addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) - setPackage(ctx.packageName) - } - ctx.startActivity(fortuneIntent) - } - } + is FortuneCreateStatus.Creating -> { + context?.launchFortune() + } @@ - is FortuneCreateStatus.Success -> { - if (hasUnseenFortune) { - context?.let { ctx -> - val uri = "orbitapp://fortune".toUri() - val fortuneIntent = - Intent(Intent.ACTION_VIEW, uri).apply { - addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) - setPackage(ctx.packageName) - } - ctx.startActivity(fortuneIntent) - } - } - } + is FortuneCreateStatus.Success -> { + if (hasUnseenFortune) context?.launchFortune() + }클래스 내부에 다음 헬퍼 추가:
private fun Context.launchFortune() { val uri = "orbitapp://fortune".toUri() val fortuneIntent = Intent(Intent.ACTION_VIEW, uri).apply { addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) setPackage(packageName) } startActivity(fortuneIntent) }
84-93: URI 문자열 수동 조립 대신 Uri.Builder 사용 추천가독성과 안전성(인코딩)을 위해 Builder/Uri 대신 문자열 연결을 지양하세요.
- val uriString = - "orbitapp://mission?notificationId=$notificationId&missionType=${missionType.value}&missionCount=$missionCount" - val missionIntent = - Intent(Intent.ACTION_VIEW, uriString.toUri()).apply { + val uri = android.net.Uri.Builder() + .scheme("orbitapp") + .authority("mission") + .appendQueryParameter("notificationId", notificationId.toString()) + .appendQueryParameter("missionType", missionType.value.toString()) + .appendQueryParameter("missionCount", missionCount.toString()) + .build() + val missionIntent = Intent(Intent.ACTION_VIEW, uri).apply { addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) setPackage(ctx.packageName) }domain/src/main/java/com/yapp/domain/model/FortuneCreateStatus.kt (1)
3-8: 상태 모델링 적절생성 파이프라인을 명확히 캡슐화합니다. 이후 필요 시 Failure에 원인(payload) 확장을 고려해도 좋습니다.
feature/fortune/src/main/java/com/yapp/fortune/worker/PostFortuneWorker.kt (2)
33-38: userId 부재 시 실패 반환 정책 확인 필요현재 Result.failure()로 종료되어 재시도되지 않습니다. 로그인 지연/복귀 시 재시도를 원한다면 Result.retry() 또는 별도 재스케줄 전략이 필요합니다. 제품 의도 확인 부탁드립니다.
41-59: 상태 전이 및 재시도 처리 양호Creating → (성공) Created/점수저장, (실패/예외) Failed 후 retry 흐름이 일관적입니다.
스케줄러 측에서는 UniqueWork + KEEP 사용으로 중복 작업을 억제하는 것을 권장합니다.
feature/fortune/src/main/java/com/yapp/fortune/FortuneViewModel.kt (2)
53-72: 수집 방식 개선 제안: collectLatest + 중복 억제빠른 상태 전이에서 이전 처리 취소와 과도한 재호출 방지를 위해 collectLatest와 distinctUntilChanged를 고려하세요.
- private fun observeFortune() = intent { - fortuneRepository.fortuneCreateStatusFlow.collect { status -> + private fun observeFortune() = intent { + fortuneRepository.fortuneCreateStatusFlow + .distinctUntilChanged() + .collectLatest { status -> when (status) {필요 import:
import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.flow.distinctUntilChanged
61-65: first() 호출의 스레드 고려isFirstAlarmDismissedTodayFlow가 디스크 I/O(DataStore 등)를 동반한다면, withContext(Dispatchers.IO)로 감싸는 것을 권장합니다. 내부가 이미 IO로 전환된다면 현행 유지해도 무방합니다.
data/src/main/java/com/yapp/data/repositoryimpl/FortuneRepositoryImpl.kt (3)
22-22: 네이밍 일관성: Tooltip vs ToolTip 표기 혼재
shouldShowFortuneToolTipFlow만ToolTip(대문자 T, P)로 쓰이고, 다른 곳(예:markFortuneTooltipShown)은Tooltip으로 사용됩니다. 한 가지 표기로 통일해 주세요.적용 예:
- override val shouldShowFortuneToolTipFlow: Flow<Boolean> = fortuneLocalDataSource.shouldShowFortuneToolTipFlow + override val shouldShowFortuneTooltipFlow: Flow<Boolean> = fortuneLocalDataSource.shouldShowFortuneTooltipFlow
27-31: API 네이밍 정렬 제안: markFortuneAs vs markFortune**레포지토리 계층은
markFortuneAsCreating/Created/Failed, 로컬 DS는markFortuneCreating/Created/Failed로 접두사만 달라 가독성이 떨어집니다. 도메인/데이터 계층 전반에서 동일한 패턴으로 통일하는 것을 권장합니다.
40-41: toDomain 변환 예외 가능성 확인 및 map vs mapCatching 선택
mapCatching { it.toDomain() }사용은 안전합니다. 만약toDomain()이 예외를 던지지 않도록 보장된다면map { ... }가 더 간결합니다. 그렇지 않다면 현재 구현 유지가 맞습니다. 확인 부탁드립니다.Also applies to: 45-46
data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSource.kt (2)
12-12: 네이밍 일관성: Tooltip vs ToolTip프로퍼티
shouldShowFortuneToolTipFlow의 표기를Tooltip으로 통일해 주세요(메서드markFortuneTooltipShown과 일치).적용 예:
- val shouldShowFortuneToolTipFlow: Flow<Boolean> + val shouldShowFortuneTooltipFlow: Flow<Boolean>
13-13: “오늘” 기준의 시간대/경계 정의를 인터페이스에서 명시해 주세요
isFirstAlarmDismissedTodayFlow/markFirstAlarmDismissedToday()의 “Today” 기준(로컬 타임존? 서버 타임? 자정 기준?)을 KDoc으로 명시하는 것이 안전합니다. 타임존 변경/서머타임 전환 시 오해 소지가 있습니다.Also applies to: 24-24
data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.kt (2)
23-36: 다중 수집자에서 중복 결합 비용 절감
combine(...).distinctUntilChanged()는 cold flow라 다중 수집 시 결합이 반복됩니다. 핫으로 승격해 캐시를 두면 비용과 지연을 줄일 수 있습니다. 예:stateIn(appScope, SharingStarted.Eagerly, FortuneCreateStatus.Idle).
20-20: 네이밍 일관성: Tooltip vs ToolTip여기서는
shouldShowFortuneToolTipFlow(ToolTip)와markFortuneTooltipShown(Tooltip)이 혼재합니다.Tooltip으로 통일 권장.적용 예:
- override val shouldShowFortuneToolTipFlow = userPreferences.shouldShowFortuneToolTipFlow + override val shouldShowFortuneTooltipFlow = userPreferences.shouldShowFortuneTooltipFlowAlso applies to: 55-57
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmInteractionActivityReceiver.kt(2 hunks)core/datastore/src/main/java/com/yapp/datastore/UserPreferences.kt(2 hunks)data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSource.kt(1 hunks)data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.kt(3 hunks)data/src/main/java/com/yapp/data/repositoryimpl/FortuneRepositoryImpl.kt(2 hunks)domain/src/main/java/com/yapp/domain/model/FortuneCreateStatus.kt(1 hunks)domain/src/main/java/com/yapp/domain/repository/FortuneRepository.kt(1 hunks)feature/fortune/src/main/java/com/yapp/fortune/FortuneViewModel.kt(4 hunks)feature/fortune/src/main/java/com/yapp/fortune/worker/PostFortuneWorker.kt(1 hunks)feature/mission/src/main/java/com/yapp/mission/MissionViewModel.kt(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- core/datastore/src/main/java/com/yapp/datastore/UserPreferences.kt
- domain/src/main/java/com/yapp/domain/repository/FortuneRepository.kt
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-14T15:32:32.587Z
Learnt from: DongChyeon
PR: YAPP-Github/Orbit-Android#252
File: feature/fortune/src/main/java/com/yapp/fortune/worker/PostFortuneWorker.kt:24-56
Timestamp: 2025-09-14T15:32:32.587Z
Learning: 운세 요청 간에는 최소 1분 간격이 보장되어 있어서 PostFortuneWorker에서 동시성/원자성 문제를 고려하지 않아도 됨
Applied to files:
feature/fortune/src/main/java/com/yapp/fortune/worker/PostFortuneWorker.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). (2)
- GitHub Check: build
- GitHub Check: Continuous Deployment
🔇 Additional comments (9)
feature/mission/src/main/java/com/yapp/mission/MissionViewModel.kt (1)
10-10: 적절한 import 추가새 모델/Flow 기반 전환에 필요한 의존성 추가가 타당합니다.
Also applies to: 17-17
core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmInteractionActivityReceiver.kt (2)
45-48: goAsync + finally { pending.finish() } 패턴 적절비동기 처리에서 브로드캐스트 수명 보장을 잘 처리했습니다.
Also applies to: 95-97
49-55: I/O 경계 분리 적절Flow 읽기를 IO 컨텍스트로 옮긴 점 좋습니다.
feature/fortune/src/main/java/com/yapp/fortune/worker/PostFortuneWorker.kt (1)
23-29: 중복 요청 회피 로직 적절Creating/Success 조기 반환으로 중복 포스팅을 방지합니다.
feature/fortune/src/main/java/com/yapp/fortune/FortuneViewModel.kt (2)
33-34: 초기 observeFortune 전환 좋습니다상태 기반 반응형 흐름이 명확해졌습니다.
86-88: 포춘 열람 상태 선반영 좋습니다markFortuneSeen()을 성공 처리 직후 호출하는 순서가 자연스럽습니다.
data/src/main/java/com/yapp/data/repositoryimpl/FortuneRepositoryImpl.kt (1)
36-36: 일괄 초기화 진입점 추가는 적절합니다
clearFortuneData()로 상태를 한 번에 초기화하는 방향은 사용성/일관성 측면에서 좋습니다.data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.kt (1)
4-8: 확인: java.time 사용에 대한 하위 호환 문제 없음프로젝트 minSdk가 28(≥26)이고 build-logic의 compileOptions가 JavaVersion.VERSION_17로 설정되어 있어 core library desugaring(디슈가링)은 필요하지 않습니다.
확인 위치: build-logic/src/main/java/com/yapp/convention/KotlinAndroid.kt (defaultConfig minSdk = 28, compileOptions = JavaVersion.VERSION_17), gradle/libs.versions.toml (minSdk = "28").
data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSource.kt (1)
26-26: 원자성 확인 — DataStore.edit 단일 트랜잭션으로 처리됨
clearFortuneData는 core/datastore/src/main/java/com/yapp/datastore/UserPreferences.kt 199–202행에서 dataStore.edit { pref.remove(Keys.FORTUNE_ID); pref.remove(Keys.FORTUNE_DATE) } 형태로 구현되어 있어 동일 트랜잭션 내에서 키 제거가 수행되며 원자성이 보장됩니다.
data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.kt
Outdated
Show resolved
Hide resolved
data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.kt
Show resolved
Hide resolved
feature/mission/src/main/java/com/yapp/mission/MissionViewModel.kt
Outdated
Show resolved
Hide resolved
…om-sheet [FEAT] 업데이트 공지 바텀시트
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
feature/mission/src/main/java/com/yapp/mission/MissionViewModel.kt (1)
137-156: REAL 모드에서 스케줄 미보장 시 Idle→뒤로가기 문제 재발 가능현재 상태 조회만 수행하고 있어, 작업이 아직 enqueue되지 않은 타이밍엔
Idle로 판단되어 뒤로가기가 발생할 수 있습니다. 미션 완료 시점에 스케줄을 보장한 뒤 상태를 판정하세요. 이전 리뷰와 동일 이슈입니다.아래처럼 상태 읽기 전에 스케줄 호출을 추가하는 것을 권장합니다:
private fun completeMission(type: String) = intent { performHapticSuccess() logMissionSuccess(type) if (state.missionMode != MissionMode.REAL) { postSideEffect(MissionContract.SideEffect.NavigateBack) return@intent } + // 1) 오늘 작업 스케줄 보장 + postFortuneTaskScheduler.enqueueOnceForToday() + val fortuneCreateStatus = fortuneRepository.fortuneCreateStatusFlow.first() val hasUnseenFortune = fortuneRepository.hasUnseenFortuneFlow.first() - val shouldOpenFortune = ( - fortuneCreateStatus is FortuneCreateStatus.Creating || - fortuneCreateStatus is FortuneCreateStatus.Success && hasUnseenFortune - ) + val shouldOpenFortune = + fortuneCreateStatus is FortuneCreateStatus.Creating || + (fortuneCreateStatus is FortuneCreateStatus.Success && hasUnseenFortune) || + // 2) Idle이라면 스케줄 직후 대기/진입 전략 중 택1 + fortuneCreateStatus is FortuneCreateStatus.Idle주입 추가(파일 외 변경; 예시):
// 생성자에 스케줄러 주입 class MissionViewModel @Inject constructor( private val analyticsHelper: AnalyticsHelper, private val hapticFeedbackManager: HapticFeedbackManager, private val fortuneRepository: FortuneRepository, private val app: Application, private val savedStateHandle: SavedStateHandle, private val postFortuneTaskScheduler: PostFortuneTaskScheduler, // 새로 주입 ) : ViewModel(), ...data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.kt (1)
22-34: 상태 우선순위(실패 > 생성중 > 성공 > 대기) 적절 — 자정 경계 재평가와 Clock 주입은 선택현재 결합 로직은 단순·명확하고
distinctUntilChanged()로 불필요한 방출을 억제합니다. 다만todayEpoch()는 업스트림 플로우 변화가 없으면 자정에 재평가되지 않습니다. 과거에 동일 제안을 드렸고(테스트·타임존 개선 목적의 Clock 주입 및 자정 tick 플로우), 팀에서 “운세 생성 시에만 상태 갱신이면 충분”하다고 정리된 것으로 알고 있습니다. 그 결론 유지에 동의하며, 아래는 선택 적용 가능한 최소 변경안입니다.@@ -import java.time.LocalDate +import java.time.Clock +import java.time.LocalDate @@ -class FortuneLocalDataSourceImpl @Inject constructor( - private val userPreferences: UserPreferences, -) : FortuneLocalDataSource { +class FortuneLocalDataSourceImpl @Inject constructor( + private val userPreferences: UserPreferences, + private val clock: Clock, +) : FortuneLocalDataSource { @@ - private fun todayEpoch(): Long = LocalDate.now().toEpochDay() + private fun todayEpoch(): Long = LocalDate.now(clock).toEpochDay()자정 자동 재평가가 필요해질 경우에만
todayEpochFlow를 추가해combine(..., todayEpochFlow)로 확장하는 방식을 권장드립니다.
🧹 Nitpick comments (19)
feature/home/src/main/AndroidManifest.xml (1)
3-3: 권한 선언 위치를 앱 모듈로 일원화 검토feature 모듈(라이브러리)에서
ACCESS_NETWORK_STATE를 선언하면 최종 병합에는 문제없지만, 권한 관리가 분산되어 추적/검토가 어려워집니다. 앱 모듈app/src/main/AndroidManifest.xml에 이미 선언되어 있다면 여기서는 제거하는 편이 권장됩니다. WorkManager 네트워크 제약만 사용한다면 대개 권한 없이도 동작하지만,ConnectivityManager직접 조회가 있다면 앱 모듈에서만 선언해 주세요.제거 예:
- <uses-permission android:name="android.permission.ACCESS_NETWORK_STATE"/>해당 feature 내에서
ConnectivityManager/네트워크 콜백을 직접 사용 중인지 확인 부탁드립니다. 앱 모듈에도 동일 권한이 있는 경우 중복 선언을 정리해 주세요.feature/home/build.gradle.kts (1)
22-22: Coil 네트워크 스택/버전 정합성 점검 제안
- 원격 이미지를 로드한다면
coil-network-okhttp(또는 카탈로그의libs.coil.network.okhttp) 추가를 고려하세요.- 모듈 간 Coil 아티팩트가 늘어날 가능성이 있다면 BOM(또는 버전 카탈로그 단일 버전)으로 정합성 확보를 권장합니다.
app/src/main/java/com/yapp/orbit/di/AppVersionModule.kt (1)
14-17: @nAmed 대신 타입‑세이프한 @qualifier 사용 권장문자열 키 오타를 방지하려면 전용
@Qualifier로 교체하세요.다음처럼 같은 파일에 정의/적용 가능합니다:
import dagger.hilt.components.SingletonComponent -import javax.inject.Named import javax.inject.Singleton +import javax.inject.Qualifier +import kotlin.annotation.AnnotationRetention.BINARY + +@Qualifier +@Retention(BINARY) +annotation class AppVersion @Module @InstallIn(SingletonComponent::class) object AppVersionModule { @Provides @Singleton - @Named("appVersion") - fun provideAppVersion(): String = BuildConfig.VERSION_NAME + @AppVersion + fun provideAppVersion(): String = BuildConfig.VERSION_NAME }주입 지점의
@Named("appVersion")도@AppVersion로 변경 필요.data/src/main/java/com/yapp/data/local/datasource/UserLocalDataSource.kt (2)
9-10: Epoch 단위 명시 필요 (day vs millis)
updateNoticeLastShownDateEpochFlow가toEpochDay()(일 단위)인지, epoch millis인지가 이름만으론 불명확합니다. 혼동 방지를 위해 KDoc 또는 명확한 네이밍(예:...EpochDayFlow또는...EpochMillisFlow)을 권장합니다.interface UserLocalDataSource { @@ - val updateNoticeLastShownDateEpochFlow: Flow<Long?> + /** + * 마지막 노출 일자(UTC/시스템 타임존 기준)의 epoch day. + * 값은 LocalDate.toEpochDay() 결과(Long) 또는 null. + */ + val updateNoticeLastShownEpochDayFlow: Flow<Long?>
15-16: “오늘” 개념의 시간대 의존성 제거 권장데이터 계층에서 “today”를 결정하면 타임존 변경/테스트가 어렵습니다. 호출 측에서 epochDay를 계산해 전달하도록 오버로드 추가를 권장합니다(기존 API는 유지 후 deprecated).
interface UserLocalDataSource { @@ - suspend fun markUpdateNoticeShownToday() + /** + * 호출자가 계산한 epoch day 기준으로 노출 시점을 기록. + */ + suspend fun markUpdateNoticeShown(epochDay: Long) + @Deprecated("시간대/테스트 독립성을 위해 markUpdateNoticeShown(epochDay)를 사용하세요.") + suspend fun markUpdateNoticeShownToday()domain/src/main/java/com/yapp/domain/repository/FortuneRepository.kt (2)
9-9: Epoch 단위 문서화/네이밍 정교화
fortuneDateEpochFlow가 epoch day(Long)라면 이름을fortuneEpochDayFlow등으로 바꾸거나 KDoc으로 단위를 명시해 주세요.interface FortuneRepository { @@ - val fortuneDateEpochFlow: Flow<Long?> + /** + * 오늘 생성된 포춘의 날짜(epoch day, LocalDate.toEpochDay()) 또는 null. + */ + val fortuneEpochDayFlow: Flow<Long?>
25-26: “오늘” 의존 메서드의 타임존 리스크
markFirstAlarmDismissedToday()역시 시간대 경계(자정) 이슈가 있습니다. 호출자가epochDay를 전달하는 오버로드 추가를 권장합니다(기존 API는 deprecated 유지).- suspend fun markFirstAlarmDismissedToday() + suspend fun markFirstAlarmDismissed(epochDay: Long) + @Deprecated("시간대/테스트 독립성을 위해 markFirstAlarmDismissed(epochDay)를 사용하세요.") + suspend fun markFirstAlarmDismissedToday()feature/alarm-interaction/src/main/java/com/yapp/alarm/interaction/snooze/AlarmSnoozeTimerViewModel.kt (1)
47-49: null 처리 의도 확인 및 Clock 주입 제안
fortuneDate == null이면 항상isFirstMission = true가 됩니다. 의도라면 OK, 아니라면 null을 “모름=오늘 아님”이 아닌 “모름=오늘일 수도”로 다르게 처리해야 할 수 있습니다.- 테스트 용이성과 일관성을 위해
Clock(또는ZoneId)을 주입받아LocalDate.now(clock)로 계산하도록 리팩터를 권장합니다.- val todayDate = LocalDate.now().toEpochDay() + val todayDate = LocalDate.now().toEpochDay() + // 필요 시: val todayDate = LocalDate.now(clock).toEpochDay() - val isFirstMission = fortuneDate != todayDate + val isFirstMission = fortuneDate?.let { it != todayDate } ?: /* null 전략 */ truefeature/mission/src/main/java/com/yapp/mission/MissionViewModel.kt (1)
133-135: 해프틱 성공 중복 호출 가능성
handleMissionProgress()(라인 105 부근)와completeMission()(라인 134)에서 SUCCESS 해프틱이 두 번 울릴 수 있습니다. 한 곳으로 일원화하세요.core/datastore/src/main/java/com/yapp/datastore/UserPreferences.kt (2)
81-86: 자정 롤오버 시 UI가 갱신되지 않을 수 있음 (DataStore flow는 변경 시에만 emit).todayEpoch()는 map 내부에서 계산되지만, Preferences 자체 변경이 없으면(자정에 아무 edit도 없으면) 해당 flows가 재‑emit되지 않아 다음 날로 넘어가도 값이 갱신되지 않을 수 있습니다. hasUnseenFortuneFlow, shouldShowFortuneToolTipFlow, isFirstAlarmDismissedTodayFlow 모두 동일 리스크가 있습니다. 실제로는 다른 편집 이벤트가 자주 발생하면 문제가 드러나지 않을 수 있으나, 보수적으로 자정 롤오버 시 재평가 트리거를 두는 편이 안전합니다.
- 옵션 A: Preferences와 무관하게 자정에 1회 emit하는 todayEpochFlow()를 만들어 combine 하세요.
- 옵션 B: 앱 포그라운드 진입/재개 시 한 번 invalidate(예: viewModel에서 load* 호출)하여 새 epoch 기준으로 recompute 하세요.
필요 시 todayEpochFlow 구현 초안을 제안할 수 있습니다. 적용 의향 확인 부탁드립니다.
Also applies to: 89-97, 108-116
44-45: Clock 주입으로 테스트 용이성 및 재현성 향상.로컬 타임존/시간 의존 로직의 테스트를 쉽게 하려면 Clock을 주입받아 todayEpoch을 계산하는 방식을 권장합니다(기존 호출부 영향 없음).
+import java.time.Clock ... - private fun todayEpoch(): Long = LocalDate.now().toEpochDay() + private fun todayEpoch(clock: Clock = Clock.systemDefaultZone()): Long = + LocalDate.now(clock).toEpochDay()feature/home/src/main/java/com/yapp/home/HomeContract.kt (1)
21-22: 업데이트 공지 가시성 상태 추가 LGTM. 다만 용어 정합성 점검 권장.ViewModel에서 shouldShowFortuneToolTipFlow를 hasNewFortune에 매핑하고 있어 의미가 살짝 어긋납니다. 차후 혼동 방지를 위해 hasNewFortune → shouldShowFortuneTooltip(또는 newFortuneBadgeVisible) 식으로 이름 정리 고려 부탁드립니다.
feature/home/src/main/java/com/yapp/home/component/bottomsheet/UpdateNoticeBottomSheet.kt (1)
50-62: 버전/배너 URL 계산을 UI 밖으로 분리 권장.테스트 용이성과 재사용성을 위해 versionName, imageUrl 계산은 VM/호출 측에서 주입하고, 컴포저블은 순수하게 그리기만 하도록 유지하는 것을 권장합니다.
feature/home/src/main/java/com/yapp/home/HomeViewModel.kt (2)
366-373: 명칭 혼동: shouldShowTooltip을 hasNewFortune으로 전달.유지보수성을 위해 지역 변수/상태명을 tooltip 의미로 통일하는 것이 좋습니다.
- }.collect { (finalFortuneScore, hasNewFortune) -> + }.collect { (finalFortuneScore, shouldShowTooltip) -> reduce { state.copy( lastFortuneScore = finalFortuneScore, - hasNewFortune = hasNewFortune, - isToolTipVisible = hasNewFortune, + hasNewFortune = shouldShowTooltip, + isToolTipVisible = shouldShowTooltip, ) } }
457-464: 시스템 서비스 직접 접근을 추상화하면 테스트가 쉬워집니다.Connectivity 체크를 작은 인터페이스(예: NetworkStatusProvider)를 통해 주입하면 단위 테스트에서 네트워크 상태를 쉽게 모의할 수 있습니다. 현재 방식도 동작상 문제는 없습니다.
data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSource.kt (1)
8-16: 새 플로우·상태 노출 설계 적절 — 간단한 KDoc 보강 제안에폭일(Long) 기반 날짜, 가시성 관련 Boolean 플로우, 그리고
FortuneCreateStatus노출 모두 방향 좋습니다. 각 플로우/상태의 의미(초기값, 리셋 타이밍, 스레드 안전/원자성 기대)를 KDoc으로 명시하면 사용처 혼동을 줄일 수 있습니다.data/src/main/java/com/yapp/data/repositoryimpl/FortuneRepositoryImpl.kt (2)
27-36: 메서드 네이밍 일관성 소폭 개선 제안리포지토리 레이어는
markFortuneAsCreating/…AsFailed, 로컬 레이어는markFortuneCreating/…Failed로 접두어가 약간 다릅니다. 통일하면 호출부 가독성이 좋아집니다. 변경 범위가 넓다면 유지해도 무방합니다.
40-46: 원격 응답 매핑의 예외 안전성 확보 LGTM — 에러 도메인화는 차기 과제로
.mapCatching { it.toDomain() }로 매핑 단계 예외가 안전하게 포착됩니다. 향후 필요 시 네트워크/파싱 오류를 도메인 오류로 매핑하는 계층(예:Result.failure(DomainError.Network))을 추가하는 것을 고려해 볼 수 있습니다.data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.kt (1)
18-20: 철자 통일 제안: Tooltip
shouldShowFortuneToolTipFlow의ToolTip표기가 일반적으로는Tooltip으로 쓰입니다. 전역 검색 후 리네이밍을 고려해 주세요(낙타표기 유지 시shouldShowFortuneTooltipFlow).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
project.dot.pngis excluded by!**/*.png
📒 Files selected for processing (21)
app/src/main/AndroidManifest.xml(3 hunks)app/src/main/java/com/yapp/orbit/di/AppVersionModule.kt(1 hunks)core/datastore/src/main/java/com/yapp/datastore/UserPreferences.kt(3 hunks)data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSource.kt(1 hunks)data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.kt(2 hunks)data/src/main/java/com/yapp/data/local/datasource/UserLocalDataSource.kt(1 hunks)data/src/main/java/com/yapp/data/local/datasource/UserLocalDataSourceImpl.kt(2 hunks)data/src/main/java/com/yapp/data/repositoryimpl/FortuneRepositoryImpl.kt(2 hunks)data/src/main/java/com/yapp/data/repositoryimpl/UserInfoRepositoryImpl.kt(1 hunks)domain/src/main/java/com/yapp/domain/repository/FortuneRepository.kt(1 hunks)domain/src/main/java/com/yapp/domain/repository/UserInfoRepository.kt(1 hunks)feature/alarm-interaction/src/main/java/com/yapp/alarm/interaction/snooze/AlarmSnoozeTimerViewModel.kt(1 hunks)feature/home/build.gradle.kts(1 hunks)feature/home/src/main/AndroidManifest.xml(1 hunks)feature/home/src/main/java/com/yapp/home/HomeContract.kt(2 hunks)feature/home/src/main/java/com/yapp/home/HomeScreen.kt(17 hunks)feature/home/src/main/java/com/yapp/home/HomeViewModel.kt(9 hunks)feature/home/src/main/java/com/yapp/home/component/bottomsheet/UpdateNoticeBottomSheet.kt(1 hunks)feature/home/src/main/res/values/strings.xml(2 hunks)feature/mission/src/main/java/com/yapp/mission/MissionViewModel.kt(2 hunks)gradle/libs.versions.toml(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- feature/home/src/main/res/values/strings.xml
- app/src/main/AndroidManifest.xml
- gradle/libs.versions.toml
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-15T07:43:50.275Z
Learnt from: DongChyeon
PR: YAPP-Github/Orbit-Android#254
File: data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.kt:22-34
Timestamp: 2025-09-15T07:43:50.275Z
Learning: FortuneCreateStatusFlow에서 todayEpoch()를 combine 내부에서 직접 호출하는 것이 충분한 이유: 운세 생성할 때마다 fortuneDateEpochFlow가 변경되어 combine이 재평가되므로, 그 순간의 todayEpoch() 계산으로 충분함. 운세 요청 간격 제약으로 인해 자정 롤오버 문제는 실용적으로 발생하지 않음.
Applied to files:
data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.ktfeature/home/src/main/java/com/yapp/home/HomeViewModel.kt
📚 Learning: 2025-09-14T15:32:44.064Z
Learnt from: DongChyeon
PR: YAPP-Github/Orbit-Android#252
File: domain/src/main/java/com/yapp/domain/repository/FortuneRepository.kt:16-17
Timestamp: 2025-09-14T15:32:44.064Z
Learning: Fortune creation requests have at least a 1-minute gap between them in the Orbit Android app, making atomic guards for race condition prevention unnecessary in the fortune creation flow.
Applied to files:
data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.kt
🧬 Code graph analysis (4)
feature/home/src/main/java/com/yapp/home/component/bottomsheet/UpdateNoticeBottomSheet.kt (1)
core/designsystem/src/main/java/com/yapp/designsystem/theme/Theme.kt (1)
OrbitTheme(12-28)
data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.kt (1)
core/datastore/src/main/java/com/yapp/datastore/UserPreferences.kt (1)
todayEpoch(44-44)
core/datastore/src/main/java/com/yapp/datastore/UserPreferences.kt (1)
data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.kt (1)
todayEpoch(36-36)
feature/home/src/main/java/com/yapp/home/HomeScreen.kt (2)
feature/home/src/main/java/com/yapp/home/HomeViewModel.kt (1)
processAction(55-87)feature/home/src/main/java/com/yapp/home/component/bottomsheet/UpdateNoticeBottomSheet.kt (1)
UpdateNoticeBottomSheet(50-134)
⏰ 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). (2)
- GitHub Check: build
- GitHub Check: Continuous Deployment
🔇 Additional comments (17)
feature/home/build.gradle.kts (1)
22-22: Coil Compose 의존성 추가 LGTMHome에서 배너 이미지 로딩 용도라면 이 추가만으로 충분합니다.
app/src/main/java/com/yapp/orbit/di/AppVersionModule.kt (2)
11-18: 버전 주입 모듈 LGTM
BuildConfig.VERSION_NAME를 DI로 노출하는 접근 적절합니다.
17-17: 디버그 suffix 포함 여부 확인 필요
VERSION_NAME에versionNameSuffix가 붙는 빌드 변형이 있다면, UI/로깅/원격 구성에서 원하는 값이 맞는지 확인해 주세요.domain/src/main/java/com/yapp/domain/repository/FortuneRepository.kt (1)
18-23: 상태 플로우의 안정성(idempotency, distinctUntilChanged) 확인UI thrash 방지를 위해 각 mutator가 idemponent하게 상태를 기록하고,
fortuneCreateStatusFlow는 내부에서distinctUntilChanged()가 적용되는지 확인 부탁드립니다.core/datastore/src/main/java/com/yapp/datastore/UserPreferences.kt (1)
146-161: 운세 생성 시 상태 플래그 초기화 로직 적절.오늘 신규 운세 여부(isNewForToday) 판정 후 SEEN/TOOLTIP_SHOWN 초기화 방식이 기대 동작과 부합합니다. 👍
feature/home/src/main/java/com/yapp/home/HomeContract.kt (1)
62-64: 새 액션 추가 적절.OnClickDontShowAgain / HideUpdateNotice 도입이 UI 플로우와 일치합니다.
domain/src/main/java/com/yapp/domain/repository/UserInfoRepository.kt (1)
11-13: 도메인 계층에 업데이트 공지 관리 API 노출 적절.두 개의 Flow와 두 개의 변경 함수 추가 방향성 문제 없습니다. 구현/DI 일치 여부만 최종 확인 부탁드립니다.
Also applies to: 17-18
data/src/main/java/com/yapp/data/repositoryimpl/UserInfoRepositoryImpl.kt (1)
20-21: 위임 구현 문제 없음.LocalDataSource에 올바르게 위임하고 있습니다.
Also applies to: 26-27
data/src/main/java/com/yapp/data/local/datasource/UserLocalDataSourceImpl.kt (1)
14-16: UserPreferences 위임 매핑 정확.업데이트 공지 관련 Flow/Mutator 위임이 일관됩니다.
Also applies to: 29-35
feature/home/src/main/java/com/yapp/home/HomeScreen.kt (3)
416-422: API 35 이상에서만 인셋 보정 적용 로직 확인 필요.sheetHalfExpandHeight = screenHeight - contentHeight - offset 계산에서, offset을 35+에서만 적용하도록 한 근거가 명확하면 OK이나, 디바이스/제스처 내비 조합에 따라 음수/비정상 값 가능성 있습니다. 실제 기기(API 33/34/35)로 반쯤 펼침 높이가 의도대로 보이는지 점검 부탁드립니다.
250-258: UpdateNoticeBottomSheet 연동 LGTM.콜백과 액션 매핑이 명확합니다.
413-422: 컴파일 오류: Density 컨텍스트 없이 toDp() 호출.placeable.height.toDp()는 Density 컨텍스트가 필요합니다. 아래처럼 수정하세요.
@@ - val contentHeight = placeable.height.toDp() + val density = LocalDensity.current + val contentHeight = with(density) { placeable.height.toDp() }Likely an incorrect or invalid review comment.
feature/home/src/main/java/com/yapp/home/HomeViewModel.kt (1)
378-405: 오프라인 시 업데이트 공지 무조건 숨김 정책 확인 필요.배너 이미지만 실패해도 텍스트/버튼으로 공지를 노출하고 싶을 수 있습니다. 현재는 isOnlineNow()==false면 바로 return으로 숨깁니다. 기획 의도 확인 부탁드립니다(오프라인에서도 노출해야 한다면 이미지 로딩 실패 시 대체 UI로 전환).
data/src/main/java/com/yapp/data/repositoryimpl/FortuneRepositoryImpl.kt (1)
18-26: 데이터 소스 위임·플로우 표면 재정렬 LGTM로컬 데이터 소스의 새 플로우/상태를 그대로 노출하는 구성 깔끔합니다. 네이밍과 도메인 리포지토리 시그니처가 일치합니다.
data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.kt (2)
38-55: 변이 위임 LGTM — 생성/성공 시 플래그 정합성 확인모든 변이 메서드가
UserPreferences로 일관되게 위임되어 좋습니다.markFortuneCreated가fortuneId/fortuneDateEpoch설정과 함께isFortuneCreating=false,isFortuneFailed=false까지 한 트랜잭션에서 반영되는지 한번만 확인 부탁드립니다(중간 상태 노출 방지).동일 검증은 상위 파일 코멘트의 스크립트로 확인 가능합니다.
66-72: 정리 메서드 위임 LGTM
markFirstAlarmDismissedToday와clearFortuneData위임 모두 일관적입니다.data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSource.kt (1)
17-27: 변이 메서드 원자성(트랜잭션) 확인 요청markFortuneCreating / markFortuneCreated / markFortuneFailed / markFortuneSeen / markFortuneTooltipShown / clearFortuneData가 UserPreferences(DataStore) 내 관련 키들을 반드시 단일 edit { } 블록(원자적 트랜잭션)으로 함께 갱신하는지 확인하세요. 중간 상태 불일치가 Flow 결합으로 UI 깜박임을 유발합니다.
제공하신 검증 스크립트가 파일을 검색하지 못했습니다. 리포지토리 루트에서 아래 스크립트를 실행한 출력 결과를 여기에 붙여 제출하세요.
#!/bin/bash set -euo pipefail echo "검증 시작: fortune 관련 변이 메서드와 DataStore edit 블록 검색" rg -nC5 -g '!**/build/**' -g '**/*.kt' '\bfun\s+markFortuneCreated\(' || true rg -nC5 -g '!**/build/**' -g '**/*.kt' '\bfun\s+markFortuneCreating\(' || true rg -nC5 -g '!**/build/**' -g '**/*.kt' '\bfun\s+markFortuneFailed\(' || true rg -nC5 -g '!**/build/**' -g '**/*.kt' '\bfun\s+markFortuneSeen\(' || true rg -nC5 -g '!**/build/**' -g '**/*.kt' '\bfun\s+markFortuneTooltipShown\(' || true rg -nC5 -g '!**/build/**' -g '**/*.kt' '\bfun\s+clearFortuneData\(' || true rg -nC3 -g '!**/build/**' -g '**/*.kt' 'edit\s*\{' || true rg -nC3 -g '!**/build/**' -g '**/*.kt' 'isFortuneCreating|isFortuneFailed|fortuneId|fortuneDateEpoch|fortuneTooltipShown|fortuneSeen|saveFortuneImageId|saveFortuneScore|markFirstAlarmDismissedToday' || true
Related issue 🛠
closed #<issue_number>
어떤 변경사항이 있었나요?
CheckPoint ✅
PR이 다음 요구 사항을 충족하는지 확인하세요.
Work Description ✏️
Uncompleted Tasks 😅
To Reviewers 📢
Summary by CodeRabbit
신규 기능
버그 수정
리팩터링
UI/스타일
기타