refactor: FCM 토큰 및 알림 설정 동기화 로직 개선#226
Merged
seoyoon513 merged 3 commits intodevelopfrom Nov 15, 2025
Merged
Conversation
Walkthrough알림 동기화 판단 로직을 분리한 유틸 함수 추가 및 FCM 토큰의 로컬 저장/비교 API 제거; 리포지토리에서 로컬 토큰 비교/저장 로직을 삭제하고 프레젠터에서 "효과적인 알림 활성화"를 계산해 새로운 유틸을 사용하도록 변경했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Presenter
participant Repo as DefaultUserRepository
participant Server
rect rgba(200,240,200,0.3)
Presenter->>Presenter: userSettingEnabled & isPermissionGranted\n=> effectiveNotificationEnabled
Presenter->>Presenter: shouldSyncNotification(effective, lastSynced)
end
alt shouldSync == true
Presenter->>Repo: syncNotificationSettings(effectiveNotificationEnabled)
Repo->>Server: register/update notification settings (always send)
Server-->>Repo: 200 OK
Repo-->>Presenter: result
else shouldSync == false
Presenter-->>Presenter: no network sync
end
(다른 변경—FCM 토큰 관련 데이터소스 제거와 리포지토리 내부 토큰 비교 삭제—은 제어 흐름을 단순화하여 "항상 서버에 동기화" 경로로 통일됨.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
시
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 (1)
💤 Files with no reviewable changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/NotificationSyncUtils.kt(1 hunks)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultUserRepository.kt(1 hunks)core/datastore/api/src/main/kotlin/com/ninecraft/booket/core/datastore/api/datasource/NotificationDataSource.kt(0 hunks)core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/datasource/DefaultNotificationDataSource.kt(0 hunks)feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomePresenter.kt(2 hunks)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationPresenter.kt(2 hunks)
💤 Files with no reviewable changes (2)
- core/datastore/api/src/main/kotlin/com/ninecraft/booket/core/datastore/api/datasource/NotificationDataSource.kt
- core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/datasource/DefaultNotificationDataSource.kt
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: seoyoon513
Repo: YAPP-Github/Reed-Android PR: 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-08-28T12:25:54.058Z
Learnt from: easyhooon
Repo: YAPP-Github/Reed-Android PR: 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/HomePresenter.ktfeature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationPresenter.kt
📚 Learning: 2025-10-17T08:03:00.309Z
Learnt from: seoyoon513
Repo: YAPP-Github/Reed-Android PR: 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/HomePresenter.ktfeature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationPresenter.kt
🧬 Code graph analysis (2)
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomePresenter.kt (2)
core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/NotificationSyncUtils.kt (1)
shouldSyncNotification(3-4)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationPresenter.kt (1)
syncNotificationSettings(62-81)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationPresenter.kt (2)
core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/NotificationSyncUtils.kt (1)
shouldSyncNotification(3-4)feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomePresenter.kt (1)
syncNotificationSettings(77-84)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ci-build
- GitHub Check: Compose Stability Check
🔇 Additional comments (4)
core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/NotificationSyncUtils.kt (1)
3-4: 간결하고 명확한 유틸리티 함수입니다.초기 동기화(
lastSynced == null)와 상태 변경(lastSynced != effectiveEnabled) 케이스를 모두 올바르게 처리하고 있으며, HomePresenter와 NotificationPresenter에서 중복되던 로직을 효과적으로 추출했습니다.feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationPresenter.kt (1)
93-107: 알림 설정 동기화 로직 개선이 잘 되었습니다.
userSettingEnabled와isPermissionGranted를 결합한effectiveNotificationEnabled값을 사용하여 실제 알림 상태를 서버에 동기화하는 방식이 직관적이고 정확합니다. 공통 유틸리티 함수를 사용하여 HomePresenter와의 일관성도 확보되었습니다.feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomePresenter.kt (1)
116-130: NotificationPresenter와 일관된 동기화 로직 개선입니다.동일한 패턴으로 리팩토링되어 코드 일관성이 확보되었으며, 공통 유틸리티 함수를 사용하여 중복 로직을 제거했습니다.
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultUserRepository.kt (1)
41-48: 동기화 호출 패턴 확인 완료 - 설계가 의도적이고 타당합니다.검증 결과,
syncFcmToken()은 다음 3곳에서 호출됩니다:
- 앱 시작 시 (SplashPresenter:56) - 사용자 프로필 로드 후 매번 호출
- 로그인 후 (LoginPresenter:93) - 로그인 성공 시 호출
- FCM 토큰 갱신 시 (ReedFirebaseMessagingService:35) - Firebase에서 새 토큰 발급 시 호출
앱 시작 시마다 네트워크 호출이 발생하는 것은 맞으나, 이는 기기 등록 불일치를 방지하기 위한 의도적인 설계입니다. 호출 패턴이 적절하고 필요한 생명주기에서만 동기화되므로 문제 없습니다.
Contributor
혹시 불일치가 발생할 엣지케이스가 어떤 경우일까요? 어떤 흐름으로 흘러갔을때 발생할수있을지 정리해보면 좋을것같군요. |
Contributor
Author
이런 경우가 흔치 않겠지만, 서버에서 deviceId–fcmToken 정보가 삭제될 경우 클라이언트에서 대응할 수 있는 방법이 없다는 점이 가장 큰 리스크인 것 같아요. 기존 구조는 클라이언트가 로컬에 저장된 값을 기준으로 동작하기 때문에 한 번 등록에 성공하면 앱 재설치나 FCM 토큰 만료 같은 이벤트가 발생하지 않는 이상 서버에 재등록을 시도하지 않아요. 그 결과 서버와 상태가 어긋나더라도 다시 맞출 계기가 없어지고, 이런 불일치는 발생 빈도가 낮아도 한 번 발생하면 복구하기 어려운 치명적인 버그라고 생각했습니다🥲 또 이번 배포처럼 충분한 엣지 케이스를 테스트하기 어려운 상황에서는 비용이 발생하더라도 매번 upsert하는 방식이 안전하다는 판단도 있었습니다. 향후 시스템이 안정화되면 다시 로컬 데이터를 기반으로 판단하거나 서버에서 내려주는 값을 참고하는 구조로 전환하는 것도 좋을 것 같아요 |
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.
🔗 관련 이슈
📙 작업 설명
🧪 테스트 내역
💬 추가 설명 or 리뷰 포인트
Summary by CodeRabbit
릴리즈 노트
개선 사항
리팩토링