From 9ee38a8b4916a7c65cf51c4e4f60f59e8ff7de43 Mon Sep 17 00:00:00 2001
From: Guillaume Charest <1690085+gcharest@users.noreply.github.com>
Date: Tue, 10 Sep 2024 16:21:00 +0000
Subject: [PATCH] fix: ensure client is in channel to perform action

---
 app/modules/incident/incident_helper.py       | 12 ++-
 .../modules/incident/test_incident_helper.py  | 86 ++++++++++++++++++-
 2 files changed, 96 insertions(+), 2 deletions(-)

diff --git a/app/modules/incident/incident_helper.py b/app/modules/incident/incident_helper.py
index 624cd9f4..896c610f 100644
--- a/app/modules/incident/incident_helper.py
+++ b/app/modules/incident/incident_helper.py
@@ -130,6 +130,16 @@ def close_incident(client: WebClient, body, ack, respond):
     channel_name = body["channel_name"]
     user_id = body["user_id"]
 
+    # ensure the bot is actually in the channel before performing actions
+    try:
+        response = client.conversations_info(channel=channel_id)
+        channel_info = response.get("channel", None)
+        if channel_info is None or not channel_info.get("is_member", False):
+            client.conversations_join(channel=channel_id)
+    except Exception as e:
+        logging.error(f"Failed to join the channel {channel_id}: {e}")
+        return
+
     if not channel_name.startswith("incident-"):
         try:
             response = client.chat_postEphemeral(
@@ -198,7 +208,7 @@ def close_incident(client: WebClient, body, ack, respond):
         )
 
 
-def stale_incidents(client, body, ack):
+def stale_incidents(client: WebClient, body, ack: Ack):
     ack()
 
     placeholder = {
diff --git a/app/tests/modules/incident/test_incident_helper.py b/app/tests/modules/incident/test_incident_helper.py
index d2398a81..cab6ccad 100644
--- a/app/tests/modules/incident/test_incident_helper.py
+++ b/app/tests/modules/incident/test_incident_helper.py
@@ -3,7 +3,6 @@
 from modules import incident_helper
 import logging
 
-
 from unittest.mock import ANY, MagicMock, call, patch
 
 SLACK_SECURITY_USER_GROUP_ID = os.getenv("SLACK_SECURITY_USER_GROUP_ID")
@@ -485,6 +484,91 @@ def test_close_incident_not_incident_channel():
     )
 
 
+@patch("modules.incident.incident_helper.logging")
+@patch(
+    "modules.incident.incident_helper.incident_document.update_incident_document_status"
+)
+@patch(
+    "modules.incident.incident_helper.incident_folder.update_spreadsheet_incident_status"
+)
+@patch(
+    "integrations.google_workspace.google_docs.extract_google_doc_id",
+    return_value="dummy_document_id",
+)
+def test_close_incident_when_client_not_in_channel(
+    mock_extract_id, mock_update_spreadsheet, mock_update_document_status, mock_logging
+):
+    # the client is not in the channel so it needs to be added
+    mock_client = MagicMock()
+    mock_ack = MagicMock()
+    mock_respond = MagicMock()
+    # Mock the response of the private message to have been posted as expected
+    mock_client.conversations_info.return_value = {
+        "ok": True,
+        "channel": {"id": "C12345", "name": "incident-channel", "is_member": False},
+    }
+    mock_client.conversations_join.return_value = {"ok": True}
+
+    # Call close_incident
+    incident_helper.close_incident(
+        mock_client,
+        {
+            "channel_id": "C12345",
+            "user_id": "12345",
+            "channel_name": "incident-channel",
+        },
+        mock_ack,
+        mock_respond,
+    )
+
+    # Assert that ack was called
+    mock_ack.assert_called_once()
+
+    # Assert that the client was added to the channel
+    mock_client.conversations_join.assert_called_once_with(channel="C12345")
+
+
+def test_close_incident_when_client_not_in_channel_throws_error(
+    caplog,
+):
+    # the client is not in the channel so it needs to be added
+    mock_client = MagicMock()
+    mock_ack = MagicMock()
+    mock_respond = MagicMock()
+    # Mock the response of the private message to have been posted as expected
+    mock_client.conversations_info.return_value = {
+        "ok": True,
+        "channel": {"id": "C12345", "name": "incident-channel", "is_member": False},
+    }
+    mock_client.conversations_join.return_value = {"ok": False, "error": "is_archived"}
+
+    exception_message = "is_archived"
+    mock_client.conversations_join.side_effect = Exception(exception_message)
+
+    # Call close_incident
+    with caplog.at_level(logging.ERROR):
+        incident_helper.close_incident(
+            mock_client,
+            {
+                "channel_id": "C12345",
+                "user_id": "12345",
+                "channel_name": "incident-channel",
+            },
+            mock_ack,
+            mock_respond,
+        )
+
+        assert caplog.records
+
+        # Find the specific log message we're interested in
+        log_messages = [record.message for record in caplog.records]
+
+        expected_message = "Failed to join the channel C12345: is_archived"
+        assert (
+            expected_message in log_messages
+        ), "Expected error message not found in log records"
+
+
 # Test that the channel that the command is ran in,  is not an incident channel.
 def test_close_incident_cant_send_private_message(caplog):
     mock_client = MagicMock()