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: #3053 ## Which issue(s) this PR fixes grafana/terraform-provider-grafana#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)
grafana · Oct 3, 2023 · d604fa1 · d604fa1
1 parent d9c3d08
commit d604fa1
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 12 deletions.
diff --git 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
@@ -48,7 +48,7 @@ class Meta:
 
 
 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 Meta(ActionCreateSerializer.Meta):
             "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
@@ -354,28 +354,59 @@ 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()
 
     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
@@ -2,7 +2,6 @@
 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