diff --git a/src/sentry/api/endpoints/project_rule_details.py b/src/sentry/api/endpoints/project_rule_details.py index 8584be8c13ec9c..6c37a6d8eb270e 100644 --- a/src/sentry/api/endpoints/project_rule_details.py +++ b/src/sentry/api/endpoints/project_rule_details.py @@ -42,6 +42,7 @@ from sentry.models.team import Team from sentry.models.user import User from sentry.rules.actions import trigger_sentry_app_action_creators_for_issues +from sentry.rules.actions.utils import get_changed_data, get_updated_rule_data from sentry.signals import alert_rule_edited from sentry.tasks.integrations.slack import find_channel_id_for_rule @@ -228,6 +229,13 @@ def put(self, request: Request, project, rule) -> Response: - Filters - help control noise by triggering an alert only if the issue matches the specified criteria. - Actions - specify what should happen when the trigger conditions are met and the filters match. """ + rule_data_before = dict(rule.data) + if rule.environment_id: + rule_data_before["environment_id"] = rule.environment_id + if rule.owner: + rule_data_before["owner"] = rule.owner + rule_data_before["label"] = rule.label + serializer = DrfRuleSerializer( context={"project": project, "organization": project.organization}, data=request.data, @@ -367,7 +375,9 @@ def put(self, request: Request, project, rule) -> Response: if features.has( "organizations:rule-create-edit-confirm-notification", project.organization ): - send_confirmation_notification(rule=rule, new=False) + new_rule_data = get_updated_rule_data(rule) + changed_data = get_changed_data(rule, new_rule_data, rule_data_before) + send_confirmation_notification(rule=rule, new=False, changed=changed_data) return Response(serialize(updated_rule, request.user)) return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) diff --git a/src/sentry/api/endpoints/project_rules.py b/src/sentry/api/endpoints/project_rules.py index f1f70caf0ebf22..a556ab6128b295 100644 --- a/src/sentry/api/endpoints/project_rules.py +++ b/src/sentry/api/endpoints/project_rules.py @@ -38,13 +38,14 @@ from sentry.utils.safe import safe_execute -def send_confirmation_notification(rule: Rule, new: bool): +def send_confirmation_notification(rule: Rule, new: bool, changed: dict | None = None): for action in rule.data.get("actions", ()): action_inst = instantiate_action(rule, action) safe_execute( action_inst.send_confirmation_notification, rule=rule, new=new, + changed=changed, ) diff --git a/src/sentry/api/serializers/models/rule.py b/src/sentry/api/serializers/models/rule.py index 26e0d24640670e..d764a485f2002a 100644 --- a/src/sentry/api/serializers/models/rule.py +++ b/src/sentry/api/serializers/models/rule.py @@ -14,7 +14,7 @@ from sentry.services.hybrid_cloud.user.service import user_service -def _generate_rule_label(project, rule, data): +def generate_rule_label(project, rule, data): from sentry.rules import rules rule_cls = rules.get(data["id"]) @@ -202,7 +202,7 @@ def get_attrs(self, item_list, user, **kwargs): def serialize(self, obj, attrs, user, **kwargs) -> RuleSerializerResponse: environment = attrs["environment"] all_conditions = [ - dict(list(o.items()) + [("name", _generate_rule_label(obj.project, obj, o))]) + dict(list(o.items()) + [("name", generate_rule_label(obj.project, obj, o))]) for o in obj.data.get("conditions", []) ] @@ -212,7 +212,7 @@ def serialize(self, obj, attrs, user, **kwargs) -> RuleSerializerResponse: actions.append( dict( list(action.items()) - + [("name", _generate_rule_label(obj.project, obj, action))] + + [("name", generate_rule_label(obj.project, obj, action))] ) ) except serializers.ValidationError: diff --git a/src/sentry/integrations/slack/actions/notification.py b/src/sentry/integrations/slack/actions/notification.py index 7a34b4bb66017d..8b1c46da027ab6 100644 --- a/src/sentry/integrations/slack/actions/notification.py +++ b/src/sentry/integrations/slack/actions/notification.py @@ -139,14 +139,16 @@ def send_notification(event: GroupEvent, futures: Sequence[RuleFuture]) -> None: metrics.incr("notifications.sent", instance="slack.notification", skip_internal=False) yield self.future(send_notification, key=key) - def send_confirmation_notification(self, rule: Rule, new: bool): + def send_confirmation_notification( + self, rule: Rule, new: bool, changed: dict[str, Any] | None = None + ): integration = self.get_integration() if not integration: # Integration removed, rule still active. return channel = self.get_option("channel_id") - blocks = SlackRuleSaveEditMessageBuilder(rule=rule, new=new).build() + blocks = SlackRuleSaveEditMessageBuilder(rule=rule, new=new, changed=changed).build() payload = { "text": blocks.get("text"), "blocks": json.dumps(blocks.get("blocks")), diff --git a/src/sentry/integrations/slack/message_builder/notifications/rule_save_edit.py b/src/sentry/integrations/slack/message_builder/notifications/rule_save_edit.py index d48f905519974d..fccdfc681ffe10 100644 --- a/src/sentry/integrations/slack/message_builder/notifications/rule_save_edit.py +++ b/src/sentry/integrations/slack/message_builder/notifications/rule_save_edit.py @@ -13,10 +13,12 @@ def __init__( self, rule: Rule, new: bool, + changed: dict | None = None, ) -> None: super().__init__() self.rule = rule self.new = new + self.changed = changed def linkify_rule(self): org_slug = self.rule.project.organization.slug @@ -40,13 +42,21 @@ def build(self) -> SlackBlock: rule_url = self.linkify_rule() project = self.rule.project.slug if self.new: - rule_text = f"{rule_url} was created in the *{project}* project and will send notifications here." + rule_text = f"Alert rule {rule_url} was created in the *{project}* project and will send notifications here." else: - rule_text = f"{rule_url} in the *{project}* project was updated." + rule_text = f"Alert rule {rule_url} in the *{project}* project was updated." + # TODO potentially use old name if it's changed? - # TODO add short summary of the trigger & filter conditions blocks.append(self.get_markdown_block(rule_text)) + if not self.new and self.changed: + changes_text = "*Changes*\n" + for changes in self.changed.values(): + for change in changes: + changes_text += f"• {change}\n" + + blocks.append(self.get_markdown_block(changes_text)) + settings_link = self.get_settings_url() blocks.append(self.get_context_block(settings_link)) diff --git a/src/sentry/rules/actions/base.py b/src/sentry/rules/actions/base.py index da2ca4ea4591ba..a65cd7f6aa7805 100644 --- a/src/sentry/rules/actions/base.py +++ b/src/sentry/rules/actions/base.py @@ -3,6 +3,7 @@ import abc import logging from collections.abc import Generator +from typing import Any from sentry.eventstore.models import GroupEvent from sentry.models.rule import Rule @@ -54,7 +55,7 @@ def after( >>> print(future) """ - def send_confirmation_notification(self, rule: Rule, new: bool): + def send_confirmation_notification(self, rule: Rule, new: bool, changed: dict[str, Any]): """ Send a notification confirming that a rule was created or edited """ diff --git a/src/sentry/rules/actions/utils.py b/src/sentry/rules/actions/utils.py new file mode 100644 index 00000000000000..3013516343e4a8 --- /dev/null +++ b/src/sentry/rules/actions/utils.py @@ -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 + + 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 diff --git a/tests/sentry/api/endpoints/test_project_rule_details.py b/tests/sentry/api/endpoints/test_project_rule_details.py index da1e3eac944433..5af1f58a9e402a 100644 --- a/tests/sentry/api/endpoints/test_project_rule_details.py +++ b/tests/sentry/api/endpoints/test_project_rule_details.py @@ -1013,7 +1013,7 @@ def test_slack_confirmation_notification_contents(self): "channel": "#old_channel_name", } ] - self.rule.update(data={"conditions": conditions, "actions": actions}) + self.rule.update(data={"conditions": conditions, "actions": actions}, label="my rule") actions[0]["channel"] = "#new_channel_name" actions[0]["channel_id"] = "new_channel_id" @@ -1054,13 +1054,18 @@ def test_slack_confirmation_notification_contents(self): content_type="application/json", body=json.dumps(payload), ) + staging_env = self.create_environment( + self.project, name="staging", organization=self.organization + ) payload = { - "name": "#new_channel_name", + "name": "new rule", "actionMatch": "any", "filterMatch": "any", "actions": actions, "conditions": conditions, "frequency": 30, + "environment": staging_env.name, + "owner": get_actor_for_user(self.user).get_actor_identifier(), } response = self.get_success_response( self.organization.slug, self.project.slug, self.rule.id, status_code=200, **payload @@ -1069,12 +1074,22 @@ def test_slack_confirmation_notification_contents(self): rule_label = response.data["name"] assert response.data["actions"][0]["channel_id"] == "new_channel_id" data = parse_qs(responses.calls[1].request.body) - message = f" in the *{self.project.slug}* project was updated." + message = f"Alert rule in the *{self.project.slug}* project was updated." assert data["text"][0] == message rendered_blocks = json.loads(data["blocks"][0]) assert rendered_blocks[0]["text"]["text"] == message + changes = "*Changes*\n" + changes += "• Added action 'Send a notification to the Awesome Team Slack workspace to new_channel_name (optionally, an ID: new_channel_id) and show tags [] in notification'\n" + changes += "• Removed action 'Send a notification to the Awesome Team Slack workspace to #old_channel_name (optionally, an ID: old_channel_id) and show tags [] in notification'\n" + changes += "• Changed frequency from *None* to *30*\n" + changes += f"• Added *{staging_env.name}* environment\n" + changes += "• Changed rule name from *my rule* to *new rule*\n" + changes += "• Changed trigger from *None* to *any*\n" + changes += "• Changed filter from *None* to *any*\n" + changes += f"• Changed owner from *Unassigned* to *{self.user.email}*\n" + assert rendered_blocks[1]["text"]["text"] == changes assert ( - rendered_blocks[1]["elements"][0]["text"] + rendered_blocks[2]["elements"][0]["text"] == "" ) diff --git a/tests/sentry/api/endpoints/test_project_rules.py b/tests/sentry/api/endpoints/test_project_rules.py index ef975ca1d5fbf4..30f2dbec06da31 100644 --- a/tests/sentry/api/endpoints/test_project_rules.py +++ b/tests/sentry/api/endpoints/test_project_rules.py @@ -423,7 +423,7 @@ def test_slack_confirmation_notification_contents(self): rule_label = response.data["name"] assert response.data["actions"][0]["channel_id"] == self.channel_id data = parse_qs(responses.calls[1].request.body) - message = f" was created in the *{self.project.slug}* project and will send notifications here." + message = f"Alert rule was created in the *{self.project.slug}* project and will send notifications here." assert data["text"][0] == message rendered_blocks = json.loads(data["blocks"][0]) assert rendered_blocks[0]["text"]["text"] == message