From 06ad7047f3d99d2b149f81bd7411d263787f4276 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Thu, 21 Sep 2023 13:30:00 -0600 Subject: [PATCH 1/3] Fix field for remapping user to accept blank and null as it was before other webhooks updates --- engine/apps/public_api/serializers/action.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/apps/public_api/serializers/action.py b/engine/apps/public_api/serializers/action.py index 853369f8b8..e5d290eaa6 100644 --- a/engine/apps/public_api/serializers/action.py +++ b/engine/apps/public_api/serializers/action.py @@ -9,7 +9,7 @@ class ActionCreateSerializer(WebhookCreateSerializer): team_id = TeamPrimaryKeyRelatedField(allow_null=True, default=CurrentTeamDefault(), source="team") - user = serializers.CharField(required=False, source="username") + user = serializers.CharField(required=False, source="username", allow_null=True, allow_blank=True) trigger_type = WebhookTriggerTypeField(required=False) forward_whole_payload = serializers.BooleanField(required=False, source="forward_all") From 40b01c7cb3be06d444cfdfd6e1d53f7ce10627c9 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Thu, 21 Sep 2023 13:44:09 -0600 Subject: [PATCH 2/3] Update CHANGELOG, add test --- CHANGELOG.md | 4 +++ .../public_api/tests/test_custom_actions.py | 28 ++++++++++++++----- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 27ffc08391..4d186aee0b 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 - Unify breadcrumbs behaviour with other Grafana Apps and main core ([#1906](https://github.com/grafana/oncall/issues/1906)) +### Fixed + +- Fix regression in public actions endpoint handling user field by @mderynck ([#3053](https://github.com/grafana/oncall/pull/3053)) + ## v1.3.38 (2023-09-19) ### Fixed diff --git a/engine/apps/public_api/tests/test_custom_actions.py b/engine/apps/public_api/tests/test_custom_actions.py index f763b7a373..42194fa688 100644 --- a/engine/apps/public_api/tests/test_custom_actions.py +++ b/engine/apps/public_api/tests/test_custom_actions.py @@ -160,17 +160,31 @@ def test_get_custom_action( @pytest.mark.django_db -def test_create_custom_action(make_organization_and_user_with_token): +@pytest.mark.parametrize( + "data", + [ + ( + { + "name": "Test outgoing webhook", + "url": "https://example.com", + } + ), + ( + { + "name": "Test outgoing webhook", + "url": "https://example.com", + "user": None, + "password": None, + "forward_whole_payload": True, + } + ), + ], +) +def test_create_custom_action(make_organization_and_user_with_token, data): organization, user, token = make_organization_and_user_with_token() client = APIClient() url = reverse("api-public:actions-list") - - data = { - "name": "Test outgoing webhook", - "url": "https://example.com", - } - response = client.post(url, data=data, format="json", HTTP_AUTHORIZATION=f"{token}") custom_action = Webhook.objects.get(public_primary_key=response.data["id"]) From 6cca474ad6fb1971625dd65ae90d49781edf8e45 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Fri, 22 Sep 2023 11:11:18 -0600 Subject: [PATCH 3/3] Consistency pass for actions/webhooks validators --- engine/apps/public_api/serializers/action.py | 19 +++++---- .../apps/public_api/serializers/webhooks.py | 20 +++++++--- .../public_api/tests/test_custom_actions.py | 13 +++++++ engine/apps/public_api/tests/test_webhooks.py | 39 +++++++++++++++++++ 4 files changed, 78 insertions(+), 13 deletions(-) diff --git a/engine/apps/public_api/serializers/action.py b/engine/apps/public_api/serializers/action.py index e5d290eaa6..d9af4ff922 100644 --- a/engine/apps/public_api/serializers/action.py +++ b/engine/apps/public_api/serializers/action.py @@ -23,7 +23,6 @@ class Meta: "team_id", "user", "data", - "user", "password", "authorization_header", "trigger_template", @@ -37,6 +36,12 @@ class Meta: extra_kwargs = { "name": {"required": True, "allow_null": False, "allow_blank": False}, "url": {"required": True, "allow_null": False, "allow_blank": False}, + "data": {"required": False, "allow_null": True, "allow_blank": True}, + "password": {"required": False, "allow_null": True, "allow_blank": True}, + "authorization_header": {"required": False, "allow_null": True, "allow_blank": True}, + "trigger_template": {"required": False, "allow_null": True, "allow_blank": True}, + "headers": {"required": False, "allow_null": True, "allow_blank": True}, + "integration_filter": {"required": False, "allow_null": True}, } validators = [UniqueTogetherValidator(queryset=Webhook.objects.all(), fields=["name", "organization"])] @@ -51,13 +56,13 @@ class Meta(ActionCreateSerializer.Meta): extra_kwargs = { "name": {"required": False, "allow_null": False, "allow_blank": False}, "is_webhook_enabled": {"required": False, "allow_null": False}, - "user": {"required": False, "allow_null": True, "allow_blank": False}, - "password": {"required": False, "allow_null": True, "allow_blank": False}, - "authorization_header": {"required": False, "allow_null": True, "allow_blank": False}, - "trigger_template": {"required": False, "allow_null": True, "allow_blank": False}, - "headers": {"required": False, "allow_null": True, "allow_blank": False}, + "user": {"required": False, "allow_null": True, "allow_blank": True}, + "password": {"required": False, "allow_null": True, "allow_blank": True}, + "authorization_header": {"required": False, "allow_null": True, "allow_blank": True}, + "trigger_template": {"required": False, "allow_null": True, "allow_blank": True}, + "headers": {"required": False, "allow_null": True, "allow_blank": True}, "url": {"required": False, "allow_null": False, "allow_blank": False}, - "data": {"required": False, "allow_null": True, "allow_blank": False}, + "data": {"required": False, "allow_null": True, "allow_blank": True}, "forward_whole_payload": {"required": False, "allow_null": False}, "http_method": {"required": False, "allow_null": False, "allow_blank": False}, "integration_filter": {"required": False, "allow_null": True}, diff --git a/engine/apps/public_api/serializers/webhooks.py b/engine/apps/public_api/serializers/webhooks.py index 44a92634f8..70f3d6a420 100644 --- a/engine/apps/public_api/serializers/webhooks.py +++ b/engine/apps/public_api/serializers/webhooks.py @@ -78,6 +78,14 @@ class Meta: "name": {"required": True, "allow_null": False, "allow_blank": False}, "url": {"required": True, "allow_null": False, "allow_blank": False}, "http_method": {"required": True, "allow_null": False, "allow_blank": False}, + "username": {"required": False, "allow_null": True, "allow_blank": True}, + "password": {"required": False, "allow_null": True, "allow_blank": True}, + "authorization_header": {"required": False, "allow_null": True, "allow_blank": True}, + "trigger_template": {"required": False, "allow_null": True, "allow_blank": True}, + "headers": {"required": False, "allow_null": True, "allow_blank": True}, + "data": {"required": False, "allow_null": True, "allow_blank": True}, + "forward_all": {"required": False, "allow_null": False}, + "integration_filter": {"required": False, "allow_null": True}, } validators = [UniqueTogetherValidator(queryset=Webhook.objects.all(), fields=["name", "organization"])] @@ -157,13 +165,13 @@ class Meta(WebhookCreateSerializer.Meta): extra_kwargs = { "name": {"required": False, "allow_null": False, "allow_blank": False}, "is_webhook_enabled": {"required": False, "allow_null": False}, - "username": {"required": False, "allow_null": True, "allow_blank": False}, - "password": {"required": False, "allow_null": True, "allow_blank": False}, - "authorization_header": {"required": False, "allow_null": True, "allow_blank": False}, - "trigger_template": {"required": False, "allow_null": True, "allow_blank": False}, - "headers": {"required": False, "allow_null": True, "allow_blank": False}, + "username": {"required": False, "allow_null": True, "allow_blank": True}, + "password": {"required": False, "allow_null": True, "allow_blank": True}, + "authorization_header": {"required": False, "allow_null": True, "allow_blank": True}, + "trigger_template": {"required": False, "allow_null": True, "allow_blank": True}, + "headers": {"required": False, "allow_null": True, "allow_blank": True}, "url": {"required": False, "allow_null": False, "allow_blank": False}, - "data": {"required": False, "allow_null": True, "allow_blank": False}, + "data": {"required": False, "allow_null": True, "allow_blank": True}, "forward_all": {"required": False, "allow_null": False}, "http_method": {"required": False, "allow_null": False, "allow_blank": False}, "integration_filter": {"required": False, "allow_null": True}, diff --git a/engine/apps/public_api/tests/test_custom_actions.py b/engine/apps/public_api/tests/test_custom_actions.py index 42194fa688..62504c38d0 100644 --- a/engine/apps/public_api/tests/test_custom_actions.py +++ b/engine/apps/public_api/tests/test_custom_actions.py @@ -175,6 +175,19 @@ def test_get_custom_action( "url": "https://example.com", "user": None, "password": None, + "data": None, + "authorization_header": None, + "forward_whole_payload": True, + } + ), + ( + { + "name": "Test outgoing webhook", + "url": "https://example.com", + "user": "", + "password": "", + "data": "", + "authorization_header": "", "forward_whole_payload": True, } ), diff --git a/engine/apps/public_api/tests/test_webhooks.py b/engine/apps/public_api/tests/test_webhooks.py index 0e6feb3fbe..efe7870f05 100644 --- a/engine/apps/public_api/tests/test_webhooks.py +++ b/engine/apps/public_api/tests/test_webhooks.py @@ -163,6 +163,45 @@ def test_create_webhook(make_organization_and_user_with_token): assert response.data == expected_result +@pytest.mark.django_db +@pytest.mark.parametrize( + "optional_value", + [ + None, + "", + ], +) +def test_create_webhook_optional_fields(make_organization_and_user_with_token, optional_value): + organization, user, token = make_organization_and_user_with_token() + client = APIClient() + + url = reverse("api-public:webhooks-list") + + data = { + "name": "Test outgoing webhook with nested data", + "url": "https://example.com", + "http_method": "POST", + "trigger_type": "acknowledge", + "data": optional_value, + "username": optional_value, + "password": optional_value, + "authorization_header": optional_value, + "trigger_template": optional_value, + "headers": optional_value, + "forward_all": True, + "is_webhook_enabled": True, + "integration_filter": optional_value, + } + + response = client.post(url, data=data, format="json", HTTP_AUTHORIZATION=f"{token}") + webhook = Webhook.objects.get(public_primary_key=response.data["id"]) + + expected_result = _get_expected_result(webhook) + + assert response.status_code == status.HTTP_201_CREATED + assert response.json() == expected_result + + @pytest.mark.django_db def test_create_webhook_nested_data(make_organization_and_user_with_token): organization, user, token = make_organization_and_user_with_token()