-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
ref(rules): Add changes to rule edit confirmation notification #66847
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
Changes from all commits
de88cd8
d8fd5b6
065b2f2
67e4ede
ffbbec9
dd731e2
4ae40b8
eba8d41
4217385
52feefd
50fea34
68d71e6
f7b5d75
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,114 @@ | ||
from collections import defaultdict | ||
from typing import Any, DefaultDict | ||
|
||
from sentry.api.serializers.models.rule import generate_rule_label | ||
from sentry.models.environment import Environment | ||
from sentry.models.rule import Rule | ||
|
||
|
||
def get_updated_rule_data(rule: Rule) -> dict[str, Any]: | ||
rule_data = dict(rule.data) | ||
if rule.environment_id: | ||
rule_data["environment_id"] = rule.environment_id | ||
if rule.owner: | ||
rule_data["owner"] = rule.owner | ||
rule_data["label"] = rule.label | ||
return rule_data | ||
|
||
|
||
def check_value_changed( | ||
present_state: dict[str, Any], prior_state: dict[str, Any], key: str, word: str | ||
) -> str | None: | ||
if present_state.get(key) != prior_state.get(key): | ||
old_value = prior_state.get(key) | ||
new_value = present_state.get(key) | ||
return f"Changed {word} from *{old_value}* to *{new_value}*" | ||
return None | ||
|
||
|
||
def generate_diff_labels( | ||
present_state: dict[str, Any], | ||
prior_state: dict[str, Any], | ||
rule: Rule, | ||
changed_data: DefaultDict[str, list[str]], | ||
key: str, | ||
statement: str, | ||
) -> DefaultDict[str, list[str]]: | ||
for data in prior_state.get(key, []): | ||
if data not in present_state.get(key, []): | ||
label = generate_rule_label(rule.project, rule, data) | ||
changed_data[data["id"]].append(statement.format(label)) | ||
|
||
return changed_data | ||
|
||
|
||
def get_changed_data( | ||
rule: Rule, rule_data: dict[str, Any], rule_data_before: dict[str, Any] | ||
) -> dict[str, Any]: | ||
""" | ||
Generate a list per type of issue alert rule data of what changes occurred on edit. | ||
""" | ||
changed_data: DefaultDict[str, list[str]] = defaultdict(list) | ||
changed_data = generate_diff_labels( | ||
rule_data_before, rule_data, rule, changed_data, "conditions", "Added condition '{}'" | ||
) | ||
changed_data = generate_diff_labels( | ||
rule_data, rule_data_before, rule, changed_data, "conditions", "Removed condition '{}'" | ||
) | ||
changed_data = generate_diff_labels( | ||
rule_data_before, rule_data, rule, changed_data, "actions", "Added action '{}'" | ||
) | ||
changed_data = generate_diff_labels( | ||
rule_data, rule_data_before, rule, changed_data, "actions", "Removed action '{}'" | ||
) | ||
|
||
frequency_text = check_value_changed(rule_data, rule_data_before, "frequency", "frequency") | ||
if frequency_text: | ||
changed_data["changed_frequency"].append(frequency_text) | ||
|
||
if rule_data.get("environment_id") and not rule_data_before.get("environment_id"): | ||
environment = None | ||
try: | ||
environment = Environment.objects.get(id=rule_data.get("environment_id")) | ||
except Environment.DoesNotExist: | ||
pass | ||
|
||
if environment: | ||
changed_data["environment"].append(f"Added *{environment.name}* environment") | ||
|
||
if rule_data_before.get("environment_id") and not rule_data.get("environment_id"): | ||
environment = None | ||
try: | ||
environment = Environment.objects.get(id=rule_data.get("environment_id")) | ||
except Environment.DoesNotExist: | ||
pass | ||
Comment on lines
+79
to
+84
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm pretty sure this is a typo -- the type checker is pointing this out as a bug when I enable model checking this is always passing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix here - #73665 |
||
|
||
if environment: | ||
changed_data["environment"].append(f"Removed *{environment.name}* environment") | ||
|
||
label_text = check_value_changed(rule_data, rule_data_before, "label", "rule name") | ||
if label_text: | ||
changed_data["changed_label"].append(label_text) | ||
|
||
action_match_text = check_value_changed(rule_data, rule_data_before, "action_match", "trigger") | ||
if action_match_text: | ||
changed_data["action_match"].append(action_match_text) | ||
|
||
filter_match_text = check_value_changed(rule_data, rule_data_before, "filter_match", "filter") | ||
if filter_match_text: | ||
changed_data["filter_match"].append(filter_match_text) | ||
|
||
if rule_data_before.get("owner") != rule_data.get("owner"): | ||
old_owner = rule_data_before.get("owner") | ||
old_actor = "Unassigned" | ||
if old_owner: | ||
old_actor = old_owner.resolve() | ||
|
||
new_owner = rule_data.get("owner") | ||
new_actor = "Unassigned" | ||
if new_owner: | ||
new_actor = new_owner.resolve() | ||
owner_changed_text = f"Changed owner from *{old_actor}* to *{new_actor}*" | ||
changed_data["owner"].append(owner_changed_text) | ||
|
||
return changed_data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small nit: can skip returning None
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mypy wouldn't let me 😭
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
apparently by design: python/mypy#7511 (comment)
There's a flag
--no-warn-no-return
to disable this but looks like we don't have it on