-
Notifications
You must be signed in to change notification settings - Fork 0
[PC-1644] fcm 알림 채널 변경 및 중요도 변경 #206
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
Conversation
📝 WalkthroughWalkthroughThe notification service is updated to use a dedicated notice channel (NOTICE_CHANNEL_ID) instead of a generic background channel, with increased importance level set to HIGH and enhanced metadata including channel description and badge display configuration. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
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: 0
🧹 Nitpick comments (2)
app/src/main/java/com/puzzle/piece/notification/NotificationService.kt (2)
96-101: Remove unusedBACKGROUND_CHANNELconstant.The
BACKGROUND_CHANNELconstant is no longer used anywhere in the code. Keeping it as a comment reference adds clutter. Consider removing it entirely.♻️ Suggested cleanup
companion object { - // 기존 ID - private const val BACKGROUND_CHANNEL = "백그라운드 알림" - // 중요 알림 private const val NOTICE_CHANNEL_ID = "piece_official_notice" private const val NOTICE_CHANNEL_NAME = "공지 및 주요 알림"
103-117: Consider deleting the old notification channel for clean migration.Users upgrading from previous versions will still see the old "백그라운드 알림" channel in their notification settings. Deleting it during channel creation ensures a cleaner user experience.
♻️ Suggested fix
fun createNotificationChannel(context: Context) { val notificationManager = context.getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager + // 기존 채널 삭제 + notificationManager.deleteNotificationChannel("백그라운드 알림") + val channel = NotificationChannel( NOTICE_CHANNEL_ID, NOTICE_CHANNEL_NAME, NotificationManager.IMPORTANCE_HIGH ).apply { description = "서비스의 중요한 공지와 알림을 전달합니다." setShowBadge(true) } notificationManager.createNotificationChannel(channel) }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/java/com/puzzle/piece/notification/NotificationService.kt
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: comst19
Repo: Piece-Puzzly/Piece-Android PR: 203
File: feature/setting/src/main/java/com/puzzle/setting/graph/main/SettingScreen.kt:242-246
Timestamp: 2026-01-10T10:27:20.895Z
Learning: In the Piece-Android project's notification policy: When users re-enable system notification permission (after previously disabling it), the app automatically turns ON both app and server push notification settings, interpreting this as the user's intent to receive notifications again.
Learnt from: comst19
Repo: Piece-Puzzly/Piece-Android PR: 203
File: feature/setting/src/main/java/com/puzzle/setting/graph/main/SettingScreen.kt:242-246
Timestamp: 2026-01-10T10:27:20.895Z
Learning: In the Piece-Android project's SettingScreen.kt and AccessRightsPage.kt: The notification toggle represents the actual notification receivable state (combining system permission and app setting), and when system permission becomes granted, the app automatically enables push notifications through onPushNotificationCheckedChange() callback - this is intentional behavior per the app's notification policy.
📚 Learning: 2026-01-10T10:27:20.895Z
Learnt from: comst19
Repo: Piece-Puzzly/Piece-Android PR: 203
File: feature/setting/src/main/java/com/puzzle/setting/graph/main/SettingScreen.kt:242-246
Timestamp: 2026-01-10T10:27:20.895Z
Learning: In the Piece-Android project's SettingScreen.kt and AccessRightsPage.kt: The notification toggle represents the actual notification receivable state (combining system permission and app setting), and when system permission becomes granted, the app automatically enables push notifications through onPushNotificationCheckedChange() callback - this is intentional behavior per the app's notification policy.
Applied to files:
app/src/main/java/com/puzzle/piece/notification/NotificationService.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: build
🔇 Additional comments (1)
app/src/main/java/com/puzzle/piece/notification/NotificationService.kt (1)
75-83: LGTM!The notification builder changes are appropriate for high-priority notifications. Using
PRIORITY_HIGHandDEFAULT_ALLensures the notifications are prominent and use default sound/vibration patterns, which aligns well with the new "공지 및 주요 알림" channel purpose.
kkh725
left a comment
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.
고생하셨습니다.
…205) * [PC-1642] MyProfile 수정, 로그아웃, 탈퇴, 토큰 만료 시 로컬 MyProfile 데이터 제거 * [PC-1642] ci 빌드 실패 수정 * [PC-1642] ci 빌드 실패 수정 * [PC-1644] fcm 알림 채널 변경 및 중요도 변경 (#206) * [PC-1642] 불필요한 join 처리 제거 * [PC-1642] MyProfile 수정, 로그아웃, 탈퇴, 토큰 만료 시 로컬 MyProfile 데이터 제거 * [PC-1642] ci 빌드 실패 수정 * [PC-1642] ci 빌드 실패 수정 * [PC-1642] 불필요한 join 처리 제거 * [PC-1642] 토큰 만료 처리 및 세션 만료 로직 일원화
1. ⭐️ 변경된 내용
2. 🖼️ 스크린샷(선택)
3. 💡 알게된 부분
4. 📌 이 부분은 꼭 봐주세요!
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.