Skip to content

🔧 chore(slos): Update Halt to Success for Bot Commands #81271

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 25, 2024
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
4 changes: 2 additions & 2 deletions src/sentry/integrations/discord/webhooks/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ def help_handler(self, input: CommandInput) -> IntegrationResponse[str]:
def link_user_handler(self, _: CommandInput) -> IntegrationResponse[str]:
if self.request.has_identity():
return IntegrationResponse(
interaction_result=EventLifecycleOutcome.HALTED,
interaction_result=EventLifecycleOutcome.SUCCESS,
response=ALREADY_LINKED_MESSAGE.format(email=self.request.get_identity_str()),
outcome_reason=str(MessageCommandHaltReason.ALREADY_LINKED),
context_data={
Expand Down Expand Up @@ -120,7 +120,7 @@ def link_user_handler(self, _: CommandInput) -> IntegrationResponse[str]:
def unlink_user_handler(self, input: CommandInput) -> IntegrationResponse[str]:
if not self.request.has_identity():
return IntegrationResponse(
interaction_result=EventLifecycleOutcome.HALTED,
interaction_result=EventLifecycleOutcome.SUCCESS,
response=NOT_LINKED_MESSAGE,
outcome_reason=str(MessageCommandHaltReason.NOT_LINKED),
)
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/integrations/msteams/webhook.py
Original file line number Diff line number Diff line change
Expand Up @@ -669,7 +669,7 @@ def link_user_handler(self, input: CommandInput) -> IntegrationResponse[Adaptive

if has_linked_identity:
return IntegrationResponse(
interaction_result=EventLifecycleOutcome.HALTED,
interaction_result=EventLifecycleOutcome.SUCCESS,
response=build_already_linked_identity_command_card(),
outcome_reason=str(MessageCommandHaltReason.ALREADY_LINKED),
context_data={
Expand Down
8 changes: 4 additions & 4 deletions src/sentry/integrations/slack/webhooks/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ def link_user_handler(self, input: CommandInput) -> IntegrationResponse[Response
response = self.endpoint.link_user(self.request)
if ALREADY_LINKED_MESSAGE.format(username=self.request.identity_str) in str(response.data):
return IntegrationResponse(
interaction_result=EventLifecycleOutcome.HALTED,
interaction_result=EventLifecycleOutcome.SUCCESS,
response=response,
outcome_reason=str(MessageCommandHaltReason.ALREADY_LINKED),
context_data={
Expand All @@ -182,7 +182,7 @@ def unlink_user_handler(self, input: CommandInput) -> IntegrationResponse[Respon
response = self.endpoint.unlink_user(self.request)
if NOT_LINKED_MESSAGE in str(response.data):
return IntegrationResponse(
interaction_result=EventLifecycleOutcome.HALTED,
interaction_result=EventLifecycleOutcome.SUCCESS,
response=response,
outcome_reason=str(MessageCommandHaltReason.NOT_LINKED),
context_data={
Expand All @@ -200,7 +200,7 @@ def link_team_handler(self, input: CommandInput) -> IntegrationResponse[Response
for message, reason in self.TEAM_HALT_MAPPINGS.items():
if message in str(response.data):
return IntegrationResponse(
interaction_result=EventLifecycleOutcome.HALTED,
interaction_result=EventLifecycleOutcome.SUCCESS,
response=response,
outcome_reason=str(reason),
)
Expand All @@ -215,7 +215,7 @@ def unlink_team_handler(self, input: CommandInput) -> IntegrationResponse[Respon
for message, reason in self.TEAM_HALT_MAPPINGS.items():
if message in str(response.data):
return IntegrationResponse(
interaction_result=EventLifecycleOutcome.HALTED,
interaction_result=EventLifecycleOutcome.SUCCESS,
response=response,
outcome_reason=str(reason),
)
Expand Down
20 changes: 6 additions & 14 deletions tests/sentry/integrations/discord/webhooks/test_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,10 @@
from sentry.integrations.discord.requests.base import DiscordRequestTypes
from sentry.integrations.discord.webhooks.command import HELP_MESSAGE, NOT_LINKED_MESSAGE
from sentry.integrations.discord.webhooks.types import DiscordResponseTypes
from sentry.integrations.messaging.metrics import (
MessageCommandFailureReason,
MessageCommandHaltReason,
)
from sentry.integrations.messaging.metrics import MessageCommandFailureReason
from sentry.integrations.types import EventLifecycleOutcome
from sentry.testutils.cases import APITestCase
from tests.sentry.integrations.utils.test_assert_metrics import (
assert_failure_metric,
assert_halt_metric,
)
from tests.sentry.integrations.utils.test_assert_metrics import assert_failure_metric

WEBHOOK_URL = "/extensions/discord/interactions/"

Expand Down Expand Up @@ -211,10 +205,9 @@ def test_link_already_linked(self, mock_record):
assert data["data"]["flags"] == EPHEMERAL_FLAG
assert response.status_code == 200

start, halt = mock_record.mock_calls
start, success = mock_record.mock_calls
assert start.args[0] == EventLifecycleOutcome.STARTED
assert halt.args[0] == EventLifecycleOutcome.HALTED
assert_halt_metric(mock_record, MessageCommandHaltReason.ALREADY_LINKED.value)
assert success.args[0] == EventLifecycleOutcome.SUCCESS

@mock.patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
def test_unlink_no_identity(self, mock_record):
Expand All @@ -239,10 +232,9 @@ def test_unlink_no_identity(self, mock_record):
assert data["data"]["flags"] == EPHEMERAL_FLAG
assert response.status_code == 200

start, halt = mock_record.mock_calls
start, success = mock_record.mock_calls
assert start.args[0] == EventLifecycleOutcome.STARTED
assert halt.args[0] == EventLifecycleOutcome.HALTED
assert_halt_metric(mock_record, MessageCommandHaltReason.NOT_LINKED.value)
assert success.args[0] == EventLifecycleOutcome.SUCCESS

@mock.patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
def test_unlink(self, mock_record):
Expand Down
7 changes: 2 additions & 5 deletions tests/sentry/integrations/msteams/test_webhook.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
from django.test import override_settings
from django.urls import reverse

from sentry.integrations.messaging.metrics import MessageCommandHaltReason
from sentry.integrations.models.integration import Integration
from sentry.integrations.msteams.utils import ACTION_TYPE
from sentry.integrations.types import EventLifecycleOutcome
Expand All @@ -17,7 +16,6 @@
from sentry.testutils.silo import assume_test_silo_mode
from sentry.users.models.identity import Identity
from sentry.utils import jwt
from tests.sentry.integrations.utils.test_assert_metrics import assert_halt_metric

from .test_helpers import (
DECODED_TOKEN,
Expand Down Expand Up @@ -546,10 +544,9 @@ def test_link_command_already_linked(self, mock_time, mock_decode, mock_record):
)
assert "Bearer my_token" in responses.calls[3].request.headers["Authorization"]

start, halt = mock_record.mock_calls
start, success = mock_record.mock_calls
assert start.args[0] == EventLifecycleOutcome.STARTED
assert halt.args[0] == EventLifecycleOutcome.HALTED
assert_halt_metric(mock_record, MessageCommandHaltReason.ALREADY_LINKED.value)
assert success.args[0] == EventLifecycleOutcome.SUCCESS

@responses.activate
@mock.patch("sentry.utils.jwt.decode")
Expand Down
27 changes: 10 additions & 17 deletions tests/sentry/integrations/slack/webhooks/commands/test_link_team.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import responses
from rest_framework import status

from sentry.integrations.messaging.metrics import MessageCommandHaltReason
from sentry.integrations.slack.webhooks.command import (
CHANNEL_ALREADY_LINKED_MESSAGE,
INSUFFICIENT_ROLE_MESSAGE,
Expand All @@ -18,7 +17,6 @@
from sentry.testutils.helpers.features import with_feature
from sentry.testutils.silo import assume_test_silo_mode
from tests.sentry.integrations.slack.webhooks.commands import SlackCommandsTest
from tests.sentry.integrations.utils.test_assert_metrics import assert_halt_metric

OTHER_SLACK_ID = "UXXXXXXX2"

Expand Down Expand Up @@ -67,10 +65,9 @@ def test_link_another_team_to_channel(self, mock_record):
assert CHANNEL_ALREADY_LINKED_MESSAGE in get_response_text(data)

assert len(mock_record.mock_calls) == 2
start, halt = mock_record.mock_calls
start, success = mock_record.mock_calls
assert start.args[0] == EventLifecycleOutcome.STARTED
assert halt.args[0] == EventLifecycleOutcome.HALTED
assert_halt_metric(mock_record, MessageCommandHaltReason.CHANNEL_ALREADY_LINKED.value)
assert success.args[0] == EventLifecycleOutcome.SUCCESS

@with_feature("organizations:slack-multiple-team-single-channel-linking")
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
Expand Down Expand Up @@ -117,10 +114,9 @@ def test_link_team_from_dm(self, mock_record):
data = orjson.loads(response.content)
assert LINK_FROM_CHANNEL_MESSAGE in get_response_text(data)

start, halt = mock_record.mock_calls
start, success = mock_record.mock_calls
assert start.args[0] == EventLifecycleOutcome.STARTED
assert halt.args[0] == EventLifecycleOutcome.HALTED
assert_halt_metric(mock_record, MessageCommandHaltReason.LINK_FROM_CHANNEL.value)
assert success.args[0] == EventLifecycleOutcome.SUCCESS

@responses.activate
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
Expand All @@ -134,10 +130,9 @@ def test_link_team_identity_does_not_exist(self, mock_record):
data = self.send_slack_message("link team", user_id=OTHER_SLACK_ID)
assert LINK_USER_FIRST_MESSAGE in get_response_text(data)

start, halt = mock_record.mock_calls
start, success = mock_record.mock_calls
assert start.args[0] == EventLifecycleOutcome.STARTED
assert halt.args[0] == EventLifecycleOutcome.HALTED
assert_halt_metric(mock_record, MessageCommandHaltReason.LINK_USER_FIRST.value)
assert success.args[0] == EventLifecycleOutcome.SUCCESS

@responses.activate
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
Expand All @@ -156,10 +151,9 @@ def test_link_team_insufficient_role(self, mock_record):
data = self.send_slack_message("link team", user_id=OTHER_SLACK_ID)
assert INSUFFICIENT_ROLE_MESSAGE in get_response_text(data)

start, halt = mock_record.mock_calls
start, success = mock_record.mock_calls
assert start.args[0] == EventLifecycleOutcome.STARTED
assert halt.args[0] == EventLifecycleOutcome.HALTED
assert_halt_metric(mock_record, MessageCommandHaltReason.INSUFFICIENT_ROLE.value)
assert success.args[0] == EventLifecycleOutcome.SUCCESS

@responses.activate
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
Expand Down Expand Up @@ -231,10 +225,9 @@ def test_unlink_no_team(self, mock_record):
)
assert TEAM_NOT_LINKED_MESSAGE in get_response_text(data)

start, halt = mock_record.mock_calls
start, success = mock_record.mock_calls
assert start.args[0] == EventLifecycleOutcome.STARTED
assert halt.args[0] == EventLifecycleOutcome.HALTED
assert_halt_metric(mock_record, MessageCommandHaltReason.TEAM_NOT_LINKED.value)
assert success.args[0] == EventLifecycleOutcome.SUCCESS

@responses.activate
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
from unittest.mock import patch

from sentry.integrations.messaging.metrics import MessageCommandHaltReason
from sentry.integrations.models.organization_integration import OrganizationIntegration
from sentry.integrations.slack.views.link_identity import SUCCESS_LINKED_MESSAGE, build_linking_url
from sentry.integrations.slack.views.unlink_identity import (
Expand All @@ -13,7 +12,6 @@
from sentry.testutils.silo import control_silo_test
from sentry.users.models.identity import Identity
from tests.sentry.integrations.slack.webhooks.commands import SlackCommandsTest
from tests.sentry.integrations.utils.test_assert_metrics import assert_halt_metric


@control_silo_test
Expand Down Expand Up @@ -51,10 +49,9 @@ def test_link_command_already_linked(self, mock_record):
data = self.send_slack_message("link")
assert "You are already linked as" in get_response_text(data)

start, halt = mock_record.mock_calls
start, success = mock_record.mock_calls
assert start.args[0] == EventLifecycleOutcome.STARTED
assert halt.args[0] == EventLifecycleOutcome.HALTED
assert_halt_metric(mock_record, MessageCommandHaltReason.ALREADY_LINKED.value)
assert success.args[0] == EventLifecycleOutcome.SUCCESS


@control_silo_test
Expand Down Expand Up @@ -134,7 +131,6 @@ def test_unlink_command_already_unlinked(self, mock_record):
data = self.send_slack_message("unlink")
assert NOT_LINKED_MESSAGE in get_response_text(data)

start, halt = mock_record.mock_calls
start, success = mock_record.mock_calls
assert start.args[0] == EventLifecycleOutcome.STARTED
assert halt.args[0] == EventLifecycleOutcome.HALTED
assert_halt_metric(mock_record, MessageCommandHaltReason.NOT_LINKED.value)
assert success.args[0] == EventLifecycleOutcome.SUCCESS
Loading