Skip to content

Comments

Prevent default notification channel creation if notifications are disabled#6054

Merged
VelikovPetar merged 3 commits intodevelopfrom
bug/AND-965_fix_default_channel_created_when_notifications_are_disabled
Dec 22, 2025
Merged

Prevent default notification channel creation if notifications are disabled#6054
VelikovPetar merged 3 commits intodevelopfrom
bug/AND-965_fix_default_channel_created_when_notifications_are_disabled

Conversation

@VelikovPetar
Copy link
Contributor

@VelikovPetar VelikovPetar commented Dec 22, 2025

🎯 Goal

The ChatClient automatically creates the default notification channel even 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 MessagingStyleNotificationFactory lazy, which creates the notificationManager preemptively (and therefore the default channel).
Additionally, defers the creation of a fallback NotificationHandler only if notificationConfig. pushNotificationsEnabled == true.

private val notificationManager: NotificationManager by lazy {
        (context.getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager).also { notificationManager ->
            if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
                notificationManager.createNotificationChannel(notificationChannel())
            }
        }
 }

// BEFORE
private val factory: MessagingStyleNotificationFactory = MessagingStyleNotificationFactory(
        context = context,
        notificationManager = notificationManager, // <-- problematic, immediately creates the manager and the default channel, even if not needed
        notificationChannelId = getNotificationChannelId(),
        userIconBuilder = userIconBuilder,
        newMessageIntent = newMessageIntent,
        notificationTextFormatter = notificationTextFormatter,
        actionsProvider = actionsProvider,
        notificationBuilderTransformer = notificationBuilderTransformer,
)
    
// AFTER
private val factory: MessagingStyleNotificationFactory by lazy {
        MessagingStyleNotificationFactory(
            context = context,
            notificationManager = notificationManager, // called only when the factory is initialised 
            notificationChannelId = getNotificationChannelId(),
            userIconBuilder = userIconBuilder,
            newMessageIntent = newMessageIntent,
            notificationTextFormatter = notificationTextFormatter,
            actionsProvider = actionsProvider,
            notificationBuilderTransformer = notificationBuilderTransformer,
        )
}

🎨 UI Changes

  1. Set NotificationConfig(pushNotificationsEnabled = false)
  2. Install app
  3. Open app settings
  4. There should be no New chat messages channel in the app notification settings

Summary by CodeRabbit

  • Bug Fixes
    • Fixed an issue where default notification channels were created even when SDK notifications are disabled.

✏️ Tip: You can customize this high-level summary in your review settings.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 22, 2025

SDK Size Comparison 📏

SDK Before After Difference Status
stream-chat-android-client 5.25 MB 5.25 MB 0.00 MB 🟢
stream-chat-android-offline 5.48 MB 5.48 MB 0.00 MB 🟢
stream-chat-android-ui-components 10.60 MB 10.60 MB 0.00 MB 🟢
stream-chat-android-compose 12.81 MB 12.81 MB 0.00 MB 🟢

@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
13.3% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@VelikovPetar VelikovPetar marked this pull request as ready for review December 22, 2025 09:48
@VelikovPetar VelikovPetar requested a review from a team as a code owner December 22, 2025 09:48
@coderabbitai
Copy link

coderabbitai bot commented Dec 22, 2025

Walkthrough

The 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 pushNotificationsEnabled flag. MessagingStyleNotificationHandler's factory property is converted to lazy initialization.

Changes

Cohort / File(s) Change Summary
Changelog
CHANGELOG.md
Added fixed entry documenting the bug fix: "Fix default notification channel created even if SDK notifications are disabled."
Notification Handler Initialization
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/ChatClient.kt
Removed default handler substitution in Builder.internalBuild(). Instead of creating a default handler via factory when none is provided, the potentially null notificationsHandler is now passed directly.
Conditional Notification Setup
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/di/ChatModule.kt
Made notificationsHandler constructor parameter nullable. Added defaultNotificationHandler() helper to create default handlers. Renamed buildNotification() to buildChatNotifications() with nullable handler parameter. Updated method to return NoOpChatNotifications when pushNotificationsEnabled is false, otherwise constructs handler with default if none provided.
Factory Initialization
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationHandler.kt
Changed private factory property from eager to lazy initialization using Kotlin's by lazy for deferred construction on first access.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Pay special attention to the null-safety handling of notificationsHandler flowing through ChatClient.Builder into ChatModule
  • Verify the pushNotificationsEnabled condition in buildChatNotifications() correctly prevents notification setup when disabled
  • Confirm lazy initialization of the factory in MessagingStyleNotificationHandler maintains expected thread-safety semantics

Poem

🐰 A handler sleeps till it's called upon,
No channels made when notifications are gone,
Lazy factories spring to life on demand,
The SDK now follows the developer's command! 🔔✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and accurately summarizes the main change: preventing default notification channel creation when notifications are disabled.
Description check ✅ Passed The description includes Goal and Implementation details sections with clear explanations and code examples. UI Changes section describes the expected behavior. However, Testing, Contributor Checklist, and Reviewer Checklist sections are not completed.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch bug/AND-965_fix_default_channel_created_when_notifications_are_disabled

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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)
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 factory to lazy initialization ensures the notification channel is only created when notifications are actually displayed, not during handler construction. Since factory accesses the lazy notificationManager (which creates the channel), and factory is only used in showNotification* and dismissChannelNotifications methods, 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 notificationsHandler parameter 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 704fc94 and 569d5aa.

📒 Files selected for processing (4)
  • CHANGELOG.md
  • stream-chat-android-client/src/main/java/io/getstream/chat/android/client/ChatClient.kt
  • stream-chat-android-client/src/main/java/io/getstream/chat/android/client/di/ChatModule.kt
  • stream-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.kt
  • stream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationHandler.kt
  • stream-chat-android-client/src/main/java/io/getstream/chat/android/client/ChatClient.kt
**/*.kt

📄 CodeRabbit inference engine (AGENTS.md)

**/*.kt: Use @OptIn annotations 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.kt
  • stream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/handler/MessagingStyleNotificationHandler.kt
  • stream-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 consistent

The 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 notificationsHandler directly to ChatModule (instead of defaulting it here) correctly defers the handler creation logic. This allows ChatModule to conditionally create the default handler only when pushNotificationsEnabled is 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 buildChatNotifications method correctly implements the PR objective:

  1. When pushNotificationsEnabled is false: Returns NoOpChatNotifications, avoiding any handler or channel creation
  2. When pushNotificationsEnabled is true: Creates ChatNotificationsImpl with either the provided handler or a lazily-created default handler

This ensures the notification channel is only created when notifications are actually enabled and needed.

@VelikovPetar VelikovPetar merged commit df309c0 into develop Dec 22, 2025
12 of 13 checks passed
@VelikovPetar VelikovPetar deleted the bug/AND-965_fix_default_channel_created_when_notifications_are_disabled branch December 22, 2025 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants