Skip to content

feat(nimbus): Send channel notifications in slack DMs #14179

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.

Sign up for GitHub

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
yashikakhurana merged 4 commits into from
Dec 16, 2025
+331 −31
Merged

feat(nimbus): Send channel notifications in slack DMs #14179

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0c93034
feat(nimbus): Send channel notifications in slack DMs
yashikakhurana Dec 11, 2025
bdbed16
feat(nimbus): Use lookup only to return users id
yashikakhurana Dec 11, 2025
14f4cc8
feat(nimbus): Don't send dms if the user is in channel
yashikakhurana Dec 15, 2025
b15364e
Merge branch 'main' into 14173
yashikakhurana Dec 16, 2025
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
54 changes: 48 additions & 6 deletions experimenter/experimenter/slack/notification.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,39 @@ def _get_slack_client():
return WebClient(token=settings.SLACK_AUTH_TOKEN)


def _get_user_mentions(client, emails):
def _lookup_users(client, emails):
mentions = []
user_ids = []
for email in emails:
if not email:
continue
try:
response = client.users_lookupByEmail(email=email)
mentions.append(f"<@{response['user']['id']}>")
user_id = response["user"]["id"]
mentions.append(f"<@{user_id}>")
user_ids.append(user_id)
except SlackApiError as e:
logger.warning(f"Could not find Slack user for {email}: {e}")
continue
return " ".join(mentions)
return " ".join(mentions), user_ids
Copy link
Collaborator

@jaredlockhart jaredlockhart Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean I'd probably keep this function just to lookup and return the user ids and move constructing the mentions elsewhere rather than fold them both into one func

Copy link
Contributor Author

@yashikakhurana yashikakhurana Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure i can do that change

Copy link
Contributor Author

@yashikakhurana yashikakhurana Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay done!! 🎉



def _send_dm_to_user(client, user_id, message, channel_message_link=None):
try:
conversation = client.conversations_open(users=[user_id])
channel_id = conversation["channel"]["id"]

# Add channel message link if provided
dm_message = message
if channel_message_link:
dm_message = f"{message}\n\nView in channel: {channel_message_link}"

client.chat_postMessage(
channel=channel_id, text=dm_message, unfurl_links=False, unfurl_media=False
)
logger.info(f"DM sent to user {user_id}")
except SlackApiError as e:
logger.warning(f"Failed to send DM to user {user_id}: {e}")


def send_slack_notification(
Expand All @@ -47,13 +68,20 @@ def send_slack_notification(

channel = settings.SLACK_NIMBUS_CHANNEL

all_user_ids = []

requesting_user_mention = ""
if requesting_user_email:
requesting_user_mention = _get_user_mentions(client, [requesting_user_email])
requesting_user_mention, requesting_user_ids = _lookup_users(
client, [requesting_user_email]
)
if requesting_user_ids:
all_user_ids.extend(requesting_user_ids)
# Exclude requesting_user_email from email_addresses to avoid duplicate mentions
email_addresses = [e for e in email_addresses if e and e != requesting_user_email]

mentions = _get_user_mentions(client, email_addresses)
mentions, mentioned_user_ids = _lookup_users(client, email_addresses)
all_user_ids.extend(mentioned_user_ids)

if requesting_user_mention:
message = (
Expand All @@ -67,10 +95,24 @@ def send_slack_notification(
message = f"{message} {mentions}"

try:
client.chat_postMessage(
response = client.chat_postMessage(
channel=channel, text=message, unfurl_links=False, unfurl_media=False
)
logger.info(f"Slack notification sent for experiment {experiment.name}")

# Get the permalink to the channel message
channel_message_link = None
try:
permalink_response = client.chat_getPermalink(
channel=channel, message_ts=response["ts"]
)
channel_message_link = permalink_response["permalink"]
except SlackApiError as e:
logger.warning(f"Could not get permalink for channel message: {e}")

for user_id in all_user_ids:
_send_dm_to_user(client, user_id, message, channel_message_link)

except SlackApiError as e:
logger.error(f"Failed to send Slack notification for {experiment.name}: {e}")
raise
171 changes: 148 additions & 23 deletions experimenter/experimenter/slack/tests/test_notification.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,15 @@ def test_send_slack_notification_success(self, mock_webclient):
mock_client = Mock()
mock_webclient.return_value = mock_client
mock_client.users_lookupByEmail.return_value = {"user": {"id": "U123456"}}
mock_client.chat_postMessage.return_value = {"ok": True}
mock_client.chat_postMessage.return_value = {
"ok": True,
"ts": "1234567890.123456",
}
mock_client.chat_getPermalink.return_value = {
"ok": True,
"permalink": "https://mozilla.slack.com/archives/C123/p1234567890123456",
}
mock_client.conversations_open.return_value = {"channel": {"id": "D123456"}}

action_text = NimbusConstants.SLACK_EMAIL_ACTIONS[
NimbusExperiment.EmailType.EXPERIMENT_END
Expand All @@ -31,11 +39,14 @@ def test_send_slack_notification_success(self, mock_webclient):
)

mock_client.users_lookupByEmail.assert_called_once_with(email="test@example.com")
mock_client.chat_postMessage.assert_called_once()

# Verify message format includes experiment URL
call_args = mock_client.chat_postMessage.call_args
message = call_args.kwargs["text"]
# Should be called once for channel message and once for DM
self.assertEqual(mock_client.chat_postMessage.call_count, 2)
mock_client.chat_getPermalink.assert_called_once()
mock_client.conversations_open.assert_called_once()

# Verify channel message format includes experiment URL
channel_call = mock_client.chat_postMessage.call_args_list[0]
message = channel_call.kwargs["text"]
self.assertIn(self.experiment.experiment_url, message)
self.assertIn(self.experiment.name, message)
self.assertIn(action_text, message)
Expand All @@ -62,7 +73,14 @@ def test_send_slack_notification_user_not_found(self, mock_webclient):
mock_client.users_lookupByEmail.side_effect = SlackApiError(
message="User not found", response={"error": "users_not_found"}
)
mock_client.chat_postMessage.return_value = {"ok": True}
mock_client.chat_postMessage.return_value = {
"ok": True,
"ts": "1234567890.123456",
}
mock_client.chat_getPermalink.return_value = {
"ok": True,
"permalink": "https://mozilla.slack.com/archives/C123/p1234567890123456",
}

send_slack_notification(
experiment_id=self.experiment.id,
Expand Down Expand Up @@ -127,7 +145,15 @@ def test_send_slack_notification_with_requesting_user(self, mock_webclient):
{"user": {"id": "U123456"}}, # requesting user
{"user": {"id": "U789012"}}, # mentioned user
]
mock_client.chat_postMessage.return_value = {"ok": True}
mock_client.chat_postMessage.return_value = {
"ok": True,
"ts": "1234567890.123456",
}
mock_client.chat_getPermalink.return_value = {
"ok": True,
"permalink": "https://mozilla.slack.com/archives/C123/p1234567890123456",
}
mock_client.conversations_open.return_value = {"channel": {"id": "D123456"}}

send_slack_notification(
experiment_id=self.experiment.id,
Expand All @@ -140,9 +166,12 @@ def test_send_slack_notification_with_requesting_user(self, mock_webclient):

# Should call users_lookupByEmail twice
self.assertEqual(mock_client.users_lookupByEmail.call_count, 2)
mock_client.chat_postMessage.assert_called_once()
# Should be called once for channel message and twice for DMs
self.assertEqual(mock_client.chat_postMessage.call_count, 3)
mock_client.chat_getPermalink.assert_called_once()
self.assertEqual(mock_client.conversations_open.call_count, 2)

call_args = mock_client.chat_postMessage.call_args
call_args = mock_client.chat_postMessage.call_args_list[0]
message = call_args.kwargs["text"]
# format: @user action: Experiment Name @mentions
self.assertIn("<@U123456>", message) # requesting user
Expand All @@ -166,7 +195,15 @@ def test_send_slack_notification_requesting_user_not_found(self, mock_webclient)
),
{"user": {"id": "U789012"}},
]
mock_client.chat_postMessage.return_value = {"ok": True}
mock_client.chat_postMessage.return_value = {
"ok": True,
"ts": "1234567890.123456",
}
mock_client.chat_getPermalink.return_value = {
"ok": True,
"permalink": "https://mozilla.slack.com/archives/C123/p1234567890123456",
}
mock_client.conversations_open.return_value = {"channel": {"id": "D123456"}}

send_slack_notification(
experiment_id=self.experiment.id,
Expand All @@ -177,13 +214,14 @@ def test_send_slack_notification_requesting_user_not_found(self, mock_webclient)
requesting_user_email="nonexistent@example.com",
)

mock_client.chat_postMessage.assert_called_once()
# Should be called once for channel message and once for DM to mentioned user
self.assertEqual(mock_client.chat_postMessage.call_count, 2)

# Verify message format without requesting user mention
call_args = mock_client.chat_postMessage.call_args
# Verify channel message format without requesting user mention
call_args = mock_client.chat_postMessage.call_args_list[0]
message = call_args.kwargs["text"]
self.assertNotIn("<@U123456>", message) # requesting user not found
self.assertIn("<@U789012>", message) # mentioned user still there
self.assertIn("<@U789012>", message) # mentioned user should be present
self.assertNotIn("<@U123456>", message) # requesting user should not be present

@override_settings(SLACK_AUTH_TOKEN="test-token")
@patch("experimenter.slack.notification.WebClient")
Expand All @@ -195,7 +233,15 @@ def test_send_slack_notification_no_duplicate_mention(self, mock_webclient):
{"user": {"id": "U123456"}},
{"user": {"id": "U789012"}},
]
mock_client.chat_postMessage.return_value = {"ok": True}
mock_client.chat_postMessage.return_value = {
"ok": True,
"ts": "1234567890.123456",
}
mock_client.chat_getPermalink.return_value = {
"ok": True,
"permalink": "https://mozilla.slack.com/archives/C123/p1234567890123456",
}
mock_client.conversations_open.return_value = {"channel": {"id": "D123456"}}

send_slack_notification(
experiment_id=self.experiment.id,
Expand All @@ -207,9 +253,12 @@ def test_send_slack_notification_no_duplicate_mention(self, mock_webclient):
)

self.assertEqual(mock_client.users_lookupByEmail.call_count, 2)
mock_client.chat_postMessage.assert_called_once()
# Should be called once for channel message and twice for DMs
self.assertEqual(mock_client.chat_postMessage.call_count, 3)
mock_client.chat_getPermalink.assert_called_once()
self.assertEqual(mock_client.conversations_open.call_count, 2)

call_args = mock_client.chat_postMessage.call_args
call_args = mock_client.chat_postMessage.call_args_list[0]
message = call_args.kwargs["text"]
mention_count = message.count("<@U123456>")
self.assertEqual(
Expand All @@ -232,7 +281,15 @@ def test_send_slack_notification_channel_setting(self, mock_webclient):
mock_client = Mock()
mock_webclient.return_value = mock_client
mock_client.users_lookupByEmail.return_value = {"user": {"id": "U123456"}}
mock_client.chat_postMessage.return_value = {"ok": True}
mock_client.chat_postMessage.return_value = {
"ok": True,
"ts": "1234567890.123456",
}
mock_client.chat_getPermalink.return_value = {
"ok": True,
"permalink": "https://mozilla.slack.com/archives/C123/p1234567890123456",
}
mock_client.conversations_open.return_value = {"channel": {"id": "D123456"}}

send_slack_notification(
experiment_id=self.experiment.id,
Expand All @@ -242,7 +299,7 @@ def test_send_slack_notification_channel_setting(self, mock_webclient):
],
)

call_args = mock_client.chat_postMessage.call_args
call_args = mock_client.chat_postMessage.call_args_list[0]
self.assertEqual(call_args.kwargs["channel"], "custom-channel")
self.assertEqual(call_args.kwargs["unfurl_links"], False)
self.assertEqual(call_args.kwargs["unfurl_media"], False)
Expand All @@ -253,7 +310,15 @@ def test_send_slack_notification_empty_email(self, mock_webclient):
mock_client = Mock()
mock_webclient.return_value = mock_client
mock_client.users_lookupByEmail.return_value = {"user": {"id": "U123456"}}
mock_client.chat_postMessage.return_value = {"ok": True}
mock_client.chat_postMessage.return_value = {
"ok": True,
"ts": "1234567890.123456",
}
mock_client.chat_getPermalink.return_value = {
"ok": True,
"permalink": "https://mozilla.slack.com/archives/C123/p1234567890123456",
}
mock_client.conversations_open.return_value = {"channel": {"id": "D123456"}}

send_slack_notification(
experiment_id=self.experiment.id,
Expand All @@ -264,4 +329,64 @@ def test_send_slack_notification_empty_email(self, mock_webclient):
)

mock_client.users_lookupByEmail.assert_called_once_with(email="test@example.com")
mock_client.chat_postMessage.assert_called_once()
# Should be called once for channel message and once for DM
self.assertEqual(mock_client.chat_postMessage.call_count, 2)
mock_client.chat_getPermalink.assert_called_once()
mock_client.conversations_open.assert_called_once()

@override_settings(SLACK_AUTH_TOKEN="test-token")
@patch("experimenter.slack.notification.WebClient")
def test_send_slack_notification_dm_fails(self, mock_webclient):
mock_client = Mock()
mock_webclient.return_value = mock_client
mock_client.users_lookupByEmail.return_value = {"user": {"id": "U123456"}}
mock_client.chat_postMessage.side_effect = [
{"ok": True, "ts": "1234567890.123456"}, # Channel message succeeds
SlackApiError(
message="DM failed", response={"error": "channel_not_found"}
), # DM fails
]
mock_client.chat_getPermalink.return_value = {
"ok": True,
"permalink": "https://mozilla.slack.com/archives/C123/p1234567890123456",
}
mock_client.conversations_open.return_value = {"channel": {"id": "D123456"}}

send_slack_notification(
experiment_id=self.experiment.id,
email_addresses=["test@example.com"],
action_text=NimbusConstants.SLACK_EMAIL_ACTIONS[
NimbusExperiment.EmailType.EXPERIMENT_END
],
)

# Should call chat_postMessage twice (channel + DM attempt)
self.assertEqual(mock_client.chat_postMessage.call_count, 2)
mock_client.conversations_open.assert_called_once()

@override_settings(SLACK_AUTH_TOKEN="test-token")
@patch("experimenter.slack.notification.WebClient")
def test_send_slack_notification_permalink_fails(self, mock_webclient):
mock_client = Mock()
mock_webclient.return_value = mock_client
mock_client.users_lookupByEmail.return_value = {"user": {"id": "U123456"}}
mock_client.chat_postMessage.return_value = {
"ok": True,
"ts": "1234567890.123456",
}
mock_client.chat_getPermalink.side_effect = SlackApiError(
message="Permalink failed", response={"error": "not_found"}
)
mock_client.conversations_open.return_value = {"channel": {"id": "D123456"}}

send_slack_notification(
experiment_id=self.experiment.id,
email_addresses=["test@example.com"],
action_text=NimbusConstants.SLACK_EMAIL_ACTIONS[
NimbusExperiment.EmailType.EXPERIMENT_END
],
)

# Should still send DM even if permalink fails
self.assertEqual(mock_client.chat_postMessage.call_count, 2)
mock_client.conversations_open.assert_called_once()