-
Notifications
You must be signed in to change notification settings - Fork 481
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
Implementation of Teams batch APIs #6655
Conversation
/// The status map for processed users. | ||
/// </value> | ||
[JsonProperty(PropertyName = "statusMap")] | ||
public IDictionary<string, int> StatusMap { get; } = new Dictionary<string, int>(); |
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.
on our side it's Dictionary <int, int>, where key is statusCode
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.
Changed. Thanks!
/// <param name="tenantId"> The tenant ID. </param> | ||
/// <param name="cancellationToken"> The cancellation token. </param> | ||
/// <returns> The operation Id. </returns> | ||
public static async Task<string> SendMessageToListOfUsersAsync(ITurnContext turnContext, IActivity activity, List<object> teamsMembers, string tenantId, CancellationToken cancellationToken = default) |
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.
Is there a reason why we're using object and not specifying a concrete type for list of members?
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.
We created a TeamMember model with a string Id property to handle this.
{ | ||
if (operations is TeamsOperations teamsOperations) | ||
{ | ||
using (var result = await teamsOperations.SendMessageToListOfUsersAsync(activity, teamsMembers, tenantId, null, cancellationToken).ConfigureAwait(false)) |
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'm not fully familiar with the implementation here, and I'm wondering why one of the parameters is always null, when we're calling methods from teamsOperations?
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 null parameter is for customHeaders that are used for creating the request. We removed the null value in the calls because it defaults to null.
/// </summary> | ||
/// <value>The id of the failed entry.</value> | ||
[JsonProperty(PropertyName = "id")] | ||
public string Id { get; set; } |
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.
in the response we are returning property name will be called "entryId"
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.
Changed. Thanks!
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.dll |
#minor
Description
This PR implements the new Teams batch APIs in TeamsOperations.
Specific Changes
Microsoft.Bot.Connector/RetryAction
class to handle the retry logic of the operations.BatchFailedEntriesResponse
,BatchFailedEntry
, andBatchOperationState
classes in Bot.Schema/Teams to handle the operation response types.Testing
These images show some of the new operations working.