feat: 푸시 알림 클라이언트 로직 구현 및 서버 API 연동#204
Conversation
- 사용자가 알림 권한을 허용/거부했을 때 서버와 상태를 동기화하는 기능 추가 - 설정 화면에서 알림 토글 시 서버와 상태를 동기화하도록 수정
- 시스템 알림 권한과 서버에 마지막으로 동기화된 상태가 다를 경우에만 동기화 진행
- 로컬 토큰과 원격 토큰을 비교하여 변경되었을 때만 서버에 업데이트 요청 -`FirebaseMessagingService`의 `onNewToken` 콜백에서 `syncFcmToken`을 호출하여 토큰 갱신 시 자동으로 동기화되도록 처리
WalkthroughFCM 토큰 동기화와 푸시 수신 서비스가 추가되었고, 알림 채널 초기화자와 Firebase 메시징 의존성·DI 제공자가 도입되었습니다. 데이터·네트워크·데이터스토어·UI API가 토큰·알림 설정 중심으로 확장되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant FM as FirebaseMessaging
participant SVC as ReedFirebaseMessagingService
participant Repo as UserRepository
participant Server as ReedServer
participant Sys as AndroidNotification
FM->>SVC: onNewToken(token)
SVC->>Repo: syncFcmToken(token) (백그라운드 코루틴)
Repo->>Server: PUT /profile/fcm-token (FcmTokenRequest)
Server-->>Repo: UserProfileResponse
Repo->>Repo: 로컬 토큰 저장
FM->>SVC: onMessageReceived(message)
SVC->>SVC: Notification 생성 (REED_CHANNEL_ID)
SVC->>Sys: 알림 표시
sequenceDiagram
autonumber
participant User as 사용자
participant UI as 설정 UI
participant Pres as NotificationPresenter
participant Repo as UserRepository
participant Server as ReedServer
User->>UI: 알림 토글(enabled)
UI->>Pres: OnNotificationToggle(enabled)
Pres->>Repo: updateNotificationSettings(enabled)
Repo->>Server: PUT /profile/notification-settings (NotificationSettingsRequest)
Server-->>Repo: UserProfileResponse
Pres->>UI: 상태 갱신 / sideEffect(ShowToast)
sequenceDiagram
autonumber
participant System as OS
participant Home as 홈 UI
participant Pres as HomePresenter
participant Repo as UserRepository
System->>Home: 권한 변경(허용/거부)
Home->>Pres: OnNotificationPermissionResult(granted)
Pres->>Repo: getLastSyncedNotificationEnabled()
alt lastSynced가 없거나 불일치
Pres->>Repo: syncNotificationSettings(granted)
else
Pres->>Pres: 동기화 생략
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 주의할 점:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-10-17T08:03:00.309ZApplied to files:
📚 Learning: 2025-09-01T11:05:37.684ZApplied to files:
🧬 Code graph analysis (1)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationUi.kt (2)
⏰ 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)
🔇 Additional comments (5)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (1)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationUi.kt (1)
172-178: 중복 유틸 안내(중복 코멘트)checkSystemNotificationEnabled는 HomeUi.kt에도 동일 구현이 있으므로 공용화 권장 코멘트를 해당 파일에 남겼습니다. 여기서는 중복 코멘트를 생략합니다.
🧹 Nitpick comments (18)
core/designsystem/src/main/res/drawable/ic_notification.xml (1)
1-9: LGTM!알림 아이콘 리소스가 적절하게 정의되었습니다. 흰색 아이콘(#ffffff)은 일반적인 알림 디자인 패턴을 따릅니다.
다양한 기기와 알림 배경에서 아이콘의 가시성을 테스트해보시기를 권장드립니다.
feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashPresenter.kt (1)
57-57: FCM 토큰 동기화 실패 처리 고려토큰 동기화 실패 시 로깅이나 에러 처리가 없어 문제 추적이 어려울 수 있습니다. 사용자 경험에 영향을 주지 않는 것이 의도된 것으로 보이지만, 최소한 실패 시 로깅을 추가하는 것을 권장합니다.
다음과 같이 에러 로깅을 추가할 수 있습니다:
- userRepository.syncFcmToken() + userRepository.syncFcmToken() + .onFailure { exception -> + Logger.e("FCM token sync failed: ${exception.message}") + } navigator.resetRoot(HomeScreen)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/HandleNotificationSideEffects.kt (1)
21-21: 불필요한 else 분기 제거 권장빈 else 분기는 제거할 수 있습니다.
when (state.sideEffect) { is NotificationSideEffect.ShowToast -> { Toast.makeText(context, state.sideEffect.message, Toast.LENGTH_SHORT).show() } - - else -> {} }feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomePresenter.kt (1)
66-73: 오류 처리 일관화(로그만 남기지 말고 인증 만료/네트워크 예외를 공통 처리)현재 실패 시 Logger.e만 수행합니다. NotificationPresenter처럼 handleException을 사용하면 401 시 로그인 화면으로 보내고, 네트워크 오류에 사용자 피드백을 줄 수 있습니다. 홈에서도 최소한 인증 만료 대응을 권장합니다.
예시:
+import com.ninecraft.booket.core.common.utils.handleException +import com.ninecraft.booket.feature.screens.LoginScreen @@ suspend fun syncNotificationSettings(isGranted: Boolean) { userRepository.updateNotificationSettings(isGranted) .onSuccess { userRepository.setLastNotificationSyncedEnabled(isGranted) - }.onFailure { exception -> - Logger.e("Failed to update notification settings: $exception") - } + }.onFailure { exception -> + handleException( + exception = exception, + onError = { msg -> Logger.e(msg) }, + onLoginRequired = { navigator.resetRoot(LoginScreen()) }, + ) + } }feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt (1)
236-242: 중복 유틸 정리: checkSystemNotificationEnabled를 공용으로 추출 권장동일 구현이 Settings/NotificationUi.kt에도 존재합니다. core-common 등으로 추출해 단일 소스로 유지해 주세요. (테스트 용이성·일관성 향상)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationUi.kt (1)
165-168: 접근성/i18n: contentDescription 문자열 하드코딩 회피영문 하드코딩 대신 string 리소스로 옮겨 주세요.
- contentDescription = "Chevron Right Icon", + contentDescription = stringResource(R.string.cd_chevron_right),strings.xml (settings 모듈)에 다음을 추가해 주세요:
<string name="cd_chevron_right">이동</string>app/src/main/kotlin/com/ninecraft/booket/ReedFirebaseMessagingService.kt (4)
56-67: 알림 채널 보장 후 notify 호출O+에서 채널이 없으면 알림이 표시되지 않습니다. 안전하게
createNotificationChannel(this)를 호출한 뒤notify하세요. 스타트업 초기화가 있더라도 중복 호출은 무해합니다.- val builder = NotificationCompat.Builder(this, REED_CHANNEL_ID) + // 채널 보장 + createNotificationChannel(this) + val builder = NotificationCompat.Builder(this, REED_CHANNEL_ID) .setSmallIcon(R.drawable.ic_notification) ... - val manager = getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager - manager.notify(System.currentTimeMillis().toInt(), builder.build()) + val manager = getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager + // 양수 범위의 랜덤 ID로 충돌/오버플로 최소화 + val notificationId = kotlin.random.Random.nextInt(1, Int.MAX_VALUE) + manager.notify(notificationId, builder.build())
66-66: Notification ID로System.currentTimeMillis().toInt()사용 지양
toInt()로 인한 오버플로/음수 및 충돌 가능성이 있습니다. 양수 범위 랜덤 또는 내부 카운터 사용을 권장합니다. 위 코멘트의 diff 참고.
42-44: 데이터 전용(data-only) 메시지 처리 보완
message.notification가 null인 케이스에서message.data를 폴백으로 사용하면 수신 커버리지가 넓어집니다.- val title = message.notification?.title ?: "Reed" - val body = message.notification?.body ?: "" + val title = message.notification?.title ?: message.data["title"] ?: "Reed" + val body = message.notification?.body ?: message.data["body"] ?: ""
74-78: 채널 문자열을 리소스로 이전 고려채널 이름/설명은 문자열 리소스로 이전하면 i18n 및 관리에 유리합니다.
core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/datasource/DefaultNotificationDataSource.kt (3)
24-28: FCM 토큰 삭제/초기화 경로 부재로그아웃 등에서 토큰 제거가 필요할 수 있습니다. null 또는 빈 문자열로 의미를 표현하기보다 키 제거 API를 제공하세요.
- override suspend fun setFcmToken(fcmToken: String) { - dataStore.edit { prefs -> - prefs[FCM_TOKEN] = fcmToken - } - } + override suspend fun setFcmToken(fcmToken: String) { + dataStore.edit { prefs -> + if (fcmToken.isBlank()) { + prefs.remove(FCM_TOKEN) + } else { + prefs[FCM_TOKEN] = fcmToken + } + } + }원하면
clearFcmToken()별도 API도 추가 가능합니다.
42-52: 동기화 상태(Boolean?) 초기화 지원
lastSyncedNotificationEnabled가 nullable인 만큼, 키 제거를 통한 “미동기화 상태” 초기화 메서드가 있으면 유용합니다.override suspend fun setLastSyncedNotificationEnabled(isEnabled: Boolean) { dataStore.edit { prefs -> prefs[LAST_SYNCED_NOTIFICATION_ENABLED] = isEnabled } } + + // 선택: 동기화 상태 초기화 + suspend fun clearLastSyncedNotificationEnabled() { + dataStore.edit { prefs -> prefs.remove(LAST_SYNCED_NOTIFICATION_ENABLED) } + }
54-58: 사소한 네이밍/스타일
companion object Companion로 이름을 붙일 필요는 없습니다. 기본companion object권장. (취향 차, 선택 사항)core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/UserRepository.kt (2)
17-21:syncFcmToken오버로드의 계약(Contract) 명시 필요두 메서드 모두 “원격 갱신 실패 시 Result.failure를 반환하고, 로컬 토큰은 갱신하지 않는다”는 불변식을 명시해 주세요. 아래 impl에서 실패 전파가 중요합니다.
27-31: 네이밍 일관성: setLastNotificationSyncedEnabled
NotificationDataSource.setLastSyncedNotificationEnabled와 네이밍이 비대칭입니다. 통일 고려 요청.core/datastore/api/src/main/kotlin/com/ninecraft/booket/core/datastore/api/datasource/NotificationDataSource.kt (1)
6-13: 토큰/동기화 상태 초기화 API 제안실사용 시 토큰/동기화 상태를 제거해야 하는 시점이 있습니다. nullable 파라미터 처리 혹은 명시적 clear 메서드를 인터페이스에 추가하는 방안을 제안합니다.
interface NotificationDataSource { val fcmToken: Flow<String> - suspend fun setFcmToken(fcmToken: String) + suspend fun setFcmToken(fcmToken: String) + suspend fun clearFcmToken() @@ val lastSyncedNotificationEnabled: Flow<Boolean?> suspend fun setLastSyncedNotificationEnabled(isEnabled: Boolean) + suspend fun clearLastSyncedNotificationEnabled() }core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultUserRepository.kt (2)
78-90: 토큰 획득 코루틴 래핑 간소화 제안
kotlinx-coroutines-play-services를 사용 중이라면firebaseMessaging.token.await()로 간단히 대체 가능합니다. 의존이 없다면 현 구현 유지.private suspend fun getRemoteFcmToken(): String = com.google.android.gms.tasks.Tasks.await(firebaseMessaging.token) // 또는 token.await() 확장함수
74-76: 불필요한toModel()제거로 오버헤드 축소FCM 토큰 업데이트 응답을 사용하지 않는다면 매핑을 생략해 비용을 줄이세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
app/src/main/AndroidManifest.xml(3 hunks)app/src/main/kotlin/com/ninecraft/booket/ReedFirebaseMessagingService.kt(1 hunks)app/src/main/kotlin/com/ninecraft/booket/initializer/NotificationChannelInitializer.kt(1 hunks)build-logic/src/main/kotlin/AndroidFirebaseConventionPlugin.kt(1 hunks)core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/UserRepository.kt(1 hunks)core/data/impl/build.gradle.kts(1 hunks)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/di/FirebaseModule.kt(2 hunks)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/mapper/ResponseToModel.kt(1 hunks)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultUserRepository.kt(2 hunks)core/datastore/api/src/main/kotlin/com/ninecraft/booket/core/datastore/api/datasource/NotificationDataSource.kt(1 hunks)core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/datasource/DefaultNotificationDataSource.kt(2 hunks)core/designsystem/src/main/res/drawable/ic_notification.xml(1 hunks)core/model/src/main/kotlin/com/ninecraft/booket/core/model/UserProfileModel.kt(1 hunks)core/network/src/main/kotlin/com/ninecraft/booket/core/network/request/FcmTokenRequest.kt(1 hunks)core/network/src/main/kotlin/com/ninecraft/booket/core/network/request/NotificationSettingsRequest.kt(1 hunks)core/network/src/main/kotlin/com/ninecraft/booket/core/network/response/UserProfileResponse.kt(1 hunks)core/network/src/main/kotlin/com/ninecraft/booket/core/network/service/ReedService.kt(2 hunks)feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomePresenter.kt(4 hunks)feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt(4 hunks)feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUiState.kt(1 hunks)feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginPresenter.kt(1 hunks)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/HandleNotificationSideEffects.kt(1 hunks)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationPresenter.kt(2 hunks)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationUi.kt(4 hunks)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationUiState.kt(1 hunks)feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashPresenter.kt(1 hunks)gradle/libs.versions.toml(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#46
File: feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/component/InfiniteLazyColumn.kt:83-95
Timestamp: 2025-07-14T00:46:03.843Z
Learning: seoyoon513과 팀은 한국어 주석을 선호하며, 한국어 주석을 영어로 번역하라는 제안을 하지 않아야 함
📚 Learning: 2025-10-17T08:03:00.309Z
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#192
File: feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt:74-76
Timestamp: 2025-10-17T08:03:00.309Z
Learning: Android 13 이상에서는 POST_NOTIFICATIONS 런타임 권한과 시스템 알림 설정(areNotificationsEnabled)이 완전히 동기화되어 있음. 하나의 토글로 둘 다 함께 변경되므로, checkSelfPermission() 체크만으로 충분함. Android 12 이하에서는 런타임 권한이 없으므로 areNotificationsEnabled()만 사용해야 함.
Applied to files:
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.ktfeature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationUi.kt
🧬 Code graph analysis (8)
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt (1)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationUi.kt (1)
checkSystemNotificationEnabled(172-178)
app/src/main/kotlin/com/ninecraft/booket/initializer/NotificationChannelInitializer.kt (1)
app/src/main/kotlin/com/ninecraft/booket/ReedFirebaseMessagingService.kt (1)
createNotificationChannel(80-91)
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomePresenter.kt (1)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationPresenter.kt (1)
syncNotificationSettings(60-79)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/HandleNotificationSideEffects.kt (1)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/osslicenses/OssLicensesUiState.kt (1)
eventSink(6-8)
build-logic/src/main/kotlin/AndroidFirebaseConventionPlugin.kt (1)
build-logic/src/main/kotlin/com/ninecraft/booket/convention/Dependencies.kt (1)
implementation(6-8)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationPresenter.kt (4)
core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/HandleException.kt (1)
handleException(19-46)feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomePresenter.kt (1)
syncNotificationSettings(66-73)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultUserRepository.kt (1)
updateNotificationSettings(74-76)core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/UserRepository.kt (1)
updateNotificationSettings(31-31)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationUi.kt (2)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/HandleNotificationSideEffects.kt (1)
HandleNotificationSideEffects(8-28)feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt (1)
checkSystemNotificationEnabled(236-242)
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultUserRepository.kt (4)
core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/RunSuspendCatching.kt (1)
runSuspendCatching(16-30)core/network/src/main/kotlin/com/ninecraft/booket/core/network/service/ReedService.kt (1)
updateFcmToken(56-57)core/datastore/api/src/main/kotlin/com/ninecraft/booket/core/datastore/api/datasource/NotificationDataSource.kt (1)
setFcmToken(7-7)core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/datasource/DefaultNotificationDataSource.kt (1)
setFcmToken(24-28)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci-build
🔇 Additional comments (20)
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUiState.kt (1)
46-46: 변경사항이 일관되게 적용되었습니다: 호환성 문제 없음코드베이스 전체를 검증한 결과,
isGranted에서granted로의 이름 변경이 모든 호출처와 접근처에 일관되게 적용되었습니다:
- ✅ 모든 접근처 업데이트됨:
event.granted로 올바르게 변경됨 (HomePresenter.kt:107, NotificationPresenter.kt:93)- ✅ 호환성 문제 없음: 위치 인수를 통한 호출이므로 프로퍼티 이름 변경이 영향을 주지 않음
- ✅ 일관된 변경: NotificationUiState.kt와 함께 동일하게 적용됨
다만, 코드베이스 내 boolean 프로퍼티 네이밍 스타일이 혼재되어 있습니다 (
isGuestMode,isPermissionGrantedvsgranted,enabled). 장기적으로는 전체 코드베이스에서 네이밍 컨벤션을 통일하는 것을 고려해보세요.gradle/libs.versions.toml (1)
152-152: LGTM!Firebase Messaging 라이브러리가 올바르게 추가되었습니다. BOM을 통한 버전 관리 방식도 다른 Firebase 라이브러리들과 일관성 있게 적용되었습니다.
build-logic/src/main/kotlin/AndroidFirebaseConventionPlugin.kt (1)
22-22: LGTM!Firebase Messaging 의존성이 적절하게 추가되었습니다. 다른 Firebase 의존성들과 동일한 패턴을 따르고 있습니다.
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/di/FirebaseModule.kt (1)
31-33: LGTM!FirebaseMessaging 인스턴스가 올바르게 제공되고 있습니다. Singleton 스코프와 getInstance() 패턴이 적절하게 사용되었습니다.
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationUiState.kt (3)
8-12: LGTM!NotificationUiState에 side effect 필드가 적절하게 추가되었습니다. nullable 타입과 기본값 null 설정이 올바릅니다.
14-20: 훌륭한 설계입니다!UUID 기반 key를 사용하여 동일한 메시지가 여러 번 표시될 때 발생할 수 있는 Compose 재구성 문제를 방지하고 있습니다. private 접근자로 설정하여 외부 비교를 막은 것도 의도적인 설계로 보입니다.
22-27: LGTM!새로운 이벤트들이 적절하게 정의되었습니다. InitSideEffect는 side effect 초기화에, OnNotificationPermissionResult는 권한 결과 처리에 사용될 것으로 보입니다.
core/data/impl/build.gradle.kts (1)
31-31: LGTM!Firebase Messaging 의존성이 올바른 위치에 추가되었습니다. BOM 플랫폼과 함께 적절하게 구성되었습니다.
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/mapper/ResponseToModel.kt (1)
45-54: LGTM!
notificationEnabled필드가 UserProfileModel로 적절하게 매핑되고 있습니다.feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginPresenter.kt (1)
90-90: FCM 토큰 동기화 에러 처리 패턴 검증 완료
syncFcmToken()호출 패턴을 검증한 결과, 다음이 확인되었습니다:
- SplashPresenter와 LoginPresenter 모두 동일한 방식으로
Result<Unit>반환값을 무시하고 있습니다- DefaultUserRepository 구현에서
runSuspendCatching을 통해 내부적으로 예외 처리 및 로깅이 이루어집니다- 이 패턴은 일관되고 의도적인 설계로, PR의 opt-out 전략을 따르고 있습니다
호출 지점에서 결과를 무시하는 것은 토큰 동기화 실패가 사용자의 로그인 흐름을 방해하지 않도록 하는 opt-out 전략의 일부입니다. 에러는 내부에서 적절히 처리되고 있으므로 추가적인 수정이 필요하지 않습니다.
core/network/src/main/kotlin/com/ninecraft/booket/core/network/request/NotificationSettingsRequest.kt (1)
6-10: LGTM!알림 설정 요청 모델이 올바르게 구현되었습니다.
core/network/src/main/kotlin/com/ninecraft/booket/core/network/request/FcmTokenRequest.kt (1)
6-10: LGTM!FCM 토큰 요청 모델이 올바르게 구현되었습니다.
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/HandleNotificationSideEffects.kt (1)
8-28: 사이드 이펙트 처리 로직 확인됨Toast 표시 및 사이드 이펙트 초기화가 올바르게 구현되었습니다.
app/src/main/kotlin/com/ninecraft/booket/initializer/NotificationChannelInitializer.kt (1)
7-16: LGTM!알림 채널 초기화가 androidx.startup 패턴을 사용하여 올바르게 구현되었습니다. 앱 시작 시 알림 채널을 생성하는 것이 적절합니다.
core/network/src/main/kotlin/com/ninecraft/booket/core/network/service/ReedService.kt (1)
56-60: LGTM!FCM 토큰 업데이트와 알림 설정 API 엔드포인트가 올바르게 정의되었습니다. PUT 메서드 사용과 UserProfileResponse 반환이 적절합니다.
core/model/src/main/kotlin/com/ninecraft/booket/core/model/UserProfileModel.kt (1)
12-12: LGTM!UserProfileModel에
notificationEnabled필드가 추가되어 네트워크 레이어 변경사항과 일관성을 유지합니다.core/network/src/main/kotlin/com/ninecraft/booket/core/network/response/UserProfileResponse.kt (1)
18-19: 서버 배포 상태 확인 필수검증 결과,
notificationEnabled필드가 기본값 없이 필수 필드로 정의되어 있습니다. kotlinx.serialization에서는 기본값이 없는 필드를 필수로 처리하므로, 서버가 이 필드를 반환하지 않으면 역직렬화 실패(MissingFieldException)가 발생하여 앱이 충돌합니다.
- 현재 코드: 필드에 기본값 없음 → 서버 응답 필수
- 영향 범위:
getUserProfile(),updateFcmToken(),updateNotificationSettings()세 엔드포인트 모두 반영- 필수 조치: 서버 배포 우선 완료 확인 또는 기본값 추가
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationPresenter.kt (1)
60-79: 권한-서버 동기화 판단 로직 적절함권한 해제 시 서버 Off, 권한 재허용은 userEnabled가 true일 때만 On 동기화하는 분기 구조가 의도(옵트아웃 유지)에 부합합니다. 홈도 동일 기준으로 맞추면 일관성이 좋아집니다.
홈 쪽 로직을 정렬하면 시연 항목 “사용자 Off 유지”가 모든 화면에서 보장됩니다.
Also applies to: 91-103
app/src/main/AndroidManifest.xml (1)
80-87: FCM 서비스/리소스 배선 검증 완료 - 모든 요소 정상 확인검증 결과:
- ReedFirebaseMessagingService 경로:
com.ninecraft.booket.ReedFirebaseMessagingService→ 매니페스트의.ReedFirebaseMessagingService상대 경로와 일치 ✓- NotificationChannelInitializer: androidx.startup 프로바이더를 통해 매니페스트에 등록되어 있으며, createNotificationChannel 함수를 올바르게 호출 ✓
- 리소스 검증:
ic_notification드로어블과green_500색상이 모두 core/designsystem에 존재하고, 매니페스트 메타-데이터에서 올바르게 참조됨 ✓core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/datasource/DefaultNotificationDataSource.kt (1)
30-35: 기본값 true 확인 요청옵트아웃 전략이라면 기본값
true는 합리적입니다. 초기 앱 설치 후 서버와의 첫 동기화 시 의도한 UX인지 한 번 더 확인 부탁드립니다.
| val manager = context.getSystemService(NOTIFICATION_SERVICE) as NotificationManager | ||
| manager.createNotificationChannel(channel) | ||
| } |
There was a problem hiding this comment.
컴파일 에러: NOTIFICATION_SERVICE 미수입 사용
NOTIFICATION_SERVICE가 정규화되지 않아 컴파일 오류가 납니다. Context. 접두어를 붙이거나 정적 임포트를 추가하세요.
- val manager = context.getSystemService(NOTIFICATION_SERVICE) as NotificationManager
+ val manager = context.getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| val manager = context.getSystemService(NOTIFICATION_SERVICE) as NotificationManager | |
| manager.createNotificationChannel(channel) | |
| } | |
| val manager = context.getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager | |
| manager.createNotificationChannel(channel) | |
| } |
🤖 Prompt for AI Agents
In app/src/main/kotlin/com/ninecraft/booket/ReedFirebaseMessagingService.kt
around lines 89 to 91, the symbol NOTIFICATION_SERVICE is unqualified causing a
compile error; update the reference to Context.NOTIFICATION_SERVICE or add an
appropriate static import so the call becomes
context.getSystemService(Context.NOTIFICATION_SERVICE) (or import
Context.NOTIFICATION_SERVICE) and then cast to NotificationManager as before,
ensuring the Context qualifier is present.
- runSuspendCatching으로 감싸지 않고 예외를 그대로 던지도록 수정
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt (1)
240-246: 코드 중복: 공통 유틸리티로 추출을 권장합니다
checkSystemNotificationEnabled함수가NotificationUi.kt(lines 171-177)에도 동일하게 존재합니다. 이 헬퍼 함수를core/ui또는core/common모듈의 공통 유틸리티로 추출하여 중복을 제거해 주세요.예시:
// core/ui/src/main/kotlin/com/ninecraft/booket/core/ui/util/NotificationUtils.kt fun Context.isNotificationEnabled(): Boolean { return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { ContextCompat.checkSelfPermission(this, Manifest.permission.POST_NOTIFICATIONS) == PackageManager.PERMISSION_GRANTED } else { NotificationManagerCompat.from(this).areNotificationsEnabled() } }core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultUserRepository.kt (1)
78-90: 취소 시 리스너 정리 추가 고려Firebase Task API를 코루틴으로 변환하는 구현이 올바릅니다. 성공/실패 케이스 모두 적절히 처리되고 있습니다.
코루틴 취소 시 Firebase 리스너를 정리하는 핸들러를 추가하면 더 좋습니다:
private suspend fun getRemoteFcmToken(): String = suspendCancellableCoroutine { continuation -> + val listener = firebaseMessaging.token.addOnCompleteListener { task -> - firebaseMessaging.token.addOnCompleteListener { task -> if (task.isSuccessful) { continuation.resume(task.result) } else { task.exception?.let { exception -> continuation.resumeWithException(exception) } ?: continuation.resumeWithException( Exception("Unknown error occurred while fetching FCM token"), ) } } + + continuation.invokeOnCancellation { + // Firebase Task는 취소할 수 없지만, 리소스 정리 의도를 명시 + } }참고: Firebase Task는 취소 불가하지만 명시적 cancellation 핸들러는 코드 의도를 명확히 하고 향후 유지보수에 도움이 됩니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultUserRepository.kt(2 hunks)feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomePresenter.kt(4 hunks)feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt(4 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#46
File: feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/component/InfiniteLazyColumn.kt:83-95
Timestamp: 2025-07-14T00:46:03.843Z
Learning: seoyoon513과 팀은 한국어 주석을 선호하며, 한국어 주석을 영어로 번역하라는 제안을 하지 않아야 함
📚 Learning: 2025-10-17T08:03:00.309Z
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#192
File: feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt:74-76
Timestamp: 2025-10-17T08:03:00.309Z
Learning: Android 13 이상에서는 POST_NOTIFICATIONS 런타임 권한과 시스템 알림 설정(areNotificationsEnabled)이 완전히 동기화되어 있음. 하나의 토글로 둘 다 함께 변경되므로, checkSelfPermission() 체크만으로 충분함. Android 12 이하에서는 런타임 권한이 없으므로 areNotificationsEnabled()만 사용해야 함.
Applied to files:
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt
🧬 Code graph analysis (3)
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt (1)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationUi.kt (1)
checkSystemNotificationEnabled(172-178)
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultUserRepository.kt (4)
core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/RunSuspendCatching.kt (1)
runSuspendCatching(16-30)core/network/src/main/kotlin/com/ninecraft/booket/core/network/service/ReedService.kt (1)
updateFcmToken(56-57)core/datastore/api/src/main/kotlin/com/ninecraft/booket/core/datastore/api/datasource/NotificationDataSource.kt (1)
setFcmToken(7-7)core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/datasource/DefaultNotificationDataSource.kt (1)
setFcmToken(24-28)
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomePresenter.kt (1)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationPresenter.kt (1)
syncNotificationSettings(60-79)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci-build
🔇 Additional comments (8)
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomePresenter.kt (2)
66-73: 에러 처리 일관성 고려가 필요할 수 있습니다NotificationPresenter의 syncNotificationSettings는 실패 시 Toast를 보여주고 인증 오류 시 로그인 화면으로 이동하지만, 여기서는 로그만 남기고 있습니다. 홈 화면에서 덜 침해적인 UX를 의도하신 것이라면 괜찮지만, 동기화 실패 시 사용자가 전혀 알지 못할 수 있습니다.
설정 화면과 다르게 처리하는 명확한 의도가 있는지 확인해 주세요.
105-118: 이전 리뷰의 opt-out 이슈가 올바르게 수정되었습니다이제
userEnabled를 확인하여 사용자가 명시적으로 Off 설정한 경우 OS 권한 재허용 시에도 Off 상태를 유지합니다. NotificationPresenter의 로직과 일관되게 구현되었습니다.feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt (1)
72-81: Android 12 이하 기기에서 false 이벤트 전송 이슈가 수정되었습니다이제 Android 12 이하에서도
isGranted값(false 포함)을 Presenter로 전달하여 서버 동기화가 올바르게 동작합니다. 이전 리뷰의 제안이 잘 반영되었습니다.core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultUserRepository.kt (5)
3-3: LGTM: 의존성 추가가 적절합니다FCM 토큰 동기화를 위한 FirebaseMessaging, 네트워크 요청 타입, 로깅, 코루틴 유틸리티 임포트가 모두 적절하게 추가되었습니다. DI를 통한 FirebaseMessaging 주입도 올바릅니다.
Also applies to: 9-10, 13-19, 25-25
41-52: 수정 완료 확인: 네트워크 실패 시 로컬 토큰 갱신 방지이전 리뷰에서 지적된 critical 이슈가 올바르게 수정되었습니다.
updateFcmToken()이 더 이상 Result를 반환하지 않고 예외를 직접 전파하므로, 원격 업데이트 실패 시setFcmToken()이 실행되지 않습니다.토큰 비교 후 조기 반환하는 로직(lines 45-48)도 불필요한 네트워크 호출을 방지하여 효율적입니다.
54-57: LGTM: 명시적 토큰 동기화도 올바르게 구현됨오버로드된
syncFcmToken(fcmToken: String)메서드도 동일한 패턴으로 수정되어, 원격 업데이트 실패 시 로컬 저장이 수행되지 않습니다.
59-76: LGTM: 알림 설정 관리 메서드들이 일관성 있게 구현됨알림 활성화 상태 조회/설정 및 동기화 상태 추적 메서드들이 적절하게 구현되었습니다. DataSource 위임 및
runSuspendCatching을 통한 에러 핸들링 패턴이 일관적으로 적용되었습니다.
92-92: LGTM: 헬퍼 메서드들이 올바르게 구현됨
getLocalFcmToken(): DataStore flow에서 첫 값을 가져오는 표준 패턴입니다.setFcmToken(): DataSource에 위임하는 단순 래퍼입니다.updateFcmToken(): 이전 리뷰의 핵심 수정 사항 - Result 래핑을 제거하여 예외가 자연스럽게 전파되도록 변경되었습니다. 서비스 응답(UserProfileResponse)을 무시하는 것은 토큰 동기화에서는 성공/실패 여부만 중요하므로 적절합니다.Also applies to: 94-100
이거는 기획 의도인가요 |
기획에서는 설정에서 권한을 획득하면 토글 자동 ON까지만 이긴 합니다 |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationPresenter.kt (1)
34-59: 토글 연타 시 경쟁 조건 여전 — 최신 요청만 유지 + 취소 시 롤백 금지 필요이전 리뷰의 핵심 우려(연속 토글에서 응답 역전)가 부분적으로 해소됐지만, 동시(in‑flight) 요청이 남아있어 취소/역전 시점에 오래된 요청의 실패 콜백이 ‘이전 상태(prev)’로 다시 덮을 수 있습니다. 최신 요청만 유효하게 하고, 취소(CancellationException)인 경우에는 롤백/토스트를 건너뛰어야 합니다. 또한 동일값(no‑op)일 때는 네트워크를 생략하세요.
아래와 같이 최소 수정 제안드립니다(핵심만 포함).
- 함수 외부(동일 스코프) 보조 상태 추가:
// 외부 보완 코드 import kotlinx.coroutines.Job import kotlinx.coroutines.CancellationException var updateJob by rememberRetained { mutableStateOf<Job?>(null) }
- 해당 블록 내 변경(diff):
fun updateNotificationSettings(enabled: Boolean) { - scope.launch { + // 최신 요청만 유지 + updateJob?.cancel() + updateJob = scope.launch { val prevNotificationEnabled = userRepository.getUserNotificationEnabled() + // 동일값이면 조기 종료 + if (prevNotificationEnabled == enabled) return@launch userRepository.setUserNotificationEnabled(enabled) userRepository.updateNotificationSettings(enabled) .onSuccess { userRepository.setLastNotificationSyncedEnabled(enabled) } .onFailure { exception -> + // 취소는 실패로 간주하지 않음(롤백/토스트 생략) + if (exception is CancellationException) return@onFailure val handleErrorMessage = { message: String -> Logger.e(message) sideEffect = NotificationSideEffect.ShowToast(message) } handleException( exception = exception, onError = handleErrorMessage, onLoginRequired = { navigator.resetRoot(LoginScreen()) }, ) userRepository.setUserNotificationEnabled(prevNotificationEnabled) } - } + } }
🧹 Nitpick comments (3)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationPresenter.kt (3)
44-47: 에러 처리 람다 중복 최소화동일한 토스트/로그 처리 람다가 두 곳에 반복됩니다. 로컬 헬퍼로 추출해 중복을 제거하면 가독성↑ 유지보수성↑ 입니다.
예:
fun notifyError(message: String) { Logger.e(message) sideEffect = NotificationSideEffect.ShowToast(message) }Also applies to: 67-70
61-80: lastSynced는 서버 응답 기준으로 갱신 권장현재는 요청 파라미터
enabled를 그대로 반영합니다. 서버가 정규화/보정한 값(예: 서버 정책으로 강제 OFF)을 반환할 수 있으므로, 응답 모델의 값을 신뢰하는 편이 안전합니다.- .onSuccess { - userRepository.setLastNotificationSyncedEnabled(enabled) - } + .onSuccess { profile -> + userRepository.setLastNotificationSyncedEnabled(profile.notificationEnabled) + }
107-109: 토글 연타 UX: 요청 중 토글 잠금/상태 표시 고려in‑flight가 있을 때 토글을 잠깐 비활성화하거나 로딩 인디케이터를 보여주면 오작동 체감이 줄어듭니다. 앞서 제안한
updateJob존재 여부로 UI 제어 가능합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationPresenter.kt(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#46
File: feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/component/InfiniteLazyColumn.kt:83-95
Timestamp: 2025-07-14T00:46:03.843Z
Learning: seoyoon513과 팀은 한국어 주석을 선호하며, 한국어 주석을 영어로 번역하라는 제안을 하지 않아야 함
🧬 Code graph analysis (1)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationPresenter.kt (3)
core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/HandleException.kt (1)
handleException(19-46)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultUserRepository.kt (1)
updateNotificationSettings(74-76)core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/UserRepository.kt (1)
updateNotificationSettings(31-31)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci-build
🔇 Additional comments (5)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationPresenter.kt (5)
5-16: 필요 import 추가 적절상태/보존/로그/네비게이션 도입 맥락에 맞는 import들이며 문제 없습니다.
31-33: 상태 소스 전환 및 sideEffect 보존 처리 좋습니다
isUserNotificationEnabled로의 전환과rememberRetained사용이 기획 의도(사용자 의도 독립 관리)에 부합합니다.
92-104: 권한 OFF 시 ‘자동 서버 OFF 동기화’가 기획 의도인지 확인 필요설명에 따르면 “권한 재허용 시 자동 ON 동기화”만 보장된다고 들었습니다. 현재 로직은 권한이 거부되면
lastSynced != false일 때 서버를 자동으로 OFF로 동기화합니다. 설정 화면에서도 자동 OFF가 의도라면 OK, 아니라면 아래처럼 “자동 ON만” 수행하도록 단순화 가능합니다.예시(의도: 권한 재허용 시에만 자동 ON):
- val shouldSync = (!isPermissionGranted && lastSyncedServerEnabled != false) || - (userEnabled && (lastSyncedServerEnabled == null || lastSyncedServerEnabled != isPermissionGranted)) - if (shouldSync) { - syncNotificationSettings(isPermissionGranted) - } + if (isPermissionGranted && userEnabled && lastSyncedServerEnabled != true) { + syncNotificationSettings(true) + }
114-114: sideEffect 전달 경로 OK
InitSideEffect로 소거하고 단발성 토스트를 전달하는 패턴 괜찮습니다.
34-59: 모든 심볼 및 suspend 문맥 검증 완료됨검증 결과:
✅ UserRepository 인터페이스 모든 메서드 존재 확인
getUserNotificationEnabled(),setUserNotificationEnabled(),updateNotificationSettings(),setLastNotificationSyncedEnabled(),getLastSyncedNotificationEnabled()모두 정의됨✅ suspend 문맥 유효성 확인
updateNotificationSettings함수 본체가scope.launch { }코루틴 블록 내에서 실행되므로 모든 suspend 함수 호출이 유효함syncNotificationSettings는 suspend 함수로 선언되어 있어 suspend 호출이 모두 유효함✅ Result.onFailure 패턴 유효성 확인
updateNotificationSettings는runSuspendCatching으로 감싸진Result<UserProfileModel>을 반환.onSuccess및.onFailure체이닝이 코드베이스 전반에 걸쳐 일관되게 사용됨코드가 검증되었으며 문제가 없습니다.
| class NotificationChannelInitializer : Initializer<Unit> { | ||
|
|
||
| override fun create(context: Context) { | ||
| createNotificationChannel(context) |
|
저는 기존에 권한 허용 여부나, 로컬에 저장된 토큰이 있는지에 상관없이 앱 시작시 스플래시에서 firebase 찔러서 token 받아오고, API를 통해 받아온 토큰을 서버에 전달하는 식으로 구현하곤 했었습니다. 알림 권한 허용 여부랑 서버에 FCM 토큰을 등록하는건 별개라고 생각하기도 하고(권한 허용 여부와 직접적인 관련이 있는 건 토글 API) https://velog.io/@mraz3068/FCM-Token-renewal-cases 현 방식도 괜찮은 것 같습니다. |
현재는 fcm 전송 api와 알림 수신 여부 api가 분리되어 있어서 회원 정보가 식별된 시점(회원가입 성공, 자동로그인 이후)에 fcm 토큰을 전달하고 있어요. (말씀해주신 대로 수신 여부와 관계 없이 전달) 다만 불필요한 api 호출을 줄이기 위해, 로컬에 저장된 fcm 토큰과 새로 발급받은 토큰을 비교한 뒤 서버 업데이트 여부를 결정하는 로직은 여러 케이스들을 고려해야 할 것 같아요. (로그아웃, 회원탈퇴에 대한 대응 필요) 알람 수신 여부는 아래와 같이 진행하고 있습니다
=> 물론 실제 권한 획득 여부 함께 체크해서 업데이트 합니다..! |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/di/FirebaseModule.kt (1)
32-34: LGTM! 이전 제안이 잘 반영되었습니다.
Firebase.messaging을 사용한 구현이 간결하고 코틀린스럽습니다. Singleton 스코프와 Provides 어노테이션도 적절하게 적용되었습니다.core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultUserRepository.kt (1)
76-83: LGTM! 이전 제안이 반영되어 코드가 간결해졌습니다.
firebaseMessaging.token.await()를 사용하여 이전 리뷰에서 제안된 방식으로 구현되었습니다. 예외 처리도 적절하게 되어 있습니다.
🧹 Nitpick comments (1)
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt (1)
241-247: 코드 중복: 공통 유틸리티로 추출 권장
checkSystemNotificationEnabled함수가NotificationUi.kt에도 동일하게 존재합니다. 이러한 공통 권한 체크 로직은 core 모듈의 유틸리티로 추출하여 재사용하는 것이 유지보수성과 일관성 측면에서 유리합니다.예를 들어
core/common또는core/ui모듈에 다음과 같이 추출할 수 있습니다:// core/common/src/main/kotlin/com/ninecraft/booket/core/common/util/NotificationPermissionUtil.kt fun Context.isNotificationPermissionGranted(): Boolean { return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { ContextCompat.checkSelfPermission(this, Manifest.permission.POST_NOTIFICATIONS) == PackageManager.PERMISSION_GRANTED } else { NotificationManagerCompat.from(this).areNotificationsEnabled() } }그 후 두 파일에서 이 확장 함수를 사용:
- val isGranted = checkSystemNotificationEnabled(context) + val isGranted = context.isNotificationPermissionGranted()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
app/src/main/kotlin/com/ninecraft/booket/ReedFirebaseMessagingService.kt(1 hunks)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/di/FirebaseModule.kt(2 hunks)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/mapper/ResponseToModel.kt(1 hunks)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultUserRepository.kt(2 hunks)core/network/src/main/kotlin/com/ninecraft/booket/core/network/service/ReedService.kt(2 hunks)feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomePresenter.kt(5 hunks)feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt(4 hunks)feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginPresenter.kt(1 hunks)gradle/libs.versions.toml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/mapper/ResponseToModel.kt
- feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomePresenter.kt
- feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginPresenter.kt
- core/network/src/main/kotlin/com/ninecraft/booket/core/network/service/ReedService.kt
- app/src/main/kotlin/com/ninecraft/booket/ReedFirebaseMessagingService.kt
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#46
File: feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/component/InfiniteLazyColumn.kt:83-95
Timestamp: 2025-07-14T00:46:03.843Z
Learning: seoyoon513과 팀은 한국어 주석을 선호하며, 한국어 주석을 영어로 번역하라는 제안을 하지 않아야 함
📚 Learning: 2025-10-17T08:03:00.309Z
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#192
File: feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt:74-76
Timestamp: 2025-10-17T08:03:00.309Z
Learning: Android 13 이상에서는 POST_NOTIFICATIONS 런타임 권한과 시스템 알림 설정(areNotificationsEnabled)이 완전히 동기화되어 있음. 하나의 토글로 둘 다 함께 변경되므로, checkSelfPermission() 체크만으로 충분함. Android 12 이하에서는 런타임 권한이 없으므로 areNotificationsEnabled()만 사용해야 함.
Applied to files:
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt
📚 Learning: 2025-10-28T05:53:09.370Z
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#204
File: app/src/main/kotlin/com/ninecraft/booket/ReedFirebaseMessagingService.kt:80-91
Timestamp: 2025-10-28T05:53:09.370Z
Learning: Reed-Android 프로젝트의 minSdk는 28이므로, API 26+ (Android O) 이상에서만 사용 가능한 API (예: NotificationChannel)에 대한 런타임 SDK 버전 체크를 제안하지 않아야 함
Applied to files:
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt
📚 Learning: 2025-08-28T12:25:54.058Z
Learnt from: easyhooon
PR: YAPP-Github/Reed-Android#174
File: feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt:128-133
Timestamp: 2025-08-28T12:25:54.058Z
Learning: In BookSearchPresenter.kt, when a guest user tries to register a book and is redirected to login, the bottom sheet (isBookRegisterBottomSheetVisible) and selection state (selectedBookIsbn, selectedBookStatus) are intentionally kept open/preserved so that when the user returns from login, they can continue from where they left off without losing context.
Applied to files:
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt
🧬 Code graph analysis (2)
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt (1)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationUi.kt (1)
checkSystemNotificationEnabled(172-178)
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultUserRepository.kt (4)
core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/RunSuspendCatching.kt (1)
runSuspendCatching(16-30)core/network/src/main/kotlin/com/ninecraft/booket/core/network/service/ReedService.kt (1)
updateFcmToken(56-57)core/datastore/api/src/main/kotlin/com/ninecraft/booket/core/datastore/api/datasource/NotificationDataSource.kt (1)
setFcmToken(7-7)core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/datasource/DefaultNotificationDataSource.kt (1)
setFcmToken(24-28)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci-build
🔇 Additional comments (7)
gradle/libs.versions.toml (1)
153-153: Firebase Messaging 라이브러리 추가 승인firebase-messaging 라이브러리가 올바르게 추가되었습니다. BOM(firebase-bom) 버전 관리 패턴을 따르고 있으며, 다른 Firebase 라이브러리들과 일관되게 구성되어 있습니다. FCM 토큰 동기화 및 푸시 알림 기능 구현을 위한 기반이 잘 마련되었습니다.
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt (1)
70-83: 권한 체크 로직 정확하게 구현됨Android 13+ 및 12 이하에서의 알림 권한 처리가 올바르게 구현되었습니다:
- Android 13+: 권한이 허용된 경우 즉시 동기화하고, 거부된 경우 런타임 권한 요청
- Android 12 이하: 시스템 알림 설정 상태를 직접 확인하여 동기화
과거 리뷰에서 지적된 Android 12 이하 비허용 케이스도 line 81에서 적절히 처리되고 있습니다.
Based on learnings
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultUserRepository.kt (5)
39-50: 토큰 비교 로직으로 불필요한 서버 호출을 효과적으로 방지합니다.원격 토큰과 로컬 토큰을 비교하여 변경이 있을 때만 서버에 업데이트하는 로직이 PR 목표에 맞게 잘 구현되었습니다. 이전 리뷰에서 지적된 네트워크 실패 시 로컬 갱신 문제도 해결되었습니다(
updateFcmToken이 실패하면setFcmToken이 실행되지 않음).
52-55: 명시적 토큰 동기화 로직이 적절합니다.
onNewToken()콜백에서 사용하기에 적합한 구조이며, 이전 리뷰에서 지적된 실패 전파 문제도 해결되었습니다.
57-70: 알림 설정 상태 관리 API가 명확하게 구성되었습니다.사용자의 명시적 의도(
userNotificationEnabled)와 마지막 동기화 상태(lastSyncedNotificationEnabled)를 분리하여 관리하는 구조가 PR에서 언급한 Opt-out 전략을 잘 지원합니다.
72-74: 서버 알림 설정 업데이트 API가 적절하게 구현되었습니다.서버 업데이트 로직이 간결하며, 로컬 상태 관리는 호출하는 쪽에서 처리하도록 설계된 것이 적절합니다.
91-93: FCM 토큰 서버 업데이트가 예외 전파를 올바르게 처리합니다.이전 리뷰에서 지적된 문제가 해결되어, 이제 서버 업데이트 실패 시 예외가 적절하게 전파되어 로컬 토큰이 잘못 갱신되는 것을 방지합니다.
| initialValue = checkSystemNotificationEnabled(context), | ||
| key1 = lifecycleOwner, | ||
| ) { | ||
| // 포그라운드 복귀 시 OS 권한 동기화 |
There was a problem hiding this comment.
맞다 그리고 이부분 lifecycleResumeEffect 사용해서 개발자가 observer를 제거하지 않는 등의 휴면이슈를 해결할 수 있을 것 같아요. Composable 함수 내부라서 사용가능할 것으로 보입니다~(좀 더 Compose스러운 코드)
lifecycleResumeEffect {
val value = checkSystemNotificationEnabled(context)
state.eventSink(NotificationUiEvent.OnNotificationPermissionResult(value))
onPauseOrDispose {
// 필요시 정리 작업(내부적으로 사용되는 observer는 해당 컴포저블 파괴되면 자동으로 제거)
}
}There was a problem hiding this comment.
엣 ProduceState 내부군요(코루틴 스코프) 취소...
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultAuthRepository.kt (1)
37-40: 회원탈퇴 시 알림 데이터 정리 추가 필요검증 결과,
logout()에서는clearNotificationDataStore()를 호출하여 FCM 토큰, 사용자 알림 설정 등 모든 알림 관련 데이터를 정리하지만,withdraw()에서는 이를 호출하지 않습니다. 회원탈퇴는 계정 영구 삭제이므로 로그아웃과 동일하게 알림 데이터도 정리되어야 합니다.
withdraw()메서드에서clearTokens()다음에clearNotificationDataStore()를 추가하세요:override suspend fun withdraw() = runSuspendCatching { service.withdraw() clearTokens() clearNotificationDataStore() }
🧹 Nitpick comments (1)
core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/datasource/DefaultNotificationDataSource.kt (1)
18-28: FCM 토큰 저장/조회 구현이 적절하나, 빈 문자열 저장 시 처리 확인 필요FCM 토큰의 Flow와 setter가 올바르게 구현되었습니다. 다만
setFcmToken에서 빈 문자열이나 유효하지 않은 토큰에 대한 검증이 없는데, 상위 레이어(Repository)에서 검증이 이루어지는지 확인이 필요합니다.빈 토큰이 저장될 경우 서버 동기화 로직에서 불필요한 API 호출이 발생할 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultAuthRepository.kt(4 hunks)core/datastore/api/src/main/kotlin/com/ninecraft/booket/core/datastore/api/datasource/NotificationDataSource.kt(1 hunks)core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/datasource/DefaultNotificationDataSource.kt(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#46
File: feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/component/InfiniteLazyColumn.kt:83-95
Timestamp: 2025-07-14T00:46:03.843Z
Learning: seoyoon513과 팀은 한국어 주석을 선호하며, 한국어 주석을 영어로 번역하라는 제안을 하지 않아야 함
🧬 Code graph analysis (1)
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultAuthRepository.kt (2)
core/datastore/api/src/main/kotlin/com/ninecraft/booket/core/datastore/api/datasource/NotificationDataSource.kt (1)
clearNotificationDataStore(15-15)core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/datasource/DefaultNotificationDataSource.kt (1)
clearNotificationDataStore(54-58)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci-build
🔇 Additional comments (7)
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultAuthRepository.kt (2)
5-5: NotificationDataSource 의존성 추가가 적절합니다.로그아웃 시 알림 관련 데이터를 정리하기 위한 의존성 주입이 올바르게 구현되었습니다.
Also applies to: 19-19
68-70: 로그아웃 시 알림 데이터 정리 로직이 적절합니다.로그아웃 시 알림 관련 DataStore를 정리하여 다음 로그인 시 깨끗한 상태로 시작할 수 있도록 합니다.
core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/datasource/DefaultNotificationDataSource.kt (4)
7-7: FCM 토큰 저장을 위한 String 타입 키 추가가 적절합니다.기존의 단순 boolean flag에서 실제 FCM 토큰을 저장하는 구조로 변경되어 토큰 비교를 통한 불필요한 서버 호출 방지가 가능해졌습니다.
Also applies to: 61-61
30-40: Opt-out 전략에 따른 기본값 true 설정이 적절합니다.PR 목표에 명시된 대로
USER_NOTIFICATION_ENABLED의 기본값을true로 설정하여 opt-out 전략을 구현했습니다. 이는 사용자가 명시적으로 거부하지 않는 한 알림을 활성화하겠다는 의도를 반영합니다.시스템 권한(
isPermissionGranted)과는 별도로 관리되므로, 실제 알림 발송 여부는 권한과 이 플래그를 모두 고려해야 합니다.
42-52: nullable 타입을 통한 첫 동기화 감지 설계가 우수합니다.
lastSyncedNotificationEnabled를Flow<Boolean?>로 선언하여 아직 한 번도 동기화되지 않은 상태(null)와 동기화된 상태(true/false)를 명확히 구분할 수 있습니다. 이를 통해 불필요한 서버 호출을 효과적으로 방지할 수 있습니다.
54-58: DataStore 전체 정리 구현이 적절합니다.
prefs.clear()를 통해 알림 관련 모든 데이터를 한 번에 정리하는 간결한 구현입니다. 로그아웃 시 깨끗한 상태로 초기화됩니다.core/datastore/api/src/main/kotlin/com/ninecraft/booket/core/datastore/api/datasource/NotificationDataSource.kt (1)
6-15: 토큰 기반 알림 관리 인터페이스로의 전환이 우수합니다.기존의 단순한
isNotificationEnabledboolean flag에서 다음과 같이 세분화된 인터페이스로 개선되었습니다:
fcmToken: 실제 FCM 토큰 저장 및 비교를 통한 불필요한 서버 호출 방지isUserNotificationEnabled: 사용자의 명시적 알림 설정 의도 관리lastSyncedNotificationEnabled: 마지막 서버 동기화 상태 추적 (nullable로 첫 동기화 감지)clearNotificationDataStore(): 로그아웃 시 데이터 정리이 설계는 PR 목표에 명시된 opt-out 전략과 효율적인 서버 동기화 요구사항을 잘 반영하고 있습니다.
easyhooon
left a comment
easyhooon
left a comment
There was a problem hiding this comment.
문제 없어보입니다~ 엣지 케이스 잡느라 수고하셧습니다~
| ) | ||
|
|
||
| val builder = NotificationCompat.Builder(this, REED_CHANNEL_ID) | ||
| .setSmallIcon(R.drawable.ic_notification) |
There was a problem hiding this comment.
해당 아이콘으로 기존 런처 아이콘이 아닌 notification 전용 벡터 아이콘으로 만들어서 넣어줬는데요, 기존에 런처 아이콘과 동일한 아이콘을 사용할 경우(mipmap/ic_launcher) 기기 OS 별로 다르게 노출되기 때문입니다.
[에뮬레이터 Pixel 기종]
[갤럭시 S21]
따라서 statusBar 등에 노출되는 알림용 아이콘의 통일성을 위해, 흰색 단색 로고 'r'만 남긴 알림 전용 아이콘 (ic_notification)으로 적용했습니다.
[에뮬레이터 Pixel 기종]
[갤럭시 S21]
|
@easyhooon 화면 크기 대응이 안되어있었네요;; 요 작업까지 해서 머지하겠습니다~! 
|
캡쳐사진만 보면 나름 잘되어있는거같긴합니다. |
캡쳐 사진은 대응 후 사진입니다...^^;;(머쓱) 대응 안한건 잊어주세요 |
2 participants
🔗 관련 이슈
📙 작업 설명
✅ FCM 토큰 발급 및 동기화
FirebaseMessagingService.onNewToken()콜백을 통해 토큰 만료나 기기 변경 등으로 새 토큰이 발급될 경우 자동으로 서버 업데이트 수행✅ 알림 설정 동기화
Opt-out 전략 기반으로 “사용자의 수신 의도”(userNotificationEnabled) 와 “시스템의 권한 상태”(isPermissionGranted) 를 각각 독립적으로 관리
[홈 화면]
lastSyncedNotificationEnabled값과 비교하여 변경이 감지된 경우에만 서버 동기화 요청(Opt-out 전략을 위한 의도된 불일치, 시스템 권한 허용 시 자동 ON 하기 위함)
[알림 설정 화면]
userNotificationEnabled업데이트 및 서버 동기화 진행시스템 권한 허용/해제 변화를 감지하고 필요 시 자동으로 서버 동기화를 트리거
🧪 테스트 내역
📸 스크린샷 또는 시연 영상
✅ 최초 진입 시 알람 권한 거부한 경우
시스템 환경 설정에서 권한 허용 후 앱 진입시 토글 On으로 자동 동기화
Reed_Opt-out.mp4
✅ 사용자가 알림 토글을 직접 Off(명시적 거절) 한 경우
“명시적 거절” 이후에는 시스템 권한을 재허용하더라도 토글 Off 상태 반영
Reed_Opt-out.mp4
✅ 토글 조작 시 서버 알람 동기화 실패한 경우
Toast 메시지로 실패 안내 후 토글 상태를 원래대로 롤백
Reed_.mp4
💬 추가 설명 or 리뷰 포인트
Summary by CodeRabbit
새로운 기능
Chores