diff --git a/src/sentry/integrations/msteams/card_builder/block.py b/src/sentry/integrations/msteams/card_builder/block.py index 6b1715e44cbf..a585126677be 100644 --- a/src/sentry/integrations/msteams/card_builder/block.py +++ b/src/sentry/integrations/msteams/card_builder/block.py @@ -137,11 +137,11 @@ class ContainerBlock(TypedDict): class InputChoice(TypedDict): title: str - value: object + value: str class _InputChoiceSetBlockNotRequired(TypedDict, total=False): - value: object + value: str class InputChoiceSetBlock(_InputChoiceSetBlockNotRequired): @@ -240,7 +240,7 @@ def create_container_block(*items: Block) -> ContainerBlock: def create_input_choice_set_block( - id: str, choices: Sequence[tuple[str, object]], default_choice: object + id: str, choices: Sequence[tuple[str, str]], default_choice: str | None ) -> InputChoiceSetBlock: default_choice_arg: _InputChoiceSetBlockNotRequired default_choice_arg = {"value": default_choice} if default_choice else {} diff --git a/src/sentry/integrations/msteams/card_builder/issues.py b/src/sentry/integrations/msteams/card_builder/issues.py index 8f4b18963945..74fa2c9421e5 100644 --- a/src/sentry/integrations/msteams/card_builder/issues.py +++ b/src/sentry/integrations/msteams/card_builder/issues.py @@ -56,7 +56,7 @@ class MSTeamsIssueMessageBuilder(MSTeamsMessageBuilder): def __init__( self, group: Group, - event: Event | GroupEvent, + event: Event | GroupEvent | None, rules: Sequence[Rule], integration: RpcIntegration, ): @@ -67,12 +67,11 @@ def __init__( def generate_action_payload(self, action_type: ACTION_TYPE) -> Any: # we need nested data or else Teams won't handle the payload correctly - assert self.event.group is not None return { "payload": { "actionType": action_type, - "groupId": self.event.group.id, - "eventId": self.event.event_id, + "groupId": self.group.id, + "eventId": self.event.event_id if self.event else None, "rules": [rule.id for rule in self.rules], "integrationId": self.integration.id, } @@ -156,8 +155,8 @@ def build_input_choice_card( card_title: str, input_id: str, submit_button_title: str, - choices: Sequence[tuple[str, Any]], - default_choice: Any = None, + choices: Sequence[tuple[str, str]], + default_choice: str | None = None, ) -> AdaptiveCard: return MSTeamsMessageBuilder().build( title=create_text_block(card_title, weight=TextWeight.BOLDER), diff --git a/src/sentry/integrations/msteams/card_builder/utils.py b/src/sentry/integrations/msteams/card_builder/utils.py index 8e7dd53662e4..e02b6a8a63d4 100644 --- a/src/sentry/integrations/msteams/card_builder/utils.py +++ b/src/sentry/integrations/msteams/card_builder/utils.py @@ -100,12 +100,12 @@ class IssueConstants: ARCHIVE_INPUT_ID = "archiveInput" ARCHIVE_INPUT_TITLE = "Archive until this happens again..." ARCHIVE_INPUT_CHOICES = [ - ("Archive forever", -1), - ("1 time", 1), - ("10 times", 10), - ("100 times", 100), - ("1,000 times", 1000), - ("10,000 times", 10000), + ("Archive forever", "-1"), + ("1 time", "1"), + ("10 times", "10"), + ("100 times", "100"), + ("1,000 times", "1000"), + ("10,000 times", "10000"), ] UNARCHIVE = "Unarchive" diff --git a/src/sentry/integrations/msteams/webhook.py b/src/sentry/integrations/msteams/webhook.py index 5d003cd0269e..9fb9ad83bbe8 100644 --- a/src/sentry/integrations/msteams/webhook.py +++ b/src/sentry/integrations/msteams/webhook.py @@ -634,19 +634,22 @@ def _handle_action_submitted(self, request: Request) -> Response: rules = tuple(Rule.objects.filter(id__in=payload["rules"])) # pull the event based off our payload - event = eventstore.backend.get_event_by_id(group.project_id, payload["eventId"]) - if event is None: - logger.info( - "msteams.action.event-missing", - extra={ - "team_id": team_id, - "integration_id": integration.id, - "organization_id": group.organization.id, - "event_id": payload["eventId"], - "project_id": group.project_id, - }, - ) - return self.respond(status=404) + event = None + event_id = payload.get("eventId") + if event_id: + event = eventstore.backend.get_event_by_id(group.project_id, event_id) + if event is None: + logger.info( + "msteams.action.event-missing", + extra={ + "team_id": team_id, + "integration_id": integration.id, + "organization_id": group.organization.id, + "event_id": event_id, + "project_id": group.project_id, + }, + ) + return self.respond(status=404) # refresh issue and update card group.refresh_from_db() diff --git a/src/sentry/notifications/platform/msteams/renderers/issue.py b/src/sentry/notifications/platform/msteams/renderers/issue.py index 37192e45fb98..8793631adda2 100644 --- a/src/sentry/notifications/platform/msteams/renderers/issue.py +++ b/src/sentry/notifications/platform/msteams/renderers/issue.py @@ -2,11 +2,11 @@ from collections.abc import Sequence from datetime import datetime -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any from sentry import eventstore from sentry.integrations.types import IntegrationProviderSlug -from sentry.models.group import Group +from sentry.models.group import Group, GroupStatus from sentry.models.project import Project from sentry.models.rule import Rule from sentry.notifications.platform.msteams.provider import MSTeamsRenderable @@ -28,8 +28,10 @@ Action, Block, ColumnSetBlock, + ContainerBlock, TextBlock, ) + from sentry.integrations.msteams.utils import ACTION_TYPE @renderer_registry.register(NotificationProviderKey.MSTEAMS, sources=[NotificationSource.ISSUE]) @@ -70,12 +72,12 @@ def render[DataT: NotificationData]( cls.build_description(group=group, event=event), cls.build_footer(group=group, event=event, rules=rules), cls.build_assignee_note(group), + cls.build_actions(group=group, data=data, rules=rules), ] return MSTeamsMessageBuilder().build( title=cls.build_title(group=group, issue_url=issue_url), fields=fields, - actions=cls.build_actions(issue_url=issue_url), ) @classmethod @@ -183,7 +185,130 @@ def build_assignee_note(cls, group: Group) -> TextBlock | None: return None @classmethod - def build_actions(cls, *, issue_url: str) -> list[Action]: - from sentry.integrations.msteams.card_builder.block import ActionType, OpenUrlAction + def build_action_payload( + cls, *, action_type: ACTION_TYPE, data: IssueNotificationData, rules: Sequence[Rule] + ) -> dict[str, Any]: + # Teams posts this back to the webhook when the action is used, and only handles it + # correctly when the contents are nested under a `payload` key. + return { + "payload": { + "actionType": action_type, + "groupId": data.group_id, + "eventId": data.event_id, + "rules": [rule.id for rule in rules], + } + } - return [OpenUrlAction(type=ActionType.OPEN_URL, title="View Issue", url=issue_url)] + @classmethod + def build_action( + cls, + *, + toggled: bool, + action: ACTION_TYPE, + action_title: str, + reverse_action: ACTION_TYPE, + reverse_action_title: str, + data: IssueNotificationData, + rules: Sequence[Rule], + **card_kwargs: Any, + ) -> Action: + """ + Build the action for a state the issue is not currently in. An issue which is not resolved + gets a Resolve button revealing a card of options, and a resolved one gets an Unresolve + button which submits directly. + """ + from sentry.integrations.msteams.card_builder.block import ( + ActionType, + ShowCardAction, + SubmitAction, + ) + from sentry.integrations.msteams.card_builder.issues import MSTeamsIssueMessageBuilder + + if toggled: + return SubmitAction( + type=ActionType.SUBMIT, + title=reverse_action_title, + data=cls.build_action_payload(action_type=reverse_action, data=data, rules=rules), + ) + + card = MSTeamsIssueMessageBuilder.build_input_choice_card( + data=cls.build_action_payload(action_type=action, data=data, rules=rules), + **card_kwargs, + ) + return ShowCardAction(type=ActionType.SHOW_CARD, title=action_title, card=card) + + @classmethod + def build_assignee_choices(cls, group: Group) -> Sequence[tuple[str, str]]: + from sentry.integrations.messaging.message_builder import format_actor_options_non_slack + from sentry.integrations.msteams.card_builder import ME + + teams = group.project.teams.all().order_by("slug") + return [("Me", ME)] + [ + (team["text"], team["value"]) for team in format_actor_options_non_slack(teams) + ] + + @classmethod + def build_actions( + cls, *, group: Group, data: IssueNotificationData, rules: Sequence[Rule] + ) -> ContainerBlock: + from sentry.integrations.msteams.card_builder import ME + from sentry.integrations.msteams.card_builder.block import ( + create_action_set_block, + create_container_block, + ) + from sentry.integrations.msteams.card_builder.utils import IssueConstants + from sentry.integrations.msteams.utils import ACTION_TYPE + + status = group.get_status() + + resolve_action = cls.build_action( + toggled=GroupStatus.RESOLVED == status, + action=ACTION_TYPE.RESOLVE, + action_title=IssueConstants.RESOLVE, + reverse_action=ACTION_TYPE.UNRESOLVE, + reverse_action_title=IssueConstants.UNRESOLVE, + data=data, + rules=rules, + card_title=IssueConstants.RESOLVE, + submit_button_title=IssueConstants.RESOLVE, + input_id=IssueConstants.RESOLVE_INPUT_ID, + choices=IssueConstants.RESOLVE_INPUT_CHOICES, + ) + + archive_action = cls.build_action( + toggled=GroupStatus.IGNORED == status, + action=ACTION_TYPE.ARCHIVE, + action_title=IssueConstants.ARCHIVE, + reverse_action=ACTION_TYPE.UNRESOLVE, + reverse_action_title=IssueConstants.UNARCHIVE, + data=data, + rules=rules, + card_title=IssueConstants.ARCHIVE_INPUT_TITLE, + submit_button_title=IssueConstants.ARCHIVE, + input_id=IssueConstants.ARCHIVE_INPUT_ID, + choices=IssueConstants.ARCHIVE_INPUT_CHOICES, + ) + + try: + assignee = group.get_assignee() + except Actor.InvalidActor: + assignee = None + + assign_action = cls.build_action( + toggled=assignee is not None, + action=ACTION_TYPE.ASSIGN, + action_title=IssueConstants.ASSIGN, + reverse_action=ACTION_TYPE.UNASSIGN, + reverse_action_title=IssueConstants.UNASSIGN, + data=data, + rules=rules, + card_title=IssueConstants.ASSIGN_INPUT_TITLE, + submit_button_title=IssueConstants.ASSIGN, + input_id=IssueConstants.ASSIGN_INPUT_ID, + choices=cls.build_assignee_choices(group), + default_choice=ME, + ) + + return create_container_block( + create_action_set_block(resolve_action, archive_action, assign_action) + ) diff --git a/tests/sentry/integrations/msteams/test_action_state_change.py b/tests/sentry/integrations/msteams/test_action_state_change.py index 71150a84d013..fa316e99b453 100644 --- a/tests/sentry/integrations/msteams/test_action_state_change.py +++ b/tests/sentry/integrations/msteams/test_action_state_change.py @@ -86,6 +86,7 @@ def post_webhook( archive_input: str | None = None, assign_input: str | None = None, include_integration_id: bool = True, + include_event_id: bool = True, ) -> Response: replyToId = "12345" @@ -106,7 +107,7 @@ def post_webhook( action_payload: dict[str, Any] = { "groupId": group_id or self.group1.id, - "eventId": self.event1.event_id, + "eventId": self.event1.event_id if include_event_id else None, "actionType": action_type, "rules": [], } @@ -441,6 +442,20 @@ def test_resolve_issue_without_integration_id_in_personal_chat(self, verify: Mag assert resp.status_code == 200, resp.content assert self.group1.get_status() == GroupStatus.RESOLVED + @responses.activate + @patch("sentry.integrations.msteams.webhook.verify_signature", return_value=True) + def test_resolve_issue_without_event_id(self, verify: MagicMock) -> None: + resp = self.post_webhook( + action_type=ACTION_TYPE.RESOLVE, + resolve_input="resolved", + include_event_id=False, + ) + self.group1 = Group.objects.get(id=self.group1.id) + + assert resp.status_code == 200, resp.content + assert self.group1.get_status() == GroupStatus.RESOLVED + assert b"Unresolve" in responses.calls[0].request.body + @responses.activate @patch("sentry.integrations.msteams.webhook.verify_signature", return_value=True) def test_no_resolve_input(self, verify: MagicMock) -> None: diff --git a/tests/sentry/notifications/platform/msteams/renderers/test_issue.py b/tests/sentry/notifications/platform/msteams/renderers/test_issue.py index fab1d6b804bc..cbc23963dc44 100644 --- a/tests/sentry/notifications/platform/msteams/renderers/test_issue.py +++ b/tests/sentry/notifications/platform/msteams/renderers/test_issue.py @@ -1,13 +1,17 @@ from __future__ import annotations -from typing import Any +from typing import Any, cast from unittest.mock import MagicMock, patch import pytest from sentry import eventstore -from sentry.integrations.messaging.message_builder import build_attachment_title, build_footer -from sentry.integrations.msteams.card_builder import MSTEAMS_URL_FORMAT +from sentry.integrations.messaging.message_builder import ( + build_attachment_title, + build_footer, + format_actor_options_non_slack, +) +from sentry.integrations.msteams.card_builder import ME, MSTEAMS_URL_FORMAT from sentry.integrations.msteams.card_builder.base import MSTeamsMessageBuilder from sentry.integrations.msteams.card_builder.block import ( Action, @@ -15,19 +19,24 @@ AdaptiveCard, Block, ContentAlignment, - OpenUrlAction, + ShowCardAction, + SubmitAction, TextSize, TextWeight, + create_action_set_block, create_column_block, create_column_set_block, + create_container_block, create_footer_column_block, create_footer_logo_block, create_footer_text_block, create_text_block, ) +from sentry.integrations.msteams.card_builder.issues import MSTeamsIssueMessageBuilder from sentry.integrations.msteams.card_builder.utils import IssueConstants +from sentry.integrations.msteams.utils import ACTION_TYPE from sentry.issues.issue_occurrence import IssueEvidence, IssueOccurrence -from sentry.models.group import Group +from sentry.models.group import Group, GroupStatus from sentry.models.groupassignee import GroupAssignee from sentry.models.project import Project from sentry.notifications.platform.msteams.provider import ( @@ -133,6 +142,70 @@ def _build_expected_card( ), ) + def payload(action_type: ACTION_TYPE) -> dict[str, Any]: + return { + "payload": { + "actionType": action_type, + "groupId": group.id, + "eventId": event.event_id, + "rules": [1], + } + } + + teams = group.project.teams.all().order_by("slug") + assignee_choices = [("Me", ME)] + [ + (team["text"], team["value"]) for team in format_actor_options_non_slack(teams) + ] + + assign_action: Action + if assignee: + assign_action = SubmitAction( + type=ActionType.SUBMIT, + title=IssueConstants.UNASSIGN, + data=payload(ACTION_TYPE.UNASSIGN), + ) + else: + assign_action = ShowCardAction( + type=ActionType.SHOW_CARD, + title=IssueConstants.ASSIGN, + card=MSTeamsIssueMessageBuilder.build_input_choice_card( + data=payload(ACTION_TYPE.ASSIGN), + card_title=IssueConstants.ASSIGN_INPUT_TITLE, + submit_button_title=IssueConstants.ASSIGN, + input_id=IssueConstants.ASSIGN_INPUT_ID, + choices=assignee_choices, + default_choice=ME, + ), + ) + + actions = create_container_block( + create_action_set_block( + ShowCardAction( + type=ActionType.SHOW_CARD, + title=IssueConstants.RESOLVE, + card=MSTeamsIssueMessageBuilder.build_input_choice_card( + data=payload(ACTION_TYPE.RESOLVE), + card_title=IssueConstants.RESOLVE, + submit_button_title=IssueConstants.RESOLVE, + input_id=IssueConstants.RESOLVE_INPUT_ID, + choices=IssueConstants.RESOLVE_INPUT_CHOICES, + ), + ), + ShowCardAction( + type=ActionType.SHOW_CARD, + title=IssueConstants.ARCHIVE, + card=MSTeamsIssueMessageBuilder.build_input_choice_card( + data=payload(ACTION_TYPE.ARCHIVE), + card_title=IssueConstants.ARCHIVE_INPUT_TITLE, + submit_button_title=IssueConstants.ARCHIVE, + input_id=IssueConstants.ARCHIVE_INPUT_ID, + choices=IssueConstants.ARCHIVE_INPUT_CHOICES, + ), + ), + assign_action, + ) + ) + fields: list[Block | None] = [] if description: fields.append( @@ -145,12 +218,9 @@ def _build_expected_card( IssueConstants.ASSIGNEE_NOTE.format(assignee=assignee), size=TextSize.SMALL ) ) + fields.append(actions) - actions: list[Action] = [ - OpenUrlAction(type=ActionType.OPEN_URL, title="View Issue", url=issue_url) - ] - - return MSTeamsMessageBuilder().build(title=title, fields=fields, actions=actions) + return MSTeamsMessageBuilder().build(title=title, fields=fields) def test_render_raises_on_invalid_data(self) -> None: from sentry.notifications.platform.templates.seer import SeerAutofixError @@ -175,6 +245,61 @@ def test_render_produces_card(self) -> None: assert result == self._build_expected_card(group=group, event=event) + def _card_actions(self, card: AdaptiveCard) -> list[Any]: + container = cast(Any, card["body"][-1]) + return cast(list[Any], container["items"][0]["actions"]) + + def test_render_offers_reverse_actions_for_resolved_issue(self) -> None: + data, _, group = self._create_data() + group.update(status=GroupStatus.RESOLVED, substatus=None) + + result = IssueMSTeamsRenderer.render( + data=data, + rendered_template=NotificationRenderedTemplate(subject="Issue Alert", body=[]), + ) + + actions = self._card_actions(result) + assert [action["title"] for action in actions] == [ + IssueConstants.UNRESOLVE, + IssueConstants.ARCHIVE, + IssueConstants.ASSIGN, + ] + + resolve_action = actions[0] + assert resolve_action["type"] == ActionType.SUBMIT + assert resolve_action["data"]["payload"]["actionType"] == ACTION_TYPE.UNRESOLVE + assert "integrationId" not in resolve_action["data"]["payload"] + + def test_render_offers_unassign_for_assigned_issue(self) -> None: + data, _, group = self._create_data() + GroupAssignee.objects.assign(group, self.user) + + result = IssueMSTeamsRenderer.render( + data=data, + rendered_template=NotificationRenderedTemplate(subject="Issue Alert", body=[]), + ) + + assign_action = self._card_actions(result)[-1] + assert assign_action["type"] == ActionType.SUBMIT + assert assign_action["title"] == IssueConstants.UNASSIGN + assert assign_action["data"]["payload"]["actionType"] == ACTION_TYPE.UNASSIGN + + def test_render_choice_values_are_strings(self) -> None: + # Adaptive Cards requires Input.ChoiceSet values to be strings, and Teams returns + # them as strings regardless of what was sent. + data, _, _ = self._create_data() + + result = IssueMSTeamsRenderer.render( + data=data, + rendered_template=NotificationRenderedTemplate(subject="Issue Alert", body=[]), + ) + + for action in self._card_actions(result): + for block in action["card"]["body"]: + if block.get("type") != "Input.ChoiceSet": + continue + assert all(isinstance(choice["value"], str) for choice in block["choices"]) + def test_render_with_tags(self) -> None: data, event, group = self._create_data( tags=["level"], diff --git a/tests/sentry/notifications/platform/msteams/renderers/test_issue_parity.py b/tests/sentry/notifications/platform/msteams/renderers/test_issue_parity.py new file mode 100644 index 000000000000..abb0e75bbf87 --- /dev/null +++ b/tests/sentry/notifications/platform/msteams/renderers/test_issue_parity.py @@ -0,0 +1,98 @@ +from __future__ import annotations + +from typing import Any + +from sentry.integrations.msteams.card_builder.block import AdaptiveCard +from sentry.integrations.msteams.card_builder.issues import MSTeamsIssueMessageBuilder +from sentry.integrations.services.integration import integration_service +from sentry.models.group import GroupStatus +from sentry.models.groupassignee import GroupAssignee +from sentry.notifications.platform.msteams.renderers.issue import IssueMSTeamsRenderer +from sentry.notifications.platform.templates.issue import ( + IssueNotificationData, + SerializableRuleProxy, +) +from sentry.notifications.platform.types import NotificationRenderedTemplate +from sentry.testutils.cases import TestCase + + +def without_integration_id(card: Any) -> Any: + """ + Strip `integrationId` from a card so the two builders can be compared. The platform renderer + cannot emit it, since it has no access to the target the card is being sent to. + """ + if isinstance(card, dict): + return {k: without_integration_id(v) for k, v in card.items() if k != "integrationId"} + if isinstance(card, list): + return [without_integration_id(v) for v in card] + return card + + +class IssueCardLegacyParityTest(TestCase): + """ + The platform renderer must produce the same card as the legacy issue alert card builder for + every group state, so that the cutover is invisible to users and the webhook can keep + handling action payloads from cards built by either path. + """ + + def setUp(self) -> None: + super().setUp() + self.integration = self.create_integration( + organization=self.organization, + provider="msteams", + name="Fellowship of the Ring", + external_id="f3ll0wsh1p", + metadata={"service_url": "https://smba.trafficmanager.net/amer"}, + ) + self.event = self.store_event( + data={"message": "oh no", "level": "error"}, + project_id=self.project.id, + ) + assert self.event.group is not None + self.issue_group = self.event.group + self.rule = self.create_project_rule(project=self.project, name="Issue Stream") + + def legacy_card(self) -> AdaptiveCard: + rpc_integration = integration_service.get_integration(integration_id=self.integration.id) + assert rpc_integration is not None + return MSTeamsIssueMessageBuilder( + self.issue_group, self.event, [self.rule], rpc_integration + ).build_group_card() + + def platform_card(self) -> AdaptiveCard: + data = IssueNotificationData( + group_id=self.issue_group.id, + event_id=self.event.event_id, + notification_uuid="", + rule=SerializableRuleProxy.from_rule(self.rule), + ) + return IssueMSTeamsRenderer.render( + data=data, + rendered_template=NotificationRenderedTemplate(subject="Issue Alert", body=[]), + ) + + def assert_parity(self) -> None: + self.issue_group.refresh_from_db() + assert without_integration_id(self.legacy_card()) == without_integration_id( + self.platform_card() + ) + + def test_parity_for_unresolved_issue(self) -> None: + self.assert_parity() + + def test_parity_for_resolved_issue(self) -> None: + self.issue_group.update(status=GroupStatus.RESOLVED, substatus=None) + self.assert_parity() + + def test_parity_for_archived_issue(self) -> None: + self.issue_group.update(status=GroupStatus.IGNORED, substatus=None) + self.assert_parity() + + def test_parity_for_assigned_issue(self) -> None: + GroupAssignee.objects.assign(self.issue_group, self.user) + self.assert_parity() + + def test_parity_for_issue_with_teams(self) -> None: + self.create_team(organization=self.organization, slug="micro-team") + self.project.add_team(self.create_team(organization=self.organization, slug="rivendell")) + self.assert_parity()