From 2950de8e9e43f5a2c3ca1aa201b62b7eed5c0462 Mon Sep 17 00:00:00 2001 From: Raj Joshi Date: Tue, 16 Jul 2024 15:17:46 -0700 Subject: [PATCH] chore(slack): Update Tests for `get_channel_id` (#73548) I GA'ed the feature, so I am updating the tests here. --- .../integrations/slack/utils/channel.py | 120 +-------- .../slack/find_channel_id_for_rule.py | 23 +- .../test_notification_actions_details.py | 59 +++-- .../test_notification_actions_index.py | 25 +- .../incidents/action_handlers/test_slack.py | 17 ++ .../test_organization_alert_rule_details.py | 2 +- .../test_organization_alert_rule_index.py | 2 +- .../incidents/endpoints/test_serializers.py | 51 ++-- tests/sentry/incidents/test_logic.py | 250 +++++++++--------- .../integrations/slack/test_notify_action.py | 176 +++++------- tests/sentry/integrations/slack/test_tasks.py | 121 ++------- tests/sentry/integrations/slack/test_utils.py | 120 --------- .../slack/utils/test_mock_slack_response.py | 23 ++ 13 files changed, 321 insertions(+), 668 deletions(-) create mode 100644 tests/sentry/integrations/slack/utils/test_mock_slack_response.py diff --git a/src/sentry/integrations/slack/utils/channel.py b/src/sentry/integrations/slack/utils/channel.py index bb7e21f1817c53..56a36560855fcf 100644 --- a/src/sentry/integrations/slack/utils/channel.py +++ b/src/sentry/integrations/slack/utils/channel.py @@ -7,9 +7,7 @@ from django.core.exceptions import ValidationError from slack_sdk.errors import SlackApiError -from sentry import features from sentry.integrations.services.integration import RpcIntegration -from sentry.integrations.slack.client import SlackClient from sentry.integrations.slack.metrics import ( SLACK_UTILS_CHANNEL_FAILURE_DATADOG_METRIC, SLACK_UTILS_CHANNEL_SUCCESS_DATADOG_METRIC, @@ -19,7 +17,6 @@ from sentry.models.integrations.integration import Integration from sentry.models.organization import Organization from sentry.shared_integrations.exceptions import ( - ApiError, ApiRateLimitedError, DuplicateDisplayNameError, IntegrationError, @@ -90,10 +87,7 @@ def get_channel_id( # This means some users are unable to create/update alert rules. To avoid this, we attempt # to find the channel id asynchronously if it takes longer than a certain amount of time, # which I have set as the SLACK_DEFAULT_TIMEOUT - arbitrarily - to 10 seconds. - if features.has("organizations:slack-sdk-get-channel-id", organization): - return get_channel_id_with_timeout(integration, channel_name, timeout) - - return get_channel_id_with_timeout_deprecated(integration, channel_name, timeout) + return get_channel_id_with_timeout(integration, channel_name, timeout) def validate_channel_id(name: str, integration_id: int | None, input_channel_id: str) -> None: @@ -210,6 +204,7 @@ def check_user_with_timeout( for page in get_slack_user_list(integration, organization=None, kwargs=payload) for user in page ) + try: for user in user_generator: # The "name" field is unique (this is the username for users) @@ -325,114 +320,3 @@ def check_for_channel( } _logger.exception("slack.chat_deleteScheduledMessage.error", extra=logger_params) raise - - -def get_channel_id_with_timeout_deprecated( - integration: Integration | RpcIntegration, - name: str | None, - timeout: int, -) -> SlackChannelIdData: - """ - Fetches the internal slack id of a channel using scheduled message. - :param integration: The slack integration - :param name: The name of the channel - :param timeout: Our self-imposed time limit. - :return: a SlackChannelIdData object - """ - - payload = { - "exclude_archived": False, - "exclude_members": True, - "types": "public_channel,private_channel", - } - - time_to_quit = time.time() + timeout - - client = SlackClient(integration_id=integration.id) - id_data: SlackChannelIdData | None = None - found_duplicate = False - prefix = "" - channel_id = None - try: # Check for channel - channel_id = check_for_channel_deprecated(client, name) - prefix = "#" - except ApiError as e: - if str(e) != "channel_not_found": - raise - # Check if user - cursor = "" - while True: - # Slack limits the response of `.list` to 1000 channels - try: - items = client.get("/users.list", params=dict(payload, cursor=cursor, limit=1000)) - except ApiRateLimitedError as e: - _logger.info( - "rule.slack.user_list_rate_limited", - extra={"error": str(e), "integration_id": integration.id}, - ) - raise - except ApiError as e: - _logger.info( - "rule.slack.user_list_other_error", - extra={"error": str(e), "integration_id": integration.id}, - ) - return SlackChannelIdData(prefix, None, False) - - if not isinstance(items, dict): - continue - - for c in items["members"]: - # The "name" field is unique (this is the username for users) - # so we return immediately if we find a match. - # convert to lower case since all names in Slack are lowercase - if name and c["name"].lower() == name.lower(): - prefix = "@" - return SlackChannelIdData(prefix, c["id"], False) - # If we don't get a match on a unique identifier, we look through - # the users' display names, and error if there is a repeat. - profile = c.get("profile") - if profile and profile.get("display_name") == name: - if id_data: - found_duplicate = True - else: - prefix = "@" - id_data = SlackChannelIdData(prefix, c["id"], False) - - cursor = items.get("response_metadata", {}).get("next_cursor", None) - if time.time() > time_to_quit: - return SlackChannelIdData(prefix, None, True) - - if not cursor: - break - - if found_duplicate: - raise DuplicateDisplayNameError(name) - elif id_data: - return id_data - - return SlackChannelIdData(prefix, channel_id, False) - - -def check_for_channel_deprecated(client: SlackClient, name: str) -> str | None: - msg_response = client.post( - "/chat.scheduleMessage", - data={ - "channel": name, - "text": "Sentry is verifying your channel is accessible for sending you alert rule notifications", - "post_at": int(time.time() + 500), - }, - ) - - if "channel" in msg_response: - client.post( - "/chat.deleteScheduledMessage", - params=dict( - { - "channel": msg_response["channel"], - "scheduled_message_id": msg_response["scheduled_message_id"], - } - ), - ) - - return msg_response["channel"] - return None diff --git a/src/sentry/tasks/integrations/slack/find_channel_id_for_rule.py b/src/sentry/tasks/integrations/slack/find_channel_id_for_rule.py index 39fad003d40ecd..1e14f714cd6eee 100644 --- a/src/sentry/tasks/integrations/slack/find_channel_id_for_rule.py +++ b/src/sentry/tasks/integrations/slack/find_channel_id_for_rule.py @@ -2,7 +2,6 @@ from collections.abc import Sequence from typing import Any -from sentry import features from sentry.incidents.models.alert_rule import AlertRuleTriggerAction from sentry.integrations.services.integration import integration_service from sentry.integrations.slack.utils import ( @@ -10,10 +9,7 @@ RedisRuleStatus, strip_channel_name, ) -from sentry.integrations.slack.utils.channel import ( - get_channel_id_with_timeout, - get_channel_id_with_timeout_deprecated, -) +from sentry.integrations.slack.utils.channel import get_channel_id_with_timeout from sentry.mediators.project_rules.creator import Creator from sentry.mediators.project_rules.updater import Updater from sentry.models.project import Project @@ -78,18 +74,11 @@ def find_channel_id_for_rule( # endpoints but need some time limit imposed. 3 minutes should be more than enough time, # we can always update later try: - if features.has("organizations:slack-sdk-get-channel-id", organization): - channel_data = get_channel_id_with_timeout( - integration, - channel_name, - 3 * 60, - ) - else: - channel_data = get_channel_id_with_timeout_deprecated( - integration, - channel_name, - 3 * 60, - ) + channel_data = get_channel_id_with_timeout( + integration, + channel_name, + timeout=3 * 60, + ) except DuplicateDisplayNameError: # if we find a duplicate display name and nothing else, we # want to set the status to failed. This just lets us skip diff --git a/tests/sentry/api/endpoints/notifications/test_notification_actions_details.py b/tests/sentry/api/endpoints/notifications/test_notification_actions_details.py index a66320ee4a7ef4..e7e05636bf1210 100644 --- a/tests/sentry/api/endpoints/notifications/test_notification_actions_details.py +++ b/tests/sentry/api/endpoints/notifications/test_notification_actions_details.py @@ -1,7 +1,5 @@ from unittest.mock import MagicMock, patch -import orjson -import responses from rest_framework import serializers, status from sentry.api.serializers.base import serialize @@ -19,6 +17,7 @@ from sentry.testutils.cases import APITestCase from sentry.testutils.helpers.slack import install_slack from sentry.testutils.silo import assume_test_silo_mode +from tests.sentry.integrations.slack.utils.test_mock_slack_response import mock_slack_response class NotificationActionsDetailsEndpointTest(APITestCase): @@ -50,6 +49,26 @@ def setUp(self): ) self.login_as(user=self.user) + def mock_msg_schedule_response(self, channel_id, result_name="channel"): + if channel_id == "channel_not_found": + body = {"ok": False, "error": "channel_not_found"} + else: + body = { + "ok": True, + result_name: channel_id, + "scheduled_message_id": "Q1298393284", + } + return mock_slack_response("chat_scheduleMessage", body=body) + + def mock_msg_delete_scheduled_response(self, channel_id, result_name="channel"): + if channel_id == "channel_not_found": + body = {"ok": False, "error": "channel_not_found"} + else: + body = { + "ok": True, + } + return mock_slack_response("chat_deleteScheduledMessage", body=body) + def test_requires_organization_access(self): for method in ["GET", "PUT", "DELETE"]: self.get_error_response( @@ -214,7 +233,6 @@ class MockActionRegistration(ActionRegistration): assert error_message in str(response.data) @patch.dict(NotificationAction._registry, {}) - @responses.activate def test_put_with_slack_validation(self): class MockActionRegistration(ActionRegistration): pass @@ -233,31 +251,16 @@ class MockActionRegistration(ActionRegistration): self.mock_register(data)(MockActionRegistration) - responses.add( - method=responses.POST, - url="https://slack.com/api/chat.scheduleMessage", - status=200, - content_type="application/json", - body=orjson.dumps( - {"ok": "true", "channel": channel_id, "scheduled_message_id": "Q1298393284"} - ), - ) - responses.add( - method=responses.POST, - url="https://slack.com/api/chat.deleteScheduledMessage", - status=200, - content_type="application/json", - body=orjson.dumps({"ok": True}), - ) - - response = self.get_success_response( - self.organization.slug, - self.notif_action.id, - status_code=status.HTTP_202_ACCEPTED, - method="PUT", - **data, - ) - assert response.data["targetIdentifier"] == channel_id + with self.mock_msg_schedule_response(channel_id): + with self.mock_msg_delete_scheduled_response(channel_id): + response = self.get_success_response( + self.organization.slug, + self.notif_action.id, + status_code=status.HTTP_202_ACCEPTED, + method="PUT", + **data, + ) + assert response.data["targetIdentifier"] == channel_id @patch.dict(NotificationAction._registry, {}) def test_put_with_pagerduty_validation(self): diff --git a/tests/sentry/api/endpoints/notifications/test_notification_actions_index.py b/tests/sentry/api/endpoints/notifications/test_notification_actions_index.py index 30d5a6b863a81e..0b21313108f58e 100644 --- a/tests/sentry/api/endpoints/notifications/test_notification_actions_index.py +++ b/tests/sentry/api/endpoints/notifications/test_notification_actions_index.py @@ -1,6 +1,5 @@ from unittest.mock import MagicMock, patch -import orjson import responses from rest_framework import serializers, status @@ -19,6 +18,7 @@ from sentry.testutils.cases import APITestCase from sentry.testutils.helpers.slack import install_slack from sentry.testutils.silo import assume_test_silo_mode +from tests.sentry.integrations.slack.utils.test_mock_slack_response import mock_slack_response class NotificationActionsIndexEndpointTest(APITestCase): @@ -263,7 +263,12 @@ class MockActionRegistration(ActionRegistration): @patch.dict(NotificationAction._registry, {}) @responses.activate - def test_post_with_slack_validation(self): + @mock_slack_response( + "chat_scheduleMessage", + body={"ok": True, "channel": "CABC123", "scheduled_message_id": "Q1298393284"}, + ) + @mock_slack_response("chat_deleteScheduledMessage", body={"ok": True}) + def test_post_with_slack_validation(self, mock_delete, mock_schedule): class MockActionRegistration(ActionRegistration): pass @@ -281,22 +286,6 @@ class MockActionRegistration(ActionRegistration): self.mock_register(data)(MockActionRegistration) - responses.add( - method=responses.POST, - url="https://slack.com/api/chat.scheduleMessage", - status=200, - content_type="application/json", - body=orjson.dumps( - {"ok": "true", "channel": channel_id, "scheduled_message_id": "Q1298393284"} - ), - ) - responses.add( - method=responses.POST, - url="https://slack.com/api/chat.deleteScheduledMessage", - status=200, - content_type="application/json", - body=orjson.dumps({"ok": True}), - ) response = self.get_success_response( self.organization.slug, status_code=status.HTTP_201_CREATED, diff --git a/tests/sentry/incidents/action_handlers/test_slack.py b/tests/sentry/incidents/action_handlers/test_slack.py index 1321ab8cf7e036..3fd8a830757611 100644 --- a/tests/sentry/incidents/action_handlers/test_slack.py +++ b/tests/sentry/incidents/action_handlers/test_slack.py @@ -1,6 +1,7 @@ from unittest.mock import patch import orjson +import pytest import responses from sentry.constants import ObjectStatus @@ -17,12 +18,28 @@ from sentry.models.options.organization_option import OrganizationOption from sentry.testutils.helpers.datetime import freeze_time from sentry.utils import json +from tests.sentry.integrations.slack.utils.test_mock_slack_response import mock_slack_response from . import FireTest @freeze_time() class SlackActionHandlerTest(FireTest): + @pytest.fixture(autouse=True) + def mock_chat_postEphemeral(self): + with mock_slack_response( + "chat_scheduleMessage", + body={"ok": True, "channel": "chan-id", "scheduled_message_id": "Q1298393284"}, + ) as self.mock_schedule: + yield + + @pytest.fixture(autouse=True) + def mock_chat_unfurl(self): + with mock_slack_response( + "chat_deleteScheduledMessage", body={"ok": True} + ) as self.mock_delete: + yield + @responses.activate def setUp(self): token = "xoxp-xxxxxxxxx-xxxxxxxxxx-xxxxxxxxxxxx" diff --git a/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py b/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py index 1cded6d0ac47c8..39f79b6e2376b6 100644 --- a/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py +++ b/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py @@ -1049,7 +1049,7 @@ def test_create_slack_alert_with_empty_channel_id( assert resp.data == {"uuid": "abc123"} @patch( - "sentry.integrations.slack.utils.channel.get_channel_id_with_timeout_deprecated", + "sentry.integrations.slack.utils.channel.get_channel_id_with_timeout", side_effect=[ SlackChannelIdData("#", "10", False), SlackChannelIdData("#", "10", False), diff --git a/tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py b/tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py index 5f8167a5633a54..cad146f2a1fb78 100644 --- a/tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py +++ b/tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py @@ -734,7 +734,7 @@ def test_kicks_off_slack_async_job( mock_find_channel_id_for_alert_rule.assert_called_once_with(kwargs=kwargs) @patch( - "sentry.integrations.slack.utils.channel.get_channel_id_with_timeout_deprecated", + "sentry.integrations.slack.utils.channel.get_channel_id_with_timeout", side_effect=[ SlackChannelIdData("#", "10", False), SlackChannelIdData("#", "10", False), diff --git a/tests/sentry/incidents/endpoints/test_serializers.py b/tests/sentry/incidents/endpoints/test_serializers.py index a021f76415360e..1e7afa5a2da24d 100644 --- a/tests/sentry/incidents/endpoints/test_serializers.py +++ b/tests/sentry/incidents/endpoints/test_serializers.py @@ -10,7 +10,6 @@ from rest_framework import serializers from rest_framework.exceptions import ErrorDetail from slack_sdk.errors import SlackApiError -from slack_sdk.web.slack_response import SlackResponse from sentry.auth.access import from_user from sentry.incidents.logic import ( @@ -41,13 +40,13 @@ from sentry.models.environment import Environment from sentry.models.user import User from sentry.sentry_apps.services.app import app_service -from sentry.shared_integrations.exceptions import ApiError from sentry.silo.base import SiloMode from sentry.snuba.dataset import Dataset from sentry.snuba.models import SnubaQuery, SnubaQueryEventType from sentry.testutils.cases import TestCase from sentry.testutils.silo import assume_test_silo_mode from sentry.testutils.skips import requires_snuba +from tests.sentry.integrations.slack.utils.test_mock_slack_response import mock_slack_response pytestmark = [pytest.mark.sentry_metrics, requires_snuba] @@ -485,7 +484,7 @@ def test_invalid_slack_channel(self): serializer = AlertRuleSerializer(context=self.context, data=base_params) assert serializer.is_valid() - with pytest.raises(ApiError): + with pytest.raises(serializers.ValidationError): serializer.save() # Make sure the rule was not created. @@ -533,7 +532,7 @@ def test_threshold_type(self): self.run_fail_validation_test({"thresholdType": 50}, {"thresholdType": invalid_values}) @patch( - "sentry.integrations.slack.utils.channel.get_channel_id_with_timeout_deprecated", + "sentry.integrations.slack.utils.channel.get_channel_id_with_timeout", return_value=SlackChannelIdData("#", None, True), ) def test_channel_timeout(self, mock_get_channel_id): @@ -562,7 +561,7 @@ def test_channel_timeout(self, mock_get_channel_id): ) @patch( - "sentry.integrations.slack.utils.channel.get_channel_id_with_timeout_deprecated", + "sentry.integrations.slack.utils.channel.get_channel_id_with_timeout", return_value=SlackChannelIdData("#", None, True), ) def test_invalid_team_with_channel_timeout(self, mock_get_channel_id): @@ -809,33 +808,26 @@ def test_validation_no_params(self): class TestAlertRuleTriggerActionSerializer(TestAlertRuleSerializerBase): def mock_conversations_list(self, channels): - return patch( - "slack_sdk.web.client.WebClient.conversations_list", - return_value=SlackResponse( - client=None, - http_verb="POST", - api_url="https://slack.com/api/conversations.list", - req_args={}, - data={"ok": True, "channels": channels}, - headers={}, - status_code=200, - ), - ) + return mock_slack_response("conversations_list", body={"ok": True, "channels": channels}) def mock_conversations_info(self, channel): - return patch( - "slack_sdk.web.client.WebClient.conversations_info", - return_value=SlackResponse( - client=None, - http_verb="POST", - api_url="https://slack.com/api/conversations.info", - req_args={"channel": channel}, - data={"ok": True, "channel": channel}, - headers={}, - status_code=200, - ), + return mock_slack_response( + "conversations_info", + body={"ok": True, "channel": channel}, + req_args={"channel": channel}, ) + def patch_msg_schedule_response(self, channel_id, result_name="channel"): + if channel_id == "channel_not_found": + body = {"ok": False, "error": "channel_not_found"} + else: + body = { + "ok": True, + result_name: channel_id, + "scheduled_message_id": "Q1298393284", + } + return mock_slack_response("chat_scheduleMessage", body) + @cached_property def other_project(self): return self.create_project() @@ -1082,9 +1074,10 @@ def test_slack(self): "integration": str(self.integration.id), } ) + serializer = AlertRuleTriggerActionSerializer(context=self.context, data=base_params) assert serializer.is_valid() - with pytest.raises(ApiError): + with pytest.raises(serializers.ValidationError): serializer.save() def test_valid_slack_channel_id_sdk(self): diff --git a/tests/sentry/incidents/test_logic.py b/tests/sentry/incidents/test_logic.py index 57ef17cd06f750..6cb72e8f347a9f 100644 --- a/tests/sentry/incidents/test_logic.py +++ b/tests/sentry/incidents/test_logic.py @@ -3,12 +3,14 @@ from unittest import mock from unittest.mock import patch +import orjson import pytest import responses from django.core import mail from django.forms import ValidationError from django.test import override_settings from django.utils import timezone +from slack_sdk.web.slack_response import SlackResponse from sentry.constants import ObjectStatus from sentry.incidents.events import ( @@ -80,7 +82,7 @@ from sentry.integrations.services.integration.serial import serialize_integration from sentry.models.group import GroupStatus from sentry.models.integrations.organization_integration import OrganizationIntegration -from sentry.shared_integrations.exceptions import ApiError, ApiRateLimitedError, ApiTimeoutError +from sentry.shared_integrations.exceptions import ApiRateLimitedError, ApiTimeoutError from sentry.silo.base import SiloMode from sentry.snuba.dataset import Dataset from sentry.snuba.models import QuerySubscription, SnubaQuery, SnubaQueryEventType @@ -91,7 +93,6 @@ from sentry.testutils.helpers.options import override_options from sentry.testutils.silo import assume_test_silo_mode, assume_test_silo_mode_of from sentry.types.actor import Actor -from sentry.utils import json pytestmark = [pytest.mark.sentry_metrics] @@ -1482,6 +1483,48 @@ def alert_rule(self): def trigger(self): return create_alert_rule_trigger(self.alert_rule, "hello", 1000) + def patch_msg_schedule_response(self, channel_id, result_name="channel"): + if channel_id == "channel_not_found": + bodydict = {"ok": False, "error": "channel_not_found"} + else: + bodydict = { + "ok": True, + result_name: channel_id, + "scheduled_message_id": "Q1298393284", + } + return patch( + "slack_sdk.web.client.WebClient.chat_scheduleMessage", + return_value=SlackResponse( + client=None, + http_verb="POST", + api_url="https://slack.com/api/chat.scheduleMessage", + req_args={}, + data=bodydict, + headers={}, + status_code=200, + ), + ) + + def patch_msg_delete_scheduled_response(self, channel_id): + if channel_id == "channel_not_found": + bodydict = {"ok": False, "error": "channel_not_found"} + else: + bodydict = { + "ok": True, + } + return patch( + "slack_sdk.web.client.WebClient.chat_deleteScheduledMessage", + return_value=SlackResponse( + client=None, + http_verb="POST", + api_url="https://slack.com/api/chat.deleteScheduleMessage", + req_args={}, + data=bodydict, + headers={}, + status_code=200, + ), + ) + class CreateAlertRuleTriggerActionTest(BaseAlertRuleTriggerActionTest, TestCase): def test(self): @@ -1524,36 +1567,22 @@ def test_slack(self): target_type = AlertRuleTriggerAction.TargetType.SPECIFIC channel_name = "#some_channel" channel_id = "s_c" - responses.add( - method=responses.POST, - url="https://slack.com/api/chat.scheduleMessage", - status=200, - content_type="application/json", - body=json.dumps( - {"ok": "true", "channel": channel_id, "scheduled_message_id": "Q1298393284"} - ), - ) - responses.add( - method=responses.POST, - url="https://slack.com/api/chat.deleteScheduledMessage", - status=200, - content_type="application/json", - body=json.dumps({"ok": True}), - ) - action = create_alert_rule_trigger_action( - self.trigger, - type, - target_type, - target_identifier=channel_name, - integration_id=integration.id, - ) - assert action.alert_rule_trigger == self.trigger - assert action.type == type.value - assert action.target_type == target_type.value - assert action.target_identifier == channel_id - assert action.target_display == channel_name - assert action.integration_id == integration.id + with self.patch_msg_schedule_response(channel_id): + with self.patch_msg_delete_scheduled_response(channel_id): + action = create_alert_rule_trigger_action( + self.trigger, + type, + target_type, + target_identifier=channel_name, + integration_id=integration.id, + ) + assert action.alert_rule_trigger == self.trigger + assert action.type == type.value + assert action.target_type == target_type.value + assert action.target_identifier == channel_id + assert action.target_display == channel_name + assert action.integration_id == integration.id def test_slack_not_existing(self): integration, _ = self.create_provider_integration_for( @@ -1566,17 +1595,19 @@ def test_slack_not_existing(self): type = AlertRuleTriggerAction.Type.SLACK target_type = AlertRuleTriggerAction.TargetType.SPECIFIC channel_name = "#some_channel_that_doesnt_exist" - with pytest.raises(ApiError): - create_alert_rule_trigger_action( - self.trigger, - type, - target_type, - target_identifier=channel_name, - integration_id=integration.id, - ) + with self.patch_msg_schedule_response("channel_not_found"): + with pytest.raises(InvalidTriggerActionError): + create_alert_rule_trigger_action( + self.trigger, + type, + target_type, + target_identifier=channel_name, + integration_id=integration.id, + ) @responses.activate - def test_slack_rate_limiting(self): + @patch("slack_sdk.web.client.WebClient._perform_urllib_http_request") + def test_slack_rate_limiting(self, mock_api_call): """Should handle 429 from Slack on new Metric Alert creation""" integration, _ = self.create_provider_integration_for( self.organization, @@ -1592,29 +1623,21 @@ def test_slack_rate_limiting(self): target_type = AlertRuleTriggerAction.TargetType.SPECIFIC channel_name = "#some_channel" - responses.add( - method=responses.POST, - url="https://slack.com/api/chat.scheduleMessage", - status=200, - content_type="application/json", - body=json.dumps({"ok": False, "error": "channel_not_found"}), - ) + mock_api_call.return_value = { + "body": orjson.dumps({"ok": False, "error": "ratelimited"}).decode(), + "headers": {}, + "status": 429, + } - responses.add( - method=responses.GET, - url="https://slack.com/api/users.list", - status=429, - content_type="application/json", - body=json.dumps({"ok": False, "error": "ratelimited"}), - ) - with pytest.raises(ApiRateLimitedError): - create_alert_rule_trigger_action( - self.trigger, - type, - target_type, - target_identifier=channel_name, - integration_id=integration.id, - ) + with self.patch_msg_schedule_response("channel_not_found"): + with pytest.raises(ApiRateLimitedError): + create_alert_rule_trigger_action( + self.trigger, + type, + target_type, + target_identifier=channel_name, + integration_id=integration.id, + ) @patch("sentry.integrations.msteams.utils.get_channel_id", return_value="some_id") def test_msteams(self, mock_get_channel_id): @@ -1856,36 +1879,22 @@ def test_slack(self): target_type = AlertRuleTriggerAction.TargetType.SPECIFIC channel_name = "#some_channel" channel_id = "s_c" - responses.add( - method=responses.POST, - url="https://slack.com/api/chat.scheduleMessage", - status=200, - content_type="application/json", - body=json.dumps( - {"ok": "true", "channel": channel_id, "scheduled_message_id": "Q1298393284"} - ), - ) - responses.add( - method=responses.POST, - url="https://slack.com/api/chat.deleteScheduledMessage", - status=200, - content_type="application/json", - body=json.dumps({"ok": True}), - ) - action = update_alert_rule_trigger_action( - self.action, - type, - target_type, - target_identifier=channel_name, - integration_id=integration.id, - ) - assert action.alert_rule_trigger == self.trigger - assert action.type == type.value - assert action.target_type == target_type.value - assert action.target_identifier == channel_id - assert action.target_display == channel_name - assert action.integration_id == integration.id + with self.patch_msg_schedule_response(channel_id): + with self.patch_msg_delete_scheduled_response(channel_id): + action = update_alert_rule_trigger_action( + self.action, + type, + target_type, + target_identifier=channel_name, + integration_id=integration.id, + ) + assert action.alert_rule_trigger == self.trigger + assert action.type == type.value + assert action.target_type == target_type.value + assert action.target_identifier == channel_id + assert action.target_display == channel_name + assert action.integration_id == integration.id def test_slack_not_existing(self): integration, _ = self.create_provider_integration_for( @@ -1898,17 +1907,18 @@ def test_slack_not_existing(self): type = AlertRuleTriggerAction.Type.SLACK target_type = AlertRuleTriggerAction.TargetType.SPECIFIC channel_name = "#some_channel_that_doesnt_exist" - with pytest.raises(ApiError): - update_alert_rule_trigger_action( - self.action, - type, - target_type, - target_identifier=channel_name, - integration_id=integration.id, - ) - - @responses.activate - def test_slack_rate_limiting(self): + with self.patch_msg_schedule_response("channel_not_found"): + with pytest.raises(InvalidTriggerActionError): + update_alert_rule_trigger_action( + self.action, + type, + target_type, + target_identifier=channel_name, + integration_id=integration.id, + ) + + @patch("slack_sdk.web.client.WebClient._perform_urllib_http_request") + def test_slack_rate_limiting(self, mock_api_call): """Should handle 429 from Slack on existing Metric Alert update""" integration, _ = self.create_provider_integration_for( self.organization, @@ -1924,29 +1934,21 @@ def test_slack_rate_limiting(self): target_type = AlertRuleTriggerAction.TargetType.SPECIFIC channel_name = "#some_channel" - responses.add( - method=responses.POST, - url="https://slack.com/api/chat.scheduleMessage", - status=200, - content_type="application/json", - body=json.dumps({"ok": False, "error": "channel_not_found"}), - ) + mock_api_call.return_value = { + "body": orjson.dumps({"ok": False, "error": "ratelimited"}).decode(), + "headers": {}, + "status": 429, + } - responses.add( - method=responses.GET, - url="https://slack.com/api/users.list", - status=429, - content_type="application/json", - body=json.dumps({"ok": False, "error": "ratelimited"}), - ) - with pytest.raises(ApiRateLimitedError): - update_alert_rule_trigger_action( - self.action, - type, - target_type, - target_identifier=channel_name, - integration_id=integration.id, - ) + with self.patch_msg_schedule_response("channel_not_found"): + with pytest.raises(ApiRateLimitedError): + update_alert_rule_trigger_action( + self.action, + type, + target_type, + target_identifier=channel_name, + integration_id=integration.id, + ) @patch("sentry.integrations.msteams.utils.get_channel_id", return_value="some_id") def test_msteams(self, mock_get_channel_id): diff --git a/tests/sentry/integrations/slack/test_notify_action.py b/tests/sentry/integrations/slack/test_notify_action.py index e1337485d7b6de..a51e69a68ac08e 100644 --- a/tests/sentry/integrations/slack/test_notify_action.py +++ b/tests/sentry/integrations/slack/test_notify_action.py @@ -18,6 +18,7 @@ from tests.sentry.integrations.slack.test_notifications import ( additional_attachment_generator_block_kit, ) +from tests.sentry.integrations.slack.utils.test_mock_slack_response import mock_slack_response pytestmark = [requires_snuba] @@ -25,34 +26,34 @@ class SlackNotifyActionTest(RuleTestCase): rule_cls = SlackNotifyServiceAction - def mock_conversations_list(self, channels): - return patch( - "slack_sdk.web.client.WebClient.conversations_list", - return_value=SlackResponse( - client=None, - http_verb="POST", - api_url="https://slack.com/api/conversations.list", - req_args={}, - data={"ok": True, "channels": channels}, - headers={}, - status_code=200, - ), - ) + def mock_list(self, list_type, channels, result_name="channels"): + return mock_slack_response(f"{list_type}_list", body={"ok": True, result_name: channels}) def mock_conversations_info(self, channel): - return patch( - "slack_sdk.web.client.WebClient.conversations_info", - return_value=SlackResponse( - client=None, - http_verb="POST", - api_url="https://slack.com/api/conversations.info", - req_args={"channel": channel}, - data={"ok": True, "channel": channel}, - headers={}, - status_code=200, - ), + return mock_slack_response( + "conversations_info", + body={"ok": True, "channel": channel}, + req_args={"channel": channel}, ) + def mock_msg_schedule_response(self, channel_id, result_name="channel"): + if channel_id == "channel_not_found": + body = {"ok": False, "error": "channel_not_found"} + else: + body = { + "ok": True, + result_name: channel_id, + "scheduled_message_id": "Q1298393284", + } + return mock_slack_response("chat_scheduleMessage", body) + + def mock_msg_delete_scheduled_response(self, channel_id, result_name="channel"): + if channel_id == "channel_not_found": + body = {"ok": False, "error": "channel_not_found"} + else: + body = {"ok": True} + return mock_slack_response("chat_deleteScheduledMessage", body) + def setUp(self): self.organization = self.get_event().project.organization self.integration, self.org_integration = self.create_provider_integration_for( @@ -148,26 +149,11 @@ def test_valid_bot_channel_selected(self): data={"workspace": integration.id, "channel": "#my-channel", "tags": ""} ) - responses.add( - method=responses.POST, - url="https://slack.com/api/chat.scheduleMessage", - status=200, - content_type="application/json", - body=orjson.dumps( - {"ok": "true", "channel": "chan-id", "scheduled_message_id": "Q1298393284"} - ), - ) - responses.add( - method=responses.POST, - url="https://slack.com/api/chat.deleteScheduledMessage", - status=200, - content_type="application/json", - body=orjson.dumps({"ok": True}), - ) - - form = rule.get_form_instance() - assert form.is_valid() - self.assert_form_valid(form, "chan-id", "#my-channel") + with self.mock_msg_schedule_response("chan-id"): + with self.mock_msg_delete_scheduled_response("chan-id"): + form = rule.get_form_instance() + assert form.is_valid() + self.assert_form_valid(form, "chan-id", "#my-channel") @responses.activate def test_valid_member_selected(self): @@ -175,14 +161,6 @@ def test_valid_member_selected(self): data={"workspace": self.integration.id, "channel": "@morty", "tags": ""} ) - responses.add( - method=responses.POST, - url="https://slack.com/api/chat.scheduleMessage", - status=200, - content_type="application/json", - body=orjson.dumps({"ok": False, "error": "channel_not_found"}), - ) - members = { "ok": "true", "members": [ @@ -191,16 +169,11 @@ def test_valid_member_selected(self): ], } - responses.add( - method=responses.GET, - url="https://slack.com/api/users.list", - status=200, - content_type="application/json", - body=orjson.dumps(members), - ) - - form = rule.get_form_instance() - self.assert_form_valid(form, "morty-id", "@morty") + with self.mock_msg_schedule_response("channel_not_found"): + with self.mock_list("users", members["members"], "members"): + form = rule.get_form_instance() + assert form.is_valid() + self.assert_form_valid(form, "morty-id", "@morty") @responses.activate def test_invalid_channel_selected(self): @@ -232,35 +205,36 @@ def test_invalid_channel_selected(self): assert len(form.errors) == 1 @responses.activate - def test_rate_limited_response(self): + @patch("slack_sdk.web.client.WebClient.users_list") + def test_rate_limited_response(self, mock_api_call): """Should surface a 429 from Slack to the frontend form""" - responses.add( - method=responses.POST, - url="https://slack.com/api/chat.scheduleMessage", - status=200, - content_type="application/json", - body=orjson.dumps({"ok": False, "error": "channel_not_found"}), - ) - responses.add( - method=responses.GET, - url="https://slack.com/api/users.list", - status=429, - content_type="application/json", - body=orjson.dumps({"ok": False, "error": "ratelimited"}), - ) - rule = self.get_rule( - data={ - "workspace": self.integration.id, - "channel": "#my-channel", - "input_channel_id": "", - "tags": "", - } + mock_api_call.side_effect = SlackApiError( + message="ratelimited", + response=SlackResponse( + client=None, + http_verb="POST", + api_url="https://slack.com/api/users.list", + req_args={}, + data={"ok": False, "error": "rate_limited"}, + headers={}, + status_code=429, + ), ) - form = rule.get_form_instance() - assert not form.is_valid() - assert SLACK_RATE_LIMITED_MESSAGE in str(form.errors.values()) + with self.mock_msg_schedule_response("channel_not_found"): + rule = self.get_rule( + data={ + "workspace": self.integration.id, + "channel": "#my-channel", + "input_channel_id": "", + "tags": "", + } + ) + + form = rule.get_form_instance() + assert not form.is_valid() + assert SLACK_RATE_LIMITED_MESSAGE in str(form.errors.values()) def test_channel_id_provided_sdk(self): channel = {"name": "my-channel", "id": "C2349874"} @@ -329,14 +303,6 @@ def test_display_name_conflict(self): data={"workspace": self.integration.id, "channel": "@morty", "tags": ""} ) - responses.add( - method=responses.POST, - url="https://slack.com/api/chat.scheduleMessage", - status=200, - content_type="application/json", - body=orjson.dumps({"ok": False, "error": "channel_not_found"}), - ) - members = { "ok": "true", "members": [ @@ -345,19 +311,13 @@ def test_display_name_conflict(self): ], } - responses.add( - method=responses.GET, - url="https://slack.com/api/users.list", - status=200, - content_type="application/json", - body=orjson.dumps(members), - ) - - form = rule.get_form_instance() - assert not form.is_valid() - assert [ - "Slack: Multiple users were found with display name '@morty'. Please use your username, found at sentry.slack.com/account/settings#username." - ] in form.errors.values() + with self.mock_msg_schedule_response("channel_not_found"): + with self.mock_list("users", members["members"], "members"): + form = rule.get_form_instance() + assert not form.is_valid() + assert [ + "Slack: Multiple users were found with display name '@morty'. Please use your username, found at sentry.slack.com/account/settings#username." + ] in form.errors.values() def test_disabled_org_integration(self): org = self.create_organization(owner=self.user) diff --git a/tests/sentry/integrations/slack/test_tasks.py b/tests/sentry/integrations/slack/test_tasks.py index fe4b3ee2c2e36f..4d21a0e157019c 100644 --- a/tests/sentry/integrations/slack/test_tasks.py +++ b/tests/sentry/integrations/slack/test_tasks.py @@ -18,8 +18,10 @@ post_message, ) from sentry.testutils.cases import TestCase -from sentry.testutils.helpers import install_slack, with_feature +from sentry.testutils.helpers import install_slack +from sentry.testutils.helpers.features import with_feature from sentry.testutils.skips import requires_snuba +from tests.sentry.integrations.slack.utils.test_mock_slack_response import mock_slack_response pytestmark = [requires_snuba] @@ -30,24 +32,18 @@ def setUp(self): self.uuid = uuid4().hex @pytest.fixture(autouse=True) - def setup_responses(self): - responses.add( - method=responses.POST, - url="https://slack.com/api/chat.scheduleMessage", - status=200, - content_type="application/json", - body=orjson.dumps( - {"ok": "true", "channel": "chan-id", "scheduled_message_id": "Q1298393284"} - ), - ) - responses.add( - method=responses.POST, - url="https://slack.com/api/chat.deleteScheduledMessage", - status=200, - content_type="application/json", - body=orjson.dumps({"ok": True}), - ) - with responses.mock: + def mock_chat_scheduleMessage(self): + with mock_slack_response( + "chat_scheduleMessage", + body={"ok": True, "channel": "chan-id", "scheduled_message_id": "Q1298393284"}, + ) as self.mock_schedule: + yield + + @pytest.fixture(autouse=True) + def mock_chat_deleteScheduledMessage(self): + with mock_slack_response( + "chat_deleteScheduledMessage", body={"ok": True} + ) as self.mock_delete: yield def metric_alert_data(self): @@ -171,7 +167,7 @@ def test_task_existing_rule(self, mock_set_value): @responses.activate @patch.object(RedisRuleStatus, "set_value", return_value=None) @patch( - "sentry.integrations.slack.utils.channel.get_channel_id_with_timeout_deprecated", + "sentry.integrations.slack.utils.channel.get_channel_id_with_timeout", return_value=SlackChannelIdData("#", "chan-id", False), ) def test_task_new_alert_rule(self, mock_get_channel_id, mock_set_value): @@ -202,37 +198,6 @@ def test_task_new_alert_rule(self, mock_get_channel_id, mock_set_value): @patch.object(RedisRuleStatus, "set_value", return_value=None) @patch( "sentry.integrations.slack.utils.channel.get_channel_id_with_timeout", - return_value=SlackChannelIdData("#", "chan-id", False), - ) - @with_feature("organizations:slack-sdk-get-channel-id") - def test_task_new_alert_rule_with_sdk(self, mock_get_channel_id, mock_set_value): - alert_rule_data = self.metric_alert_data() - - data = { - "data": alert_rule_data, - "uuid": self.uuid, - "organization_id": self.organization.id, - "user_id": self.user.id, - } - - with self.tasks(): - with self.feature(["organizations:incidents"]): - find_channel_id_for_alert_rule(**data) - - rule = AlertRule.objects.get(name="New Rule") - assert rule.created_by_id == self.user.id - mock_set_value.assert_called_with("success", rule.id) - mock_get_channel_id.assert_called_with( - serialize_integration(self.integration), "my-channel", 180 - ) - - trigger_action = AlertRuleTriggerAction.objects.get(integration_id=self.integration.id) - assert trigger_action.target_identifier == "chan-id" - - @responses.activate - @patch.object(RedisRuleStatus, "set_value", return_value=None) - @patch( - "sentry.integrations.slack.utils.channel.get_channel_id_with_timeout_deprecated", return_value=SlackChannelIdData("#", None, False), ) def test_task_failed_id_lookup(self, mock_get_channel_id, mock_set_value): @@ -258,32 +223,6 @@ def test_task_failed_id_lookup(self, mock_get_channel_id, mock_set_value): @patch.object(RedisRuleStatus, "set_value", return_value=None) @patch( "sentry.integrations.slack.utils.channel.get_channel_id_with_timeout", - return_value=SlackChannelIdData("#", None, False), - ) - @with_feature("organizations:slack-sdk-get-channel-id") - def test_task_failed_id_lookup_with_sdk(self, mock_get_channel_id, mock_set_value): - alert_rule_data = self.metric_alert_data() - - data = { - "data": alert_rule_data, - "uuid": self.uuid, - "organization_id": self.organization.id, - } - - with self.tasks(): - with self.feature(["organizations:incidents"]): - find_channel_id_for_alert_rule(**data) - - assert not AlertRule.objects.filter(name="New Rule").exists() - mock_set_value.assert_called_with("failed") - mock_get_channel_id.assert_called_with( - serialize_integration(self.integration), "my-channel", 180 - ) - - @responses.activate - @patch.object(RedisRuleStatus, "set_value", return_value=None) - @patch( - "sentry.integrations.slack.utils.channel.get_channel_id_with_timeout_deprecated", return_value=SlackChannelIdData("#", None, True), ) def test_task_timeout_id_lookup(self, mock_get_channel_id, mock_set_value): @@ -309,32 +248,6 @@ def test_task_timeout_id_lookup(self, mock_get_channel_id, mock_set_value): @patch.object(RedisRuleStatus, "set_value", return_value=None) @patch( "sentry.integrations.slack.utils.channel.get_channel_id_with_timeout", - return_value=SlackChannelIdData("#", None, True), - ) - @with_feature("organizations:slack-sdk-get-channel-id") - def test_task_timeout_id_lookup_with_sdk(self, mock_get_channel_id, mock_set_value): - alert_rule_data = self.metric_alert_data() - - data = { - "data": alert_rule_data, - "uuid": self.uuid, - "organization_id": self.organization.id, - } - - with self.tasks(): - with self.feature(["organizations:incidents"]): - find_channel_id_for_alert_rule(**data) - - assert not AlertRule.objects.filter(name="New Rule").exists() - mock_set_value.assert_called_with("failed") - mock_get_channel_id.assert_called_with( - serialize_integration(self.integration), "my-channel", 180 - ) - - @responses.activate - @patch.object(RedisRuleStatus, "set_value", return_value=None) - @patch( - "sentry.integrations.slack.utils.channel.get_channel_id_with_timeout_deprecated", return_value=SlackChannelIdData("#", "chan-id", False), ) def test_task_existing_metric_alert(self, mock_get_channel_id, mock_set_value): @@ -401,7 +314,7 @@ def test_task_existing_metric_alert_with_sdk(self, mock_get_channel_id, mock_set @patch("sentry.integrations.slack.sdk_client.metrics") @patch("slack_sdk.web.client.WebClient._perform_urllib_http_request") @responses.activate - def test_post_message_success_sdk(self, mock_api_call, mock_metrics): + def test_post_message_success(self, mock_api_call, mock_metrics): mock_api_call.return_value = { "body": orjson.dumps({"ok": True}).decode(), "headers": {}, diff --git a/tests/sentry/integrations/slack/test_utils.py b/tests/sentry/integrations/slack/test_utils.py index 770872ada95180..fa8655fe12b7ac 100644 --- a/tests/sentry/integrations/slack/test_utils.py +++ b/tests/sentry/integrations/slack/test_utils.py @@ -15,7 +15,6 @@ from sentry.shared_integrations.exceptions import ApiRateLimitedError, DuplicateDisplayNameError from sentry.testutils.cases import TestCase from sentry.testutils.helpers import install_slack -from sentry.testutils.helpers.features import with_feature from sentry.testutils.skips import requires_snuba pytestmark = [requires_snuba] @@ -55,33 +54,6 @@ def setUp(self): "response_metadata": {"next_cursor": ""}, } - def tearDown(self): - self.resp.__exit__(None, None, None) - - def add_list_response(self, list_type, channels, result_name="channels"): - self.resp = responses.mock - self.resp.add( - method=responses.GET, - url="https://slack.com/api/%s.list" % list_type, - status=200, - content_type="application/json", - body=orjson.dumps({"ok": "true", result_name: channels}), - ) - - def add_msg_response(self, channel_id, result_name="channel"): - if channel_id == "channel_not_found": - bodydict = {"ok": False, "error": "channel_not_found"} - else: - bodydict = {"ok": True, result_name: channel_id, "scheduled_message_id": "Q1298393284"} - - self.resp.add( - method=responses.POST, - url="https://slack.com/api/chat.scheduleMessage", - status=200, - content_type="application/json", - body=orjson.dumps(bodydict), - ) - def patch_mock_list(self, list_type, channels, result_name="channels"): return patch( "slack_sdk.web.client.WebClient.%s_list" % list_type, @@ -123,18 +95,6 @@ def run_valid_test(self, channel, expected_prefix, expected_id, timed_out): self.organization, self.integration, channel ) - def test_valid_channel_selected(self): - self.add_msg_response("m-c") - self.resp.add( - method=responses.POST, - url="https://slack.com/api/chat.deleteScheduledMessage", - status=200, - content_type="application/json", - body=orjson.dumps({"ok": True}), - ) - self.run_valid_test("#My-Channel", CHANNEL_PREFIX, "m-c", False) - - @with_feature("organizations:slack-sdk-get-channel-id") @patch("sentry.integrations.slack.sdk_client.metrics") @patch("slack_sdk.web.client.WebClient._perform_urllib_http_request") def test_valid_channel_selected_sdk(self, mock_api_call, mock_metrics): @@ -155,18 +115,6 @@ def test_valid_channel_selected_sdk(self, mock_api_call, mock_metrics): tags={"ok": True, "status": 200}, ) - def test_valid_private_channel_selected(self): - self.add_msg_response("m-p-c") - self.resp.add( - method=responses.POST, - url="https://slack.com/api/chat.deleteScheduledMessage", - status=200, - content_type="application/json", - body=orjson.dumps({"ok": True}), - ) - self.run_valid_test("#my-private-channel", CHANNEL_PREFIX, "m-p-c", False) - - @with_feature("organizations:slack-sdk-get-channel-id") @patch("sentry.integrations.slack.sdk_client.metrics") @patch("slack_sdk.web.client.WebClient._perform_urllib_http_request") def test_valid_private_channel_selected_sdk(self, mock_api_call, mock_metrics): @@ -187,20 +135,6 @@ def test_valid_private_channel_selected_sdk(self, mock_api_call, mock_metrics): tags={"ok": True, "status": 200}, ) - def test_valid_member_selected(self): - self.add_msg_response("channel_not_found") - self.add_list_response( - "users", - [ - {"name": "first-morty", "id": "m", "profile": {"display_name": "Morty"}}, - {"name": "other-user", "id": "o-u", "profile": {"display_name": "Jimbob"}}, - {"name": "better_morty", "id": "bm", "profile": {"display_name": "Morty"}}, - ], - result_name="members", - ) - self.run_valid_test("@first-morty", MEMBER_PREFIX, "m", False) - - @with_feature("organizations:slack-sdk-get-channel-id") def test_valid_member_selected_sdk_client(self): response_list = [ {"name": "first-morty", "id": "m", "profile": {"display_name": "Morty"}}, @@ -212,20 +146,6 @@ def test_valid_member_selected_sdk_client(self): with self.patch_mock_list("users", response_list, "members"): self.run_valid_test("@first-morty", MEMBER_PREFIX, "m", False) - def test_valid_member_selected_display_name(self): - self.add_msg_response("channel_not_found") - self.add_list_response( - "users", - [ - {"name": "first-morty", "id": "m", "profile": {"display_name": "Morty"}}, - {"name": "other-user", "id": "o-u", "profile": {"display_name": "Jimbob"}}, - {"name": "better_morty", "id": "bm", "profile": {"display_name": "Morty"}}, - ], - result_name="members", - ) - self.run_valid_test("@Jimbob", MEMBER_PREFIX, "o-u", False) - - @with_feature("organizations:slack-sdk-get-channel-id") def test_valid_member_selected_display_name_sdk_client(self): response_list = [ {"name": "first-morty", "id": "m", "profile": {"display_name": "Morty"}}, @@ -237,21 +157,6 @@ def test_valid_member_selected_display_name_sdk_client(self): with self.patch_mock_list("users", response_list, "members"): self.run_valid_test("@Jimbob", MEMBER_PREFIX, "o-u", False) - def test_invalid_member_selected_display_name(self): - self.add_msg_response("channel_not_found") - self.add_list_response( - "users", - [ - {"name": "first-morty", "id": "m", "profile": {"display_name": "Morty"}}, - {"name": "other-user", "id": "o-u", "profile": {"display_name": "Jimbob"}}, - {"name": "better_morty", "id": "bm", "profile": {"display_name": "Morty"}}, - ], - result_name="members", - ) - with pytest.raises(DuplicateDisplayNameError): - get_channel_id(self.organization, self.integration, "@Morty") - - @with_feature("organizations:slack-sdk-get-channel-id") def test_invalid_member_selected_display_name_sdk_client(self): response_list = [ {"name": "first-morty", "id": "m", "profile": {"display_name": "Morty"}}, @@ -264,14 +169,6 @@ def test_invalid_member_selected_display_name_sdk_client(self): with pytest.raises(DuplicateDisplayNameError): get_channel_id(self.organization, self.integration, "@Morty") - def test_invalid_channel_selected(self): - self.add_msg_response("channel_not_found") - assert ( - get_channel_id(self.organization, self.integration, "#fake-channel").channel_id is None - ) - assert get_channel_id(self.organization, self.integration, "@fake-user").channel_id is None - - @with_feature("organizations:slack-sdk-get-channel-id") def test_invalid_channel_selected_sdk_client(self): with self.patch_msg_response("channel_not_found"): assert ( @@ -282,20 +179,6 @@ def test_invalid_channel_selected_sdk_client(self): get_channel_id(self.organization, self.integration, "@fake-user").channel_id is None ) - def test_rate_limiting(self): - """Should handle 429 from Slack when searching for users""" - self.add_msg_response("channel_not_found") - self.resp.add( - method=responses.GET, - url="https://slack.com/api/users.list", - status=429, - content_type="application/json", - body=orjson.dumps({"ok": False, "error": "ratelimited"}), - ) - with pytest.raises(ApiRateLimitedError): - get_channel_id(self.organization, self.integration, "@user") - - @with_feature("organizations:slack-sdk-get-channel-id") @patch("slack_sdk.web.client.WebClient._perform_urllib_http_request") def test_rate_limiting_sdk_client(self, mock_api_call): """Should handle 429 from Slack when searching for users""" @@ -311,7 +194,6 @@ def test_rate_limiting_sdk_client(self, mock_api_call): get_channel_id(self.organization, self.integration, "@user") @patch("slack_sdk.web.client.WebClient._perform_urllib_http_request") - @with_feature("organizations:slack-sdk-get-channel-id") def test_user_list_pagination_sdk_client(self, mock_api_call): self.response_json["response_metadata"] = {"next_cursor": "dXNlcjpVMEc5V0ZYTlo"} mock_api_call.return_value = { @@ -323,7 +205,6 @@ def test_user_list_pagination_sdk_client(self, mock_api_call): self.run_valid_test("@wayne-rigsby", MEMBER_PREFIX, "UXXXXXXX4", False) @patch("slack_sdk.web.client.WebClient._perform_urllib_http_request") - @with_feature("organizations:slack-sdk-get-channel-id") def test_user_list_multi_pagination_sdk_client(self, mock_api_call): self.response_json["response_metadata"] = {"next_cursor": "dXNlcjpVMEc5V0ZYTlo"} mock_api_call.side_effect = [ @@ -353,7 +234,6 @@ def test_user_list_multi_pagination_sdk_client(self, mock_api_call): self.run_valid_test("@red-john", MEMBER_PREFIX, "UXXXXXXX5", False) @patch("slack_sdk.web.client.WebClient._perform_urllib_http_request") - @with_feature("organizations:slack-sdk-get-channel-id") def test_user_list_pagination_failure_sdk_client(self, mock_api_call): self.response_json["response_metadata"] = {"next_cursor": "dXNlcjpVMEc5V0ZYTlo"} mock_api_call.side_effect = [ diff --git a/tests/sentry/integrations/slack/utils/test_mock_slack_response.py b/tests/sentry/integrations/slack/utils/test_mock_slack_response.py new file mode 100644 index 00000000000000..a57c54ee15218b --- /dev/null +++ b/tests/sentry/integrations/slack/utils/test_mock_slack_response.py @@ -0,0 +1,23 @@ +from unittest.mock import patch + +from slack_sdk.web.slack_response import SlackResponse + + +def mock_slack_response( + method_name, body, status_code=200, http_verb="POST", api_url=None, req_args=None, headers=None +): + if api_url is None: + api_url = f"https://slack.com/api/{method_name}" + + return patch( + f"slack_sdk.web.client.WebClient.{method_name}", + return_value=SlackResponse( + client=None, + http_verb=http_verb, + api_url=api_url, + req_args=req_args or {}, + data=body, + headers=headers or {}, + status_code=status_code, + ), + )