diff --git a/CHANGELOG.md b/CHANGELOG.md index 340b87e63d..2cc9fedb83 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/engine/apps/alerts/models/alert_receive_channel.py b/engine/apps/alerts/models/alert_receive_channel.py index f06a3ddc79..7c0d27915b 100644 --- a/engine/apps/alerts/models/alert_receive_channel.py +++ b/engine/apps/alerts/models/alert_receive_channel.py @@ -519,42 +519,35 @@ def heartbeat_module(self): # 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): diff --git a/engine/apps/alerts/tests/test_alert_receiver_channel.py b/engine/apps/alerts/tests/test_alert_receiver_channel.py index 12aa63db68..566b5be6a3 100644 --- a/engine/apps/alerts/tests/test_alert_receiver_channel.py +++ b/engine/apps/alerts/tests/test_alert_receiver_channel.py @@ -6,6 +6,7 @@ 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) diff --git a/engine/apps/alerts/tests/test_default_templates.py b/engine/apps/alerts/tests/test_default_templates.py index 50c1ecd6c5..a1c81d9f42 100644 --- a/engine/apps/alerts/tests/test_default_templates.py +++ b/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" diff --git a/engine/apps/api/serializers/alert_receive_channel.py b/engine/apps/api/serializers/alert_receive_channel.py index 7d070d4f13..3fdb8c0381 100644 --- a/engine/apps/api/serializers/alert_receive_channel.py +++ b/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 @@ def get_alert_count(self, obj): 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() diff --git a/engine/apps/api/tests/test_alert_receive_channel.py b/engine/apps/api/tests/test_alert_receive_channel.py index 9a48780302..3002c70b66 100644 --- a/engine/apps/api/tests/test_alert_receive_channel.py +++ b/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 diff --git a/engine/apps/api/views/alert_receive_channel.py b/engine/apps/api/views/alert_receive_channel.py index cd4e2552eb..5bb06fb613 100644 --- a/engine/apps/api/views/alert_receive_channel.py +++ b/engine/apps/api/views/alert_receive_channel.py @@ -172,20 +172,16 @@ def get_queryset(self, eager=True, ignore_filtering_by_available_teams=False): @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"]) diff --git a/engine/config_integrations/direct_paging.py b/engine/config_integrations/direct_paging.py index 8b400c8322..40d6e791e8 100644 --- a/engine/config_integrations/direct_paging.py +++ b/engine/config_integrations/direct_paging.py @@ -54,3 +54,5 @@ resolve_condition = None acknowledge_condition = None + +example_payload = None diff --git a/engine/config_integrations/heartbeat.py b/engine/config_integrations/heartbeat.py index e339b56faa..60699c4507 100644 --- a/engine/config_integrations/heartbeat.py +++ b/engine/config_integrations/heartbeat.py @@ -26,4 +26,4 @@ acknowledge_condition = None -example_payload = {"foo": "bar"} +example_payload = None diff --git a/engine/config_integrations/inbound_email.py b/engine/config_integrations/inbound_email.py index 42ecba2320..591fe68ef8 100644 --- a/engine/config_integrations/inbound_email.py +++ b/engine/config_integrations/inbound_email.py @@ -9,7 +9,7 @@ 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 @@ resolve_condition = '{{ payload.get("message", "").upper() == "OK" }}' acknowledge_condition = None + +example_payload = {"subject": "Test email subject", "message": "Test email message", "sender": "sender@example.com"} diff --git a/engine/config_integrations/maintenance.py b/engine/config_integrations/maintenance.py index 60b8710be8..af69e68c89 100644 --- a/engine/config_integrations/maintenance.py +++ b/engine/config_integrations/maintenance.py @@ -7,7 +7,7 @@ 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 @@ resolve_condition = None acknowledge_condition = None + +example_payload = None diff --git a/engine/config_integrations/manual.py b/engine/config_integrations/manual.py index 416be3e651..9f7ff0e5d1 100644 --- a/engine/config_integrations/manual.py +++ b/engine/config_integrations/manual.py @@ -54,3 +54,5 @@ resolve_condition = None acknowledge_condition = None + +example_payload = None diff --git a/engine/config_integrations/slack_channel.py b/engine/config_integrations/slack_channel.py index cd8ef14f14..05021935f1 100644 --- a/engine/config_integrations/slack_channel.py +++ b/engine/config_integrations/slack_channel.py @@ -40,3 +40,5 @@ acknowledge_condition = None source_link = '{{ payload.get("amixr_mixin", {}).get("permalink", "")}}' + +example_payload = None