Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
from sentry.incidents.endpoints.serializers.incident import DetailedIncidentSerializer
from sentry.incidents.logic import update_incident_status
from sentry.incidents.models.incident import IncidentStatus, IncidentStatusMethod
from sentry.users.services.user.serial import serialize_generic_user


class IncidentSerializer(serializers.Serializer):
Expand Down Expand Up @@ -56,8 +55,6 @@ def put(self, request: Request, organization, incident) -> Response:
incident = update_incident_status(
incident=incident,
status=result["status"],
user=serialize_generic_user(request.user),
comment=result.get("comment"),
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the only call to update_incident_status that passes user and comment so I checked the API access logs to confirm the only usage is by customers who are probably using it to close incidents. I also ensured that we haven't sent the incident activity emails just in case a customer was for some reason using that part of it.

status_method=IncidentStatusMethod.MANUAL,
)
return Response(
Expand Down
9 changes: 0 additions & 9 deletions src/sentry/incidents/events.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,5 @@ class IncidentStatusUpdatedEvent(BaseIncidentEvent):
)


class IncidentCommentCreatedEvent(BaseIncidentEvent):
type = "incident.comment"
attributes = BaseIncidentEvent.attributes + (
analytics.Attribute("user_id", required=False),
analytics.Attribute("activity_id", required=False),
)


analytics.register(IncidentCreatedEvent)
analytics.register(IncidentStatusUpdatedEvent)
analytics.register(IncidentCommentCreatedEvent)
31 changes: 0 additions & 31 deletions src/sentry/incidents/logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,6 @@ def create_incident(
def update_incident_status(
incident: Incident,
status: IncidentStatus,
user: RpcUser | None = None,
comment: str | None = None,
status_method: IncidentStatusMethod = IncidentStatusMethod.RULE_TRIGGERED,
date_closed: datetime | None = None,
) -> Incident:
Expand All @@ -216,13 +214,9 @@ def update_incident_status(
create_incident_activity(
incident,
IncidentActivityType.STATUS_CHANGE,
user=user,
value=status.value,
previous_value=incident.status,
comment=comment,
)
if user:
subscribe_to_incident(incident, user.id)

prev_status = incident.status
kwargs: dict[str, Any] = {"status": status.value, "status_method": status_method.value}
Expand Down Expand Up @@ -260,12 +254,9 @@ def create_incident_activity(
user: RpcUser | User | None = None,
value: str | int | None = None,
previous_value: str | int | None = None,
comment: str | None = None,
mentioned_user_ids: Collection[int] = (),
date_added: datetime | None = None,
) -> IncidentActivity:
if activity_type == IncidentActivityType.COMMENT and user:
subscribe_to_incident(incident, user.id)
value = str(value) if value is not None else None
previous_value = str(previous_value) if previous_value is not None else None
kwargs = {}
Expand All @@ -277,7 +268,6 @@ def create_incident_activity(
user_id=user.id if user else None,
value=value,
previous_value=previous_value,
comment=comment,
notification_uuid=uuid4(),
**kwargs,
)
Expand All @@ -295,22 +285,6 @@ def create_incident_activity(
for mentioned_user_id in user_ids_to_subscribe
]
)
transaction.on_commit(
lambda: tasks.send_subscriber_notifications.apply_async(
kwargs={"activity_id": activity.id}, countdown=10
),
router.db_for_write(IncidentSubscription),
)
if activity_type == IncidentActivityType.COMMENT:
analytics.record(
"incident.comment",
incident_id=incident.id,
organization_id=incident.organization_id,
incident_type=incident.type,
user_id=user.id if user else None,
activity_id=activity.id,
)

return activity


Expand Down Expand Up @@ -481,11 +455,6 @@ def get_incident_aggregates(
return aggregated_result[0]


def subscribe_to_incident(incident: Incident, user_id: int) -> IncidentSubscription:
subscription, _ = IncidentSubscription.objects.get_or_create(incident=incident, user_id=user_id)
return subscription


def unsubscribe_from_incident(incident: Incident, user_id: int) -> None:
IncidentSubscription.objects.filter(incident=incident, user_id=user_id).delete()

Expand Down
103 changes: 0 additions & 103 deletions src/sentry/incidents/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,15 @@

import logging
from typing import Any
from urllib.parse import urlencode

from django.urls import reverse

from sentry.auth.access import from_user
from sentry.incidents.models.alert_rule import (
AlertRuleStatus,
AlertRuleTriggerAction,
AlertRuleTriggerActionMethod,
)
from sentry.incidents.models.incident import (
INCIDENT_STATUS,
Incident,
IncidentActivity,
IncidentActivityType,
IncidentStatus,
IncidentStatusMethod,
)
Expand All @@ -31,107 +25,11 @@
from sentry.snuba.models import QuerySubscription
from sentry.snuba.query_subscriptions.consumer import register_subscriber
from sentry.tasks.base import instrumented_task
from sentry.users.models.user import User
from sentry.users.services.user import RpcUser
from sentry.users.services.user.service import user_service
from sentry.utils import metrics
from sentry.utils.email import MessageBuilder
from sentry.utils.http import absolute_uri

logger = logging.getLogger(__name__)


@instrumented_task(
name="sentry.incidents.tasks.send_subscriber_notifications",
queue="incidents",
silo_mode=SiloMode.REGION,
)
def send_subscriber_notifications(activity_id: int) -> None:
Copy link
Member Author

Choose a reason for hiding this comment

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

I confirmed via sendgrid that we haven't sent one of these in the last 30 days. There are no IncidentActivity rows with a type of comment since 2021, and since user isn't passed to update_incident_status nobody was getting subscribed so we never found a subscriber to execute the code on lines 83 and 84.

from sentry.incidents.logic import get_incident_subscribers, unsubscribe_from_incident

try:
activity = IncidentActivity.objects.select_related(
"incident", "incident__organization"
).get(id=activity_id)
except IncidentActivity.DoesNotExist:
return

if activity.user_id is None:
return

activity_user = user_service.get_user(user_id=activity.user_id)

# Only send notifications for specific activity types.
if activity.type not in (
IncidentActivityType.COMMENT.value,
IncidentActivityType.STATUS_CHANGE.value,
):
return

# Check that the user still has access to at least one of the projects
# related to the incident. If not then unsubscribe them.
projects = list(activity.incident.projects.all())
for subscriber in get_incident_subscribers(activity.incident):
subscriber_user = user_service.get_user(user_id=subscriber.user_id)
if subscriber_user is None:
continue

access = from_user(subscriber_user, activity.incident.organization)
if not any(project for project in projects if access.has_project_access(project)):
unsubscribe_from_incident(activity.incident, subscriber_user.id)
elif subscriber_user.id != activity.user_id:
msg = generate_incident_activity_email(activity, subscriber_user, activity_user)
msg.send_async([subscriber_user.email])


def generate_incident_activity_email(
activity: IncidentActivity, user: RpcUser | User, activity_user: RpcUser | User | None = None
) -> MessageBuilder:
incident = activity.incident
return MessageBuilder(
subject=f"Activity on Alert {incident.title} (#{incident.identifier})",
template="sentry/emails/incidents/activity.txt",
html_template="sentry/emails/incidents/activity.html",
type="incident.activity",
context=build_activity_context(activity, user, activity_user),
)


def build_activity_context(
activity: IncidentActivity, user: RpcUser | User, activity_user: RpcUser | None = None
) -> dict[str, Any]:
if activity_user is None:
activity_user = user_service.get_user(user_id=activity.user_id)

if activity.type == IncidentActivityType.COMMENT.value:
action = "left a comment"
else:
action = "changed status from {} to {}".format(
INCIDENT_STATUS[IncidentStatus(int(activity.previous_value))],
INCIDENT_STATUS[IncidentStatus(int(activity.value))],
)
incident = activity.incident

action = f"{action} on alert {incident.title} (#{incident.identifier})"

return {
"user_name": activity_user.name if activity_user else "Sentry",
"action": action,
"link": absolute_uri(
reverse(
"sentry-metric-alert",
kwargs={
"organization_slug": incident.organization.slug,
"incident_id": incident.identifier,
},
)
)
+ "?"
+ urlencode({"referrer": "incident_activity_email"}),
"comment": activity.comment,
}


@register_subscriber(SUBSCRIPTION_METRICS_LOGGER)
def handle_subscription_metrics_logger(
subscription_update: QuerySubscriptionUpdate, subscription: QuerySubscription
Expand Down Expand Up @@ -265,7 +163,6 @@ def auto_resolve_snapshot_incidents(alert_rule_id: int, **kwargs: Any) -> None:
update_incident_status(
incident,
IncidentStatus.CLOSED,
comment="This alert has been auto-resolved because the rule that triggered it has been modified or deleted.",
status_method=IncidentStatusMethod.RULE_UPDATED,
)

Expand Down
6 changes: 0 additions & 6 deletions src/sentry/testutils/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

from sentry.eventstore.models import Event
from sentry.incidents.models.alert_rule import AlertRule, AlertRuleMonitorTypeInt
from sentry.incidents.models.incident import IncidentActivityType
from sentry.integrations.models.integration import Integration
from sentry.integrations.models.organization_integration import OrganizationIntegration
from sentry.models.activity import Activity
Expand Down Expand Up @@ -374,11 +373,6 @@ def create_incident(self, organization=None, projects=None, *args, **kwargs):
def create_incident_activity(self, *args, **kwargs):
return Factories.create_incident_activity(*args, **kwargs)

def create_incident_comment(self, incident, *args, **kwargs):
return self.create_incident_activity(
incident, type=IncidentActivityType.COMMENT.value, *args, **kwargs
)

def create_incident_trigger(self, incident, alert_rule_trigger, status):
return Factories.create_incident_trigger(incident, alert_rule_trigger, status=status)

Expand Down
2 changes: 0 additions & 2 deletions src/sentry/web/debug_urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
from sentry.web.frontend.debug.debug_error_embed import DebugErrorPageEmbedView
from sentry.web.frontend.debug.debug_feedback_issue import DebugFeedbackIssueEmailView
from sentry.web.frontend.debug.debug_generic_issue import DebugGenericIssueEmailView
from sentry.web.frontend.debug.debug_incident_activity_email import DebugIncidentActivityEmailView
from sentry.web.frontend.debug.debug_incident_trigger_email import DebugIncidentTriggerEmailView
from sentry.web.frontend.debug.debug_incident_trigger_email_activated_alert import (
DebugIncidentActivatedAlertTriggerEmailView,
Expand Down Expand Up @@ -148,7 +147,6 @@
re_path(
r"^debug/mail/sso-unlinked/no-password/$", DebugSsoUnlinkedNoPasswordEmailView.as_view()
),
re_path(r"^debug/mail/incident-activity/$", DebugIncidentActivityEmailView.as_view()),
re_path(r"^debug/mail/incident-trigger/$", DebugIncidentTriggerEmailView.as_view()),
re_path(
r"^debug/mail/activated-incident-trigger/$",
Expand Down
28 changes: 0 additions & 28 deletions src/sentry/web/frontend/debug/debug_incident_activity_email.py

This file was deleted.

37 changes: 2 additions & 35 deletions tests/sentry/api/serializers/test_incident_activity.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,47 +8,21 @@
from sentry.incidents.models.incident import IncidentActivityType
from sentry.testutils.cases import SnubaTestCase, TestCase
from sentry.testutils.helpers.datetime import before_now, freeze_time
from sentry.users.services.user.service import user_service


class IncidentActivitySerializerTest(TestCase, SnubaTestCase):
def test_simple(self):
activity = create_incident_activity(
incident=self.create_incident(),
activity_type=IncidentActivityType.COMMENT,
user=self.user,
comment="hello",
activity_type=IncidentActivityType.CREATED,
)
result = serialize(activity)

assert result["id"] == str(activity.id)
assert result["incidentIdentifier"] == str(activity.incident.identifier)
assert (
result["user"]
== user_service.serialize_many(filter=dict(user_ids=[activity.user_id]))[0]
)
assert result["type"] == activity.type
assert result["value"] is None
assert result["previousValue"] is None
assert result["comment"] == activity.comment
assert result["dateCreated"] == activity.date_added

def test_no_user(self):
activity = create_incident_activity(
incident=self.create_incident(),
activity_type=IncidentActivityType.COMMENT,
user=None,
comment="hello",
)
result = serialize(activity)

assert result["id"] == str(activity.id)
assert result["incidentIdentifier"] == str(activity.incident.identifier)
assert result["user"] is None
assert result["type"] == activity.type
assert result["value"] is None
assert result["previousValue"] is None
assert result["comment"] == activity.comment
assert result["dateCreated"] == activity.date_added

def test_event_stats(self):
Expand All @@ -68,20 +42,13 @@ def test_event_stats(self):
)
activity = create_incident_activity(
incident=incident,
activity_type=IncidentActivityType.COMMENT,
user=self.user,
comment="hello",
activity_type=IncidentActivityType.CREATED,
)
result = serialize(activity)

assert result["id"] == str(activity.id)
assert result["incidentIdentifier"] == str(activity.incident.identifier)
assert (
result["user"]
== user_service.serialize_many(filter=dict(user_ids=[activity.user_id]))[0]
)
assert result["type"] == activity.type
assert result["value"] is None
assert result["previousValue"] is None
assert result["comment"] == activity.comment
assert result["dateCreated"] == activity.date_added
Loading
Loading