Skip to content

Commit

Permalink
refactor: concatenate subject and message in broadcaster()
Browse files Browse the repository at this point in the history
  • Loading branch information
louisevelayo committed Dec 4, 2023
1 parent 9aa4bd7 commit 23b1638
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 16 deletions.
10 changes: 5 additions & 5 deletions node_monitor/bot_slack.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,16 @@ class SlackBot:

def __init__(self, slack_token: str) -> None:
self.client = slack_sdk.WebClient(token=slack_token)


def send_message(
self, slack_channel_name: str, subject: str,
self, slack_channel_name: str,
message: str) -> None | SlackApiError:
"""Send a message to a single Slack channel."""
dispatch = f"{subject}\n\n{message}"
try:
self.client.chat_postMessage(
channel=slack_channel_name,
text=dispatch)
text=message)
except SlackApiError as e:
# You will get a SlackApiError if "ok" is False
# If ok is False, e.response["error"] contains a
Expand All @@ -28,13 +28,13 @@ def send_message(


def send_messages(
self, slack_channel_names: List[str], subject: str,
self, slack_channel_names: List[str],
message: str) -> None | SlackApiError:
"""Send a message to multiple Slack channels."""
# Propagate the last error that occurs
err = None
for slack_channel_name in slack_channel_names:
this_err = self.send_message(slack_channel_name, subject, message)
this_err = self.send_message(slack_channel_name, message)
if this_err is not None:
err = this_err
return err
13 changes: 8 additions & 5 deletions node_monitor/bot_telegram.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,15 @@ def __init__(self, telegram_token: str) -> None:


def send_message(
self, chat_id: str, subject: str, message: str
self, chat_id: str, message: str
) -> None | requests.exceptions.HTTPError:
"""Send a message to a single Telegram chat."""
dispatch = f"{subject}\n\n{message}"
max_message_length = 4096
message_parts = [dispatch[i:i + max_message_length] for i in range(0, len(dispatch), max_message_length)]

# TODO: use itertools.batched here when python version is updated to >=3.12.
message_parts = [
message[i:i + max_message_length]
for i in range(0, len(message), max_message_length)]

try:
for part in message_parts:
Expand All @@ -30,13 +33,13 @@ def send_message(


def send_messages(
self, chat_ids: List[str], subject: str, message: str
self, chat_ids: List[str], message: str
) -> None | requests.exceptions.HTTPError:
"""Send a message to multiple Telegram chats."""
# Propagate the last error that occurs
err = None
for chat_id in chat_ids:
this_err = self.send_message(chat_id, subject, message)
this_err = self.send_message(chat_id, message)
if this_err is not None:
err = this_err
return err
Expand Down
5 changes: 3 additions & 2 deletions node_monitor/node_monitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ def broadcaster(node_provider_id: str,
"""Broadcasts a generic message to a subscriber through their
selected communication channel(s)."""
preferences = subscribers[node_provider_id]
dispatch = f"{subject}\n\n{message}"
if preferences['notify_email'] == True:
recipients = email_recipients.get(node_provider_id, [])
if recipients:
Expand All @@ -125,12 +126,12 @@ def broadcaster(node_provider_id: str,
if self.slack_bot:
channels = slack_channels.get(node_provider_id, [])
if channels:
err1 = self.slack_bot.send_messages(channels, subject, message)
err1 = self.slack_bot.send_messages(channels, dispatch)
if preferences['notify_telegram'] == True:
if self.telegram_bot:
chats = telegram_chats.get(node_provider_id, [])
if chats:
err2 = self.telegram_bot.send_messages(chats, subject, message)
err2 = self.telegram_bot.send_messages(chats, dispatch)
return None

return broadcaster
Expand Down
6 changes: 4 additions & 2 deletions tests/test_bot_slack.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@ def test_send_message(mock_web_client):
expected_channel = "#node-monitor"
expected_subject = "Subject message"
expected_message = "Hello, Slack!"
dispatch = f"{expected_subject}\n\n{expected_message}"

slack_bot = SlackBot(c.TOKEN_SLACK)
slack_bot.send_message(expected_channel, expected_subject, expected_message)
slack_bot.send_message(expected_channel, dispatch)

mock_client.chat_postMessage.assert_called_once_with(
channel=expected_channel,
Expand All @@ -31,10 +32,11 @@ def test_send_message_slack(fake_data):
slack_channel_name = "node-monitor"

subject, message = messages.nodes_compromised_message([fakenode], fakelabel)
dispatch = f"{subject}\n\n{message}"

## SlackBot.send_message() normally returns an error without raising
## an exception to prevent NodeMonitor from crashing if the message
## fails to send. We make sure to raise it here to purposely fail the test.
err = slack_bot.send_message(slack_channel_name, subject, message)
err = slack_bot.send_message(slack_channel_name, dispatch)
if err is not None:
raise err
6 changes: 4 additions & 2 deletions tests/test_bot_telegram.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,15 @@ def test_send_message(mock_post):
chat_id = "1234567890"
subject = "Test subject"
message = "Test message"
dispatch = f"{subject}\n\n{message}"
payload = {
"chat_id": chat_id,
"text": f"{subject}\n\n{message}"
}
mock_response = mock_post.return_value
mock_response.raise_for_status.return_value = None

telegram_bot.send_message(chat_id, subject, message)
telegram_bot.send_message(chat_id, dispatch)

mock_post.assert_called_once_with(
f"https://api.telegram.org/bot{telegram_bot.telegram_token}/sendMessage",
Expand All @@ -38,7 +39,8 @@ def test_send_live_message(fake_data):
chat_id = "-1001925583150"

subject, message = messages.nodes_compromised_message([fakenode], fakelabel)
dispatch = f"{subject}\n\n{message}"

err = telegram_bot.send_message(chat_id, subject, message)
err = telegram_bot.send_message(chat_id, dispatch)
if err is not None:
raise err

0 comments on commit 23b1638

Please sign in to comment.