feat(workflow_engine): Add create methods to validators #89769
Conversation
- Remove requirement for condition_group_id so condition_groups can create their own data conditions appropriately - Added Tests
github-actions
bot
added
the
Scope: Backend
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## master #89769 +/- ##
========================================
Coverage 85.47% 85.47%
========================================
Files 10194 10194
Lines 574925 575066 +141
Branches 22643 22643
========================================
+ Hits 491432 491559 +127
- Misses 82898 82912 +14
Partials 595 595 |
src/sentry/workflow_engine/endpoints/validators/base/workflow.py
Outdated
Show resolved
Hide resolved
| @@ -26,7 +26,7 @@ class AbstractDataConditionValidator( | |||
| type = serializers.ChoiceField(choices=[(t.value, t.value) for t in Condition]) | |||
| comparison = serializers.JSONField(required=True) | |||
| condition_result = serializers.JSONField(required=True) | |||
| condition_group_id = serializers.IntegerField(required=True) | |||
| condition_group_id = serializers.IntegerField(required=False) | |||
There was a problem hiding this comment.
i suppose the DataCondition will error if this isn't set and we try to create the object, since condition_group_id is required. do we make it optional because we only create these inside of DataConditionGroupValidator?
There was a problem hiding this comment.
Yes - that way the API doesn't need to set the condition_group_id on every request -- also it won't have the DCG id when we initially make a workflow - so if we re-use those validators it'll fail there as well.
| create_audit_entry( | ||
| request=self.context["request"], | ||
| organization=validated_value["organization_id"], | ||
| target_object=workflow.id, | ||
| event=audit_log.get_event_id("WORKFLOW_ADD"), | ||
| data=workflow.get_audit_log_data(), | ||
| ) |
There was a problem hiding this comment.
does this need to be unindented? it's currently in the for loop going through the action_filters
There was a problem hiding this comment.
👍 (this is why i don't like whitespace based languages 🤣)
| return value | ||
|
|
||
| def create(self, validated_value: dict[str, Any]) -> Workflow: | ||
| condition_group_validator = BaseDataConditionGroupValidator() |
There was a problem hiding this comment.
should the Workflow validator automatically convert DataConditionGroup.logic_type of ANY into ANY_SHORT_CIRCUIT? there's some discussion here about how to handle it in the FE #89233 (comment)
There was a problem hiding this comment.
i don't feel like it should auto-convert that. It probably could based on the type but it would add some artificial constraints that i'm not sure i agree with.
we should not convert any to any_short_circuit though as they are specifically used in different places for different reasons.
| WorkflowDataConditionGroup.objects.create( | ||
| condition_group=new_condition_group, | ||
| workflow=workflow, | ||
| ) | ||
|
|
||
| for action in actions: | ||
| new_action = action_validator.create(action) | ||
| DataConditionGroupAction.objects.create( | ||
| action=new_action, | ||
| condition_group=new_condition_group, | ||
| ) |
There was a problem hiding this comment.
super nit: can maybe bulk create the WorkflowDataConditionGroups and DataConditionGroupActions but idk how much time that would actually save
There was a problem hiding this comment.
I'll add a todo to come back and optimize this / check if there's a better way to bulk create.
My expectation would be that there's only one or two of these per workflow, so it's probably okay for a bit.
Description
Add the
.createmethod to data condition, data condition group, and workflow validators. This allows us to validate and create objects safely. This will also safely create items in an atomic transaction -- bubbling up to the workflow layer.