-
Notifications
You must be signed in to change notification settings - Fork 281
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
[Teams] Add TeamsInfo helper class, minor fix in ConversationUpdateBot scenario #1253
Conversation
Pull Request Test Coverage Report for Build 81742
💛 - Coveralls |
@@ -852,7 +852,7 @@ export class BotFrameworkAdapter extends BotAdapter implements IUserTokenProvide | |||
* @remarks | |||
* Override this in a derived class to create a mock connector client for unit testing. | |||
*/ | |||
protected createConnectorClient(serviceUrl: string): ConnectorClient { | |||
public createConnectorClient(serviceUrl: string): ConnectorClient { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I switched this to public because the BotFrameworkAdapter.credentials
is protected, and ConnectorClient.credentials
is not.
I also wanted the new ConnectorClient to include the USER_AGENT
, not sure if this is a problem in C#...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you not make the Credentials on ConnectorClient public - that would also bring the two implementations closer together.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The credentials in the connectorClient are public, I use them in TeamsInfo.getTeamsConnectorClient()
return new TeamsConnectorClient(connectorClient.credentials, { baseUri: context.activity.serviceUrl });
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding USER_AGENT header - the C# uses the same HttpClient as the regular connector. So that should be good.
Its really just a couple of extra REST calls on top of the current connector
and we need some additional unit tests? |
Fixes: #1214
Description
Adds the TeamsInfo helper class.
Testing
Tested manually, unit tests are coming next