ref(rules): Add changes to rule edit confirmation notification (#66847)

Follow up to #66285 to add
details about what changed happened when a rule was edited to the
notification.

<img width="748" alt="Screenshot 2024-03-12 at 4 40 24 PM"
src="https://github.com/getsentry/sentry/assets/29959063/e0d7c5ca-35e5-4a66-bc67-e53c8d27dcce">


Note that I've made a couple follow up tickets to address poor rendering
of issue type enums and frequency:
getsentry/team-core-product-foundations#158
and
getsentry/team-core-product-foundations#157

---------

Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>

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)

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,
         )
 
 

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:

src/sentry/integrations/slack/actions/notification.py

@@ -145,14 +145,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")),

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))
 

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
         """

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

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"<http://testserver/organizations/{self.organization.slug}/alerts/rules/{self.project.slug}/{rule_id}/details/|*{rule_label}*> in the *{self.project.slug}* project was updated."
+        message = f"Alert rule <http://testserver/organizations/{self.organization.slug}/alerts/rules/{self.project.slug}/{rule_id}/details/|*{rule_label}*> 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"]
             == "<http://testserver/settings/account/notifications/alerts/|*Notification Settings*>"
         )
 

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"<http://testserver/organizations/{self.organization.slug}/alerts/rules/{self.project.slug}/{rule_id}/details/|*{rule_label}*> was created in the *{self.project.slug}* project and will send notifications here."
+        message = f"Alert rule <http://testserver/organizations/{self.organization.slug}/alerts/rules/{self.project.slug}/{rule_id}/details/|*{rule_label}*> 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