Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(workflow-engine): add NewHighPriorityIssueConditionHandler #82846

Merged
merged 3 commits into from
Jan 3, 2025

Conversation

ameliahsu
Copy link
Member

add NewHighPriorityIssueConditionHandler

@ameliahsu ameliahsu requested review from cathteng and a team January 3, 2025 00:11
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jan 3, 2025
@@ -6,6 +6,18 @@
from sentry.workflow_engine.types import DataConditionHandler, WorkflowJob


def is_new_event(job: WorkflowJob) -> bool:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

comparison=True,
condition_result=True,
)
self.group_event.group.priority = PriorityLevel.HIGH
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we use update() for all of these? i feel like it would be better if the DB state reflected the class state

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

like self.group_event.group.update(priority=PriorityLevel.HIGH)


# This will only pass for new issues
self.group_event.group.update(priority=PriorityLevel.HIGH)
self.job.update({"group_state": {"is_new_group_environment": True}})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this needs to be self.job["group_state"]["is_new_group_environment"] = True. you can call update() because job is a dictionary, but mypy is complaining that group_state is missing some keys.

we should call update() for self.group_event.group because it's a Django model and we want to save the changes to the db

Copy link

codecov bot commented Jan 3, 2025

Codecov Report

Attention: Patch coverage is 98.55072% with 1 line in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
..._engine/handlers/condition/group_state_handlers.py 94.44% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #82846      +/-   ##
==========================================
+ Coverage   84.84%   84.88%   +0.03%     
==========================================
  Files        9410     9410              
  Lines      536845   536563     -282     
  Branches    21049    21049              
==========================================
- Hits       455508   455467      -41     
+ Misses      80865    80624     -241     
  Partials      472      472              

@ameliahsu ameliahsu merged commit 1ff6f85 into master Jan 3, 2025
49 checks passed
@ameliahsu ameliahsu deleted the mia/aci/new-high-priority-issue branch January 3, 2025 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants