From d604fa1d06801632bc3df52594773c53d6227b70 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Tue, 3 Oct 2023 08:33:05 -0600 Subject: [PATCH] Fix action/webhook API not accepting empty/null user (#3094) # What this PR does Fix update calls made by terraform when user field is empty or not present. Should have been part of: https://github.com/grafana/oncall/pull/3053 ## Which issue(s) this PR fixes https://github.com/grafana/terraform-provider-grafana/issues/1025 ## 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 | 1 + engine/apps/public_api/serializers/action.py | 3 +- .../public_api/tests/test_custom_actions.py | 45 +++++++++++++++---- engine/apps/public_api/views/action.py | 3 +- 4 files changed, 40 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c9c0308b..72a2eaac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Accept empty and null user when updating webhook via API @mderynck ([#3094](https://github.com/grafana/oncall/pull/3094))) - Fix slack notification for a shift which end is affected by a taken swap ([#3092](https://github.com/grafana/oncall/pull/3092)) ## v1.3.40 (2023-08-28) diff --git a/engine/apps/public_api/serializers/action.py b/engine/apps/public_api/serializers/action.py index d9af4ff9..90295a6b 100644 --- a/engine/apps/public_api/serializers/action.py +++ b/engine/apps/public_api/serializers/action.py @@ -48,7 +48,7 @@ class ActionCreateSerializer(WebhookCreateSerializer): class ActionUpdateSerializer(ActionCreateSerializer): - 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") @@ -63,7 +63,6 @@ class ActionUpdateSerializer(ActionCreateSerializer): "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": 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/tests/test_custom_actions.py b/engine/apps/public_api/tests/test_custom_actions.py index 62504c38..e4f3aa1f 100644 --- a/engine/apps/public_api/tests/test_custom_actions.py +++ b/engine/apps/public_api/tests/test_custom_actions.py @@ -354,9 +354,44 @@ def test_create_custom_action_valid_after_render_use_all_data(make_organization_ @pytest.mark.django_db +@pytest.mark.django_db +@pytest.mark.parametrize( + "data", + [ + ( + { + "name": "RENAMED", + "url": "https://example.com", + } + ), + ( + { + "name": "RENAMED 1", + "url": "https://example.com", + "user": None, + "password": None, + "data": None, + "authorization_header": None, + "forward_whole_payload": True, + } + ), + ( + { + "name": "RENAMED 2", + "url": "https://example.com", + "user": "", + "password": "", + "data": "", + "authorization_header": "", + "forward_whole_payload": True, + } + ), + ], +) def test_update_custom_action( make_organization_and_user_with_token, make_custom_webhook, + data, ): organization, user, token = make_organization_and_user_with_token() client = APIClient() @@ -364,18 +399,14 @@ def test_update_custom_action( custom_action = make_custom_webhook(organization=organization) url = reverse("api-public:actions-detail", kwargs={"pk": custom_action.public_primary_key}) - - data = { - "name": "RENAMED", - } - assert custom_action.name != data["name"] response = client.put(url, data=data, format="json", HTTP_AUTHORIZATION=f"{token}") + custom_action.refresh_from_db() expected_result = { "id": custom_action.public_primary_key, - "name": data["name"], + "name": custom_action.name, "team_id": None, "url": custom_action.url, "data": custom_action.data, @@ -392,8 +423,6 @@ def test_update_custom_action( } assert response.status_code == status.HTTP_200_OK - custom_action.refresh_from_db() - assert custom_action.name == expected_result["name"] assert response.data == expected_result diff --git a/engine/apps/public_api/views/action.py b/engine/apps/public_api/views/action.py index f8ff0f23..1dd81bbe 100644 --- a/engine/apps/public_api/views/action.py +++ b/engine/apps/public_api/views/action.py @@ -2,7 +2,6 @@ from django_filters import rest_framework as filters from rest_framework.permissions import IsAuthenticated from rest_framework.viewsets import ModelViewSet -from apps.api.serializers.webhook import WebhookSerializer from apps.auth_token.auth import ApiTokenAuthentication from apps.public_api.serializers.action import ActionCreateSerializer, ActionUpdateSerializer from apps.public_api.throttlers.user_throttle import UserThrottle @@ -19,7 +18,7 @@ class ActionView(RateLimitHeadersMixin, PublicPrimaryKeyMixin, UpdateSerializerM pagination_class = FiftyPageSizePaginator throttle_classes = [UserThrottle] - model = WebhookSerializer + model = Webhook serializer_class = ActionCreateSerializer update_serializer_class = ActionUpdateSerializer