feat(workflow-engine): add EventAttributeConditionHandler
#82741
Conversation
github-actions
bot
added
the
Scope: Backend
| def evaluate_value(job: WorkflowJob, comparison: Any) -> bool: | ||
| event = job["event"] | ||
|
|
||
| attribute = comparison.get("attribute", "") |
There was a problem hiding this comment.
i think comparison is stored as a string so you'll need to do json.loads(comparison) to get it as a dictionary
| ) -> DataCondition: | ||
| return DataCondition.objects.create( | ||
| type=Condition.EVENT_ATTRIBUTE, | ||
| comparison=True, |
There was a problem hiding this comment.
where are the attribute, match type, and desired value being stored? one way to do this is to put them in a dictionary and json.dump them into comparison here
| @condition_handler_registry.register(Condition.EVENT_ATTRIBUTE) | ||
| class EventAttributeConditionHandler(DataConditionHandler[WorkflowJob]): | ||
| @staticmethod | ||
| def evaluate_value(job: WorkflowJob, comparison: Any) -> bool: |
There was a problem hiding this comment.
nit: if there's a lot happening in this function you can also break it up into several smaller ones
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## master #82741 +/- ##
==========================================
+ Coverage 87.49% 87.64% +0.15%
==========================================
Files 9408 9409 +1
Lines 538586 545517 +6931
Branches 21036 21036
==========================================
+ Hits 471245 478137 +6892
- Misses 66970 67009 +39
Partials 371 371 |
| dc = self.create_data_condition( | ||
| type=self.condition, | ||
| comparison=json.dumps( | ||
| {"match": MatchType.NOT_STARTS_WITH, "attribute": "platform", "value": "py"} | ||
| ), | ||
| condition_result=True, | ||
| ) |
There was a problem hiding this comment.
if you want to avoid creating another data condition, you can also update the comparison field from the previously created data condition
| return False | ||
|
|
||
| desired_value = str(desired_value).lower() | ||
| attribute_values = [str(value).lower() for value in attribute_values if value is not None] |
There was a problem hiding this comment.
should this go inside get_attribute_values?
| comparison = { | ||
| "match": MatchType.EQUAL, | ||
| "value": "php", | ||
| "attribute": "platform", | ||
| } |
There was a problem hiding this comment.
you can make self.payload on line 54 this without updating it here, you're not using it anywhere else rn
| ) | ||
| self.assert_does_not_pass(self.dc, self.job) | ||
|
|
||
| def test_equals(self): |
There was a problem hiding this comment.
it might be more readable if you split the tests into one class to test the comparisons (equals, contains, etc) and one to test the attributes?
tests/sentry/workflow_engine/handlers/condition/test_group_event_handlers.py
Show resolved
Hide resolved
| self.assert_does_not_pass(self.dc, self.job) | ||
|
|
||
| def test_equals(self): | ||
| self.dc.comparison = json.dumps( |
There was a problem hiding this comment.
if you wanted to change the value in the DB you should do self.dc.update(comparison=...) or self.dc.comparison = ... and self.dc.save()
i think it doesn't affect it here because the model class maintains state and we aren't querying for the data condition somewhere else
add `EventAttributeConditionHandler`
add
EventAttributeConditionHandler