Skip to content

Conversation

@DongChyeon
Copy link
Member

@DongChyeon DongChyeon commented Aug 20, 2025

Related issue 🛠

closed #246

어떤 변경사항이 있었나요?

  • 🐞 BugFix Something isn't working
  • 🎨 Design Markup & styling
  • 📃 Docs Documentation writing and editing (README.md, etc.)
  • ✨ Feature Feature
  • 🔨 Refactor Code refactoring
  • ⚙️ Setting Development environment setup
  • ✅ Test Test related (Junit, etc.)

CheckPoint ✅

PR이 다음 요구 사항을 충족하는지 확인하세요.

  • PR 컨벤션에 맞게 작성했습니다. (필수)
  • merge할 브랜치의 위치를 확인해 주세요(main❌/develop⭕) (필수)
  • Approve된 PR은 assigner가 머지하고, 수정 요청이 온 경우 수정 후 다시 push를 합니다. (필수)
  • BugFix의 경우, 버그의 원인을 파악하였습니다. (선택)

Work Description ✏️

2025-08-20.2.09.06.mov
  • 특정 날짜의 가장 이른 알람 해제 시에만 부적 보상 획득 가능
  • 운세를 아직 보지 않은 상태에서, MissionType이 None이 아닌 경우 미션 화면으로 이동
  • 오늘 처음 알람을 해제하면 운세 자동 생성
  • 알람이 울릴 때부터 운세 생성 로직 실행
  • 운세 화면에서 운세 생성 중일 경우 로딩 화면 표시

Uncompleted Tasks 😅

  • N/A

To Reviewers 📢

Summary by CodeRabbit

  • New Features

    • 포춘 화면 딥링크 지원 (orbitapp://fortune)
    • 하루 1회 포춘 자동 생성/게시 예약(WorkManager 기반)
  • Changes

    • 미션 완료 시 포춘 생성/대기 또는 미확인 포춘이면 자동으로 포춘 화면으로 이동
    • 포춘 보상 판정: 날짜 기반 → 오늘 첫 알람 기반으로 변경
    • 툴팁 신호 로직: “신규 포춘” → “툴팁 노출 신호”로 전환
    • 앱 링크 매칭 범위 조정(스킴 기준)
  • UI

    • 미션 화면의 오류 다이얼로그 제거
    • 포춘 로딩 화면 애니메이션/표현 개선(Lottie 및 동적 연출)
  • Chores

    • WorkManager·Hilt 워커 관련 의존성 및 초기화 설정 추가 (컴파일 SDK 정렬 포함)

@coderabbitai
Copy link

coderabbitai bot commented Aug 20, 2025

Walkthrough

알람 발생 시 WorkManager로 하루 한 번 운세 생성 작업을 예약하고, 운세 생성 상태(생성중/실패/미확인/툴팁/일일 해제)를 중심으로 저장소·데이터스토어·뷰모델·네비·화면 흐름을 재구성했습니다. 또한 Hilt Worker·WorkManager 관련 종속성 및 DI 바인딩을 추가했습니다.

Changes

Cohort / File(s) Summary
Gradle / 버전·의존성
gradle/libs.versions.toml, app/build.gradle.kts, feature/fortune/build.gradle.kts, build-logic/src/main/java/com/yapp/convention/HiltAndroid.kt, build-logic/src/main/java/.../orbit.android.feature.gradle.kts
WorkManager 및 Hilt Worker 라이브러리/버전 추가, compileSdk=35 설정, 빌드스크립트 Hilt 관련 조정 및 fortune 모듈의 WorkManager 테스트 의존성 추가
앱 매니페스트·Application
app/src/main/AndroidManifest.xml, app/src/main/java/com/yapp/orbit/OrbitApplication.kt
androidx.startup Provider 추가 및 WorkManager 초기화 메타데이터 제거, Application에서 HiltWorkerFactory로 WorkManager Configuration 제공, 딥링크 data에서 host 제거(스킴만 사용)
도메인 계약 변경
domain/.../FortuneRepository.kt, domain/.../AlarmRepository.kt, domain/.../AlarmDay.kt
FortuneRepository에 상태·라이프사이클 플로우 및 메서드 대거 추가(tryMarkFortuneCreating 등) 및 기존 일부 API 제거; AlarmRepository의 dismissed-id API 제거; DayOfWeek↔AlarmDay 변환 추가
데이터 계층(DataStore/로컬/레포지토리)
core/datastore/.../UserPreferences.kt, data/.../local/datasource/*, data/.../repositoryimpl/*RepositoryImpl.kt
운세 관련 플로우·상태 및 API 전면 재구성(생성중/실패/미확인/툴팁/일일해제 등), dismissed-id 영속성 API 제거, clearFortuneData 도입 및 레포지토리 위임 정리
알람 흐름(리시버/서비스/스케줄러)
core/alarm/.../AlarmReceiver.kt, AlarmInteractionActivityReceiver.kt, services/AlarmService.kt, scheduler/PostFortuneTaskScheduler.kt
오늘 기준 울림·일일 첫 해제 판정 로직 도입, fortune 미확인 시 딥링크로 포춘 열기, AlarmService에서 PostFortuneTaskScheduler 주입 및 enqueue 호출, PostFortuneTaskScheduler 인터페이스 추가
Fortune 피처(워커/스케줄러/DI/네비/VM/UI)
feature/fortune/.../worker/PostFortuneWorker.kt, .../scheduler/WorkManagerPostFortuneTaskScheduler.kt, .../di/SchedulerModule.kt, .../FortuneNavGraph.kt, .../FortuneViewModel.kt, .../FortuneScreen.kt
Hilt CoroutineWorker(PostFortuneWorker) 추가, WorkManager 기반 스케줄러 구현 및 DI 바인딩, Fortune 딥링크 추가, ViewModel이 combine으로 로드/상태 판단, 로딩·성공·실패 플로우 처리 및 UI 변경
네트워크 DI 단순화
core/network/.../NetworkModule.kt, core/network/.../Qualifier.kt, data/.../remote/di/ServiceModule.kt
여러 OkHttp/Retrofit 바인딩과 Qualifier 제거 → 단일 OkHttp/Retrofit 제공으로 통합, 관련 Qualifier 어노테이션 파일 삭제 및 ServiceModule에서 qualifier 제거
UI·뷰모델 변경(미션/홈/알람인터랙션)
feature/mission/..., feature/home/..., feature/alarm-interaction/...
Mission에서 포춘 직접 생성 제거하고 상태 기반 네비게이션으로 변경(재시도 액션/다이얼로그 제거), 홈 툴팁 플로우 교체, 알람 인터랙션 미션 시작 판정 단순화
리소스 추가
core/designsystem/src/main/res/raw/fortune_loading.json
Lottie 애니메이션 리소스 추가(내장 자산 포함)

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User as 사용자
  participant Alarm as AlarmReceiver/Service
  participant Sched as PostFortuneTaskScheduler
  participant WM as WorkManager
  participant Worker as PostFortuneWorker
  participant Repo as FortuneRepository

  User->>Alarm: 알람 울림/해제
  Alarm->>Sched: enqueueOnceForToday()
  Sched->>WM: enqueueUniqueWork("post_fortune_YYYY-MM-DD")
  WM->>Worker: doWork()
  Worker->>Repo: hasUnseenFortuneFlow.first()
  alt 이미 미확인 운세 있음
    Worker-->>WM: Result.success
  else 미생성/미확정
    Worker->>Repo: tryMarkFortuneCreating()
    alt 락 획득 실패
      Worker-->>WM: Result.success
    else 락 획득
      Worker->>Repo: postFortune(userId)
      alt 성공
        Worker->>Repo: markFortuneAsCreated(id), saveFortuneScore(score)
        Worker-->>WM: Result.success
      else 실패/예외
        Worker->>Repo: markFortuneAsFailed()
        Worker-->>WM: Result.retry
      end
    end
  end
Loading
sequenceDiagram
  participant Mission as MissionViewModel
  participant Repo as FortuneRepository
  participant Nav as Navigation
  participant FortuneVM as FortuneViewModel

  Mission->>Repo: hasUnseenFortuneFlow.first(), isFortuneCreatingFlow.first()
  alt 생성중/미확인
    Mission->>Nav: 이동 -> Fortune 화면
    FortuneVM->>Repo: observe(fortuneId, isFirstAlarmDismissedToday, isCreating)
    alt isCreating
      FortuneVM->>UI: 로딩 표시
    else fortuneId 존재
      FortuneVM->>Repo: getFortune(fortuneId), markFortuneSeen()
      FortuneVM->>UI: 운세 및 보상 여부 표시
    end
  else 아님
    Mission->>Nav: 뒤로 이동
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60–90 minutes

Assessment against linked issues

Objective Addressed Explanation
알람 시 운세 생성 API 호출 및 응답 운세 ID 로컬 저장 (#246)
미션 수행/알람 해제 시 운세 ID로 조회 (#246)
운세 생성 완료 전 로딩 UI 표시 (#246)

Assessment against linked issues: Out-of-scope changes

Code Change Explanation
홈 화면 툴팁 플로우 교체 (feature/home/src/main/java/com/yapp/home/HomeViewModel.kt) 툴팁 표시 로직·플로우 변경은 이슈의 "API 호출 시점 변경" 목적과 직접적 관련 없음
AndroidManifest 딥링크 host 제거 (app/src/main/AndroidManifest.xml) 딥링크 매칭 범위 수정은 호출 시점 목표와 직접적 관련 불분명
빌드스크립트 Hilt 구성 제거/이동 (build-logic/.../orbit.android.feature.gradle.kts) 빌드 로직 리팩토링으로 기능적 요구사항(#246) 범위를 벗어남
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/#246-fortune-api-call-timing

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@DongChyeon
Copy link
Member Author

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Aug 20, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
feature/fortune/build.gradle.kts (1)

11-24: feature/fortune/build.gradle.kts에 Hilt Worker 의존성 추가 필요

아래 두 파일에서 Hilt Worker를 사용 중이나, 해당 모듈의 build.gradle.kts에 관련 의존성이 누락되어 있습니다. 반드시 추가해주세요.

• feature/fortune/src/main/java/com/yapp/fortune/worker/PostFortuneWorker.kt
@HiltWorker
• app/src/main/java/com/yapp/orbit/OrbitApplication.kt
HiltWorkerFactory

추가할 의존성:

dependencies {
    // 기존 모듈 의존성 아래에 추가
    implementation(libs.androidx.hilt.work)
    kapt(libs.androidx.hilt.compiler)
}
core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmInteractionActivityReceiver.kt (1)

21-23: Hilt 필드 주입 누락 위험: 런타임 등록된 BroadcastReceiver 검토 필요
런타임에 AlarmInteractionActivityReceiver(this)로 직접 생성 후 registerReceiver로 등록하여 Hilt가 필드 주입을 수행하지 않습니다.
onReceive 진입부에서 수동 주입(AndroidInjection.inject(this, context) 또는 EntryPointAccessors) 도입하거나, Manifest에 선언하여 시스템이 생성 시 바로 주입되도록 변경을 검토해주세요.

문제 위치

  • core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmInteractionActivityReceiver.kt
    • @AndroidEntryPoint 선언된 BroadcastReceiver 클래스
  • feature/alarm-interaction/src/main/java/com/yapp/alarm/interaction/AlarmInteractionActivity.kt
    • line 30: private val broadcastReceiver = AlarmInteractionActivityReceiver(this)
    • line 104: registerReceiver(broadcastReceiver, filter, RECEIVER_EXPORTED)
🧹 Nitpick comments (27)
app/src/main/AndroidManifest.xml (1)

49-49: Compose Navigation 딥링크 패턴 검증 및 Manifest intent-filter 구체화 제안

아래 navDeepLink 패턴이 의도한 호스트·쿼리만 처리하는지 확인해주세요.
필요 시 AndroidManifest.xml의 <intent-filter>에 host 또는 pathPrefix를 추가하거나, uriPattern을 더 구체화하는 방안을 고려할 수 있습니다.

  • feature/mission/src/main/java/com/yapp/mission/MissionNavGraph.kt:18
    uriPattern = "orbitapp://mission?notificationId={notificationId}&missionType={missionType}&missionCount={missionCount}"
  • feature/fortune/src/main/java/com/yapp/fortune/FortuneNavGraph.kt:25
    uriPattern = "orbitapp://fortune?hasReward={hasReward}"
core/datastore/src/main/java/com/yapp/datastore/UserPreferences.kt (1)

39-40: 로컬 날짜 기준(today) 사용의 타임존 영향 검토

LocalDate.now()는 디바이스 타임존을 따릅니다. 서버/백엔드가 다른 타임존 기준이면 날짜 경계 시 어긋날 수 있습니다. 서버 기준(예: KST/UTC)을 명확히 하거나 필요 시 ZoneId를 주입받아 계산하는 방법을 고려하세요.

domain/src/main/java/com/yapp/domain/repository/FortuneRepository.kt (4)

11-14: 분산된 Boolean 플로우 대신 단일 상태 플로우로 통합 고려

isCreating/failed/unseen/tooltip 노출 여부를 개별 Boolean Flow로 유지하면 상태 불일치가 발생하기 쉽습니다(예: isFortuneCreatingFlow와 isFortuneFailedFlow가 동시에 true). 단일 상태 모델(예: Sealed class)로 일원화하면 일관성이 좋아지고 UI/워커 로직도 단순해집니다.

예시(도메인 설계 제안):

-    val hasUnseenFortuneFlow: Flow<Boolean>
-    val shouldShowFortuneToolTipFlow: Flow<Boolean>
-    val isFortuneCreatingFlow: Flow<Boolean>
-    val isFortuneFailedFlow: Flow<Boolean>
+    val fortuneStateFlow: Flow<FortuneState>
// domain layer
sealed interface FortuneState {
    data object None : FortuneState
    data object Creating : FortuneState
    data class Created(val fortuneId: Long, val unseen: Boolean) : FortuneState
    data class Failed(val reason: String? = null) : FortuneState
    data class Tooltip(val visible: Boolean) : FortuneState // 또는 Tooltip은 UI 전용 별도 Flow
}

12-12: 네이밍 통일: ToolTip → Tooltip

관례상 tooltip은 단일 단어입니다. shouldShowFortuneToolTipFlowshouldShowFortuneTooltipFlow로 변경하면 가독성과 검색성이 좋아집니다. 연관 레이어(DS/Repo/UI) 전체 리네임 필요.


24-24: clearFortuneData 원자성 확보

여러 키를 한 번에 초기화할 때(특히 DataStore) edit { ... } 블록 내에서 일괄 처리해 부분 갱신/중간 상태 노출을 방지하세요. 워커/화면이 동시 접근하는 시나리오에서 중요합니다.


16-21: tryMarkFortuneCreating만 노출하고 markFortuneAsCreating은 내부 전용으로 전환
tryMarkFortuneCreating()이 CAS 기반으로 CREATING 상태로 전이하므로, 외부에서 별도의 markFortuneAsCreating() 호출은 중복이며 잠재적 경쟁 조건을 유발할 수 있습니다.
아래 위치를 검토하여 markFortuneAsCreating()을 도메인 API에서 제거하거나, 내부(private) 전용으로만 노출하도록 리팩터링하세요.

  • domain/src/main/java/com/yapp/domain/repository/FortuneRepository.kt
    suspend fun markFortuneAsCreating() 시그니처 제거
  • data/src/main/java/com/yapp/data/repositoryimpl/FortuneRepositoryImpl.kt
    override suspend fun markFortuneAsCreating() 제거
  • feature/fortune/src/main/java/com/yapp/fortune/worker/PostFortuneWorker.kt
    fortuneRepository.markFortuneAsCreating() 호출 삭제 (or 내부 로직으로 대체)
  • data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSource*.kt
    • 필요시 markFortuneCreating()을 내부 전용(private/package internal)으로만 사용하도록 접근 제한
feature/home/src/main/java/com/yapp/home/HomeViewModel.kt (2)

358-364: 변수 네이밍 일관화로 혼동 방지

collect { (finalFortuneScore, hasNewFortune) -> ... }에서 두 번째 값은 실제로 “툴팁 표시 여부”입니다. 의미가 뒤섞이지 않도록 네이밍을 맞추는 것이 좋습니다.

적용 예:

-        }.collect { (finalFortuneScore, hasNewFortune) ->
+        }.collect { (finalFortuneScore, shouldShowTooltip) ->
             reduce {
                 state.copy(
                     lastFortuneScore = finalFortuneScore,
-                    hasNewFortune = hasNewFortune,
-                    isToolTipVisible = hasNewFortune,
+                    hasNewFortune = shouldShowTooltip,
+                    isToolTipVisible = shouldShowTooltip,
                 )
             }
         }

341-343: 툴팁 상태 전이 시점 확인

포춘 화면 진입 직전에 markFortuneTooltipShown()을 호출하면, 실제 화면에서 툴팁을 보지 못했더라도 플래그가 꺼질 수 있습니다(네비게이션 실패/취소 등). 의도된 UX인지 확인 바랍니다. 필요하다면 실제 화면 진입(onResume 등) 시 확정적으로 호출하는 방식을 고려하세요.

feature/mission/src/main/java/com/yapp/mission/MissionViewModel.kt (1)

136-146: 두 Flow의 스냅샷을 원자적으로 판정하도록 결합

연속된 first() 호출은 사이에 상태가 바뀔 여지가 있습니다. combine(...).first()로 한 번에 스냅샷을 취해 분기 판단을 하는 편이 안전합니다.

적용 diff:

-            val hasUnseenFortune = fortuneRepository.hasUnseenFortuneFlow.first()
-            val isFortuneCreating = fortuneRepository.isFortuneCreatingFlow.first()
-
-            if (hasUnseenFortune || isFortuneCreating) {
+            val navigateToFortune = kotlinx.coroutines.flow.combine(
+                fortuneRepository.hasUnseenFortuneFlow,
+                fortuneRepository.isFortuneCreatingFlow,
+            ) { unseen, creating -> unseen || creating }.first()
+
+            if (navigateToFortune) {
                 postSideEffect(MissionContract.SideEffect.NavigateToFortune)
             } else {
                 postSideEffect(MissionContract.SideEffect.NavigateBack)
             }

추가로 필요한 import:

import kotlinx.coroutines.flow.combine
data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSource.kt (2)

10-13: 로컬 DS에서도 상태를 단일 스트림으로 표현 권장

도메인과 동일하게 Boolean Flow들의 동시성/일관성 문제가 발생할 수 있습니다. 단일 상태 Flow(예: FortuneState)를 노출하면 저장/전이 로직의 무결성을 보장하기 쉽습니다.


15-20: 생성 전이 API 정리 및 원자성 보장

  • tryMarkFortuneCreating()이 CAS 역할을 한다면, 외부에는 이것만 노출하고 markFortuneCreating()는 내부에서만 사용하거나 제거하는 편이 안전합니다.
  • markFortuneCreated(fortuneId) 시 fortuneId, fortuneDate, unseen 상태를 반드시 한 트랜잭션으로 기록하세요.
core/alarm/src/main/java/com/yapp/alarm/scheduler/PostFortuneTaskScheduler.kt (1)

3-5: 계약(KDoc)으로 의도 고정 (하루 1회/이미 예약 시 no-op/백오프·네트워크 제약 명시)

인터페이스만 보면 해석의 여지가 있습니다. “하루 1회만”, “이미 예약되어 있으면 no-op”, “네트워크 제약·백오프 적용” 등의 계약을 KDoc으로 명확히 남겨두면 구현·테스트 모두 수월해집니다.

아래처럼 메서드에 계약을 문서화하는 것을 제안드립니다:

 interface PostFortuneTaskScheduler {
-    fun enqueueOnceForToday()
+    /**
+     * 오늘 날짜 기준으로 1일 1회만 작업을 예약합니다.
+     * - 이미 오늘자 작업이 ENQUEUED/RUNNING 상태이면 no-op 여야 합니다.
+     * - 네트워크 제약 및 backoff 정책을 적용해야 합니다.
+     * - (권장) 유니크 워크 이름 규칙을 문서화/상수화합니다. 예: "post_fortune_<yyyy-MM-dd>"
+     */
+    fun enqueueOnceForToday()
 }

추가로 (선택): 반환값으로 enqueued 여부(Boolean)나 unique work name(String)을 돌려주면 호출부에서 진단/로깅에 도움이 됩니다. 필요 시 후속 PR로 검토해도 됩니다.

feature/fortune/src/main/java/com/yapp/fortune/FortuneNavGraph.kt (1)

8-8: 제안: 딥링크 경로 상수화 및 hasReward 파싱 안정성 강화

  • feature/fortune/src/main/java/com/yapp/fortune/FortuneNavGraph.kt
    navDeepLink { uriPattern = "orbitapp://fortune?hasReward={hasReward}" } 문자열을 FortuneDeepLinks.FORTUNE_URI_PATTERN 등 상수로 분리해 모듈 간 중복·오타를 방지하세요.
  • feature/fortune/src/main/java/com/yapp/fortune/FortuneViewModel.kt
    • 현재 savedStateHandle.get<String>("hasReward")?.toBooleanStrictOrNull() ?: false으로 파싱하며, 발신부(AlarmInteractionActivityReceiver)에서 Boolean.toString()(소문자)으로 전송하므로 현 구조에서도 안정적입니다.
    • 다만, 추후 다른 송신 경로에서 대소문자나 "1" 등 다른 표현이 유입될 수 있으니
    – 파싱 단계에서 toBoolean()(대소문자 구분 없이) 사용하거나
    – 송신 시 .lowercase()를 강제해 항상 소문자 “true”/“false”만 전달되도록 처리
    방식을 고려해 일관성을 유지하세요.
  • 보안상 hasReward는 UI 상태용 힌트로만 활용하고, 실제 보상 획득/소비 여부는 서버 검증 로직을 통해 처리해야 합니다.
feature/fortune/src/main/java/com/yapp/fortune/di/SchedulerModule.kt (1)

16-18: 바인딩 파라미터 네이밍 가독성 소폭 개선 제안

구현체 파라미터명을 impl 또는 workManagerPostFortuneTaskScheduler로 두면 호출 시 타입 의도가 더 명확해집니다(선호도 차이 수준).

-    abstract fun bindsPostFortuneTaskScheduler(
-        postFortuneTaskScheduler: WorkManagerPostFortuneTaskScheduler,
-    ): PostFortuneTaskScheduler
+    abstract fun bindsPostFortuneTaskScheduler(
+        impl: WorkManagerPostFortuneTaskScheduler,
+    ): PostFortuneTaskScheduler
app/src/main/java/com/yapp/orbit/OrbitApplication.kt (1)

11-11: 클래스 선언부의 불필요한 괄호 제거 제안

가독성을 위해 생성자 없는 Application 서브클래스는 괄호를 생략하는 편이 일반적입니다.

아래처럼 변경을 제안합니다:

- class OrbitApplication() : Application(), Configuration.Provider {
+ class OrbitApplication : Application(), Configuration.Provider {
feature/fortune/src/main/java/com/yapp/fortune/scheduler/WorkManagerPostFortuneTaskScheduler.kt (2)

25-31: 태그 추가로 추적/취소 가능성 확보 제안

운영/디버깅을 위해 워크에 태그를 부여해 상태 조회·취소를 쉽게 하는 것을 권장합니다.

아래처럼 태그와 상수 도입을 제안합니다:

+ private const val TAG_POST_FORTUNE = "post_fortune"
+ private fun workNameFor(date: LocalDate) = "post_fortune_${date}"

     override fun enqueueOnceForToday() {
-        val name = "post_fortune_${LocalDate.now()}"
+        val name = workNameFor(LocalDate.now())
         val constraints = Constraints.Builder()
             .setRequiredNetworkType(NetworkType.CONNECTED)
             .build()
         val req = OneTimeWorkRequestBuilder<PostFortuneWorker>()
             .setConstraints(constraints)
             .setBackoffCriteria(BackoffPolicy.EXPONENTIAL, 15, TimeUnit.SECONDS)
+            .addTag(TAG_POST_FORTUNE)
             .build()
         WorkManager.getInstance(context)
             .enqueueUniqueWork(name, ExistingWorkPolicy.KEEP, req)
     }

27-27: 가독성: Duration API 사용 고려

WorkManager 2.7+에서는 Duration 기반 오버로드를 지원합니다. 매직 넘버/단위를 줄여 읽기 좋습니다.

예: .setBackoffCriteria(BackoffPolicy.EXPONENTIAL, Duration.ofSeconds(15))

core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmReceiver.kt (2)

114-117: 중복 로직 정리: ringsToday 확장 대신 공용 유틸/모델 메서드 사용 검토

동일한 판정 로직이 여러 곳에 흩어지면 매핑 변경 시 누락 위험이 큽니다. Alarm(모델) 또는 공용 유틸에 ringsToday(LocalDate)가 이미 있다면 재사용하시고, 없다면 이번에 공용화하는 것을 권장합니다.


129-136: 애널리틱스: ‘첫 알람’ 지표의 의미 혼선 가능성

현재 isFirstAlarm이 단순히 리스트 첫 요소와 ID를 비교합니다. 실제 요구사항은 “오늘 가장 이른 알람”과의 비교에 가깝습니다. 동일 의미로 쓰려면 earliestIdToday를 재사용하는 편이 더 일관됩니다.

예: val isFirstAlarm = earliestIdToday == notificationId

core/alarm/src/main/java/com/yapp/alarm/services/AlarmService.kt (1)

121-126: 중복 스케줄링 오버헤드 최소화 제안

handleIntent는 울림/해제 두 경로에서 모두 호출됩니다. UniqueWork + KEEP으로 중복 실행은 막히지만, 불필요한 enqueue 호출은 줄일 수 있습니다. ‘울림’ 케이스에서만 스케줄하도록 가드하는 것을 권장합니다.

- postFortuneTaskScheduler.enqueueOnceForToday()
+ if (!isDismiss) {
+     postFortuneTaskScheduler.enqueueOnceForToday()
+ }
core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmInteractionActivityReceiver.kt (1)

44-58: 유효하지 않은 미션 데이터 시 아무 동작 없이 종료됨 — 의도인지 확인 필요

hasValidMissionData가 false인 경우, hasUnseenFortune=false라면 아무 화면 전환 없이 코루틴을 종료합니다. PR 목표와 UX에 비추어 의도된 동작인지 확인 부탁드립니다. 최소한 디버깅 로그를 남기거나(예: 미션/운세 없음으로 인한 no-op) 홈으로 복귀 등 안전한 폴백이 있으면 운영상 편합니다.

feature/fortune/src/main/java/com/yapp/fortune/worker/PostFortuneWorker.kt (2)

29-33: userId 없음: 실패(failure)보다는 재시도(retry)가 안전할 수 있음

로그인/유저 식별이 늦게 초기화되는 경로가 있다면, 즉시 실패보다는 백오프 재시도가 더 안전합니다. 현재는 markFortuneAsFailed 후 Result.failure를 반환하여 워크가 종료됩니다. 요구사항상 userId 부재가 일시적일 수 있는지 확인해 주세요.

일시적 상황(앱 시작 직후 등)을 대비하려면 다음처럼 바꿀 수 있습니다.

-                fortuneRepository.markFortuneAsFailed()
-                return Result.failure()
+                fortuneRepository.markFortuneAsFailed()
+                return Result.retry()

35-37: tryMarkFortuneCreating 직후 markFortuneAsCreating 중복 호출 가능성

tryMarkFortuneCreating()가 상태 전이를 포함(creating=true 세팅)하는 구현이라면, 이어지는 markFortuneAsCreating() 호출은 중복일 수 있습니다. 로컬 구현 세mantics에 따라 하나로 통합하는 편이 명확합니다. 필요 시 원자적 API(예: acquireAndMarkCreating)로 통합 제안드립니다.

feature/fortune/src/main/java/com/yapp/fortune/FortuneViewModel.kt (3)

56-75: 생성 실패(isFortuneFailedFlow) 상태 반영 제안

로딩(isCreating)만 반영되고 실패 상태는 무시됩니다. 실패 시 로딩 해제 및 에러 UI(스낵바/리트라이 버튼 등)를 위한 상태 갱신이 필요합니다. 아래처럼 실패 플로우를 함께 결합해 최소한 로딩만이라도 해제하도록 제안합니다.

-    private fun observeFortune() = intent {
-        combine(
-            fortuneRepository.fortuneIdFlow,
-            fortuneRepository.isFortuneCreatingFlow,
-        ) { fortuneId: Long?, isCreating: Boolean ->
-            Pair(fortuneId, isCreating)
-        }.collect { (fortuneId, isCreating) ->
-            when {
-                isCreating -> {
-                    reduce { state.copy(isLoading = true) }
-                }
-                fortuneId != null -> {
-                    fetchAndUpdateFortune(fortuneId, hasReward == true)
-                }
-                else -> {
-                    reduce { state.copy(isLoading = false) }
-                }
-            }
-        }
-    }
+    private fun observeFortune() = intent {
+        combine(
+            fortuneRepository.fortuneIdFlow,
+            fortuneRepository.isFortuneCreatingFlow,
+            fortuneRepository.isFortuneFailedFlow,
+        ) { fortuneId: Long?, isCreating: Boolean, isFailed: Boolean ->
+            Triple(fortuneId, isCreating, isFailed)
+        }.collect { (fortuneId, isCreating, isFailed) ->
+            when {
+                isCreating -> {
+                    reduce { state.copy(isLoading = true) }
+                }
+                isFailed -> {
+                    reduce { state.copy(isLoading = false) }
+                }
+                fortuneId != null -> {
+                    fetchAndUpdateFortune(fortuneId, hasReward)
+                }
+                else -> {
+                    reduce { state.copy(isLoading = false) }
+                }
+            }
+        }
+    }

68-69: 불필요한 불리언 비교 제거

hasReward는 이미 Boolean입니다. == true 비교는 불필요합니다.

-                    fetchAndUpdateFortune(fortuneId, hasReward == true)
+                    fetchAndUpdateFortune(fortuneId, hasReward)

83-91: 랜덤 이미지 선택 시 최초 한 번 저장하여 일관성 유지 권장

savedImageId가 없는 최초 진입에서 매번 랜덤이 되면 UI가 흔들릴 수 있습니다. 최초 생성 시점에 저장해 두면 이후 동일 이미지를 유지할 수 있습니다.

         val savedImageId = fortuneRepository.fortuneImageIdFlow.firstOrNull()
         val imageId = savedImageId ?: getRandomImage()
+        if (savedImageId == null) {
+            fortuneRepository.saveFortuneImageId(imageId)
+        }

         val formattedTitle = fortune.dailyFortuneTitle.replace(",", ",\n").trim()
data/src/main/java/com/yapp/data/repositoryimpl/FortuneRepositoryImpl.kt (1)

25-33: 상태 전이 메서드 네이밍/위임 일관성 확인

Repository 레벨 네이밍(markFortuneAsCreating)과 LocalDataSource 네이밍(markFortuneCreating)이 다릅니다. 큰 문제는 아니나 일관성 차원에서 통일을 고려해 주세요.

📜 Review details

Configuration used: .coderabbit.yaml
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.

📥 Commits

Reviewing files that changed from the base of the PR and between e8f852f and 979670c.

📒 Files selected for processing (32)
  • app/build.gradle.kts (2 hunks)
  • app/src/main/AndroidManifest.xml (2 hunks)
  • app/src/main/java/com/yapp/orbit/OrbitApplication.kt (1 hunks)
  • build-logic/src/main/java/com/yapp/convention/HiltAndroid.kt (1 hunks)
  • build-logic/src/main/java/orbit.android.feature.gradle.kts (0 hunks)
  • core/alarm/src/main/java/com/yapp/alarm/AlarmConstants.kt (1 hunks)
  • core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmInteractionActivityReceiver.kt (2 hunks)
  • core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmReceiver.kt (4 hunks)
  • core/alarm/src/main/java/com/yapp/alarm/scheduler/PostFortuneTaskScheduler.kt (1 hunks)
  • core/alarm/src/main/java/com/yapp/alarm/services/AlarmService.kt (4 hunks)
  • core/datastore/src/main/java/com/yapp/datastore/UserPreferences.kt (2 hunks)
  • data/src/main/java/com/yapp/data/local/datasource/AlarmLocalDataSource.kt (0 hunks)
  • data/src/main/java/com/yapp/data/local/datasource/AlarmLocalDataSourceImpl.kt (0 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 (1 hunks)
  • data/src/main/java/com/yapp/data/repositoryimpl/AlarmRepositoryImpl.kt (0 hunks)
  • data/src/main/java/com/yapp/data/repositoryimpl/FortuneRepositoryImpl.kt (1 hunks)
  • domain/src/main/java/com/yapp/domain/repository/AlarmRepository.kt (0 hunks)
  • domain/src/main/java/com/yapp/domain/repository/FortuneRepository.kt (1 hunks)
  • feature/alarm-interaction/src/main/java/com/yapp/alarm/interaction/action/AlarmActionViewModel.kt (1 hunks)
  • feature/fortune/build.gradle.kts (1 hunks)
  • feature/fortune/src/main/java/com/yapp/fortune/FortuneNavGraph.kt (2 hunks)
  • feature/fortune/src/main/java/com/yapp/fortune/FortuneViewModel.kt (2 hunks)
  • feature/fortune/src/main/java/com/yapp/fortune/di/SchedulerModule.kt (1 hunks)
  • feature/fortune/src/main/java/com/yapp/fortune/page/FortunePager.kt (1 hunks)
  • feature/fortune/src/main/java/com/yapp/fortune/scheduler/WorkManagerPostFortuneTaskScheduler.kt (1 hunks)
  • feature/fortune/src/main/java/com/yapp/fortune/worker/PostFortuneWorker.kt (1 hunks)
  • feature/home/src/main/java/com/yapp/home/HomeViewModel.kt (2 hunks)
  • feature/mission/src/main/java/com/yapp/mission/MissionContract.kt (0 hunks)
  • feature/mission/src/main/java/com/yapp/mission/MissionScreen.kt (0 hunks)
  • feature/mission/src/main/java/com/yapp/mission/MissionViewModel.kt (2 hunks)
  • gradle/libs.versions.toml (4 hunks)
💤 Files with no reviewable changes (7)
  • feature/mission/src/main/java/com/yapp/mission/MissionContract.kt
  • domain/src/main/java/com/yapp/domain/repository/AlarmRepository.kt
  • feature/mission/src/main/java/com/yapp/mission/MissionScreen.kt
  • data/src/main/java/com/yapp/data/local/datasource/AlarmLocalDataSourceImpl.kt
  • build-logic/src/main/java/orbit.android.feature.gradle.kts
  • data/src/main/java/com/yapp/data/local/datasource/AlarmLocalDataSource.kt
  • data/src/main/java/com/yapp/data/repositoryimpl/AlarmRepositoryImpl.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Continuous Deployment
🔇 Additional comments (26)
build-logic/src/main/java/com/yapp/convention/HiltAndroid.kt (1)

17-19: androidx.hilt 컴파일러 버전(1.2.0)은 KSP 지원됨

  • libs.versions.toml에서 androidx.hilt:hilt-compiler에 지정된 버전은 1.2.0이며, 이 버전부터 KSP를 공식 지원합니다.
  • Hilt-Worker(hilt-work) 추가도 Hilt-Worker 통합을 위해 올바른 방향입니다.
core/alarm/src/main/java/com/yapp/alarm/AlarmConstants.kt (1)

12-12: EXTRA_IS_FIRST_TODAY 사용처 및 기본값 확인 완료

  • AlarmReceiver에서
    putExtra(AlarmConstants.EXTRA_IS_FIRST_TODAY, isEarliestToday)로 설정되고
  • AlarmInteractionActivityReceiver에서
    getBooleanExtra(AlarmConstants.EXTRA_IS_FIRST_TODAY, false)로 가져와 기본값(false) 처리도 적절합니다

모든 브로드캐스트 경로에서 동일한 키를 사용하고 있어 추가 조치가 필요하지 않습니다.

core/datastore/src/main/java/com/yapp/datastore/UserPreferences.kt (6)

33-37: 포춘 상태 키 체계 명확하고 확장성 있음

FORTUNE_SEEN / TOOLTIP_SHOWN / CREATING / FAILED로 상태를 분리해 표현한 점 좋습니다. 추후 상태 전이(재시도, 취소 등)도 무리 없이 확장할 수 있어 보입니다.


84-92: 툴팁 노출 플래그 초기값 처리 타당

키가 없을 때 tooltipNotShown = true로 간주하여 당일 포춘이 있으면 툴팁을 노출하는 로직은 합리적입니다. 다른 플로우들과 동일하게 catch도 포함되어 있어 일관성도 좋습니다.


93-102: 생성/실패 상태 플로우 분리 적절

isFortuneCreatingFlow, isFortuneFailedFlow로 UI 상태 표현이 명확해집니다. WorkManager 연동 시 진행/실패 UI 전환에 유용합니다.


121-133: tryMarkFortuneCreating의 원자성 확보 OK, 다중 트리거 경합 대비됨

edit{} 내부에서 현재 상태를 확인 후 플래그를 세팅하는 방식은 원자적이며 경합에 안전합니다. 반환값을 람다 외부 변수로 전달하는 패턴도 DataStore 특성상 유효합니다.


142-156: markFortuneCreated: 신규/당일 판별 및 플래그 리셋 로직 LGTM

isNewForToday에 따라 SEEN/TOOLTIP_SHOWN을 초기화하는 로직이 깔끔합니다. 생성 완료 시 CREATING=false, FAILED=false도 함께 정리되어 일관적입니다.


193-203: clearFortuneData 범위 적절

포춘 관련 키만 선택적으로 제거하여 사용자 전체 데이터와 분리 정리가 가능한 점 좋습니다. 재시도/재생성 시 깨끗한 상태 보장이 가능합니다.

feature/home/src/main/java/com/yapp/home/HomeViewModel.kt (1)

352-353: 툴팁 가시성 소스 교체(LGTM)

hasNewFortuneFlowshouldShowFortuneToolTipFlow로의 교체는 이번 PR의 의도(툴팁 노출 조건 분리)에 부합합니다.

data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSource.kt (1)

24-24: clearFortuneData는 전체 키를 한 번에 초기화

DataStore/SharedPreferences 등에서 한 번의 edit 트랜잭션으로 처리해 중간 상태가 외부로 노출되지 않도록 구현 확인이 필요합니다.

feature/fortune/build.gradle.kts (1)

15-15: core.alarm 의존성 추가(LGTM)

알람과의 통합을 위해 core.alarm 의존성 추가는 이번 기능 변경과 정합적입니다.

feature/fortune/src/main/java/com/yapp/fortune/page/FortunePager.kt (1)

54-54: 포맷팅 변경만 있음 — 동작 영향 없음

빈 줄 추가로 가독성만 개선되었습니다. 기능적 변화는 없습니다.

Also applies to: 60-60

feature/fortune/src/main/java/com/yapp/fortune/di/SchedulerModule.kt (1)

11-13: 정석적인 Hilt 모듈 구성

@BINDS + @Installin(SingletonComponent) 구성 적절합니다.

app/src/main/java/com/yapp/orbit/OrbitApplication.kt (2)

20-23: WorkManager × Hilt 연동 구성 적절합니다

Configuration.Provider로 HiltWorkerFactory를 주입해 넘기는 방식이 정석입니다. 별다른 이슈 없어 보입니다.


13-23: WorkManager 중복 초기화 없음 확인

  • AndroidManifest.xml에서 androidx.startup.InitializationProvidertools:node="remove"가 적용되어 자동 초기화가 제거됨을 확인했습니다.
  • 따라서 HiltWorkerFactory를 통한 WorkManager 초기화 경로만 단일하게 유지되고 있습니다.
feature/fortune/src/main/java/com/yapp/fortune/scheduler/WorkManagerPostFortuneTaskScheduler.kt (1)

20-31: 하루 1회 고유 이름으로 큐잉하는 전략 적절합니다

날짜 기반 unique work name + KEEP 정책으로 중복 실행이 효과적으로 방지됩니다. 네트워크 제약과 지수 백오프 설정도 방향성 OK입니다.

core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmReceiver.kt (1)

140-147: 브로드캐스트에 ‘오늘 첫 알람’ 플래그 전달 추가 LGTM

EXTRA_IS_FIRST_TODAY로 UI 측 분기(미션/보상/로딩 등) 의사결정이 수월해집니다. 선택된 네이밍도 명확합니다.

core/alarm/src/main/java/com/yapp/alarm/services/AlarmService.kt (2)

55-56: Fortune 의존성 분리 방향 좋아요

Repository 직접 호출을 스케줄러 인터페이스로 치환해 Service를 도메인에서 분리한 점 좋습니다. 테스트 용이성과 교체 가능성이 높아졌습니다.


128-132: 미션 화면 이동 여부 단순화 LGTM

MissionType != NONE 기준으로 분기하는 단순화는 요구사항에 부합합니다.

gradle/libs.versions.toml (2)

32-32: WorkManager 버전 추가 LGTM

androidx-work 2.10.3 선언 적절합니다. KTX 런타임/테스트 라이브러리와 함께 쓰는 구성이 일반적입니다.


107-110: Hilt Worker 의존성 선언 누락 여부 수동 확인 필요
아래 두 의존성이 Worker 모듈(예: feature/fortune, app)에 모두 선언되어 있는지 검증해 주세요. 누락 시 Worker 주입이 실패합니다.

  • implementation(libs.hilt-worker)
  • ksp(libs.androidx.hilt.compiler)

자동 검색 스크립트 결과가 빈 값으로 나와 수동 확인이 필요합니다.

feature/fortune/src/main/java/com/yapp/fortune/worker/PostFortuneWorker.kt (1)

38-52: 성공/실패 분기와 재시도 전략 적절

성공 시 ID/점수 저장, 실패 시 failed 마킹 + retry로 복구 가능성이 유지됩니다. 예외 케이스도 동일한 경로로 수렴하여 일관적입니다.

data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.kt (2)

14-17: 새 상태 플로우(미확인/툴팁/생성중/실패) 위임 OK

UserPreferences 위임으로 일관된 단일 소스 유지. 문제 없습니다.


19-53: 상태 전이/저장 API 위임 일관성 양호

  • tryMark/markCreated/markFailed/markSeen 시맨틱이 깔끔하게 분리되어 있습니다.
  • clearFortuneData로 정리 포인트가 명확합니다.
data/src/main/java/com/yapp/data/repositoryimpl/FortuneRepositoryImpl.kt (2)

20-23: 새 플로우 공개 OK

로컬 데이터소스와 1:1 위임으로 레이어 간 책임 분리가 잘 되어 있습니다.


35-43: 원격 매핑 mapCatching 전환 적절

간결하고 예외 처리 일관적입니다.

@DongChyeon
Copy link
Member Author

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Aug 20, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmReceiver.kt (1)

109-149: BroadcastReceiver에서 goAsync 없이 비동기 작업 실행 — 수명 종료로 작업 누락 위험

onReceive 내부에서 코루틴을 띄우고 있는데 goAsync()를 사용하지 않아, 리시버 콜백 종료 후 프로세스가 정리되면 백그라운드 작업(earliest 계산 및 브로드캐스트 송신)이 중간에 끊길 수 있습니다. PendingResult를 확보해 finally에서 finish()로 마무리해 주세요.

다음 수정안을 제안합니다:

-                CoroutineScope(Dispatchers.IO).launch {
+                val pending = goAsync()
+                CoroutineScope(Dispatchers.IO).launch {
+                    try {
                     val alarms = alarmUseCase.getAllAlarms().first()

                     val isSnoozeId = notificationId >= AlarmConstants.SNOOZE_ID_OFFSET

                     fun Alarm.ringsToday(): Boolean {
                         if (repeatDays == 0) return true

                         val todayAlarmDay = LocalDate.now().dayOfWeek.toAlarmDay()
                         return (repeatDays and todayAlarmDay.bitValue) != 0
                     }

                     val earliestIdToday: Long? = alarms
                         .asSequence()
                         .filter { (it.isAlarmActive || it.id == notificationId) && it.ringsToday() }
                         .sortedWith(compareBy({ it.hour }, { it.minute }, { it.second }))
                         .firstOrNull()
                         ?.id

                     val isEarliestAlarmDismissedToday =
                         !isSnoozeId && (earliestIdToday == notificationId)

                     val isFirstAlarm = alarms.firstOrNull()?.id == notificationId
                     analyticsHelper.logEvent(
                         AnalyticsEvent(
                             type = "alarm_dismiss",
                             properties = mapOf(
                                 AnalyticsEvent.AlarmPropertiesKeys.ALARM_ID to "$notificationId",
                                 AnalyticsEvent.AlarmPropertiesKeys.DISMISS_IS_FIRST_ALARM to isFirstAlarm,
                             ),
                         ),
                     )

                     sendBroadCastToCloseAlarmInteractionActivity(
                         context = context,
                         notificationId = notificationId,
                         missionType = missionType,
                         missionCount = missionCount,
                         isEarliestToday = isEarliestAlarmDismissedToday,
                     )
-                }
+                    } finally {
+                        pending.finish()
+                    }
+                }
♻️ Duplicate comments (3)
core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmReceiver.kt (2)

114-119: 요일 비트 매핑 불일치 이슈를 정확히 수정했습니다.

DayOfWeek → AlarmDay 매핑으로 전환(toAlarmDay)하며 bitValue로 검사해 기존 ordinal 기반 오판정을 해소했습니다. 재발 가능성 낮습니다.


121-127: 단발/비활성 알람 포함한 ‘오늘 가장 이른 알람’ 판정 로직 개선이 적절합니다.

현재 해제한 알람(notificationId)을 활성 여부와 무관하게 비교군에 포함해 단발 알람 비활성화 타이밍 이슈를 해소했습니다.

core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmInteractionActivityReceiver.kt (1)

44-80: goAsync + Main 디스패처 적용으로 수신자 수명/스레드 안정성 확보

PendingResult를 확보하고 finally에서 finish() 호출, UI 동작은 Main에서 처리로 방향이 올바르게 수정되었습니다. 이전 위험 지점이 해소되었습니다.

🧹 Nitpick comments (4)
domain/src/main/java/com/yapp/domain/model/AlarmDay.kt (2)

16-18: AlarmDay → DayOfWeek 변환 수식 정확

ordinal 기준 일요일(0) → 7, 월요일(1) → 1로 매핑되어 ISO 8601(Mon=1..Sun=7)과 일치합니다. 로직은 문제 없습니다. 다만 수식의 의도를 명확히 하기 위해 간단한 KDoc 주석과 양방향 불변식 테스트 추가를 권장합니다.

원하시면 두 함수에 대한 KDoc/유닛 테스트 템플릿을 바로 제공하겠습니다.


20-22: DayOfWeek → AlarmDay 역변환도 정확하지만 enum 순서 의존 리스크 존재

value(1..7) % 7을 index로 사용해 SUN이 0으로, MON..SAT는 1..6으로 잘 매핑됩니다. 다만 entries/ordinal에 의존하므로 enum 순서 변경 시 침묵 실패가 발생할 수 있습니다. 가독성과 안전성을 위해 명시적 when 매핑으로의 전환을 선택지로 제안합니다.

아래는 동일 동작을 더 명시적으로 표현한 대안입니다.

-fun DayOfWeek.toAlarmDay(): AlarmDay {
-    val index = (this.value % 7)
-    return AlarmDay.entries[index]
-}
+fun DayOfWeek.toAlarmDay(): AlarmDay = when (this) {
+    DayOfWeek.MONDAY -> AlarmDay.MON
+    DayOfWeek.TUESDAY -> AlarmDay.TUE
+    DayOfWeek.WEDNESDAY -> AlarmDay.WED
+    DayOfWeek.THURSDAY -> AlarmDay.THU
+    DayOfWeek.FRIDAY -> AlarmDay.FRI
+    DayOfWeek.SATURDAY -> AlarmDay.SAT
+    DayOfWeek.SUNDAY -> AlarmDay.SUN
+}

또한 회귀 방지를 위해 다음 불변식을 테스트로 보강해 주세요:

  • forAll d in AlarmDay: d.toDayOfWeek().toAlarmDay() == d
  • forAll dow in DayOfWeek: dow.toAlarmDay().toDayOfWeek() == dow

테스트 코드 초안이 필요하시면 제공하겠습니다.

core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmReceiver.kt (1)

37-39: 미사용 DI 제거 제안: FortuneRepository 주입 및 import 정리

해당 파일에서 fortuneRepository는 더 이상 사용되지 않습니다. 불필요한 의존성 주입과 import를 제거해 경량화하세요.

Also applies to: 17-17

core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmInteractionActivityReceiver.kt (1)

56-64: 컨텍스트 접근 중복 최소화(선택): ctx 바인딩으로 가독성 개선

context?.let 블록을 두 번 사용하는 대신 try 블록 초기에 val ctx = context ?: return@launch 로 바인딩하면 중복을 줄일 수 있습니다. 기능 영향은 없습니다.

예시:

val pending = goAsync()
CoroutineScope(Dispatchers.Main).launch {
    try {
        val ctx = context ?: return@launch

        if (!hasValidMissionData) {
            val hasUnseenFortune = withContext(Dispatchers.IO) { fortuneRepository.hasUnseenFortuneFlow.first() }
            if (hasUnseenFortune) {
                val isFirstToday = intent.getBooleanExtra(AlarmConstants.EXTRA_IS_FIRST_TODAY, false)
                val uri = "orbitapp://fortune?hasReward=$isFirstToday".toUri()
                val fortuneIntent = Intent(Intent.ACTION_VIEW, uri).apply {
                    addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)
                    setPackage(ctx.packageName)
                }
                ctx.startActivity(fortuneIntent)
            }
            return@launch
        }

        val uriString = "orbitapp://mission?notificationId=$notificationId&missionType=${missionType.value}&missionCount=$missionCount"
        val missionIntent = Intent(Intent.ACTION_VIEW, uriString.toUri()).apply {
            addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)
            setPackage(ctx.packageName)
        }
        ctx.startActivity(missionIntent)
    } finally {
        pending.finish()
    }
}

Also applies to: 68-77

📜 Review details

Configuration used: .coderabbit.yaml
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.

📥 Commits

Reviewing files that changed from the base of the PR and between 979670c and abaa8ea.

📒 Files selected for processing (8)
  • app/build.gradle.kts (2 hunks)
  • app/src/main/AndroidManifest.xml (2 hunks)
  • core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmInteractionActivityReceiver.kt (2 hunks)
  • core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmReceiver.kt (4 hunks)
  • core/datastore/src/main/java/com/yapp/datastore/UserPreferences.kt (2 hunks)
  • domain/src/main/java/com/yapp/domain/model/AlarmDay.kt (2 hunks)
  • feature/alarm-interaction/src/main/java/com/yapp/alarm/interaction/action/AlarmActionViewModel.kt (1 hunks)
  • feature/fortune/build.gradle.kts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
  • app/build.gradle.kts
  • feature/fortune/build.gradle.kts
  • feature/alarm-interaction/src/main/java/com/yapp/alarm/interaction/action/AlarmActionViewModel.kt
  • app/src/main/AndroidManifest.xml
  • core/datastore/src/main/java/com/yapp/datastore/UserPreferences.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Continuous Deployment
🔇 Additional comments (3)
domain/src/main/java/com/yapp/domain/model/AlarmDay.kt (1)

3-3: java.time.DayOfWeek import 전환: 문제 없음

FQCN을 import로 치환해 시그니처가 간결해졌습니다. 타입 자체는 동일하므로 바이너리/소스 호환성 문제는 없습니다.

core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmReceiver.kt (1)

142-149: EXTRA_IS_FIRST_TODAY 전달 추가가 흐름 일관성을 높입니다.

해제 시점에 계산한 ‘오늘 첫 알람 여부’를 브로드캐스트로 전파하고, 수신 측에서 hasReward로 활용 가능해졌습니다. 의도와 구현이 잘 맞습니다.

Also applies to: 195-203

core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmInteractionActivityReceiver.kt (1)

48-50: IO 컨텍스트에서 Flow.first() 호출로 메인 블로킹 회피 — 적절합니다

hasUnseenFortune 조회를 withContext(Dispatchers.IO)로 수행해 메인 스레드 블로킹을 피했습니다. 문제없습니다.

@DongChyeon
Copy link
Member Author

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Aug 20, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (3)
core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmInteractionActivityReceiver.kt (1)

44-76: goAsync + Main 스레드 전환으로 수신자 라이프사이클/스레드 이슈 해결

pending = goAsync()로 생명주기를 연장하고, CoroutineScope(Dispatchers.Main)에서 startActivity를 호출하도록 수정한 점 좋습니다. finally에서 pending.finish() 호출도 빠짐없습니다.

core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmReceiver.kt (2)

114-119: 요일 비트 매핑 오류 수정 확인: toAlarmDay 사용으로 정합성 확보

DayOfWeek.ordinal 대신 toAlarmDay().bitValue로 검사하도록 변경되어 요일→비트 매핑 불일치 문제가 해결되었습니다.


121-127: 단발 알람 배제 문제 해결: 비교군 필터 수정 적절

earliestIdToday 계산 시 (it.isAlarmActive || it.id == notificationId) && it.ringsToday()로 보정해, 울림 직후 비활성화되는 단발 알람도 정확히 비교군에 포함됩니다.

🧹 Nitpick comments (13)
data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSource.kt (2)

16-21: 상태 전이 API를 단일 상태로 모델링하는 대안 제안

tryMarkFortuneCreating/markFortuneCreating/markFortuneCreated/markFortuneFailed/markFortuneSeen 등 개별 메서드가 늘어나 전이 조합이 분기별로 분산될 수 있습니다. FortuneStatus(Idle/Creating/Created(id,date)/Failed/Seen 등) 단일 플로우 + setStatus(…) 형태로 단순화하면 일관성을 보장하기 쉽습니다. 도입 비용이 크다면 최소한 KDoc로 각 함수의 전이 규약을 명시하세요.


24-27: 일일 첫 해제 표시와 초기화 API 명확성 유지

markFirstAlarmDismissedToday()/clearFortuneData() 노출은 적절합니다. markFirstAlarmDismissedToday의 “하루 경계에서 자동 리셋(날짜 비교 기반)” 동작을 인터페이스 주석으로 명시해 두면 사용처 혼선을 줄일 수 있습니다.

core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmReceiver.kt (3)

133-140: isFirstAlarm 계산 기준 통일 필요(현재 목록 첫 요소 비교는 부정확할 수 있음)

analytics에 전송하는 DISMISS_IS_FIRST_ALARM은 전체 목록 첫 요소(정렬/필터 미적용)와 비교하고 있어, earliestIdToday 산출 기준과 불일치합니다. 동일 기준으로 계산하도록 통일하세요.

아래처럼 earliestIdToday 비교를 재사용하는 방식을 권장합니다:

-                    val isFirstAlarm = alarms.firstOrNull()?.id == notificationId
+                    val isFirstAlarm = earliestIdToday == notificationId

109-151: BroadcastReceiver 비동기 처리 시 goAsync 사용 권장

onReceive 내부에서 코루틴을 띄워 DataStore 접근/브로드캐스트를 수행하므로, 수신자 수명 보장을 위해 goAsync()로 PendingResult를 확보하는 것이 안전합니다. 작업 완료 후 반드시 finish() 하세요.

아래 패치를 적용하면 됩니다:

                 context.stopService(alarmServiceIntent)
 
-                CoroutineScope(Dispatchers.IO).launch {
+                val pending = goAsync()
+                CoroutineScope(Dispatchers.IO).launch {
+                    try {
                     val alarms = alarmUseCase.getAllAlarms().first()
@@
-                    sendBroadCastToCloseAlarmInteractionActivity(
+                    sendBroadCastToCloseAlarmInteractionActivity(
                         context = context,
                         notificationId = notificationId,
                         missionType = missionType,
                         missionCount = missionCount,
                     )
-                }
+                    } finally {
+                        pending.finish()
+                    }
+                }

160-164: extras 강제 언래핑(!!) NPE 가능성 제거

putExtras(intent.extras!!)는 extras가 없을 때 NPE를 유발합니다. 안전하게 처리하세요.

     private fun createAlarmServiceIntent(
         context: Context,
         intent: Intent,
     ): Intent {
-        return Intent(context, AlarmService::class.java).apply {
-            putExtras(intent.extras!!)
-        }
+        return Intent(context, AlarmService::class.java).apply {
+            intent.extras?.let { putExtras(it) }
+        }
     }
core/datastore/src/main/java/com/yapp/datastore/UserPreferences.kt (1)

88-96: 변수 네이밍 혼동 가능성: tooltipNotShown → tooltipShown 권장

논리는 맞지만, tooltipNotShown에 ‘보여줌’ 값(true/false)을 담고 그 뒤에 ‘!’를 취해 가독성이 떨어집니다. 네이밍을 바꾸면 의도가 명확해집니다.

-        .map { pref ->
-            val hasTodayFortune = pref[Keys.FORTUNE_DATE] == today() && pref[Keys.FORTUNE_ID] != null
-            val tooltipNotShown = pref[Keys.FORTUNE_TOOLTIP_SHOWN] ?: false
-            hasTodayFortune && !tooltipNotShown
-        }
+        .map { pref ->
+            val hasTodayFortune = pref[Keys.FORTUNE_DATE] == today() && pref[Keys.FORTUNE_ID] != null
+            val tooltipShown = pref[Keys.FORTUNE_TOOLTIP_SHOWN] ?: false
+            hasTodayFortune && !tooltipShown
+        }
data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.kt (1)

20-42: 상태 전이 메서드 위임 일관성 양호

tryMarkFortuneCreating/markFortuneCreated 등 상태 전이 메서드들이 모두 UserPreferences로 일관되게 위임됩니다. 테스트 용이성을 위해 메서드별 KDoc(전이 전제/부작용) 추가를 고려해 주세요.

domain/src/main/java/com/yapp/domain/repository/FortuneRepository.kt (4)

11-15: 새 상태 Flow 추가 방향성은 적절합니다만, 'ToolTip' 철자 일관성 이슈가 있습니다.

프로퍼티 이름이 shouldShowFortuneToolTipFlow(ToolTip)인데, 메서드는 markFortuneTooltipShown(Tooltip)을 사용합니다. 도메인 인터페이스에서의 네이밍은 전역적으로 파급되므로 지금 일관화하는 편이 좋습니다.

아래처럼 Tooltip으로 통일을 제안합니다:

-    val shouldShowFortuneToolTipFlow: Flow<Boolean>
+    val shouldShowFortuneTooltipFlow: Flow<Boolean>

20-22: 상태 전이 API는 좋습니다. ‘멱등성(idempotency)’ 보장을 문서화하세요.

markFortuneAsFailed/Seen/TooltipShown은 여러 경로에서 중복 호출될 수 있습니다. 호출이 중복되어도 상태가 깨지지 않도록(멱등) 구현/문서화하면 디버깅이 수월해집니다.


25-26: ‘하루 첫 해제’ 플래그는 TZ/자정 경계 이슈에 취약할 수 있습니다 — 날짜 기반으로의 개선 고려

isFirstAlarmDismissedTodayFlow: Flow<Boolean> + markFirstAlarmDismissedToday() 조합은 “오늘”의 기준이 어느 시간대를 따르는지, 자정 경계에서 자동 리셋이 어떻게 되는지 모호합니다. LocalDate(또는 ZonedDateTime) 기반 저장/판단으로 개선을 고려해 주세요.

예시(도메인 관점):

  • val firstAlarmDismissedDateFlow: Flow<LocalDate?>
  • suspend fun markFirstAlarmDismissed(date: LocalDate = clock.today())
  • 테스트 용이성을 위해 Clock 주입(또는 nowProvider) 고려

17-19: 명확한 원자성 보장 및 사용 규약 문서화

현재 tryMarkFortuneCreating()은 DataStore.edit 블록 내에서 이전 상태를 검사한 뒤 플래그를 설정해 CAS(원자적) 시맨틱을 충족합니다. 반면 markFortuneAsCreating()은 단순히 플래그만 설정하므로, 외부에서 이미 획득된 흐름에서만 호출되어야 합니다.

다음 사항을 반영해 주세요:

  • core/datastore/src/main/java/com/yapp/datastore/UserPreferences.kt
    • tryMarkFortuneCreating() (약 134행): “CAS 방식으로 플래그를 설정하고, 시작 가능 여부를 반환”하는 원자적 메서드임을 KDoc으로 명시
    • markFortuneCreating() (약 148행): “비원자적 단순 설정. 반드시 tryMarkFortuneCreating() 성공 후 호출”임을 KDoc으로 문서화

필요하다면 두 메서드를 통합하거나, 호출 책임이 명확해지도록 네이밍/구조를 개선하는 것도 고려해 주세요.

data/src/main/java/com/yapp/data/repositoryimpl/FortuneRepositoryImpl.kt (2)

20-24: 로컬 DataSource Flow 위임은 적절합니다. 다만 'ToolTip' 철자 일관화 필요

도메인 인터페이스와 동일하게 shouldShowFortuneToolTipFlowshouldShowFortuneTooltipFlow로 통일하는 것을 권장합니다. 인터페이스 변경 시 본 구현과 모든 사용처를 함께 변경해 주세요.

-    override val shouldShowFortuneToolTipFlow: Flow<Boolean> = fortuneLocalDataSource.shouldShowFortuneToolTipFlow
+    override val shouldShowFortuneTooltipFlow: Flow<Boolean> = fortuneLocalDataSource.shouldShowFortuneTooltipFlow

34-34: 일일 최초 해제 마킹은 멱등성/경쟁 상태에 안전해야 합니다.

markFirstAlarmDismissedToday()가 자정 경계(동시 트리거)에서도 중복 호출에 안전하도록 구현(예: 날짜 키 비교 후 set)이 되었는지 확인해 주세요.

📜 Review details

Configuration used: .coderabbit.yaml
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.

📥 Commits

Reviewing files that changed from the base of the PR and between abaa8ea and 0cdb890.

📒 Files selected for processing (9)
  • core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmInteractionActivityReceiver.kt (2 hunks)
  • core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmReceiver.kt (3 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 (2 hunks)
  • data/src/main/java/com/yapp/data/repositoryimpl/FortuneRepositoryImpl.kt (1 hunks)
  • domain/src/main/java/com/yapp/domain/repository/FortuneRepository.kt (1 hunks)
  • feature/fortune/src/main/java/com/yapp/fortune/FortuneNavGraph.kt (2 hunks)
  • feature/fortune/src/main/java/com/yapp/fortune/FortuneViewModel.kt (4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • feature/fortune/src/main/java/com/yapp/fortune/FortuneViewModel.kt
  • feature/fortune/src/main/java/com/yapp/fortune/FortuneNavGraph.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Continuous Deployment
🔇 Additional comments (15)
data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSource.kt (1)

10-15: 운세 상태 플로우 확장 설계, 목적과 일치합니다

hasUnseen/tooltip/creating/failed/첫 해제 여부 등 상태 플로우 추가가 PR 목표(생성 시점 변경, 일일 상태 추적, 로딩/실패 표시)에 부합합니다. 도메인에서의 활용 범위가 명확해졌습니다.

core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmInteractionActivityReceiver.kt (2)

48-60: hasUnseenFortune 조회를 IO 컨텍스트로 오프로드한 점 적절

flow.first()를 withContext(Dispatchers.IO)로 감싼 처리로 메인 블로킹을 피했습니다. 현재 구조에서는 충분합니다.


51-59: 딥링크 hasReward 파라미터 제거 영향 없음 확인

  • feature/fortune/src/main/java/com/yapp/fortune/FortuneNavGraph.kt:
    navDeepLink에 "orbitapp://fortune"만 정의되어 있어 쿼리 파라미터를 기대하지 않음
  • feature/fortune/src/main/java/com/yapp/fortune/FortuneViewModel.kt:
    DataStore의 isFirstAlarmDismissedToday를 기반으로 hasReward 상태 처리
  • feature/fortune/src/main/java/com/yapp/fortune/page/FortunePager.kt, FortuneCompletePage.kt 등 화면:
    추가 파라미터 없이 정상 동작 확인

위 검증 결과, 딥링크에서 hasReward 파라미터를 제거한 것은 의도된 변경이며 별도 조치가 필요 없습니다.

core/datastore/src/main/java/com/yapp/datastore/UserPreferences.kt (5)

79-86: hasUnseenFortuneFlow 정의 적절(예외 처리 포함, 오늘+미열람 조건 반영)

DataStore 예외를 catch로 흡수하고, 오늘 날짜+ID 존재+미열람 조건을 정확히 반영했습니다.


107-114: 일일 첫 해제 플로우(today 비교) 동작 타당

날짜와 플래그를 동시 확인해 자정 이후 자동 리셋되는 형태로 구성되어 있습니다. 일일 보상/툴팁 판단에 적합합니다.


134-146: tryMarkFortuneCreating 원자적 시작 제어 구현 적절

오늘 생성 여부/진행 중 여부를 단일 edit 블록에서 판정·갱신해 경합을 줄였습니다. WorkManager 트리거와의 동시성에서도 안정적일 것으로 보입니다.


155-169: markFortuneCreated 전이 처리 적절(미열람/툴팁 리셋 조건 포함)

새로운 일자/ID에 대해서만 seen/tooltip을 리셋하는 조건이 잘 들어갔습니다. creating/failed 상태 해제도 적절합니다.


213-224: clearFortuneData 범위 충분(관련 키 일괄 제거)

운세 관련 키 전부 제거하고 있어 재초기화 시 일관성이 확보됩니다.

data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.kt (2)

14-19: 새로운 상태 플로우 위임 매핑 적절

UserPreferences의 확장된 플로우를 그대로 위임 노출하고 있어 레포지토리 계층과의 정합성이 확보되었습니다.


52-58: 일일 첫 해제/초기화 위임도 문제 없음

markFirstAlarmDismissedToday/clearFortuneData 위임이 명확합니다.

domain/src/main/java/com/yapp/domain/repository/FortuneRepository.kt (1)

27-28: 단일 초기화 엔트리포인트(clearFortuneData) 도입 좋아요.

분산된 clear 메서드를 하나로 모아 상태 초기화 일관성을 높였습니다.

data/src/main/java/com/yapp/data/repositoryimpl/FortuneRepositoryImpl.kt (4)

36-36: clearFortuneData 위임 — LGTM

상태 초기화를 한 곳으로 위임해 유지보수성이 좋아졌습니다.


40-40: mapCatching으로 도메인 변환 예외를 흡수 — 개선점 좋습니다.

DTO→도메인 변환에서 발생 가능한 예외를 Result로 안전하게 감싸도록 개선되었습니다.


45-45: getFortune도 동일한 예외 안전 매핑 적용 — 일관성 OK

post/get 경로의 매핑 일관성이 확보되었습니다.


26-31: 원자적 생성 상태 전이 구현 확인 완료
tryMarkFortuneCreating()은 UserPreferences의 DataStore.edit 내에서 조건부로 FORTUNE_CREATING 플래그 설정·중복 작업 방지를,
markFortuneCreating()은 별도 보정용으로 동작하도록 위임되어 있습니다.
PostFortuneWorker에서 tryMarkFortuneCreating() 결과로 중복 호출을 차단하고 있으니 의도한 원자성이 보장됩니다.

추가 수정 없이 머지하셔도 됩니다.

@DongChyeon
Copy link
Member Author

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Aug 21, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmReceiver.kt (1)

109-150: BroadcastReceiver 내 코루틴 — goAsync() 없이 비동기 처리 시 작업 유실 위험

onReceive 내부에서 CoroutineScope(Dispatchers.IO).launch { ... }만 사용하는 현재 구조는, 브로드캐스트 수신자 수명 종료 후(메서드 반환 직후) OS에 의해 작업이 중단되어 DB 쓰기/브로드캐스트 전송/로그 적재가 유실될 수 있습니다. goAsync()로 지연 종료를 선언하고 finish()finally에서 보장하세요.

권장 패치:

-                CoroutineScope(Dispatchers.IO).launch {
+                val pendingResult = goAsync()
+                CoroutineScope(Dispatchers.IO).launch {
+                    try {
                     val alarms = alarmUseCase.getAllAlarms().first()
                     val isSnoozeId = notificationId >= AlarmConstants.SNOOZE_ID_OFFSET
                     fun Alarm.ringsToday(): Boolean {
                         if (repeatDays == 0) return true
                         val todayAlarmDay = LocalDate.now().dayOfWeek.toAlarmDay()
                         return (repeatDays and todayAlarmDay.bitValue) != 0
                     }
                     val earliestIdToday: Long? = alarms
                         .asSequence()
                         .filter { (it.isAlarmActive || it.id == notificationId) && it.ringsToday() }
                         .sortedWith(compareBy({ it.hour }, { it.minute }, { it.second }))
                         .firstOrNull()
                         ?.id
                     val isEarliestAlarmDismissedToday =
                         !isSnoozeId && (earliestIdToday == notificationId)
                     if (isEarliestAlarmDismissedToday) fortuneRepository.markFirstAlarmDismissedToday()
                     val isFirstAlarm = earliestIdToday == notificationId
                     analyticsHelper.logEvent(
                         AnalyticsEvent(
                             type = "alarm_dismiss",
                             properties = mapOf(
                                 AnalyticsEvent.AlarmPropertiesKeys.ALARM_ID to "$notificationId",
                                 AnalyticsEvent.AlarmPropertiesKeys.DISMISS_IS_FIRST_ALARM to isFirstAlarm,
                             ),
                         ),
                     )
                     sendBroadCastToCloseAlarmInteractionActivity(
                         context = context,
                         notificationId = notificationId,
                         missionType = missionType,
                         missionCount = missionCount,
                     )
-                }
+                    } finally {
+                        pendingResult.finish()
+                    }
+                }
♻️ Duplicate comments (2)
core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmReceiver.kt (2)

114-119: 요일 비트 매핑 버그 수정 확인 — toAlarmDay 기반 검사로 정합성 확보

DayOfWeek → AlarmDay 매핑 후 bitValue로 검사하는 방식으로 이전 ordinal 불일치 문제를 정확히 해결했습니다. 굿.


15-15: toAlarmDay import 추가로 도메인 모델 변환 일관성 확보

도메인 변환 함수를 통해 요일 매핑을 표준화한 점 👍

🧹 Nitpick comments (3)
core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmReceiver.kt (3)

121-127: O(n log n) 정렬 대신 O(n) 최소값 선택으로 간결·효율화 가능

정렬 후 첫 원소를 가져오기보다 minWithOrNull을 사용하면 불필요한 정렬 비용과 중간 컬렉션 생성을 줄일 수 있습니다.

-                    val earliestIdToday: Long? = alarms
-                        .asSequence()
-                        .filter { (it.isAlarmActive || it.id == notificationId) && it.ringsToday() }
-                        .sortedWith(compareBy({ it.hour }, { it.minute }, { it.second }))
-                        .firstOrNull()
-                        ?.id
+                    val earliestIdToday: Long? = alarms
+                        .asSequence()
+                        .filter { (it.isAlarmActive || it.id == notificationId) && it.ringsToday() }
+                        .minWithOrNull(
+                            compareBy<Alarm>({ it.hour }).thenBy { it.minute }.thenBy { it.second }
+                        )
+                        ?.id

128-134: 보상 플래그 저장 실패 시에도 UX 유지(브로드캐스트/로그)되도록 예외 흡수

DB/레포지토리 쓰기 실패가 전체 흐름을 깨지 않도록 runCatching/try-catch로 감싸는 것을 권장합니다. 브로드캐스트는 항상 실행되도록 보장하면 안정적입니다.

-                    if (isEarliestAlarmDismissedToday) fortuneRepository.markFirstAlarmDismissedToday()
+                    if (isEarliestAlarmDismissedToday) {
+                        runCatching { fortuneRepository.markFirstAlarmDismissedToday() }
+                            .onFailure { Log.w("AlarmReceiver", "markFirstAlarmDismissedToday 실패", it) }
+                    }

24-24: 시간 의존성 주입 권장 — LocalDate.now() 대신 Clock 주입으로 테스트성/재현성 향상

시간대/자정 경계 이슈와 테스트 용이성을 위해 java.time.Clock을 DI로 주입하고 LocalDate.now(clock)을 사용하면 좋습니다. 알람/미션 로직은 날짜·시간 경계에 민감합니다.

예시(파일 외 참고용):

class AlarmReceiver @Inject constructor(
    private val clock: Clock,
) : BroadcastReceiver() { /* ... */ }

// 사용
val todayAlarmDay = LocalDate.now(clock).dayOfWeek.toAlarmDay()
📜 Review details

Configuration used: .coderabbit.yaml
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.

📥 Commits

Reviewing files that changed from the base of the PR and between 0cdb890 and fdd4fe2.

📒 Files selected for processing (1)
  • core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmReceiver.kt (3 hunks)
🔇 Additional comments (3)
core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmReceiver.kt (3)

144-149: 브로드캐스트 전송 시점 이동 OK

오늘-기반 판정과 애널리틱스 로깅 이후에 액티비티 종료 브로드캐스트를 보내도록 순서를 정리한 점 합리적입니다. 위의 goAsync 적용까지 병행하면 안정성이 보장됩니다.


110-110: getAllAlarms().first() 사용 적절

Room Flow의 첫 스냅샷을 IO 컨텍스트에서 수집하는 방식은 적절합니다. 위의 goAsync 적용과 예외 처리 보강만 병행해 주세요.


112-112: SNOOZE ID 판별 로직 검토 필요

현재 SNOOZE_ID_OFFSET(값: 10000)와 DB에서 자동 생성되는 일반 알람 ID 간의 충돌 가능성이 있어, 실제 알람 ID가 offset 이상이 되면 스누즈 ID로 잘못 분류될 수 있습니다.

  • SNOOZE_ID_OFFSET 정의:
    • 파일: core/alarm/src/main/java/com/yapp/alarm/AlarmConstants.kt (라인 19)
  • 일반 알람 ID 자동 생성:
    • 파일: core/database/src/main/java/com/yapp/database/AlarmEntity.kt (@PrimaryKey(autoGenerate = true))
  • isSnoozeId 검사 위치:
    • 파일: core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmReceiver.kt (라인 112)

Offset 값이 실제 생성 가능한 알람 ID 범위보다 항상 크다는 가정을 재확인해주세요.
필요 시:

  • Offset 값을 충분히 큰 값으로 조정
  • 혹은 ID 필드 대신 별도 플래그 사용 방식으로 로직 개선 검토

@DongChyeon
Copy link
Member Author

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Sep 6, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (4)
core/network/src/main/java/com/yapp/network/di/NetworkModule.kt (1)

41-50: 타임아웃 300초는 과도합니다 — 호출 지연·UX 저하 가능, callTimeout과 보수적 값 권장

알람 트리거-백그라운드 시나리오라도 5분 타임아웃은 워커 재시도/취소 전략과 충돌할 수 있습니다. 일반적으로 connect=10s / read=30s / write=30s / call=60s 수준이 안전합니다. 또한 인증이 필요한 API라면 인증 인터셉터 주입 여부도 함께 확인 부탁드립니다.

권장 변경안:

-    fun provideHttpClient(
-        loggingInterceptor: HttpLoggingInterceptor,
-    ): OkHttpClient =
+    fun provideHttpClient(
+        loggingInterceptor: HttpLoggingInterceptor,
+        // authInterceptor: Interceptor, // 사용 중이라면 주입
+    ): OkHttpClient =
         OkHttpClient.Builder()
             .retryOnConnectionFailure(true)
             .addInterceptor(loggingInterceptor)
-            .readTimeout(300, TimeUnit.SECONDS)
-            .writeTimeout(300, TimeUnit.SECONDS)
-            .connectTimeout(300, TimeUnit.SECONDS)
+            // .addInterceptor(authInterceptor)
+            .callTimeout(60, TimeUnit.SECONDS)
+            .connectTimeout(10, TimeUnit.SECONDS)
+            .readTimeout(30, TimeUnit.SECONDS)
+            .writeTimeout(30, TimeUnit.SECONDS)
             .build()

추가 점검:

  • 장시간 처리(API 생성 대기 등)가 꼭 필요하다면 개별 API에만 별도 클라이언트/callTimeout을 적용하거나 WorkManager 백오프/재시도 정책으로 처리하는 편이 운영상 안전합니다.
feature/fortune/src/main/java/com/yapp/fortune/FortuneScreen.kt (3)

191-199: Preview 환경에서 무한 루프 방지 제안

LaunchedEffect 내부의 무한 루프는 Preview에서 불필요한 코루틴 실행/리소스 점유를 유발할 수 있습니다. Preview 감지 시 루프를 건너뛰도록 가드해 주세요.

다음처럼 수정하면 Preview에서 루프가 실행되지 않습니다.

@@
-    var isDelivering by remember { mutableStateOf(false) }
+    var isDelivering by remember { mutableStateOf(false) }
+    val isInPreview = LocalInspectionMode.current
@@
-    LaunchedEffect(Unit) {
-        while (true) {
-            delay(2000)
-            isDelivering = !isDelivering
-        }
-    }
+    LaunchedEffect(Unit) {
+        if (!isInPreview) {
+            while (true) {
+                delay(2000)
+                isDelivering = !isDelivering
+            }
+        }
+    }

추가 import:

import androidx.compose.ui.platform.LocalInspectionMode

220-225: Lottie 크기 고정값(375x267dp) → 반응형 레이아웃으로

고정 폭/높이는 소형 기기에서 잘림을 유발할 수 있습니다. 가로폭에 맞추고 비율을 유지하도록 변경을 권장합니다. 최대 폭은 기존 디자인(375dp)을 유지합니다.

@@
-            LottieAnimation(
-                modifier = Modifier
-                    .width(375.dp)
-                    .height(267.dp),
+            LottieAnimation(
+                modifier = Modifier
+                    .fillMaxWidth()
+                    .sizeIn(maxWidth = 375.dp)
+                    .aspectRatio(375f / 267f),
                 resId = core.designsystem.R.raw.fortune_loading,
             )

추가 import:

import androidx.compose.foundation.layout.aspectRatio
import androidx.compose.foundation.layout.sizeIn

206-218: 이미지 전환 접근성(선택)

말풍선 이미지는 장식적 용도라면 contentDescription=null 유지가 맞습니다. 다만 로딩 상태를 보조기기에 알릴 필요가 있다면 상위 Box에 semantics로 상태를 노출하는 방안을 고려해 주세요.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fdd4fe2 and 500862c.

⛔ Files ignored due to path filters (7)
  • core/designsystem/src/main/res/drawable-xhdpi/ic_100_buble.png is excluded by !**/*.png
  • core/designsystem/src/main/res/drawable-xhdpi/ic_fortune_delivering_speech_bubble.png is excluded by !**/*.png
  • core/designsystem/src/main/res/drawable-xhdpi/ic_fortune_waiting_speech_bubble.png is excluded by !**/*.png
  • core/designsystem/src/main/res/drawable-xxhdpi/ic_100_buble.png is excluded by !**/*.png
  • core/designsystem/src/main/res/drawable-xxhdpi/ic_fortune_delivering_speech_bubble.png is excluded by !**/*.png
  • core/designsystem/src/main/res/drawable-xxhdpi/ic_fortune_waiting_speech_bubble.png is excluded by !**/*.png
  • core/designsystem/src/main/res/drawable/ic_100_buble.png is excluded by !**/*.png
📒 Files selected for processing (5)
  • core/designsystem/src/main/res/raw/fortune_loading.json (1 hunks)
  • core/network/src/main/java/com/yapp/network/di/NetworkModule.kt (2 hunks)
  • core/network/src/main/java/com/yapp/network/di/Qualifier.kt (0 hunks)
  • data/src/main/java/com/yapp/data/remote/di/ServiceModule.kt (1 hunks)
  • feature/fortune/src/main/java/com/yapp/fortune/FortuneScreen.kt (3 hunks)
💤 Files with no reviewable changes (1)
  • core/network/src/main/java/com/yapp/network/di/Qualifier.kt
✅ Files skipped from review due to trivial changes (1)
  • core/designsystem/src/main/res/raw/fortune_loading.json
🔇 Additional comments (4)
data/src/main/java/com/yapp/data/remote/di/ServiceModule.kt (1)

16-17: 아래 수정된 스크립트로 Kotlin 파일(*.kt)을 glob 패턴으로 지정해 Qualifier/Provider 중복·잔존 여부를 재확인해 주세요.

#!/bin/bash
set -euo pipefail
echo "== Qualifier(@Auth/@NoneAuth/@S3) 사용 흔적 검색 =="
rg -nP '@(Auth|NoneAuth|S3)\b' -C2 --glob '*.kt' || true

echo "== Retrofit/OkHttpClient Provider 다중 정의 점검 =="
rg -nP '\b@Provides\b.*\bRetrofit\b' -C3 --glob '*.kt' || true
rg -nP '\b@Provides\b.*\bOkHttpClient\b' -C3 --glob '*.kt' || true

echo "== ApiService 주입 지점 점검(qualifier 기대 여부) =="
rg -nP 'Retrofit\.create|fun\s+provides?ApiService\(' -C2 --glob '*.kt' || true
core/network/src/main/java/com/yapp/network/di/NetworkModule.kt (1)

54-63: 단일 Retrofit·단일 baseUrl 전환 관련 외부 도메인 호출 검증 필요
현재 .kt 파일 전역 검색 결과, @Url 어노테이션이나 "http:///"https:// 패턴을 이용한 절대 URL 호출이 발견되지 않았습니다. S3 등 별도 호스트 호출이 필요한 API가 있다면

  • @Url 어노테이션을 통한 절대경로 사용
  • 또는 S3 전용 Retrofit 인스턴스(경량 바인딩) 추가
    중 하나를 적용해 오작동을 방지하세요.
feature/fortune/src/main/java/com/yapp/fortune/FortuneScreen.kt (2)

6-13: 레이아웃/치수 및 상태·코루틴 import 추가 적절

모두 실제로 사용되고 있고 불필요한 import 없음. 변경사항 깔끔합니다.

Also applies to: 20-20, 40-40


232-235: Preview 추가 좋습니다

테마 포함 Preview 구성 깔끔합니다. 위의 Preview 가드가 적용되면 프리뷰 성능도 안정적일 것입니다.

@DongChyeon DongChyeon merged commit a7723cd into release/1.1.3 Sep 8, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants