diff --git a/packages/notifyS3UploadFunction/tests/conftest.py b/packages/notifyS3UploadFunction/tests/conftest.py index fcc73440d..d81fc8142 100644 --- a/packages/notifyS3UploadFunction/tests/conftest.py +++ b/packages/notifyS3UploadFunction/tests/conftest.py @@ -4,7 +4,6 @@ from unittest.mock import MagicMock, Mock, patch import os - TEST_BOT_TOKEN = "test-bot-token" diff --git a/packages/slackBotFunction/app/services/bedrock.py b/packages/slackBotFunction/app/services/bedrock.py index 44d020196..8bb06f0b3 100644 --- a/packages/slackBotFunction/app/services/bedrock.py +++ b/packages/slackBotFunction/app/services/bedrock.py @@ -8,7 +8,6 @@ from app.core.config import get_retrieve_generate_config, get_logger from app.services.prompt_loader import load_prompt - logger = get_logger() diff --git a/packages/slackBotFunction/app/services/slack.py b/packages/slackBotFunction/app/services/slack.py index 66fe280e0..288718b86 100644 --- a/packages/slackBotFunction/app/services/slack.py +++ b/packages/slackBotFunction/app/services/slack.py @@ -3,7 +3,6 @@ from slack_sdk import WebClient from app.core.config import bot_messages, get_logger - logger = get_logger() @@ -12,7 +11,10 @@ def get_friendly_channel_name(channel_id: str, client: WebClient) -> str: try: conversations_info_response = client.conversations_info(channel=channel_id) if conversations_info_response["ok"]: - friendly_channel_name = conversations_info_response["channel"]["name"] + channel_info = conversations_info_response["channel"] + if channel_info.get("is_im"): + return "Direct Message" + friendly_channel_name = channel_info.get("name", channel_id) else: logger.warning( "There was a problem getting the friendly channel name", diff --git a/packages/slackBotFunction/app/slack/slack_events.py b/packages/slackBotFunction/app/slack/slack_events.py index 726968cc6..5453d6867 100644 --- a/packages/slackBotFunction/app/slack/slack_events.py +++ b/packages/slackBotFunction/app/slack/slack_events.py @@ -40,7 +40,6 @@ from app.services.ai_processor import process_ai_query - logger = get_logger() processing_error_message = "Error processing message" @@ -506,7 +505,7 @@ def process_slack_message(event: Dict[str, Any], event_id: str, client: WebClien user_query = re.sub(r"<@[UW][A-Z0-9]+(\|[^>]+)?>", "", event["text"]).strip() logger.info( - f"Processing message from user {user_id}", + "Processing message", extra={"user_query": user_query, "conversation_key": conversation_key, "event_id": event_id}, ) @@ -570,7 +569,11 @@ def log_query_stats(user_query: str, event: Dict[str, Any], channel: str, client end_time = time.time() duration = end_time - start_time is_direct_message = event.get("channel_type") == constants.CHANNEL_TYPE_IM - friendly_channel_name = get_friendly_channel_name(channel_id=channel, client=client) + if is_direct_message: + friendly_channel_name = "Direct Message" + else: + friendly_channel_name = get_friendly_channel_name(channel_id=channel, client=client) + reporting_info = { "query_length": query_length, "start_time": start_time, diff --git a/packages/slackBotFunction/tests/test_privacy_changes.py b/packages/slackBotFunction/tests/test_privacy_changes.py new file mode 100644 index 000000000..56639acde --- /dev/null +++ b/packages/slackBotFunction/tests/test_privacy_changes.py @@ -0,0 +1,90 @@ +import sys +from unittest.mock import Mock, patch +from app.services.slack import get_friendly_channel_name + + +def test_get_friendly_channel_name_returns_direct_message_for_im(): + mock_client = Mock() + mock_client.conversations_info.return_value = {"ok": True, "channel": {"is_im": True, "id": "D12345"}} + + result = get_friendly_channel_name("D12345", mock_client) + assert result == "Direct Message" + + +def test_get_friendly_channel_name_returns_name_for_public_channel(): + mock_client = Mock() + mock_client.conversations_info.return_value = { + "ok": True, + "channel": {"is_im": False, "name": "general", "id": "C12345"}, + } + + result = get_friendly_channel_name("C12345", mock_client) + assert result == "general" + + +def test_log_query_stats_masks_dm_channel(mock_env, mock_get_parameter): + # reload module to ensure clean state and correct patching + if "app.slack.slack_events" in sys.modules: + del sys.modules["app.slack.slack_events"] + + from app.slack.slack_events import log_query_stats + import app.slack.slack_events + + mock_client = Mock() + + event = {"event_ts": "1234567890.123", "channel_type": "im"} + + with patch.object(app.slack.slack_events, "logger") as mock_logger: + log_query_stats(user_query="test", event=event, channel="D123", client=mock_client, thread_ts="123") + + assert mock_logger.info.called + + _, kwargs = mock_logger.info.call_args + reporting_info = kwargs["extra"]["reporting_info"] + assert reporting_info["channel"] == "Direct Message" + assert reporting_info["is_direct_message"] is True + + mock_client.conversations_info.assert_not_called() + + +def test_process_slack_message_log_privacy(mock_env, mock_get_parameter): + if "app.slack.slack_events" in sys.modules: + del sys.modules["app.slack.slack_events"] + + from app.slack.slack_events import process_slack_message + import app.slack.slack_events + + with patch.object(app.slack.slack_events, "logger") as mock_logger, patch.object( + app.slack.slack_events, "conversation_key_and_root", return_value=("key", "ts") + ), patch.object(app.slack.slack_events, "get_conversation_session_data", return_value={}), patch.object( + app.slack.slack_events, "get_friendly_channel_name" + ), patch.object( + app.slack.slack_events, "process_ai_query" + ) as mock_process_ai: + + mock_client = Mock() + mock_client.chat_postMessage.return_value = {"ts": "123"} + mock_process_ai.return_value = {"kb_response": {}, "text": "response", "session_id": "sid"} + + event = { + "user": "U12345", + "channel": "C123", + "text": "hello", + "ts": "123", + "event_ts": "123", + "channel_type": "channel", + } + + process_slack_message(event, "evt1", mock_client) + + processing_call = None + for call in mock_logger.info.call_args_list: + args, _ = call + if args and "Processing message" in args[0]: + processing_call = call + break + + assert processing_call is not None + args, _ = processing_call + assert "from user" not in args[0] + assert "U12345" not in args[0] diff --git a/poetry.lock b/poetry.lock index 208933745..850f2d736 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1091,7 +1091,7 @@ files = [ {file = "colorama-0.4.6-py2.py3-none-any.whl", hash = "sha256:4f1d9991f5acc0ca119f9d443620b77f9d6b33703e51011c16baf57afb285fc6"}, {file = "colorama-0.4.6.tar.gz", hash = "sha256:08695f5cb7ed6e0531a20572697297273c47b8cae5a63ffc6d6ed5c201be6e44"}, ] -markers = {dev = "platform_system == \"Windows\" or sys_platform == \"win32\"", preprocessingfunction = "platform_system == \"Windows\""} +markers = {dev = "sys_platform == \"win32\" or platform_system == \"Windows\"", preprocessingfunction = "platform_system == \"Windows\""} [[package]] name = "coverage" diff --git a/scripts/run_regression_tests.py b/scripts/run_regression_tests.py index 772c6a5f9..be906eb57 100644 --- a/scripts/run_regression_tests.py +++ b/scripts/run_regression_tests.py @@ -4,6 +4,7 @@ Script to generate user defined unique ID which can be used to check the status of the regression test run to be reported to the CI. """ + import argparse from datetime import datetime, timedelta, timezone import random