Add "Open System Settings" button to denied notifications banner (WAR-3575)

app/src/terminal/view.rs

@@ -674,6 +674,47 @@ const NOTIFICATIONS_LEARN_MORE_URL: &str =
 pub const NOTIFICATIONS_TROUBLESHOOT_URL: &str =
     "https://docs.warp.dev/terminal/more-features/notifications#troubleshooting-notifications";
 
+/// Bundle identifier for the Notifications pane on macOS 13 (Ventura) and later, where System
+/// Preferences was replaced by System Settings and the pane moved to an extension bundle.
+#[cfg(target_os = "macos")]
+const MAC_SYSTEM_SETTINGS_NOTIFICATIONS_PANE_ID: &str =
+    "com.apple.Notifications-Settings.extension";
+
+/// Bundle identifier for the Notifications pane on pre-Ventura macOS (System Preferences),
+/// down through the project's minimum-supported macOS 10.14.
+#[cfg(target_os = "macos")]
+const MAC_LEGACY_SYSTEM_PREFERENCES_NOTIFICATIONS_PANE_ID: &str =
+    "com.apple.preference.notifications";
+
+/// Builds the `x-apple.systempreferences:` URL that deep-links into the Notifications pane of
+/// macOS System Settings (or System Preferences on older macOS), given whether the current OS
+/// is in the "System Settings" era (see [`warp_core::macos::is_system_settings_era`]) and the
+/// app's bundle identifier. Split out from [`mac_notification_settings_url`] so both branches can
+/// be exercised deterministically in tests regardless of which macOS version they run on.
+///
+/// When possible, the URL is scoped with Warp's own bundle identifier (via the undocumented but
+/// widely-relied-upon `?id=` query parameter) so the user lands directly on Warp's entry instead
+/// of the generic Notifications list.
+#[cfg(target_os = "macos")]
+fn mac_notification_settings_url_for(is_system_settings_era: bool, app_id: &str) -> String {
+    let pane_id = if is_system_settings_era {
+        MAC_SYSTEM_SETTINGS_NOTIFICATIONS_PANE_ID
+    } else {
+        MAC_LEGACY_SYSTEM_PREFERENCES_NOTIFICATIONS_PANE_ID
+    };
+    format!("x-apple.systempreferences:{pane_id}?id={app_id}")
+}
+
+/// Builds the `x-apple.systempreferences:` URL that deep-links into the Notifications pane of
+/// macOS System Settings for the current OS version and app.
+#[cfg(target_os = "macos")]
+fn mac_notification_settings_url() -> String {
+    mac_notification_settings_url_for(
+        warp_core::macos::is_system_settings_era(),
+        &ChannelState::app_id().to_string(),
+    )
+}
+
 const DEBOUNCE_PERIOD: Duration = Duration::from_millis(40);
 
 /// Key used in user preferences to persist the "don't show again" choice for the OSC 52
@@ -785,7 +826,7 @@ pub struct BlockNotification {
 }
 
 /// The reason for sending/discovering the notification
-#[derive(Copy, Clone, Debug, Serialize)]
+#[derive(Copy, Clone, Debug, Serialize, PartialEq, Eq)]
 pub enum NotificationsTrigger {
     LongRunningCommand(bool /* command_succeeded */, Duration),
     AgentTaskCompleted(bool /* task_succeeded */),
@@ -25312,6 +25353,10 @@ impl TerminalView {
                     }
                 });
             }
+            #[cfg(target_os = "macos")]
+            OpenSystemSettings => {
+                ctx.open_url(&mac_notification_settings_url());
+            }
         }
 
         send_telemetry_from_ctx!(TelemetryEvent::NotificationsErrorBannerAction(action), ctx);
@@ -25430,6 +25475,10 @@ impl TerminalView {
                     NotificationsDiscoveryBanner::Closed;
                 ctx.notify();
             }
+            #[cfg(target_os = "macos")]
+            OpenSystemSettings => {
+                ctx.open_url(&mac_notification_settings_url());
+            }
         }
 
         send_telemetry_from_ctx!(

app/src/terminal/view/inline_banner/notifications_discovery.rs

@@ -1,3 +1,4 @@
+use pathfinder_color::ColorU;
 use serde::Serialize;
 use warpui::Element;
 use warpui::elements::MouseStateHandle;
@@ -11,13 +12,18 @@ use crate::appearance::Appearance;
 use crate::terminal::session_settings::NotificationsMode;
 use crate::terminal::view::{InlineBannerId, NotificationsTrigger, TerminalAction};
 
-#[derive(Clone, Copy, Debug, Serialize)]
+#[derive(Clone, Copy, Debug, Serialize, PartialEq, Eq)]
 pub enum NotificationsDiscoveryBannerAction {
     LearnMore,
     Troubleshoot,
     TurnOn(NotificationsTrigger),
     Configure,
     Close,
+    /// Opens the Notifications pane of System Settings, deep-linked to Warp's own entry when
+    /// possible. Only offered once the user has denied the OS-level permissions request, since
+    /// macOS won't show the request again.
+    #[cfg(target_os = "macos")]
+    OpenSystemSettings,
 }
 
 #[derive(Default)]
@@ -27,6 +33,8 @@ pub struct NotificationsDiscoveryBannerMouseStates {
     pub turn_on: MouseStateHandle,
     pub configure: MouseStateHandle,
     pub close: MouseStateHandle,
+    #[cfg(target_os = "macos")]
+    pub open_system_settings: MouseStateHandle,
 }
 
 /// State necessary to render the (singleton) notifications discovery banner.
@@ -35,15 +43,16 @@ pub struct NotificationsDiscoveryBannerState {
     pub mouse_states: NotificationsDiscoveryBannerMouseStates,
 }
 
-pub fn render_inline_notifications_discovery_banner(
+/// Builds the title and (non-close) buttons offered by the banner for the given mode/outcome.
+/// Extracted from [`render_inline_notifications_discovery_banner`] so tests can assert on
+/// exactly which actions are offered without needing to introspect the rendered `Element` tree.
+fn notifications_discovery_banner_title_and_buttons(
     trigger: NotificationsTrigger,
     request_outcome: Option<RequestPermissionsOutcome>,
     state: &NotificationsDiscoveryBannerState,
     notifications_mode: NotificationsMode,
-    appearance: &Appearance,
-) -> Box<dyn Element> {
-    let active_ui_text_color = appearance.theme().active_ui_text_color().into_solid();
-
+    active_ui_text_color: ColorU,
+) -> (&'static str, Vec<InlineBannerTextButton>) {
     let learn_more_button = InlineBannerTextButton {
         text: "Learn more".to_string(),
         text_color: active_ui_text_color,
@@ -102,49 +111,87 @@ pub fn render_inline_notifications_discovery_banner(
         NotificationsMode::Enabled => {
             // Determine the messaging based on what the user's response was to the
             // permissions request (if any)
-            let (title, docs_button) = match request_outcome {
+            let (title, mut leading_buttons) = match request_outcome {
                 Some(request_outcome) => match request_outcome {
                     RequestPermissionsOutcome::Accepted => (
                         "Success! You are now ready to receive desktop notifications.",
-                        learn_more_button,
-                    ),
-                    RequestPermissionsOutcome::PermissionsDenied => (
-                        "Warp was denied permissions to send you notifications.",
-                        troubleshoot_button,
+                        vec![learn_more_button],
                     ),
+                    // One push below is macOS-only, so this can't be a single `vec![...]`
+                    // literal on all platforms.
+                    #[allow(clippy::vec_init_then_push)]
+                    RequestPermissionsOutcome::PermissionsDenied => {
+                        let mut buttons = vec![];
+                        // Once macOS has denied the request, it won't show the OS prompt again,
+                        // so offer a direct path to System Settings instead.
+                        #[cfg(target_os = "macos")]
+                        buttons.push(InlineBannerTextButton {
+                            text: "Open System Settings".to_string(),
+                            text_color: active_ui_text_color,
+                            button_state: InlineBannerButtonState {
+                                on_click_event: TerminalAction::NotificationsDiscoveryBanner(
+                                    NotificationsDiscoveryBannerAction::OpenSystemSettings,
+                                ),
+                                mouse_state_handle: state.mouse_states.open_system_settings.clone(),
+                            },
+                            font: Default::default(),
+                            position_id: None,
+                            variant: InlineBannerTextButtonVariant::Primary,
+                        });
+                        buttons.push(troubleshoot_button);
+                        (
+                            "Warp was denied permissions to send you notifications.",
+                            buttons,
+                        )
+                    }
                     RequestPermissionsOutcome::OtherError { .. } => (
                         "Something went wrong while requesting permissions.",
-                        troubleshoot_button,
+                        vec![troubleshoot_button],
                     ),
                 },
                 None => (
                     "Don't forget to 'Allow' the permissions request to finish setting up notifications.",
-                    learn_more_button,
+                    vec![learn_more_button],
                 ),
             };
 
-            (
-                title,
-                vec![
-                    docs_button,
-                    InlineBannerTextButton {
-                        text: "Configure notifications".to_string(),
-                        text_color: active_ui_text_color,
-                        button_state: InlineBannerButtonState {
-                            on_click_event: TerminalAction::NotificationsDiscoveryBanner(
-                                NotificationsDiscoveryBannerAction::Configure,
-                            ),
-                            mouse_state_handle: state.mouse_states.configure.clone(),
-                        },
-                        font: Default::default(),
-                        position_id: None,
-                        variant: InlineBannerTextButtonVariant::Secondary,
-                    },
-                ],
-            )
+            leading_buttons.push(InlineBannerTextButton {
+                text: "Configure notifications".to_string(),
+                text_color: active_ui_text_color,
+                button_state: InlineBannerButtonState {
+                    on_click_event: TerminalAction::NotificationsDiscoveryBanner(
+                        NotificationsDiscoveryBannerAction::Configure,
+                    ),
+                    mouse_state_handle: state.mouse_states.configure.clone(),
+                },
+                font: Default::default(),
+                position_id: None,
+                variant: InlineBannerTextButtonVariant::Secondary,
+            });
+
+            (title, leading_buttons)
         }
     };
 
+    (title, buttons)
+}
+
+pub fn render_inline_notifications_discovery_banner(
+    trigger: NotificationsTrigger,
+    request_outcome: Option<RequestPermissionsOutcome>,
+    state: &NotificationsDiscoveryBannerState,
+    notifications_mode: NotificationsMode,
+    appearance: &Appearance,
+) -> Box<dyn Element> {
+    let active_ui_text_color = appearance.theme().active_ui_text_color().into_solid();
+    let (title, buttons) = notifications_discovery_banner_title_and_buttons(
+        trigger,
+        request_outcome,
+        state,
+        notifications_mode,
+        active_ui_text_color,
+    );
+
     let close_button = InlineBannerCloseButton(InlineBannerButtonState {
         on_click_event: TerminalAction::NotificationsDiscoveryBanner(
             NotificationsDiscoveryBannerAction::Close,
@@ -163,3 +210,7 @@ pub fn render_inline_notifications_discovery_banner(
         },
     )
 }
+
+#[cfg(test)]
+#[path = "notifications_discovery_tests.rs"]
+mod tests;

app/src/terminal/view/inline_banner/notifications_discovery_tests.rs

@@ -0,0 +1,105 @@
+use pathfinder_color::ColorU;
+
+use super::*;
+
+fn state() -> NotificationsDiscoveryBannerState {
+    NotificationsDiscoveryBannerState {
+        banner_id: 0,
+        mouse_states: Default::default(),
+    }
+}
+
+fn has_action(
+    buttons: &[InlineBannerTextButton],
+    action: NotificationsDiscoveryBannerAction,
+) -> bool {
+    buttons.iter().any(|button| {
+        matches!(
+            &button.button_state.on_click_event,
+            TerminalAction::NotificationsDiscoveryBanner(a) if *a == action
+        )
+    })
+}
+
+#[cfg(target_os = "macos")]
+#[test]
+fn enabled_permissions_denied_offers_open_system_settings() {
+    let (_, buttons) = notifications_discovery_banner_title_and_buttons(
+        NotificationsTrigger::NeedsAttention,
+        Some(RequestPermissionsOutcome::PermissionsDenied),
+        &state(),
+        NotificationsMode::Enabled,
+        ColorU::white(),
+    );
+
+    assert!(has_action(
+        &buttons,
+        NotificationsDiscoveryBannerAction::OpenSystemSettings
+    ));
+    assert!(has_action(
+        &buttons,
+        NotificationsDiscoveryBannerAction::Troubleshoot
+    ));
+    assert!(has_action(
+        &buttons,
+        NotificationsDiscoveryBannerAction::Configure
+    ));
+}
+
+#[cfg(not(target_os = "macos"))]
+#[test]
+fn enabled_permissions_denied_offers_no_system_settings_cta_on_non_mac() {
+    let (_, buttons) = notifications_discovery_banner_title_and_buttons(
+        NotificationsTrigger::NeedsAttention,
+        Some(RequestPermissionsOutcome::PermissionsDenied),
+        &state(),
+        NotificationsMode::Enabled,
+        ColorU::white(),
+    );
+
+    assert!(has_action(
+        &buttons,
+        NotificationsDiscoveryBannerAction::Troubleshoot
+    ));
+    assert!(has_action(
+        &buttons,
+        NotificationsDiscoveryBannerAction::Configure
+    ));
+}
+
+#[test]
+fn enabled_accepted_does_not_offer_open_system_settings() {
+    let (_, buttons) = notifications_discovery_banner_title_and_buttons(
+        NotificationsTrigger::NeedsAttention,
+        Some(RequestPermissionsOutcome::Accepted),
+        &state(),
+        NotificationsMode::Enabled,
+        ColorU::white(),
+    );
+
+    #[cfg(target_os = "macos")]
+    assert!(!has_action(
+        &buttons,
+        NotificationsDiscoveryBannerAction::OpenSystemSettings
+    ));
+    assert!(has_action(
+        &buttons,
+        NotificationsDiscoveryBannerAction::Configure
+    ));
+}
+
+#[test]
+fn unset_offers_turn_on() {
+    let (_, buttons) = notifications_discovery_banner_title_and_buttons(
+        NotificationsTrigger::NeedsAttention,
+        None,
+        &state(),
+        NotificationsMode::Unset,
+        ColorU::white(),
+    );
+
+    assert!(has_action(
+        &buttons,
+        NotificationsDiscoveryBannerAction::TurnOn(NotificationsTrigger::NeedsAttention)
+    ));
+}