Skip to content

Conversation

@david-lyft
Copy link
Contributor

@david-lyft david-lyft commented May 29, 2025

  • Changes field name to align with other match types
  • Sets args for reaction match_type for route rule matching
  • Added event_type to payload, for reaction types this will allow callbacks to differentiate between reaction_added and reaction_removed events

@david-lyft david-lyft requested review from colinsl and Copilot May 29, 2025 17:05
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves the reaction message handler by renaming the field from "reaction" to "emoji_name" and updating the associated properties and matching logic. It also adds support for setting args for route rule matching and updates processor logic to use the new field name.

  • Renames the Slack reaction payload field to "emoji_name".
  • Adds a new "args" property and updates routing match logic.
  • Updates processor logic to use the "emoji_name" instead of the old "reaction" field.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
omnibot/services/slack/reaction.py Renames "reaction" field to "emoji_name" and updates error handling.
omnibot/services/slack/base_message.py Adds "event_type" to payload and new "args" property; adapts set_match for reaction.
omnibot/processor.py Adjusts regex matching to use the "emoji_name" property.
Comments suppressed due to low confidence (1)

omnibot/services/slack/reaction.py:22

  • The exception handler in the reaction event block no longer re-raises the exception after logging an error. This change may silently suppress errors when the 'reaction' field is missing; consider re-raising the exception to maintain consistent error propagation.
except Exception:

raise
try:
self._payload["reaction"] = event["reaction"]
self._payload["emoji_name"] = event["reaction"]
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need to convert this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reaction is used for the match, similar for command (uses commandfield) and regex (uses regexfield) match_type so we need a different field name for this

@david-lyft david-lyft merged commit 94c85e7 into master May 30, 2025
5 checks passed
@david-lyft david-lyft deleted the INTPROD-9641_2 branch May 30, 2025 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants