Update: [AEA-6136] - change logging + tests (#360)

## Summary

- Routine Change

### Details

- removes user ID's which we were logging prior.

---------

Co-authored-by: Beenyaa <bencegadanyi1@hotmail.com>

Author: bencegadanyi1-nhs

packages/notifyS3UploadFunction/tests/conftest.py

@@ -4,7 +4,6 @@
 from unittest.mock import MagicMock, Mock, patch
 import os
 
-
 TEST_BOT_TOKEN = "test-bot-token"
 
 

packages/slackBotFunction/app/services/bedrock.py

@@ -7,7 +7,6 @@
 
 from app.core.config import BedrockConfig, get_logger
 
-
 logger = get_logger()
 
 

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",

packages/slackBotFunction/app/slack/slack_events.py

@@ -41,7 +41,6 @@
 
 from app.services.ai_processor import process_ai_query
 
-
 logger = get_logger()
 
 processing_error_message = "Error processing message"
@@ -513,7 +512,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},
         )
 
@@ -577,7 +576,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,

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]

poetry.lock

@@ -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