Skip to content

Commit bb094bd

Browse files
authored
ref(workflow_engine): Fix config values to be camel cased (#95014)
# Description - Updated all the tests I could find related to this to ensure cases were working as expected - Started adding a new test and found the action wasn't being correctly created in a PUT request if there's a new action. (0b74ddc - is the isolated fix, can pull out as a separate PR if that's helpful for reviewing) - The solution for the `config` feels not quite right; `"config": convert_dict_key_case(obj.config, snake_to_camel_case),` -- it works, but this only ensures the config is camel cased correctly. when using the `CamelSnakeSerializer` it would throw errors though 🤔
1 parent 8423643 commit bb094bd

File tree

6 files changed

+67
-34
lines changed

6 files changed

+67
-34
lines changed

src/sentry/workflow_engine/endpoints/serializers.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
from sentry.api.paginator import OffsetPaginator
1313
from sentry.api.serializers import Serializer, register, serialize
1414
from sentry.api.serializers.models.group import BaseGroupSerializerResponse
15+
from sentry.api.serializers.rest_framework.base import convert_dict_key_case, snake_to_camel_case
1516
from sentry.grouping.grouptype import ErrorGroupType
1617
from sentry.models.group import Group
1718
from sentry.models.options.project_option import ProjectOption
@@ -51,7 +52,7 @@ def serialize(self, obj: Action, *args, **kwargs) -> ActionSerializerResponse:
5152
"type": obj.type,
5253
"integrationId": str(obj.integration_id) if obj.integration_id else None,
5354
"data": obj.data,
54-
"config": obj.config,
55+
"config": convert_dict_key_case(obj.config, snake_to_camel_case),
5556
}
5657

5758

@@ -353,7 +354,7 @@ def serialize(self, obj: Detector, attrs: Mapping[str, Any], user, **kwargs) ->
353354
"dateUpdated": obj.date_updated,
354355
"dataSources": attrs.get("data_sources"),
355356
"conditionGroup": attrs.get("condition_group"),
356-
"config": attrs.get("config"),
357+
"config": convert_dict_key_case(attrs.get("config"), snake_to_camel_case),
357358
"enabled": obj.enabled,
358359
}
359360

@@ -424,7 +425,7 @@ def serialize(self, obj: Workflow, attrs: Mapping[str, Any], user, **kwargs) ->
424425
"triggers": attrs.get("triggers"),
425426
"actionFilters": attrs.get("actionFilters"),
426427
"environment": obj.environment.name if obj.environment else None,
427-
"config": obj.config,
428+
"config": convert_dict_key_case(obj.config, snake_to_camel_case),
428429
"detectorIds": attrs.get("detectorIds"),
429430
"enabled": obj.enabled,
430431
"lastTriggered": attrs.get("lastTriggered"),

src/sentry/workflow_engine/endpoints/validators/base/workflow.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,14 @@ def update_or_create_actions(
8989

9090
validator = BaseActionValidator(context=self.context)
9191
for action in actions_data:
92-
self._update_or_create(action, validator, Action)
92+
action_instance = self._update_or_create(action, validator, Action)
93+
94+
# If this is a new action, associate it to the condition group
95+
if action.get("id") is None:
96+
DataConditionGroupAction.objects.create(
97+
action=action_instance,
98+
condition_group=condition_group,
99+
)
93100

94101
def update_or_create_data_condition_group(
95102
self,

tests/sentry/workflow_engine/endpoints/test_organization_detector_index.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -309,8 +309,8 @@ def setUp(self):
309309
],
310310
},
311311
"config": {
312-
"threshold_period": 1,
313-
"detection_type": AlertRuleDetectionType.STATIC.value,
312+
"thresholdPeriod": 1,
313+
"detectionType": AlertRuleDetectionType.STATIC.value,
314314
},
315315
}
316316

tests/sentry/workflow_engine/endpoints/test_organization_workflow_index.py

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,7 @@ def setUp(self):
359359
"enabled": True,
360360
"config": {},
361361
"triggers": {"logicType": "any", "conditions": []},
362-
"action_filters": [],
362+
"actionFilters": [],
363363
}
364364

365365
def test_create_workflow__basic(self):
@@ -390,7 +390,7 @@ def test_create_workflow__with_triggers(self):
390390
{
391391
"type": "eq",
392392
"comparison": 1,
393-
"condition_result": True,
393+
"conditionResult": True,
394394
}
395395
],
396396
}
@@ -408,23 +408,23 @@ def test_create_workflow__with_triggers(self):
408408
)
409409

410410
def test_create_workflow__with_actions(self):
411-
self.valid_workflow["action_filters"] = [
411+
self.valid_workflow["actionFilters"] = [
412412
{
413413
"logicType": "any",
414414
"conditions": [
415415
{
416416
"type": "eq",
417417
"comparison": 1,
418-
"condition_result": True,
418+
"conditionResult": True,
419419
}
420420
],
421421
"actions": [
422422
{
423423
"type": Action.Type.SLACK,
424424
"config": {
425-
"target_identifier": "test",
426-
"target_display": "Test",
427-
"target_type": 0,
425+
"targetIdentifier": "test",
426+
"targetDisplay": "Test",
427+
"targetType": 0,
428428
},
429429
"data": {},
430430
"integrationId": 1,
@@ -461,7 +461,7 @@ def test_create_workflow__invalid_triggers(self):
461461
"conditions": [
462462
{
463463
"comparison": 1,
464-
"condition_result": True,
464+
"conditionResult": True,
465465
}
466466
],
467467
}
@@ -480,7 +480,7 @@ def test_create_workflow__invalid_actions(self):
480480
"conditions": [
481481
{
482482
"comparison": 1,
483-
"condition_result": True,
483+
"conditionResult": True,
484484
}
485485
],
486486
"actions": [

tests/sentry/workflow_engine/endpoints/test_serializers.py

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
from sentry.snuba.models import QuerySubscriptionDataSourceHandler, SnubaQuery
1111
from sentry.snuba.subscriptions import create_snuba_query, create_snuba_subscription
1212
from sentry.testutils.cases import TestCase
13-
from sentry.testutils.factories import default_detector_config_data
1413
from sentry.testutils.helpers.datetime import before_now, freeze_time
1514
from sentry.testutils.skips import requires_snuba
1615
from sentry.workflow_engine.endpoints.serializers import (
@@ -45,7 +44,10 @@ def test_serialize_simple(self):
4544
"dataSources": None,
4645
"conditionGroup": None,
4746
"workflowIds": [],
48-
"config": default_detector_config_data[MetricIssue.slug],
47+
"config": {
48+
"thresholdPeriod": 1,
49+
"detectionType": "static",
50+
},
4951
"owner": None,
5052
"enabled": detector.enabled,
5153
}
@@ -151,12 +153,15 @@ def test_serialize_full(self):
151153
"type": "email",
152154
"data": {},
153155
"integrationId": None,
154-
"config": {"target_type": 1, "target_identifier": "123"},
156+
"config": {"targetType": 1, "targetIdentifier": "123"},
155157
}
156158
],
157159
},
158160
"workflowIds": [str(workflow.id)],
159-
"config": default_detector_config_data[MetricIssue.slug],
161+
"config": {
162+
"thresholdPeriod": 1,
163+
"detectionType": "static",
164+
},
160165
"owner": self.user.get_actor_identifier(),
161166
"enabled": detector.enabled,
162167
}
@@ -283,7 +288,7 @@ def test_serialize_full(self):
283288
"type": "email",
284289
"data": {},
285290
"integrationId": None,
286-
"config": {"target_type": 1, "target_identifier": "123"},
291+
"config": {"targetType": 1, "targetIdentifier": "123"},
287292
}
288293
],
289294
}
@@ -335,11 +340,10 @@ def test_serialize_with_integration(self):
335340
"type": "opsgenie",
336341
"data": {"priority": "P1"},
337342
"integrationId": str(self.integration.id),
338-
"config": {"target_type": 0, "target_identifier": "123"},
343+
"config": {"targetType": 0, "targetIdentifier": "123"},
339344
}
340345

341346
def test_serialize_with_integration_and_config(self):
342-
343347
action2 = self.create_action(
344348
type=Action.Type.SLACK,
345349
data={"tags": "bar"},
@@ -359,9 +363,9 @@ def test_serialize_with_integration_and_config(self):
359363
"data": {"tags": "bar"},
360364
"integrationId": str(self.integration.id),
361365
"config": {
362-
"target_type": 0,
363-
"target_display": "freddy frog",
364-
"target_identifier": "123-id",
366+
"targetType": 0,
367+
"targetDisplay": "freddy frog",
368+
"targetIdentifier": "123-id",
365369
},
366370
}
367371

@@ -492,7 +496,7 @@ def test_serialize_full(self):
492496
"type": "email",
493497
"data": {},
494498
"integrationId": None,
495-
"config": {"target_type": 1, "target_identifier": "123"},
499+
"config": {"targetType": 1, "targetIdentifier": "123"},
496500
}
497501
],
498502
},

tests/sentry/workflow_engine/endpoints/validators/test_base_workflow.py

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
DataConditionGroup,
1515
DataConditionGroupAction,
1616
Workflow,
17+
WorkflowDataConditionGroup,
1718
)
1819
from tests.sentry.workflow_engine.test_base import MockActionHandler
1920

@@ -443,9 +444,9 @@ def test_update__add_new_filter(self):
443444
{
444445
"type": Action.Type.SLACK,
445446
"config": {
446-
"target_identifier": "foo",
447-
"target_display": "bar",
448-
"target_type": 0,
447+
"targetIdentifier": "bar",
448+
"targetDisplay": "baz",
449+
"targetType": 0,
449450
},
450451
"data": {},
451452
"integrationId": 1,
@@ -464,6 +465,26 @@ def test_update__add_new_filter(self):
464465
self.workflow.refresh_from_db()
465466

466467
assert self.workflow.workflowdataconditiongroup_set.count() == 2
468+
new_action_filter = (
469+
WorkflowDataConditionGroup.objects.filter(workflow=self.workflow)
470+
.order_by("-date_added")
471+
.first()
472+
)
473+
474+
assert new_action_filter is not None
475+
assert new_action_filter.condition_group is not None
476+
477+
new_actions = Action.objects.filter(
478+
dataconditiongroupaction__condition_group__in=[new_action_filter.condition_group.id]
479+
)
480+
481+
assert new_actions.count() == 1
482+
assert new_actions[0].type == Action.Type.SLACK
483+
assert new_actions[0].config == {
484+
"target_identifier": "bar",
485+
"target_display": "baz",
486+
"target_type": 0,
487+
}
467488

468489
def test_update__remove_one_filter(self):
469490
# Configuration for the test
@@ -547,9 +568,9 @@ def test_update__add_new_action(self):
547568
{
548569
"type": Action.Type.SLACK,
549570
"config": {
550-
"target_identifier": "foo",
551-
"target_display": "bar",
552-
"target_type": 0,
571+
"targetIdentifier": "foo",
572+
"targetDisplay": "bar",
573+
"targetType": 0,
553574
},
554575
"data": {},
555576
"integrationId": 1,
@@ -577,8 +598,8 @@ def test_update__modify_action(self):
577598
"id": action.id,
578599
"type": Action.Type.EMAIL,
579600
"config": {
580-
"target_identifier": "foo",
581-
"target_type": 0,
601+
"targetIdentifier": "foo",
602+
"targetType": 0,
582603
},
583604
"data": {},
584605
}

0 commit comments

Comments
 (0)