From 4dfcd68153da4f60166383db3951c9535fb09f3b Mon Sep 17 00:00:00 2001 From: Rohit Malhotra Date: Sun, 15 Mar 2026 14:23:06 -0400 Subject: [PATCH] (Hotfix): followup messages for slack conversations (#13411) Co-authored-by: openhands --- .../storage/slack_conversation_store.py | 4 +- enterprise/tests/unit/conftest.py | 1 + .../storage/test_slack_conversation_store.py | 75 +++++++++++++++++++ 3 files changed, 78 insertions(+), 2 deletions(-) create mode 100644 enterprise/tests/unit/storage/test_slack_conversation_store.py diff --git a/enterprise/storage/slack_conversation_store.py b/enterprise/storage/slack_conversation_store.py index 5fbbb0e958..53b1350df5 100644 --- a/enterprise/storage/slack_conversation_store.py +++ b/enterprise/storage/slack_conversation_store.py @@ -25,10 +25,10 @@ class SlackConversationStore: return result.scalar_one_or_none() async def create_slack_conversation( - self, slack_converstion: SlackConversation + self, slack_conversation: SlackConversation ) -> None: async with a_session_maker() as session: - session.merge(slack_converstion) + await session.merge(slack_conversation) await session.commit() @classmethod diff --git a/enterprise/tests/unit/conftest.py b/enterprise/tests/unit/conftest.py index c273f81423..e0a929a722 100644 --- a/enterprise/tests/unit/conftest.py +++ b/enterprise/tests/unit/conftest.py @@ -28,6 +28,7 @@ from storage.org import Org from storage.org_invitation import OrgInvitation # noqa: F401 from storage.org_member import OrgMember from storage.role import Role +from storage.slack_conversation import SlackConversation # noqa: F401 from storage.stored_conversation_metadata import StoredConversationMetadata from storage.stored_conversation_metadata_saas import ( StoredConversationMetadataSaas, diff --git a/enterprise/tests/unit/storage/test_slack_conversation_store.py b/enterprise/tests/unit/storage/test_slack_conversation_store.py new file mode 100644 index 0000000000..6a7e83a2b7 --- /dev/null +++ b/enterprise/tests/unit/storage/test_slack_conversation_store.py @@ -0,0 +1,75 @@ +"""Unit tests for SlackConversationStore.""" + +from unittest.mock import patch + +import pytest +from sqlalchemy import select +from storage.slack_conversation import SlackConversation +from storage.slack_conversation_store import SlackConversationStore + + +@pytest.fixture +def slack_conversation_store(): + """Create SlackConversationStore instance.""" + return SlackConversationStore() + + +class TestSlackConversationStore: + """Test cases for SlackConversationStore.""" + + @pytest.mark.asyncio + async def test_create_slack_conversation_persists_to_database( + self, slack_conversation_store, async_session_maker + ): + """Test that create_slack_conversation actually stores data in the database. + + This test verifies that the await statement is present before session.merge(). + Without the await, the data won't be persisted and subsequent lookups will + return None even though we just created the conversation. + """ + channel_id = 'C123456' + parent_id = '1234567890.123456' + conversation_id = 'conv-test-123' + keycloak_user_id = 'user-123' + + slack_conversation = SlackConversation( + conversation_id=conversation_id, + channel_id=channel_id, + keycloak_user_id=keycloak_user_id, + parent_id=parent_id, + ) + + with patch( + 'storage.slack_conversation_store.a_session_maker', async_session_maker + ): + # Create the slack conversation + await slack_conversation_store.create_slack_conversation(slack_conversation) + + # Verify we can retrieve the conversation using the store method + result = await slack_conversation_store.get_slack_conversation( + channel_id=channel_id, + parent_id=parent_id, + ) + + # This assertion would fail if the await was missing before session.merge() + # because the data wouldn't be persisted to the database + assert result is not None, ( + 'Slack conversation was not persisted to the database. ' + 'Ensure await is used before session.merge() in create_slack_conversation.' + ) + assert result.conversation_id == conversation_id + assert result.channel_id == channel_id + assert result.parent_id == parent_id + assert result.keycloak_user_id == keycloak_user_id + + # Also verify directly in the database + async with async_session_maker() as session: + db_result = await session.execute( + select(SlackConversation).where( + SlackConversation.channel_id == channel_id, + SlackConversation.parent_id == parent_id, + ) + ) + db_conversation = db_result.scalar_one_or_none() + assert db_conversation is not None + assert db_conversation.conversation_id == conversation_id