Fix bug in slack tool pagination (#232)
The `get_conversation_metadata_by_name` tool retrieves conversation metadata from another tool, `list_conversations_metadata`, but was accessing the `next_cursor` using the Slack API response dict structure, instead of the tool response structure. As a result, in that tool, the tool would never actually paginate to the second page. This PR fixes it and also adjust tests to capture the issue appropriately.
This commit is contained in:
parent
778b7af83f
commit
27d8aa7f43
2 changed files with 8 additions and 8 deletions
|
|
@ -489,7 +489,7 @@ async def get_conversation_metadata_by_name(
|
||||||
|
|
||||||
while should_continue:
|
while should_continue:
|
||||||
response = await list_conversations_metadata(context, next_cursor=next_cursor)
|
response = await list_conversations_metadata(context, next_cursor=next_cursor)
|
||||||
next_cursor = response.get("response_metadata", {}).get("next_cursor")
|
next_cursor = response["next_cursor"]
|
||||||
|
|
||||||
for conversation in response["conversations"]:
|
for conversation in response["conversations"]:
|
||||||
response_conversation_name = (
|
response_conversation_name = (
|
||||||
|
|
|
||||||
|
|
@ -316,7 +316,7 @@ async def test_get_conversation_metadata_by_name(
|
||||||
sample_conversation = extract_conversation_metadata(mock_channel_info)
|
sample_conversation = extract_conversation_metadata(mock_channel_info)
|
||||||
mock_list_conversations_metadata.return_value = {
|
mock_list_conversations_metadata.return_value = {
|
||||||
"conversations": [sample_conversation],
|
"conversations": [sample_conversation],
|
||||||
"response_metadata": {"next_cursor": None},
|
"next_cursor": None,
|
||||||
}
|
}
|
||||||
|
|
||||||
response = await get_conversation_metadata_by_name(mock_context, sample_conversation["name"])
|
response = await get_conversation_metadata_by_name(mock_context, sample_conversation["name"])
|
||||||
|
|
@ -336,11 +336,11 @@ async def test_get_conversation_metadata_by_name_triggering_pagination(
|
||||||
mock_list_conversations_metadata.side_effect = [
|
mock_list_conversations_metadata.side_effect = [
|
||||||
{
|
{
|
||||||
"conversations": [another_conversation],
|
"conversations": [another_conversation],
|
||||||
"response_metadata": {"next_cursor": "123"},
|
"next_cursor": "123",
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"conversations": [target_conversation],
|
"conversations": [target_conversation],
|
||||||
"response_metadata": {"next_cursor": None},
|
"next_cursor": None,
|
||||||
},
|
},
|
||||||
]
|
]
|
||||||
|
|
||||||
|
|
@ -365,11 +365,11 @@ async def test_get_conversation_metadata_by_name_not_found(
|
||||||
mock_list_conversations_metadata.side_effect = [
|
mock_list_conversations_metadata.side_effect = [
|
||||||
{
|
{
|
||||||
"conversations": [second_conversation],
|
"conversations": [second_conversation],
|
||||||
"response_metadata": {"next_cursor": "123"},
|
"next_cursor": "123",
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"conversations": [first_conversation],
|
"conversations": [first_conversation],
|
||||||
"response_metadata": {"next_cursor": None},
|
"next_cursor": None,
|
||||||
},
|
},
|
||||||
]
|
]
|
||||||
|
|
||||||
|
|
@ -505,11 +505,11 @@ async def test_get_members_in_conversation_by_name_triggering_pagination(
|
||||||
mock_list_conversations_metadata.side_effect = [
|
mock_list_conversations_metadata.side_effect = [
|
||||||
{
|
{
|
||||||
"conversations": [extract_conversation_metadata(conversation1)],
|
"conversations": [extract_conversation_metadata(conversation1)],
|
||||||
"response_metadata": {"next_cursor": "123"},
|
"next_cursor": "123",
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"conversations": [extract_conversation_metadata(conversation2)],
|
"conversations": [extract_conversation_metadata(conversation2)],
|
||||||
"response_metadata": {"next_cursor": None},
|
"next_cursor": None,
|
||||||
},
|
},
|
||||||
]
|
]
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue