chore(slack): Update Tests for get_channel_id (#73548)

I GA'ed the feature, so I am updating the tests here.

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_type>.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

src/sentry/tasks/integrations/slack/find_channel_id_for_rule.py

@@ -2,18 +2,14 @@
 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 (
  SLACK_RATE_LIMITED_MESSAGE,
  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

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):

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,

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"

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),

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),