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

Add support for channelData field in conversation references #712

Merged
merged 12 commits into from
Feb 13, 2019

Conversation

benbrown
Copy link
Contributor

@benbrown benbrown commented Jan 7, 2019

Description

The conversationReference type is missing fields necessary to use createConversation with MS Teams. This PR adds those fields and amends the functions so that they will work with Teams.

Specific Changes

Adds the channelData field to the conversation reference. Information in channelData may be required for operations that use references -- for example, createConversation.

Amends the adapter's createConversation method to pass through the tenant id, which is necessary for using this method with MS Teams.

Testing

  • Connect a bot to Teams.
  • Using a conversation reference extracted from an incoming GROUP message, use adapter.createConversation to create a new PRIVATE context.
  • Use the new context to send a private message to the user. Note that it is received as a DM, instead of a reply in the group channel.

@benbrown benbrown requested a review from Stevenic January 7, 2019 18:04
@coveralls
Copy link

coveralls commented Jan 7, 2019

Pull Request Test Coverage Report for Build #1965

  • 1 of 3 (33.33%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.04%) to 87.406%

Changes Missing Coverage Covered Lines Changed/Added Lines %
libraries/botbuilder/src/botFrameworkAdapter.ts 1 3 33.33%
Totals Coverage Status
Change from base Build #1964: -0.04%
Covered Lines: 3032
Relevant Lines: 3338

💛 - Coveralls

@cleemullins
Copy link
Contributor

@benbrown, is there anything we need to pickup on the C# side for this?

@benbrown benbrown changed the title DCR: Add support for channelData field in conversation references Add support for channelData field in conversation references Jan 8, 2019
Copy link
Member

@stevengum stevengum left a comment

Choose a reason for hiding this comment

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

Except for my comment about botbuilder-schema, it looks fine to me! Once that bit is squared away feel free to merge.

* @member {any} [channelData] Channel specific payload for re-creating the
* conversation
*/
channelData?: any;
Copy link
Member

Choose a reason for hiding this comment

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

Uh-oh, this looks like it's hand-crafted! We need to chat with Dan/Jeff/someone from the other side of the house to get this change into the Swagger definitions.

Or did you regenerate the Swagger files and this code was originally missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shoot! You are right, I added this by hand.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps this information can go into the conversationReference.channel field instead. However, that would still require a change to the schema. I'll collect some feedback and regroup.

@cleemullins cleemullins added blocked Current progress is blocked on something else. Parity with C# labels Feb 7, 2019
@cleemullins
Copy link
Contributor

This PR is tied to: microsoft/botbuilder-dotnet#1362

Both should be merged at the same time.

@benbrown
Copy link
Contributor Author

benbrown commented Feb 8, 2019

OK! The service code has changed to include this new field in the schema.

We need to regenerate!

@benbrown
Copy link
Contributor Author

benbrown commented Feb 8, 2019

I have manually made the very small schema changes. These should match any future update from swagger.

@benbrown
Copy link
Contributor Author

Just waiting on one final tweak to the dot-net version, then we can merge both!

microsoft/botbuilder-dotnet#1362 (comment)

@benbrown benbrown removed blocked Current progress is blocked on something else. Parity with C# labels Feb 13, 2019
@benbrown benbrown merged commit f5f6453 into master Feb 13, 2019
@benbrown benbrown deleted the benbrown/teamsbits branch February 13, 2019 21:09
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.

5 participants