Adding proper handling of user ids (#401)

app/commands/incident.py

@@ -11,6 +11,7 @@
     rearrange_by_datetime_ascending,
     convert_epoch_to_datetime_est,
     extract_google_doc_id,
+    replace_user_id_with_handle,
 )
 from integrations.google_drive import (
     get_timeline_section,
@@ -408,16 +409,17 @@ def handle_reaction_added(client, ack, body, logger):
                 user = client.users_profile_get(user=message["user"])
                 # get the full name of the user so that we include it into the timeline
                 user_full_name = user["profile"]["real_name"]
+                user_handle = "@" + user["profile"]["display_name"]
 
                 # get the current timeline section content
                 content = get_timeline_section(document_id)
 
+                message = replace_user_id_with_handle(user_handle, message["text"])
+
                 # if the message already exists in the timeline, then don't put it there again
                 if content and message_date_time not in content:
                     # append the new message to the content
-                    content += (
-                        f"{message_date_time} {user_full_name}: {message['text']}"
-                    )
+                    content += f"{message_date_time} {user_full_name}: {message}"
 
                     # if there is an image in the message, then add it to the timeline
                     if "files" in message:
@@ -462,6 +464,7 @@ def handle_reaction_removed(client, ack, body, logger):
             user = client.users_profile_get(user=message["user"])
             # get the user's full name
             user_full_name = user["profile"]["real_name"]
+            user_handle = "@" + user["profile"]["display_name"]
 
             # get the incident report document id from the incident channel
             # get and update the incident document
@@ -478,10 +481,10 @@ def handle_reaction_removed(client, ack, body, logger):
             # Retrieve the current content of the timeline
             content = get_timeline_section(document_id)
 
+            message = replace_user_id_with_handle(user_handle, message["text"])
+
             # Construct the message to remove
-            message_to_remove = (
-                f"\n{message_date_time} {user_full_name}: {message['text']}\n"
-            )
+            message_to_remove = f"\n{message_date_time} {user_full_name}: {message}\n"
             # if there is a file in the message, then add it to the message to remove
             if "files" in message:
                 image = message["files"][0]["url_private"]

app/commands/utils.py

@@ -223,3 +223,15 @@ def extract_google_doc_id(url):
         return match.group(1)
     else:
         return None
+
+
+# Function to replace the user id with the user handle in a message:w
+def replace_user_id_with_handle(user_handle, message):
+    if not user_handle or not message:
+        logging.error("User handle or message is empty or None")
+        return None
+
+    user_id_pattern = r"<@\w+>"
+    if re.search(user_id_pattern, message):
+        message = re.sub(user_id_pattern, user_handle, message)
+    return message

app/tests/commands/test_incident.py

@@ -985,12 +985,44 @@ def test_handle_reaction_added_adding_new_message_to_timeline():
     mock_client.users_profile_get.assert_called_once()
 
 
+def test_handle_reaction_added_adding_new_message_to_timeline_user_handle():
+    logger = MagicMock()
+    mock_client = MagicMock()
+    mock_client.conversations_info.return_value = {"channel": {"name": "incident-123"}}
+    mock_client.conversations_history.return_value = {
+        "ok": True,
+        "messages": [
+            {
+                "type": "message",
+                "user": "U123ABC456",
+                "text": "<U123ABC456> says Sample test message",
+                "ts": "1512085950.000216",
+            }
+        ],
+    }
+    body = {
+        "event": {
+            "reaction": "floppy_disk",
+            "item": {"channel": "C123456", "ts": "123456"},
+        }
+    }
+
+    incident.handle_reaction_added(mock_client, lambda: None, body, logger)
+
+    # Make assertion that the function calls the correct functions
+    mock_client.conversations_history.assert_called_once()
+    mock_client.bookmarks_list.assert_called_once()
+    mock_client.users_profile_get.assert_called_once()
+
+
 def test_handle_reaction_removed_successful_message_removal():
     # Mock the client and logger
     logger = MagicMock()
     mock_client = MagicMock()
     mock_client.conversations_info.return_value = {"channel": {"name": "incident-123"}}
-    mock_client.users_profile_get.return_value = {"profile": {"real_name": "John Doe"}}
+    mock_client.users_profile_get.return_value = {
+        "profile": {"real_name": "John Doe", "display_name": "John"}
+    }
     mock_client.bookmarks_list.return_value = {
         "ok": True,
         "bookmarks": [{"title": "Incident report", "link": "http://example.com"}],
@@ -1022,6 +1054,45 @@ def test_handle_reaction_removed_successful_message_removal():
     mock_client.users_profile_get.assert_called_once()
 
 
+def test_handle_reaction_removed_successful_message_removal_user_id():
+    # Mock the client and logger
+    logger = MagicMock()
+    mock_client = MagicMock()
+    mock_client.conversations_info.return_value = {"channel": {"name": "incident-123"}}
+    mock_client.users_profile_get.return_value = {
+        "profile": {"real_name": "John Doe", "display_name": "John"}
+    }
+    mock_client.bookmarks_list.return_value = {
+        "ok": True,
+        "bookmarks": [{"title": "Incident report", "link": "http://example.com"}],
+    }
+    mock_client.get_timeline_section.return_value = "Sample test message"
+    mock_client.replace_text_between_headings.return_value = True
+
+    body = {
+        "event": {
+            "reaction": "floppy_disk",
+            "item": {"channel": "C123456", "ts": "123456"},
+        }
+    }
+    mock_client.conversations_history.return_value = {
+        "ok": True,
+        "messages": [
+            {
+                "type": "message",
+                "user": "U123ABC456",
+                "text": "<U123ABC456> says Sample test message",
+                "ts": "1512085950.000216",
+            }
+        ],
+    }
+
+    incident.handle_reaction_removed(mock_client, lambda: None, body, logger)
+    mock_client.conversations_history.assert_called_once()
+    mock_client.bookmarks_list.assert_called_once()
+    mock_client.users_profile_get.assert_called_once()
+
+
 def test_handle_reaction_removed_message_not_in_timeline():
     logger = MagicMock()
     mock_client = MagicMock()

app/tests/commands/test_utils.py

@@ -277,3 +277,44 @@ def test_extract_googe_doc_id_empty_string():
 
 def test_extract_googe_doc_id_none_input():
     assert utils.extract_google_doc_id(None) is None
+
+
+def test_replace_user_id_with_valid_handle():
+    assert (
+        utils.replace_user_id_with_handle("@user", "Hello <@U12345>, how are you?")
+        == "Hello @user, how are you?"
+    )
+
+
+def test_replace_user_id_with_no_pattern_in_message():
+    assert (
+        utils.replace_user_id_with_handle("@user", "Hello user, how are you?")
+        == "Hello user, how are you?"
+    )
+
+
+def test_replace_user_id_with_empty_handle():
+    assert (
+        utils.replace_user_id_with_handle("", "Hello <@U12345>, how are you?") is None
+    )
+
+
+def test_replace_user_id_with_empty_message():
+    assert utils.replace_user_id_with_handle("@user", "") is None
+
+
+def test_replace_user_id_with_none_handle():
+    assert (
+        utils.replace_user_id_with_handle(None, "Hello <@U12345>, how are you?") is None
+    )
+
+
+def test_replace_user_id_with_none_message():
+    assert utils.replace_user_id_with_handle("@user", None) is None
+
+
+def test_replace_multiple_user_ids_in_message():
+    assert (
+        utils.replace_user_id_with_handle("@user", "Hi <@U12345>, meet <@U67890>")
+        == "Hi @user, meet @user"
+    )