Skip to content

Commit

Permalink
fix: patch slack ID reference issue
Browse files Browse the repository at this point in the history
  • Loading branch information
joeyorlando committed Nov 29, 2024
1 parent 5227ee3 commit af81f7e
Show file tree
Hide file tree
Showing 7 changed files with 544 additions and 161 deletions.
44 changes: 41 additions & 3 deletions engine/apps/alerts/models/alert_group.py
Original file line number Diff line number Diff line change
Expand Up @@ -1978,14 +1978,52 @@ def notify_in_slack_enabled(self):
def is_presented_in_slack(self):
return self.slack_message and self.channel.organization.slack_team_identity

# TODO: once we've finished migrating slack_message._channel_id to slack_message.channel, we can enable this
# and refactor the related code
#
# @property
# def organization(self) -> Organization:
# return self.channel.organization

# @property
# def slack_channel(self) -> typing.Optional["SlackChannel"]:
# """
# If the `AlertGroup` already has a `SlackMessage` associated with it, that means that we already
# know the channel.

# Otherwise, infer the Slack channel from the `ChannelFilter` associated with the `AlertGroup`.
# """
# if self.slack_message:
# # NOTE: this isn't available yet because we haven't finished migrating slack_message._channel_id to
# # slack_message.channel
# return self.slack_message.channel
# elif self.channel_filter and self.channel_filter.slack_channel:
# return self.channel_filter.slack_channel
# return self.organization.default_slack_channel

# @property
# def slack_channel_id(self) -> str | None:
# """
# If the `AlertGroup` already has a `SlackMessage` associated with it, that means that we already
# know the channel.

# Otherwise, infer the Slack channel ID from the `ChannelFilter` associated with the `AlertGroup`.
# """
# return self.slack_channel.slack_id if self.slack_channel else None
# END TODO:

@property
def slack_channel_id(self) -> str | None:
if not self.channel.organization.slack_team_identity:
return None
elif self.slack_message:
return self.slack_message.channel.slack_id
elif self.channel_filter:
return self.channel_filter.slack_channel_id_or_org_default_id
# TODO: once _channel_id has been fully migrated to channel, remove _channel_id
# see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p173255546
#
# return self.slack_message.channel.slack_id
return self.slack_message._channel_id
elif self.channel_filter and self.channel_filter.slack_channel_or_org_default:
return self.channel_filter.slack_channel_or_org_default.slack_id
return None

@property
Expand Down
10 changes: 2 additions & 8 deletions engine/apps/alerts/models/channel_filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,14 +172,8 @@ def slack_channel_slack_id(self) -> typing.Optional[str]:
return self.slack_channel.slack_id if self.slack_channel else None

@property
def slack_channel_id_or_org_default_id(self):
organization = self.alert_receive_channel.organization

if organization.slack_team_identity is None:
return None
elif self.slack_channel_slack_id is None:
return organization.default_slack_channel_slack_id
return self.slack_channel_slack_id
def slack_channel_or_org_default(self) -> typing.Optional["SlackChannel"]:
return self.slack_channel or self.alert_receive_channel.organization.default_slack_channel

@property
def str_for_clients(self):
Expand Down
10 changes: 2 additions & 8 deletions engine/apps/alerts/tests/test_notify_all.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,24 +9,18 @@

@pytest.mark.django_db
def test_notify_all(
make_organization,
make_slack_team_identity,
make_organization_and_user_with_slack_identities,
make_slack_channel,
make_user,
make_user_notification_policy,
make_escalation_chain,
make_escalation_policy,
make_channel_filter,
make_alert_receive_channel,
make_alert_group,
):
organization = make_organization()
slack_team_identity = make_slack_team_identity()
organization, user, slack_team_identity, _ = make_organization_and_user_with_slack_identities()
slack_channel = make_slack_channel(slack_team_identity)
organization.slack_team_identity = slack_team_identity
organization.save()

user = make_user(organization=organization)
make_user_notification_policy(
user=user,
step=UserNotificationPolicy.Step.NOTIFY,
Expand Down
43 changes: 22 additions & 21 deletions engine/apps/slack/scenarios/distribute_alerts.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ def process_signal(self, alert: Alert) -> None:
See this conversation for more context https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732800180834819?thread_ts=1732748893.183939&cid=C06K1MQ07GS
"""

alert_group = alert.group

if not alert_group:
Expand All @@ -108,6 +107,8 @@ def process_signal(self, alert: Alert) -> None:

alert_group_pk = alert_group.pk
alert_receive_channel = alert_group.channel
organization = alert_receive_channel.organization
channel_filter = alert_group.channel_filter
should_skip_escalation_in_slack = alert_group.skip_escalation_in_slack
slack_team_identity = self.slack_team_identity
slack_team_identity_pk = slack_team_identity.pk
Expand All @@ -128,17 +129,29 @@ def process_signal(self, alert: Alert) -> None:
if num_updated_rows == 1:
# this will be the case in the event that we haven't yet created a Slack message for this alert group

slack_channel = (
alert_group.channel_filter.slack_channel
if alert_group.channel_filter
# if channel filter is deleted mid escalation, use default Slack channel
else alert_receive_channel.organization.default_slack_channel
)
# if channel filter is deleted mid escalation, use the organization's default Slack channel
slack_channel = channel_filter.slack_channel if channel_filter else organization.default_slack_channel

# slack_channel can be None if
# - the channel filter is deleted mid escalation
# - AND the organization does not have a default slack channel set
# in this case, we have to skip posting to Slack because we don't know where to post to
if slack_channel is None:
logger.info(
f"Skipping posting message to Slack for alert_group {alert_group_pk} because we don't know which "
f"Slack channel to post to. channel_filter={channel_filter} "
f"organization.default_slack_channel={organization.default_slack_channel}"
)

alert_group.slack_message_sent = False
alert_group.reason_to_skip_escalation = AlertGroup.CHANNEL_NOT_SPECIFIED
alert_group.save(update_fields=["slack_message_sent", "reason_to_skip_escalation"])
return

slack_channel_id = slack_channel.slack_id

try:
self._post_alert_group_to_slack(
slack_team_identity=slack_team_identity,
alert_group=alert_group,
alert=alert,
attachments=alert_group.render_slack_attachments(),
Expand Down Expand Up @@ -193,24 +206,12 @@ def process_signal(self, alert: Alert) -> None:

def _post_alert_group_to_slack(
self,
slack_team_identity: SlackTeamIdentity,
alert_group: AlertGroup,
alert: Alert,
attachments,
slack_channel: typing.Optional[SlackChannel],
slack_channel: SlackChannel,
blocks: Block.AnyBlocks,
) -> None:
# slack_channel can be None if org default slack channel for slack_team_identity is not set
if slack_channel is None:
logger.info(
f"Failed to post message to Slack for alert_group {alert_group.pk} because slack_channel is None"
)

alert_group.reason_to_skip_escalation = AlertGroup.CHANNEL_NOT_SPECIFIED
alert_group.save(update_fields=["reason_to_skip_escalation"])

return

try:
result = self._slack_client.chat_postMessage(
channel=slack_channel.slack_id,
Expand Down
4 changes: 2 additions & 2 deletions engine/apps/slack/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -303,14 +303,14 @@ def post_slack_rate_limit_message(integration_id):
return

default_route = integration.channel_filters.get(is_default=True)
if (slack_channel_id := default_route.slack_channel_id_or_org_default_id) is not None:
if (slack_channel := default_route.slack_channel_or_org_default) is not None:
text = (
f"Delivering and updating alert groups of integration {integration.verbal_name} in Slack is "
f"temporarily stopped due to rate limit. You could find new alert groups at "
f"<{integration.new_incidents_web_link}|web page "
'"Alert Groups">'
)
post_message_to_channel(integration.organization, slack_channel_id, text)
post_message_to_channel(integration.organization, slack_channel.slack_id, text)


@shared_dedicated_queue_retry_task(
Expand Down
Loading

0 comments on commit af81f7e

Please sign in to comment.