From 1c1fd5c04729079bbefd1a44317dacc5e846dd05 Mon Sep 17 00:00:00 2001 From: Cathy Teng Date: Wed, 27 Mar 2024 10:11:23 -0700 Subject: [PATCH 1/3] don't raise RuntimeError for no team id --- src/sentry/integrations/slack/requests/base.py | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/src/sentry/integrations/slack/requests/base.py b/src/sentry/integrations/slack/requests/base.py index a769106a1740db..f022d3fe9dceff 100644 --- a/src/sentry/integrations/slack/requests/base.py +++ b/src/sentry/integrations/slack/requests/base.py @@ -23,17 +23,6 @@ def _get_field_id_option(data: Mapping[str, Any], field_name: str) -> str | None return id_option -def get_field_id(data: Mapping[str, Any], field_name: str) -> str: - """ - TODO(mgaeta): Hack to convert optional strings to string. SlackRequest - should be refactored to deserialize `data` in the constructor. - """ - id_option = _get_field_id_option(data, field_name) - if not id_option: - raise RuntimeError - return id_option - - @dataclasses.dataclass(frozen=True) class SlackRequestError(Exception): """ @@ -116,7 +105,7 @@ def integration(self) -> RpcIntegration: @property def channel_id(self) -> str: - return get_field_id(self.data, "channel") + return _get_field_id_option(self.data, "channel") @property def response_url(self) -> str: @@ -124,11 +113,11 @@ def response_url(self) -> str: @property def team_id(self) -> str: - return get_field_id(self.data, "team") + return _get_field_id_option(self.data, "team") @property def user_id(self) -> str: - return get_field_id(self.data, "user") + return _get_field_id_option(self.data, "user") @property def data(self) -> Mapping[str, Any]: From 121726dc100a21249e177200f1054f8f5a30fbbb Mon Sep 17 00:00:00 2001 From: Cathy Teng Date: Wed, 27 Mar 2024 10:15:16 -0700 Subject: [PATCH 2/3] fix test and typing --- src/sentry/integrations/slack/requests/base.py | 6 +++--- tests/sentry/integrations/slack/test_requests.py | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/sentry/integrations/slack/requests/base.py b/src/sentry/integrations/slack/requests/base.py index f022d3fe9dceff..1488d7c7243105 100644 --- a/src/sentry/integrations/slack/requests/base.py +++ b/src/sentry/integrations/slack/requests/base.py @@ -104,7 +104,7 @@ def integration(self) -> RpcIntegration: return self._integration @property - def channel_id(self) -> str: + def channel_id(self) -> str | None: return _get_field_id_option(self.data, "channel") @property @@ -112,11 +112,11 @@ def response_url(self) -> str: return self.data.get("response_url", "") @property - def team_id(self) -> str: + def team_id(self) -> str | None: return _get_field_id_option(self.data, "team") @property - def user_id(self) -> str: + def user_id(self) -> str | None: return _get_field_id_option(self.data, "user") @property diff --git a/tests/sentry/integrations/slack/test_requests.py b/tests/sentry/integrations/slack/test_requests.py index 80583a5126ef7b..6473eb925bb94e 100644 --- a/tests/sentry/integrations/slack/test_requests.py +++ b/tests/sentry/integrations/slack/test_requests.py @@ -118,6 +118,7 @@ def test_none_in_data(self): request.META = (options.get("slack.signing-secret"), self.request.body) slack_request = SlackRequest(request) + assert slack_request.team_id is None assert slack_request.logging_data == { "slack_channel_id": "1", "slack_user_id": "2", From a9916e069b347be043e07f3a85ebe1afe7c5fa82 Mon Sep 17 00:00:00 2001 From: Cathy Teng Date: Thu, 28 Mar 2024 13:49:11 -0700 Subject: [PATCH 3/3] put back logic for channel_id --- src/sentry/integrations/slack/requests/base.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/sentry/integrations/slack/requests/base.py b/src/sentry/integrations/slack/requests/base.py index 1488d7c7243105..c0859c166a084c 100644 --- a/src/sentry/integrations/slack/requests/base.py +++ b/src/sentry/integrations/slack/requests/base.py @@ -23,6 +23,17 @@ def _get_field_id_option(data: Mapping[str, Any], field_name: str) -> str | None return id_option +def get_field_id(data: Mapping[str, Any], field_name: str) -> str: + """ + TODO(mgaeta): Hack to convert optional strings to string. SlackRequest + should be refactored to deserialize `data` in the constructor. + """ + id_option = _get_field_id_option(data, field_name) + if not id_option: + raise RuntimeError + return id_option + + @dataclasses.dataclass(frozen=True) class SlackRequestError(Exception): """ @@ -105,7 +116,7 @@ def integration(self) -> RpcIntegration: @property def channel_id(self) -> str | None: - return _get_field_id_option(self.data, "channel") + return get_field_id(self.data, "channel") @property def response_url(self) -> str: