-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Add app-specific NotificationIconResourceProvider for K-9 & Thunderbird push notifications #9579
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
base: main
Are you sure you want to change the base?
Add app-specific NotificationIconResourceProvider for K-9 & Thunderbird push notifications #9579
Conversation
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.
Hi @SttApollo, thank you for your contribution! Before we can merge your changes, there are a few items that need to be addressed:
- Ensure that your code passes the CI checks.
- Please rebase your commit messages according to our Git Commit Guide.
- Review and consider addressing all the comments made in this PR.
Additionally, regarding notification debugging, we now have a :feature:debug-settings
module. This module was introduced to test the new notification system and should include the notification debugging functionality you introduced there.
(click to expand) Here is an example patch of what would be needed to introduce your notification debugging functionality there:
Subject: [PATCH] chore(notifications): add legacy notification test
---
Index: app-k9mail/src/debug/kotlin/app/k9mail/DebugNotificationActivity.kt
===================================================================
diff --git a/app-k9mail/src/debug/kotlin/app/k9mail/DebugNotificationActivity.kt b/app-k9mail/src/debug/kotlin/app/k9mail/DebugNotificationActivity.kt
deleted file mode 100644
--- a/app-k9mail/src/debug/kotlin/app/k9mail/DebugNotificationActivity.kt (revision a036ca941adae1356fda8beb8d1fc80048578354)
+++ /dev/null (revision a036ca941adae1356fda8beb8d1fc80048578354)
@@ -1,60 +0,0 @@
-package app.k9mail
-
-import android.app.Activity
-import android.os.Bundle
-import android.util.Log
-import android.widget.Button
-import android.widget.LinearLayout
-import android.widget.TextView
-import android.widget.Toast
-import app.k9mail.core.android.common.provider.NotificationIconResourceProvider
-import com.fsck.k9.notification.NotificationChannelManager
-import org.koin.java.KoinJavaComponent.inject
-
-class DebugNotificationActivity : Activity() {
-
- override fun onCreate(savedInstanceState: Bundle?) {
- super.onCreate(savedInstanceState)
-
- Log.d("DebugNotifActivity", "onCreate called")
-
- val layout = LinearLayout(this).apply {
- orientation = LinearLayout.VERTICAL
- setPadding(64, 128, 64, 64)
- }
-
- val title = TextView(this).apply {
- text = "🔔 Debug Push Notification"
- textSize = 22f
- }
-
- val button = Button(this).apply {
- text = "Trigger Notification"
- setOnClickListener {
- try {
- val iconProvider: Lazy<NotificationIconResourceProvider> =
- inject(NotificationIconResourceProvider::class.java)
- val channelManager: Lazy<NotificationChannelManager> =
- inject(NotificationChannelManager::class.java)
-
- NotificationTester.showTestPushNotification(
- context = this@DebugNotificationActivity,
- iconProvider = iconProvider,
- channelManager = channelManager,
- )
- Toast.makeText(this@DebugNotificationActivity, "Notification sent!", Toast.LENGTH_SHORT).show()
- } catch (e: Exception) {
- Log.e("DebugNotifActivity", "Failed to inject dependencies", e)
- Toast.makeText(this@DebugNotificationActivity, "Error: ${e.message}", Toast.LENGTH_LONG).show()
- }
- }
- }
-
- layout.addView(title)
- layout.addView(button)
-
- setContentView(layout)
-
- Log.d("DebugNotifActivity", "Activity setup complete")
- }
-}
Index: app-k9mail/src/debug/kotlin/app/k9mail/DebugNotificationReceiver.kt
===================================================================
diff --git a/app-k9mail/src/debug/kotlin/app/k9mail/DebugNotificationReceiver.kt b/app-k9mail/src/debug/kotlin/app/k9mail/DebugNotificationReceiver.kt
deleted file mode 100644
--- a/app-k9mail/src/debug/kotlin/app/k9mail/DebugNotificationReceiver.kt (revision a036ca941adae1356fda8beb8d1fc80048578354)
+++ /dev/null (revision a036ca941adae1356fda8beb8d1fc80048578354)
@@ -1,25 +0,0 @@
-package app.k9mail
-
-import android.content.BroadcastReceiver
-import android.content.Context
-import android.content.Intent
-import android.util.Log
-import app.k9mail.core.android.common.provider.NotificationIconResourceProvider
-import com.fsck.k9.notification.NotificationChannelManager
-import org.koin.java.KoinJavaComponent.inject
-
-class DebugNotificationReceiver : BroadcastReceiver() {
- private val iconProvider: Lazy<NotificationIconResourceProvider> =
- inject(NotificationIconResourceProvider::class.java)
- private val channelManager: Lazy<NotificationChannelManager> =
- inject(NotificationChannelManager::class.java)
-
- override fun onReceive(context: Context, intent: Intent) {
- Log.d("DebugNotifReceiver", "Received debug notification broadcast")
- NotificationTester.showTestPushNotification(
- context,
- iconProvider,
- channelManager,
- )
- }
-}
Index: app-k9mail/src/debug/kotlin/app/k9mail/NotificationTester.kt
===================================================================
diff --git a/app-k9mail/src/debug/kotlin/app/k9mail/NotificationTester.kt b/app-k9mail/src/debug/kotlin/app/k9mail/NotificationTester.kt
deleted file mode 100644
--- a/app-k9mail/src/debug/kotlin/app/k9mail/NotificationTester.kt (revision a036ca941adae1356fda8beb8d1fc80048578354)
+++ /dev/null (revision a036ca941adae1356fda8beb8d1fc80048578354)
@@ -1,26 +0,0 @@
-package app.k9mail
-
-import android.content.Context
-import androidx.core.app.NotificationCompat
-import androidx.core.app.NotificationManagerCompat
-import app.k9mail.core.android.common.provider.NotificationIconResourceProvider
-import com.fsck.k9.notification.NotificationChannelManager
-
-object NotificationTester {
- fun showTestPushNotification(
- context: Context,
- iconProvider: Lazy<NotificationIconResourceProvider>,
- channelManager: Lazy<NotificationChannelManager>,
- ) {
- val n = NotificationCompat.Builder(context, channelManager.value.pushChannelId)
- .setSmallIcon(iconProvider.value.pushNotificationIcon)
- .setContentTitle("🔔 Debug Push")
- .setContentText("This is a test using the app icon")
- .setPriority(NotificationCompat.PRIORITY_HIGH)
- .build()
-
- @Suppress("MissingPermission")
- NotificationManagerCompat.from(context)
- .notify(0xDEAD, n)
- }
-}
Index: feature/debug-settings/src/main/kotlin/net/thunderbird/feature/debug/settings/notification/legacy/DebugLegacyNotificationSection.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/feature/debug-settings/src/main/kotlin/net/thunderbird/feature/debug/settings/notification/legacy/DebugLegacyNotificationSection.kt b/feature/debug-settings/src/main/kotlin/net/thunderbird/feature/debug/settings/notification/legacy/DebugLegacyNotificationSection.kt
new file mode 100644
--- /dev/null (date 1754486141789)
+++ b/feature/debug-settings/src/main/kotlin/net/thunderbird/feature/debug/settings/notification/legacy/DebugLegacyNotificationSection.kt (date 1754486141789)
@@ -0,0 +1,58 @@
+package net.thunderbird.feature.debug.settings.notification.legacy
+
+import androidx.compose.foundation.layout.Arrangement
+import androidx.compose.foundation.layout.Column
+import androidx.compose.foundation.layout.fillMaxWidth
+import androidx.compose.runtime.Composable
+import androidx.compose.ui.Alignment
+import androidx.compose.ui.Modifier
+import androidx.compose.ui.platform.LocalContext
+import androidx.compose.ui.res.stringResource
+import app.k9mail.core.ui.compose.common.mvi.observeWithoutEffect
+import app.k9mail.core.ui.compose.designsystem.atom.button.ButtonFilled
+import app.k9mail.core.ui.compose.designsystem.atom.text.TextTitleSmall
+import app.k9mail.core.ui.compose.theme2.MainTheme
+import net.thunderbird.feature.debug.settings.DebugSection
+import net.thunderbird.feature.debug.settings.R
+import org.koin.androidx.compose.koinViewModel
+
+@Composable
+fun DebugLegacyNotificationSection(
+ modifier: Modifier = Modifier,
+ viewModel: DebugLegacyNotificationSectionViewModel = koinViewModel(),
+) {
+ val (_, dispatch) = viewModel.observeWithoutEffect()
+ val context = LocalContext.current
+
+ DebugLegacyNotificationSection(
+ onTriggerNotificationClick = {
+ dispatch(DebugLegacyNotificationSectionContract.Event.TriggerNotification(context))
+ },
+ modifier = modifier,
+ )
+}
+
+@Composable
+private fun DebugLegacyNotificationSection(
+ onTriggerNotificationClick: () -> Unit,
+ modifier: Modifier = Modifier,
+) {
+ DebugSection(
+ title = stringResource(R.string.debug_settings_legacy_notifications_title),
+ modifier = modifier,
+ ) {
+ Column(
+ modifier = Modifier.fillMaxWidth(),
+ verticalArrangement = Arrangement.spacedBy(MainTheme.spacings.default),
+ ) {
+ TextTitleSmall(
+ text = stringResource(R.string.debug_settings_legacy_notifications_debug_push),
+ )
+ ButtonFilled(
+ text = stringResource(R.string.debug_settings_notifications_trigger_notification),
+ onClick = onTriggerNotificationClick,
+ modifier = Modifier.align(Alignment.CenterHorizontally),
+ )
+ }
+ }
+}
Index: feature/debug-settings/src/main/kotlin/net/thunderbird/feature/debug/settings/notification/legacy/DebugLegacyNotificationSectionContract.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/feature/debug-settings/src/main/kotlin/net/thunderbird/feature/debug/settings/notification/legacy/DebugLegacyNotificationSectionContract.kt b/feature/debug-settings/src/main/kotlin/net/thunderbird/feature/debug/settings/notification/legacy/DebugLegacyNotificationSectionContract.kt
new file mode 100644
--- /dev/null (date 1754485211877)
+++ b/feature/debug-settings/src/main/kotlin/net/thunderbird/feature/debug/settings/notification/legacy/DebugLegacyNotificationSectionContract.kt (date 1754485211877)
@@ -0,0 +1,16 @@
+package net.thunderbird.feature.debug.settings.notification.legacy
+
+import android.content.Context
+import app.k9mail.core.ui.compose.common.mvi.UnidirectionalViewModel
+
+interface DebugLegacyNotificationSectionContract {
+ interface ViewModel : UnidirectionalViewModel<State, Event, Effect>
+
+ data object State
+
+ sealed interface Event {
+ data class TriggerNotification(val context: Context) : Event
+ }
+
+ sealed interface Effect
+}
Index: feature/debug-settings/src/main/res/values/strings.xml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/feature/debug-settings/src/main/res/values/strings.xml b/feature/debug-settings/src/main/res/values/strings.xml
--- a/feature/debug-settings/src/main/res/values/strings.xml (revision a036ca941adae1356fda8beb8d1fc80048578354)
+++ b/feature/debug-settings/src/main/res/values/strings.xml (date 1754485211880)
@@ -15,4 +15,7 @@
<string name="debug_settings_notification_status_log">Notification status log</string>
<string name="debug_settings_notifications_status">"Status: "</string>
<string name="debug_settings_notifications_clear_status_log">Clear status log</string>
+ <!-- Legacy notification system -->
+ <string name="debug_settings_legacy_notifications_title">Legacy Notifications</string>
+ <string name="debug_settings_legacy_notifications_debug_push">🔔 Debug Push Notification</string>
</resources>
Index: feature/debug-settings/src/main/AndroidManifest.xml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/feature/debug-settings/src/main/AndroidManifest.xml b/feature/debug-settings/src/main/AndroidManifest.xml
new file mode 100644
--- /dev/null (date 1754485765103)
+++ b/feature/debug-settings/src/main/AndroidManifest.xml (date 1754485765103)
@@ -0,0 +1,7 @@
+<manifest
+ xmlns:android="http://schemas.android.com/apk/res/android">
+
+ <!-- Required for legacy notification tests. Must be removed once migration to the new system is done -->
+ <uses-permission android:name="android.permission.POST_NOTIFICATIONS" />
+
+</manifest>
Index: feature/debug-settings/src/main/kotlin/net/thunderbird/feature/debug/settings/notification/legacy/DebugLegacyNotificationSectionViewModel.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/feature/debug-settings/src/main/kotlin/net/thunderbird/feature/debug/settings/notification/legacy/DebugLegacyNotificationSectionViewModel.kt b/feature/debug-settings/src/main/kotlin/net/thunderbird/feature/debug/settings/notification/legacy/DebugLegacyNotificationSectionViewModel.kt
new file mode 100644
--- /dev/null (date 1754485765101)
+++ b/feature/debug-settings/src/main/kotlin/net/thunderbird/feature/debug/settings/notification/legacy/DebugLegacyNotificationSectionViewModel.kt (date 1754485765101)
@@ -0,0 +1,34 @@
+package net.thunderbird.feature.debug.settings.notification.legacy
+
+import android.content.Context
+import androidx.core.app.NotificationCompat
+import androidx.core.app.NotificationManagerCompat
+import app.k9mail.core.android.common.provider.NotificationIconResourceProvider
+import app.k9mail.core.ui.compose.common.mvi.BaseViewModel
+import com.fsck.k9.notification.NotificationChannelManager
+import net.thunderbird.feature.debug.settings.notification.legacy.DebugLegacyNotificationSectionContract.Effect
+import net.thunderbird.feature.debug.settings.notification.legacy.DebugLegacyNotificationSectionContract.Event
+import net.thunderbird.feature.debug.settings.notification.legacy.DebugLegacyNotificationSectionContract.State
+import net.thunderbird.feature.debug.settings.notification.legacy.DebugLegacyNotificationSectionContract.ViewModel
+
+class DebugLegacyNotificationSectionViewModel(
+ private val iconProvider: NotificationIconResourceProvider,
+ private val channelManager: NotificationChannelManager,
+) : BaseViewModel<State, Event, Effect>(initialState = State), ViewModel {
+ override fun event(event: Event) {
+ when (event) {
+ is Event.TriggerNotification -> onTriggerNotification(event.context)
+ }
+ }
+
+ private fun onTriggerNotification(context: Context) {
+ val notification = NotificationCompat.Builder(context, channelManager.pushChannelId)
+ .setSmallIcon(iconProvider.pushNotificationIcon)
+ .setContentTitle("🔔 Debug Push")
+ .setContentText("This is a test using the app icon")
+ .setPriority(NotificationCompat.PRIORITY_HIGH)
+ .build()
+
+ NotificationManagerCompat.from(context).notify(0xDEAD, notification)
+ }
+}
Index: feature/debug-settings/src/debug/kotlin/net/thunderbird/feature/debug/settings/inject/FeatureDebugSettingsModule.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/feature/debug-settings/src/debug/kotlin/net/thunderbird/feature/debug/settings/inject/FeatureDebugSettingsModule.kt b/feature/debug-settings/src/debug/kotlin/net/thunderbird/feature/debug/settings/inject/FeatureDebugSettingsModule.kt
--- a/feature/debug-settings/src/debug/kotlin/net/thunderbird/feature/debug/settings/inject/FeatureDebugSettingsModule.kt (revision a036ca941adae1356fda8beb8d1fc80048578354)
+++ b/feature/debug-settings/src/debug/kotlin/net/thunderbird/feature/debug/settings/inject/FeatureDebugSettingsModule.kt (date 1754485915733)
@@ -3,7 +3,10 @@
import net.thunderbird.feature.debug.settings.navigation.DefaultSecretDebugSettingsNavigation
import net.thunderbird.feature.debug.settings.navigation.SecretDebugSettingsNavigation
import net.thunderbird.feature.debug.settings.notification.DebugNotificationSectionViewModel
+import net.thunderbird.feature.debug.settings.notification.legacy.DebugLegacyNotificationSectionContract
+import net.thunderbird.feature.debug.settings.notification.legacy.DebugLegacyNotificationSectionViewModel
import org.koin.core.module.dsl.viewModel
+import org.koin.dsl.bind
import org.koin.dsl.module
val featureDebugSettingsModule = module {
@@ -16,4 +19,11 @@
notificationReceiver = get(),
)
}
+
+ viewModel {
+ DebugLegacyNotificationSectionViewModel(
+ iconProvider = get(),
+ channelManager = get(),
+ )
+ }
}
Index: feature/debug-settings/build.gradle.kts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/feature/debug-settings/build.gradle.kts b/feature/debug-settings/build.gradle.kts
--- a/feature/debug-settings/build.gradle.kts (revision a036ca941adae1356fda8beb8d1fc80048578354)
+++ b/feature/debug-settings/build.gradle.kts (date 1754485384586)
@@ -16,4 +16,9 @@
implementation(projects.core.outcome)
implementation(projects.feature.mail.account.api)
implementation(projects.feature.notification.api)
+
+ // legacy.core and core.android.common are required for legacy notification tests.
+ // Must be removed once migration to the new system is done
+ implementation(projects.core.android.common)
+ implementation(projects.legacy.core)
}
Index: feature/debug-settings/src/main/kotlin/net/thunderbird/feature/debug/settings/SecretDebugSettingsScreen.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/feature/debug-settings/src/main/kotlin/net/thunderbird/feature/debug/settings/SecretDebugSettingsScreen.kt b/feature/debug-settings/src/main/kotlin/net/thunderbird/feature/debug/settings/SecretDebugSettingsScreen.kt
--- a/feature/debug-settings/src/main/kotlin/net/thunderbird/feature/debug/settings/SecretDebugSettingsScreen.kt (revision a036ca941adae1356fda8beb8d1fc80048578354)
+++ b/feature/debug-settings/src/main/kotlin/net/thunderbird/feature/debug/settings/SecretDebugSettingsScreen.kt (date 1754486141794)
@@ -1,6 +1,8 @@
package net.thunderbird.feature.debug.settings
+import androidx.compose.foundation.layout.Arrangement
import androidx.compose.foundation.layout.Column
+import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.rememberScrollState
import androidx.compose.foundation.verticalScroll
@@ -13,6 +15,7 @@
import app.k9mail.core.ui.compose.designsystem.template.Scaffold
import app.k9mail.core.ui.compose.theme2.MainTheme
import net.thunderbird.feature.debug.settings.notification.DebugNotificationSection
+import net.thunderbird.feature.debug.settings.notification.legacy.DebugLegacyNotificationSection
@Composable
fun SecretDebugSettingsScreen(
@@ -38,8 +41,10 @@
.padding(paddingValues)
.verticalScroll(rememberScrollState())
.padding(MainTheme.spacings.double),
+ verticalArrangement = Arrangement.spacedBy(MainTheme.spacings.double),
) {
DebugNotificationSection()
+ DebugLegacyNotificationSection()
}
}
}
Please let me know if you have any questions
app-k9mail/src/main/kotlin/app/k9mail/provider/K9AppNotificationIconProvider.kt
Outdated
Show resolved
Hide resolved
legacy/core/src/main/java/com/fsck/k9/notification/NotificationChannelManager.kt
Outdated
Show resolved
Hide resolved
legacy/ui/legacy/src/test/java/com/fsck/k9/TestCoreResourceProvider.kt
Outdated
Show resolved
Hide resolved
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.
We now have a debug-settings
module where we use to test notifications. The notification debugging should be done there.
Additionally, there is no need to add a new activity or a broadcast receiver to debug the notification. Please consider a different approach.
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.
Hi! Thanks for pointing me to the debug-settings module. I’ve removed the manual DebugNotificationActivity + receiver and added a new PushIconTestNotification entry to the System-notification dropdown in the Secret Debug Settings screen to verify NotificationIconResourceProvider for each app without touching existing notification code.
The test works in K-9 Mail. Thunderbird still shows an unrelated DI issue, so I kept that outside the scope of this PR.
Please let me know if this works or if you'd like any further adjustments!
Screen_recording_20250808_211847.webm
a036ca9
to
daaef4d
Compare
Hi @rafaeltonholo - thank you for the speedy and thorough review! 🙏🏽
Feel free to share additional thoughts, and I'll make the updates |
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.
Hi @SttApollo,
I apologize for the delay in reviewing this pull request again; I haven't had much time to go through PRs recently.
Thank you for addressing the previous comments and for using the new notification system to verify the icon notification. However, before we can merge this pull request, we need to make a few adjustments:
- We need to ensure that we only include changes that are necessary to address the problem from the specified GitHub Issue. There are several files with unrelated changes. I have left comments on those files; please revert the changes made to them.
- You are updating the app's icon in the manifest by using a drawable instead of the mipmap version. This change has caused the badging to fail. You will need to revert both the icon change in the manifest and the badging change.
- There are some files with code style issues that may cause the build to fail through Detekt. Please ensure you run
./gradlew spotlessCheck
and./gradlew detekt
to fix all reported issues before pushing the code.
I have tested the changes, and for both K-9 and Thunderbird, the icon is updated correctly for the "Waiting for new emails" notification. We just need to address the issues mentioned above before proceeding.
Thank you!
25b4400
to
c6c1978
Compare
Introduces NotificationIconResourceProvider to allow different app flavors to supply their own push notification icon. This change includes: - The NotificationIconResourceProvider interface. - Implementations for K-9 Mail and Thunderbird. - A new PushIconTestNotification in the debug settings to verify the provider. - Removal of the old manual DebugNotificationActivity. Fixes thunderbird#8932
c6c1978
to
0893cdc
Compare
Uh oh!
There was an error while loading. Please reload this page.