Browse Source

Fix: Mattermost/Slack Webhook Support

Root Cause: The generic webhook sent a JSON payload with custom field names plus timestamp and source fields. Mattermost/Slack webhooks only accept {"text": "..."}
format and reject unknown fields with HTTP 400.

Users just need to select "Slack / Mattermost" from the Payload Format dropdown when configuring their Mattermost webhook.

Closes #133
maziggy 4 months ago
parent
commit
6c5aad1b73

+ 21 - 10
backend/app/services/notification_service.py

@@ -356,22 +356,33 @@ class NotificationService:
             return False, f"HTTP {response.status_code}: {response.text[:200]}"
 
     async def _send_webhook(self, config: dict, title: str, message: str) -> tuple[bool, str]:
-        """Send notification via generic webhook (POST JSON)."""
+        """Send notification via generic webhook (POST JSON).
+
+        Supports two payload formats:
+        - generic: Custom field names with timestamp/source metadata
+        - slack: Slack/Mattermost compatible format (just {"text": "..."})
+        """
         webhook_url = config.get("webhook_url", "").strip()
         auth_header = config.get("auth_header", "").strip()
-        custom_field_title = config.get("field_title", "title").strip() or "title"
-        custom_field_message = config.get("field_message", "message").strip() or "message"
+        payload_format = config.get("payload_format", "generic").strip()
 
         if not webhook_url:
             return False, "Webhook URL is required"
 
-        # Build payload with custom field names
-        data = {
-            custom_field_title: title,
-            custom_field_message: message,
-            "timestamp": datetime.now().isoformat(),
-            "source": "Bambuddy",
-        }
+        # Build payload based on format
+        if payload_format == "slack":
+            # Slack/Mattermost format - just text field
+            data = {"text": f"*{title}*\n{message}"}
+        else:
+            # Generic format with custom field names
+            custom_field_title = config.get("field_title", "title").strip() or "title"
+            custom_field_message = config.get("field_message", "message").strip() or "message"
+            data = {
+                custom_field_title: title,
+                custom_field_message: message,
+                "timestamp": datetime.now().isoformat(),
+                "source": "Bambuddy",
+            }
 
         headers = {"Content-Type": "application/json"}
         if auth_header:

+ 32 - 0
backend/tests/unit/services/test_notification_service.py

@@ -557,6 +557,38 @@ class TestNotificationProviderTypes:
             assert success is False
             assert "Connection failed" in message or "error" in message.lower()
 
+    @pytest.mark.asyncio
+    async def test_webhook_slack_format_sends_text_only(self, service):
+        """Verify Slack/Mattermost format sends only text field."""
+        config = {
+            "webhook_url": "http://mattermost.local/hooks/abc123",
+            "payload_format": "slack",
+        }
+
+        mock_response = MagicMock()
+        mock_response.status_code = 200
+
+        mock_client = AsyncMock()
+        mock_client.post = AsyncMock(return_value=mock_response)
+
+        with patch.object(service, "_get_client", new_callable=AsyncMock) as mock_get_client:
+            mock_get_client.return_value = mock_client
+
+            success, message = await service._send_webhook(config, "Test Title", "Test Message")
+
+            assert success is True
+            mock_client.post.assert_called_once()
+
+            # Verify payload format is Slack-compatible
+            call_args = mock_client.post.call_args
+            payload = call_args.kwargs.get("json") or call_args[1].get("json")
+            assert "text" in payload
+            assert "*Test Title*" in payload["text"]
+            assert "Test Message" in payload["text"]
+            # Should NOT have generic fields
+            assert "timestamp" not in payload
+            assert "source" not in payload
+
 
 class TestNotificationVariableFallbacks:
     """Tests for notification variable fallback values."""

+ 10 - 4
frontend/src/components/AddNotificationModal.tsx

@@ -206,9 +206,13 @@ export function AddNotificationModal({ provider, onClose }: AddNotificationModal
       case 'webhook':
         return [
           { key: 'webhook_url', label: 'Webhook URL', placeholder: 'https://example.com/webhook', type: 'text', required: true },
+          { key: 'payload_format', label: 'Payload Format', type: 'select', required: false, options: [
+            { value: 'generic', label: 'Generic JSON' },
+            { value: 'slack', label: 'Slack / Mattermost' },
+          ]},
           { key: 'auth_header', label: 'Authorization', placeholder: 'Bearer token (optional)', type: 'password', required: false },
-          { key: 'field_title', label: 'Title Field Name', placeholder: 'title', type: 'text', required: false },
-          { key: 'field_message', label: 'Message Field Name', placeholder: 'message', type: 'text', required: false },
+          { key: 'field_title', label: 'Title Field Name', placeholder: 'title', type: 'text', required: false, showIf: (cfg: Record<string, string>) => cfg.payload_format !== 'slack' },
+          { key: 'field_message', label: 'Message Field Name', placeholder: 'message', type: 'text', required: false, showIf: (cfg: Record<string, string>) => cfg.payload_format !== 'slack' },
         ];
       default:
         return [];
@@ -290,12 +294,14 @@ export function AddNotificationModal({ provider, onClose }: AddNotificationModal
           {/* Provider-specific configuration */}
           <div className="space-y-3">
             <p className="text-sm text-bambu-gray">Configuration</p>
-            {configFields.map((field) => (
+            {configFields
+              .filter((field) => !('showIf' in field) || (field as { showIf?: (cfg: Record<string, string>) => boolean }).showIf?.(config) !== false)
+              .map((field) => (
               <div key={field.key}>
                 <label className="block text-sm text-bambu-gray mb-1">
                   {field.label} {field.required && '*'}
                 </label>
-                {field.type === 'select' && field.options ? (
+                {field.type === 'select' && 'options' in field && field.options ? (
                   <select
                     value={config[field.key] || field.options[0]?.value || ''}
                     onChange={(e) => {