Prevent default notification channel creation if notifications are disabled#6054
Conversation
SDK Size Comparison 📏
|
|
WalkthroughThe changes refactor notification handler initialization to prevent default notification channels from being created when SDK notifications are disabled. Default handler creation is deferred to ChatModule, where it's conditionally applied based on the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationHandler.kt (1)
72-83: Good lazy initialization to defer notification channel creation.Converting
factoryto lazy initialization ensures the notification channel is only created when notifications are actually displayed, not during handler construction. Sincefactoryaccesses the lazynotificationManager(which creates the channel), andfactoryis only used inshowNotification*anddismissChannelNotificationsmethods, this correctly defers channel creation until needed.Optional: Add explanatory comment
Consider adding a brief comment explaining the lazy initialization:
+ // Lazy initialization defers notification channel creation until first notification is shown private val factory: MessagingStyleNotificationFactory by lazy { MessagingStyleNotificationFactory(stream-chat-android-client/src/main/java/io/getstream/chat/android/client/di/ChatModule.kt (1)
128-128: Consider documenting the nullable notification handler parameter.The nullable
notificationsHandlerparameter is a key part of the fix, allowing the default handler creation to be deferred. While the internal API status makes this acceptable, a brief KDoc comment would help future maintainers understand when this parameter should be null versus provided.Optional: Add parameter documentation
* @param notificationConfig Global client configuration including API key, endpoints, timeouts, and feature flags. + * @param notificationsHandler Optional custom notification handler. If null and pushNotificationsEnabled is true, + * a default handler will be created. If pushNotificationsEnabled is false, no handler is created regardless. * @param apiModelTransformers Transformers that map between network DTOs and domain models (in/outgoing).As per coding guidelines, documenting internal APIs is optional, but this parameter's behavior is subtle enough that documentation would add value.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
CHANGELOG.mdstream-chat-android-client/src/main/java/io/getstream/chat/android/client/ChatClient.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/di/ChatModule.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationHandler.kt
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{kt,kts}
📄 CodeRabbit inference engine (AGENTS.md)
Format and apply Kotlin style with Spotless (4 spaces, no wildcard imports, licence headers)
Files:
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/di/ChatModule.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationHandler.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/ChatClient.kt
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/*.kt: Use@OptInannotations explicitly; avoid suppressions unless documented
Document public APIs with KDoc, including thread expectations and state notes
Files:
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/di/ChatModule.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationHandler.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/ChatClient.kt
🧬 Code graph analysis (1)
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationHandler.kt (1)
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/ChatNotificationHandler.kt (1)
getNotificationChannelId(74-80)
🔇 Additional comments (3)
CHANGELOG.md (1)
15-15: Changelog entry looks correct and consistentThe description and formatting match the existing Fixed entries and accurately summarize this PR’s behavior change.
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/ChatClient.kt (1)
5003-5003: LGTM! Clean refactoring of notification handler initialization.The change to pass the nullable
notificationsHandlerdirectly toChatModule(instead of defaulting it here) correctly defers the handler creation logic. This allowsChatModuleto conditionally create the default handler only whenpushNotificationsEnabledis true, preventing unnecessary notification channel creation.stream-chat-android-client/src/main/java/io/getstream/chat/android/client/di/ChatModule.kt (1)
207-225: Excellent implementation of conditional notification handler creation.The refactored
buildChatNotificationsmethod correctly implements the PR objective:
- When
pushNotificationsEnabledisfalse: ReturnsNoOpChatNotifications, avoiding any handler or channel creation- When
pushNotificationsEnabledistrue: CreatesChatNotificationsImplwith either the provided handler or a lazily-created default handlerThis ensures the notification channel is only created when notifications are actually enabled and needed.


🎯 Goal
The ChatClient automatically creates the default
notification channeleven if notifications are disabled.This PR defers the channel creation lazily to the moment when the channel is actually needed.
🛠 Implementation details
Make the initialisation of the
MessagingStyleNotificationFactorylazy, which creates thenotificationManagerpreemptively (and therefore the default channel).Additionally, defers the creation of a fallback
NotificationHandleronly ifnotificationConfig. pushNotificationsEnabled == true.🎨 UI Changes
NotificationConfig(pushNotificationsEnabled = false)New chat messageschannel in the app notification settingsSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.