-
Notifications
You must be signed in to change notification settings - Fork 1
[REFACTOR] 운세 생성 상태 관리용 FortuneCreateStatusFlow 도입 #252
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[REFACTOR] 운세 생성 상태 관리용 FortuneCreateStatusFlow 도입 #252
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Walkthrough운세 생성 상태 관리를 기존의 복수 불린 플래그에서 단일 스트림 Flow로 통합했다. 도메인 모델을 추가하고, 리포지토리/로컬 데이터소스/뷰모델/워커/리시버가 이 스트림을 구독하도록 변경했다. 불필요한 tryMarkFortuneCreating 및 관련 불린 플로우를 제거했다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant VM as FortuneViewModel
participant Repo as FortuneRepository
participant DS as FortuneLocalDataSource
Note over Repo,DS: 단일 스트림으로 상태 제공
DS->>Repo: fortuneCreateStatusFlow
Repo-->>VM: fortuneCreateStatusFlow
VM->>Repo: collect fortuneCreateStatusFlow
alt Creating
VM->>VM: show loading
else Success(fortuneId)
VM->>Repo: isFirstAlarmDismissedTodayFlow.first()
VM->>Repo: fetch fortune by fortuneId
VM->>VM: update UI
else Failure or Idle
VM->>VM: NavigateToHome
end
sequenceDiagram
autonumber
participant Worker as PostFortuneWorker
participant Repo as FortuneRepository
participant DS as FortuneLocalDataSource
participant API as RemoteService
Worker->>Repo: fortuneCreateStatusFlow.first()
alt Creating or Success
Worker-->>Worker: Result.success()
else Failure or Idle
Worker->>Repo: get userId
alt userId missing
Worker->>Repo: markFortuneFailed()
Worker-->>Worker: Result.failure()
else userId exists
Worker->>Repo: markFortuneCreating()
Worker->>API: postFortune(userId)
alt API success
Worker->>Repo: markFortuneCreated(fortuneId)
Worker->>Repo: saveFortuneScore(...)
Worker-->>Worker: Result.success()
else API error/throwable
Worker->>Repo: markFortuneFailed()
Worker-->>Worker: Result.retry()
end
end
end
sequenceDiagram
autonumber
participant Receiver as AlarmInteractionActivityReceiver
participant Repo as FortuneRepository
Receiver->>Repo: fortuneCreateStatusFlow.first()
alt Creating or Success
Receiver->>Receiver: launch "orbitapp://fortune"
else Failure or Idle
alt hasValidMissionData
Receiver->>Receiver: launch "orbitapp://mission?..."
else
Receiver->>Receiver: no-op
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/fortune/src/main/java/com/yapp/fortune/FortuneViewModel.kt (1)
86-101: 마킹 실패가 전체 플로우를 깨지 않도록 방어
markFortuneSeen()실패 시 예외 전파로 UI 갱신이 중단될 수 있습니다. 로깅 후 진행하도록 방어하세요.적용 제안(diff):
- fortuneRepository.markFortuneSeen() + try { + fortuneRepository.markFortuneSeen() + } catch (t: Throwable) { + Log.w("FortuneViewModel", "운세 확인 마킹 실패", t) + }
🧹 Nitpick comments (9)
domain/src/main/java/com/yapp/domain/model/FortuneCreateStatus.kt (2)
3-8: 상태 계약(KDoc) 보강 제안소비자(Worker/VM/Receiver)가 의미를 일관되게 해석할 수 있도록 상태 전이 규칙을 주석으로 명시해 주세요.
다음과 같이 KDoc을 추가하는 것을 제안합니다:
package com.yapp.domain.model +/** + * Fortune 생성 상태의 단일 소스. + * Contract: + * - Idle: 아직 오늘 생성 시도 전 또는 상태 초기화됨. + * - Creating: 서버 생성 요청 진행 중(중복 생성 방지를 위한 락 의미 포함). + * - Success(fortuneId): 오늘 생성 성공 및 저장 완료. + * - Failure: 생성 실패(네트워크/서버/자격 문제 등). 재시도 정책은 상위 계층이 결정. + */ sealed class FortuneCreateStatus { data object Idle : FortuneCreateStatus() data object Creating : FortuneCreateStatus() data class Success(val fortuneId: Long) : FortuneCreateStatus() data object Failure : FortuneCreateStatus() }
3-8: 명명 일관성: Failure ↔ markFortuneAsFailed도메인 이벤트(
markFortuneAsFailed)와 상태(Failure)의 시제가 달라 가독성이 떨어집니다.Failure→Failed로 통일을 검토해 주세요. (크로스 파일 변경이므로 추후 배치 권장)feature/fortune/src/main/java/com/yapp/fortune/worker/PostFortuneWorker.kt (1)
33-38: userId 결여 시 실패 반환 대신 재시도 고려로그인 지연/세션 복구 등 일시적 상황일 수 있으니
Result.retry()가 더 안전합니다(백오프 정책 적용).다음 변경을 제안합니다:
- ?: run { - // 사용자 없으면 실패 상태 표시 후 실패 반환 - fortuneRepository.markFortuneAsFailed() - return Result.failure() - } + ?: run { + // 사용자 정보 없음: 재시도로 넘겨 세션/로그인 회복 기회 부여 + fortuneRepository.markFortuneAsFailed() + return Result.retry() + }feature/mission/src/main/java/com/yapp/mission/MissionViewModel.kt (2)
137-145: 타이포 수정: fortuneCreateStaus → fortuneCreateStatus가독성 및 오타 방지를 위해 변수명 수정 권장.
- val fortuneCreateStaus = fortuneRepository.fortuneCreateStatusFlow.first() - - when (fortuneCreateStaus) { + val fortuneCreateStatus = fortuneRepository.fortuneCreateStatusFlow.first() + + when (fortuneCreateStatus) { is FortuneCreateStatus.Creating, is FortuneCreateStatus.Success -> { postSideEffect(MissionContract.SideEffect.NavigateToFortune) } FortuneCreateStatus.Failure, FortuneCreateStatus.Idle -> { postSideEffect(MissionContract.SideEffect.NavigateBack) }
137-145: Idle→Creating 전환 레이스 대비 단기 관찰 옵션즉시
first()로 판단하면 직후 Creating으로 전이되는 케이스를 놓칠 수 있습니다. 1~2초 제한으로Creating/Success를 기다렸다가 없으면 Back으로 처리하는 수집 로직을 검토해 주세요.core/alarm/src/main/java/com/yapp/alarm/receivers/AlarmInteractionActivityReceiver.kt (1)
45-67: BroadcastReceiver 코루틴 사용은 적절. 다만 딥링크 대상 보장 점검 필요
goAsync()+finally{pending.finish()}패턴은 좋습니다. 딥링크 대상(Activity)이 항상 존재하는지 점검해주세요. 미존재 시ActivityNotFoundException위험이 있으니 필요하면resolveActivity체크를 권장합니다.feature/fortune/src/main/java/com/yapp/fortune/FortuneViewModel.kt (2)
67-69: Failure/Idle에서 로딩 해제 누락 가능성바로 홈으로 내비게이트하더라도
isLoading을 명시적으로 false로 내려 두면 화면 전환 지연/되돌아오기 시 스피너 잔상 위험을 줄일 수 있습니다.적용 제안(diff):
- is FortuneCreateStatus.Failure, FortuneCreateStatus.Idle -> { - postSideEffect(FortuneContract.SideEffect.NavigateToHome) - } + is FortuneCreateStatus.Failure, FortuneCreateStatus.Idle -> { + reduce { state.copy(isLoading = false) } + postSideEffect(FortuneContract.SideEffect.NavigateToHome) + }
53-66: 상태 전이 정책 확인 요청(제품 결정 사항)
Idle또는Failure에서 즉시 홈으로 보내는 현재 정책이 의도된 UX인지 확인 바랍니다. 알람 진입 시 네트워크 지연 등으로 잠깐Idle이 나왔다가Creating/Success로 바뀌면 ‘튕김’ 경험이 날 수 있습니다. 필요하면 짧은 그레이스 타임(예: 300–500ms) 후 내비게이션을 트리거하는 방식을 고려하세요.data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSource.kt (1)
15-16: 새 상태 플로우의 방출 규칙을 KDoc으로 명세해 주세요소비자(뷰모델/워커/리시버)가 일관되게 동작하려면 우선순위/전이 조건이 문서화되어야 합니다. 예: Failure > Creating > Success(today & id != null) > Idle.
적용 제안(diff):
- val fortuneCreateStatusFlow: Flow<FortuneCreateStatus> + /** + * Fortune 생성 상태의 단일 소스. + * 방출 우선순위(동시 참일 때): Failure > Creating > Success(오늘 & id 존재) > Idle + * 이 플로우는 DataStore 기반 하위 플로우 결합 결과를 distinctUntilChanged 후 제공합니다. + */ + val fortuneCreateStatusFlow: Flow<FortuneCreateStatus>
📜 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(0 hunks)data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSource.kt(2 hunks)data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.kt(2 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(2 hunks)feature/fortune/src/main/java/com/yapp/fortune/FortuneViewModel.kt(2 hunks)feature/fortune/src/main/java/com/yapp/fortune/worker/PostFortuneWorker.kt(2 hunks)feature/mission/src/main/java/com/yapp/mission/MissionViewModel.kt(2 hunks)
💤 Files with no reviewable changes (1)
- core/datastore/src/main/java/com/yapp/datastore/UserPreferences.kt
🔇 Additional comments (5)
data/src/main/java/com/yapp/data/repositoryimpl/FortuneRepositoryImpl.kt (1)
25-26: LGTM: 단일 Flow 위임으로 상태 일원화
fortuneCreateStatusFlow를 로컬 DS에 위임하는 구조 적절합니다. 상위 계층의 구독 비용/빈도를 고려해 DS 측에서distinctUntilChanged()가 적용되어 있는지만 확인하면 좋겠습니다.feature/fortune/src/main/java/com/yapp/fortune/worker/PostFortuneWorker.kt (1)
24-33: Flowfirst()블로킹 가능성 점검
fortuneCreateStatusFlow.first()가 cold이고 초기값이 보장되지 않으면 대기할 수 있습니다. DS/StateFlow로 초기 emit 보장 여부 확인 부탁드립니다.domain/src/main/java/com/yapp/domain/model/FortuneCreateStatus.kt (1)
3-8: data object 사용 — Kotlin 버전 확인 완료gradle/libs.versions.toml에 kotlin = "2.0.0"로 설정되어 있으므로 data object 사용에 호환성 문제가 없습니다. (kotlinlang.org)
data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.kt (2)
23-36: 확인: Success는 data class이므로 distinctUntilChanged가 값 비교로 중복 방출을 억제합니다.
domain/src/main/java/com/yapp/domain/model/FortuneCreateStatus.kt에서data class Success(val fortuneId: Long)로 정의되어 있습니다.
37-37: 확인: fortuneDate는 ISO_DATE로 저장·비교됩니다.
근거: core/datastore/src/main/java/com/yapp/datastore/UserPreferences.kt의 today()가 DateTimeFormatter.ISO_DATE로 값을 저장(pref[Keys.FORTUNE_DATE] = today()); data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.kt의 today()와 feature/home/src/main/java/com/yapp/home/HomeViewModel.kt·feature/alarm-interaction/src/main/java/com/yapp/alarm/interaction/snooze/AlarmSnoozeTimerViewModel.kt의 비교도 LocalDate.now().format(DateTimeFormatter.ISO_DATE)를 사용합니다.
Related issue 🛠
closed #251
어떤 변경사항이 있었나요?
CheckPoint ✅
PR이 다음 요구 사항을 충족하는지 확인하세요.
Work Description ✏️
운세 생성 상태를 hasUnseenFortune, isFortuneCreating, isFortuneFailed 플래그 대신 fortuneCreateStatusFlow로 통합 노출하도록 리팩토링
Uncompleted Tasks 😅
현재 오프라인 상태에서 운세 생성 시 API 호출이 이루어지지 않고 홈 화면으로 이동. 이후 네트워크가 연결되면 운세가 생성되지만, 사용자에게 알림이 부족한 상태
고민 중인 대응 방안:
To Reviewers 📢
Summary by CodeRabbit