From aca3c0adfe993a3d172de7bff39b01f76044b4f3 Mon Sep 17 00:00:00 2001 From: David Larsen Date: Tue, 24 Feb 2026 18:14:02 -0500 Subject: [PATCH] Fix webhook notifier not reading URL from dashboard config WebhookNotifier.__init__ read self.config.get('url') but the param resolved from notifications.yaml and app_config uses the key 'webhook_url'. This caused dashboard-configured webhook URLs to be silently ignored, logging "no webhook URL configured" at send time even though the notifier loaded successfully. Aligns the config key with the parameter name, matching the pattern used by JiraNotifier and other notifiers. Env var fallback (INPUT_WEBHOOK_URL) is unaffected. --- .../core/notification/webhook_notifier.py | 2 +- tests/test_webhook_notifier_params.py | 83 +++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 tests/test_webhook_notifier_params.py diff --git a/socket_basics/core/notification/webhook_notifier.py b/socket_basics/core/notification/webhook_notifier.py index 6180944..a5a5279 100644 --- a/socket_basics/core/notification/webhook_notifier.py +++ b/socket_basics/core/notification/webhook_notifier.py @@ -19,7 +19,7 @@ def __init__(self, params: Dict[str, Any] | None = None): super().__init__(params or {}) # Webhook URL from params, env variable, or app config self.url = ( - self.config.get('url') or + self.config.get('webhook_url') or get_webhook_url() ) diff --git a/tests/test_webhook_notifier_params.py b/tests/test_webhook_notifier_params.py new file mode 100644 index 0000000..e5cb233 --- /dev/null +++ b/tests/test_webhook_notifier_params.py @@ -0,0 +1,83 @@ +import os + +from socket_basics.core.notification.webhook_notifier import WebhookNotifier +from socket_basics.core.notification.manager import NotificationManager + + +def _base_cfg(): + return { + "notifiers": { + "webhook": { + "module_path": "socket_basics.core.notification.webhook_notifier", + "class": "WebhookNotifier", + "parameters": [ + {"name": "webhook_url", "env_variable": "INPUT_WEBHOOK_URL", "type": "str"}, + ], + } + } + } + + +def test_webhook_notifier_reads_url_from_params(): + """webhook_url param from dashboard config should populate self.url""" + n = WebhookNotifier({"webhook_url": "https://hooks.example.com/endpoint"}) + assert n.url == "https://hooks.example.com/endpoint" + + +def test_webhook_notifier_url_is_none_without_config(monkeypatch): + """Without any config or env var, url should be falsy""" + monkeypatch.delenv("WEBHOOK_URL", raising=False) + monkeypatch.delenv("INPUT_WEBHOOK_URL", raising=False) + n = WebhookNotifier({}) + assert not n.url + + +def test_webhook_notifier_falls_back_to_env_var(monkeypatch): + """INPUT_WEBHOOK_URL env var should work as fallback when params empty""" + monkeypatch.setenv("INPUT_WEBHOOK_URL", "https://env.example.com/hook") + n = WebhookNotifier({}) + assert n.url == "https://env.example.com/hook" + + +def test_webhook_notifier_params_take_precedence_over_env(monkeypatch): + """Dashboard config (params) should take precedence over env var""" + monkeypatch.setenv("INPUT_WEBHOOK_URL", "https://env.example.com/hook") + n = WebhookNotifier({"webhook_url": "https://dashboard.example.com/hook"}) + assert n.url == "https://dashboard.example.com/hook" + + +def test_webhook_enabled_via_app_config(monkeypatch): + """Webhook notifier should load when webhook_url is in app_config (dashboard)""" + monkeypatch.delenv("WEBHOOK_URL", raising=False) + monkeypatch.delenv("INPUT_WEBHOOK_URL", raising=False) + + cfg = _base_cfg() + nm = NotificationManager(cfg, app_config={"webhook_url": "https://app.example.com/hook"}) + nm.load_from_config() + + webhook = next(n for n in nm.notifiers if getattr(n, "name", "") == "webhook") + assert webhook.url == "https://app.example.com/hook" + + +def test_webhook_enabled_via_env_var(monkeypatch): + """Webhook notifier should load when INPUT_WEBHOOK_URL env var is set""" + monkeypatch.setenv("INPUT_WEBHOOK_URL", "https://env.example.com/hook") + + cfg = _base_cfg() + nm = NotificationManager(cfg, app_config={}) + nm.load_from_config() + + webhook = next(n for n in nm.notifiers if getattr(n, "name", "") == "webhook") + assert webhook.url == "https://env.example.com/hook" + + +def test_webhook_app_config_precedence_over_env(monkeypatch): + """app_config webhook_url should take precedence over env var in manager flow""" + monkeypatch.setenv("INPUT_WEBHOOK_URL", "https://env.example.com/hook") + + cfg = _base_cfg() + nm = NotificationManager(cfg, app_config={"webhook_url": "https://dashboard.example.com/hook"}) + nm.load_from_config() + + webhook = next(n for n in nm.notifiers if getattr(n, "name", "") == "webhook") + assert webhook.url == "https://dashboard.example.com/hook"