fis: improve based on code review

WilliamBergamin · Jul 14, 2026 · Jul 2, 2026 · Jul 3, 2026 · Jul 6, 2026 · Jul 7, 2026
commit 25768ba0df4b721623dd20171f27b14fb6108e51
@@ -34,7 +34,7 @@ async def __call__(
 
         return await self.client.assistant_threads_setSuggestedPrompts(
             channel_id=self.channel_id,
-            thread_ts=thread_ts or self.thread_ts,
+            thread_ts=thread_ts if thread_ts is not None else self.thread_ts,
             prompts=prompts_arg,
             title=title,
         )
@@ -34,7 +34,7 @@ def __call__(
 
         return self.client.assistant_threads_setSuggestedPrompts(
             channel_id=self.channel_id,
-            thread_ts=thread_ts or self.thread_ts,
+            thread_ts=thread_ts if thread_ts is not None else self.thread_ts,
             prompts=prompts_arg,
             title=title,
         )
@@ -1,3 +1,5 @@
+from unittest.mock import MagicMock, patch
+
 import pytest
 from slack_sdk import WebClient
 from slack_sdk.web import SlackResponse
@@ -33,13 +35,44 @@ def test_set_suggested_prompts_objects(self):
 
     def test_set_suggested_prompts_without_thread_ts(self):
         set_suggested_prompts = SetSuggestedPrompts(client=self.web_client, channel_id="C111")
-        response: SlackResponse = set_suggested_prompts(prompts=["One", "Two"])
-        assert response.status_code == 200
+        with patch.object(
+            self.web_client, self.web_client.assistant_threads_setSuggestedPrompts.__name__, return_value=MagicMock()
+        ) as mock_api:
+            set_suggested_prompts(prompts=["One", "Two"])
+            mock_api.assert_called_once_with(
+                channel_id="C111",
+                thread_ts=None,
+                prompts=[{"title": "One", "message": "One"}, {"title": "Two", "message": "Two"}],
+                title=None,
+            )
 
     def test_set_suggested_prompts_thread_ts_override(self):
-        set_suggested_prompts = SetSuggestedPrompts(client=self.web_client, channel_id="C111")
-        response: SlackResponse = set_suggested_prompts(prompts=["One", "Two"], thread_ts="123.123")
-        assert response.status_code == 200
+        # The call-time thread_ts must win over the stored one
+        set_suggested_prompts = SetSuggestedPrompts(client=self.web_client, channel_id="C111", thread_ts="999.999")
+        with patch.object(
+            self.web_client, self.web_client.assistant_threads_setSuggestedPrompts.__name__, return_value=MagicMock()
+        ) as mock_api:
+            set_suggested_prompts(prompts=["One", "Two"], thread_ts="123.123")
+            mock_api.assert_called_once_with(
+                channel_id="C111",
+                thread_ts="123.123",
+                prompts=[{"title": "One", "message": "One"}, {"title": "Two", "message": "Two"}],
+                title=None,
+            )
+
+    def test_set_suggested_prompts_thread_ts_override_falsy(self):
+        # An explicitly passed falsy thread_ts must be forwarded, not swallowed by the stored value
+        set_suggested_prompts = SetSuggestedPrompts(client=self.web_client, channel_id="C111", thread_ts="123.123")
+        with patch.object(
+            self.web_client, self.web_client.assistant_threads_setSuggestedPrompts.__name__, return_value=MagicMock()
+        ) as mock_api:
+            set_suggested_prompts(prompts=["One", "Two"], thread_ts="")
+            mock_api.assert_called_once_with(
+                channel_id="C111",
+                thread_ts="",
+                prompts=[{"title": "One", "message": "One"}, {"title": "Two", "message": "Two"}],
+                title=None,
+            )
 
     def test_set_suggested_prompts_invalid(self):
         set_suggested_prompts = SetSuggestedPrompts(client=self.web_client, channel_id="C111", thread_ts="123.123")

@@ -30,6 +30,35 @@ def next():
     }
 )
 
+# A bot-authored top-level DM is also in scope: set_suggested_prompts is attached for any IM message.
+bot_im_message_event_body = build_payload(
+    {
+        "type": "message",
+        "ts": "1726133700.887259",
+        "text": "A DM authored by a bot",
+        "user": "UB111",
+        "bot_id": "B111",
+        "app_id": "A222",
+        "channel": "D111",
+        "event_ts": "1726133700.887259",
+        "channel_type": "im",
+    }
+)
+
+# A file_share DM is in scope too (subtype "file_share" passes is_im_message_event).
+file_share_im_message_event_body = build_payload(
+    {
+        "user": "W222",
+        "type": "message",
+        "subtype": "file_share",
+        "ts": "1726133700.887259",
+        "text": "uploaded a file",
+        "channel": "D111",
+        "event_ts": "1726133700.887259",
+        "channel_type": "im",
+    }
+)
+
 
 class TestAttachingConversationKwargs:
     def test_assistant_event_attaches_kwargs(self):
@@ -68,7 +97,6 @@ def test_top_level_dm_attaches_suggested_prompts_but_not_set_title(self):
         resp = middleware.process(req=req, resp=BoltResponse(status=404), next=next)
 
         assert resp.status == 200
-        # set_suggested_prompts is available for any DM to the app
         assert "set_suggested_prompts" in req.context
         # set_title is assistant-thread-only; a top-level DM is not an assistant thread
         assert "set_title" not in req.context
@@ -80,6 +108,28 @@ def test_top_level_dm_attaches_suggested_prompts_but_not_set_title(self):
         assert "say_stream" in req.context
         assert "set_status" in req.context
 
+    def test_bot_dm_attaches_suggested_prompts(self):
+        # set_suggested_prompts is intentionally attached for any IM message, including bot-authored DMs.
+        middleware = AttachingConversationKwargs()
+        req = BoltRequest(body=bot_im_message_event_body, mode="socket_mode")
+        req.context["client"] = WebClient(token="xoxb-test")
+
+        resp = middleware.process(req=req, resp=BoltResponse(status=404), next=next)
+
+        assert resp.status == 200
+        assert "set_suggested_prompts" in req.context
+
+    def test_file_share_dm_attaches_suggested_prompts(self):
+        # A file_share DM is in scope for set_suggested_prompts.
+        middleware = AttachingConversationKwargs()
+        req = BoltRequest(body=file_share_im_message_event_body, mode="socket_mode")
+        req.context["client"] = WebClient(token="xoxb-test")
+
+        resp = middleware.process(req=req, resp=BoltResponse(status=404), next=next)
+
+        assert resp.status == 200
+        assert "set_suggested_prompts" in req.context
+
     def test_non_assistant_event_does_not_attach_kwargs(self):
         middleware = AttachingConversationKwargs()
         req = BoltRequest(body=channel_user_message_event_body, mode="socket_mode")

@@ -1,4 +1,5 @@
 import asyncio
+from unittest.mock import MagicMock, patch
 
 import pytest
 from slack_sdk.web.async_client import AsyncWebClient
@@ -44,14 +45,61 @@ async def test_set_suggested_prompts_objects(self):
     @pytest.mark.asyncio
     async def test_set_suggested_prompts_without_thread_ts(self):
         set_suggested_prompts = AsyncSetSuggestedPrompts(client=self.web_client, channel_id="C111")
-        response: AsyncSlackResponse = await set_suggested_prompts(prompts=["One", "Two"])
-        assert response.status_code == 200
+        mock_api = MagicMock()
+
+        async def fake_api(**kwargs):
+            return mock_api(**kwargs)
+
+        with patch.object(
+            self.web_client, self.web_client.assistant_threads_setSuggestedPrompts.__name__, side_effect=fake_api
+        ):
+            await set_suggested_prompts(prompts=["One", "Two"])
+            mock_api.assert_called_once_with(
+                channel_id="C111",
+                thread_ts=None,
+                prompts=[{"title": "One", "message": "One"}, {"title": "Two", "message": "Two"}],
+                title=None,
+            )
 
     @pytest.mark.asyncio
     async def test_set_suggested_prompts_thread_ts_override(self):
-        set_suggested_prompts = AsyncSetSuggestedPrompts(client=self.web_client, channel_id="C111")
-        response: AsyncSlackResponse = await set_suggested_prompts(prompts=["One", "Two"], thread_ts="123.123")
-        assert response.status_code == 200
+        # The call-time thread_ts must win over the stored one
+        set_suggested_prompts = AsyncSetSuggestedPrompts(client=self.web_client, channel_id="C111", thread_ts="999.999")
+        mock_api = MagicMock()
+
+        async def fake_api(**kwargs):
+            return mock_api(**kwargs)
+
+        with patch.object(
+            self.web_client, self.web_client.assistant_threads_setSuggestedPrompts.__name__, side_effect=fake_api
+        ):
+            await set_suggested_prompts(prompts=["One", "Two"], thread_ts="123.123")
+            mock_api.assert_called_once_with(
+                channel_id="C111",
+                thread_ts="123.123",
+                prompts=[{"title": "One", "message": "One"}, {"title": "Two", "message": "Two"}],
+                title=None,
+            )
+
+    @pytest.mark.asyncio
+    async def test_set_suggested_prompts_thread_ts_override_falsy(self):
+        # An explicitly passed falsy thread_ts must be forwarded, not swallowed by the stored value
+        set_suggested_prompts = AsyncSetSuggestedPrompts(client=self.web_client, channel_id="C111", thread_ts="123.123")
+        mock_api = MagicMock()
+
+        async def fake_api(**kwargs):
+            return mock_api(**kwargs)
+
+        with patch.object(
+            self.web_client, self.web_client.assistant_threads_setSuggestedPrompts.__name__, side_effect=fake_api
+        ):
+            await set_suggested_prompts(prompts=["One", "Two"], thread_ts="")
+            mock_api.assert_called_once_with(
+                channel_id="C111",
+                thread_ts="",
+                prompts=[{"title": "One", "message": "One"}, {"title": "Two", "message": "Two"}],
+                title=None,
+            )
 
     @pytest.mark.asyncio
     async def test_set_suggested_prompts_invalid(self):

@@ -33,6 +33,35 @@ async def next():
     }
 )
 
+# A bot-authored top-level DM is also in scope: set_suggested_prompts is attached for any IM message.
+bot_im_message_event_body = build_payload(
+    {
+        "type": "message",
+        "ts": "1726133700.887259",
+        "text": "A DM authored by a bot",
+        "user": "UB111",
+        "bot_id": "B111",
+        "app_id": "A222",
+        "channel": "D111",
+        "event_ts": "1726133700.887259",
+        "channel_type": "im",
+    }
+)
+
+# A file_share DM is in scope too (subtype "file_share" passes is_im_message_event).
+file_share_im_message_event_body = build_payload(
+    {
+        "user": "W222",
+        "type": "message",
+        "subtype": "file_share",
+        "ts": "1726133700.887259",
+        "text": "uploaded a file",
+        "channel": "D111",
+        "event_ts": "1726133700.887259",
+        "channel_type": "im",
+    }
+)
+
 
 class TestAsyncAttachingConversationKwargs:
     @pytest.mark.asyncio
@@ -86,6 +115,30 @@ async def test_top_level_dm_attaches_suggested_prompts_but_not_set_title(self):
         assert "say_stream" in req.context
         assert "set_status" in req.context
 
+    @pytest.mark.asyncio
+    async def test_bot_dm_attaches_suggested_prompts(self):
+        # set_suggested_prompts is intentionally attached for any IM message, including bot-authored DMs.
+        middleware = AsyncAttachingConversationKwargs()
+        req = AsyncBoltRequest(body=bot_im_message_event_body, mode="socket_mode")
+        req.context["client"] = AsyncWebClient(token="xoxb-test")
+
+        resp = await middleware.async_process(req=req, resp=BoltResponse(status=404), next=next)
+
+        assert resp.status == 200
+        assert "set_suggested_prompts" in req.context
+
+    @pytest.mark.asyncio
+    async def test_file_share_dm_attaches_suggested_prompts(self):
+        # A file_share DM is in scope for set_suggested_prompts.
+        middleware = AsyncAttachingConversationKwargs()
+        req = AsyncBoltRequest(body=file_share_im_message_event_body, mode="socket_mode")
+        req.context["client"] = AsyncWebClient(token="xoxb-test")
+
+        resp = await middleware.async_process(req=req, resp=BoltResponse(status=404), next=next)
+
+        assert resp.status == 200
+        assert "set_suggested_prompts" in req.context
+
     @pytest.mark.asyncio
     async def test_non_assistant_event_does_not_attach_kwargs(self):
         middleware = AsyncAttachingConversationKwargs()