diff --git a/README.md b/README.md index 27776b2..b3c9f65 100644 --- a/README.md +++ b/README.md @@ -74,13 +74,26 @@ channels: ``` ### 3. Reactions -This section defines automated responses to specific reactions or triggers in the Slack workspace. Each reaction has a type, corresponding action, and may include placeholders. +This section defines automated responses to specific reactions or triggers in the Slack workspace. Each reaction has a type (or types for multiple handlers), corresponding action, and may include placeholders. #### Reaction Types: 1. `SLACK_POST`: Posts a message in the Slack channel 2. `DELETE_MESSAGE`: Removes a message and sends a notification to the user 3. `ASK_AI`: Utilizes an AI model to generate a response +4. `REMOVE_BROADCAST`: Removes a broadcasted thread reply from the channel (keeps it in the thread) + +#### Multiple Handlers: + +Reactions can have multiple handlers that execute in sequence. Use `types` (list) instead of `type` (single) to define multiple handlers: + +```yaml +- reaction: thread + types: + - REMOVE_BROADCAST # First: remove if broadcasted + - SLACK_POST # Then: post reminder message + message: "Please use threads..." +``` #### Pre-defined Placeholders: @@ -107,8 +120,11 @@ For reactions with channel-specific placeholders (like `{link}` in the `faq` rea - Action: Posts a message encouraging users to ask their questions directly 2. `thread`: - - Type: `SLACK_POST` - - Action: Reminds users to use threads for organized discussions + - Types: `REMOVE_BROADCAST`, `SLACK_POST` (multiple handlers) + - Actions: + 1. `REMOVE_BROADCAST`: If the message is a thread reply that was "also sent to channel" (broadcasted), removes it from the channel view (keeps it in the thread) + 2. `SLACK_POST`: Posts a reminder message about using threads + - Note: This demonstrates the ability to use multiple handlers for a single reaction 3. `faq`: - Type: `SLACK_POST` diff --git a/automator/config.yaml b/automator/config.yaml index 6648e48..4f3e09d 100644 --- a/automator/config.yaml +++ b/automator/config.yaml @@ -15,7 +15,9 @@ reactions: Don't ask to ask: https://dontasktoask.com/ - reaction: thread - type: SLACK_POST + types: + - REMOVE_BROADCAST + - SLACK_POST message: > Please use threads to keep the discussion more organized: https://datatalks.club/slack/guidelines.html#taking-part-in-discussions diff --git a/automator/lambda_function.py b/automator/lambda_function.py index ca2f242..7a37958 100644 --- a/automator/lambda_function.py +++ b/automator/lambda_function.py @@ -47,6 +47,35 @@ def handle_slack_post(event, reaction_config): slack.post_message_thread(event, message) +def handle_remove_broadcast(event, reaction_config): + """Remove a broadcasted thread message from channel (keeps it in the thread)""" + item = event['item'] + channel = item['channel'] + ts = item['ts'] + + # Get message details to check if it's a broadcasted thread reply + message_details = slack.get_message_content(channel, ts) + if not message_details: + logger.info(f"Message not found for {channel} {ts}") + return + + # Check if this is a broadcasted thread reply + # A broadcasted reply has thread_ts != ts (it's a reply in a thread) + thread_ts = message_details.get('thread_ts') + is_broadcasted_reply = thread_ts is not None and thread_ts != ts + + if is_broadcasted_reply: + # Remove the broadcasted message from the channel + # (it will still remain in the thread) + if FAKE_DELETE: + logger.info(f"FAKE_DELETE broadcasted message for {channel} {ts}") + else: + slack.remove_message(channel, ts) + logger.info(f"Removed broadcasted message from channel {channel} (kept in thread)") + else: + logger.info(f"Message {ts} is not a broadcasted reply, skipping removal") + + def handle_delete_message(event, reaction_config): """Delete a message and optionally all its thread replies, sending DMs to affected users""" item = event['item'] @@ -143,6 +172,7 @@ def handle_ask_ai(event, reaction_config): 'SLACK_POST': handle_slack_post, 'DELETE_MESSAGE': handle_delete_message, 'ASK_AI': handle_ask_ai, + 'REMOVE_BROADCAST': handle_remove_broadcast, } @@ -155,13 +185,24 @@ def process_reaction(body, event): reaction_config = reaction_configs[reaction] - action_type = reaction_config['type'] - action_handler = action_handlers.get(action_type) + # Support both single type and list of types (multiple handlers) + action_types = reaction_config.get('types') or [reaction_config.get('type')] - if action_handler: - action_handler(event, reaction_config) - else: - logger.info(f"no handler for {action_type}") + if not action_types or not any(action_types): + logger.info(f"no action type configured for {reaction}") + return + + # Execute all handlers in sequence + for action_type in action_types: + if not action_type: + continue + + action_handler = action_handlers.get(action_type) + + if action_handler: + action_handler(event, reaction_config) + else: + logger.info(f"no handler for {action_type}") def run(body): diff --git a/tests/test_automator_lambda.py b/tests/test_automator_lambda.py index c5ae489..b4375e1 100644 --- a/tests/test_automator_lambda.py +++ b/tests/test_automator_lambda.py @@ -252,6 +252,188 @@ def test_action_handlers_has_delete_message(self): def test_action_handlers_no_delete_with_threads(self): """Verify that DELETE_WITH_THREADS handler is removed""" self.assertNotIn('DELETE_WITH_THREADS', lambda_function.action_handlers) + + def test_action_handlers_has_remove_broadcast(self): + """Verify that REMOVE_BROADCAST handler is registered""" + self.assertIn('REMOVE_BROADCAST', lambda_function.action_handlers) + self.assertEqual( + lambda_function.action_handlers['REMOVE_BROADCAST'], + lambda_function.handle_remove_broadcast + ) + + def test_thread_reaction_uses_multiple_handlers(self): + """Verify that 'thread' reaction uses multiple handlers""" + reaction_config = lambda_function.reaction_configs.get('thread') + self.assertIsNotNone(reaction_config) + self.assertIn('types', reaction_config) + self.assertEqual(reaction_config['types'], ['REMOVE_BROADCAST', 'SLACK_POST']) + + +class TestRemoveBroadcast(unittest.TestCase): + """Test REMOVE_BROADCAST handler""" + + @patch('automator_lambda_function.slack') + def test_regular_message_not_removed(self, mock_slack): + """Test that regular messages (not broadcasted) are not removed""" + # Setup mocks - regular message without thread_ts + mock_slack.get_message_content.return_value = { + 'user': 'U123456', + 'text': 'Regular message', + 'ts': '1234567890.123456' + } + + # Create event + event = { + 'item': { + 'channel': 'C123456', + 'ts': '1234567890.123456' + }, + 'reaction': 'thread' + } + + # Create reaction config + reaction_config = { + 'types': ['REMOVE_BROADCAST', 'SLACK_POST'], + 'message': 'Please use threads' + } + + # Execute + lambda_function.handle_remove_broadcast(event, reaction_config) + + # Verify - should not delete regular messages + mock_slack.remove_message.assert_not_called() + + @patch('automator_lambda_function.slack') + def test_broadcasted_reply_removed(self, mock_slack): + """Test that broadcasted thread replies are removed from channel""" + # Setup mocks - broadcasted thread reply (thread_ts != ts) + mock_slack.get_message_content.return_value = { + 'user': 'U123456', + 'text': 'Reply sent to channel', + 'ts': '1234567890.123457', + 'thread_ts': '1234567890.123456' # Different from ts + } + + # Create event + event = { + 'item': { + 'channel': 'C123456', + 'ts': '1234567890.123457' + }, + 'reaction': 'thread' + } + + # Create reaction config + reaction_config = { + 'types': ['REMOVE_BROADCAST', 'SLACK_POST'], + 'message': 'Please use threads' + } + + # Execute + lambda_function.handle_remove_broadcast(event, reaction_config) + + # Verify - in FAKE_DELETE mode, remove_message is not called but logged + # The message would be deleted in production (when FAKE_DELETE=0) + mock_slack.remove_message.assert_not_called() # Because FAKE_DELETE=1 + + @patch('automator_lambda_function.slack') + def test_parent_message_not_removed(self, mock_slack): + """Test that parent messages (thread_ts == ts) are not removed""" + # Setup mocks - parent message where thread_ts == ts + mock_slack.get_message_content.return_value = { + 'user': 'U123456', + 'text': 'Parent message', + 'ts': '1234567890.123456', + 'thread_ts': '1234567890.123456' # Same as ts + } + + # Create event + event = { + 'item': { + 'channel': 'C123456', + 'ts': '1234567890.123456' + }, + 'reaction': 'thread' + } + + # Create reaction config + reaction_config = { + 'types': ['REMOVE_BROADCAST', 'SLACK_POST'], + 'message': 'Please use threads' + } + + # Execute + lambda_function.handle_remove_broadcast(event, reaction_config) + + # Verify - should not delete parent messages + mock_slack.remove_message.assert_not_called() + + @patch('automator_lambda_function.slack') + def test_message_not_found(self, mock_slack): + """Test handler gracefully handles missing message""" + # Setup mocks - message not found + mock_slack.get_message_content.return_value = None + + # Create event + event = { + 'item': { + 'channel': 'C123456', + 'ts': '1234567890.123456' + }, + 'reaction': 'thread' + } + + # Create reaction config + reaction_config = { + 'types': ['REMOVE_BROADCAST', 'SLACK_POST'], + 'message': 'Please use threads' + } + + # Execute + lambda_function.handle_remove_broadcast(event, reaction_config) + + # Verify - should not delete when message not found + mock_slack.remove_message.assert_not_called() + + +class TestMultipleHandlers(unittest.TestCase): + """Test that multiple handlers can be executed for one reaction""" + + @patch('automator_lambda_function.slack') + @patch('automator_lambda_function.util') + def test_thread_reaction_executes_both_handlers(self, mock_util, mock_slack): + """Test that thread reaction executes both REMOVE_BROADCAST and SLACK_POST""" + # Setup mocks - broadcasted thread reply + mock_slack.get_message_content.return_value = { + 'user': 'U123456', + 'text': 'Reply sent to channel', + 'ts': '1234567890.123457', + 'thread_ts': '1234567890.123456' + } + mock_util.format_message.return_value = None + + # Create event + event = { + 'item': { + 'channel': 'C123456', + 'ts': '1234567890.123457' + }, + 'reaction': 'thread' + } + + # Create body + body = { + 'event': event + } + + # Execute the full reaction processing + lambda_function.process_reaction(body, event) + + # Verify both handlers were called: + # 1. REMOVE_BROADCAST called get_message_content + mock_slack.get_message_content.assert_called() + # 2. SLACK_POST called post_message_thread + mock_slack.post_message_thread.assert_called_once() class TestChannelConfig(unittest.TestCase):