Allow support channel settings to be either channel name OR channel ID

bennettbot/bot.py

@@ -12,6 +12,7 @@
 from workspace.techsupport.jobs import get_dates_from_config as get_tech_support_dates
 
 from . import job_configs, scheduler, settings
+from .config import get_support_config
 from .logger import log_call, logger
 from .slack import notify_slack
 
@@ -108,27 +109,23 @@ def register_listeners(app, config, channels, bot_user_id, internal_user_ids):
     The listeners are defined inside this function to allow different config to be
     passed in for tests.
     """
-
-    bennett_admins_channel_id = channels[settings.SLACK_BENNETT_ADMINS_CHANNEL]
-    # Match "bennett-admins" or "bennet-admins" as a word (treating hyphens as
-    # word characters), except if it's preceded by a slash to avoid matching it
-    # in URLs
-    bennett_admins_regex = re.compile(
-        r".*(^|[^\w\-/])bennett?-admins($|[^\w\-]).*", flags=re.I
-    )
-
-    tech_support_channel_id = channels[settings.SLACK_TECH_SUPPORT_CHANNEL]
-    # Match "tech-support" as a word (treating hyphens as word characters), except if
-    # it's preceded by a slash to avoid matching it in URLs
-    tech_support_regex = re.compile(
-        r".*(^|[^\w\-/])tech-support($|[^\w\-]).*", flags=re.I
-    )
+    support_config = get_support_config(channels)
+    # Check that channel settings mapped to valid channel IDs
+    for support in support_config.values():
+        if support["support_channel"] not in channels.values():
+            raise ValueError(
+                f"{support['keyword']} channel id '{support['support_channel']}' not found"
+            )
 
     @app.event(
         "app_mention",
         # Don't match app mentions that include tech support keywords; these will be
         # matched by the tech-support listener
-        matchers=[lambda event: not tech_support_regex.match(event["text"])],
+        matchers=[
+            lambda event: not support_config["tech-support"]["regex"].match(
+                event["text"]
+            )
+        ],
     )
     def job_listener(event, say, ack):
         """Respond to every Slack message that mentions the bot (and is not a
@@ -147,7 +144,7 @@ def job_listener(event, say, ack):
         # only match DMs with the bot that are non-tech support messages
         matchers=[
             lambda message: message["channel_type"] == "im"
-            and not tech_support_regex.match(message["text"])
+            and not support_config["tech-support"]["regex"].match(message["text"])
         ],
     )
     def im_job_listener(event, say, ack):
@@ -231,12 +228,12 @@ def _listener(event, say, is_im=False):
         include_apology = text != "help"
         handle_help(event, say, config, include_apology)
 
-    def build_matcher(regex, channel_id):
+    def build_matcher(*, regex, support_channel, **kwargs):
         """Builds matcher to match messages matching given regex but not posted in given channel."""
 
         def matcher(event):
             # Only match messages posted outside of the channel itself
-            if event["channel"] == channel_id:
+            if event["channel"] == support_channel:
                 return False
             # only match messages that are not posted by a bot, to avoid reposting reminders etc
             # (the event dict will include the key "bot_id")
@@ -256,23 +253,25 @@ def matcher(event):
 
     @app.event(
         {"type": "message"},
-        matchers=[build_matcher(bennett_admins_regex, bennett_admins_channel_id)],
+        matchers=[build_matcher(**support_config["bennett-admins"])],
     )
     def repost_to_bennett_admins(event, say, ack):
         repost_support_request_to_channel(
-            event, say, ack, "bennett-admins", bennett_admins_channel_id
+            event, say, ack, **support_config["bennett-admins"]
         )
 
     @app.event(
         {"type": "message"},
-        matchers=[build_matcher(tech_support_regex, tech_support_channel_id)],
+        matchers=[build_matcher(**support_config["tech-support"])],
     )
     def repost_to_tech_support(event, say, ack):
         repost_support_request_to_channel(
-            event, say, ack, "tech-support", tech_support_channel_id
+            event, say, ack, **support_config["tech-support"]
         )
 
-    def repost_support_request_to_channel(event, say, ack, keyword, channel_id):
+    def repost_support_request_to_channel(
+        event, say, ack, *, keyword, reaction, support_channel, **kwargs
+    ):
         # acknowledge the messages
         ack()
         # Our matcher filters only allows messages with no subtype (i.e. just
@@ -290,7 +289,6 @@ def repost_support_request_to_channel(event, say, ack, keyword, channel_id):
             # an exception; if this happens, then the user is editing something
             # other than the keyword in the message, and we don't need to repost
             # it again. We let the default error handler will deal with it.
-            reaction = {"tech-support": "sos", "bennett-admins": "flamingo"}[keyword]
             app.client.reactions_add(
                 channel=channel, timestamp=message["ts"], name=reaction
             )
@@ -309,7 +307,7 @@ def repost_support_request_to_channel(event, say, ack, keyword, channel_id):
             message_url = app.client.chat_getPermalink(
                 channel=channel, message_ts=message["ts"]
             )["permalink"]
-            say(message_url, channel=channel_id)
+            say(message_url, channel=support_channel)
         else:
             say(
                 f"Sorry, I can't call {keyword} from this conversation.",

bennettbot/config.py

@@ -0,0 +1,39 @@
+import re
+
+from . import settings
+
+
+def get_support_config(channels=None):
+    channels = channels or {}
+    return {
+        "tech-support": {
+            "keyword": "tech-support",
+            # Use the support setting (a channel name or channel ID) to get the channel.
+            # We first attempt to retrieve channel ID from the channels dict (if provided),
+            # otherwise we default to using the setting value as is.
+            "support_channel": channels.get(
+                settings.SLACK_TECH_SUPPORT_CHANNEL, settings.SLACK_TECH_SUPPORT_CHANNEL
+            ),
+            # Match "tech-support" as a word (treating hyphens as word characters), except if
+            # it's preceded by a slash to avoid matching it in URLs
+            "regex": re.compile(r".*(^|[^\w\-/])tech-support($|[^\w\-]).*", flags=re.I),
+            "reaction": "sos",
+        },
+        "bennett-admins": {
+            "keyword": "bennett-admins",
+            # Use the support setting (a channel name or channel ID) to get the channel.
+            # We first attempt to retrieve channel ID from the channels dict (if provided),
+            # otherwise we default to using the setting value as is.
+            "support_channel": channels.get(
+                settings.SLACK_BENNETT_ADMINS_CHANNEL,
+                settings.SLACK_BENNETT_ADMINS_CHANNEL,
+            ),
+            # Match "bennett-admins" or "bennet-admins" as a word (treating hyphens as
+            # word characters), except if it's preceded by a slash to avoid matching it
+            # in URLs
+            "regex": re.compile(
+                r".*(^|[^\w\-/])bennett?-admins($|[^\w\-]).*", flags=re.I
+            ),
+            "reaction": "flamingo",
+        },
+    }

bennettbot/dispatcher.py

@@ -13,6 +13,7 @@
 import requests
 
 from . import job_configs, scheduler, settings
+from .config import get_support_config
 from .logger import logger
 from .slack import notify_slack, slack_web_client
 
@@ -211,16 +212,7 @@ def __init__(self, bot_slack_client, user_slack_client):
         self.bot_slack_client = bot_slack_client
         self.user_slack_client = user_slack_client
 
-        self.config = {
-            "tech-support": {
-                "reaction": "sos",
-                "channel": settings.SLACK_TECH_SUPPORT_CHANNEL,
-            },
-            "bennett-admins": {
-                "reaction": "flamingo",
-                "channel": settings.SLACK_BENNETT_ADMINS_CHANNEL,
-            },
-        }
+        self.config = get_support_config()
 
     def run_check(self):  # pragma: no cover
         """Start running the check in a new subprocess."""
@@ -248,7 +240,7 @@ def do_check(self, run_fn=lambda: True, delay=10):  # pragma: no branch
     def check_messages(self, keyword, after):
         logger.debug("Checking %s messages", keyword)
         reaction = self.config[keyword]["reaction"]
-        channel = self.config[keyword]["channel"]
+        channel = self.config[keyword]["support_channel"]
         messages = self.user_slack_client.search_messages(
             query=(
                 # Search for messages with the keyword but without the expected reaction

tests/test_bot.py

@@ -56,6 +56,52 @@ def test_joined_channels(mock_app):
     )
 
 
+@httpretty.activate(allow_net_connect=False)
+@pytest.mark.parametrize(
+    "setting,value",
+    [
+        # Note: channel names are set in pyproject.toml
+        # Channel ids are defined by the response from the response to
+        # the conversations.list endpoint, defined in mock_http_request.py
+        # support channel settings can be a channel name
+        ("SLACK_TECH_SUPPORT_CHANNEL", "techsupport"),
+        ("SLACK_BENNETT_ADMINS_CHANNEL", "bennettadmins"),
+        # support channel settings can be a channel ID
+        ("SLACK_TECH_SUPPORT_CHANNEL", "C0001"),
+        ("SLACK_BENNETT_ADMINS_CHANNEL", "C0000"),
+    ],
+)
+def test_register_listeners_support_channel_settings(setting, value):
+    register_bot_uris()
+    app = App(raise_error_for_unhandled_request=True)
+    channels = bot.get_channels(app.client)
+    bot_user_id, internal_user_ids = bot.get_users_info(app.client)
+    bot.join_all_channels(app.client, channels, bot_user_id)
+    with patch(f"bennettbot.settings.{setting}", value):
+        bot.register_listeners(app, config, channels, bot_user_id, internal_user_ids)
+
+
+@httpretty.activate(allow_net_connect=False)
+@pytest.mark.parametrize(
+    "setting,value",
+    [
+        ("SLACK_TECH_SUPPORT_CHANNEL", "foo"),
+        ("SLACK_BENNETT_ADMINS_CHANNEL", "C9999"),
+    ],
+)
+def test_register_listeners_invalid_support_channel_settings(setting, value):
+    register_bot_uris()
+    app = App(raise_error_for_unhandled_request=True)
+    channels = bot.get_channels(app.client)
+    bot_user_id, internal_user_ids = bot.get_users_info(app.client)
+    bot.join_all_channels(app.client, channels, bot_user_id)
+    with (
+        patch(f"bennettbot.settings.{setting}", value),
+        pytest.raises(ValueError, match=f"channel id '{value}' not found"),
+    ):
+        bot.register_listeners(app, config, channels, bot_user_id, internal_user_ids)
+
+
 def test_schedule_job(mock_app):
     handle_message(mock_app, "<@U1234> test do job 10")
 

tests/test_dispatcher.py

@@ -524,21 +524,6 @@ def build_log_dir(job_type_with_namespace):
     )
 
 
-def test_message_checker_config():
-    checker = MessageChecker(slack_web_client("bot"), slack_web_client("user"))
-    # channel IDs are retrieved from mock_web_api_server
-    assert checker.config == {
-        "tech-support": {
-            "reaction": "sos",
-            "channel": settings.SLACK_TECH_SUPPORT_CHANNEL,
-        },
-        "bennett-admins": {
-            "reaction": "flamingo",
-            "channel": settings.SLACK_BENNETT_ADMINS_CHANNEL,
-        },
-    }
-
-
 def test_message_checker_run(freezer):
     freezer.move_to("2024-10-08 23:30")
     httpretty_register(