Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#5676] BotFrameworkAdapter CreateConversationAsync overrides conversationParameters.ChannelData #5727

Merged

Conversation

MartinLuccanera
Copy link
Contributor

Fixes #5676

Description

This PR updates the CreateConversationAsync method to stop overriding the conversationParameters.ChannelData with the TenantId.

Detailed Changes

  • Updated BotFrameworkAdapter class to add the TenantId as a new property in conversationParameters.ChannelData instead of overriding the object.

Testing

These images show a Teams bot failing on the createConversation before the fix and working after applying it.
image

@MartinLuccanera MartinLuccanera requested a review from a team as a code owner June 23, 2021 19:58
@johnataylor
Copy link
Member

This whole method is basically redundant now right?

We don't seem to use this particular overload in samples 57 or 58 which is presumably why haven't seen this reported.

Anyhow could you also mark the method as obsolete.

Copy link
Member

@johnataylor johnataylor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method should be marked obsolete now, right?

@ceciliaavila
Copy link
Collaborator

The method should be marked obsolete now, right?

Hi @johnataylor, we confirmed that this overload is not being used in the samples and the only difference with the original method is that it uses the conversationReference to set the TenantId in the conversationParameters.
We marked it as obsolete.
Thanks.

@johnataylor johnataylor merged commit 9316bce into microsoft:main Jun 24, 2021
@ceciliaavila ceciliaavila deleted the southworks/fix/teams-start-conversation branch June 25, 2021 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BotFrameworkAdapter CreateConversationAsync overrides conversationParameters.ChannelData
3 participants