Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions docs/root/adding_new_slack_apps.rst
Copy link
Contributor

Choose a reason for hiding this comment

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

feel this needs to go on infradocs

Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ Adding normal slack apps
* Note 2: the configuration `bot.name` for your bot should match your app user_handle
* Note 3: if your app user_handle does not match your `bot.name` you need to
specify `match_mention: true` to receive callbacks
* Note 4: if you want your app to handle thread replies, you need to
specify `match_reply: true` to receive callbacks


#. Add interactive component configuration (if the bot needs interactive components), using the ``Interactive Components`` link in the sidebar:
Expand Down
3 changes: 3 additions & 0 deletions omnibot/processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@ def _process_message_message_handlers(message: Message):
command_matched = False
handler_called = False
for handler in bot.message_handlers:
# We only match replies if the handler is set to match them
if message.thread_ts and not handler.get("match_reply", False):
continue
# We only match commands against directed messages
if handler["match_type"] == "command":
if not _should_handle_command(handler, message):
Expand Down
16 changes: 12 additions & 4 deletions omnibot/services/slack/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ def __init__(self, bot: Bot, event: dict, event_trace: dict):
super().__init__(bot, event, event_trace, "message")
self._payload["ts"] = event["ts"]
self._payload["thread_ts"] = event.get("thread_ts")
self._payload["parent_user_id"] = event.get("parent_user_id")
Copy link
Contributor

Choose a reason for hiding this comment

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

what is a parent_user_id?

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not reference to parent message

message.parent.user.id

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is the user id for the first message in the thread and exists for thread replies but not for regular messages

self._check_unsupported()
try:
self._payload["text"] = event["text"]
Expand Down Expand Up @@ -50,10 +51,6 @@ def _check_unsupported(self):
elif self.user and self.user == "USLACKBOT":
logger.debug("ignoring message from slackbot", extra=self.event_trace)
unsupported = True
# Ignore threads
elif self.thread_ts:
logger.debug("ignoring thread message", extra=self.event_trace)
unsupported = True
# For now, ignore all event subtypes
elif self.subtype:
extra = {"subtype": self.subtype}
Expand Down Expand Up @@ -201,8 +198,19 @@ def mentioned(self):

@property
def thread_ts(self):
"""
The timestamp of the parent message, if this is a reply to a message.
:return:
Copy link

Copilot AI Jul 21, 2025

Choose a reason for hiding this comment

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

The docstring for thread_ts property is missing return type information and doesn't document that it can return None when not in a thread.

Suggested change
:return:
:return: The timestamp of the parent message as a string, or None if the message is not part of a thread.
:rtype: Optional[str]

Copilot uses AI. Check for mistakes.
"""
return self._payload["thread_ts"]

@property
def parent_user_id(self):
"""
The user_id of the parent message, if this is a reply to a message.
"""
return self._payload["parent_user_id"]
Copy link

Copilot AI Jul 21, 2025

Choose a reason for hiding this comment

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

The parent_user_id property can return None when parent_user_id is not present in the event, but this isn't documented in the docstring. Consider adding a return type annotation and documenting the None case.

Copilot uses AI. Check for mistakes.

@property
def channels(self):
return self._payload.get("channels", {})
Expand Down
5 changes: 0 additions & 5 deletions tests/unit/omnibot/services/slack/message_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,11 +124,6 @@ def test_message(mocker):
with pytest.raises(MessageUnsupportedError):
_message = Message(_bot, event_copy, event_trace)

event_copy = copy.deepcopy(event)
event_copy["thread_ts"] = "1234568.00"
with pytest.raises(MessageUnsupportedError):
_message = Message(_bot, event_copy, event_trace)

event_copy = copy.deepcopy(event)
event_copy["subtype"] = "some_subtype"
with pytest.raises(MessageUnsupportedError):
Expand Down