Skip to content

Commit eb8a0c4

Browse files
getsentry-botcathteng
authored andcommitted
Revert "chore(slack): default message builder to block kit (#68016)"
This reverts commit 5b6682b. Co-authored-by: cathteng <70817427+cathteng@users.noreply.github.com>
1 parent abd95bc commit eb8a0c4

File tree

17 files changed

+1413
-145
lines changed

17 files changed

+1413
-145
lines changed

src/sentry/integrations/slack/message_builder/issues.py

Lines changed: 106 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
from django.utils.translation import gettext as _
1111
from sentry_relay.processing import parse_release
1212

13-
from sentry import tagstore
13+
from sentry import features, tagstore
1414
from sentry.api.endpoints.group_details import get_group_global_count
1515
from sentry.constants import LOG_LEVELS_MAP
1616
from sentry.eventstore.models import GroupEvent
@@ -22,6 +22,7 @@
2222
format_actor_option,
2323
format_actor_options,
2424
get_color,
25+
get_timestamp,
2526
get_title_link,
2627
)
2728
from sentry.integrations.slack.message_builder import (
@@ -264,20 +265,36 @@ def get_context(group: Group) -> str:
264265
return context_text.rstrip()
265266

266267

268+
def get_option_groups(group: Group) -> Sequence[Mapping[str, Any]]:
269+
all_members = group.project.get_members_as_rpc_users()
270+
members = list({m.id: m for m in all_members}.values())
271+
teams = group.project.teams.all()
272+
273+
option_groups = []
274+
if teams:
275+
option_groups.append({"text": "Teams", "options": format_actor_options(teams)})
276+
277+
if members:
278+
option_groups.append({"text": "People", "options": format_actor_options(members)})
279+
280+
return option_groups
281+
282+
267283
def get_option_groups_block_kit(group: Group) -> Sequence[Mapping[str, Any]]:
268284
all_members = group.project.get_members_as_rpc_users()
269285
members = list({m.id: m for m in all_members}.values())
270286
teams = group.project.teams.all()
287+
use_block_kit = features.has("organizations:slack-block-kit", group.project.organization)
271288

272289
option_groups = []
273290
if teams:
274-
team_options = format_actor_options(teams, True)
291+
team_options = format_actor_options(teams, use_block_kit)
275292
option_groups.append(
276293
{"label": {"type": "plain_text", "text": "Teams"}, "options": team_options}
277294
)
278295

279296
if members:
280-
member_options = format_actor_options(members, True)
297+
member_options = format_actor_options(members, use_block_kit)
281298
option_groups.append(
282299
{"label": {"type": "plain_text", "text": "People"}, "options": member_options}
283300
)
@@ -408,51 +425,91 @@ def build_actions(
408425
identity: RpcIdentity | None = None,
409426
) -> tuple[Sequence[MessageAction], str, str]:
410427
"""Having actions means a button will be shown on the Slack message e.g. ignore, resolve, assign."""
428+
use_block_kit = features.has("organizations:slack-block-kit", project.organization)
429+
411430
if actions and identity:
412431
text = get_action_text(text, actions, identity)
413432
return [], text, "_actioned_issue"
414433

415434
status = group.get_status()
416435

417-
def _ignore_button() -> MessageAction | None:
436+
def _ignore_button(use_block_kit) -> MessageAction | None:
418437
if group.issue_category == GroupCategory.FEEDBACK:
419438
return None
439+
if use_block_kit:
440+
if status == GroupStatus.IGNORED:
441+
return MessageAction(
442+
name="status", label="Mark as Ongoing", value="unresolved:ongoing"
443+
)
444+
return MessageAction(name="status", label="Archive", value="archive_dialog")
445+
420446
if status == GroupStatus.IGNORED:
421-
return MessageAction(name="status", label="Mark as Ongoing", value="unresolved:ongoing")
447+
return MessageAction(
448+
name="status",
449+
label="Mark as Ongoing",
450+
value="unresolved:ongoing",
451+
)
422452

423-
return MessageAction(name="status", label="Archive", value="archive_dialog")
453+
return MessageAction(
454+
name="status",
455+
label="Archive",
456+
value="ignored:until_escalating",
457+
)
458+
459+
def _resolve_button(use_block_kit) -> MessageAction:
460+
if use_block_kit:
461+
if status == GroupStatus.RESOLVED:
462+
return MessageAction(
463+
name="unresolved:ongoing", label="Unresolve", value="unresolved:ongoing"
464+
)
465+
if not project.flags.has_releases:
466+
return MessageAction(name="status", label="Resolve", value="resolved")
467+
return MessageAction(
468+
name="status",
469+
label="Resolve",
470+
value="resolve_dialog",
471+
)
424472

425-
def _resolve_button() -> MessageAction:
426473
if status == GroupStatus.RESOLVED:
427474
return MessageAction(
428-
name="unresolved:ongoing", label="Unresolve", value="unresolved:ongoing"
475+
name="status",
476+
label="Unresolve",
477+
value="unresolved:ongoing",
429478
)
430-
if not project.flags.has_releases:
431-
return MessageAction(name="status", label="Resolve", value="resolved")
432479

480+
if not project.flags.has_releases:
481+
return MessageAction(
482+
name="status",
483+
label="Resolve",
484+
value="resolved",
485+
)
433486
return MessageAction(
434-
name="status",
435-
label="Resolve",
487+
name="resolve_dialog",
488+
label="Resolve...",
436489
value="resolve_dialog",
437490
)
438491

439-
def _assign_button() -> MessageAction:
492+
def _assign_button(use_block_kit) -> MessageAction:
440493
assignee = group.get_assignee()
441494
assign_button = MessageAction(
442495
name="assign",
443496
label="Select Assignee...",
444497
type="select",
445-
selected_options=format_actor_options([assignee], True) if assignee else [],
446-
option_groups=get_option_groups_block_kit(group),
498+
selected_options=format_actor_options([assignee]) if assignee else [],
499+
option_groups=(
500+
get_option_groups(group)
501+
if not use_block_kit
502+
else get_option_groups_block_kit(group)
503+
),
447504
)
448505
return assign_button
449506

450507
action_list = [
451508
a
452509
for a in [
453-
_resolve_button(),
454-
_ignore_button(),
455-
_assign_button(),
510+
_resolve_button(use_block_kit),
511+
_ignore_button(use_block_kit),
512+
_assign_button(use_block_kit),
456513
]
457514
if a is not None
458515
]
@@ -495,6 +552,9 @@ def __init__(
495552
self.skip_fallback = skip_fallback
496553
self.notes = notes
497554
self.commits = commits
555+
self.use_block_kit = features.has(
556+
"organizations:slack-block-kit", group.project.organization
557+
)
498558

499559
@property
500560
def escape_text(self) -> bool:
@@ -508,7 +568,14 @@ def build(self, notification_uuid: str | None = None) -> SlackBlock | SlackAttac
508568
text = build_attachment_text(self.group, self.event) or ""
509569
text = text.strip(" \n")
510570

511-
text = escape_slack_markdown_text(text)
571+
if self.use_block_kit:
572+
text = escape_slack_markdown_text(text)
573+
if not self.use_block_kit and self.escape_text:
574+
text = escape_slack_text(text)
575+
# XXX(scefali): Not sure why we actually need to do this just for unfurled messages.
576+
# If we figure out why this is required we should note it here because it's quite strange
577+
if self.is_unfurl:
578+
text = escape_slack_text(text)
512579

513580
# This link does not contain user input (it's a static label and a url), must not escape it.
514581
replay_link = build_attachment_replay_link(self.group, self.event)
@@ -517,6 +584,7 @@ def build(self, notification_uuid: str | None = None) -> SlackBlock | SlackAttac
517584
# If an event is unspecified, use the tags of the latest event (if one exists).
518585
event_for_tags = self.event or self.group.get_latest_event()
519586
color = get_color(event_for_tags, self.notification, self.group)
587+
fields = build_tag_fields(event_for_tags, self.tags)
520588
footer = (
521589
self.notification.build_notification_footer(self.recipient, ExternalProviders.SLACK)
522590
if self.notification and self.recipient
@@ -550,6 +618,25 @@ def build(self, notification_uuid: str | None = None) -> SlackBlock | SlackAttac
550618
)
551619
title = build_attachment_title(obj)
552620

621+
if not features.has("organizations:slack-block-kit", self.group.project.organization):
622+
if replay_link:
623+
text += f"\n\n{replay_link}"
624+
if action_text and self.identity:
625+
text += "\n" + action_text
626+
627+
return self._build(
628+
actions=payload_actions,
629+
callback_id=json.dumps({"issue": self.group.id}),
630+
color=color,
631+
fallback=self.build_fallback_text(obj, project.slug),
632+
fields=fields,
633+
footer=footer,
634+
text=text,
635+
title=title,
636+
title_link=title_link,
637+
ts=get_timestamp(self.group, self.event) if not self.issue_details else None,
638+
)
639+
553640
# build up the blocks for newer issue alert formatting #
554641

555642
# build title block

src/sentry/testutils/cases.py

Lines changed: 15 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -646,21 +646,16 @@ def detect_performance_problems_interceptor(
646646
perf_problem.fingerprint = fingerprint
647647
return perf_problems
648648

649-
with (
650-
mock.patch(
651-
"sentry.issues.ingest.send_issue_occurrence_to_eventstream",
652-
side_effect=send_issue_occurrence_to_eventstream,
653-
) as mock_eventstream,
654-
mock.patch(
655-
"sentry.event_manager.detect_performance_problems",
656-
side_effect=detect_performance_problems_interceptor,
657-
),
658-
mock.patch.object(
659-
issue_type, "noise_config", new=NoiseConfig(noise_limit, timedelta(minutes=1))
660-
),
661-
override_options(
662-
{"performance.issues.all.problem-detection": 1.0, detector_option: 1.0}
663-
),
649+
with mock.patch(
650+
"sentry.issues.ingest.send_issue_occurrence_to_eventstream",
651+
side_effect=send_issue_occurrence_to_eventstream,
652+
) as mock_eventstream, mock.patch(
653+
"sentry.event_manager.detect_performance_problems",
654+
side_effect=detect_performance_problems_interceptor,
655+
), mock.patch.object(
656+
issue_type, "noise_config", new=NoiseConfig(noise_limit, timedelta(minutes=1))
657+
), override_options(
658+
{"performance.issues.all.problem-detection": 1.0, detector_option: 1.0}
664659
):
665660
event = perf_event_manager.save(project_id)
666661
if mock_eventstream.call_args:
@@ -2856,15 +2851,14 @@ def responses_context(self):
28562851
def assert_performance_issue_attachments(
28572852
self, attachment, project_slug, referrer, alert_type="workflow"
28582853
):
2859-
assert "N+1 Query" in attachment["text"]
2854+
assert attachment["title"] == "N+1 Query"
28602855
assert (
2861-
"db - SELECT `books_author`.`id`, `books_author`.`name` FROM `books_author` WHERE `books_author`.`id` = %s LIMIT 21"
2862-
in attachment["blocks"][1]["text"]["text"]
2856+
attachment["text"]
2857+
== "db - SELECT `books_author`.`id`, `books_author`.`name` FROM `books_author` WHERE `books_author`.`id` = %s LIMIT 21"
28632858
)
2864-
title_link = attachment["blocks"][0]["text"]["text"][13:][1:-1]
2865-
notification_uuid = self.get_notification_uuid(title_link)
2859+
notification_uuid = self.get_notification_uuid(attachment["title_link"])
28662860
assert (
2867-
attachment["blocks"][-2]["elements"][0]["text"]
2861+
attachment["footer"]
28682862
== f"{project_slug} | production | <http://testserver/settings/account/notifications/{alert_type}/?referrer={referrer}&notification_uuid={notification_uuid}|Notification Settings>"
28692863
)
28702864

tests/sentry/integrations/slack/notifications/test_assigned.py

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
from sentry.testutils.cases import PerformanceIssueTestCase, SlackActivityNotificationTest
99
from sentry.testutils.helpers.features import with_feature
1010
from sentry.testutils.helpers.notifications import TEST_ISSUE_OCCURRENCE, TEST_PERF_ISSUE_OCCURRENCE
11-
from sentry.testutils.helpers.slack import get_blocks_and_fallback_text
11+
from sentry.testutils.helpers.slack import get_attachment, get_blocks_and_fallback_text
1212
from sentry.testutils.skips import requires_snuba
1313
from sentry.types.activity import ActivityType
1414
from sentry.types.integrations import ExternalProviders
@@ -111,6 +111,22 @@ def test_multiple_orgs(self):
111111
channel = data["channel"][0]
112112
assert channel == self.identity.external_id
113113

114+
@responses.activate
115+
def test_assignment(self):
116+
"""
117+
Test that a Slack message is sent with the expected payload when an issue is assigned
118+
"""
119+
with self.tasks():
120+
self.create_notification(self.group, AssignedActivityNotification).send()
121+
attachment, text = get_attachment()
122+
assert text == f"Issue assigned to {self.name} by themselves"
123+
assert attachment["title"] == self.group.title
124+
notification_uuid = self.get_notification_uuid(attachment["title_link"])
125+
assert (
126+
attachment["footer"]
127+
== f"{self.project.slug} | <http://testserver/settings/account/notifications/workflow/?referrer=assigned_activity-slack-user&notification_uuid={notification_uuid}|Notification Settings>"
128+
)
129+
114130
@responses.activate
115131
@with_feature("organizations:slack-block-kit")
116132
def test_assignment_block(self):
@@ -133,6 +149,29 @@ def test_assignment_block(self):
133149
== f"{self.project.slug} | <http://testserver/settings/account/notifications/workflow/?referrer=assigned_activity-slack-user&notification_uuid={notification_uuid}|Notification Settings>"
134150
)
135151

152+
@responses.activate
153+
@mock.patch(
154+
"sentry.eventstore.models.GroupEvent.occurrence",
155+
return_value=TEST_ISSUE_OCCURRENCE,
156+
new_callable=mock.PropertyMock,
157+
)
158+
def test_assignment_generic_issue(self, occurrence):
159+
"""
160+
Test that a Slack message is sent with the expected payload when a generic issue type is assigned
161+
"""
162+
event = self.store_event(
163+
data={"message": "Hellboy's world", "level": "error"}, project_id=self.project.id
164+
)
165+
group_event = event.for_group(event.groups[0])
166+
167+
with self.tasks():
168+
self.create_notification(group_event.group, AssignedActivityNotification).send()
169+
attachment, text = get_attachment()
170+
assert text == f"Issue assigned to {self.name} by themselves"
171+
self.assert_generic_issue_attachments(
172+
attachment, self.project.slug, "assigned_activity-slack-user"
173+
)
174+
136175
@responses.activate
137176
@mock.patch(
138177
"sentry.eventstore.models.GroupEvent.occurrence",
@@ -163,6 +202,26 @@ def test_assignment_generic_issue_block(self, occurrence):
163202
"assigned_activity-slack",
164203
)
165204

205+
@responses.activate
206+
@mock.patch(
207+
"sentry.eventstore.models.GroupEvent.occurrence",
208+
return_value=TEST_PERF_ISSUE_OCCURRENCE,
209+
new_callable=mock.PropertyMock,
210+
)
211+
def test_assignment_performance_issue(self, occurrence):
212+
"""
213+
Test that a Slack message is sent with the expected payload when a performance issue is assigned
214+
"""
215+
event = self.create_performance_issue()
216+
with self.tasks():
217+
self.create_notification(event.group, AssignedActivityNotification).send()
218+
219+
attachment, text = get_attachment()
220+
assert text == f"Issue assigned to {self.name} by themselves"
221+
self.assert_performance_issue_attachments(
222+
attachment, self.project.slug, "assigned_activity-slack-user"
223+
)
224+
166225
@responses.activate
167226
@mock.patch(
168227
"sentry.eventstore.models.GroupEvent.occurrence",

0 commit comments

Comments
 (0)