[feat] #211 FCM 푸시 알림 구현#216
Merged
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughFirebase Cloud Messaging SDK 의존성을 추가하고, NotificationRepository 인터페이스와 NotificationRepositoryImpl(DataStore 로컬 저장 + 원격 PATCH API), NekiFirebaseMessagingService(토큰 저장 및 알림 표시), NekiApplication 알림 채널 초기화, MainViewModel 토큰 서버 동기화를 구현합니다. TokenRepositoryImpl의 복호화 예외 처리도 강화됩니다. ChangesFCM 푸시 알림 기능 구현
Sequence Diagram(s)sequenceDiagram
actor FCM as FCM 서버
participant NekiFirebaseMessagingService
participant NotificationRepositoryImpl
participant DataStore
participant NotificationService
participant MainViewModel
participant Server as 백엔드 서버
rect rgba(70, 130, 180, 0.5)
note over FCM, DataStore: 토큰 발급 및 로컬 저장
FCM->>NekiFirebaseMessagingService: onNewToken(token)
NekiFirebaseMessagingService->>NotificationRepositoryImpl: savePushToken(token)
NotificationRepositoryImpl->>DataStore: PUSH_TOKEN 키로 저장
end
rect rgba(60, 179, 113, 0.5)
note over MainViewModel, Server: 앱 진입 시 토큰 서버 동기화
MainViewModel->>NotificationRepositoryImpl: getPushToken()
NotificationRepositoryImpl->>DataStore: data.map().first()
DataStore-->>MainViewModel: token
MainViewModel->>NotificationService: updateNotification(deviceToken, pushAgreed)
NotificationService->>Server: PATCH /api/notifications
Server-->>NotificationService: 200 OK
end
rect rgba(210, 105, 30, 0.5)
note over FCM, NekiFirebaseMessagingService: 포그라운드 메시지 수신 및 알림 표시
FCM->>NekiFirebaseMessagingService: onMessageReceived(RemoteMessage)
NekiFirebaseMessagingService->>NekiFirebaseMessagingService: showNotification(title, body)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Ojongseok
marked this pull request as draft
June 18, 2026 14:30
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/com/neki/android/app/NekiFirebaseMessagingService.kt`:
- Around line 32-34: The code in NekiFirebaseMessagingService.kt only checks
message.notification for title and body, causing data-only FCM payloads to be
ignored and return early without displaying notifications. Modify the logic to
also check message.data as a fallback source for title and body values. Extract
the title and body from message.notification if available, otherwise retrieve
them from message.data map using appropriate key names, and only return if
neither source provides the required values. This ensures the showNotification
method is called for both notification-based and data-only message payloads in
both foreground and background scenarios.
In
`@core/data/src/main/java/com/neki/android/core/data/repository/impl/NotificationRepositoryImpl.kt`:
- Around line 25-27: The savePushToken method lacks exception handling for the
dataStore.edit operation, which can cause exceptions to propagate and prevent
proper monitoring of token save failures. Wrap the dataStore.edit call inside a
try-catch block within the savePushToken method, and in the catch block, log the
exception with appropriate context information so that token storage failures
can be captured, monitored, and debugged effectively instead of propagating
uncaught.
- Around line 32-42: The updateNotification method declares a return type of
Result<Unit>, but the notificationService.updateNotification() call returns
BasicNullableResponse<Unit> which is not being unwrapped. You need to add logic
after calling notificationService.updateNotification() to extract the response,
validate its resultCode field, and explicitly return Unit to match the declared
return type. Follow the pattern used in other Repository implementations where
you check the response code and then return Unit on success.
In
`@core/data/src/main/java/com/neki/android/core/data/repository/impl/TokenRepositoryImpl.kt`:
- Around line 40-46: The hasTokens() method has a compilation error because
dataStore.edit is a suspend function but it is being called within the getOrElse
lambda, which is a non-suspend context. Replace the runCatching and getOrElse
pattern with a try/catch block that wraps the entire logic including the calls
to CryptoManager.decrypt, ACCESS_TOKEN, REFRESH_TOKEN, and dataStore.edit. This
will allow dataStore.edit to be properly called within the suspend context. The
try block should contain the token decryption and validation logic, while the
catch block should handle exceptions by removing the tokens and returning false.
In
`@feature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/main/ArchiveMainViewModel.kt`:
- Around line 63-66: The updateNotificationToken function may return early
without registering the token with the server if the FCM token is not yet
available at the time of the EnterArchiveMainScreen intent (the check around
lines 103-106), and since there is no re-synchronization trigger within the same
session, registration will be delayed until the next screen entry. Modify the
updateNotificationToken function to implement a callback or listener mechanism
that monitors for asynchronous FCM token generation and automatically attempts
server registration once the token becomes available, rather than failing
silently if the token is not immediately present.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7c8ccbfd-893b-4327-bfe0-1ba52a543da2
📒 Files selected for processing (13)
app/build.gradle.ktsapp/src/main/AndroidManifest.xmlapp/src/main/java/com/neki/android/app/NekiApplication.ktapp/src/main/java/com/neki/android/app/NekiFirebaseMessagingService.ktcore/data-api/src/main/java/com/neki/android/core/dataapi/repository/NotificationRepository.ktcore/data/src/main/java/com/neki/android/core/data/remote/api/NotificationService.ktcore/data/src/main/java/com/neki/android/core/data/remote/model/request/UpdateNotificationRequest.ktcore/data/src/main/java/com/neki/android/core/data/repository/di/RepositoryModule.ktcore/data/src/main/java/com/neki/android/core/data/repository/impl/NotificationRepositoryImpl.ktcore/data/src/main/java/com/neki/android/core/data/repository/impl/TokenRepositoryImpl.ktfeature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/main/ArchiveMainViewModel.ktfeature/auth/impl/build.gradle.ktsgradle/libs.versions.toml
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Ojongseok
force-pushed
the
feat/#211-fcm-push-notification
branch
from
June 18, 2026 22:59
256f40a to
e3377e5
Compare
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Ojongseok
marked this pull request as ready for review
June 18, 2026 23:02
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/com/neki/android/app/NekiFirebaseMessagingService.kt`:
- Around line 22-27: The onNewToken method creates an unstructured
CoroutineScope without proper job management, causing potential coroutine leaks
and missing exception handling. Replace the CoroutineScope(Dispatchers.IO)
instantiation with a properly supervised context that combines a SupervisorJob
with Dispatchers.IO and includes a CoroutineExceptionHandler to log any
exceptions that occur during the notificationRepository.savePushToken(token)
call. Additionally, ensure the coroutine scope is properly canceled when the
service is destroyed by implementing lifecycle management (store the job/scope
as a class member and cancel it in an onDestroy or similar cleanup method).
In
`@core/data/src/main/java/com/neki/android/core/data/repository/impl/NotificationRepositoryImpl.kt`:
- Around line 25-27: The savePushToken method does not validate that the token
is not blank before saving it to the datastore, allowing empty strings to be
stored and later synced to the server as invalid deviceToken values. Add an
isBlank() check in the savePushToken function to return early if the token is
blank, preventing empty strings from being persisted. Additionally, apply the
same validation logic in the other location (around lines 32-40) where the token
is being sent to the server to ensure blank tokens are not transmitted upstream.
In
`@core/data/src/main/java/com/neki/android/core/data/repository/impl/TokenRepositoryImpl.kt`:
- Around line 51-64: The getAccessToken() and getRefreshToken() methods catch
decryption exceptions silently using getOrElse but do not log the errors, making
it difficult to debug token decryption failures in production. Modify both
methods to add error logging using Timber.e inside the getOrElse lambda to
capture the actual exception details, while still maintaining the fallback
behavior of returning an empty string on failure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 478c1c38-728b-4b6f-af85-db5fbee8b41b
📒 Files selected for processing (13)
app/build.gradle.ktsapp/src/main/AndroidManifest.xmlapp/src/main/java/com/neki/android/app/NekiApplication.ktapp/src/main/java/com/neki/android/app/NekiFirebaseMessagingService.ktcore/data-api/src/main/java/com/neki/android/core/dataapi/repository/NotificationRepository.ktcore/data/src/main/java/com/neki/android/core/data/remote/api/NotificationService.ktcore/data/src/main/java/com/neki/android/core/data/remote/model/request/UpdateNotificationRequest.ktcore/data/src/main/java/com/neki/android/core/data/repository/di/RepositoryModule.ktcore/data/src/main/java/com/neki/android/core/data/repository/impl/NotificationRepositoryImpl.ktcore/data/src/main/java/com/neki/android/core/data/repository/impl/TokenRepositoryImpl.ktfeature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/main/ArchiveMainViewModel.ktfeature/auth/impl/build.gradle.ktsgradle/libs.versions.toml
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
core/data/src/main/java/com/neki/android/core/data/repository/impl/TokenRepositoryImpl.kt (1)
44-47: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win복호화 실패 시 로깅 추가를 권장합니다.
hasTokens(),getAccessToken(),getRefreshToken()세 메서드 모두 토큰 복호화 예외 발생 시 로그를 남기지 않습니다. 토큰 복호화 실패는 키체인 손상이나 앱 업데이트 후 마이그레이션 문제를 나타낼 수 있으므로,Timber.e로 예외를 기록하면 프로덕션 환경에서의 디버깅에 도움이 됩니다.로깅 추가 예시
} catch (e: Exception) { + Timber.e(e, "Token decryption failed, clearing tokens") dataStore.edit { it.remove(ACCESS_TOKEN); it.remove(REFRESH_TOKEN) } false }runCatching { preferences[ACCESS_TOKEN]?.let { CryptoManager.decrypt(it) } ?: "" - }.getOrElse { "" } + }.getOrElse { e -> + Timber.e(e, "Access token decryption failed") + "" + }runCatching { preferences[REFRESH_TOKEN]?.let { CryptoManager.decrypt(it) } ?: "" - }.getOrElse { "" } + }.getOrElse { e -> + Timber.e(e, "Refresh token decryption failed") + "" + }Also applies to: 51-57, 59-65
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/data/src/main/java/com/neki/android/core/data/repository/impl/TokenRepositoryImpl.kt` around lines 44 - 47, The exception handlers in the hasTokens(), getAccessToken(), and getRefreshToken() methods do not log errors when token decryption fails. Add Timber.e logging with the exception details in each catch block (the one around lines 44-47 and the similar ones in lines 51-57 and 59-65) to capture token decryption failures before clearing the tokens. This will help identify keychain corruption or migration issues in production.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In
`@core/data/src/main/java/com/neki/android/core/data/repository/impl/TokenRepositoryImpl.kt`:
- Around line 44-47: The exception handlers in the hasTokens(),
getAccessToken(), and getRefreshToken() methods do not log errors when token
decryption fails. Add Timber.e logging with the exception details in each catch
block (the one around lines 44-47 and the similar ones in lines 51-57 and 59-65)
to capture token decryption failures before clearing the tokens. This will help
identify keychain corruption or migration issues in production.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: bcc6647c-4ada-44d2-bf30-313d2c811ed4
📒 Files selected for processing (1)
core/data/src/main/java/com/neki/android/core/data/repository/impl/TokenRepositoryImpl.kt
ikseong00
reviewed
Jun 19, 2026
ikseong00
left a comment
ikseong00
left a comment
Contributor
There was a problem hiding this comment.
음, 추가적으로 로그아웃/회원탈퇴 시에 토큰 관리가 어떻게 되고 있나요??
현재 테스트 해봤을 때, 로그아웃 해도 알림이 옵니다! 요부분 서버측이랑 같이 논의해볼 수 있을까요?
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/com/neki/android/app/main/MainViewModel.kt`:
- Around line 36-38: The MainIntent.EnterMainScreen is triggered only once in
the init block of MainViewModel, but if the FCM token becomes available after
init completes, there is no mechanism to retry server registration. Add a retry
or re-trigger mechanism that listens for FCM token availability and re-invokes
store.onIntent(MainIntent.EnterMainScreen) when the token becomes available
after initialization, ensuring server registration is not skipped even if the
token is obtained after the initial screen entry.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 202b9497-d7ac-45fc-9404-e179a9297b77
📒 Files selected for processing (2)
app/src/main/java/com/neki/android/app/main/MainContract.ktapp/src/main/java/com/neki/android/app/main/MainViewModel.kt
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Ojongseok
force-pushed
the
feat/#211-fcm-push-notification
branch
from
June 20, 2026 06:08
79f8d06 to
c08bc28
Compare
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Ojongseok
commented
Jun 20, 2026
ikseong00
approved these changes
Jun 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 관련 이슈
📙 작업 설명
NekiFirebaseMessagingService구현onNewToken()) 시 로컬 저장 후 푸시 토큰 서버 전송 시점을 홈(아카이빙 메인) 진입으로 통합(약관, 로그인, 스플래시 분리하지 않고 한 번만 처리하기 위함)TokenRepository복호화 실패 시 예외 처리 추가🧪 테스트 내역
📸 스크린샷 또는 시연 영상 (선택)
Summary by CodeRabbit
Release Notes