From d937f1cee6846cb5bc4af34df03b90d47b931d65 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 17 Jan 2026 15:16:31 +0000 Subject: [PATCH 1/4] Initial plan From cb5c767dd5bb3b8ed382104449efe336a51a011d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 17 Jan 2026 15:19:34 +0000 Subject: [PATCH 2/4] Fix thread reaction to post to original thread for broadcasted messages Co-authored-by: alexeygrigorev <875246+alexeygrigorev@users.noreply.github.com> --- automator/lambda_function.py | 12 +++ tests/test_automator_lambda.py | 132 +++++++++++++++++++++++++++++++++ 2 files changed, 144 insertions(+) diff --git a/automator/lambda_function.py b/automator/lambda_function.py index 7a37958..ae1b57e 100644 --- a/automator/lambda_function.py +++ b/automator/lambda_function.py @@ -32,6 +32,7 @@ def get_channel_name(channel_id): def handle_slack_post(event, reaction_config): channel_id = event['item']['channel'] channel_name = get_channel_name(channel_id) + ts = event['item']['ts'] if 'placeholders' in reaction_config: message = util.format_message( @@ -44,6 +45,17 @@ def handle_slack_post(event, reaction_config): else: message = reaction_config['message'] + # Check if this is a broadcasted thread reply + message_details = slack.get_message_content(channel_id, ts) + if message_details: + thread_ts = message_details.get('thread_ts') + # If it's a broadcasted reply (thread_ts != ts), use the original thread + if thread_ts is not None and thread_ts != ts: + # Modify event to use the original thread_ts + event = event.copy() + event['item'] = event['item'].copy() + event['item']['ts'] = thread_ts + slack.post_message_thread(event, message) diff --git a/tests/test_automator_lambda.py b/tests/test_automator_lambda.py index b4375e1..663369d 100644 --- a/tests/test_automator_lambda.py +++ b/tests/test_automator_lambda.py @@ -436,6 +436,138 @@ def test_thread_reaction_executes_both_handlers(self, mock_util, mock_slack): mock_slack.post_message_thread.assert_called_once() +class TestSlackPost(unittest.TestCase): + """Test SLACK_POST handler for different message types""" + + @patch('automator_lambda_function.slack') + def test_regular_message_post(self, mock_slack): + """Test that regular messages post to the correct thread""" + # 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 = { + 'message': 'Please use threads' + } + + # Execute + lambda_function.handle_slack_post(event, reaction_config) + + # Verify - should post to the same ts (the message itself) + mock_slack.post_message_thread.assert_called_once() + call_args = mock_slack.post_message_thread.call_args + posted_event = call_args[0][0] + self.assertEqual(posted_event['item']['ts'], '1234567890.123456') + + @patch('automator_lambda_function.slack') + def test_broadcasted_reply_post_to_original_thread(self, mock_slack): + """Test that broadcasted thread replies post to the original thread""" + # 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 - this is the original thread + } + + # Create event - reacting to the broadcasted message + event = { + 'item': { + 'channel': 'C123456', + 'ts': '1234567890.123457' # The broadcasted message ts + }, + 'reaction': 'thread' + } + + # Create reaction config + reaction_config = { + 'message': 'Please use threads' + } + + # Execute + lambda_function.handle_slack_post(event, reaction_config) + + # Verify - should post to the original thread_ts, not the broadcast message ts + mock_slack.post_message_thread.assert_called_once() + call_args = mock_slack.post_message_thread.call_args + posted_event = call_args[0][0] + # The key fix: should use thread_ts (parent) not the broadcast message ts + self.assertEqual(posted_event['item']['ts'], '1234567890.123456') + + @patch('automator_lambda_function.slack') + def test_thread_parent_message_post(self, mock_slack): + """Test that parent thread messages post to themselves""" + # 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 - this is the thread parent + } + + # Create event + event = { + 'item': { + 'channel': 'C123456', + 'ts': '1234567890.123456' + }, + 'reaction': 'thread' + } + + # Create reaction config + reaction_config = { + 'message': 'Please use threads' + } + + # Execute + lambda_function.handle_slack_post(event, reaction_config) + + # Verify - should post to the same ts (creates a thread on this message) + mock_slack.post_message_thread.assert_called_once() + call_args = mock_slack.post_message_thread.call_args + posted_event = call_args[0][0] + self.assertEqual(posted_event['item']['ts'], '1234567890.123456') + + @patch('automator_lambda_function.slack') + def test_message_not_found_gracefully_handled(self, mock_slack): + """Test that missing messages are handled gracefully""" + # 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 = { + 'message': 'Please use threads' + } + + # Execute + lambda_function.handle_slack_post(event, reaction_config) + + # Verify - should still post (fallback to original behavior) + mock_slack.post_message_thread.assert_called_once() + + class TestChannelConfig(unittest.TestCase): """Test that channel configurations are correct""" From 27e16ac8c7a08b1613004ac7c9444d065910f6dd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 17 Jan 2026 15:21:15 +0000 Subject: [PATCH 3/4] Add comprehensive tests for broadcasted message handling Co-authored-by: alexeygrigorev <875246+alexeygrigorev@users.noreply.github.com> --- tests/test_automator_lambda.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/test_automator_lambda.py b/tests/test_automator_lambda.py index 663369d..ecdcc4f 100644 --- a/tests/test_automator_lambda.py +++ b/tests/test_automator_lambda.py @@ -434,6 +434,13 @@ def test_thread_reaction_executes_both_handlers(self, mock_util, mock_slack): mock_slack.get_message_content.assert_called() # 2. SLACK_POST called post_message_thread mock_slack.post_message_thread.assert_called_once() + + # Verify the message was posted to the ORIGINAL thread, not the broadcast message + call_args = mock_slack.post_message_thread.call_args + posted_event = call_args[0][0] + self.assertEqual(posted_event['item']['ts'], '1234567890.123456', + "Message should be posted to original thread, not broadcast message") + class TestSlackPost(unittest.TestCase): From 8e87dd951ee66ca7a2e6ca57e54b92070f678023 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 17 Jan 2026 18:15:09 +0000 Subject: [PATCH 4/4] Revert broadcast handling and add thread_please reaction Co-authored-by: alexeygrigorev <875246+alexeygrigorev@users.noreply.github.com> --- automator/config.yaml | 21 +++- automator/lambda_function.py | 12 --- tests/test_automator_lambda.py | 180 +++++---------------------------- 3 files changed, 42 insertions(+), 171 deletions(-) diff --git a/automator/config.yaml b/automator/config.yaml index 4f3e09d..121e37a 100644 --- a/automator/config.yaml +++ b/automator/config.yaml @@ -15,13 +15,28 @@ reactions: Don't ask to ask: https://dontasktoask.com/ - reaction: thread - types: - - REMOVE_BROADCAST - - SLACK_POST + type: SLACK_POST message: > Please use threads to keep the discussion more organized: https://datatalks.club/slack/guidelines.html#taking-part-in-discussions + - reaction: thread_please + type: DELETE_MESSAGE + message: | + Hi <@{user}>! + + You posted this message in <#{channel}>: + + > {user_message} + + Please don't use the "Also send to channel" (broadcast) functionality when replying in threads. This is against our community guidelines: + + https://datatalks.club/slack/guidelines.html#taking-part-in-discussions + + Your message was removed from the channel. Feel free to repost it in the thread without using the broadcast functionality. + + Thank you for helping us keep the community organized! + - reaction: faq type: SLACK_POST placeholders: diff --git a/automator/lambda_function.py b/automator/lambda_function.py index ae1b57e..7a37958 100644 --- a/automator/lambda_function.py +++ b/automator/lambda_function.py @@ -32,7 +32,6 @@ def get_channel_name(channel_id): def handle_slack_post(event, reaction_config): channel_id = event['item']['channel'] channel_name = get_channel_name(channel_id) - ts = event['item']['ts'] if 'placeholders' in reaction_config: message = util.format_message( @@ -45,17 +44,6 @@ def handle_slack_post(event, reaction_config): else: message = reaction_config['message'] - # Check if this is a broadcasted thread reply - message_details = slack.get_message_content(channel_id, ts) - if message_details: - thread_ts = message_details.get('thread_ts') - # If it's a broadcasted reply (thread_ts != ts), use the original thread - if thread_ts is not None and thread_ts != ts: - # Modify event to use the original thread_ts - event = event.copy() - event['item'] = event['item'].copy() - event['item']['ts'] = thread_ts - slack.post_message_thread(event, message) diff --git a/tests/test_automator_lambda.py b/tests/test_automator_lambda.py index ecdcc4f..76c7ba7 100644 --- a/tests/test_automator_lambda.py +++ b/tests/test_automator_lambda.py @@ -241,6 +241,17 @@ def test_to_welcome_has_thread_message(self): self.assertEqual(reaction_config['type'], 'DELETE_MESSAGE') self.assertIn('thread_message', reaction_config) + def test_thread_please_reaction_exists(self): + """Verify that 'thread_please' reaction is configured""" + reaction_config = lambda_function.reaction_configs.get('thread_please') + self.assertIsNotNone(reaction_config) + self.assertEqual(reaction_config['type'], 'DELETE_MESSAGE') + self.assertIn('message', reaction_config) + # Verify the message contains expected text + message = reaction_config['message'] + self.assertIn('broadcast', message.lower()) + self.assertIn('guidelines', message.lower()) + def test_action_handlers_has_delete_message(self): """Verify that DELETE_MESSAGE handler is registered""" self.assertIn('DELETE_MESSAGE', lambda_function.action_handlers) @@ -261,12 +272,14 @@ def test_action_handlers_has_remove_broadcast(self): lambda_function.handle_remove_broadcast ) - def test_thread_reaction_uses_multiple_handlers(self): - """Verify that 'thread' reaction uses multiple handlers""" + def test_thread_reaction_uses_single_handler(self): + """Verify that 'thread' reaction uses single SLACK_POST handler""" 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']) + self.assertIn('type', reaction_config) + self.assertEqual(reaction_config['type'], 'SLACK_POST') + # Should not have 'types' (multiple handlers) + self.assertNotIn('types', reaction_config) class TestRemoveBroadcast(unittest.TestCase): @@ -401,15 +414,9 @@ class TestMultipleHandlers(unittest.TestCase): @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' - } + def test_thread_reaction_executes_single_handler(self, mock_util, mock_slack): + """Test that thread reaction executes only SLACK_POST""" + # Setup mocks mock_util.format_message.return_value = None # Create event @@ -429,150 +436,11 @@ def test_thread_reaction_executes_both_handlers(self, mock_util, mock_slack): # 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() - - # Verify the message was posted to the ORIGINAL thread, not the broadcast message - call_args = mock_slack.post_message_thread.call_args - posted_event = call_args[0][0] - self.assertEqual(posted_event['item']['ts'], '1234567890.123456', - "Message should be posted to original thread, not broadcast message") - - - -class TestSlackPost(unittest.TestCase): - """Test SLACK_POST handler for different message types""" - - @patch('automator_lambda_function.slack') - def test_regular_message_post(self, mock_slack): - """Test that regular messages post to the correct thread""" - # 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 = { - 'message': 'Please use threads' - } - - # Execute - lambda_function.handle_slack_post(event, reaction_config) - - # Verify - should post to the same ts (the message itself) - mock_slack.post_message_thread.assert_called_once() - call_args = mock_slack.post_message_thread.call_args - posted_event = call_args[0][0] - self.assertEqual(posted_event['item']['ts'], '1234567890.123456') - - @patch('automator_lambda_function.slack') - def test_broadcasted_reply_post_to_original_thread(self, mock_slack): - """Test that broadcasted thread replies post to the original thread""" - # 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 - this is the original thread - } - - # Create event - reacting to the broadcasted message - event = { - 'item': { - 'channel': 'C123456', - 'ts': '1234567890.123457' # The broadcasted message ts - }, - 'reaction': 'thread' - } - - # Create reaction config - reaction_config = { - 'message': 'Please use threads' - } - - # Execute - lambda_function.handle_slack_post(event, reaction_config) - - # Verify - should post to the original thread_ts, not the broadcast message ts - mock_slack.post_message_thread.assert_called_once() - call_args = mock_slack.post_message_thread.call_args - posted_event = call_args[0][0] - # The key fix: should use thread_ts (parent) not the broadcast message ts - self.assertEqual(posted_event['item']['ts'], '1234567890.123456') - - @patch('automator_lambda_function.slack') - def test_thread_parent_message_post(self, mock_slack): - """Test that parent thread messages post to themselves""" - # 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 - this is the thread parent - } - - # Create event - event = { - 'item': { - 'channel': 'C123456', - 'ts': '1234567890.123456' - }, - 'reaction': 'thread' - } - - # Create reaction config - reaction_config = { - 'message': 'Please use threads' - } - - # Execute - lambda_function.handle_slack_post(event, reaction_config) - - # Verify - should post to the same ts (creates a thread on this message) - mock_slack.post_message_thread.assert_called_once() - call_args = mock_slack.post_message_thread.call_args - posted_event = call_args[0][0] - self.assertEqual(posted_event['item']['ts'], '1234567890.123456') - - @patch('automator_lambda_function.slack') - def test_message_not_found_gracefully_handled(self, mock_slack): - """Test that missing messages are handled gracefully""" - # 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 = { - 'message': 'Please use threads' - } - - # Execute - lambda_function.handle_slack_post(event, reaction_config) - - # Verify - should still post (fallback to original behavior) + # Verify only SLACK_POST handler was called: + # SLACK_POST called post_message_thread mock_slack.post_message_thread.assert_called_once() + # REMOVE_BROADCAST should NOT be called (no get_message_content for removal) + mock_slack.remove_message.assert_not_called() class TestChannelConfig(unittest.TestCase):