Fix demo alert for inbound email (#2081)

# What this PR does
Fix HTTP 500 when sending demo alert for inbound integration email. 

- Make all integration configs have consistent `is_demo_alert_enabled`
and `example_payload` values
- Add tests

## Checklist

- [x] Unit, integration, and e2e (if applicable) tests updated
- [x] Documentation added (or `pr:no public docs` PR label added if not
required)
- [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not
required)

CHANGELOG.md

@@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## Unreleased
+
+### Fixed
+
+- Fix demo alert for inbound email integration by @vadimkerr ([#2081](https://github.com/grafana/oncall/pull/2081))
+
 ## v1.2.35 (2023-06-01)
 
 ### Fixed

engine/apps/alerts/models/alert_receive_channel.py

@@ -519,42 +519,35 @@ class AlertReceiveChannel(IntegrationOptionsMixin, MaintainableObject):
     # Demo alerts
     def send_demo_alert(self, force_route_id=None, payload=None):
         logger.info(f"send_demo_alert integration={self.pk} force_route_id={force_route_id}")
+
+        if not self.is_demo_alert_enabled:
+            raise UnableToSendDemoAlert("Unable to send demo alert for this integration.")
+
         if payload is None:
             payload = self.config.example_payload
-        if self.is_demo_alert_enabled:
-            if self.has_alertmanager_payload_structure:
-                if (alerts := payload.get("alerts", None)) and type(alerts) == list and len(alerts):
-                    for alert in alerts:
-                        create_alertmanager_alerts.apply_async(
-                            [],
-                            {
-                                "alert_receive_channel_pk": self.pk,
-                                "alert": alert,
-                                "is_demo": True,
-                                "force_route_id": force_route_id,
-                            },
-                        )
-                else:
-                    raise UnableToSendDemoAlert(
-                        "Unable to send demo alert as payload has no 'alerts' key, it is not array, or it is empty."
-                    )
-            else:
-                create_alert.apply_async(
-                    [],
-                    {
-                        "title": "Demo alert",
-                        "message": "Demo alert",
-                        "image_url": None,
-                        "link_to_upstream_details": None,
-                        "alert_receive_channel_pk": self.pk,
-                        "integration_unique_data": None,
-                        "raw_request_data": payload,
-                        "is_demo": True,
-                        "force_route_id": force_route_id,
-                    },
+
+        if self.has_alertmanager_payload_structure:
+            alerts = payload.get("alerts", None)
+            if not isinstance(alerts, list) or not len(alerts):
+                raise UnableToSendDemoAlert(
+                    "Unable to send demo alert as payload has no 'alerts' key, it is not array, or it is empty."
+                )
+            for alert in alerts:
+                create_alertmanager_alerts.delay(
+                    alert_receive_channel_pk=self.pk, alert=alert, is_demo=True, force_route_id=force_route_id
                 )
         else:
-            raise UnableToSendDemoAlert("Unable to send demo alert for this integration")
+            create_alert.delay(
+                title="Demo alert",
+                message="Demo alert",
+                image_url=None,
+                link_to_upstream_details=None,
+                alert_receive_channel_pk=self.pk,
+                integration_unique_data=None,
+                raw_request_data=payload,
+                is_demo=True,
+                force_route_id=force_route_id,
+            )
 
     @property
     def has_alertmanager_payload_structure(self):

engine/apps/alerts/tests/test_alert_receiver_channel.py

@@ -6,6 +6,7 @@ from django.urls import reverse
 
 from apps.alerts.models import AlertReceiveChannel
 from common.api_helpers.utils import create_engine_url
+from common.exceptions import UnableToSendDemoAlert
 
 
 @pytest.mark.django_db
@@ -145,6 +146,21 @@ def test_send_demo_alert_alertmanager_payload_shape(
     assert mocked_create_alert.call_args.args[1]["force_route_id"] is None
 
 
+@mock.patch("apps.integrations.tasks.create_alert.apply_async", return_value=None)
+@pytest.mark.parametrize(
+    "integration", [config.slug for config in AlertReceiveChannel._config if not config.is_demo_alert_enabled]
+)
+@pytest.mark.django_db
+def test_send_demo_alert_not_enabled(mocked_create_alert, make_organization, make_alert_receive_channel, integration):
+    organization = make_organization()
+    alert_receive_channel = make_alert_receive_channel(organization, integration=integration)
+
+    with pytest.raises(UnableToSendDemoAlert):
+        alert_receive_channel.send_demo_alert()
+
+    assert not mocked_create_alert.called
+
+
 @pytest.mark.django_db
 def test_notify_maintenance_no_general_channel(make_organization, make_alert_receive_channel):
     organization = make_organization(general_log_channel_id=None)

engine/apps/alerts/tests/test_default_templates.py

@@ -102,3 +102,26 @@ def test_default_templates_are_valid():
                     jinja_template_env.from_string(template)
                 except TemplateSyntaxError as e:
                     pytest.fail(e.message)
+
+
+@pytest.mark.parametrize("config", AlertReceiveChannel._config)
+def test_is_demo_alert_enabled(config):
+    # is_demo_alert_enabled must be defined
+    try:
+        assert isinstance(config.is_demo_alert_enabled, bool), "is_demo_alert_enabled must be bool"
+    except AttributeError:
+        pytest.fail("is_demo_alert_enabled must be defined")
+
+    # example_payload must be defined
+    try:
+        assert config.example_payload is None or isinstance(
+            config.example_payload, dict
+        ), "example_payload must be dict or None"
+    except AttributeError:
+        pytest.fail("example_payload must be defined")
+
+    # example_payload must be provided when is_demo_alert_enabled is True
+    if config.is_demo_alert_enabled:
+        assert config.example_payload, "example_payload must be defined and non-empty"
+    else:
+        assert config.example_payload is None, "example_payload must be None if is_demo_alert_enabled is False"

engine/apps/api/serializers/alert_receive_channel.py

@@ -49,7 +49,7 @@ class AlertReceiveChannelSerializer(EagerLoadingMixin, serializers.ModelSerializ
     heartbeat = serializers.SerializerMethodField()
     allow_delete = serializers.SerializerMethodField()
     description_short = serializers.CharField(max_length=250, required=False, allow_null=True)
-    demo_alert_payload = serializers.SerializerMethodField()
+    demo_alert_payload = serializers.CharField(source="config.example_payload", read_only=True)
     routes_count = serializers.SerializerMethodField()
     connected_escalations_chains_count = serializers.SerializerMethodField()
 
@@ -162,14 +162,6 @@ class AlertReceiveChannelSerializer(EagerLoadingMixin, serializers.ModelSerializ
     def get_alert_groups_count(self, obj):
         return 0
 
-    def get_demo_alert_payload(self, obj):
-        if obj.is_demo_alert_enabled:
-            try:
-                return obj.config.example_payload
-            except AttributeError:
-                return "{}"
-        return None
-
     def get_routes_count(self, obj) -> int:
         return obj.channel_filters.count()
 

engine/apps/api/tests/test_alert_receive_channel.py

@@ -772,3 +772,45 @@ def test_stop_maintenance_integration(
     assert alert_receive_channel.maintenance_uuid is None
     assert alert_receive_channel.maintenance_started_at is None
     assert alert_receive_channel.maintenance_author is None
+
+
+@pytest.mark.django_db
+def test_alert_receive_channel_send_demo_alert(
+    make_organization_and_user_with_plugin_token,
+    make_user_auth_headers,
+    make_alert_receive_channel,
+):
+    organization, user, token = make_organization_and_user_with_plugin_token()
+    alert_receive_channel = make_alert_receive_channel(
+        organization, integration=AlertReceiveChannel.INTEGRATION_GRAFANA
+    )
+    client = APIClient()
+
+    url = reverse(
+        "api-internal:alert_receive_channel-send-demo-alert",
+        kwargs={"pk": alert_receive_channel.public_primary_key},
+    )
+
+    response = client.post(url, format="json", **make_user_auth_headers(user, token))
+    assert response.status_code == status.HTTP_200_OK
+
+
+@pytest.mark.django_db
+def test_alert_receive_channel_send_demo_alert_not_enabled(
+    make_organization_and_user_with_plugin_token,
+    make_user_auth_headers,
+    make_alert_receive_channel,
+):
+    organization, user, token = make_organization_and_user_with_plugin_token()
+    alert_receive_channel = make_alert_receive_channel(
+        organization, integration=AlertReceiveChannel.INTEGRATION_DIRECT_PAGING
+    )
+    client = APIClient()
+
+    url = reverse(
+        "api-internal:alert_receive_channel-send-demo-alert",
+        kwargs={"pk": alert_receive_channel.public_primary_key},
+    )
+
+    response = client.post(url, format="json", **make_user_auth_headers(user, token))
+    assert response.status_code == status.HTTP_400_BAD_REQUEST

engine/apps/api/views/alert_receive_channel.py

@@ -172,20 +172,16 @@ class AlertReceiveChannelView(
     @action(detail=True, methods=["post"], throttle_classes=[DemoAlertThrottler])
     def send_demo_alert(self, request, pk):
         alert_receive_channel = AlertReceiveChannel.objects.get(public_primary_key=pk)
-        demo_alert_payload = request.data.get("demo_alert_payload", None)
+        payload = request.data.get("demo_alert_payload", None)
 
-        if not demo_alert_payload:
-            # If no payload provided, use the demo payload for backword compatibility
-            payload = alert_receive_channel.config.example_payload
-        else:
-            if type(demo_alert_payload) != dict:
-                raise BadRequest(detail="Payload for demo alert must be a valid json object")
-            payload = demo_alert_payload
+        if payload is not None and not isinstance(payload, dict):
+            raise BadRequest(detail="Payload for demo alert must be a valid json object")
 
         try:
             alert_receive_channel.send_demo_alert(payload=payload)
         except UnableToSendDemoAlert as e:
             raise BadRequest(detail=str(e))
+
         return Response(status=status.HTTP_200_OK)
 
     @action(detail=False, methods=["get"])

engine/config_integrations/direct_paging.py

@@ -54,3 +54,5 @@ grouping_id = """{{ payload }}"""
 resolve_condition = None
 
 acknowledge_condition = None
+
+example_payload = None

engine/config_integrations/heartbeat.py

@@ -26,4 +26,4 @@ resolve_condition = '{{ payload.get("is_resolve", False) == True }}'
 
 acknowledge_condition = None
 
-example_payload = {"foo": "bar"}
+example_payload = None

engine/config_integrations/inbound_email.py

@@ -9,7 +9,7 @@ description = None
 is_displayed_on_web = settings.FEATURE_INBOUND_EMAIL_ENABLED
 is_featured = False
 is_able_to_autoresolve = True
-is_demo_alert_enabled = False
+is_demo_alert_enabled = True
 
 
 # Default templates
@@ -46,3 +46,5 @@ grouping_id = '{{ payload.get("subject", "").upper() }}'
 resolve_condition = '{{ payload.get("message", "").upper() == "OK" }}'
 
 acknowledge_condition = None
+
+example_payload = {"subject": "Test email subject", "message": "Test email message", "sender": "sender@example.com"}

engine/config_integrations/maintenance.py

@@ -7,7 +7,7 @@ description = None
 is_displayed_on_web = False
 is_featured = False
 is_able_to_autoresolve = False
-is_demo_alert_enabled = True
+is_demo_alert_enabled = False
 
 description = None
 
@@ -45,3 +45,5 @@ grouping_id = None
 resolve_condition = None
 
 acknowledge_condition = None
+
+example_payload = None

engine/config_integrations/manual.py

@@ -54,3 +54,5 @@ grouping_id = """{{ payload }}"""
 resolve_condition = None
 
 acknowledge_condition = None
+
+example_payload = None

engine/config_integrations/slack_channel.py

@@ -40,3 +40,5 @@ resolve_condition = None
 acknowledge_condition = None
 
 source_link = '{{ payload.get("amixr_mixin", {}).get("permalink", "")}}'
+
+example_payload = None