From 9c3979a7123d56fe3632afe3c68ffcdeb9737268 Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Mon, 11 Sep 2023 10:50:18 -0300 Subject: [PATCH] Update escalation policies public API to handle new webhooks (#2999) Support new webhooks when creating or updating. Return existing information if still using previous custom button actions. Fixed #2998 --- CHANGELOG.md | 4 + .../escalation_policies.md | 2 +- .../serializers/escalation_policies.py | 29 ++++- .../tests/test_escalation_policies.py | 110 ++++++++++++++++++ 4 files changed, 139 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f57225a..89eca15f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Notify user via Slack/mobile push-notification when their shift swap request is taken by @joeyorlando ([#2992](https://github.com/grafana/oncall/pull/2992)) +### Fixed + +- Update escalation policies public API to handle new webhooks ([#2999](https://github.com/grafana/oncall/pull/2999)) + ## v1.3.36 (2023-09-07) ### Added diff --git a/docs/sources/oncall-api-reference/escalation_policies.md b/docs/sources/oncall-api-reference/escalation_policies.md index 2188ec0c..ce0fdd66 100644 --- a/docs/sources/oncall-api-reference/escalation_policies.md +++ b/docs/sources/oncall-api-reference/escalation_policies.md @@ -37,7 +37,7 @@ The above command returns JSON structured in the following way: | `type` | Yes | One of: `wait`, `notify_persons`, `notify_person_next_each_time`, `notify_on_call_from_schedule`, `notify_user_group`, `trigger_action`, `resolve`, `notify_whole_channel`, `notify_if_time_from_to`. | | `important` | Optional | Default is `false`. Will assign "important" to personal notification rules if `true`. This can be used to distinguish alerts on which you want to be notified immediately by phone. Applicable for types `notify_persons`, `notify_on_call_from_schedule`, and `notify_user_group`. | | `duration` | If type = `wait` | The duration, in seconds, when type `wait` is chosen. Valid values are: `60`, `300`, `900`, `1800`, `3600`. | -| `action_to_trigger` | If type = `trigger_action` | ID of an action, or webhook. | +| `action_to_trigger` | If type = `trigger_action` | ID of a webhook. | | `group_to_notify` | If type = `notify_user_group` | ID of a `User Group`. | | `persons_to_notify` | If type = `notify_persons` | List of user IDs. | | `persons_to_notify_next_each_time` | If type = `notify_person_next_each_time` | List of user IDs. | diff --git a/engine/apps/public_api/serializers/escalation_policies.py b/engine/apps/public_api/serializers/escalation_policies.py index 87e1719f..35c2b616 100644 --- a/engine/apps/public_api/serializers/escalation_policies.py +++ b/engine/apps/public_api/serializers/escalation_policies.py @@ -4,10 +4,11 @@ from datetime import timedelta from django.utils.functional import cached_property from rest_framework import fields, serializers -from apps.alerts.models import CustomButton, EscalationChain, EscalationPolicy +from apps.alerts.models import EscalationChain, EscalationPolicy from apps.schedules.models import OnCallSchedule from apps.slack.models import SlackUserGroup from apps.user_management.models import User +from apps.webhooks.models import Webhook from common.api_helpers.custom_fields import ( CustomTimeField, OrganizationFilteredPrimaryKeyRelatedField, @@ -36,6 +37,15 @@ class EscalationPolicyTypeField(fields.CharField): return step_type +class WebhookTransitionField(OrganizationFilteredPrimaryKeyRelatedField): + def get_attribute(self, instance): + value = super().get_attribute(instance) + if value is None: + # fallback to the custom button old value + value = instance.custom_button_trigger + return value + + class EscalationPolicySerializer(EagerLoadingMixin, OrderedModelSerializer): id = serializers.CharField(read_only=True, source="public_primary_key") escalation_chain_id = OrganizationFilteredPrimaryKeyRelatedField( @@ -62,10 +72,10 @@ class EscalationPolicySerializer(EagerLoadingMixin, OrderedModelSerializer): source="notify_to_group", filter_field="slack_team_identity__organizations", ) - action_to_trigger = OrganizationFilteredPrimaryKeyRelatedField( - queryset=CustomButton.objects, + action_to_trigger = WebhookTransitionField( + queryset=Webhook.objects, required=False, - source="custom_button_trigger", + source="custom_webhook", ) important = serializers.BooleanField(required=False) notify_if_time_from = CustomTimeField(required=False, source="from_time") @@ -163,7 +173,7 @@ class EscalationPolicySerializer(EagerLoadingMixin, OrderedModelSerializer): fields_to_remove.remove("persons_to_notify_next_each_time") elif step in [EscalationPolicy.STEP_NOTIFY_GROUP, EscalationPolicy.STEP_NOTIFY_GROUP_IMPORTANT]: fields_to_remove.remove("group_to_notify") - elif step == EscalationPolicy.STEP_TRIGGER_CUSTOM_BUTTON: + elif step in (EscalationPolicy.STEP_TRIGGER_CUSTOM_BUTTON, EscalationPolicy.STEP_TRIGGER_CUSTOM_WEBHOOK): fields_to_remove.remove("action_to_trigger") elif step == EscalationPolicy.STEP_NOTIFY_IF_TIME: fields_to_remove.remove("notify_if_time_from") @@ -189,6 +199,7 @@ class EscalationPolicySerializer(EagerLoadingMixin, OrderedModelSerializer): "notify_schedule", "notify_to_group", "custom_button_trigger", + "custom_webhook", "from_time", "to_time", "num_alerts_in_window", @@ -197,6 +208,10 @@ class EscalationPolicySerializer(EagerLoadingMixin, OrderedModelSerializer): step = validated_data.get("step") important = validated_data.pop("important", None) + if step == EscalationPolicy.STEP_TRIGGER_CUSTOM_BUTTON and validated_data.get("custom_webhook"): + # migrate step to webhook + step = validated_data["step"] = EscalationPolicy.STEP_TRIGGER_CUSTOM_WEBHOOK + if step in [EscalationPolicy.STEP_NOTIFY_SCHEDULE, EscalationPolicy.STEP_NOTIFY_SCHEDULE_IMPORTANT]: validated_data_fields_to_remove.remove("notify_schedule") elif step == EscalationPolicy.STEP_WAIT: @@ -211,6 +226,8 @@ class EscalationPolicySerializer(EagerLoadingMixin, OrderedModelSerializer): validated_data_fields_to_remove.remove("notify_to_group") elif step == EscalationPolicy.STEP_TRIGGER_CUSTOM_BUTTON: validated_data_fields_to_remove.remove("custom_button_trigger") + elif step == EscalationPolicy.STEP_TRIGGER_CUSTOM_WEBHOOK: + validated_data_fields_to_remove.remove("custom_webhook") elif step == EscalationPolicy.STEP_NOTIFY_IF_TIME: validated_data_fields_to_remove.remove("from_time") validated_data_fields_to_remove.remove("to_time") @@ -262,6 +279,8 @@ class EscalationPolicyUpdateSerializer(EscalationPolicySerializer): instance.notify_to_group = None if step != EscalationPolicy.STEP_TRIGGER_CUSTOM_BUTTON: instance.custom_button_trigger = None + if step != EscalationPolicy.STEP_TRIGGER_CUSTOM_WEBHOOK: + instance.custom_webhook = None if step != EscalationPolicy.STEP_NOTIFY_IF_TIME: instance.from_time = None instance.to_time = None diff --git a/engine/apps/public_api/tests/test_escalation_policies.py b/engine/apps/public_api/tests/test_escalation_policies.py index 01d05e30..1a9a19c2 100644 --- a/engine/apps/public_api/tests/test_escalation_policies.py +++ b/engine/apps/public_api/tests/test_escalation_policies.py @@ -308,3 +308,113 @@ def test_update_escalation_policy_manual_order_duplicated_position( orders = [escalation_policy.order for escalation_policy in escalation_policies] assert orders == [1, 0, 2] # Check orders are swapped when manual_order is True + + +@pytest.mark.django_db +def test_create_escalation_policy_using_webhooks( + make_organization_and_user_with_token, + make_custom_webhook, + escalation_policies_setup, +): + organization, user, token = make_organization_and_user_with_token() + webhook = make_custom_webhook(organization) + escalation_chain, _, _ = escalation_policies_setup(organization, user) + + data_for_create = { + "escalation_chain_id": escalation_chain.public_primary_key, + "type": "trigger_action", + "position": 0, + "action_to_trigger": webhook.public_primary_key, + } + + client = APIClient() + url = reverse("api-public:escalation_policies-list") + response = client.post(url, data=data_for_create, format="json", HTTP_AUTHORIZATION=token) + + assert response.status_code == status.HTTP_201_CREATED + + escalation_policy = EscalationPolicy.objects.get(public_primary_key=response.data["id"]) + serializer = EscalationPolicySerializer(escalation_policy) + assert response.data == serializer.data + + +@pytest.mark.django_db +def test_retrieve_escalation_policy_using_button( + make_organization_and_user_with_token, + make_custom_action, + escalation_policies_setup, +): + organization, user, token = make_organization_and_user_with_token() + action = make_custom_action(organization) + escalation_chain, _, _ = escalation_policies_setup(organization, user) + + escalation_policy_action = escalation_chain.escalation_policies.create( + step=EscalationPolicy.STEP_TRIGGER_CUSTOM_BUTTON, + custom_button_trigger=action, + ) + + client = APIClient() + url = reverse("api-public:escalation_policies-detail", kwargs={"pk": escalation_policy_action.public_primary_key}) + response = client.get(url, format="json", HTTP_AUTHORIZATION=token) + + assert response.status_code == status.HTTP_200_OK + + escalation_policy = EscalationPolicy.objects.get(public_primary_key=response.data["id"]) + serializer = EscalationPolicySerializer(escalation_policy) + assert response.data == serializer.data + assert response.data["action_to_trigger"] == action.public_primary_key + + +@pytest.mark.django_db +def test_update_escalation_policy_using_button_disabled( + make_organization_and_user_with_token, + make_custom_action, + escalation_policies_setup, +): + organization, user, token = make_organization_and_user_with_token() + action = make_custom_action(organization) + other_action = make_custom_action(organization) + escalation_chain, _, _ = escalation_policies_setup(organization, user) + + escalation_policy_action = escalation_chain.escalation_policies.create( + step=EscalationPolicy.STEP_TRIGGER_CUSTOM_BUTTON, + custom_button_trigger=action, + ) + + client = APIClient() + data_to_change = {"action_to_trigger": other_action.public_primary_key} + url = reverse("api-public:escalation_policies-detail", kwargs={"pk": escalation_policy_action.public_primary_key}) + response = client.put(url, data=data_to_change, format="json", HTTP_AUTHORIZATION=token) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + + +@pytest.mark.django_db +def test_update_escalation_policy_using_button_to_webhook( + make_organization_and_user_with_token, + make_custom_action, + make_custom_webhook, + escalation_policies_setup, +): + organization, user, token = make_organization_and_user_with_token() + action = make_custom_action(organization) + webhook = make_custom_webhook(organization) + escalation_chain, _, _ = escalation_policies_setup(organization, user) + + escalation_policy_action = escalation_chain.escalation_policies.create( + step=EscalationPolicy.STEP_TRIGGER_CUSTOM_BUTTON, + custom_button_trigger=action, + ) + + client = APIClient() + data_to_change = {"action_to_trigger": webhook.public_primary_key} + url = reverse("api-public:escalation_policies-detail", kwargs={"pk": escalation_policy_action.public_primary_key}) + response = client.put(url, data=data_to_change, format="json", HTTP_AUTHORIZATION=token) + + assert response.status_code == status.HTTP_200_OK + + escalation_policy = EscalationPolicy.objects.get(public_primary_key=response.data["id"]) + serializer = EscalationPolicySerializer(escalation_policy) + assert response.data == serializer.data + # step is migrated + assert escalation_policy.step == EscalationPolicy.STEP_TRIGGER_CUSTOM_WEBHOOK