diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/moderation/notifications/NotificationTypes.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/moderation/notifications/NotificationTypes.kt index bc3d5177d6..02f06b4147 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/moderation/notifications/NotificationTypes.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/moderation/notifications/NotificationTypes.kt @@ -81,6 +81,16 @@ interface NotificationSettings { fun setEnabled(v: Boolean) + /** + * True iff the user has taken an explicit action to disable + * notifications (i.e. flipped the master switch OFF at some point). + * Used by the Settings screen to distinguish "master switch is off + * because it defaults to off on first launch" from "master switch + * is off because the user asked for it to be off". Only the former + * gets auto-enabled when the OS permission check passes. + */ + fun wasExplicitlyDisabled(): Boolean + fun setKindToggle( kind: NotifKind, v: Boolean, diff --git a/commons/src/jvmMain/kotlin/com/vitorpamplona/amethyst/commons/moderation/notifications/PreferencesNotificationSettings.kt b/commons/src/jvmMain/kotlin/com/vitorpamplona/amethyst/commons/moderation/notifications/PreferencesNotificationSettings.kt index 349fab12ea..225b85d649 100644 --- a/commons/src/jvmMain/kotlin/com/vitorpamplona/amethyst/commons/moderation/notifications/PreferencesNotificationSettings.kt +++ b/commons/src/jvmMain/kotlin/com/vitorpamplona/amethyst/commons/moderation/notifications/PreferencesNotificationSettings.kt @@ -53,8 +53,20 @@ class PreferencesNotificationSettings( override fun setEnabled(v: Boolean) { _enabled.value = v prefs.putBoolean(KEY_ENABLED, v) + // Track explicit user intent so the Settings screen can auto-enable + // on next visit for users who never touched the switch, while + // respecting users who deliberately turned it off. Only false + // → "explicit disable"; going from off to on clears the flag so + // subsequent auto-enable heuristics work normally. + if (v) { + prefs.remove(KEY_EXPLICITLY_DISABLED) + } else { + prefs.putBoolean(KEY_EXPLICITLY_DISABLED, true) + } } + override fun wasExplicitlyDisabled(): Boolean = prefs.getBoolean(KEY_EXPLICITLY_DISABLED, false) + override fun setKindToggle( kind: NotifKind, v: Boolean, @@ -92,6 +104,7 @@ class PreferencesNotificationSettings( companion object { const val NODE = "com/vitorpamplona/amethyst/notifications" private const val KEY_ENABLED = "enabled" + private const val KEY_EXPLICITLY_DISABLED = "explicitly_disabled" private const val KEY_DND_UNTIL = "dnd_until" private const val KEY_PREVIEW = "preview_in_toast" diff --git a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/moderation/notifications/PreferencesNotificationSettingsExplicitDisableTest.kt b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/moderation/notifications/PreferencesNotificationSettingsExplicitDisableTest.kt new file mode 100644 index 0000000000..4678d92b97 --- /dev/null +++ b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/moderation/notifications/PreferencesNotificationSettingsExplicitDisableTest.kt @@ -0,0 +1,86 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.commons.moderation.notifications + +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test +import java.util.UUID +import java.util.prefs.Preferences + +/** + * Pins the semantics of the new [NotificationSettings.wasExplicitlyDisabled] + * flag added to unblock the "Enable OS notifications button doesn't work on + * desktop" bug's second failure mode. + * + * The Settings screen auto-enables the master notifications switch when it + * detects that the OS permission is fine and the switch is off. That's the + * common path for users who click the "Enable OS notifications" button and + * expect it to fully take effect (button label promise). But it MUST NOT + * override users who deliberately turned notifications off. The + * [wasExplicitlyDisabled] flag is how we distinguish the two. + */ +class PreferencesNotificationSettingsExplicitDisableTest { + private fun freshNode(): Preferences { + // Use a UUID-scoped node so tests never share state and never + // pollute the real user prefs on the machine running CI/dev builds. + return Preferences.userRoot().node("amethyst-test-" + UUID.randomUUID()) + } + + @Test + fun `fresh install defaults to not-explicitly-disabled`() { + val settings = PreferencesNotificationSettings(freshNode()) + assertFalse( + "First launch must not look like a deliberate opt-out; otherwise auto-enable stays off forever", + settings.wasExplicitlyDisabled(), + ) + } + + @Test + fun `turning off marks explicitly disabled`() { + val prefs = freshNode() + val settings = PreferencesNotificationSettings(prefs) + settings.setEnabled(false) + assertTrue(settings.wasExplicitlyDisabled()) + } + + @Test + fun `turning on clears the explicit-disable flag`() { + val prefs = freshNode() + val settings = PreferencesNotificationSettings(prefs) + settings.setEnabled(false) + assertTrue(settings.wasExplicitlyDisabled()) + settings.setEnabled(true) + assertFalse( + "Toggling back on must clear the flag so subsequent OFF->auto-enable cycles work", + settings.wasExplicitlyDisabled(), + ) + } + + @Test + fun `flag persists across new instances on the same prefs node`() { + val prefs = freshNode() + PreferencesNotificationSettings(prefs).setEnabled(false) + // Second instance opens the same node \u2014 flag must survive process restart. + val reopened = PreferencesNotificationSettings(prefs) + assertTrue(reopened.wasExplicitlyDisabled()) + } +} diff --git a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/settings/NotificationSettingsScreen.kt b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/settings/NotificationSettingsScreen.kt index ef1c546c1d..161b929b68 100644 --- a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/settings/NotificationSettingsScreen.kt +++ b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/settings/NotificationSettingsScreen.kt @@ -132,6 +132,27 @@ fun NotificationSettingsScreen(onBack: (() -> Unit)? = null) { } } + // Handle the still-broken case that the previous fix missed: + // the user granted OS permission in a prior session (either via + // the older "Enable OS notifications" button whose auto-enable + // guard I initially forgot, via System Settings directly, or on + // Windows/Linux where permissionState defaults to NotApplicable). + // When they come back to Settings, permissionState == Granted so + // the "Enable OS notifications" button doesn't render, the master + // switch is still OFF from first-launch defaults, and there is no + // affordance that both tells them what's wrong and fixes it in + // one click. Auto-enable once per screen entry when we detect + // "permission is fine, but master switch is off and the user + // has never explicitly disabled it". PreferencesNotificationSettings + // exposes [wasExplicitlyDisabled] so we don't overrule a deliberate + // opt-out. + androidx.compose.runtime.LaunchedEffect(permissionState, enabled) { + val allowed = permissionState == PermissionState.Granted || permissionState == PermissionState.NotApplicable + if (allowed && !enabled && !settings.wasExplicitlyDisabled()) { + settings.setEnabled(true) + } + } + PlatformStatusCard( host = host, nativeAvailable = nativeAvailable, @@ -219,6 +240,17 @@ fun NotificationSettingsScreen(onBack: (() -> Unit)? = null) { } } PermissionState.Granted, PermissionState.NotApplicable -> { + // Turn-on button: renders only when master switch is + // off *and* the user explicitly disabled it before. + // The LaunchedEffect above auto-enables the switch + // for the common "never touched it" path; this button + // is the recovery for the deliberate-opt-out path. + if (!enabled) { + OutlinedButton( + onClick = { settings.setEnabled(true) }, + enabled = true, + ) { Text("Turn on desktop notifications") } + } OutlinedButton( onClick = { if (sendingTest) return@OutlinedButton