microsoft / botbuilder-python

The Microsoft Bot Framework provides what you need to build and connect intelligent bots that interact naturally wherever your users are talking, from text/sms to Skype, Slack, Office 365 mail and other popular services.
http://botframework.com
MIT License
672 stars 271 forks source link

Fix update_activity in messages #2101

Open NeeluGeorge opened 2 months ago

NeeluGeorge commented 2 months ago

closes: https://github.com/OfficeDev/Microsoft-Teams-Samples/issues/1226

Description

Python sdk documentation says to pass replyToId as updated_activity.id, but this was not getting passed to the reference created in update_activity

https://learn.microsoft.com/en-us/microsoftteams/platform/bots/how-to/update-and-delete-bot-messages?tabs=python#update-cards

Specific Changes

Added activity.id as the "activity_id" in reference

tracyboehrer commented 2 months ago

@NeeluGeorge What is the reason for this? What is it solving? 'apply_conversation_reference' does this, except conditionally.

NeeluGeorge commented 2 months ago

@tracyboehrer 'apply_conversation_reference' is using reference.activity_id But TurnContext.get_conversation_reference(self.activity) called before this will map reference.activity_id with the original activity's id field. Here we are passing self.activity in TurnContext.get_conversation_reference(self.activity) , not the activity we pass as argument

So our updated activity.id is left unused.This throws invalid ID error

tracyboehrer commented 2 months ago

@NeeluGeorge Thanks for that. I'll dig a little. I note that the Python get_conversation_reference is different from the DotNet and JS implementations.

tracyboehrer commented 1 month ago

@NeeluGeorge reference.activity_id should be TurnContext.activity.id. That happens in get_conversation_reference. But, Pythons get_conversation_reference is outdated, and that assignment should be conditional, which I will update. So doing this could potentially change that logic.

That doc states the Activity passed to update_activity should have its id set to the incoming activity.reply_to_id. Is there a value for that property?

NeeluGeorge commented 1 month ago

@NeeluGeorge reference.activity_id should be TurnContext.activity.id. That happens in get_conversation_reference. But, Pythons get_conversation_reference is outdated, and that assignment should be conditional, which I will update. So doing this could potentially change that logic.

That doc states the Activity passed to update_activity should have its id set to the incoming activity.reply_to_id. Is there a value for that property?

Yes @tracyboehrer

Sample code for updation

` async def post_updated_message(self):

    adaptive_card_attachment = CardFactory.adaptive_card(self.message.messageBody)

    updated_activity = MessageFactory.attachment(adaptive_card_attachment)

    updated_activity.id = self.turn_context.activity.reply_to_id

    await self.turn_context.update_activity(updated_activity)

`

Error:

` "Error occurred while reacting to message Error: (BadArgument) Invalid activity ID 1:1zUqrpY6kJ6PjcsmO2_1zjCfKsKLgLaF9vcA7h72Rd8QWCNetmMA6CVxiWCJdrz8m-8JkEngpiqrMSdN10rR6nA", "time": "2024-05-29T15:09:39.146814"}

` Issue in update activity:

reference = TurnContext.get_conversation_reference(self.activity)

Here self.activity is having a different id.

Only our updated_activity has id= 'reply_to_id'

But this is left unused while creating reference